Assume volume is detached if node doesn't exist #29485
Conversation
saad-ali
added
release-note
k8s-github-robot
added
the
size/M
| @@ -869,7 +869,7 @@ func (c *Cloud) InstanceID(name string) (string, error) { | |||
| } | |||
| inst, err := c.getInstanceByNodeName(name) | |||
| if err != nil { | |||
| return "", err | |||
| return "", fmt.Errorf("getInstanceByNodeName failed for %q with %v", name, err) | |||
There was a problem hiding this comment.
Yes, we have to be really careful with when we return InstanceNotFound. According to the comments on cloudprovider.Instances, we must return InstanceNotFound, but actually that's wrong and it only applies to ExternalID (AFAICT), and you didn't wrap it there. I'll put in a separate PR to fix the comments.
To (try to) be clear: But what you're done here is right, though it would be wrong if the docs were right. At least I think so!
|
LGTM |
|
Doc fix for the contract: #29490 |
saad-ali
force-pushed
the
fixIssue29358InstanceNotFound
branch
from
fa2e107 to
89fd358
Compare
|
Thanks @justinsb |
|
GCE e2e build/test passed for commit 89fd358. |
matchstick
added
the
lgtm
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit 89fd358. |
|
Automatic merge from submit-queue |
fabioy
added
the
cherry-pick-approved
| nodeName, | ||
| err) | ||
| return nil |
There was a problem hiding this comment.
should this be continuing (as per the comment), rather than returning from the whole func?
There was a problem hiding this comment.
The behavior is correct. Perhaps the comment can be clarified. "Move on" in this case means abandon trying to update the status of a node (since it no longer exists).
There was a problem hiding this comment.
ok, wasn't sure if it was intending to also short-circuit and not update any of the other remaining nodes, or if a bad nodeName could permanently block ever getting to update those nodes' status
…ck-of-#24385-kubernetes#29485-upstream-release-1.3 Automated cherry pick of kubernetes#24385 kubernetes#29485 upstream release 1.3
Fixes #29358