Skip calling rpm-ostree kargs in no-op case #586
Conversation
If delete and append kargs args are the same in the same order there is no functional differences so we can safely skip calling rpm-ostree kargs and avoid creating a new rpm-ostree deployment. Resolves: RHEL-20767 Signed-off-by: Etienne Champetier <[email protected]>
There was a problem hiding this comment.
IIUC, this change aims mainly to resolve the issue that occurs when the append list only contains already set kargs.
Rather than checking for the equality of the append_params and delete_params lists after constructing them, wouldn't it be easier to skip adding a parameter to delete_params when the currently active parameter value is equal to the one specified by append?
In other words, we would need an additional condition here which would evaluate to True only when the currently set parameter value and the one given by append differ.
|
If I follow correctly, if you don't add the existing kargs to |
|
Oh, you're right about that. Then again, we don't need to restore anything if an existing karg is the same as a karg specified by the profile. So if that karg would not be returned in I'm okay with approving this change as it is because it's simple and should work, but I feel like it could be generalized a bit more (though I agree that would require more changes). |
|
For some kargs ordering is important (example https://bugzilla.redhat.com/show_bug.cgi?id=1776823), i think the current implementation for the ostree path remove the kargs in whatever order and add them back in the order they appear in the profile, and my patch skips only if they are the same in the same order. I like my patch because it's simple enough to reason about (and to backport / hotpatch in prod), as you are okay please merge :) |
|
Thanks, let's go with this simple change. |
If delete and append kargs args are the same in the same order there is no functional differences so we can safely skip calling rpm-ostree kargs and avoid creating a new rpm-ostree deployment.
Resolves: RHEL-20767
Fixes #585