Bug 1852047: daemon: Remove "compare digest" code for OS updates #1971
Conversation
openshift-ci-robot
added
the
approved
cgwalters
force-pushed
the
do-noop-upgrades
branch
2 times, most recently
from
6a9b5eb to
be32dcf
Compare
kikisdeliveryservice
left a comment
kikisdeliveryservice
left a comment
There was a problem hiding this comment.
Love the simplification!!
/lgtm
pkg/daemon/daemon.go
Outdated
There was a problem hiding this comment.
We can remove the error return from the function altogether at this point.
func compareOSImageURL(current, desired string) bool {
return desired == "" || desired == "://dummy" || current == desired
}("No target" log omitted for demonstration only)
There was a problem hiding this comment.
Yep you're right, it turns into a bigger patch but as long as we're here we might as well do the bigger cleanup.
cgwalters
force-pushed
the
do-noop-upgrades
branch
from
be32dcf to
83477ec
Compare
cgwalters
force-pushed
the
do-noop-upgrades
branch
from
83477ec to
b22845e
Compare
|
yay for simplified code! |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
6 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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
I am not 100% sure but there is lot of possibility that this PR merge will create merge conflict to the extensions feature https://github.com/openshift/machine-config-operator/pull/1941/files#diff-06961b075f1753956d802ba954d2cfb5L1319 . It will be good to fix this on top of #1941. Adding hold to get Pr#1941 merged. |
openshift-ci-robot
added
the
do-not-merge/hold
|
#1941 merged |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/retest Please review the full test history for this PR and help us cut down flakes. |
3 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. |
4 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. |
cgwalters
changed the title
openshift-ci-robot
added
the
bugzilla/severity-high
|
@cgwalters: This pull request references Bugzilla bug 1852047, 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
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. |
openshift-ci-robot
added
the
bugzilla/valid-bug
|
/retest Please review the full test history for this PR and help us cut down flakes. |
16 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. |
|
/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. |
|
/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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@cgwalters: Some pull requests linked via external trackers have merged: . The following pull requests linked via external trackers have not merged:
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. |
7 participants
We don't need to do this special casing anymore. This
corner case only arises for the synthetic
e2e-gcp-upgradetest, and we actually want to test the OS update path there.
Closes: #1964