-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -771,8 +771,10 @@ module YumUpgradeOutput = struct | |
| | false -> ( | ||
| take_till is_eol <* end_of_line >>= function | ||
| | ( "Installing:" | ||
| | "Installing group/module packages:" | ||
| | "Updating:" | ||
| | "Upgrading:" | ||
| | "Upgrading groups:" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| | "Removing:" | ||
| | "Reinstalling:" | ||
| | "Downgrading:" | ||
|
|
@@ -890,6 +892,7 @@ module YumUpgradeOutput = struct | |
| |> List.filter (fun (section, _) -> | ||
| match section with | ||
| | "Installing:" | ||
| | "Installing group/module packages:" | ||
| | "Updating:" | ||
| | "Upgrading:" | ||
| | "Installing for dependencies:" | ||
|
|
@@ -958,6 +961,31 @@ let get_updates_from_yum_upgrade_dry_run repositories = | |
| error "%s" (ExnHelper.string_of_exn e) ; | ||
| None | ||
|
|
||
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems same with |
||
| match Forkhelpers.execute_command_get_output cmd params with | ||
| | _, _ -> | ||
| Some [] | ||
| | exception Forkhelpers.Spawn_internal_error (stderr, stdout, Unix.WEXITED 1) | ||
| -> ( | ||
| (*Yum put the details to stderr while dnf to stdout*) | ||
| (match Pkgs.manager with Yum -> stderr | Dnf -> stdout) | ||
| |> YumUpgradeOutput.parse_output_of_dry_run | ||
| |> function | ||
| | Ok (pkgs, Some txn_file) -> | ||
| Unixext.unlink_safe txn_file ; | ||
| Some pkgs | ||
| | Ok (pkgs, None) -> | ||
| Some pkgs | ||
| | Error msg -> | ||
| error "%s" msg ; None | ||
| ) | ||
| | exception e -> | ||
| error "%s" (ExnHelper.string_of_exn e) ; | ||
| None | ||
|
|
||
| let get_latest_updates_from_redundancy ~fail_on_error ~pkgs ~fallback_pkgs = | ||
| let err = "Failed to parse output of 'yum upgrade (dry run)' correctly" in | ||
| let get_latest_updates_from_redundancy' ~fail_on_error ~pkgs ~fallback_pkgs = | ||
|
|
||
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 butgroup_upgrade ()fail, we still get the errorapply_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
upgradeorgroup_upgradeare 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.