Refactor krbinstance and dsinstance creation steps

Creation steps are currently done with:

  self.start_creation(2, "Create foo")
  self.step("do foo")
  self.foo()
  self.step("do bar")
  self.bar()
  self.done_creation()

This patch refactors that into the much more
straightforward:

  self.step("do foo", self.foo)
  self.step("do bar", self.bar)
  self.start_creation("Create foo")

Signed-off-by: Mark McLoughlin <markmc@redhat.com>

6 files changed, 107 insertions, 152 deletions

diff --git a/ipa-server/ipaserver/dsinstance.py b/ipa-server/ipaserver/dsinstance.py
index b57ac1db3..6cbffcb84 100644
--- a/ipa-server/ipaserver/dsinstance.py
+++ b/ipa-server/ipaserver/dsinstance.py
@@ -135,38 +135,29 @@ class DsInstance(service.Service):
         self.domain = host_name[host_name.find(".")+1:]
         self.__setup_sub_dict()
         
-        if ro_replica:
-            self.start_creation(15, "Configuring directory server:")
-        else:
-            self.start_creation(15, "Configuring directory server:")
-
-        self.__create_ds_user()
-        self.__create_instance()
-        self.__add_default_schemas()
+        self.step("creating directory server user", self.__create_ds_user)
+        self.step("creating directory server instance", self.__create_instance)
+        self.step("adding default schema", self.__add_default_schemas)
         if not ro_replica:
-            self.__add_memberof_module()
-        self.__add_referint_module()
-        self.__add_dna_module()
-        self.__create_indeces()
-        self.__enable_ssl()
-        self.__certmap_conf()
-        try:
-            self.step("restarting directory server")
-            self.restart()
-        except:
-            # TODO: roll back here?
-            logging.critical("Failed to restart the ds instance")
-        self.__add_default_layout()
+            self.step("enabling memberof plugin", self.__add_memberof_module)
+        self.step("enabling referential integrity plugin", self.__add_referint_module)
+        self.step("enabling distributed numeric assignment plugin", self.__add_dna_module)
+        self.step("creating indeces", self.__create_indeces)
+        self.step("configuring ssl for ds instance", self.__enable_ssl)
+        self.step("configuring certmap.conf", self.__certmap_conf)
+        self.step("restarting directory server", self.__restart_instance)
+        self.step("adding default layout", self.__add_default_layout)
         if not ro_replica:
-            self.__config_uidgid_gen_first_master()
-            self.__add_master_entry_first_master()
-            self.__init_memberof()
+            self.step("configuring Posix uid/gid generation as first master",
+                      self.__config_uidgid_gen_first_master)
+            self.step("adding master entry as first master",
+                      self.__add_master_entry_first_master)
+            self.step("initializing group membership",
+                      self.__init_memberof)
 
+        self.step("configuring directory to start on boot", self.chkconfig_on)
 
-        self.step("configuring directoy to start on boot")
-        self.chkconfig_on()
-
-        self.done_creation()
+        self.start_creation("Configuring directory server:")
 
     def __setup_sub_dict(self):
         server_root = find_server_root()
@@ -176,7 +167,6 @@ class DsInstance(service.Service):
                              SERVER_ROOT=server_root, DOMAIN=self.domain)
 
     def __create_ds_user(self):
-        self.step("creating directory server user")
 	try:
             pwd.getpwnam(self.ds_user)
             logging.debug("ds user %s exists" % self.ds_user)
@@ -190,7 +180,6 @@ class DsInstance(service.Service):
                 logging.critical("failed to add user %s" % e)
 
     def __create_instance(self):
-        self.step("creating directory server instance")
         inf_txt = ipautil.template_str(INF_TEMPLATE, self.sub_dict)
         logging.debug(inf_txt)
         inf_fd = ipautil.write_tmp_file(inf_txt)
@@ -215,7 +204,6 @@ class DsInstance(service.Service):
             logging.debug("failed to restart ds instance %s" % e)
 
     def __add_default_schemas(self):
