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.

Sign up for GitHub

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

Jump to bottom

verify host owner name after host registration #15178

Merged
merged 2 commits into from
Jun 13, 2024
Merged

verify host owner name after host registration #15178

merged 2 commits into from
Jun 13, 2024

Conversation

amolpati30
Copy link
Contributor

Problem Statement

When a host is registered, its Owner is not set to the admin that generated the registration command. It stays set to a hidden API user, Owner is displayed as Anonymous Admin.

The user who actually ran the registration process should be set as owner.

Solution

The following automation script is designed to verify that the host is registered correctly and that it is assigned the expected owner name as specified.

@amolpati30 amolpati30 requested a review from a team as a code owner May 25, 2024 08:46
@amolpati30 amolpati30 force-pushed the owner_name_identify branch from 3e7d590 to 35b204e Compare May 25, 2024 08:47
@amolpati30
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_registration.py -k test_positive_set_host_owner_correctly

@amolpati30 amolpati30 added 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 Stream Introduced in or relating directly to Satellite Stream/Master No-CherryPick PR doesnt need CherryPick to previous branches and removed 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 labels May 25, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 7116
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_registration.py -k test_positive_set_host_owner_correctly --external-logging
Test Result : =========== 15 deselected, 23 warnings, 1 error in 728.29s (0:12:08) ===========

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label May 25, 2024
@amolpati30
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_registration.py -k test_positive_set_host_owner_correctly

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 7117
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_registration.py -k test_positive_set_host_owner_correctly --external-logging
Test Result : =========== 15 deselected, 23 warnings, 1 error in 724.22s (0:12:04) ===========

@amolpati30
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_registration.py -k test_positive_set_host_owner_correctly

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 7118
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_registration.py -k test_positive_set_host_owner_correctly --external-logging
Test Result : =========== 15 deselected, 23 warnings, 1 error in 684.24s (0:11:24) ===========

Gauravtalreja1
Gauravtalreja1 reviewed May 27, 2024
View reviewed changes
tests/foreman/cli/test_registration.py Outdated

:steps:
1. Register the host.
2. Check the host is registered and verify owner name
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd suggest to check this scenario end to end, like verify when host is created/registered with non-admin user and same user is assigned as host-owner, wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what you mean by end-to-end, but I think this can be part of another test that registers a host, you just add assert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added an assertion to the existing test instead of creating a new one. Please take a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Gauravtalreja1
I will create a different PR to cover the non-admin scenario. Right now, I have added assertions for the admin user.

@lhellebr lhellebr changed the title verify host name after host registration verify host owner name after host registration May 27, 2024
@lhellebr
Copy link
Contributor

PRT failure unrelated:

failed on setup with "TypeError: Organization.__init__() got multiple values for argument 'server_config'"

@amolpati30 amolpati30 force-pushed the owner_name_identify branch from 35b204e to 7d2b46b Compare June 8, 2024 08:28
@amolpati30 amolpati30 added 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing and removed Stream Introduced in or relating directly to Satellite Stream/Master No-CherryPick PR doesnt need CherryPick to previous branches labels Jun 8, 2024
@amolpati30
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/cli/test_registration.py -k test_host_registration_end_to_end

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 7328
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_registration.py -k test_host_registration_end_to_end --external-logging
Test Result : ========== 6 passed, 9 deselected, 471 warnings in 3591.97s (0:59:51) ==========

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Jun 8, 2024
lhellebr
lhellebr approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

@lhellebr lhellebr left a comment

Choose a reason for hiding this comment

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

ACK.

Optionally, you can

  1. make the check into a function to avoid code duplication
  2. make 'Admin User' a constant

@Gauravtalreja1 Gauravtalreja1 added QETestCoverage Issues and PRs relating to a Satellite bug Easy Fix :) Easiest Fix to review and quick merge request. and removed 6.14.z Introduced in or relating directly to Satellite 6.14 labels Jun 13, 2024
@Gauravtalreja1
Copy link
Collaborator

Removing the 6.14.z label as the BZ fix is not yet cherrypicked to 6.14.z

@Gauravtalreja1 Gauravtalreja1 merged commit 423864f into SatelliteQE:master Jun 13, 2024
20 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 13, 2024
@amolpati30 @web-flow
verify host owner name after host registration (#15178)
42d880f 
* verify host name after host registration

* assertion added for host

(cherry picked from commit 423864f)
@Satellite-QE Satellite-QE mentioned this pull request Jun 13, 2024
Merged
Gauravtalreja1 pushed a commit that referenced this pull request Jun 21, 2024
@amolpati30 @Gauravtalreja1
verify host owner name after host registration (#15178)
9815ec0 
* verify host name after host registration

* assertion added for host

(cherry picked from commit 423864f)
jyejare pushed a commit to jyejare/robottelo that referenced this pull request Oct 19, 2024
@amolpati30 @jyejare
verify host owner name after host registration (SatelliteQE#15178)
34168bb 
* verify host name after host registration

* assertion added for host
@Gauravtalreja1 Gauravtalreja1 removed the Easy Fix :) Easiest Fix to review and quick merge request. label Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhellebr lhellebr lhellebr approved these changes

@Gauravtalreja1 Gauravtalreja1 Gauravtalreja1 approved these changes

Assignees

@amolpati30 amolpati30

Labels
6.15.z Introduced in or relating directly to Satellite 6.15 AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing CherryPick PR needs CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR QETestCoverage Issues and PRs relating to a Satellite bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@amolpati30 @Satellite-QE @lhellebr @Gauravtalreja1