Bug 2111817: daemon: Add a workaround for bug 2111817#3291
Conversation
So about a month ago we merged openshift@0bcb4d2 which dropped all the old workarounds we had for rpm-ostree bugs. I'm sure you are all shocked that less than a month later, we have a new one to add in this section. Actually though one of those previous bugs was arguably really a systemd bug provoked by rpm-ostree...and that's the case again here. This is all covered in https://bugzilla.redhat.com/show_bug.cgi?id=2104619 and coreos/rpm-ostree@21c82ff but TL;DR systemd's implementation of `InaccessiblePaths` has quadratic behavior in number of mounts, and so of course we only discover this when we're testing heavily loaded OCP clusters. The fix to rpm-ostree is inbound. But...adding this workaround which has the MCD dynamically reconfigure the system to drop that config will help unstick clusters so they can get the real fix.
|
Only compile tested locally, but CI should likely prove out the difference by comparing CPU time. |
openshift-ci
Bot
added
the
approved
cgwalters
changed the title
openshift-ci
Bot
added
bugzilla/severity-medium
|
@cgwalters: This pull request references Bugzilla bug 2111817, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions 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. |
|
As a general question, is this because backporting rpm-ostree changes via RHEL would take substantially longer, and we just want to do this to unblock 4.11? Or is this also required as a parallel? |
That's most definitely part of it. But to be clear, I am starting all the paperwork for that...doing it properly involves things like building in RHEL9 first etc.
I touched on the other rationale in the commit message:
In other words, to get the fixed OS, they need to be able to upgrade to it first...this unsticks that. |
|
|
||
| // See https://bugzilla.redhat.com/show_bug.cgi?id=2111817 | ||
| func bug2111817Workaround() error { | ||
| targetUnit := "/run/systemd/system/rpm-ostreed.service.d/bug2111817.conf" |
There was a problem hiding this comment.
Given the relative urgency of the issue involved, I'm not against doing it this way, but this does come with the consideration of whether we want to eventually remove this service from nodes.
Put another way, would this work with a dropin file template shipped in 4.11 instead? Then we can just remove the template file when we want it gone
There was a problem hiding this comment.
Note that the MCD is writing the file explicitly into /run. That means it's transient, and will go away on reboot (and be reapplied the next time the MCD lands - hence when we drop the workaround from the MCD, it will automatically go away).
There was a problem hiding this comment.
Ah yeah ok I see. That's fine then. I am +1 to shipping this since its only transient
Right, but as I understand it, this is only an issue in 4.11 and 4.12. If the rpm-ostree fix gets into 4.11, if the user goes from 4.10->4.11.z with fix, we wouldn't need this fix right? That was my main question (how long we would need to keep this workaround) |
I think Colin is pointing out that shipping a new rpm-ostree doesn't unstick already stuck upgrades. So the MCD workaround allows us to unstick existing stuck upgrades. I don't have a firm opinion on whether this is necessary but if we believe the rpm-ostree delivery timelines are in excess of one week (in RHCOS 4.11 by next Wednesday) this MCO change, provided it's all agreed to be safe, is probably worth pursuing. |
Ah yes sorry, you are 100% correct here. I was thinking of the 4.11 ➡️ 4.12 scenario but that's not production yet. So... https://bugzilla.redhat.com/show_bug.cgi?id=2118774 now tracks the 8.7 bug and I'll have to z-stream clone to 8.6. I think I can plumb that all through this week, fingers crossed. Dunno. I am OK to close this and pursue that path exclusively if you prefer. |
|
I launched a cluster from this PR and verified it works. You can tell by doing e.g.: The CPU timing should be very low, e.g. here on a relatively idle node: But doing |
I think based on Scott's assessment, this has value. I am +1 on getting this in in that context. /retest Just to be safe we pass the upgrade test. I can lgtm today since I don't see much risk here. |
|
Also gives us an opportunity to test the new Jira backport process :) |
|
@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. |
yuqi-zhang
left a comment
yuqi-zhang
left a comment
There was a problem hiding this comment.
/lgtm
Should be a safe workaround, to be removed when rpm-ostree fix goes in
|
[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 pull requests linked via external trackers have merged: Bugzilla bug 2111817 has been moved to the MODIFIED state. DetailsIn response to this:
Instructions 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. |
|
Since we need this fix in 4.11, backporting. |
|
@sinnykumari: #3291 failed to apply on top of branch "release-4.11": DetailsIn response to this:
Instructions 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. |
|
Manual backport PR #3292 |
6 participants
So about a month ago we merged
0bcb4d2
which dropped all the old workarounds we had for rpm-ostree bugs.
I'm sure you are all shocked that less than a month later, we have
a new one to add in this section.
Actually though one of those previous bugs was arguably really a
systemd bug provoked by rpm-ostree...and that's the case again here.
This is all covered in
https://bugzilla.redhat.com/show_bug.cgi?id=2104619
and
coreos/rpm-ostree@21c82ff
but TL;DR systemd's implementation of
InaccessiblePathshasquadratic behavior in number of mounts, and so of course
we only discover this when we're testing heavily loaded OCP clusters.
The fix to rpm-ostree is inbound. But...adding this workaround which
has the MCD dynamically reconfigure the system to drop that config will
help unstick clusters so they can get the real fix.