PR - Issue 50462 - Fix CI tests

[389-ds-bot]

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/50463

• Created at 2019-06-22 00:26:38 by mreynolds (@mreynolds389)
• Merged at 2019-06-25 21:21:46

---

Description: Port some of the failing ticket tests to suites

Resolves: #2354

Reviewed by: ?

[389-ds-bot]

Comment from mhonek (@kenoh) at 2019-06-24 13:10:42

Please, explain in the commit why changing these two lines (with a reference, to 10bffac I guess).

[389-ds-bot]

Comment from mhonek (@kenoh) at 2019-06-24 13:14:12

Please, mention in the commit message why removing this module (AFAIR there has been a discussion in some ticket or PR, but I don't remember which).

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-24 14:24:33

Please, explain in the commit why changing these two lines (with a reference, to 10bffac I guess).

The systemdd default increased, so we had to match it

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-24 14:27:09

Please, mention in the commit message why removing this module (AFAIR there has been a discussion in some ticket or PR, but I don't remember which).

Well originally I was going through the failing tests, and I found this ticket test, and ported it to suites. After porting it and removing the ticket test I found the suite test as well. But my approach added the test to an existing test. So I just kept what I had and got rid of the other suite test as it was more concise.

[389-ds-bot]

Comment from aadhikari at 2019-06-24 15:03:30

Please, mention in the commit message why removing this module (AFAIR there has been a discussion in some ticket or PR, but I don't remember which).

@kenoh did you mean this comment: #3346#comment-87722 ?

@mreynolds389 I think I am already removing ticket47838_test.py in the above PR.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-24 15:10:22

@mreynolds389 I think I am already removing ticket47838_test.py in the above PR.

That's fine, it should not mess up either of our PRs. They should both merge fine regardless who merges first.

[389-ds-bot]

Comment from mhonek (@kenoh) at 2019-06-24 15:12:42

Please, explain in the commit why changing these two lines (with a reference, to 10bffac I guess).

The systemdd default increased, so we had to match it

AFAICT, the value increased because in the forementioned commit we removed the explicit LimitNOFILES=16k we had there before, hence the SystemD's default came to life (but this is somehow a subject to change, since Viktor managed to get a different "default" value of 1M instead of 512k, AFAIR). It would be good to mention this in the commit message so that it is better back-trackable.

Please, mention in the commit message why removing this module (AFAIR there has been a discussion in some ticket or PR, but I don't remember which).

@kenoh did you mean this comment: #3346#comment-87722 ?

Correct, this one and a couple of the following. Thanks!

[389-ds-bot]

Comment from spichugi (@droideck) at 2019-06-24 15:28:29

test_fd_limits fails for me with...

[vagrant@localhost 389-ds-base]$ rpm -q 389-ds-base
389-ds-base-1.4.1.4-20190624gitb5265bb00.fc29.x86_64

        # Check systemd default
        max_fd = topology_st.standalone.config.get_attr_val_utf8(FD_ATTR)
>       assert max_fd == SYSTEMD_VAL
E       AssertionError: assert '4096' == '524288'
E         - 4096
E         + 524288

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-24 15:34:53

rebased onto bb2ef1ed36d9937987dadede6cb4a6b6cb8cd335

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-24 15:36:21

test_fd_limits fails for me with...
[vagrant@localhost 389-ds-base]$ rpm -q 389-ds-base
389-ds-base-1.4.1.4-20190624gitb5265bb00.fc29.x86_64

    # Check systemd default
    max_fd = topology_st.standalone.config.get_attr_val_utf8(FD_ATTR)

  assert max_fd == SYSTEMD_VAL

E AssertionError: assert '4096' == '524288'
E - 4096
E + 524288

What platform as you testing on? These changes are only going to make F30/RHEL 8.1

Anyway I just removed the systemd default limit test....

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-24 17:12:47

Changes made, please review...

[389-ds-bot]

Comment from vashirov (@vashirov) at 2019-06-24 17:17:11

For fd limits we can query the OS and systemd to get the limits. Something like this:
(sorry, I was not quick to enough to open a PR :) )

diff --git a/dirsrvtests/tests/suites/resource_limits/fdlimits_test.py b/dirsrvtests/tests/suites/resource_limits/fdlimits_test.py
index c8e45fed6..20bdb4b2d 100644
--- a/dirsrvtests/tests/suites/resource_limits/fdlimits_test.py
+++ b/dirsrvtests/tests/suites/resource_limits/fdlimits_test.py
@@ -2,9 +2,11 @@ import logging
 import pytest
 import os
 import ldap
+import resource
 from lib389._constants import *
 from lib389.topologies import topology_st
-from lib389.utils import ds_is_older
+from lib389.utils import ds_is_older, ensure_str
+from subprocess import check_output
 
 pytestmark = pytest.mark.tier1
 
@@ -12,9 +14,11 @@ logging.getLogger(__name__).setLevel(logging.INFO)
 log = logging.getLogger(__name__)
 
 FD_ATTR = "nsslapd-maxdescriptors"
-SYSTEMD_VAL = "16384"
+GLOBAL_LIMIT = resource.getrlimit(resource.RLIMIT_NOFILE)[1]
+SYSTEMD_LIMIT = ensure_str(check_output("systemctl show --value -p LimitNOFILE dirsrv@standalone1".split(" ")).strip())
 CUSTOM_VAL = "9000"
-TOO_HIGH_VAL = "65536"
+TOO_HIGH_VAL = str(GLOBAL_LIMIT * 2)
+TOO_HIGH_VAL2 = str(int(SYSTEMD_LIMIT) * 2)
 TOO_LOW_VAL = "0"
 
 @pytest.mark.skipif(ds_is_older("1.4.1.2"), reason="Not implemented")
@@ -37,19 +41,25 @@ def test_fd_limits(topology_st):
 
     # Check systemd default
     max_fd = topology_st.standalone.config.get_attr_val_utf8(FD_ATTR)
-    assert max_fd == SYSTEMD_VAL
+    assert max_fd == SYSTEMD_LIMIT
 
     # Check custom value is applied
     topology_st.standalone.config.set(FD_ATTR, CUSTOM_VAL)
     max_fd = topology_st.standalone.config.get_attr_val_utf8(FD_ATTR)
     assert max_fd == CUSTOM_VAL
 
-    # Attempt to use val that is too high
+    # Attempt to use value that is higher than the global system limit
     with pytest.raises(ldap.UNWILLING_TO_PERFORM):
         topology_st.standalone.config.set(FD_ATTR, TOO_HIGH_VAL)
     max_fd = topology_st.standalone.config.get_attr_val_utf8(FD_ATTR)
     assert max_fd == CUSTOM_VAL
 
+    # Attempt to use value that is higher than the value defined in the systemd service
+    with pytest.raises(ldap.UNWILLING_TO_PERFORM):
+        topology_st.standalone.config.set(FD_ATTR, TOO_HIGH_VAL2)
+    max_fd = topology_st.standalone.config.get_attr_val_utf8(FD_ATTR)
+    assert max_fd == CUSTOM_VAL
+
     # Attempt to use val that is too low
     with pytest.raises(ldap.OPERATIONS_ERROR):
         topology_st.standalone.config.set(FD_ATTR, TOO_LOW_VAL)

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-24 17:46:57

rebased onto 96aa9d36fe5028de8927ab972c385c0083935fb4

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-24 17:47:48

Thanks Viktor I added your changes. Please review...

[389-ds-bot]

Comment from vashirov (@vashirov) at 2019-06-25 20:11:02

Please update the commit message to include 'Fixes' or 'Relates' keyword before the URL (see #3502#comment-89063).

The rest looks good, thanks!

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-25 21:21:24

rebased onto 19d2029

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2019-06-25 21:21:47

Pull-Request has been merged by mreynolds389

[389-ds-bot]

Patch
50463.patch