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.

Sign up for GitHub

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

Jump to bottom

Fix a bug in validating node existence. #57484

Merged
merged 1 commit into from
Dec 21, 2017
Merged

Fix a bug in validating node existence. #57484

merged 1 commit into from
Dec 21, 2017

Conversation

brendandburns
Copy link
Contributor

@brendandburns brendandburns commented Dec 20, 2017 •
edited by enisoc
Loading

What this PR does / why we need it:
Fixes an bug where if an error was returned that was not an autorest.DetailedError we would return "not found", nil which would result in Nodes going NotReady

Which issue(s) this PR fixes
Fixes #57483

Release note:

Fixes a bug where if an error was returned that was not an `autorest.DetailedError` we would return `"not found", nil` which caused nodes to go to `NotReady` state.

@feiskyer @khendiak

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. 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. labels Dec 20, 2017
@brendandburns brendandburns mentioned this pull request Dec 20, 2017
Closed
itowlson
itowlson approved these changes Dec 21, 2017
View reviewed changes
pkg/cloudprovider/providers/azure/azure_wrap_test.go
@@ -0,0 +1,53 @@
/*
Copyright 2016 The Kubernetes Authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

ridiculous pedantry: should this be 2017?

pkg/cloudprovider/providers/azure/azure_wrap_test.go Outdated
"fmt"
"net/http"
"reflect"
"testing"
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the alignment in the import section is inconsistent, or is that the GitHub UI?

@feiskyer
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 21, 2017
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, feiskyer

Associated issue: #57483

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@brendandburns
Copy link
Contributor Author

/retest

@brendandburns brendandburns added this to the v1.10 milestone Dec 21, 2017
@k8s-cherrypick-bot
Copy link

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

@brendandburns
Copy link
Contributor Author

Once this merges...

@jdumars can we see about getting this cherry picked into 1.8.x and 1.9.x (and possibly 1.7.x?)

Thanks

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 57282, 57484). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 1bcf0b0 into kubernetes:master Dec 21, 2017
@jdumars
Copy link
Member

jdumars commented Dec 21, 2017

On it @brendandburns

@k8s-cherrypick-bot
Copy link

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

@jdumars jdumars modified the milestones: v1.10, v1.9 Dec 21, 2017
@jdumars jdumars mentioned this pull request Dec 22, 2017
Merged
@jdumars
Copy link
Member

jdumars commented Dec 22, 2017

@brendandburns I ran into some merge conflicts with azure.go on the the 1.8 cherry pick, and am not sure I can do it safely.

@brendandburns
Copy link
Contributor Author

brendandburns commented Dec 22, 2017 via email

This was referenced Dec 22, 2017
Merged
Merged
k8s-github-robot pushed a commit that referenced this pull request Dec 23, 2017
Merge pull request #57557 from jdumars/automated-cherry-pick-of-#5748…
88944a8 
…4-upstream-release-1.9

Automatic merge from submit-queue.

Automated cherry pick of #57484: Fix a bug in validating node existence.

Cherry pick of #57484 on release-1.9.

#57484: Fix a bug in validating node existence.
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.9" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

k8s-github-robot pushed a commit that referenced this pull request Dec 23, 2017
Merge pull request #57562 from brendandburns/automated-cherry-pick-of…
79d0c20 
…-#57484-upstream-release-1.8

Automatic merge from submit-queue.

Automated cherry pick of #57484

Cherry pick of #57484 on release-1.8.

#57484: Fix a bug in validating node existence.
k8s-github-robot pushed a commit that referenced this pull request Dec 28, 2017
Merge pull request #57563 from brendandburns/automated-cherry-pick-of…
a979440 
…-#57484-upstream-release-1.7

Automatic merge from submit-queue.

Automated cherry pick of #57484

Cherry pick of #57484 on release-1.7.

#57484: Fix a bug in validating node existence.
@brendandburns brendandburns mentioned this pull request Jun 13, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itowlson itowlson itowlson approved these changes

Assignees

@feiskyer feiskyer

@jdumars jdumars

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.9
Development

Successfully merging this pull request may close these issues.

An unexpected error can cause nodes in Azure to go NotReady
7 participants
@brendandburns @feiskyer @k8s-ci-robot @k8s-cherrypick-bot @k8s-github-robot @jdumars @itowlson