Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests: support subid ranges managed by FreeIPA #5755

Closed
wants to merge 1 commit into from
Closed

Tests: support subid ranges managed by FreeIPA #5755

wants to merge 1 commit into from

Conversation

aborah-sudo
Copy link
Contributor

@alexey-tikhonov
Copy link
Member

This awaits finalization of RedHatQE/shadow-utils#1 first.

@alexey-tikhonov
Copy link
Member

alexey-tikhonov commented Aug 30, 2021

Hi @aborah-sudo, is this ready for review?

@aborah-sudo
Copy link
Contributor Author

Hi @aborah-sudo, is thi ready for review?

yes

Copy link
Contributor

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some changes to be done but generally it seems good.

By the way, have you ported all the possible improvements that were done in the shadow-utils PR review?

src/tests/multihost/ipa/conftest.py Outdated Show resolved Hide resolved
src/tests/multihost/ipa/conftest.py Outdated Show resolved Hide resolved
src/tests/multihost/ipa/test_subid_ranges.py Outdated Show resolved Hide resolved
src/tests/multihost/ipa/test_subid_ranges.py Outdated Show resolved Hide resolved
src/tests/multihost/ipa/test_subid_ranges.py Outdated Show resolved Hide resolved
src/tests/multihost/ipa/test_subid_ranges.py Outdated Show resolved Hide resolved
src/tests/multihost/ipa/test_subid_ranges.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for your work.

@alexey-tikhonov
Copy link
Member

@aborah-sudo, is this test executed in PR CI?
At the moment I don't understand how both methods can succeed, because it seems depending on execution order either 'nsswitch.conf::subid' isn't set or IPA subid range isn't generated.

@aborah-sudo
Copy link
Contributor Author

@aborah-sudo, is this test executed in PR CI?
At the moment I don't understand how both methods can succeed, because it seems depending on execution order either 'nsswitch.conf::subid' isn't set or IPA subid range isn't generated.

the fixture "create_bkp_for_subid_files" will modify the /etc/nsswitch.conf file before every test function

@alexey-tikhonov
Copy link
Member

Ok, thanks. Looks much better. ACK.

Please run latest version in idm-ci.

I will leave this to @sgoveas for a final ACK.

@aborah-sudo
Copy link
Contributor Author

@alexey-tikhonov
Copy link
Member

./src/tests/multihost/ipa/test_subid_ranges.py:126:1: W391 blank line at end of file
./src/tests/multihost/ipa/conftest.py:180:80: E501 line too long (82 > 79 characters)
  1. Why tier1 was changed to tier2?

  2. why exit_status check was changed to:

        for i in exit_status.readlines():
            assert "write to uid_map failed" not in i

?

@aborah-sudo
Copy link
Contributor Author

./src/tests/multihost/ipa/test_subid_ranges.py:126:1: W391 blank line at end of file      ---- This is fixed now 
./src/tests/multihost/ipa/conftest.py:180:80: E501 line too long (82 > 79 characters)
1. Why `tier1` was changed to `tier2`?   ----   Its steeves idea to keep IPA tests in tire2 

2. why `exit_status` check was changed to:    ----  IDM ci does not work as we expect .  

exit_status was catching some weird errors which is not related to this test at all:

Check test run with empty list: https://ci-jenkins-csb-idmops.apps.ocp-c1.prod.psi.redhat.com/job/trigger-test-suite-tool/556/console ---- ['Could not chdir to home directory /home/admin: No such file or directory\n'] which not relevant and we need to check newuidmap relate errors which is not

The error we are looking for is : "write to uid_map failed" that is the exact error now configured with the test script . And after puting the exact error which is not occurring at the run time . The test run in IDM CI passed

With exact error code: https://ci-jenkins-csb-idmops.apps.ocp-c1.prod.psi.redhat.com/job/trigger-test-suite-tool/558/console

        for i in exit_status.readlines():
            assert "write to uid_map failed" not in i

?

@alexey-tikhonov
Copy link
Member

Frankly, I don't understand what exit_status captures, but (besides one mistype - see recent comment) in general test looks good to me.
@sgoveas , please take care of this further (final review).

@aborah-sudo
Copy link
Contributor Author

@sgoveas IDMCI passed with this PR without external repo . Every thing is working as expected :

https://ci-jenkins-csb-idmops.apps.ocp-c1.prod.psi.redhat.com/job/trigger-test-suite-tool/724/console

Copy link
Contributor

@sgoveas sgoveas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@pbrezina
Copy link
Member

Pushed PR: #5755

  • master
    • 8e22258 - Tests: support subid ranges managed by FreeIPA

@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Oct 13, 2021
@pbrezina pbrezina closed this Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants