URL: https://github.com/SSSD/sssd/pull/963
Author: mzidek-rh
 Title: #963: Backport recent CI changes to sssd-1-16
Action: opened

PR body:
"""

"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/963/head:pr963
git checkout pr963
From c2345a65ac542300385b414e2d3fcc78cca686df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Wed, 4 Dec 2019 13:42:36 +0100
Subject: [PATCH 1/5] ci: add rhel7
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reviewed-by: Michal Židek <[email protected]>
---
 Jenkinsfile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Jenkinsfile b/Jenkinsfile
index fc43db1ae2..cf4189ceab 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -10,6 +10,7 @@ def systems = [
   'fedora30',
   'fedora31',
   'fedora-rawhide',
+  'rhel7',
   'debian10',
 ]
 

From 42ee5b827297753b2976b653c16536a88acc936e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Wed, 4 Dec 2019 13:44:37 +0100
Subject: [PATCH 2/5] ci: set sssd-ci notification to pending state when job is
 started
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reviewed-by: Michal Židek <[email protected]>
---
 Jenkinsfile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Jenkinsfile b/Jenkinsfile
index cf4189ceab..eec944bbdc 100644
--- a/Jenkinsfile
+++ b/Jenkinsfile
@@ -299,6 +299,8 @@ try {
   }
 
   stage('Prepare systems') {
+    notification.notify('PENDING', 'Pending.')
+
     /* Notify that all systems are pending. */
     for (system in systems) {
       notification.notify('PENDING', 'Awaiting executor', system)

From 69e21c88396c4d710a4227a12e4b0ecab9eed0ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Thu, 5 Dec 2019 14:36:26 +0100
Subject: [PATCH 3/5] ci: archive ci-mock-result
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reviewed-by: Michal Židek <[email protected]>
---
 contrib/test-suite/test-suite.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/test-suite/test-suite.yml b/contrib/test-suite/test-suite.yml
index 2b091349f6..49763fa4b6 100644
--- a/contrib/test-suite/test-suite.yml
+++ b/contrib/test-suite/test-suite.yml
@@ -8,4 +8,5 @@
   - ci-*.log
   - ci-build-debug/ci-*.log
   - ci-build-debug/test-suite.log
+  - ci-build-debug/ci-mock-result/*.log
   timeout: 6 hours

From 0b318323789140ed161cededcca20a617b2d5ff6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <[email protected]>
Date: Wed, 9 Aug 2017 07:59:41 +0200
Subject: [PATCH 4/5] INTG: Increase the sleep() time so the changes are
 reflected on SSSD
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Those tests have been failing a lot recently and it does happen becase
the time to reflect the changes on SSSD is not enough for the machine
where the tests are running.

There's no reasonable explanation in the code why 4 seconds is used as
INTERACTIVE_TIMEOUT, neither a reasonable explanation why 2 seconds is
used as the time waited in order to have those changes reflected on
SSSD (neither in the code nor in the commit messages).

This patch uses the most simple empiric way to determine a better value
for this timeout, which was "run the tests a considerable amount of time
and check that there were no failures".

So, in order to avoid failures and our tests giving us more reliable
information, let's give more time so the changes are reflected on SSSD.

Resolves:
https://pagure.io/SSSD/sssd/issue/3463

Signed-off-by: Fabiano Fidêncio <[email protected]>
Reviewed-by: Michal Židek <[email protected]>
Reviewed-by: Alexey Tikhonov <[email protected]>
---
 src/tests/intg/test_enumeration.py | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/tests/intg/test_enumeration.py b/src/tests/intg/test_enumeration.py
index c105c6df02..c25588a566 100644
--- a/src/tests/intg/test_enumeration.py
+++ b/src/tests/intg/test_enumeration.py
@@ -33,6 +33,15 @@
 from util import *
 
 LDAP_BASE_DN = "dc=example,dc=com"
+
+# There is no explation neither in the code nor in the commit message that
+# introduced this timeout why 4 was chosen as value. The very same happens
+# with respect to why the time we should wait in order to have the changes
+# reflected on SSSD is INTERACTIVE_TIMEOUT/2.
+# Having INTERACTIVE_TIMEOUT/2 has been causing a lot of failures in some of
+# our CI tests, so it's been changed to INTERACTIVE_TIMEOUT and the way it's
+# been tested was just empirically by running or CI several times and checking
+# whether a failure happened or not.
 INTERACTIVE_TIMEOUT = 4
 
 
@@ -412,7 +421,7 @@ def user_and_groups_rfc2307_bis(request, ldap_conn):
 def test_add_remove_user(ldap_conn, blank_rfc2307):
     """Test user addition and removal are reflected by SSSD"""
     e = ldap_ent.user(ldap_conn.ds_inst.base_dn, "user", 2001, 2000)
-    time.sleep(INTERACTIVE_TIMEOUT/2)
+    time.sleep(INTERACTIVE_TIMEOUT)
     # Add the user
     ent.assert_passwd(ent.contains_only())
     ldap_conn.add_s(*e)
@@ -427,7 +436,7 @@ def test_add_remove_user(ldap_conn, blank_rfc2307):
 def test_add_remove_group_rfc2307(ldap_conn, blank_rfc2307):
     """Test RFC2307 group addition and removal are reflected by SSSD"""
     e = ldap_ent.group(ldap_conn.ds_inst.base_dn, "group", 2001)
-    time.sleep(INTERACTIVE_TIMEOUT/2)
+    time.sleep(INTERACTIVE_TIMEOUT)
     # Add the group
     ent.assert_group(ent.contains_only())
     ldap_conn.add_s(*e)
@@ -442,7 +451,7 @@ def test_add_remove_group_rfc2307(ldap_conn, blank_rfc2307):
 def test_add_remove_group_rfc2307_bis(ldap_conn, blank_rfc2307_bis):
     """Test RFC2307bis group addition and removal are reflected by SSSD"""
     e = ldap_ent.group_bis(ldap_conn.ds_inst.base_dn, "group", 2001)
-    time.sleep(INTERACTIVE_TIMEOUT/2)
+    time.sleep(INTERACTIVE_TIMEOUT)
     # Add the group
     ent.assert_group(ent.contains_only())
     ldap_conn.add_s(*e)
@@ -456,7 +465,7 @@ def test_add_remove_group_rfc2307_bis(ldap_conn, blank_rfc2307_bis):
 
 def test_add_remove_membership_rfc2307(ldap_conn, user_and_group_rfc2307):
     """Test user membership addition and removal are reflected by SSSD"""
-    time.sleep(INTERACTIVE_TIMEOUT/2)
+    time.sleep(INTERACTIVE_TIMEOUT)
     # Add user to group
     ent.assert_group_by_name("group", dict(mem=ent.contains_only()))
     ldap_conn.modify_s("cn=group,ou=Groups," + ldap_conn.ds_inst.base_dn,
@@ -478,7 +487,7 @@ def test_add_remove_membership_rfc2307_bis(ldap_conn,
     """
     base_dn_bytes = ldap_conn.ds_inst.base_dn.encode('utf-8')
 
-    time.sleep(INTERACTIVE_TIMEOUT/2)
+    time.sleep(INTERACTIVE_TIMEOUT)
     # Add user to group1
     ent.assert_group_by_name("group1", dict(mem=ent.contains_only()))
     ldap_conn.modify_s("cn=group1,ou=Groups," + ldap_conn.ds_inst.base_dn,

From 6ca85fd0cc817bd3ae77f08f429ec019fceecc42 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <[email protected]>
Date: Tue, 26 Nov 2019 12:42:27 +0100
Subject: [PATCH 5/5] tests: fix race condition in enumeration tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This change is relevant to Nyquist frequency. To ensure that enumeration has been
run we need to wait at least twice the enumeration timeout. In other words, we need
to make sure enumeration is run at least twice the frequency of our assertions to
ensure that it has been run at least once.

Patch was amended by Alexey Tikhonov <[email protected]> to include nice
comment originally provided by Pavel Březina at
https://github.com/SSSD/sssd/pull/947#issuecomment-559440211

Relates: https://pagure.io/SSSD/sssd/issue/3463

Reviewed-by: Michal Židek <[email protected]>
Reviewed-by: Alexey Tikhonov <[email protected]>
---
 src/tests/intg/test_enumeration.py | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/src/tests/intg/test_enumeration.py b/src/tests/intg/test_enumeration.py
index c25588a566..31c3d75c0d 100644
--- a/src/tests/intg/test_enumeration.py
+++ b/src/tests/intg/test_enumeration.py
@@ -34,15 +34,18 @@
 
 LDAP_BASE_DN = "dc=example,dc=com"
 
-# There is no explation neither in the code nor in the commit message that
-# introduced this timeout why 4 was chosen as value. The very same happens
-# with respect to why the time we should wait in order to have the changes
-# reflected on SSSD is INTERACTIVE_TIMEOUT/2.
-# Having INTERACTIVE_TIMEOUT/2 has been causing a lot of failures in some of
-# our CI tests, so it's been changed to INTERACTIVE_TIMEOUT and the way it's
-# been tested was just empirically by running or CI several times and checking
-# whether a failure happened or not.
-INTERACTIVE_TIMEOUT = 4
+# Enumeration is run periodically. It is scheduled during SSSD startup
+# and then it runs every four seconds. We do not know precisely when
+# the enumeration refresh is triggered so sleeping for four seconds is
+# not enough, we have to sleep longer time to adapt for delays (it is
+# not scheduled on precise time because of other sssd operation, it
+# takes some time to finish so each run moves it further away from
+# exact 4 seconds period, cpu scheduler, context switches, ...).
+# I agree that just little bit longer timeout may work as well.
+# However we can be sure when using twice the period because we know
+# that enumeration was indeed run at least once.
+ENUMERATION_TIMEOUT = 4
+INTERACTIVE_TIMEOUT = ENUMERATION_TIMEOUT*2
 
 
 @pytest.fixture(scope="module")
@@ -160,7 +163,7 @@ def format_interactive_conf(ldap_conn, schema):
             ldap_enumeration_refresh_timeout    = {0}
             ldap_purge_cache_timeout            = 1
             entry_cache_timeout                 = {0}
-        """).format(INTERACTIVE_TIMEOUT)
+        """).format(ENUMERATION_TIMEOUT)
 
 
 def create_conf_file(contents):
_______________________________________________
sssd-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/[email protected]

Reply via email to