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

[Tomas Babej]

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.
>
>
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

Attaching the patch 100 I missed.

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131022/68fb4990/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0100-9-ipatests-Add-Active-Directory-support-to-configurati.patch
Type: text/x-patch
Size: 6254 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131022/68fb4990/attachment.bin>