Break ipaplatform / ipalib import cycle of hell

Here is an attempt to break the import cycle of hell between ipaplatform
and ipalib. All services now pass an ipalib.api object to
services.service(). RedHatServices.__init__() still needs to do a local
import because it initializes its wellknown service dict with service
instances.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Martin Basti <mbasti@redhat.com>

20 files changed, 72 insertions, 63 deletions

diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
index b24a98916..d18d8bb1f 100644
--- a/ipaclient/install/client.py
+++ b/ipaclient/install/client.py
@@ -2822,7 +2822,7 @@ def _install(options):
         root_logger.info("%s enabled", "SSSD" if options.sssd else "LDAP")
 
         if options.sssd:
-            sssd = services.service('sssd')
+            sssd = services.service('sssd', api)
             try:
                 sssd.restart()
             except CalledProcessError:
@@ -3139,7 +3139,7 @@ def uninstall(options):
 
         root_logger.info(
             "IPA domain removed from current one, restarting SSSD service")
-        sssd = services.service('sssd')
+        sssd = services.service('sssd', api)
         try:
             sssd.restart()
         except CalledProcessError:
@@ -3153,7 +3153,7 @@ def uninstall(options):
             "Other domains than IPA domain found, IPA domain was removed "
             "from /etc/sssd/sssd.conf.")
 
-        sssd = services.service('sssd')
+        sssd = services.service('sssd', api)
         try:
             sssd.restart()
         except CalledProcessError:
@@ -3172,7 +3172,7 @@ def uninstall(options):
             "Redundant SSSD configuration file "
             "/etc/sssd/sssd.conf was moved to /etc/sssd/sssd.conf.deleted")
 
-        sssd = services.service('sssd')
+        sssd = services.service('sssd', api)
         try:
             sssd.stop()
         except CalledProcessError:
diff --git a/ipaclient/ntpconf.py b/ipaclient/ntpconf.py
index 9a7db6544..c78f80749 100644
--- a/ipaclient/ntpconf.py
+++ b/ipaclient/ntpconf.py
@@ -16,11 +16,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
+import os
+import shutil
 
+from ipalib import api
 from ipapython import ipautil
 from ipapython.ipa_log_manager import root_logger
-import shutil
-import os
 from ipaplatform.tasks import tasks
 from ipaplatform import services
 from ipaplatform.paths import paths
@@ -189,7 +190,7 @@ def check_timedate_services():
         if service == 'ntpd':
             continue
         # Make sure that the service is not enabled
-        instance = services.service(service)
+        instance = services.service(service, api)
         if instance.is_enabled() or instance.is_running():
             raise NTPConflictingService(conflicting_service=instance.service_name)
 
@@ -201,7 +202,7 @@ def force_ntpd(statestore):
     for service in services.timedate_services:
         if service == 'ntpd':
             continue
-        instance = services.service(service)
+        instance = services.service(service, api)
         enabled = instance.is_enabled()
         running = instance.is_running()
 
@@ -224,7 +225,7 @@ def restore_forced_ntpd(statestore):
         if service == 'ntpd':
             continue
         if statestore.has_state(service):
-            instance = services.service(service)
+            instance = services.service(service, api)
             enabled = statestore.restore_state(instance.service_name, 'enabled')
             running = statestore.restore_state(instance.service_name, 'running')
             if enabled:
diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py
index 750d97953..071fe2a32 100644
--- a/ipaplatform/base/services.py
+++ b/ipaplatform/base/services.py
@@ -27,10 +27,10 @@ import os
 import json
 import time
 import collections
+import warnings
 
 import six
 
-import ipalib
 from ipapython import ipautil
 from ipaplatform.paths import paths
 
@@ -63,7 +63,6 @@ class KnownServices(collections.Mapping):
     instances as its own attributes on first access (or instance creation)
     and cache them.
     """
-
     def __init__(self, d):
         self.__d = d
 
@@ -93,9 +92,17 @@ class PlatformService(object):
 
     """
 
-    def __init__(self, service_name, api=ipalib.api):
+    def __init__(self, service_name, api=None):
         self.service_name = service_name
-        self.api = api
+        if api is not None:
+            self.api = api
+        else:
+            import ipalib  # FixMe: break import cycle
+            self.api = ipalib.api
+            warnings.warn(
+                "{s.__class__.__name__}('{s.service_name}', api=None) "
+                "is deprecated.".format(s=self),
+                RuntimeWarning, stacklevel=2)
 
     def start(self, instance_name="", capture_output=True, wait=True,
         update_service_list=True):
@@ -185,10 +192,11 @@ class PlatformService(object):
 class SystemdService(PlatformService):
     SYSTEMD_SRV_TARGET = "%s.target.wants"
 
