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

[RHELC-1494, RHELC-1289] Restore disabled repos during conversion in rollback #1212

Merged
merged 6 commits into from
May 23, 2024
Merged

[RHELC-1494, RHELC-1289] Restore disabled repos during conversion in rollback #1212

merged 6 commits into from
May 23, 2024

Conversation

r0x0d
Copy link
Member

@r0x0d r0x0d commented May 8, 2024 •
edited by danmyway

In a satellite analysis/conversion, we are disabling all repositories to not interfere with the rest of the execution.

With this commit, we are introducing a way of checking what repositories are enabled prior to disable them, and back the repositories names up. In the rollback phase, we enable the repositories back on the system to leave everything the way it was.

Jira Issues:

Checklist

  • PR has been tested manually in a VM (either author or reviewer)
  • Jira issue has been made public if possible
  • [RHELC-] is part of the PR title
  • GitHub label has been added to help with Release notes
  • PR title explains the change from the user's point of view
  • Code and tests are documented properly
  • The commits are squashed to as few commits as possible (without losing data)
  • When merged: Jira issue has been updated to Release Pending if relevant

Depends on

@r0x0d r0x0d added enhancement New feature or request tests-run-tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`. labels May 8, 2024
@has-bot
Copy link
Member

has-bot commented May 8, 2024

/packit test --labels tier0

Comment generated by an automation.

@codecov Codecov
Copy link

codecov bot commented May 8, 2024 •
edited

Codecov Report

Attention: Patch coverage is 98.73418% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 95.74%. Comparing base (bd2154a) to head (a6297b4).

Files Patch % Lines
convert2rhel/backup/subscription.py 98.46% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1212      +/-   ##
==========================================
+ Coverage   95.55%   95.74%   +0.19%     
==========================================
  Files          54       55       +1     
  Lines        4720     4752      +32     
  Branches      829      834       +5     
==========================================
+ Hits         4510     4550      +40     
+ Misses        127      116      -11     
- Partials       83       86       +3
Flag Coverage Δ
centos-linux-7 90.93% <98.73%> (+0.23%) ⬆️
centos-linux-8 91.86% <98.73%> (+0.22%) ⬆️
centos-linux-9 91.91% <98.73%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@r0x0d
Copy link
Member Author

r0x0d commented May 9, 2024

@danmyway / @kokesak I'm trying to add the integration tests for this PR, and right now I did an experiment: d24850b

Let me know if this is super incorrect and needs improvements. I don't mind reworking it. Otherwise, feel free to push to the branch if you guys prefer.

@r0x0d
Copy link
Member Author

r0x0d commented May 9, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the enable-repositories-during-rollback branch from d24850b to 650007a Compare May 15, 2024 13:18
@r0x0d r0x0d force-pushed the enable-repositories-during-rollback branch from 650007a to 08b37f6 Compare May 15, 2024 18:22
@r0x0d
Copy link
Member Author

r0x0d commented May 15, 2024

/packit test --labels tier0

@r0x0d
Copy link
Member Author

r0x0d commented May 16, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the enable-repositories-during-rollback branch from 82528d4 to 4ef8e2a Compare May 20, 2024 19:05
@r0x0d
Copy link
Member Author

r0x0d commented May 20, 2024

/packit test --labels tier0

pr-watson
pr-watson approved these changes May 20, 2024
View reviewed changes
Copy link
Contributor

@pr-watson pr-watson 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

@bookwar
Copy link
Contributor

bookwar commented May 21, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the enable-repositories-during-rollback branch from 4ef8e2a to ea56a14 Compare May 21, 2024 14:49
@r0x0d
Copy link
Member Author

r0x0d commented May 21, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the enable-repositories-during-rollback branch from ea56a14 to 4070a52 Compare May 21, 2024 17:08
@r0x0d
Copy link
Member Author

r0x0d commented May 21, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the enable-repositories-during-rollback branch from 4070a52 to 59d28af Compare May 21, 2024 18:32
@r0x0d
Copy link
Member Author

r0x0d commented May 21, 2024

/packit test --labels tier0

@pr-watson
Copy link
Contributor

@r0x0d can you rebase this please

@r0x0d r0x0d force-pushed the enable-repositories-during-rollback branch from 59d28af to 0c80e0e Compare May 21, 2024 19:03
@r0x0d
Copy link
Member Author

r0x0d commented May 21, 2024

/packit test --labels tier0

@r0x0d
Copy link
Member Author

r0x0d commented May 21, 2024

/packit build

@r0x0d
Copy link
Member Author

r0x0d commented May 21, 2024

/packit test --labels tier0

@pr-watson
Copy link
Contributor

Manually retriggered failed tests

hosekadam
hosekadam reviewed May 22, 2024
View reviewed changes
convert2rhel/backup/subscription.py
Comment on lines +51 to +54
loggerinst.task("Rollback: RHSM-related actions")

if self.enabled:
try:
Copy link
Member

@hosekadam hosekadam May 22, 2024 •
edited

Choose a reason for hiding this comment

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

Suggested change
loggerinst.task("Rollback: RHSM-related actions")
if self.enabled:
try:
if self.enabled:
loggerinst.task("Rollback: RHSM-related actions")
try:

or

def restore(self):
        """Rollback subscription related changes"""
        if not self.enabled:
               return

To avoid printing the rollback message when it's not enabled

@r0x0d r0x0d force-pushed the enable-repositories-during-rollback branch from bd47ea1 to 195a9dd Compare May 22, 2024 14:51
@r0x0d r0x0d requested a review from hosekadam May 22, 2024 14:53
@r0x0d
Copy link
Member Author

r0x0d commented May 22, 2024

/packit test --labels tier0

@r0x0d r0x0d force-pushed the enable-repositories-during-rollback branch from 7968237 to 9e4fada Compare May 22, 2024 16:47
@r0x0d
Copy link
Member Author

r0x0d commented May 22, 2024

/packit test --labels tier0

hosekadam
hosekadam approved these changes May 22, 2024
View reviewed changes
Copy link
Member

@hosekadam hosekadam 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!

r0x0d and others added 5 commits May 23, 2024 11:13
@r0x0d @kokesak
Restore disabled repos during conversion in rollback
85850d9 
In a satellite analysis/conversion, we are disabling all repositories to
not interfer with the rest of the execution.

With this commit, we are introducing a way of checking what repositories
are enabled prior to disable them, and back the repositories names up.
In the rollback phase, we enable the repositories back on the system to
leave everything the way it was.
@r0x0d @kokesak
Improve unit tests to increase coverage
8fc125b
@r0x0d @kokesak
Add integration tests
87aed8f 
This test will perform the following operations:
    - Collect the enabled repositories prior to the analysis start
    - Run the analysis and assert that we successfully enabled the RHSM repositories
    - Collect the enabled repositories after the tool run to compare with the repositories prior to the analysis
@r0x0d @kokesak
Apply suggestions from code review and few modifications
dec3fda 
In this commit, we are including the following modications that are
worth mentioning:

- We are dropping the filter for rhel repositories, so if the user has
  any rhel repositories enabled before the analysis, we should re-enable
  them after the analysis is done (alongside any other repository that
  rhsm reports that is enabled)
- Updates to the integration tests to strip the white spaces and compare
  correctly the list of repositories enabled
@r0x0d @kokesak
Apply suggestions from code review
9cddbcb 
Co-authored-by: Martin "kokesak" Litwora <mlitwora@redhat.com>
@kokesak kokesak force-pushed the enable-repositories-during-rollback branch from 304e0f3 to 4db54d2 Compare May 23, 2024 09:13
@kokesak kokesak requested a review from danmyway May 23, 2024 09:18
@kokesak
Copy link
Member

kokesak commented May 23, 2024

/packit test --labels tier0

@kokesak kokesak force-pushed the enable-repositories-during-rollback branch from 4db54d2 to b29c444 Compare May 23, 2024 09:25
@kokesak
Copy link
Member

kokesak commented May 23, 2024

/packit test --labels tier0

@danmyway danmyway changed the title [RHELC-1494] Restore disabled repos during conversion in rollback [RHELC-1494, RHELC-1289] Restore disabled repos during conversion in rollback May 23, 2024
@kokesak kokesak force-pushed the enable-repositories-during-rollback branch from b29c444 to a6297b4 Compare May 23, 2024 11:04
@kokesak
Copy link
Member

kokesak commented May 23, 2024

/packit test --labels tier0

@r0x0d r0x0d merged commit 3f6ce10 into oamg:main May 23, 2024
33 checks passed
@r0x0d r0x0d deleted the enable-repositories-during-rollback branch May 23, 2024 13:57
danmyway
danmyway approved these changes May 23, 2024
View reviewed changes
Copy link
Member

@danmyway danmyway 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

r0x0d added a commit to r0x0d/convert2rhel that referenced this pull request May 23, 2024
@r0x0d
Apply missing coments from PR#1212
5aaa2ee 
The following comments are applied in this patch:

- oamg#1212 (comment)
- oamg#1212 (comment)
- oamg#1212 (comment)
@r0x0d r0x0d mentioned this pull request May 23, 2024
Open
8 tasks
@hosekadam hosekadam mentioned this pull request May 27, 2024
Merged
r0x0d added a commit to r0x0d/convert2rhel that referenced this pull request May 31, 2024
@r0x0d
Apply missing coments from PR#1212
6fb29a1 
The following comments are applied in this patch:

- oamg#1212 (comment)
- oamg#1212 (comment)
- oamg#1212 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kokesak kokesak kokesak left review comments

@danmyway danmyway danmyway approved these changes

@hosekadam hosekadam hosekadam approved these changes

@pr-watson pr-watson pr-watson approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request tests-run-tier0 PR ready to run the essential test suit. Equivalent to `/packit test --labels tier0`.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@r0x0d @has-bot @bookwar @pr-watson @kokesak @danmyway @hosekadam