-        self.step("adding default schema")
         shutil.copyfile(ipautil.SHARE_DIR + "60kerberos.ldif",
                         schema_dirname(self.realm_name) + "60kerberos.ldif")
         shutil.copyfile(ipautil.SHARE_DIR + "60samba.ldif",
@@ -225,9 +213,14 @@ class DsInstance(service.Service):
         shutil.copyfile(ipautil.SHARE_DIR + "60ipaconfig.ldif",
                         schema_dirname(self.realm_name) + "60ipaconfig.ldif")
 
-    def __ldap_mod(self, step, ldif, sub_dict = None):
-        self.step(step)
+    def __restart_instance(self):
+        try:
+            self.restart()
+        except:
+            # TODO: roll back here?
+            logging.critical("Failed to restart the ds instance")
 
+    def __ldap_mod(self, ldif, sub_dict = None):
         fd = None
         path = ipautil.SHARE_DIR + ldif
 
@@ -248,30 +241,24 @@ class DsInstance(service.Service):
             fd.close()
 
     def __add_memberof_module(self):
-        self.__ldap_mod("enabling memberof plugin", "memberof-conf.ldif")
+        self.__ldap_mod("memberof-conf.ldif")
 
     def __init_memberof(self):
-        self.__ldap_mod("initializing group membership",
-                        "memberof-task.ldif", self.sub_dict)
+        self.__ldap_mod("memberof-task.ldif", self.sub_dict)
 
     def __add_referint_module(self):
-        self.__ldap_mod("enabling referential integrity plugin",
-                        "referint-conf.ldif")
+        self.__ldap_mod("referint-conf.ldif")
 
     def __add_dna_module(self):
-        self.__ldap_mod("enabling distributed numeric assignment plugin",
-                        "dna-conf.ldif")
+        self.__ldap_mod("dna-conf.ldif")
 
     def __config_uidgid_gen_first_master(self):
-        self.__ldap_mod("configuring Posix uid/gid generation as first master",
-                        "dna-posix.ldif", self.sub_dict)
+        self.__ldap_mod("dna-posix.ldif", self.sub_dict)
 
     def __add_master_entry_first_master(self):
-        self.__ldap_mod("adding master entry as first master",
-                        "master-entry.ldif", self.sub_dict)
+        self.__ldap_mod("master-entry.ldif", self.sub_dict)
 
     def __enable_ssl(self):
-        self.step("configuring ssl for ds instance")
         dirname = config_dirname(self.realm_name)
         ca = certs.CertDB(dirname)
         ca.create_self_signed()
@@ -305,14 +292,12 @@ class DsInstance(service.Service):
         conn.unbind()
 
     def __add_default_layout(self):
-        self.__ldap_mod("adding default layout",
-                        "bootstrap-template.ldif", self.sub_dict)
+        self.__ldap_mod("bootstrap-template.ldif", self.sub_dict)
         
     def __create_indeces(self):
-        self.__ldap_mod("creating indeces", "indeces.ldif")
+        self.__ldap_mod("indeces.ldif")
 
     def __certmap_conf(self):
-        self.step("configuring certmap.conf")
         shutil.copyfile(ipautil.SHARE_DIR + "certmap.conf.template",
                         config_dirname(self.realm_name) + "certmap.conf")
 
diff --git a/ipa-server/ipaserver/httpinstance.py b/ipa-server/ipaserver/httpinstance.py
index 8a7c527af..d0329ccad 100644
--- a/ipa-server/ipaserver/httpinstance.py
+++ b/ipa-server/ipaserver/httpinstance.py
@@ -57,25 +57,19 @@ class HTTPInstance(service.Service):
         self.domain = fqdn[fqdn.find(".")+1:]
         self.sub_dict = { "REALM" : realm, "FQDN": fqdn, "DOMAIN" : self.domain }
         
-        self.start_creation(7, "Configuring the web interface")
-        
-        self.__disable_mod_ssl()
-        self.__set_mod_nss_port()
-        self.__configure_http()
-        self.__create_http_keytab()
-        self.__setup_ssl()
-        self.__setup_autoconfig()
-
-        self.step("restarting httpd")
-        self.restart()
-
-        self.step("configuring httpd to start on boot")
-        self.chkconfig_on()
-
-        self.done_creation()
+        self.step("disabling mod_ssl in httpd", self.__disable_mod_ssl)
+        self.step("Setting mod_nss port to 443", self.__set_mod_nss_port)
+        self.step("configuring httpd", self.__configure_http)
+        self.step("creating a keytab for httpd", self.__create_http_keytab)
+        self.step("Setting up ssl", self.__setup_ssl)
+        self.step("Setting up browser autoconfig", self.__setup_autoconfig)
+        self.step("configuring SELinux for httpd", self.__selinux_config)
+        self.step("restarting httpd", self.restart)
+        self.step("configuring httpd to start on boot", self.chkconfig_on)
+
+        self.start_creation("Configuring the web interface")
 
     def __selinux_config(self):
-        self.step("configuring SELinux for httpd")
         selinux=0
         try:
             if (os.path.exists('/usr/sbin/selinuxenabled')):
@@ -94,7 +88,6 @@ class HTTPInstance(service.Service):
                 self.print_msg(selinux_warning)
                 
     def __create_http_keytab(self):
-        self.step("creating a keytab for httpd")
         try:
             if ipautil.file_exists("/etc/httpd/conf/ipa.keytab"):
                 os.remove("/etc/httpd/conf/ipa.keytab")
@@ -122,7 +115,6 @@ class HTTPInstance(service.Service):
         os.chown("/etc/httpd/conf/ipa.keytab", pent.pw_uid, pent.pw_gid)
 
     def __configure_http(self):
-        self.step("configuring httpd")
         http_txt = ipautil.template_file(ipautil.SHARE_DIR + "ipa.conf", self.sub_dict)
         http_fd = open("/etc/httpd/conf.d/ipa.conf", "w")
         http_fd.write(http_txt)
@@ -130,17 +122,14 @@ class HTTPInstance(service.Service):
 
 
     def __disable_mod_ssl(self):
-        self.step("disabling mod_ssl in httpd")
         if os.path.exists(SSL_CONF):
             os.rename(SSL_CONF, "%s.moved_by_ipa" % SSL_CONF)
 
     def __set_mod_nss_port(self):
-        self.step("Setting mod_nss port to 443")
         if installutils.update_file(NSS_CONF, '8443', '443') != 0:
             print "Updating %s failed." % NSS_CONF
 
     def __setup_ssl(self):
-        self.step("Setting up ssl")
         ds_ca = certs.CertDB(dsinstance.config_dirname(self.realm))
         ca = certs.CertDB(NSS_DIR)
         ds_ca.cur_serial = 2000
diff --git a/ipa-server/ipaserver/krbinstance.py b/ipa-server/ipaserver/krbinstance.py
index 67d3019e0..5036aa53b 100644
--- a/ipa-server/ipaserver/krbinstance.py
+++ b/ipa-server/ipaserver/krbinstance.py
@@ -114,58 +114,42 @@ class KrbInstance(service.Service):
             pass
 
     def __common_post_setup(self):
-        try:
-            self.step("starting the KDC")
-            self.start()
-        except:
-            logging.critical("krb5kdc service failed to start")
-
-        self.step("configuring KDC to start on boot")
-        self.chkconfig_on()
-
-        self.step("configuring ipa-kpasswd to start on boot")
-        service.chkconfig_on("ipa-kpasswd")
-
-        self.step("starting ipa-kpasswd")
-        service.start("ipa-kpasswd")
-
+        self.step("starting the KDC", self.__start_instance)
+        self.step("configuring KDC to start on boot", self.chkconfig_on)
+        self.step("enabling and starting ipa-kpasswd", self.__enable_kpasswd)
 
     def create_instance(self, ds_user, realm_name, host_name, admin_password, master_password):
         self.master_password = master_password
 
         self.__common_setup(ds_user, realm_name, host_name, admin_password)
 
-        self.start_creation(12, "Configuring Kerberos KDC")
-        
-        self.__configure_kdc_account_password()
-        self.__configure_sasl_mappings()
-        self.__add_krb_entries()
-        self.__create_instance()
-        self.__create_ds_keytab()
-        self.__export_kadmin_changepw_keytab()
-        self.__add_pwd_extop_module()
+        self.step("setting KDC account password", self.__configure_kdc_account_password)
+        self.step("adding sasl mappings to the directory", self.__configure_sasl_mappings)
+        self.step("adding kerberos entries to the DS", self.__add_krb_entries)
+        self.step("adding defalt ACIs", self.__add_default_acis)
+        self.step("configuring KDC", self.__create_instance)
+        self.step("creating a keytab for the directory", self.__create_ds_keytab)
+        self.step("exporting the kadmin keytab", self.__export_kadmin_changepw_keytab)
+        self.step("adding the password extenstion to the directory", self.__add_pwd_extop_module)
 
         self.__common_post_setup()
 
-        self.done_creation()
-
+        self.start_creation("Configuring Kerberos KDC")
 
     def create_replica(self, ds_user, realm_name, host_name, admin_password, ldap_passwd_filename):
-        
+        self.__copy_ldap_passwd(ldap_passwd_filename)
+
         self.__common_setup(ds_user, realm_name, host_name, admin_password)
 
-        self.start_creation(9, "Configuring Kerberos KDC")
-        self.__copy_ldap_passwd(ldap_passwd_filename)
-        self.__configure_sasl_mappings()
-        self.__write_stash_from_ds()
-        self.__create_instance(replica=True)
-        self.__create_ds_keytab()
-        self.__export_kadmin_changepw_keytab()
+        self.step("adding sasl mappings to the directory", self.__configure_sasl_mappings)
+        self.step("writing stash file from DS", self.__write_stash_from_ds)
+        self.step("configuring KDC", self.__create_replica_instance)
+        self.step("creating a keytab for the directory", self.__create_ds_keytab)
+        self.step("exporting the kadmin keytab", self.__export_kadmin_changepw_keytab)
 
         self.__common_post_setup()
 
-        self.done_creation()
-
+        self.start_creation("Configuring Kerberos KDC")
 
     def __copy_ldap_passwd(self, filename):
         shutil.copy(filename, "/var/kerberos/krb5kdc/ldappwd")
@@ -173,7 +157,6 @@ class KrbInstance(service.Service):
         
         
     def __configure_kdc_account_password(self):
-        self.step("setting KDC account password")
         hexpwd = ''
 	for x in self.kdc_password:
             hexpwd += (hex(ord(x))[2:])
@@ -182,6 +165,16 @@ class KrbInstance(service.Service):
         pwd_fd.close()
         os.chmod("/var/kerberos/krb5kdc/ldappwd", 0600)
 
+    def __start_instance(self):
+        try:
+            self.start()
+        except:
+            logging.critical("krb5kdc service failed to start")
+
+    def __enable_kpasswd(self):
+        service.chkconfig_on("ipa-kpasswd")
+        service.start("ipa-kpasswd")
+
     def __setup_sub_dict(self):
         self.sub_dict = dict(FQDN=self.fqdn,
                              IP=self.ip,
@@ -191,9 +184,7 @@ class KrbInstance(service.Service):
                              HOST=self.host,
                              REALM=self.realm)
 
-    def __ldap_mod(self, step, ldif):
-        self.step(step)
-
+    def __ldap_mod(self, ldif):
         txt = ipautil.template_file(ipautil.SHARE_DIR + ldif, self.sub_dict)
         fd = ipautil.write_tmp_file(txt)
 
@@ -208,7 +199,6 @@ class KrbInstance(service.Service):
         fd.close()
 
     def __configure_sasl_mappings(self):
-        self.step("adding sasl mappings to the directory")
         # we need to remove any existing SASL mappings in the directory as otherwise they
         # they may conflict. There is no way to define the order they are used in atm.
 
@@ -258,13 +248,16 @@ class KrbInstance(service.Service):
             raise e
 
     def __add_krb_entries(self):
-        self.__ldap_mod("adding kerberos entries to the DS", "kerberos.ldif")
+        self.__ldap_mod("kerberos.ldif")
 
+    def __add_default_acis(self):
 	#Change the default ACL to avoid anonimous access to kerberos keys and othe hashes
-        self.__ldap_mod("adding defalt ACIs", "default-aci.ldif")
+        self.__ldap_mod("default-aci.ldif")
+
+    def __create_replica_instance(self):
+        self.__create_instance(replace=True)
 
     def __create_instance(self, replica=False):
-        self.step("configuring KDC")
         kdc_conf = ipautil.template_file(ipautil.SHARE_DIR+"kdc.conf.template", self.sub_dict)
         kdc_fd = open("/var/kerberos/krb5kdc/kdc.conf", "w+")
         kdc_fd.write(kdc_conf)
@@ -300,7 +293,6 @@ class KrbInstance(service.Service):
                 print "Failed to populate the realm structure in kerberos", e
 
     def __write_stash_from_ds(self):
-        self.step("writing stash file from DS")
         try:
             entry = self.conn.getEntry("cn=%s, cn=kerberos, %s" % (self.realm, self.suffix), ldap.SCOPE_SUBTREE)
         except ipaerror.exception_for(ipaerror.LDAP_NOT_FOUND), e:
@@ -322,7 +314,7 @@ class KrbInstance(service.Service):
 
     #add the password extop module
     def __add_pwd_extop_module(self):
-        self.__ldap_mod("adding the password extenstion to the directory", "pwd-extop-conf.ldif")
+        self.__ldap_mod("pwd-extop-conf.ldif")
 
         #get the Master Key from the stash file
         try:
@@ -351,7 +343,6 @@ class KrbInstance(service.Service):
             raise e
 
     def __create_ds_keytab(self):
-        self.step("creating a keytab for the directory")
         try:
             if ipautil.file_exists("/etc/dirsrv/ds.keytab"):
                 os.remove("/etc/dirsrv/ds.keytab")
@@ -380,7 +371,6 @@ class KrbInstance(service.Service):
         os.chown("/etc/dirsrv/ds.keytab", pent.pw_uid, pent.pw_gid)
 
     def __export_kadmin_changepw_keytab(self):
-        self.step("exporting the kadmin keytab")
         try:
             if ipautil.file_exists("/var/kerberos/krb5kdc/kpasswd.keytab"):
                 os.remove("/var/kerberos/krb5kdc/kpasswd.keytab")
diff --git a/ipa-server/ipaserver/ntpinstance.py b/ipa-server/ipaserver/ntpinstance.py
index 3872d93c6..b321ec075 100644
--- a/ipa-server/ipaserver/ntpinstance.py
+++ b/ipa-server/ipaserver/ntpinstance.py
@@ -25,11 +25,8 @@ from ipa import ipautil
 class NTPInstance(service.Service):
     def __init__(self):
         service.Service.__init__(self, "ntpd")
-        
-    def create_instance(self):
-        self.start_creation(3, "Configuring ntpd")
 
-        self.step("writing configuration")
+    def __write_config(self):
         # The template sets the config to point towards ntp.pool.org, but
         # they request that software not point towards the default pool.
         # We use the OS variable to point it towards either the rhel
@@ -54,11 +51,13 @@ class NTPInstance(service.Service):
         fd.write(ntp_conf)
         fd.close()
 
+    def create_instance(self):
+        self.step("writing configuration", self.__write_config)
+
         # we might consider setting the date manually using ntpd -qg in case
         # the current time is very far off.
 
-        self.step("starting ntpd")
-        self.start()
-        
-        self.step("configuring ntpd to start on boot")
-        self.chkconfig_on()
+        self.step("starting ntpd", self.start)
+        self.step("configuring ntpd to start on boot", self.chkconfig_on)
+
+        self.start_creation("Configuring ntpd")
diff --git a/ipa-server/ipaserver/service.py b/ipa-server/ipaserver/service.py
index 4b2799702..90d0e606a 100644
--- a/ipa-server/ipaserver/service.py
+++ b/ipa-server/ipaserver/service.py
@@ -45,8 +45,7 @@ def print_msg(message, output_fd=sys.stdout):
 class Service:
     def __init__(self, service_name):
         self.service_name = service_name
-        self.num_steps = -1
-        self.current_step = -1
+        self.steps = []
         self.output_fd = sys.stdout
 
     def set_output(self, fd):
@@ -69,18 +68,19 @@ class Service:
 
     def print_msg(self, message):
         print_msg(message, self.output_fd)
-        
-    def start_creation(self, num_steps, message):
-        self.num_steps = num_steps
-        self.cur_step = 0
-        self.print_msg(message)
 
-    def step(self, message):
-        self.cur_step += 1
-        self.print_msg("  [%d/%d]: %s" % (self.cur_step, self.num_steps, message))
+    def step(self, message, method):
+        self.steps.append((message, method))
 
-    def done_creation(self):
-        self.cur_step = -1
-        self.num_steps = -1
+    def start_creation(self, message):
+        self.print_msg(message)
+
+        step = 0
+        for (message, method) in self.steps:
+            self.print_msg("  [%d/%d]: %s" % (step, len(self.steps), message))
+            method()
+            step += 1
+        
         self.print_msg("done configuring %s." % self.service_name)
 
+        self.steps = []
diff --git a/ipa-server/ipaserver/webguiinstance.py b/ipa-server/ipaserver/webguiinstance.py
index 2d41f30fb..11727e113 100644
--- a/ipa-server/ipaserver/webguiinstance.py
+++ b/ipa-server/ipaserver/webguiinstance.py
@@ -24,14 +24,6 @@ class WebGuiInstance(service.Service):
         service.Service.__init__(self, "ipa-webgui")
 
     def create_instance(self):
-        self.start_creation(2, "Configuring ipa-webgui")
-
-        self.step("starting ipa-webgui")
-        service.start("ipa-webgui")
-
-        self.step("configuring ipa-webgui to start on boot")
-        service.chkconfig_on("ipa-webgui")
-
-        self.done_creation()
-
-
+        self.step("starting ipa-webgui", self.start)
+        self.step("configuring ipa-webgui to start on boot", self.chkconfig_on)
+        self.start_creation("Configuring ipa-webgui")