[Freeipa-devel] [PATCHES 100-106] Initial implementation of AD integration tests

[Tomas Babej]

On 10/16/2013 03:44 PM, Petr Viktorin wrote:
> On 10/14/2013 05:47 PM, Tomas Babej wrote:
>> Hi,
>>
>> Sending the updated version of the patchset. It includes many fixes,
>> plus incorporations of the comments by the QE. Tests are now broken down
>> into multiple test cases.
>>
>> This requires my patch 119.
>>
>> Tomas
>
> Patches 0100 & 0101:
>
> 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.

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?

> Also ipa-test-config should have an --ad-domain option to get info 
> about the AD domain.
>
>
Fixed, also modified the man page.
> Patch 0102: Looks good
>
>
> Patch 0103:
> Looks like run_repeatedly should always be called with an assert in 
> front. We may want it to raise an exception directly if it times out, 
> so forgetting the assert won't hide errors.
>
Fixed.
>
> Patch 0104: Looks good
>
>
> Patch 0105:
> In /ipatests/man/ipa-test-task.1:
> - Typo in  - s/generatee/generate/
> - Subcommand names have unescaped hyphens (e.g. 
> configure\-dns-for-trust should be configure\-dns\-for\-trust)
> - The last subcommand should be sync-time
>
Fixed.
> Wrappers for the tasks need to be added to ipatests/ipa-test-task.
>
Phew, fixed!
> Nitpicks:
> - In configure_dns_for_trust:is_subdomain, lockstep iteration over two 
> lists at once is better done with zip() than `for i in range(len(...))`.
> - In estabilish_trust_with_ad, don't use mutable values for argument 
> defaults; do `extra_args=()` and `+ list(extra_args)`
> - I can't say I'm a fan of configure_auth_to_local_rule, but I guess 
> it's OK for tests.
>
Fixed.
>
> Patch 0106: Looks OK; unfortunately I don't have an AD machine to test 
> functionality
>
>
>

I also incorporated Alexander's comments.

Thanks for reviews, Alexander and Petr!

Update patches attached.

-- 
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0106-7-ipatests-Add-AD-integration-test-case.patch
Type: text/x-patch
Size: 7765 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/17d235aa/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0105-7-ipatests-Add-AD-integration-related-tasks.patch
Type: text/x-patch
Size: 17232 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/17d235aa/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0104-7-ipatests-Add-WinHost-class.patch
Type: text/x-patch
Size: 1733 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/17d235aa/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0103-7-ipatests-Create-util-module-for-ipatests.patch
Type: text/x-patch
Size: 2896 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/17d235aa/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0102-7-ipatests-Extend-IntegrationTest-with-multiple-AD-dom.patch
Type: text/x-patch
Size: 2840 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/17d235aa/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0101-7-ipatests-Extend-domain-object-with-ad-role-support-a.patch
Type: text/x-patch
Size: 6829 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/17d235aa/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0100-7-ipatests-Add-Active-Directory-support-to-configurati.patch
Type: text/x-patch
Size: 5659 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131021/17d235aa/attachment-0006.bin>