Skip to content

add kubelet skew check for MCO upgradeable#2552

Closed
deads2k wants to merge 1 commit into
openshift:masterfrom
deads2k:kubelet-skew
Closed

add kubelet skew check for MCO upgradeable#2552
deads2k wants to merge 1 commit into
openshift:masterfrom
deads2k:kubelet-skew

Conversation

@deads2k
Copy link
Copy Markdown
Contributor

@deads2k deads2k commented Apr 29, 2021

Implementation of openshift/enhancements#762 with locality at the point of action.

Produces a condition like
KubeletSkewTooFar node/first-node must be updated or removed before the cluster can upgrade, current version v1.18

/hold just a POC demonstrating ease and benefits of locality.

cc @crawford @rphillips

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 29, 2021
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deads2k
To complete the pull request process, please assign sinnykumari after the PR has been reviewed.
You can assign the PR to them by writing /assign @sinnykumari in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 21, 2021

@deads2k: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/verify 095184c link /test verify
ci/prow/e2e-aws-workers-rhel7 095184c link /test e2e-aws-workers-rhel7
ci/prow/e2e-metal-ipi 095184c link /test e2e-metal-ipi
ci/prow/e2e-aws-disruptive 095184c link /test e2e-aws-disruptive
ci/prow/okd-e2e-aws 095184c link /test okd-e2e-aws
ci/prow/e2e-agnostic-upgrade 095184c link /test e2e-agnostic-upgrade

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@nstielau
Copy link
Copy Markdown

I'm likely biased but some recent reports of difficulty, but explicit expectations seems better than implicit expectations. I can't even think of a place where we'd need a valid override (but there very well could be one).

Comment thread pkg/operator/status.go
// don't overwrite status if we didn't run
if ran, err := optr.isKubeletSkewTooFar(); ran && err != nil {
if coStatus.Status != configv1.ConditionFalse {
coStatus.Status = configv1.ConditionFalse
Copy link
Copy Markdown

@nstielau nstielau Jun 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd still love a coStatus.DocumentationLink type field here for these use-cases, but I knw that's a bigger change ;)

@QiWang19
Copy link
Copy Markdown
Member

Implementation of openshift/enhancements#762 with locality at the point of action.

Produces a condition like
KubeletSkewTooFar node/first-node must be updated or removed before the cluster can upgrade, current version v1.18

/hold just a POC demonstrating ease and benefits of locality.

@deads2k what do you mean just a POC? Is this PR supposed to be merged?

@kikisdeliveryservice
Copy link
Copy Markdown
Contributor

this is being worked on via another pr closing as it's unneeded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants