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

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

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

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.

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

@mhonek did you mean this comment: https://pagure.io/389-ds-base/pull-request/50287#comment-87722 ?

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

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

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

@mhonek did you mean this comment: https://pagure.io/389-ds-base/pull-request/50287#comment-87722 ?

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

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

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

Changes made, please review...

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)

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

Please update the commit message to include 'Fixes' or 'Relates' keyword before the URL (see https://pagure.io/389-ds-base/pull-request/50444#comment-89063).

The rest looks good, thanks!

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3520

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.