test_integration.config: Do not save the input environment

Using the input environment saved in self._session_env
outside of the config loading meant that methods of
configuration other than environment variables wouldn't
be possible.

Restructure the roles/extra_roles to not depend on _session_env.

Part of the work for: https://fedorahosted.org/freeipa/ticket/3938

Reviewed-By: Tomas Babej <tbabej@redhat.com>

2 files changed, 25 insertions, 36 deletions

diff --git a/ipatests/test_integration/config.py b/ipatests/test_integration/config.py
index 1c2b6a50d..4821b967f 100644
--- a/ipatests/test_integration/config.py
+++ b/ipatests/test_integration/config.py
@@ -197,7 +197,8 @@ class Config(object):
 
                 for i, host in enumerate(hosts, start=1):
                     suffix = '%s%s' % (role.upper(), i)
-                    prefix = 'TESTHOST_' if role in domain.extra_roles else ''
+                    prefix = ('' if role in domain.static_roles
+                              else TESTHOST_PREFIX)
 
                     ext_hostname = host.external_hostname
                     env['%s%s%s' % (prefix, suffix,
@@ -270,7 +271,7 @@ def env_normalize(env):
 
     def extend(name, name2):
         value = env.get(name2)
-        if value:
+        if value and value not in env[name].split(' '):
             env[name] += ' ' + value
     extend('CLIENT_env1', 'CLIENT2')
     extend('CLIENT_env1', 'CLIENT2_env1')
@@ -292,48 +293,36 @@ class Domain(object):
         self.realm = self.name.upper()
         self.basedn = DN(*(('dc', p) for p in name.split('.')))
 
-        self._extra_roles = tuple()  # Serves as a cache for the domain roles
-        self._session_env = None
-
     @property
     def roles(self):
-        return self.static_roles + self.extra_roles
+        return sorted(set(host.role for host in self.hosts))
 
     @property
     def static_roles(self):
         # Specific roles for each domain type are hardcoded
         if self.type == 'IPA':
-            return ('master', 'client', 'replica', 'other')
+            return ('master', 'replica', 'client', 'other')
         else:
             return ('ad',)
 
     @property
     def extra_roles(self):
-        if self._extra_roles:
-            return self._extra_roles
-
-        roles = ()
-
-        # Extra roles can be defined via env variables of form TESTHOST_key_envX
-        for variable in self._session_env:
-            if variable.startswith('TESTHOST'):
-
-                variable_split = variable.split('_')
-
-                defines_extra_role = (
-                    variable.endswith(self._env) and
-                    # at least 3 parts, as in TESTHOST_key_env1
-                    len(variable_split) > 2 and
-                    # prohibit redefining roles
-                    variable_split[-2].lower() not in roles
-                    )
-
-                if defines_extra_role:
-                    key = '_'.join(variable_split[1:-1])
-                    roles += (key.lower(),)
-
-        self._extra_roles = roles
-        return roles
+        return [role for role in self.roles if role not in self.static_roles]
+
+    def _roles_from_env(self, env):
+        for role in self.static_roles:
+            yield role
+
+        # Extra roles are defined via env variables of form TESTHOST_key_envX
+        roles = set()
+        for var in sorted(env):
+            if var.startswith(TESTHOST_PREFIX) and var.endswith(self._env):
+                variable_split = var.split('_')
+                role_name = '_'.join(variable_split[1:-1])
+                if (role_name and not role_name[-1].isdigit()):
+                    roles.add(role_name.lower())
+        for role in sorted(roles):
+            yield role
 
     @classmethod
     def from_env(cls, env, config, index, domain_type):
@@ -349,10 +338,9 @@ class Domain(object):
         master_env = '%s_env%s' % (master_role, index)
         hostname, dot, domain_name = env[master_env].partition('.')
         self = cls(config, domain_name, index, domain_type)
-        self._session_env = env
 
-        for role in self.roles:
-            prefix = 'TESTHOST_' if role in self.extra_roles else ''
+        for role in self._roles_from_env(env):
+            prefix = '' if role in self.static_roles else TESTHOST_PREFIX
             value = env.get('%s%s%s' % (prefix, role.upper(), self._env), '')
 
             for index, hostname in enumerate(value.split(), start=1):
diff --git a/ipatests/test_integration/host.py b/ipatests/test_integration/host.py
index 507e19ed6..a35602a97 100644
--- a/ipatests/test_integration/host.py
+++ b/ipatests/test_integration/host.py
@@ -128,7 +128,8 @@ class BaseHost(object):
         env['MYBEAKERHOSTNAME'] = self.external_hostname
         env['MYIP'] = self.ip
 
-        prefix = 'TESTHOST_' if self.role in self.domain.extra_roles else ''
+        prefix = ('' if self.role in self.domain.static_roles
+                  else TESTHOST_PREFIX)
         env['MYROLE'] = '%s%s%s' % (prefix, role, self.domain._env)
         env['MYENV'] = str(self.domain.index)