Re: [Freeipa-devel] [PATCHES 100-106] Initial implementation of AD integration tests
On 10/22/2013 02:24 PM, Tomas Babej wrote: On 10/22/2013 02:15 PM, Tomas Babej wrote: On 10/22/2013 12:27 PM, Tomas Babej wrote: On 10/22/2013 10:37 AM, Petr Viktorin wrote: Replying to one part only: On 10/21/2013 04:50 PM, Tomas Babej wrote: On 10/16/2013 03:44 PM, Petr Viktorin wrote: I still think it would be simpler if IPA and AD domains shared the numbering namespace (users would need to define $AD_env2; if they had $MASTER_env1 and $AD_env1 they would be in the same Domain). But if we use _env1 for both the AD and the IPA domain, they need to be separated in Domain.from_env. With patch 0101 both MASTER_env1 and AD_env1 will be in both domains[0] and ad_domains[0]. I would rather not join IPA and AD domains as they even cannot be in the same domain, as the service records would clash. So these will always be separate / sub / super domain relationship. You're right that they should never share the same domain. But you should never say never, especially in testing -- what if we'll want to, in the future, test that the records *do* clash, or that IPA refuses to install in an AD domain? You could still set AD_env1 and MASTER_env1 to have the same domain. Another problem is that they are now separate namespaces. In all code that deals with domains you have to deal separately with the list of AD domains and separately with IPA domains. This makes every piece of code that doesn't care much about what type of domain it's dealing with (configuration, listing, possible automation scripts for turning on the VMs, etc.) more complicated. Also, in this scheme, adding a new type of domain would be quite hard, especially after more code is written with this split in mind. Do keep the domain type, though. tl;dr I'd really prefer domain 1 (IPA); domain 2 (AD) rather than IPA domain 1; AD domain 1. This will, however, require filtering the domains depending on the fact whether they contain AD or not. If a testcase wants to access an AD domain, it will still need to loop over the list of domains to see which ones are of AD type. Any code that does not care what domain type it's dealing with, can easily access all the domains by chaining the respective iterables. We could have a wrapper in the Config class for that, along the lines of get_all_domains(). So what I see here is that we're trading one complexity over another. I think we can agree on your approach since it hides the complexity from the user, especially in the ipa-test-config, which I admit is getting rather ugly, as we need to introduce new option there and that causes splitting. If needed we can have a special check that would reject IPA masters in AD domains and vice versa, if that really turns out to be necessary. With this check we should be fine. As we already pass ad_domain flag to Domain.from_env, I did incorporate code that joins the machines to the domain depending on the their role. Is that a viable solution for you? Sorry. I think this design is less sustainable than having a shared namespace for the domains. I'll send revised patchset soon. Updated patchset attached. Patch 105: Typo: 'Sets DNS ib given host for trust with the given AD ^^ Should be in, right? I'll fix it before pushing. Otherwise, these are good to go! Patch 106: In ADTrustBase, it looks like if test_install_adtrust or test_configure_dns_and_time fail, it doesn't make much sense to run the other tests. If that's the case they can go in an install() classmethod. Same with test_establish_trust* in the subclasses. Also, there's a typo in test_estabilish_trusts several times. -^ -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 0313-0314 Integration test fixes
Here are fixes for two bugs found while running integration tests under
Beaker.
--
Petr³
From c568f9c83eaf6e579efaf03fc9580ae46cf0b61c Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 24 Oct 2013 13:55:47 +0200
Subject: [PATCH] Tests: mkdir_recursive: Don't fail when top-level directory
doesn't exist
When the directory directly under root (e.g. /etc) did not exist,
mkdir_recursive failed.
Fix the issue.
---
ipatests/test_integration/transport.py | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ipatests/test_integration/transport.py b/ipatests/test_integration/transport.py
index a0bd3700ac4e9887804f021d1a2fb4eb050457d3..9b3dd5be5fc4df7b235f52e2f5dcdb9fc608ad23 100644
--- a/ipatests/test_integration/transport.py
+++ b/ipatests/test_integration/transport.py
@@ -87,10 +87,10 @@ def start_shell(self, argv, log_stdout=True):
def mkdir_recursive(self, path):
`mkdir -p` on the remote host
-if not path or path == '/':
-raise ValueError('Invalid path')
-if not self.file_exists(path or '/'):
-self.mkdir_recursive(os.path.dirname(path))
+if not self.file_exists(path):
+parent_path = os.path.dirname(path)
+if path != parent_path:
+self.mkdir_recursive(parent_path)
self.mkdir(path)
def get_file(self, remotepath, localpath):
--
1.8.3.1
From f32b4a582974e36d63913ddefdcd29965bee5848 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 23 Oct 2013 14:05:39 +0200
Subject: [PATCH] beakerlib plugin: Don't try to submit logs if they are
missing
---
ipatests/beakerlib_plugin.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/ipatests/beakerlib_plugin.py b/ipatests/beakerlib_plugin.py
index 45f34c6a6460f52fa0b5b9fb9b8e8bb2975ccafa..71c1df537fb98ae49b159bd1e80381a37dc8a51f 100644
--- a/ipatests/beakerlib_plugin.py
+++ b/ipatests/beakerlib_plugin.py
@@ -116,6 +116,7 @@ def collect_logs(self, logs_to_collect):
raiseonerr=False)
if cmd.returncode:
self.log.warn('Could not collect all requested logs')
+return
# Copy and unpack on the local side
topdirname = tempfile.mkdtemp()
--
1.8.3.1
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0201] Report error if RFC 4533 initialization failed
On 10/23/2013 05:14 PM, Petr Spacek wrote: Hello, this patch belongs to 4.0 release. It allows the user to catch some mis-configurations. It produces error messages like this: LDAP error: Critical extension is unavailable: unable to start SyncRepl session: is RFC 4533 supported on LDAP server? ACK. Patch works and looks good. Regards, Tomas Hozza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 100-106] Initial implementation of AD integration tests
On 10/24/2013 01:29 PM, Petr Viktorin wrote: On 10/22/2013 02:24 PM, Tomas Babej wrote: On 10/22/2013 02:15 PM, Tomas Babej wrote: On 10/22/2013 12:27 PM, Tomas Babej wrote: On 10/22/2013 10:37 AM, Petr Viktorin wrote: Replying to one part only: On 10/21/2013 04:50 PM, Tomas Babej wrote: On 10/16/2013 03:44 PM, Petr Viktorin wrote: I still think it would be simpler if IPA and AD domains shared the numbering namespace (users would need to define $AD_env2; if they had $MASTER_env1 and $AD_env1 they would be in the same Domain). But if we use _env1 for both the AD and the IPA domain, they need to be separated in Domain.from_env. With patch 0101 both MASTER_env1 and AD_env1 will be in both domains[0] and ad_domains[0]. I would rather not join IPA and AD domains as they even cannot be in the same domain, as the service records would clash. So these will always be separate / sub / super domain relationship. You're right that they should never share the same domain. But you should never say never, especially in testing -- what if we'll want to, in the future, test that the records *do* clash, or that IPA refuses to install in an AD domain? You could still set AD_env1 and MASTER_env1 to have the same domain. Another problem is that they are now separate namespaces. In all code that deals with domains you have to deal separately with the list of AD domains and separately with IPA domains. This makes every piece of code that doesn't care much about what type of domain it's dealing with (configuration, listing, possible automation scripts for turning on the VMs, etc.) more complicated. Also, in this scheme, adding a new type of domain would be quite hard, especially after more code is written with this split in mind. Do keep the domain type, though. tl;dr I'd really prefer domain 1 (IPA); domain 2 (AD) rather than IPA domain 1; AD domain 1. This will, however, require filtering the domains depending on the fact whether they contain AD or not. If a testcase wants to access an AD domain, it will still need to loop over the list of domains to see which ones are of AD type. Any code that does not care what domain type it's dealing with, can easily access all the domains by chaining the respective iterables. We could have a wrapper in the Config class for that, along the lines of get_all_domains(). So what I see here is that we're trading one complexity over another. I think we can agree on your approach since it hides the complexity from the user, especially in the ipa-test-config, which I admit is getting rather ugly, as we need to introduce new option there and that causes splitting. If needed we can have a special check that would reject IPA masters in AD domains and vice versa, if that really turns out to be necessary. With this check we should be fine. As we already pass ad_domain flag to Domain.from_env, I did incorporate code that joins the machines to the domain depending on the their role. Is that a viable solution for you? Sorry. I think this design is less sustainable than having a shared namespace for the domains. I'll send revised patchset soon. Updated patchset attached. Patch 105: Typo: 'Sets DNS ib given host for trust with the given AD ^^ Should be in, right? I'll fix it before pushing. c Otherwise, these are good to go! Patch 106: In ADTrustBase, it looks like if test_install_adtrust or test_configure_dns_and_time fail, it doesn't make much sense to run the other tests. If that's the case they can go in an install() classmethod. Same with test_establish_trust* in the subclasses. I made them part of the install() classmethod. As for the test_estabilish_trust, I would have to still override that in each class that uses it, since all of them use it in a slightly different way. Also, there's a typo in test_estabilish_trusts several times. -^ Typo fixed. Also attaching a patch that fixes the same type in the other parts of the codebase. -- Tomas Babej Associate Software Engeneer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From e7f777b229534a65eb7f6bc39e77c2a5b5c51983 Mon Sep 17 00:00:00 2001 From: Tomas Babej tomasba...@gmail.com Date: Thu, 24 Oct 2013 14:24:33 +0200 Subject: [PATCH] trusts: Fix typo in error message for realm-domain mismatch --- ipalib/plugins/trust.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py index 0d651f8861446cf8d31eb1ea303237bcd2b73201..32a93834394273c9f896ff5fd17bfcc753fe7b8e 100644 --- a/ipalib/plugins/trust.py +++ b/ipalib/plugins/trust.py @@ -424,14 +424,14 @@ sides. ) # If domain name and realm does not match, IPA server is not be able -# to estabilish trust with Active Directory. +# to establish trust with Active Directory. realm_not_matching_domain = (api.env.domain.upper() !=
[Freeipa-devel] [PATCH] 0077 Do not roll back failed client installation on server
Hello, This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3990 -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. From 2ec44821d5f422c6ab20202fcdc9e1f1eaad7ab1 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic akriv...@redhat.com Date: Thu, 24 Oct 2013 17:43:21 +0200 Subject: [PATCH] Do not roll back failed client installation on server In case of a failed enrollment, IPA client rolls back any changes it has made to the system. In order to have a more debuggable setup, do not roll back these changes in the case of an IPA server install. https://fedorahosted.org/freeipa/ticket/3990 --- ipa-client/ipa-install/ipa-client-install | 5 + 1 file changed, 5 insertions(+) diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index cf27788f8c189721a1f644fa5841466abfbca54e..1f66ae5d635d98ba45df13d92ca7982068d94752 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -2606,6 +2606,11 @@ def main(): if options.force: root_logger.warning( Installation failed. Force set so not rolling back changes.) +elif options.on_master: +root_logger.warning( +Installation failed. As this is IPA server, changes will not +be rolled back. +) else: root_logger.error(Installation failed. Rolling back changes.) options.unattended = True -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 0078 Make sure nsds5ReplicaStripAttrs is set on agreements
Hello, This patch addresses ticket https://fedorahosted.org/freeipa/ticket/3989 -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc. From ea2d6ce79b674ddaa681fc64d1fa4de3ee308ebd Mon Sep 17 00:00:00 2001 From: Ana Krivokapic akriv...@redhat.com Date: Thu, 24 Oct 2013 20:36:30 +0200 Subject: [PATCH] Make sure nsds5ReplicaStripAttrs is set on agreements Add nsds5ReplicaStripAttrs to the agreement LDAP entry before the agreement is created. https://fedorahosted.org/freeipa/ticket/3989 --- ipaserver/install/replication.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py index 6269ba6862eaf37c4f8bb560b3358f8f692dec2c..4d8a4687e162155d7855e11ba5048bed2ff13fa5 100644 --- a/ipaserver/install/replication.py +++ b/ipaserver/install/replication.py @@ -627,6 +627,7 @@ def setup_agreement(self, a_conn, b_hostname, port=389, if iswinsync: self.setup_winsync_agmt(entry, win_subtree) +entry['nsds5ReplicaStripAttrs'] = [ .join(STRIP_ATTRS)] a_conn.add_entry(entry) try: @@ -639,8 +640,6 @@ def setup_agreement(self, a_conn, b_hostname, port=389, # that we will have to set the memberof fixup task self.need_memberof_fixup = True -entry['nsds5ReplicaStripAttrs'] = [ .join(STRIP_ATTRS)] - wait_for_entry(a_conn, entry) def needs_memberof_fixup(self): -- 1.8.3.1 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
