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

Fix failing registration test #17346

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix failing registration test #17346

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions tests/foreman/cli/test_registration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,13 @@ def test_positive_force_register_twice(module_ak_with_cv, module_org, rhel_conte
assert result.status == 0
assert rhel_contenthost.subscribed
assert f'Unregistering from: {target_sat.hostname}' in str(result.stdout)
assert f'The registered system name is: {rhel_contenthost.hostname}' in str(result.stdout)
hostname = rhel_contenthost.execute('hostname').stdout.strip()
Copy link
Contributor

@shweta83 shweta83 Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like the unregistering of the host is not done properly and RHEL10 is only impacted here. I am guessing it an issue of RHEL10 rather than automation.

Copy link
Contributor Author

@shubhamsg199 shubhamsg199 Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, that was my initial guess but looking at the logs, it says unregistered.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@shubhamsg199 @shweta83 Looking at the test failure in CI, when hostnamectl set-hostname {name} it never worked on < RHEL 10 since we use containers for this test and especially for RHEL10 we don't have any container images created yet it and it default checkout a VM where set-hostname works and this started to fail.

2025-01-16 04:41:22 - gw0 - broker - DEBUG - a33bb7c2198e executing command: hostnamectl set-hostname ZugqfcdVhq.example.com
2025-01-16 04:41:22 - gw0 - broker - DEBUG - a33bb7c2198e command result:
    stdout:
    
    stderr:
    System has not been booted with systemd as init system (PID 1). Can't operate.
    Failed to connect to bus: Host is down
    
    status: 1

IMO this is expected behaviour, so if we're considering to fix it this way then we should consider using no_containers for this test, but before that I'd like to understand the rational behind changing the hostname step if is that really required for this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the explanation @Gauravtalreja1 , I think we should update the test and use no_containers
The reason behind changing the hostname step is because of the BZ attached which is the test requirement

Copy link
Contributor

Choose a reason for hiding this comment

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

The test scenario of the bug was testing case-sensitiveness of the hostname. But as @Gauravtalreja1 said, the scenario was never tested due to containers.
The problem here is we are asserting rhel_contenthost.hostname with hostname which will always fail. We need to fix it and run the test on VM.

assert f'The registered system name is: {hostname}' in str(result.stdout)
reg_id_new = re.search(reg_id_pattern, result.stdout).group(1)
assert f'The system has been registered with ID: {reg_id_new}' in str(result.stdout)
assert reg_id_new != reg_id_old
assert (
target_sat.cli.Host.info({'name': rhel_contenthost.hostname}, output_format='json')[
target_sat.cli.Host.info({'name': hostname.lower()}, output_format='json')[
Copy link
Collaborator

Choose a reason for hiding this comment

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

If hostname is in uppercase before registration, then after registration does it get changed to lowercase ?

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

then we don't need .lower()

'subscription-information'
]['uuid']
== reg_id_new
Expand Down
Loading