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

Fix Upgrade scenarios #16477

Merged
merged 1 commit into from
Oct 22, 2024
Merged

Fix Upgrade scenarios #16477

merged 1 commit into from
Oct 22, 2024

Conversation

shweta83
Copy link
Contributor

@shweta83 shweta83 commented Sep 24, 2024 •
edited
Loading

Problem Statement

Provisioning Template and Foreman Discovery Upgrade Scenario tests are failing with JSON parse error.

Solution

  • Updated test_pre_scenario_provisioning_templates to save hostname instead of host id in pre_upgrade_data file.
  • Saving FDI version in string format in test_pre_upgrade_fdi_version as Version type data is not supported to be stored.

Related Issues

@shweta83 shweta83 requested a review from a team as a code owner September 24, 2024 06:28
@shweta83 shweta83 marked this pull request as draft September 24, 2024 07:43
@shweta83 shweta83 force-pushed the upgrade_scenarios_test_fix branch 2 times, most recently from 12edf99 to 60ac9ff Compare September 25, 2024 08:42
@shweta83 shweta83 marked this pull request as ready for review September 25, 2024 08:42
@shweta83 shweta83 added CherryPick PR needs CherryPick to previous branches AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 6.16.z Introduced in or relating directly to Satellite 6.16 TestFailure Issues and PRs related to a test failing in automation labels Sep 25, 2024
@shweta83
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/upgrades/test_discovery.py -k test_pre_upgrade_fdi_version -m pre_upgrade

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8788
Build Status: SUCCESS
PRT Comment: pytest tests/upgrades/test_discovery.py -k test_pre_upgrade_fdi_version -m pre_upgrade --external-logging
Test Result : =========== 1 passed, 1 deselected, 10 warnings in 898.68s (0:14:58) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Sep 25, 2024
Gauravtalreja1
Gauravtalreja1 reviewed Sep 25, 2024
View reviewed changes
tests/upgrades/test_discovery.py Outdated
@@ -60,5 +60,5 @@ def test_post_upgrade_fdi_version(self, target_sat, pre_upgrade_data):
fdi_package = target_sat.execute('rpm -qa *foreman-discovery-image*').stdout
# Note: The regular exp takes care of format digit.digit.digit or digit.digit.digit-digit in the output
post_upgrade_version = Version(re.search(r'\d+\.\d+\.\d+(-\d+)?', fdi_package).group())
assert post_upgrade_version >= pre_upgrade_version
assert str(post_upgrade_version) >= pre_upgrade_version
Copy link
Collaborator

Choose a reason for hiding this comment

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

comparing versions here make more sense here, so just would like to know why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we dictionary doesn't accept Version so changing it to str.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@shweta83 I didn't get, but as we discussed in our call we agreed to keep Version as is instead of comparing strings as its more effective

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dictionary saved in pre_upgrade scenario doesn't accept Version object so I tried a new workaround for it.

Gauravtalreja1
Gauravtalreja1 reviewed Sep 25, 2024
View reviewed changes
tests/upgrades/test_provisioningtemplate.py
pre_update_data_dict = {
'provision_host_id': host.id,
'provision_host_name': host.name,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does host.id change after upgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Host.name would be more reliable here. Id doesn't change

Copy link
Collaborator

Choose a reason for hiding this comment

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

@shweta83 I think it doesn't matter if use either name or id, both should be same, but I'd like to understand rational behind changing it from id to name

@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Oct 17, 2024
@shweta83
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/upgrades/test_discovery.py -k test_pre_upgrade_fdi_version -m pre_upgrade

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8987
Build Status: SUCCESS
PRT Comment: pytest tests/upgrades/test_discovery.py -k test_pre_upgrade_fdi_version -m pre_upgrade --external-logging
Test Result : ========== 1 passed, 1 deselected, 10 warnings in 1808.27s (0:30:08) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Oct 17, 2024
@Gauravtalreja1 Gauravtalreja1 merged commit 1532b8e into SatelliteQE:master Oct 22, 2024
11 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 22, 2024
@shweta83 @web-flow
Fix Upgrade scenarios (#16477)
76e3fc4 
(cherry picked from commit 1532b8e)
github-actions bot pushed a commit that referenced this pull request Oct 22, 2024
@shweta83 @web-flow
Fix Upgrade scenarios (#16477)
0ef0dc5 
(cherry picked from commit 1532b8e)
github-actions bot pushed a commit that referenced this pull request Oct 22, 2024
@shweta83 @web-flow
Fix Upgrade scenarios (#16477)
605a29d 
(cherry picked from commit 1532b8e)
This was referenced Oct 22, 2024
Merged
Merged
Merged
github-actions bot pushed a commit that referenced this pull request Oct 22, 2024
@shweta83 @web-flow
Fix Upgrade scenarios (#16477)
978da86 
(cherry picked from commit 1532b8e)
@Satellite-QE Satellite-QE mentioned this pull request Oct 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ColeHiggins2 ColeHiggins2 ColeHiggins2 approved these changes

@Gauravtalreja1 Gauravtalreja1 Gauravtalreja1 approved these changes

@pondrejk pondrejk pondrejk approved these changes

@pnovotny pnovotny pnovotny approved these changes

Assignees

@shweta83 shweta83

Labels
6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 6.16.z Introduced in or relating directly to Satellite 6.16 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 TestFailure Issues and PRs related to a test failing in automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@shweta83 @Satellite-QE @pnovotny @pondrejk @Gauravtalreja1 @ColeHiggins2