-
Notifications
You must be signed in to change notification settings - Fork 413
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
Bug 1907333: daemon: Revert code to remove rollback #2302
Bug 1907333: daemon: Revert code to remove rollback #2302
Conversation
@cgwalters: This pull request references Bugzilla bug 1907333, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In 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. |
Should probably remove machine-config-operator/pkg/daemon/update.go Line 700 in 4a005cb
Otherwise I'm 👍 |
See https://bugzilla.redhat.com/1907333 Doing this when the MCD lands on a node the first time means we compete for I/O with lots of other containers, and on slow systems that can cause a timeout. Then a further problem is that we were fatally erroring out in the MCD rather than retrying. But we don't have control loop in the MCD today outside of trying to apply an update; let's defer this until we add one.
0ef525c
to
25a7812
Compare
Eh, I would have kept it since we might use it in the future, but OK I removed it. That said I think what we really want actually is coreos/rpm-ostree#2389 |
/test e2e-openstack |
There is progress, at least now it's deploying the cluster 👍 |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, sinnykumari The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test e2e-aws |
/retest Please review the full test history for this PR and help us cut down flakes. |
5 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@cgwalters: All pull requests linked via external trackers have merged: Bugzilla bug 1907333 has been moved to the MODIFIED state. In 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. |
...time passes...we do now! Added in #3070 So I think we could safely do a PR that re-reverts this (i.e. readds it) to fix https://bugzilla.redhat.com/show_bug.cgi?id=2104619 |
See https://bugzilla.redhat.com/1907333
Doing this when the MCD lands on a node the first time means
we compete for I/O with lots of other containers, and on
slow systems that can cause a timeout. Then a further
problem is that we were fatally erroring out in the MCD
rather than retrying.
But we don't have control loop in the MCD today outside of
trying to apply an update; let's defer this until we add one.