[Freeipa-devel] [TEST][patch-0037]Fixes of dnssec tests

[Martin Basti]

On 06.05.2016 11:14, Oleg Fayans wrote:
>
> On 05/06/2016 09:48 AM, Martin Basti wrote:
>>
>> On 06.05.2016 09:36, Oleg Fayans wrote:
>>> Tests are finally stable:
>>>
>>> ============================= test session starts
>>> ==============================
>>> platform linux2 -- Python 2.7.11 -- py-1.4.30 -- pytest-2.7.3
>>> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
>>> plugins: multihost, sourceorder
>>> collected 8 items
>>>
>>> test_integration/test_dnssec.py ........
>>>
>>> ========================= 8 passed in 5561.48 seconds
>>> ==========================
>>>
>>>
>>>
>>>
>>>
>> PATCH 38 LGTM
>>
>> PATCH 37 IIRC I refused to accept workaround for this issue when you
>> send this (almost the same) patch for first time, are you sure that we
>> want to hide real issues in tests, to just have green color there?
>>
> The underlying issue is 7 months old. Latest update in the issue from
> Peter Spacek is: "I do not have time to investigate this issue now",
> which means, that it will stay there for unpredictable amount of time
> more. If we want to have a "green" jenkins that actually tests existing
> features, we have to accept workarounds for such long-term issues
>
>> Martin
I leave decision if push this or not to different people, however I will 
do review on this.


NACK

1)
Why do you change sleep time? How is it related to workaround?

-        time.sleep(20)  # sleep a bit until LDAP changes are applied to DNS
+        time.sleep(10)  # sleep a bit until LDAP changes are applied to DNS


2)
why do you removes sleep from several places? How is this related to 
workaround?
@@ -281,13 +302,19 @@ class TestInstallDNSSECFirst(IntegrationTest):
              "--a-rec=" + self.master.ip
          ]
          self.master.run_command(args)
-        time.sleep(10)  # sleep a bit until data are provided by 
bind-dyndb-ldap

          args = [
              "ipa", "dnsrecord-add", root_zone, self.master.domain.name,
              "--ns-rec=" + self.master.hostname
          ]
          self.master.run_command(args)

3)
You restart the same replica twice
+        time.sleep(10)  # sleep a bit until LDAP changes are applied to DNS
+        # A workaround for ticket N 5348
+        self.replicas[0].run_command(["systemctl", "restart",
+                                      "named-pkcs11.service"])
+        self.replicas[0].run_command(["systemctl", "restart",
+                                      "named-pkcs11.service"])
+        # End of workaround

4)
Can you create a function doing workaround instead of copying the same 
code several times?

something like
def restart_named(*args):
     for host in args:
        # host$ systemctl restart named-pkcs11

where args are instances self.host, or self.master, or self.replica

5)
Why did you removed this comment?
-        # wait until zone is signed

6)
I'm not sure, but is sleep there needed? Because restart of named will 
download all LDAP data again. If timeout is required maybe reason why 
time is there should be rephrased, to something like, make sure the 
dnssec keys were exported for named, or so.
+        time.sleep(10)  # sleep a bit until LDAP changes are applied to DNS
+        # A workaround for ticket N 5348
+        self.master.run_command(["systemctl", "restart",
+                                 "named-pkcs11.service"])
+        self.replicas[0].run_command(["systemctl", "restart",
+                                      "named-pkcs11.service"])
+        # End of workaround

Martin^2