On 08/16/2016 08:36 AM, Stanislav Laznicka wrote:
On 08/02/2016 01:08 PM, Stanislav Laznicka wrote:
On 07/28/2016 10:57 AM, Martin Basti wrote:
Hello,

suprisingly, patch needs rebase :)

1)
Is the script error the right Exception?
I chose ScriptError because it's able to change the return value of the script, which is necessary sometimes. RuntimeError, which may seem more suitable, would not be able to do that. However, I am open for ideas on which exception type to use.

2)
Can you use rather raise Exception(), instead of raise Exception

Sure, please see the modified attached patch.
3)
I really hate to print errors to STDOUT from modules and then just call exit(1) (duplicated evil), could you replace print('xxx') with raise AnException('xxx')
Did that, only kept those prints printing directly to stderr. Not sure if those should be changed as well.

Bumping for review/opinions.

Also a self-NACK, there were some sys imports that should not have been there anymore (thanks, mbasti). Attaching a fixed version.

From 4617dbff9f8c358f4e528e1a8961ab7eb63cc2bd Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 17 Jun 2016 13:14:49 +0200
Subject: [PATCH] Remove sys.exit from install modules and scripts

sys.exit() calls sometimes make it hard to find bugs and mask code that
does not always work properly.

https://fedorahosted.org/freeipa/ticket/5750
---
 ipaserver/install/bindinstance.py          |   2 +-
 ipaserver/install/ca.py                    |  42 +++++-----
 ipaserver/install/cainstance.py            |   5 +-
 ipaserver/install/dns.py                   |   5 +-
 ipaserver/install/dsinstance.py            |   4 +-
 ipaserver/install/installutils.py          |  20 ++---
 ipaserver/install/ipa_ldap_updater.py      |   4 +-
 ipaserver/install/krainstance.py           |   4 +-
 ipaserver/install/replication.py           |  10 ++-
 ipaserver/install/server/install.py        |  75 ++++++++---------
 ipaserver/install/server/replicainstall.py | 129 ++++++++++++++---------------
 11 files changed, 148 insertions(+), 152 deletions(-)

diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 7538e145cbe37dfc21963d97dea0e835e3bd5072..c0a4647d0544a8aab1fa6701722d4b0ffe6163d5 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -473,7 +473,7 @@ def check_reverse_zones(ip_addresses, reverse_zones, options, unattended,
             except ValueError as e:
                 msg = "Reverse zone %s will not be used: %s" % (rz, e)
                 if unattended:
-                    sys.exit(msg)
+                    raise RuntimeError(msg)
                 else:
                     root_logger.warning(msg)
                 continue
diff --git a/ipaserver/install/ca.py b/ipaserver/install/ca.py
index bce804ac1c4e3eaf8dd08bed894ad45ea2d73ae1..00e0b038ca03320fd7b8268fb3eb96c5bc50a3ac 100644
--- a/ipaserver/install/ca.py
+++ b/ipaserver/install/ca.py
@@ -4,10 +4,9 @@
 
 from __future__ import print_function
 
-import sys
-
 from ipaserver.install import cainstance, dsinstance, bindinstance
 from ipapython import ipautil, certdb
+from ipapython.admintool import ScriptError
 from ipaplatform import services
 from ipaplatform.paths import paths
 from ipaserver.install import installutils, certs
@@ -30,12 +29,11 @@ def install_check(standalone, replica_config, options):
 
     if replica_config is not None:
         if standalone and api.env.ra_plugin == 'selfsign':
-            sys.exit('A selfsign CA can not be added')
+            raise ScriptError('A selfsign CA can not be added')
 
         if ((not options.promote
              and not ipautil.file_exists(replica_config.dir + "/cacert.p12"))):
-            print('CA cannot be installed in CA-less setup.')
-            sys.exit(1)
+            raise ScriptError('CA cannot be installed in CA-less setup.')
 
         if standalone and not options.skip_conncheck:
             principal = options.principal
@@ -53,7 +51,7 @@ def install_check(standalone, replica_config, options):
 
     if standalone:
         if api.Command.ca_is_enabled()['result']:
-            sys.exit(
+            raise ScriptError(
                 "One or more CA masters are already present in IPA realm "
                 "'%s'.\nIf you wish to replicate CA to this host, please "
                 "re-run 'ipa-ca-install'\nwith a replica file generated on "
@@ -64,28 +62,28 @@ def install_check(standalone, replica_config, options):
         if not cainstance.is_step_one_done():
             # This can happen if someone passes external_ca_file without
             # already having done the first stage of the CA install.
-            print("CA is not installed yet. To install with an external CA "
+            raise ScriptError(
+                  "CA is not installed yet. To install with an external CA "
                   "is a two-stage process.\nFirst run the installer with "
                   "--external-ca.")
-            sys.exit(1)
 
         external_cert_file, external_ca_file = installutils.load_external_cert(
             options.external_cert_files, options.subject)
     elif options.external_ca:
         if cainstance.is_step_one_done():
-            print("CA is already installed.\nRun the installer with "
-                  "--external-cert-file.")
-            sys.exit(1)
+            raise ScriptError(
+                "CA is already installed.\nRun the installer with "
+                "--external-cert-file.")
         if ipautil.file_exists(paths.ROOT_IPA_CSR):
-            print(("CA CSR file %s already exists.\nIn order to continue "
-                  "remove the file and run the installer again." %
-                  paths.ROOT_IPA_CSR))
-            sys.exit(1)
+            raise ScriptError(
+                "CA CSR file %s already exists.\nIn order to continue "
+                "remove the file and run the installer again." %
+                paths.ROOT_IPA_CSR)
 
     if not options.external_cert_files:
         if not cainstance.check_port():
             print("IPA requires port 8443 for PKI but it is currently in use.")
-            sys.exit("Aborting installation")
+            raise ScriptError("Aborting installation")
 
     if standalone:
         dirname = dsinstance.config_dirname(
@@ -98,9 +96,9 @@ def install_check(standalone, replica_config, options):
                 if nickname in (certdb.get_ca_nickname(realm_name),
                                 'ipaCert',
                                 'Signing-Cert'):
-                    print(("Certificate with nickname %s is present in %s, "
-                           "cannot continue." % (nickname, db.secdir)))
-                    sys.exit(1)
+                    raise ScriptError(
+                        "Certificate with nickname %s is present in %s, "
+                        "cannot continue." % (nickname, db.secdir))
 
                 cert = db.get_cert_from_db(nickname)
                 if not cert:
@@ -109,9 +107,9 @@ def install_check(standalone, replica_config, options):
                 if subject in (DN('CN=Certificate Authority', subject_base),
                                DN('CN=IPA RA', subject_base),
                                DN('CN=Object Signing Cert', subject_base)):
-                    print(("Certificate with subject %s is present in %s, "
-                           "cannot continue." % (subject, db.secdir)))
-                    sys.exit(1)
+                    raise ScriptError(
+                        "Certificate with subject %s is present in %s, "
+                        "cannot continue." % (subject, db.secdir))
 
 
 def install(standalone, replica_config, options):
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 070498fe8a394802ea55f848a268e2b6563ec472..2ec02d6628ebc9e3a9bad141ec636c84eab14cef 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -60,6 +60,7 @@ from ipapython.certdb import get_ca_nickname
 from ipapython.dn import DN
 from ipapython.ipa_log_manager import log_mgr,\
     standard_logging_setup, root_logger
+from ipapython.admintool import ScriptError
 from ipapython.secrets.kem import IPAKEMKeys
 
 from ipaserver.install import certs
@@ -590,7 +591,7 @@ class CAInstance(DogtagInstance):
         if self.external == 1:
             print("The next step is to get %s signed by your CA and re-run %s as:" % (self.csr_file, sys.argv[0]))
             print("%s --external-cert-file=/path/to/signed_certificate --external-cert-file=/path/to/external_ca_certificate" % sys.argv[0])
-            sys.exit(0)
+            raise ScriptError(rval=0)
         else:
             shutil.move(paths.CA_BACKUP_KEYS_P12,
                         paths.CACERT_P12)
@@ -1517,7 +1518,7 @@ def install_replica_ca(config, postinstall=False, ra_p12=None):
         return ca
 
     if ca.is_installed():
-        sys.exit("A CA is already configured on this system.")
+        raise ScriptError("A CA is already configured on this system.")
 
     if postinstall:
         # If installing this afterward the Apache NSS database already
diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index 44ebd39dfa7f1d947061c3b4c0347242f8502be0..fe662741ebda06c32c76f40fa11e124c1f88e126 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -22,6 +22,7 @@ from ipapython import sysrestore
 from ipapython import dnsutil
 from ipapython.dn import DN
 from ipapython.ipa_log_manager import root_logger
+from ipapython.admintool import ScriptError
 from ipapython.ipaldap import AUTOBIND_ENABLED
 from ipapython.ipautil import user_input
 from ipaserver.install.installutils import get_server_ip_address
@@ -207,8 +208,8 @@ def install_check(standalone, api, replica, options, hostname):
         # we can reinstall current server if it is dnssec master
         if dnssec_masters and api.env.host not in dnssec_masters:
             print("DNSSEC key master(s):", u','.join(dnssec_masters))
-            sys.exit("Only one DNSSEC key master is supported in current "
-                     "version.")
+            raise ScriptError(
+                "Only one DNSSEC key master is supported in current version.")
 
         if options.kasp_db_file:
             dnskeysyncd = services.service('ipa-dnskeysyncd')
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index c93b3b4ff58c4102a9de448247966ad3dd8e4e7c..f29737fc1757cb8ef2ee01f1ca808ce858c9d47e 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -22,7 +22,6 @@ from __future__ import print_function
 
 import shutil
 import pwd
-import sys
 import os
 import re
 import time
@@ -48,6 +47,7 @@ from ipaplatform.constants import constants as platformconstants
 from ipaplatform.tasks import tasks
 from ipalib.constants import CACERT
 from ipapython.dn import DN
+from ipapython.admintool import ScriptError
 from ipaplatform import services
 from ipaplatform.paths import paths
 
@@ -620,7 +620,7 @@ class DsInstance(service.Service):
             super(DsInstance, self).restart(instance)
             if not is_ds_running(instance):
                 root_logger.critical("Failed to restart the directory server. See the installation log for details.")
-                sys.exit(1)
+                raise ScriptError()
         except SystemExit as e:
             raise e
         except Exception as e:
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 66ba33326adcdb47c2ba77c573ba9b66a82b365e..7578bf8f5c0ed358014e39914ce37b65a778ea48 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -502,7 +502,7 @@ def get_server_ip_address(host_name, unattended, setup_dns, ip_addresses):
         print("The KDC service does not listen on localhost", file=sys.stderr)
         print("", file=sys.stderr)
         print("Please fix your /etc/hosts file and restart the setup program", file=sys.stderr)
-        sys.exit(1)
+        raise ScriptError()
 
     ips = []
     if len(hostaddr):
@@ -529,11 +529,11 @@ def get_server_ip_address(host_name, unattended, setup_dns, ip_addresses):
                 print("or /etc/hosts file and restart the installation.", file=sys.stderr)
                 print("Provided but not resolved address(es): %s" % \
                                     ", ".join(str(ip) for ip in (set(ip_addresses) - set(ips))), file=sys.stderr)
-                sys.exit(1)
+                raise ScriptError()
 
     if not ips:
         print("No usable IP address provided nor resolved.", file=sys.stderr)
-        sys.exit(1)
+        raise ScriptError()
 
     for ip_address in ips:
         # check /etc/hosts sanity
@@ -548,7 +548,7 @@ def get_server_ip_address(host_name, unattended, setup_dns, ip_addresses):
                 print("Chosen hostname %s does not match configured canonical hostname %s" \
                         % (host_name, primary_host), file=sys.stderr)
                 print("Please fix your /etc/hosts file and restart the installation.", file=sys.stderr)
-                sys.exit(1)
+                raise ScriptError()
 
     return ips
 
@@ -627,9 +627,9 @@ def create_replica_config(dirman_password, filename, options):
         top_dir, dir = expand_replica_info(filename, dirman_password)
     except Exception as e:
         root_logger.error("Failed to decrypt or open the replica file.")
-        print("ERROR: Failed to decrypt or open the replica file.")
-        print("Verify you entered the correct Directory Manager password.")
-        sys.exit(1)
+        raise ScriptError(
+            "ERROR: Failed to decrypt or open the replica file.\n"
+            "Verify you entered the correct Directory Manager password.")
     config = ReplicaConfig(top_dir)
     read_replica_info(dir, config)
     root_logger.debug(
@@ -639,13 +639,13 @@ def create_replica_config(dirman_password, filename, options):
         root_logger.error(
             'A replica file from a newer release (%d) cannot be installed on an older version (%d)',
             config.version, version.NUM_VERSION)
-        sys.exit(1)
+        raise ScriptError()
     config.dirman_password = dirman_password
     try:
         host = get_host_name(options.no_host_dns)
     except BadHostError as e:
         root_logger.error(str(e))
-        sys.exit(1)
+        raise ScriptError()
     if config.host_name != host:
         try:
             print("This replica was created for '%s' but this machine is named '%s'" % (config.host_name, host))
@@ -659,7 +659,7 @@ def create_replica_config(dirman_password, filename, options):
             print("")
         except KeyboardInterrupt:
             root_logger.debug("Keyboard Interrupt")
-            sys.exit(0)
+            raise ScriptError(rval=0)
     config.dir = dir
     config.ca_ds_port = read_replica_info_dogtag_port(config.dir)
     return config
diff --git a/ipaserver/install/ipa_ldap_updater.py b/ipaserver/install/ipa_ldap_updater.py
index 2f91a830ff5f8fd9e61f47b4df963118102f7a02..8b648264886e45ab3ee53c59c7ac22daf3549bc3 100644
--- a/ipaserver/install/ipa_ldap_updater.py
+++ b/ipaserver/install/ipa_ldap_updater.py
@@ -26,7 +26,6 @@
 from __future__ import print_function
 
 import os
-import sys
 
 import six
 
@@ -82,8 +81,7 @@ class LDAPUpdater(admintool.AdminTool):
         try:
             installutils.check_server_configuration()
         except RuntimeError as e:
-            print(unicode(e))
-            sys.exit(1)
+            raise admintool.ScriptError(e)
 
     def setup_logging(self):
         super(LDAPUpdater, self).setup_logging(log_file_mode='a')
diff --git a/ipaserver/install/krainstance.py b/ipaserver/install/krainstance.py
index dc44726887916c7216564679c6ea8e9902177b64..590a8407d76a1c54dd2323986a8981b7c4851daa 100644
--- a/ipaserver/install/krainstance.py
+++ b/ipaserver/install/krainstance.py
@@ -20,7 +20,6 @@
 import os
 import pwd
 import shutil
-import sys
 import tempfile
 
 from six.moves.configparser import ConfigParser
@@ -33,6 +32,7 @@ from ipaplatform.paths import paths
 from ipapython import certdb
 from ipapython import ipautil
 from ipapython.dn import DN
+from ipapython.admintool import ScriptError
 from ipaserver.install import certs
 from ipaserver.install import cainstance
 from ipaserver.install import installutils
@@ -425,7 +425,7 @@ def install_replica_kra(config, postinstall=False):
     _kra.dm_password = config.dirman_password
     _kra.subject_base = config.subject_base
     if _kra.is_installed():
-        sys.exit("A KRA is already configured on this system.")
+        raise ScriptError("A KRA is already configured on this system.")
 
     _kra.configure_instance(config.realm_name, config.host_name,
                             config.dirman_password, config.dirman_password,
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index b8b665267ea8debba9f0ce01f54a78cd67d88292..56c75e709ab74c32433c97614dba037d1a193236 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -33,6 +33,7 @@ from ipalib.cli import textui
 from ipalib.constants import CACERT
 from ipapython.ipa_log_manager import root_logger
 from ipapython import ipautil, ipaldap
+from ipapython.admintool import ScriptError
 from ipapython.dn import DN
 from ipaplatform import services
 from ipaplatform.paths import paths
@@ -76,7 +77,7 @@ def replica_conn_check(master_host, host_name, realm, check_ca,
     Check the ports used by the replica both locally and remotely to be sure
     that replication will work.
 
-    Does not return a value, will sys.exit() on failure.
+    Does not return a value, will raise ScriptError on failure.
     """
     print("Run connection check to master")
     args = [paths.IPA_REPLICA_CONNCHECK, "--master", master_host,
@@ -101,9 +102,10 @@ def replica_conn_check(master_host, host_name, realm, check_ca,
         args, raiseonerr=False, capture_output=False, nolog=nolog)
 
     if result.returncode != 0:
-        sys.exit("Connection check failed!" +
-                 "\nPlease fix your network settings according to error messages above." +
-                 "\nIf the check results are not valid it can be skipped with --skip-conncheck parameter.")
+        raise ScriptError(
+            "Connection check failed!"
+            "\nPlease fix your network settings according to error messages above."
+            "\nIf the check results are not valid it can be skipped with --skip-conncheck parameter.")
     else:
         print("Connection check OK")
 
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 94698898934844350488d5fc52d6e1e567624502..8dc7a6820d214f15fb80fef29c7296fa237bc2a9 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -25,6 +25,7 @@ from ipapython.ipa_log_manager import root_logger
 from ipapython.ipautil import (
     decrypt_file, format_netloc, ipa_generate_password, run, user_input,
     is_fips_enabled)
+from ipapython.admintool import ScriptError
 from ipaplatform import services
 from ipaplatform.paths import paths
 from ipaplatform.tasks import tasks
@@ -192,9 +193,8 @@ def read_realm_name(domain_name, unattended):
         print("An upper-case realm name is required.")
         if not user_input("Do you want to use " + upper_dom +
                           " as realm name?", True):
-            print("")
-            print("An upper-case realm name is required. Unable to continue.")
-            sys.exit(1)
+            raise ScriptError(
+                "An upper-case realm name is required. Unable to continue.")
         else:
             realm_name = upper_dom
         print("")
@@ -230,13 +230,13 @@ def read_admin_password():
 def check_dirsrv(unattended):
     (ds_unsecure, ds_secure) = dsinstance.check_ports()
     if not ds_unsecure or not ds_secure:
-        print("IPA requires ports 389 and 636 for the Directory Server.")
-        print("These are currently in use:")
+        msg = ("IPA requires ports 389 and 636 for the Directory Server.\n"
+               "These are currently in use:\n")
         if not ds_unsecure:
-            print("\t389")
+            msg += "\t389\n"
         if not ds_secure:
-            print("\t636")
-        sys.exit(1)
+            msg += "\t636\n"
+        raise ScriptError(msg)
 
 
 def set_subject_in_config(realm_name, dm_password, suffix, subject_base):
@@ -278,7 +278,7 @@ def common_cleanup(func):
                         root_logger.error("Failed to remove DS instance. You "
                                           "may need to remove instance data "
                                           "manually")
-            sys.exit(1)
+            raise ScriptError()
         finally:
             if not success and installer._installation_cleanup:
                 # Do a cautious clean up as we don't know what failed and
@@ -341,16 +341,18 @@ def install_check(installer):
     if (not options.external_ca and not options.external_cert_files and
             is_ipa_configured()):
         installer._installation_cleanup = False
-        sys.exit("IPA server is already configured on this system.\n"
-                 "If you want to reinstall the IPA server, please uninstall "
-                 "it first using 'ipa-server-install --uninstall'.")
+        raise ScriptError(
+            "IPA server is already configured on this system.\n"
+            "If you want to reinstall the IPA server, please uninstall "
+            "it first using 'ipa-server-install --uninstall'.")
 
     client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
     if client_fstore.has_files():
         installer._installation_cleanup = False
-        sys.exit("IPA client is already configured on this system.\n"
-                 "Please uninstall it before configuring the IPA server, "
-                 "using 'ipa-client-install --uninstall'")
+        raise ScriptError(
+            "IPA client is already configured on this system.\n"
+            "Please uninstall it before configuring the IPA server, "
+            "using 'ipa-client-install --uninstall'")
 
     fstore = sysrestore.FileStore(SYSRESTORE_DIR_PATH)
     sstore = sysrestore.StateFile(SYSRESTORE_DIR_PATH)
@@ -362,7 +364,7 @@ def install_check(installer):
         else:
             dm_password = read_password("Directory Manager", confirm=False)
         if dm_password is None:
-            sys.exit("Directory Manager password required")
+            raise ScriptError("Directory Manager password required")
         try:
             cache_vars = read_cache(dm_password)
             options.__dict__.update(cache_vars)
@@ -370,7 +372,7 @@ def install_check(installer):
                 options.external_ca = False
                 options.interactive = False
         except Exception as e:
-            sys.exit("Cannot process the cache file: %s" % str(e))
+            raise ScriptError("Cannot process the cache file: %s" % str(e))
 
     # We only set up the CA if the PKCS#12 options are not given.
     if options.dirsrv_cert_files:
@@ -425,7 +427,7 @@ def install_check(installer):
 
     # Check to see if httpd is already configured to listen on 443
     if httpinstance.httpd_443_configured():
-        sys.exit("Aborting installation")
+        raise ScriptError("Aborting installation")
 
     if not options.setup_dns and installer.interactive:
         if ipautil.user_input("Do you want to configure integrated DNS "
@@ -455,7 +457,7 @@ def install_check(installer):
         else:
             host_name = read_host_name(host_default, options.no_host_dns)
     except BadHostError as e:
-        sys.exit(str(e) + "\n")
+        raise ScriptError(e)
 
     host_name = host_name.lower()
     root_logger.debug("will use host_name: %s\n" % host_name)
@@ -467,7 +469,7 @@ def install_check(installer):
         try:
             validate_domain_name(domain_name)
         except ValueError as e:
-            sys.exit("Invalid domain name: %s" % unicode(e))
+            raise ScriptError("Invalid domain name: %s" % unicode(e))
     else:
         domain_name = options.domain_name
 
@@ -488,7 +490,7 @@ def install_check(installer):
                 "Enter Apache Server private key unlock",
                 confirm=False, validate=False)
             if options.http_pin is None:
-                sys.exit(
+                raise ScriptError(
                     "Apache Server private key unlock password required")
         http_pkcs12_file, http_pin, http_ca_cert = load_pkcs12(
             cert_files=options.http_cert_files,
@@ -504,7 +506,7 @@ def install_check(installer):
                 "Enter Directory Server private key unlock",
                 confirm=False, validate=False)
             if options.dirsrv_pin is None:
-                sys.exit(
+                raise ScriptError(
                     "Directory Server private key unlock password required")
         dirsrv_pkcs12_file, dirsrv_pin, dirsrv_ca_cert = load_pkcs12(
             cert_files=options.dirsrv_cert_files,
@@ -520,7 +522,7 @@ def install_check(installer):
                 "Enter Kerberos KDC private key unlock",
                 confirm=False, validate=False)
             if options.pkinit_pin is None:
-                sys.exit(
+                raise ScriptError(
                     "Kerberos KDC private key unlock password required")
         pkinit_pkcs12_file, pkinit_pin, pkinit_ca_cert = load_pkcs12(
             cert_files=options.pkinit_cert_files,
@@ -532,14 +534,15 @@ def install_check(installer):
 
     if (options.http_cert_files and options.dirsrv_cert_files and
             http_ca_cert != dirsrv_ca_cert):
-        sys.exit("Apache Server SSL certificate and Directory Server SSL "
-                 "certificate are not signed by the same CA certificate")
+        raise ScriptError(
+            "Apache Server SSL certificate and Directory Server SSL "
+            "certificate are not signed by the same CA certificate")
 
     if not options.dm_password:
         dm_password = read_dm_password()
 
         if dm_password is None:
-            sys.exit("Directory Manager password required")
+            raise ScriptError("Directory Manager password required")
     else:
         dm_password = options.dm_password
 
@@ -551,7 +554,7 @@ def install_check(installer):
     if not options.admin_password:
         admin_password = read_admin_password()
         if admin_password is None:
-            sys.exit("IPA admin password required")
+            raise ScriptError("IPA admin password required")
     else:
         admin_password = options.admin_password
 
@@ -644,7 +647,7 @@ def install_check(installer):
 
     if installer.interactive and not user_input(
             "Continue to configure the system with these values?", False):
-        sys.exit("Installation aborted")
+        raise ScriptError("Installation aborted")
 
     options.realm_name = realm_name
     options.domain_name = domain_name
@@ -892,8 +895,8 @@ def install(installer):
             args.append("--mkhomedir")
         run(args, redirect_output=True)
         print()
-    except Exception as e:
-        sys.exit("Configuration of client side components failed!")
+    except Exception:
+        raise ScriptError("Configuration of client side components failed!")
 
     # Everything installed properly, activate ipa service.
     services.knownservices.ipa.enable()
@@ -977,9 +980,7 @@ def uninstall_check(installer):
               "and configuration!\n")
         if not user_input("Are you sure you want to continue with the "
                           "uninstall procedure?", False):
-            print("")
-            print("Aborting uninstall operation.")
-            sys.exit(1)
+            raise ScriptError("Aborting uninstall operation.")
 
     try:
         conn = ipaldap.IPAdmin(
@@ -1003,9 +1004,7 @@ def uninstall_check(installer):
         if (installer.interactive and not user_input(
                 "Are you sure you want to continue with the uninstall "
                 "procedure?", False)):
-            print("")
-            print("Aborting uninstall operation.")
-            sys.exit(1)
+            raise ScriptError("Aborting uninstall operation.")
     else:
         dns.uninstall_check(options)
 
@@ -1034,9 +1033,7 @@ def uninstall_check(installer):
                 if (installer.interactive and
                         not user_input("Are you sure you want to continue with"
                                        " the uninstall procedure?", False)):
-                    print("")
-                    print("Aborting uninstall operation.")
-                    sys.exit(1)
+                    raise ScriptError("Aborting uninstall operation.")
         else:
             remove_master_from_managed_topology(api, options)
 
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index f54ff7da06c57b9c8251429cbdacc5c300805f84..c73600ccad4ededd6f5b17dd5a35479af9093166 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -13,7 +13,6 @@ import dns.reversename as dnsreversename
 import os
 import shutil
 import socket
-import sys
 import tempfile
 
 import six
@@ -23,6 +22,7 @@ from ipapython.dn import DN
 from ipapython.install.common import step
 from ipapython.install.core import Knob
 from ipapython.ipa_log_manager import root_logger
+from ipapython.admintool import ScriptError
 from ipaplatform import services
 from ipaplatform.tasks import tasks
 from ipaplatform.paths import paths
@@ -157,8 +157,7 @@ def install_ca_cert(ldap, base_dn, realm, cafile):
 
         os.chmod(constants.CACERT, 0o444)
     except Exception as e:
-        print("error copying files: " + str(e))
-        sys.exit(1)
+        raise ScriptError("error copying files: " + str(e))
 
 
 def install_http(config, auto_redirect, ca_is_configured, promote=False,
@@ -225,13 +224,13 @@ def install_dns_records(config, options, remote_api):
 def check_dirsrv():
     (ds_unsecure, ds_secure) = dsinstance.check_ports()
     if not ds_unsecure or not ds_secure:
-        print("IPA requires ports 389 and 636 for the Directory Server.")
-        print("These are currently in use:")
+        msg = ("IPA requires ports 389 and 636 for the Directory Server.\n"
+               "These are currently in use:\n")
         if not ds_unsecure:
-            print("\t389")
+            msg += "\t389\n"
         if not ds_secure:
-            print("\t636")
-        sys.exit(1)
+            msg += "\t636\n"
+        raise ScriptError(msg)
 
 
 def check_dns_resolution(host_name, dns_servers):
@@ -329,8 +328,8 @@ def configure_certmonger():
     try:
         messagebus.start()
     except Exception as e:
-        print("Messagebus service unavailable: %s" % str(e))
-        sys.exit(3)
+        raise ScriptError("Messagebus service unavailable: %s" % str(e),
+                          rval=3)
 
     # Ensure that certmonger has been started at least once to generate the
     # cas files in /var/lib/certmonger/cas.
@@ -338,14 +337,14 @@ def configure_certmonger():
     try:
         cmonger.restart()
     except Exception as e:
-        print("Certmonger service unavailable: %s" % str(e))
-        sys.exit(3)
+        raise ScriptError("Certmonger service unavailable: %s" % str(e),
+                          rval=3)
 
     try:
         cmonger.enable()
     except Exception as e:
-        print("Failed to enable Certmonger: %s" % str(e))
-        sys.exit(3)
+        raise ScriptError("Failed to enable Certmonger: %s" % str(e),
+                          rval=3)
 
 
 def remove_replica_info_dir(installer):
@@ -366,7 +365,7 @@ def common_cleanup(func):
                 remove_replica_info_dir(installer)
                 raise
         except KeyboardInterrupt:
-            sys.exit(1)
+            raise ScriptError()
         except Exception:
             print(
                 "Your system may be partly configured.\n"
@@ -509,15 +508,17 @@ def install_check(installer):
     tasks.check_selinux_status()
 
     if is_ipa_configured():
-        sys.exit("IPA server is already configured on this system.\n"
-                 "If you want to reinstall the IPA server, please uninstall "
-                 "it first using 'ipa-server-install --uninstall'.")
+        raise ScriptError(
+            "IPA server is already configured on this system.\n"
+            "If you want to reinstall the IPA server, please uninstall "
+            "it first using 'ipa-server-install --uninstall'.")
 
     client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
     if client_fstore.has_files():
-        sys.exit("IPA client is already configured on this system.\n"
-                 "Please uninstall it first before configuring the replica, "
-                 "using 'ipa-client-install --uninstall'.")
+        raise ScriptError(
+            "IPA client is already configured on this system.\n"
+            "Please uninstall it first before configuring the replica, "
+            "using 'ipa-client-install --uninstall'.")
 
     sstore = sysrestore.StateFile(paths.SYSRESTORE)
 
@@ -525,7 +526,7 @@ def install_check(installer):
 
     # Check to see if httpd is already configured to listen on 443
     if httpinstance.httpd_443_configured():
-        sys.exit("Aborting installation")
+        raise ScriptError("Aborting installation")
 
     check_dirsrv()
 
@@ -546,9 +547,9 @@ def install_check(installer):
         try:
             dirman_password = get_dirman_password()
         except KeyboardInterrupt:
-            sys.exit(0)
+            raise ScriptError(rval=0)
         if dirman_password is None:
-            sys.exit("Directory Manager password required")
+            raise ScriptError("Directory Manager password required")
 
     config = create_replica_config(dirman_password, filename, options)
     installer._top_dir = config.top_dir
@@ -644,12 +645,12 @@ def install_check(installer):
         if replman.get_replication_agreement(config.host_name):
             root_logger.info('Error: A replication agreement for this '
                              'host already exists.')
-            print('A replication agreement for this host already exists. '
-                  'It needs to be removed.')
-            print("Run this on the master that generated the info file:")
-            print(("    %% ipa-replica-manage del %s --force" %
-                  config.host_name))
-            sys.exit(3)
+            msg = ("A replication agreement for this host already exists. "
+                   "It needs to be removed.\n"
+                   "Run this on the master that generated the info file:\n"
+                   "    %% ipa-replica-manage del %s --force" %
+                   config.host_name)
+            raise ScriptError(msg, rval=3)
 
         # Detect the current domain level
         try:
@@ -680,8 +681,7 @@ def install_check(installer):
                        "this version is allowed to be installed "
                        "within this domain.")
             root_logger.error(message)
-            print(message)
-            sys.exit(3)
+            raise ScriptError(message, rval=3)
 
         # Check pre-existing host entry
         try:
@@ -693,11 +693,11 @@ def install_check(installer):
         else:
             root_logger.info('Error: Host %s already exists on the master '
                              'server.' % config.host_name)
-            print(('The host %s already exists on the master server.' %
-                  config.host_name))
-            print("You should remove it before proceeding:")
-            print("    %% ipa host-del %s" % config.host_name)
-            sys.exit(3)
+            msg = ("The host %s already exists on the master server.\n"
+                   "You should remove it before proceeding:\n"
+                   "    %% ipa host-del %s" %
+                   (config.host_name, config.host_name))
+            raise ScriptError(msg, rval=3)
 
         dns_masters = remote_api.Object['dnsrecord'].get_dns_masters()
         if dns_masters:
@@ -709,7 +709,7 @@ def install_check(installer):
                     check_dns_resolution(config.host_name, dns_masters))
                 if not resolution_ok and installer.interactive:
                     if not ipautil.user_input("Continue?", False):
-                        sys.exit(0)
+                        raise ScriptError(rval=0)
         else:
             root_logger.debug('No IPA DNS servers, '
                               'skipping forward/reverse resolution check')
@@ -724,8 +724,7 @@ def install_check(installer):
             try:
                 kra.install_check(remote_api, config, options)
             except RuntimeError as e:
-                print(str(e))
-                sys.exit(1)
+                raise ScriptError(e)
 
         if options.setup_dns:
             dns.install_check(False, remote_api, True, options,
@@ -737,11 +736,11 @@ def install_check(installer):
                 options.ip_addresses)
 
     except errors.ACIError:
-        sys.exit("\nThe password provided is incorrect for LDAP server "
-                 "%s" % config.master_host_name)
+        raise ScriptError("\nThe password provided is incorrect for LDAP server "
+                          "%s" % config.master_host_name)
     except errors.LDAPError:
-        sys.exit("\nUnable to connect to LDAP server %s" %
-                 config.master_host_name)
+        raise ScriptError("\nUnable to connect to LDAP server %s" %
+                          config.master_host_name)
     finally:
         if replman and replman.conn:
             replman.conn.unbind()
@@ -955,7 +954,7 @@ def ensure_enrolled(installer):
         ipautil.run(args, stdin=stdin, redirect_output=True)
         print()
     except Exception:
-        sys.exit("Configuration of client side components failed!")
+        raise ScriptError("Configuration of client side components failed!")
 
 
 def promotion_check_ipa_domain(master_ldap_conn, basedn):
@@ -995,9 +994,10 @@ def promote_check(installer):
     tasks.check_selinux_status()
 
     if is_ipa_configured():
-        sys.exit("IPA server is already configured on this system.\n"
-                 "If you want to reinstall the IPA server, please uninstall "
-                 "it first using 'ipa-server-install --uninstall'.")
+        raise ScriptError(
+            "IPA server is already configured on this system.\n"
+            "If you want to reinstall the IPA server, please uninstall "
+            "it first using 'ipa-server-install --uninstall'.")
 
     client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
     if not client_fstore.has_files():
@@ -1015,7 +1015,7 @@ def promote_check(installer):
 
     # Check to see if httpd is already configured to listen on 443
     if httpinstance.httpd_443_configured():
-        sys.exit("Aborting installation")
+        raise ScriptError("Aborting installation")
 
     check_dirsrv()
 
@@ -1056,7 +1056,7 @@ def promote_check(installer):
                 "Enter Apache Server private key unlock",
                 confirm=False, validate=False)
             if options.http_pin is None:
-                sys.exit(
+                raise ScriptError(
                     "Apache Server private key unlock password required")
         http_pkcs12_file, http_pin, http_ca_cert = load_pkcs12(
             cert_files=options.http_cert_files,
@@ -1072,7 +1072,7 @@ def promote_check(installer):
                 "Enter Directory Server private key unlock",
                 confirm=False, validate=False)
             if options.dirsrv_pin is None:
-                sys.exit(
+                raise ScriptError(
                     "Directory Server private key unlock password required")
         dirsrv_pkcs12_file, dirsrv_pin, dirsrv_ca_cert = load_pkcs12(
             cert_files=options.dirsrv_cert_files,
@@ -1088,7 +1088,7 @@ def promote_check(installer):
                 "Enter Kerberos KDC private key unlock",
                 confirm=False, validate=False)
             if options.pkinit_pin is None:
-                sys.exit(
+                raise ScriptError(
                     "Kerberos KDC private key unlock password required")
         pkinit_pkcs12_file, pkinit_pin, pkinit_ca_cert = load_pkcs12(
             cert_files=options.pkinit_cert_files,
@@ -1203,7 +1203,7 @@ def promote_check(installer):
             print("Run this command:")
             print("    %% ipa-replica-manage del %s --force" %
                   config.host_name)
-            sys.exit(3)
+            raise ScriptError(rval=3)
 
         # Detect if current level is out of supported range
         # for this IPA version
@@ -1218,7 +1218,7 @@ def promote_check(installer):
                        "this version is allowed to be installed "
                        "within this domain.")
             root_logger.error(message)
-            sys.exit(3)
+            raise ScriptError(rval=3)
 
         # Detect if the other master can handle replication managers
         # cn=replication managers,cn=sysaccounts,cn=etc,$SUFFIX
@@ -1234,7 +1234,7 @@ def promote_check(installer):
                    "command on the master and use a prep file to install "
                    "this replica.")
             root_logger.error(msg)
-            sys.exit(3)
+            raise ScriptError(rval=3)
 
         dns_masters = remote_api.Object['dnsrecord'].get_dns_masters()
         if dns_masters:
@@ -1246,7 +1246,7 @@ def promote_check(installer):
                     check_dns_resolution(config.host_name, dns_masters))
                 if not resolution_ok and installer.interactive:
                     if not ipautil.user_input("Continue?", False):
-                        sys.exit(0)
+                        raise ScriptError(rval=0)
         else:
             root_logger.debug('No IPA DNS servers, '
                               'skipping forward/reverse resolution check')
@@ -1264,7 +1264,7 @@ def promote_check(installer):
             if options.dirsrv_cert_files:
                 root_logger.error("Certificates could not be provided when "
                                   "CA is present on some master.")
-                sys.exit(3)
+                raise ScriptError(rval=3)
         else:
             ca_enabled = False
             if not options.dirsrv_cert_files:
@@ -1272,20 +1272,20 @@ def promote_check(installer):
                                   "installed. Use the --http-cert-file, "
                                   "--dirsrv-cert-file options to provide "
                                   "custom certificates.")
-                sys.exit(3)
+                raise ScriptError(rval=3)
 
         config.kra_host_name = service.find_providing_server('KRA', conn,
                                                              api.env.server)
         if options.setup_kra and config.kra_host_name is None:
             root_logger.error("There is no KRA server in the domain, can't "
                               "setup a KRA clone")
-            sys.exit(3)
+            raise ScriptError(rval=3)
 
         if options.setup_ca:
             if not ca_enabled:
                 root_logger.error("The remote master does not have a CA "
                                   "installed, can't set up CA")
-                sys.exit(3)
+                raise ScriptError(rval=3)
 
             options.realm_name = config.realm_name
             options.host_name = config.host_name
@@ -1296,8 +1296,7 @@ def promote_check(installer):
             try:
                 kra.install_check(remote_api, config, options)
             except RuntimeError as e:
-                print(str(e))
-                sys.exit(1)
+                raise ScriptError(e)
 
         if options.setup_dns:
             dns.install_check(False, remote_api, True, options,
@@ -1308,10 +1307,10 @@ def promote_check(installer):
                 False, options.ip_addresses)
 
     except errors.ACIError:
-        sys.exit("\nInsufficient privileges to promote the server.")
+        raise ScriptError("\nInsufficient privileges to promote the server.")
     except errors.LDAPError:
-        sys.exit("\nUnable to connect to LDAP server %s" %
-                 config.master_host_name)
+        raise ScriptError("\nUnable to connect to LDAP server %s" %
+                          config.master_host_name)
     finally:
         if replman and replman.conn:
             replman.conn.unbind()
-- 
2.7.4

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to