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.

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

Refs #37275: Enable modules and download updates after enabling postgresql module #814

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Mar 27, 2024

The dnf module switch-to functionality not only swaps module streams but forces the installation of the new packages which can cause the system to be in a bad state. For example, sync plan disable will break in some cases.

I should also mention that it breaks the paradigm we designed where package updates are downloaded but not applied before entering maintenance mode.

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.

You can't enable the satellite:el8 module as it now has broken deps. That has to happen after the switch.
And because if that the download of packages needs to also happen after the switch, as otherwise no packages are downloaded (dnf discards modules with broken deps, and thus all RPMs provided by them)

@ehelms
Copy link
Member Author

ehelms commented Mar 27, 2024

You can't enable the satellite:el8 module as it now has broken deps. That has to happen after the switch. And because if that the download of packages needs to also happen after the switch, as otherwise no packages are downloaded (dnf discards modules with broken deps, and thus all RPMs provided by them)

I was afraid of that, I hadn't tested this yet. I have another idea but need to test it.

@evgeni
Copy link
Member

evgeni commented Mar 27, 2024

Yeah, I think you need to move this whole block:

add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
if el8?
modules_to_switch = ['postgresql:13']
add_step(Procedures::Packages::SwitchModules.new(:module_names => modules_to_switch))
modules_to_enable = ["satellite:#{el_short_name}"]
add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
end
add_step(Procedures::Packages::Update.new(
:assumeyes => true,
:dnf_options => ['--downloadonly']
))

down to Migrations.

Also, frankly, it makes more sense there. PreMigrations should be non-destructive, while this code is.

@@ -39,6 +39,7 @@ class PreMigrations < Abstract
end

def compose
add_step(Procedures::SyncPlans::Disable.new)
Copy link
Member

Choose a reason for hiding this comment

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

Should the same change happen in capsule? OTOH, capsules don't have syncplans.

@evgeni
Copy link
Member

evgeni commented Mar 27, 2024

I think this should refer to https://projects.theforeman.org/issues/37275 in some way?

@ehelms
Copy link
Member Author

ehelms commented Mar 27, 2024

Yeah, I think you need to move this whole block:

add_step(Procedures::Repositories::Setup.new(:version => '6.16'))
if el8?
modules_to_switch = ['postgresql:13']
add_step(Procedures::Packages::SwitchModules.new(:module_names => modules_to_switch))
modules_to_enable = ["satellite:#{el_short_name}"]
add_step(Procedures::Packages::EnableModules.new(:module_names => modules_to_enable))
end
add_step(Procedures::Packages::Update.new(
:assumeyes => true,
:dnf_options => ['--downloadonly']
))

down to Migrations.
Also, frankly, it makes more sense there. PreMigrations should be non-destructive, while this code is.

I was hoping I could find a way to avoid that :/ since it defeats the whole point of reducing the downtime by caching downloads. Why must modularity break everything.

@ehelms
Copy link
Member Author

ehelms commented Mar 27, 2024

Updated to move all of that logic (sadly) into the Migration block after maintenance mode

@evgeni
Copy link
Member

evgeni commented Mar 27, 2024

you have a typo in the redmine, it's 37275 not 37375 ;)

@lpramuk
Copy link

lpramuk commented Mar 28, 2024

Upgrade finished.

I am able to upgrade after applying this PR

@ehelms ehelms changed the title Switch to postgresql 13 module during upgrade after services are stopped Refs #37275: Enable modules and download updates after enabling postgresql module Mar 28, 2024
@ehelms ehelms merged commit b337428 into theforeman:master Mar 28, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants