-
Notifications
You must be signed in to change notification settings - Fork 296
xapi: CA-423064: Trigger group upgrades in addition to package upgrades #6850
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
xapi: CA-423064: Trigger group upgrades in addition to package upgrades #6850
Conversation
In the event we add a package to one of the yum repository groups, that has no packages requiring it, dnf5 will not install it by default (as it has not implemented the `upgrade_group_objects_upgrade` configuration option - see https://dnf5.readthedocs.io/en/latest/dnf5.conf-todo.5.html). We do not wish to be reliant on having to ensure all new packages are required by an existing package, as such (at least until an implementation of the above option is done in dnf5) trigger a `group upgrade` in addition to the `upgrade`. Signed-off-by: Alex Brett <[email protected]>
aba165a to
6c7d41b
Compare
Signed-off-by: Alex Brett <[email protected]>
6c7d41b to
32adc7d
Compare
changlei-li
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments left to the author to decide.
| let Pkg_mgr.{cmd; params} = Pkgs.apply_group_upgrade ~repositories in | ||
| ignore (Helpers.call_script cmd params) | ||
| in | ||
| try upgrade () ; group_upgrade () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: If upgrade () success but group_upgrade () fail, we still get the error apply_updates_failed. But I think the update is partially applied. Would you consider adding a new error and log to distinguish it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the consequences of failing in upgrade or group_upgrade are the same: some RPMs have been updated and some have not. The remedy is probably also the same: just try to apply again. So I think a separate error is not very useful.
| | "Installing group/module packages:" | ||
| | "Updating:" | ||
| | "Upgrading:" | ||
| | "Upgrading groups:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Will you test the new added lines for dry_run output parser in test_repository_helpers.ml?
| let get_updates_from_yum_group_upgrade_dry_run repositories = | ||
| let Pkg_mgr.{cmd; params} = | ||
| Pkgs.get_updates_from_group_upgrade_dry_run ~repositories | ||
| in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems same with get_updates_from_yum_upgrade_dry_run except for the {cmd; params}. Maybe can refactor to a common function.
In the event we add a package to one of the yum repository groups, that has no packages requiring it, dnf5 will not install it by default (as it has not implemented the
upgrade_group_objects_upgradeconfiguration option - see https://dnf5.readthedocs.io/en/latest/dnf5.conf-todo.5.html).We do not wish to be reliant on having to ensure all new packages are required by an existing package, as such (at least until an implementation of the above option is done in dnf5) trigger a
group upgrade *in addition to theupgrade.Note that a
group upgrade *alone is not sufficient, as this will not cause any packages that are pulled in as dependencies to be upgraded, only those explicitly listed in the group(s).