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

Fixes #37357 - Add container migration to upgrade #829

Merged
merged 1 commit into from
May 15, 2024
Merged

Fixes #37357 - Add container migration to upgrade #829

merged 1 commit into from
May 15, 2024

Conversation

sjha4
Copy link
Contributor

@sjha4 sjha4 commented Apr 16, 2024

Adding container-handle-image-data migration to post-upgrade tasks with upgraded pulp.

evgeni
evgeni requested changes Apr 18, 2024
View reviewed changes
Copy link
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

can you address the rubocop issues please?

@sjha4 sjha4 force-pushed the add_container_label_migration branch from 1ad344c to 2eee300 Compare April 19, 2024 07:18
@sjha4 sjha4 requested a review from evgeni April 19, 2024 07:19
@evgeni
Copy link
Member

evgeni commented Apr 19, 2024

This is not needed on content proxies? Just the main server?

@sjha4 sjha4 force-pushed the add_container_label_migration branch from 2eee300 to 9317a24 Compare April 19, 2024 12:07
@sjha4
Copy link
Contributor Author

sjha4 commented Apr 19, 2024

This is not needed on content proxies? Just the main server?

It is..Added to proxy upgrade.. Do the same scenarios run for upstream upgrades as well?

ianballou
ianballou reviewed Apr 19, 2024
View reviewed changes
Copy link
Contributor

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

Just wanted to note that we'll also need to add the indexing task to this afterwards.

definitions/scenarios/upgrade_to_capsule_6_16.rb Outdated Show resolved Hide resolved
@sjha4 sjha4 force-pushed the add_container_label_migration branch 3 times, most recently from 5d718c7 to b37dc32 Compare April 23, 2024 12:13
@sjha4
Copy link
Contributor Author

sjha4 commented May 7, 2024

Will get to the merge conflict. Does the change look good @ehelms ?

ianballou
ianballou approved these changes May 7, 2024
View reviewed changes
Copy link
Contributor

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

Just gave this a test with a mock Satellite upgrade, the command is indeed called right before the last step, and I saw the correct message, so I think it's all good.

@sjha4 sjha4 force-pushed the add_container_label_migration branch from b37dc32 to 99ea5bb Compare May 8, 2024 13:57
@ehelms
Copy link
Member

ehelms commented May 13, 2024

@sjha4 I will get to checking this early this week.

@sjha4 sjha4 force-pushed the add_container_label_migration branch from 99ea5bb to 1d1d5c2 Compare May 13, 2024 20:55
@ehelms ehelms merged commit b65062b into theforeman:master May 15, 2024
8 checks passed
@sjha4 sjha4 mentioned this pull request May 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ianballou ianballou ianballou approved these changes

@ehelms ehelms ehelms approved these changes

@evgeni evgeni Awaiting requested review from evgeni

Assignees
No one assigned
Labels
Waiting on contributor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sjha4 @evgeni @ehelms @ianballou