daemon: Drop workarounds for rpm-ostree bugs #3239
Conversation
In 4.12 (master) we'll be shipping with at least RHEL8.6's rpm-ostree 2022.2 which ships the fixes for the two bugs we were hacking around here.
openshift-ci
bot
added
the
approved
yuqi-zhang
left a comment
yuqi-zhang
left a comment
There was a problem hiding this comment.
A few notes:
- is there any way we can validate this via CI? Or is "rpm-ostree is running and we don't fail" good enough
- I think this might help with https://bugzilla.redhat.com/show_bug.cgi?id=2104978? Is that race-y at all?
| @@ -100,38 +100,7 @@ func (r *RpmOstreeClient) loadStatus() (*rpmOstreeState, error) { | |||
| } | |||
|
|
|||
| func (r *RpmOstreeClient) Initialize() error { | |||
There was a problem hiding this comment.
Hmm, maybe it's worth removing Initialize() altogether then, since the non-rpmostree client doesn't use this either.
We did add a regression test to rpm-ostree for the original failure that motivated this. I'm reasonably confident in this change for 4.12.
Hmmmm. Ohh I see, yes this looks like when I hit this in rpm-ostree in coreos/rpm-ostree#3523 I forgot to go back and update this MCO side workaround. So...fun. Probably what we should do is:
|
|
What's fun here is systemd got fixed in RHEL9 to make start requests internally idempotent, so that workaround is also unnecessary in the future...this stuff accumulates like barnacles on a ship. |
|
Just coming back to this PR, should we merge this (and only for 4.12+)? |
I don't have a strong opinion, but I'm pretty confident in doing so. One important thing; to follow up on #3239 (comment) Actually there's no (really strong) need to backport the "systemctl start idempotence" to the MCO code here because it's only invoked once on MCD startup and hence could only come into play in some sort of crash loop of the daemon. The real bug there is fixed (well, worked around) by openshift/os#898 |
yuqi-zhang
left a comment
yuqi-zhang
left a comment
There was a problem hiding this comment.
The real bug there is fixed (well, worked around) by openshift/os#898
Yes, I saw that, ok, let's get this in to 4.12 in conjunction and see if the behaviour is all settled now. If anything, 4.11 will only get the backport via openshift/os#900 so if anything does still not work as well, we can fix in 4.12
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@cgwalters: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
3 participants
In 4.12 (master) we'll be shipping with at least RHEL8.6's
rpm-ostree 2022.2 which ships the fixes for the two bugs we were
hacking around here.