-    def __init__(self, service_name, systemd_name, **kwargs):
-        super(SystemdService, self).__init__(service_name, **kwargs)
+    def __init__(self, service_name, systemd_name, api=None):
+        super(SystemdService, self).__init__(service_name, api=api)
         self.systemd_name = systemd_name
-        self.lib_path = os.path.join(paths.LIB_SYSTEMD_SYSTEMD_DIR, self.systemd_name)
+        self.lib_path = os.path.join(paths.LIB_SYSTEMD_SYSTEMD_DIR,
+                                     self.systemd_name)
         self.lib_path_exists = None
 
     def service_instance(self, instance_name, operation=None):
diff --git a/ipaplatform/fedora/services.py b/ipaplatform/fedora/services.py
index a3b951a43..725d9eeae 100644
--- a/ipaplatform/fedora/services.py
+++ b/ipaplatform/fedora/services.py
@@ -41,17 +41,17 @@ class FedoraService(redhat_services.RedHatService):
 # Function that constructs proper Fedora-specific server classes for services
 # of specified name
 
-def fedora_service_class_factory(name):
+def fedora_service_class_factory(name, api=None):
     if name == 'domainname':
-        return FedoraService(name)
-    return redhat_services.redhat_service_class_factory(name)
+        return FedoraService(name, api)
+    return redhat_services.redhat_service_class_factory(name, api)
 
 
 # Magicdict containing FedoraService instances.
 
 class FedoraServices(redhat_services.RedHatServices):
-    def service_class_factory(self, name):
-        return fedora_service_class_factory(name)
+    def service_class_factory(self, name, api=None):
+        return fedora_service_class_factory(name, api)
 
 
 # Objects below are expected to be exported by platform module
diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 24325347c..99904ce9f 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -31,7 +31,6 @@ from ipaplatform.base import services as base_services
 
 from ipapython import ipautil, dogtag
 from ipapython.ipa_log_manager import root_logger
-from ipalib import api
 from ipaplatform.paths import paths
 
 # Mappings from service names as FreeIPA code references to these services
@@ -76,7 +75,7 @@ redhat_system_units['ods_signerd'] = redhat_system_units['ods-signerd']
 class RedHatService(base_services.SystemdService):
     system_units = redhat_system_units
 
-    def __init__(self, service_name):
+    def __init__(self, service_name, api=None):
         systemd_name = service_name
         if service_name in self.system_units:
             systemd_name = self.system_units[service_name]
@@ -86,7 +85,7 @@ class RedHatService(base_services.SystemdService):
                 # and not a foo.target. Thus, not correct service name for
                 # systemd, default to foo.service style then
                 systemd_name = "%s.service" % (service_name)
-        super(RedHatService, self).__init__(service_name, systemd_name)
+        super(RedHatService, self).__init__(service_name, systemd_name, api)
 
 
 class RedHatDirectoryService(RedHatService):
@@ -195,12 +194,12 @@ class RedHatSSHService(RedHatService):
 class RedHatCAService(RedHatService):
     def wait_until_running(self):
         root_logger.debug('Waiting until the CA is running')
-        timeout = float(api.env.startup_timeout)
+        timeout = float(self.api.env.startup_timeout)
         op_timeout = time.time() + timeout
         while time.time() < op_timeout:
             try:
                 # check status of CA instance on this host, not remote ca_host
-                status = dogtag.ca_status(api.env.host)
+                status = dogtag.ca_status(self.api.env.host)
             except Exception as e:
                 status = 'check interrupted due to error: %s' % e
             root_logger.debug('The CA status is: %s' % status)
@@ -244,31 +243,31 @@ class RedHatCAService(RedHatService):
 # Function that constructs proper Red Hat OS family-specific server classes for
 # services of specified name
 
-def redhat_service_class_factory(name):
+def redhat_service_class_factory(name, api=None):
     if name == 'dirsrv':
-        return RedHatDirectoryService(name)
+        return RedHatDirectoryService(name, api)
     if name == 'ipa':
-        return RedHatIPAService(name)
+        return RedHatIPAService(name, api)
     if name == 'sshd':
-        return RedHatSSHService(name)
+        return RedHatSSHService(name, api)
     if name in ('pki-tomcatd', 'pki_tomcatd'):
-        return RedHatCAService(name)
-    return RedHatService(name)
+        return RedHatCAService(name, api)
+    return RedHatService(name, api)
 
 
 # Magicdict containing RedHatService instances.
 
 class RedHatServices(base_services.KnownServices):
-    def service_class_factory(self, name):
-        return redhat_service_class_factory(name)
-
     def __init__(self):
+        import ipalib  # FixMe: break import cycle
         services = dict()
         for s in base_services.wellknownservices:
-            services[s] = self.service_class_factory(s)
+            services[s] = self.service_class_factory(s, ipalib.api)
         # Call base class constructor. This will lock services to read-only
         super(RedHatServices, self).__init__(services)
 
+    def service_class_factory(self, name, api=None):
+        return redhat_service_class_factory(name, api)
 
 # Objects below are expected to be exported by platform module
 
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 5d627becf..3766eb344 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -44,8 +44,6 @@ from ipapython.ipa_log_manager import root_logger, log_mgr
 from ipapython import ipautil
 import ipapython.errors
 
-from ipalib import x509 # FIXME: do not import from ipalib
-
 from ipaplatform.constants import constants
 from ipaplatform.paths import paths
 from ipaplatform.redhat.authconfig import RedHatAuthConfig
@@ -220,6 +218,8 @@ class RedHatTaskNamespace(BaseTaskNamespace):
             return True
 
     def insert_ca_certs_into_systemwide_ca_store(self, ca_certs):
+        from ipalib import x509  # FixMe: break import cycle
+
         new_cacert_path = paths.SYSTEMWIDE_IPA_CA_CRT
 
         if os.path.exists(new_cacert_path):
diff --git a/ipaplatform/rhel/services.py b/ipaplatform/rhel/services.py
index 980c84a96..7918006db 100644
--- a/ipaplatform/rhel/services.py
+++ b/ipaplatform/rhel/services.py
@@ -41,17 +41,17 @@ class RHELService(redhat_services.RedHatService):
 # Function that constructs proper RHEL-specific server classes for services
 # of specified name
 
-def rhel_service_class_factory(name):
+def rhel_service_class_factory(name, api=None):
     if name == 'domainname':
-        return RHELService(name)
-    return redhat_services.redhat_service_class_factory(name)
+        return RHELService(name, api)
+    return redhat_services.redhat_service_class_factory(name, api)
 
 
 # Magicdict containing RHELService instances.
 
 class RHELServices(redhat_services.RedHatServices):
-    def service_class_factory(self, name):
-        return rhel_service_class_factory(name)
+    def service_class_factory(self, name, api=None):
+        return rhel_service_class_factory(name, api)
 
 
 # Objects below are expected to be exported by platform module
diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 632052ab4..22a4a1d57 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -700,14 +700,14 @@ class ADTRUSTInstance(service.Service):
     def __start(self):
         try:
             self.start()
-            services.service('winbind').start()
+            services.service('winbind', api).start()
         except Exception:
             root_logger.critical("CIFS services failed to start")
 
     def __stop(self):
         self.backup_state("running", self.is_running())
         try:
-            services.service('winbind').stop()
+            services.service('winbind', api).stop()
             self.stop()
         except Exception:
             pass
@@ -859,7 +859,7 @@ class ADTRUSTInstance(service.Service):
         self.restore_state("running")
         self.restore_state("enabled")
 
-        winbind = services.service("winbind")
+        winbind = services.service("winbind", api)
         # Always try to stop and disable smb service, since we do not leave
         # working configuration after uninstall
         try:
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index f2ece576a..a944be2bb 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -617,7 +617,7 @@ class BindInstance(service.Service):
         self.forwarders = None
         self.sub_dict = None
         self.reverse_zones = []
-        self.named_regular = services.service('named-regular')
+        self.named_regular = services.service('named-regular', api)
 
     suffix = ipautil.dn_attribute_property('_suffix')
 
diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index 5b40c03c3..dd4ab7b7e 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -219,7 +219,7 @@ def install_check(standalone, api, replica, options, hostname):
                 "Only one DNSSEC key master is supported in current version.")
 
         if options.kasp_db_file:
-            dnskeysyncd = services.service('ipa-dnskeysyncd')
+            dnskeysyncd = services.service('ipa-dnskeysyncd', api)
 
             if not dnskeysyncd.is_installed():
                 raise RuntimeError("ipa-dnskeysyncd is not configured on this "
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index cd1674362..0fdffca7b 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -464,7 +464,7 @@ class HTTPInstance(service.Service):
         os.chmod(target_fname, 0o644)
 
     def enable_and_start_oddjobd(self):
-        oddjobd = services.service('oddjobd')
+        oddjobd = services.service('oddjobd', api)
         self.sstore.backup_state('oddjobd', 'running', oddjobd.is_running())
         self.sstore.backup_state('oddjobd', 'enabled', oddjobd.is_enabled())
 
@@ -485,7 +485,7 @@ class HTTPInstance(service.Service):
         enabled = self.restore_state("enabled")
 
         # Restore oddjobd to its original state
-        oddjobd = services.service('oddjobd')
+        oddjobd = services.service('oddjobd', api)
 
         if not self.sstore.restore_state('oddjobd', 'running'):
             try:
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 9e509e461..214d42c56 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -984,7 +984,7 @@ def stopped_service(service, instance_name=""):
                       'the next set of commands is being executed.', service,
                       log_instance_name)
 
