On 10/30/2013 03:57 PM, Tomas Babej wrote:
On 10/29/2013 01:00 PM, Petr Viktorin wrote:
On 10/24/2013 12:20 PM, Tomas Babej wrote:
On 10/22/2013 10:44 AM, Petr Viktorin wrote:
On 10/22/2013 10:09 AM, Tomas Babej wrote:
On 10/22/2013 09:54 AM, Petr Viktorin wrote:
On 10/22/2013 09:20 AM, Tomas Babej wrote:
Hi,

Adds support for host definition by a environment variables of the
following form:

KEYWORDHOST__envX, where X is the number of the environment
for which host referenced by a keyword should be defined.

You can also optionally use KEYWORDHOST__IP_envX to define
the IP address directly, otherwise the framework will try to resolve
the hostname.

Adds a required_keyword_hosts attribute to the IntegrationTest
class,
which can test developer use to specify the keyword hosts that this
particular test requires. If not all required keyword hosts are
available, the test will be skipped.

All keyword hosts are accessible to the IntegrationTests via the
keyword_hosts attribute, which contains a dictionary keyed by the
keywords.


Why is this necessary?
What's wrong with just extending the current scheme with more roles?



The need for keyword hosts arised with the implementation of the
legacy
client test suite.

As each of these tests needs a precise type (pre-defined and
pre-configured) of VM, and not a generic client, you need to restrict
the test case to specific host type.

One test might be restricted to RHEL 5.10 with nss-pam-ldapd,
another to
FreeBSD, next one to CentOS with nss-pam-ldapd, next to CentOS with
old
version of SSSD...

Each testcase that needs a new type of preconfigured host, we would
need
to introduce a new role, which would need to be integrated in the
framework. In such implementation, we would lose loose coupling
between
the test framework and the test themselves, and make them less
pluggable.

The framework itself (excluding the configuration part) should be able
to handle this nicely using the existing role mechanism. It's true
that in some places it's currently hard-wired to just a few roles, but
supporting completely custom roles shouldn't be a problem.
I'd prefer if this system was used, rather than basically adding a
second role system, which we'd have to support even if we get rid of
the current config part.

The envvar-based configuration is not as flexible, but I'd rather make
this part somewhat unpleasant than making the framework complex. We
plan to move to a simpler configuration method anyway.
That said, you can basically keep the variable name scheme you use in
this patch; just maybe use TESTHOST rather than KEYWORDHOST.


I rewired the patch to use the current role system.

Please look if you have any additional issues.


I only have two naming nitpicks.
- The roles aren't really "dynamic"; eventually all the "dynamicness"
should be specific just to the envvar configuration system, and a few
shortcuts like `replicas` for `host_by_role('replica')`. I think
"extra" would be a better term.
- The environment variable names could be more descriptive. They store
a hostname, not a role, so instead of $ROLE_<keyword>_envX I'd prefer
$TESTHOST_<role>_envX

Other than that the changes look great!


Thanks, updated patch attached.


ACK, pushed to
master: b1bffb5ecad0fdaa2f560efd2b75c76bedc4423c
ipa-3-3: 960c6bf301fe3a00205a895acabe47bac5ac9349


--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to