URL: https://github.com/freeipa/freeipa/pull/258
Author: tiran
Title: #258: Break ipaplatform / ipalib import cycle of hell
Action: synchronized
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/258/head:pr258
git checkout pr258
From 1896b0da9c6a1eb748232898686c68977fb753ed Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Fri, 18 Nov 2016 15:42:23 +0100
Subject: [PATCH] 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 <chei...@redhat.com>
---
ipaclient/install/client.py | 8 ++++----
ipaclient/ntpconf.py | 11 ++++++-----
ipaplatform/base/services.py | 22 +++++++++++++++-------
ipaplatform/fedora/services.py | 10 +++++-----
ipaplatform/redhat/services.py | 29 ++++++++++++++---------------
ipaplatform/redhat/tasks.py | 4 ++--
ipaplatform/rhel/services.py | 10 +++++-----
ipaserver/install/adtrustinstance.py | 6 +++---
ipaserver/install/bindinstance.py | 2 +-
ipaserver/install/dns.py | 2 +-
ipaserver/install/httpinstance.py | 4 ++--
ipaserver/install/installutils.py | 2 +-
ipaserver/install/ipa_restore.py | 2 +-
ipaserver/install/opendnssecinstance.py | 2 +-
ipaserver/install/server/replicainstall.py | 2 +-
ipaserver/install/server/upgrade.py | 10 +++++-----
ipaserver/install/service.py | 2 +-
ipaserver/install/upgradeinstance.py | 3 ++-
ipaserver/plugins/dns.py | 2 +-
ipaserver/plugins/server.py | 2 +-
20 files changed, 72 insertions(+), 63 deletions(-)
diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py
index b24a989..d18d8bb 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 9a7db65..c78f807 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 750d979..071fe2a 100644
--- a/ipaplatform/base/services.py
+++ b/ipaplatform/base/services.py
@@ -27,10 +27,10 @@
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 @@ def get_config_dir(self, instance_name=""):
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 a3b951a..725d9ee 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 2432534..99904ce 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -31,7 +31,6 @@
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 @@
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 @@ def __init__(self, service_name):
# 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 @@ def get_config_dir(self, instance_name=""):
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 @@ def is_running(self, instance_name="", wait=True):
# 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 dbe005a..11d1733 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -44,8 +44,6 @@
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
@@ -214,6 +212,8 @@ def reload_systemwide_ca_store(self):
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 980c84a..7918006 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 cab5a72..136d26b 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -697,14 +697,14 @@ def __enable_compat_tree(self):
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
@@ -856,7 +856,7 @@ def uninstall(self):
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 179eb68..24a77ff 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -617,7 +617,7 @@ def __init__(self, fstore=None, api=api):
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 5b40c03..dd4ab7b 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 4e8107e..b6affe6 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -463,7 +463,7 @@ def create_kdcproxy_conf(self):
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())
@@ -484,7 +484,7 @@ def uninstall(self):
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 9e509e4..214d42c 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 21403af..3dc6522 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -410,7 +410,7 @@ def run(self):
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 7f3269f..efaed8d 100644
--- a/ipaserver/install/opendnssecinstance.py
+++ b/ipaserver/install/opendnssecinstance.py
@@ -317,7 +317,7 @@ def uninstall(self):
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 a7b333c..596d05e 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 5f61015..82bb6fb 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1230,21 +1230,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)
@@ -1261,7 +1261,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 62bd499..ee938d6 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -143,7 +143,7 @@ def __init__(self, service_name, service_desc=None, sstore=None,
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 0d6013f..e606b38 100644
--- a/ipaserver/install/upgradeinstance.py
+++ b/ipaserver/install/upgradeinstance.py
@@ -91,7 +91,8 @@ def __init__(self, realm_name, files=[], schema_files=[]):
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 e37050a..3336e9e 100644
--- a/ipaserver/plugins/dns.py
+++ b/ipaserver/plugins/dns.py
@@ -2710,7 +2710,7 @@ def _warning_ttl_changed_reload_needed(self, result, **options):
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 ded7661..08caa1c 100644
--- a/ipaserver/plugins/server.py
+++ b/ipaserver/plugins/server.py
@@ -262,7 +262,7 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
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()
--
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