-    service_obj = services.service(service)
+    service_obj = services.service(service, api)
 
     # Figure out if the service is running, if not, yield
     if not service_obj.is_running(instance_name):
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 21403afcf..3dc65229d 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -410,7 +410,7 @@ class Restore(admintool.AdminTool):
                 self.log.info('Starting IPA services')
                 run(['ipactl', 'start'])
                 self.log.info('Restarting SSSD')
-                sssd = services.service('sssd')
+                sssd = services.service('sssd', api)
                 sssd.restart()
                 http.remove_httpd_ccache()
         finally:
diff --git a/ipaserver/install/opendnssecinstance.py b/ipaserver/install/opendnssecinstance.py
index ea6cb51d4..39b81b6b4 100644
--- a/ipaserver/install/opendnssecinstance.py
+++ b/ipaserver/install/opendnssecinstance.py
@@ -317,7 +317,7 @@ class OpenDNSSECInstance(service.Service):
         except Exception:
             pass
 
-        ods_exporter = services.service('ipa-ods-exporter')
+        ods_exporter = services.service('ipa-ods-exporter', api)
         try:
             ods_exporter.stop()
         except Exception:
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index ee1147d3c..1889bb084 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -451,7 +451,7 @@ def promote_sssd(host_name):
         sssdconfig.save_domain(domain)
         sssdconfig.write()
 
-        sssd = services.service('sssd')
+        sssd = services.service('sssd', api)
         try:
             sssd.restart()
         except CalledProcessError:
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 64810fbbf..ea38f7067 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1229,21 +1229,21 @@ def uninstall_dogtag_9(ds, http):
         dsdb.untrack_server_cert("Server-Cert")
 
     try:
-        services.service('pki-cad').disable('pki-ca')
+        services.service('pki-cad', api).disable('pki-ca')
     except Exception as e:
         root_logger.warning("Failed to disable pki-cad: %s", e)
     try:
-        services.service('pki-cad').stop('pki-ca')
+        services.service('pki-cad', api).stop('pki-ca')
     except Exception as e:
         root_logger.warning("Failed to stop pki-cad: %s", e)
 
     if serverid is not None:
         try:
-            services.service('dirsrv').disable(serverid)
+            services.service('dirsrv', api).disable(serverid)
         except Exception as e:
             root_logger.warning("Failed to disable dirsrv: %s", e)
         try:
-            services.service('dirsrv').stop(serverid)
+            services.service('dirsrv', api).stop(serverid)
         except Exception as e:
             root_logger.warning("Failed to stop dirsrv: %s", e)
 
@@ -1260,7 +1260,7 @@ def mask_named_regular():
 
     if bindinstance.named_conf_exists():
         root_logger.info('[Masking named]')
-        named = services.service('named-regular')
+        named = services.service('named-regular', api)
         try:
             named.stop()
         except Exception as e:
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index bdb8e56e4..89d6b907a 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -143,7 +143,7 @@ class Service(object):
                  keytab=None):
         self.service_name = service_name
         self.service_desc = service_desc
-        self.service = services.service(service_name)
+        self.service = services.service(service_name, api)
         self.steps = []
         self.output_fd = sys.stdout
 
diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py
index b22f15a0b..8e2843686 100644
--- a/ipaserver/install/upgradeinstance.py
+++ b/ipaserver/install/upgradeinstance.py
@@ -91,7 +91,8 @@ class IPAUpgrade(service.Service):
         self.schema_files = schema_files
 
     def __start(self):
-        services.service(self.service_name).start(self.serverid, ldapi=True)
+        srv = services.service(self.service_name, api)
+        srv.start(self.serverid, ldapi=True)
         api.Backend.ldap2.connect()
 
     def __stop_instance(self):
diff --git a/ipaserver/plugins/dns.py b/ipaserver/plugins/dns.py
index e37050a61..3336e9ead 100644
--- a/ipaserver/plugins/dns.py
+++ b/ipaserver/plugins/dns.py
@@ -2710,7 +2710,7 @@ class dnszone(DNSZoneBase):
                 options['version'],
                 result,
                 messages.ServiceRestartRequired(
-                    service=services.service('named').systemd_name,
+                    service=services.service('named', api).systemd_name,
                     server=_('<all IPA DNS servers>'), )
                 )
 
diff --git a/ipaserver/plugins/server.py b/ipaserver/plugins/server.py
index ded7661ec..08caa1cf7 100644
--- a/ipaserver/plugins/server.py
+++ b/ipaserver/plugins/server.py
@@ -262,7 +262,7 @@ class server_mod(LDAPUpdate):
 
         if 'ipalocation_location' or 'ipaserviceweight' in options:
             self.add_message(messages.ServiceRestartRequired(
-                service=services.service('named').systemd_name,
+                service=services.service('named', api).systemd_name,
                 server=keys[0], ))
 
             result = self.api.Command.dns_update_system_records()