Skip to content
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

Delete non-existent cloud provider nodes with Ready condition Unknown #72559

Merged
merged 1 commit into from
Jan 4, 2019

Conversation

andrewsykim
Copy link
Member

@andrewsykim andrewsykim commented Jan 4, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:
Also deletes nodes with Ready condition Unknown if the node does not exist in the cloud provider. This was introduced in #70344 because we assumed that deleted nodes report Ready condition as False instead of Unknown. This PR changes the logic to delete any node that has a Ready condition that is not True and does not exist in the cloud provider (consistent with what we already had prior to merging #70344).

Which issue(s) this PR fixes:
Fixes # #72499

Special notes for your reviewer:
@mtaufen I'm not sure if we should be expecting this condition to be Unknown, but I've opened this PR to bring things back to what they were prior to #70344. Let me know if you prefer another solution.

Does this PR introduce a user-facing change?:

Nodes deleted in the cloud provider with Ready condition `Unknown` should also be deleted on the API server. 

/assign @mtaufen
/sig cloud-provider node

@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jan 4, 2019
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/node Categorizes an issue or PR as relevant to SIG Node. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jan 4, 2019
@andrewsykim andrewsykim changed the title delete non-existent cloud provider nodes with Ready condition Unknown Delete non-existent cloud provider nodes with Ready condition Unknown Jan 4, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewsykim

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. sig/apps Categorizes an issue or PR as relevant to SIG Apps. labels Jan 4, 2019
@mtaufen
Copy link
Contributor

mtaufen commented Jan 4, 2019

This makes sense to me.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 4, 2019
@k8s-ci-robot k8s-ci-robot merged commit d3aa7b2 into kubernetes:master Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/node Categorizes an issue or PR as relevant to SIG Node. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants