On 07/28/2016 10:57 AM, Martin Basti wrote:
On 17.06.2016 13:56, Stanislav Laznicka wrote:
On 06/17/2016 01:01 PM, Petr Vobornik wrote:
On 17.6.2016 12:12, Stanislav Laznicka wrote:
On 06/17/2016 09:51 AM, Petr Vobornik wrote:
On 17.6.2016 09:24, Stanislav Laznicka wrote:
On 06/17/2016 08:48 AM, Petr Spacek wrote:
On 17.6.2016 08:43, Stanislav Laznicka wrote:
On 06/17/2016 07:45 AM, Petr Spacek wrote:
On 16.6.2016 17:33, Stanislav Laznicka wrote:
Hello,

This patch removes most sys.exits() from installer modules and
scripts and
replaces them with ScriptError. I only left sys.exits at places
where the user
decides yes/no on continuation of the script.
I wonder if yes/no should be replaced with KeyboardInterrupt or some
other
exception, too...

I'm not sure, it seems more clear to just really exit if the user
desires it
and it's what we say we'll do (with possible cleanup beforehand). Do
you think
we could benefit somehow by raising an exception here?
I'm just thinking out loud.

It seemed to me that it is easier to share cleanup on one except block
instead
of having if (interrupt): cleanup; if (interrupt2): same_cleanup;

etc.

Again, just wondering out loud.

If the cleanup is the same, or similar it might be more beneficial to have it in a function where you could pass what was set up already and therefore needs cleanup. But that's just an opinion coming from thinking out loud as well. I went through the code to see if there's much cleanup after these user actions and it seems that usually there's nothing much if anything. However, thinking in advance may save us much trouble in
the future, of course.

Btw the original scope of the ticket is to replace sys.exit calls ONLY in installer modules. Please don't waste time with debugging other use
cases before 4.4 is out.

I might have gotten carried away a bit. Would you suggest keeping the
sys.exits replaced only in ipaserver/install/server/replicainstall.py,
ipaserver/install/server/install.py modules which are the installer
modules managed by AdminTool? I considered the modules in
ipaserver/install/ to also be installer modules as they are heavily used
during installation and the sys.exits there mainly occur in functions
called from install()/install_check() methods. The *-install scripts
were perhaps really obviously over the scope.
Yes, modules:
  ipaserver/install/bindinstance.py          |  2 +-
  ipaserver/install/ca.py                    | 19 +++---
  ipaserver/install/cainstance.py            |  5 +-
  ipaserver/install/dns.py                   |  5 +-
  ipaserver/install/dsinstance.py            |  3 +-
  ipaserver/install/installutils.py          | 16 +++---
  ipaserver/install/ipa_ldap_updater.py      |  2 +-
  ipaserver/install/krainstance.py           |  3 +-
  ipaserver/install/replication.py           | 10 ++--
  ipaserver/install/server/install.py        | 64 +++++++++++----------
  ipaserver/install/server/replicainstall.py | 92

not modules:
install/tools/ipa-adtrust-install | 17 +++---
install/tools/ipa-ca-install | 23 ++++----
install/tools/ipa-compat-manage | 11 ++--
install/tools/ipa-dns-install | 3 +-

I'll keep the sys.exit replaces that won't make it here on the side, we
may want to do them later.
I'm a bit worried that the patch might change some behavior. Maybe I'm
wrong.

Attached is the patch with correct modules with sys.exits replaced.

I double-checked the changes and I believe the behavior shouldn't really change.




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.
From d4e820b4e59da1f2ecc6810cbb3353d6f9d53053 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                    |  40 ++++-----
 ipaserver/install/cainstance.py            |   5 +-
 ipaserver/install/dns.py                   |   5 +-
 ipaserver/install/dsinstance.py            |   3 +-
 ipaserver/install/installutils.py          |  20 ++---
 ipaserver/install/ipa_ldap_updater.py      |   3 +-
 ipaserver/install/krainstance.py           |   3 +-
 ipaserver/install/replication.py           |  10 ++-
 ipaserver/install/server/install.py        |  75 ++++++++---------
 ipaserver/install/server/replicainstall.py | 128 ++++++++++++++---------------
 11 files changed, 148 insertions(+), 146 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..64e978277a5c79d4df94bf9802cf775dfdeffc82 100644
--- a/ipaserver/install/ca.py
+++ b/ipaserver/install/ca.py
@@ -8,6 +8,7 @@ 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 +31,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 +53,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 +64,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 +98,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 +109,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..f35cd571c16a2d2ef2fe1a7d143227a868ceeac8 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -48,6 +48,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 +621,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 25f48aed1eeaa03353465bc40abf3484ec19bf3b..ee2301bd0fb5bda099a6f477e9767b4540e76408 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..42beabe1b63ef6244cd8fe3749af808fb8a66705 100644
--- a/ipaserver/install/ipa_ldap_updater.py
+++ b/ipaserver/install/ipa_ldap_updater.py
@@ -82,8 +82,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..e027b436c017a689f6a6f7bb0329230fbfebc093 100644
--- a/ipaserver/install/krainstance.py
+++ b/ipaserver/install/krainstance.py
@@ -33,6 +33,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 +426,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 65f9318201e648b30a3c13626e807ac6f3a9416d..4bd1e3c5e55b95ddfec720f67e7d2d5388d2de53 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
 
@@ -647,7 +650,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
@@ -895,8 +898,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()
@@ -980,9 +983,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(
@@ -1006,9 +1007,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)
 
@@ -1037,9 +1036,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 9d05a0be5a2679d825b4ee6bc2ea55ed358e8ff9..b14700ec7b39171149455d28e1daa709754ff81e 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -23,6 +23,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 +158,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 +225,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 +329,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 +338,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 +366,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 +509,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 +527,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 +548,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 +646,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 +682,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 +694,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 +710,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 +725,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 +737,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 +955,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 +995,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 +1016,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 +1057,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 +1073,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 +1089,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 +1204,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 +1219,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 +1235,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 +1247,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 +1265,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 +1273,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 +1297,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 +1308,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