On 29.6.2016 18:48, Lenka Doudova wrote:
> On 06/27/2016 11:05 AM, Lenka Doudova wrote:
>> On 06/27/2016 10:33 AM, Martin Babinsky wrote:
>>> On 06/27/2016 10:28 AM, Petr Spacek wrote:
>>>> On 27.6.2016 10:26, Petr Spacek wrote:
>>>>> On 27.6.2016 10:18, Martin Babinsky wrote:
>>>>>> On 06/27/2016 10:04 AM, Petr Vobornik wrote:
>>>>>>> On 06/27/2016 09:42 AM, Lenka Doudova wrote:
>>>>>>>> Hi!
>>>>>>>> With newly created AD machines in Brno lab, existing trust tests fail 
>>>>>>>> on
>>>>>>>> 'ipa dnsforwardzone-add' command claiming the zone is already present,
>>>>>>>> as new AD domain is dom-221.idm.lab.eng.brq.redhat.com.
>>>>>>>> To prevent these failures I prepared attached patch, that will still
>>>>>>>> attempt to add the forward zone, but in case of non-zero return code
>>>>>>>> will check the message if it says that the forward zone is already
>>>>>>>> configured, and lets the tests continue, if it is so.
>>>>>>>> Lenka
>>>>>>> Current approach expects that every error of ipa dnsforward-add here
>>>>>>> will mean that the zone exists. So it might hide other issues - not very
>>>>>>> good.
>>>>>>> On the other hand it is not very robust to parse error message.
>>>>>>> Question for general audience: What do you think if IPA client's exit
>>>>>>> status would be the IPA error code instead of "1" for every error. E.g.
>>>>>>> in DuplicateEntry case it's 4002.
>>>>>>> Btw, this is not a NACK.
>>>>>> Well AFAIK the exit status on POSIX systems is encoded into a single
>>>>>> byte so
>>>>>> you cannot have the return value greater that 255. We would have to 
>>>>>> devise
>>>>>> some mapping between our XMLRPC status codes and subprocess return codes.
>>>>>> Some of our exceptions have defined return values outside plain '1', e.g.
>>>>>> NotFound has return value of 2. It would be possible to extend this
>>>>>> concept on
>>>>>> other common errors.
>>>>> Even more importantly, the forward zone is completely unnecessary because
>>>>> DNS
>>>>> when DNS is set up properly. I would simply remove the dnsforwardzone-add.
>>>> Grr, I meant this:
>>>> Even more importantly, the forward zone is completely unnecessary when DNS 
>>>> is
>>>> set up properly. I would simply remove the dnsforwardzone-add.
>>> +1, our tests should not fiddle with the provisioned environment as much as
>>> they sometimes do.
>> Well, I have nothing against removing it completely, but left it there just
>> because with previous AD machines with "wild" domains it was necessary.
>> Looking at the code, your suggestion would probably mean to entirely remove
>> method configure_dns_for_trust from ipatests/test_integration/tasks.py,
>> right? I'll have to verify this won't break anything else.
>> Lenka
> Hi,
> to get back to this issue: do we really want to have the DNS configuration
> method removed? I mean, we no longer need it for our CI tests, with new AD VMs
> it works without it, but should somebody else with different setup run these
> tests, they could experience failures because their AD domain would not be
> configured in DNS by default and the test would no longer provide that
> configuration. It doesn't feel right to delete something we needed before but
> don't need anymore, in case somebody else is depending on the same
> configuration. But of course, I'll abide by your counsel.
> In case the call on DNS configuration method really is removed, should I
> remove even it's definition? It's not used anywhere else, so it would be quite
> logical.

Feel free to keep empty method around as a "hook" for other people. The
important thing is that it should do nothing by default.

Petr^2 Spacek

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to