On 05/06/2016 11:42 AM, Martin Basti wrote:
> 
> 
> 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

10 seconds proved to be enough even in our super-slow brno rhevm lab

> 
> 
> 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)

Because it's more reasonable to make changes on all hosts and then wait.

> 
> 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
My bad.

> 
> 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

Now, that's a marvelous idea! Implemented. And put the sleep interval in
it too just to keep it in one place

> 
> 5)
> Why did you removed this comment?
> -        # wait until zone is signed
Because this is kin of obvious from the name of the function:
wait_until_record_is_signed

> 
> 6)
> I'm not sure, but is sleep there needed? Because restart of named will
> download all LDAP data again.
It turns out, without this interval the restart does not always help

> 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.

Rephrased

> +        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

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From f24288061f402761180d9dfab5768e435e84ebfd Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Fri, 6 May 2016 12:21:43 +0200
Subject: [PATCH] A workaround for ticket N 5348

A freshly created dnssec-enabled zone does not always display the signature
until you restart named-pkcs11. Added restarting of this service after each
dnssec-enabled zone.

https://fedorahosted.org/freeipa/ticket/5348
---
 ipatests/test_integration/test_dnssec.py | 41 ++++++++++++++------------------
 1 file changed, 18 insertions(+), 23 deletions(-)

diff --git a/ipatests/test_integration/test_dnssec.py b/ipatests/test_integration/test_dnssec.py
index e90fb1f477ab50050f619399ee168c0e4b248ac2..0caee214a1dd9a0e66399ddb5a7d3f18338282f8 100644
--- a/ipatests/test_integration/test_dnssec.py
+++ b/ipatests/test_integration/test_dnssec.py
@@ -71,6 +71,14 @@ def wait_until_record_is_signed(nameserver, record, log, rtype="SOA",
     return False
 
 
+def restart_named(hostlist):
+        # A workaround for ticket N 5348
+        time.sleep(10)  # wait till dnssec key is exported to named
+        for host in hostlist:
+            host.run_command(["systemctl", "restart",
+                              "named-pkcs11.service"])
+
+
 class TestInstallDNSSECLast(IntegrationTest):
     """Simple DNSSEC test
 
@@ -105,6 +113,7 @@ class TestInstallDNSSECLast(IntegrationTest):
         ]
         self.master.run_command(args)
 
+        restart_named([self.master, self.replicas[0]])
         # test master
         assert wait_until_record_is_signed(
             self.master.ip, test_zone, self.log, timeout=100
@@ -125,6 +134,7 @@ class TestInstallDNSSECLast(IntegrationTest):
         ]
         self.replicas[0].run_command(args)
 
+        restart_named([self.replicas[0]])
         # test replica
         assert wait_until_record_is_signed(
             self.replicas[0].ip, test_zone_repl, self.log, timeout=300
@@ -170,8 +180,7 @@ class TestInstallDNSSECLast(IntegrationTest):
         ]
         self.master.run_command(args)
 
-        time.sleep(20)  # sleep a bit until LDAP changes are applied to DNS
-
+        restart_named([self.master])
         # test master
         assert wait_until_record_is_signed(
             self.master.ip, test_zone, self.log, timeout=100
@@ -199,7 +208,7 @@ class TestInstallDNSSECLast(IntegrationTest):
         ]
         self.master.run_command(args)
 
-        time.sleep(20)  # sleep a bit until LDAP changes are applied to DNS
+        time.sleep(10)  # wait till dnssec key is exported to named
 
         # test master
         assert not is_record_signed(
@@ -219,8 +228,7 @@ class TestInstallDNSSECLast(IntegrationTest):
         ]
         self.master.run_command(args)
 
-        time.sleep(20)  # sleep a bit until LDAP changes are applied to DNS
-
+        restart_named([self.master, self.replicas[0]])
         # test master
         assert wait_until_record_is_signed(
             self.master.ip, test_zone_repl, self.log, timeout=100
@@ -281,14 +289,13 @@ 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)
-
+        restart_named([self.master, self.replicas[0]])
         # test master
         assert wait_until_record_is_signed(
             self.master.ip, root_zone, self.log, timeout=100
@@ -319,12 +326,10 @@ class TestInstallDNSSECFirst(IntegrationTest):
             "--ns-rec=" + self.master.hostname
         ]
         self.master.run_command(args)
-
-        # wait until zone is signed
+        restart_named([self.master, self.replicas[0]])
         assert wait_until_record_is_signed(
             self.master.ip, example_test_zone, self.log, timeout=100
         ), "Zone %s is not signed (master)" % example_test_zone
-        # wait until zone is signed
         assert wait_until_record_is_signed(
             self.replicas[0].ip, example_test_zone, self.log, timeout=200
         ), "Zone %s is not signed (replica)" % example_test_zone
@@ -366,7 +371,6 @@ class TestInstallDNSSECFirst(IntegrationTest):
 
         self.master.run_command(args)
 
-        # wait until DS records it replicated
         assert wait_until_record_is_signed(
             self.replicas[0].ip, example_test_zone, self.log, timeout=100,
             rtype="DS"
@@ -457,11 +461,10 @@ class TestMigrateDNSSECMaster(IntegrationTest):
 
         self.master.run_command(args)
 
-        # wait until zone is signed
+        restart_named([self.master, self.replicas[0]])
         assert wait_until_record_is_signed(
             self.master.ip, example_test_zone, self.log, timeout=100
         ), "Zone %s is not signed (master)" % example_test_zone
-        # wait until zone is signed
         assert wait_until_record_is_signed(
             self.replicas[0].ip, example_test_zone, self.log, timeout=200
         ), "Zone %s is not signed (replica)" % example_test_zone
@@ -493,11 +496,9 @@ class TestMigrateDNSSECMaster(IntegrationTest):
         ]
         self.replicas[0].run_command(args)
 
-        # wait until zone is signed
         assert wait_until_record_is_signed(
             self.master.ip, example_test_zone, self.log, timeout=100
         ), "Zone %s is not signed after migration (master)" % example_test_zone
-        # wait until zone is signed
         assert wait_until_record_is_signed(
             self.replicas[0].ip, example_test_zone, self.log, timeout=200
         ), "Zone %s is not signed after migration (replica)" % example_test_zone
@@ -513,13 +514,11 @@ class TestMigrateDNSSECMaster(IntegrationTest):
             "--skip-overlap-check",
         ]
         self.replicas[0].run_command(args)
-
-        # wait until zone is signed
+        restart_named([self.master, self.replicas[0]])
         assert wait_until_record_is_signed(
             self.replicas[0].ip, example2_test_zone, self.log, timeout=100
         ), ("Zone %s is not signed after migration (replica - dnssec master)"
             % example2_test_zone)
-        # wait until zone is signed
         assert wait_until_record_is_signed(
             self.master.ip, example2_test_zone, self.log, timeout=200
         ), ("Zone %s is not signed after migration (master)"
@@ -529,12 +528,10 @@ class TestMigrateDNSSECMaster(IntegrationTest):
         tasks.install_replica(self.master, self.replicas[1], setup_dns=True)
 
         # test if originial zones are signed on new replica
-        # wait until zone is signed
         assert wait_until_record_is_signed(
             self.replicas[1].ip, example_test_zone, self.log, timeout=200
         ), ("Zone %s is not signed (new replica)"
             % example_test_zone)
-        # wait until zone is signed
         assert wait_until_record_is_signed(
             self.replicas[1].ip, example2_test_zone, self.log, timeout=200
         ), ("Zone %s is not signed (new replica)"
@@ -546,8 +543,7 @@ class TestMigrateDNSSECMaster(IntegrationTest):
             "--skip-overlap-check",
         ]
         self.replicas[1].run_command(args)
-
-        # wait until zone is signed
+        restart_named([self.replicas[0], self.replicas[1]])
         assert wait_until_record_is_signed(
             self.replicas[1].ip, example3_test_zone, self.log, timeout=200
         ), ("Zone %s is not signed (new replica)"
@@ -556,7 +552,6 @@ class TestMigrateDNSSECMaster(IntegrationTest):
             self.replicas[0].ip, example3_test_zone, self.log, timeout=200
         ), ("Zone %s is not signed (replica)"
             % example3_test_zone)
-        # wait until zone is signed
         assert wait_until_record_is_signed(
             self.master.ip, example3_test_zone, self.log, timeout=200
         ), ("Zone %s is not signed (master)"
-- 
1.8.3.1

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

Reply via email to