Only delete node object on GCE #13289
Conversation
|
/test ? |
|
@olemarkus: The following commands are available to trigger required jobs:
Use
In 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. |
|
/test pull-kops-e2e-aws-upgrade-123-ko123-to-klatest-kolatest |
k8s-ci-robot
added
cncf-cla: yes
|
/test pull-kops-e2e-aws-upgrade-123-ko123-to-klatest-kolatest |
1 similar comment
|
/test pull-kops-e2e-aws-upgrade-123-ko123-to-klatest-kolatest |
| @@ -222,17 +222,14 @@ func TestRollingUpdateAllNeedUpdate(t *testing.T) { | |||
| case testingclient.PatchAction: | |||
| if string(a.GetPatch()) == cordonPatch { | |||
| assertCordon(t, a) | |||
| assert.Equal(t, "", cordoned, "at most one node cordoned at a time") | |||
There was a problem hiding this comment.
I'm wondering if it would be worthwhile to hook into the TerminateInstances() mock in order to keep all of the testingclient.DeleteAction assertions.
There was a problem hiding this comment.
I thought about it, but it shouldn't actually delete any nodes, so seems strange to test for it.
We are also checking that the ASGs are empty at the end, so we know the terminations take place.
pkg/instancegroups/instancegroups.go
Outdated
| @@ -36,6 +36,7 @@ import ( | |||
| "k8s.io/klog/v2" | |||
| "k8s.io/kubectl/pkg/drain" | |||
|
|
|||
| "k8s.io/kops/pkg/apis/kops" | |||
There was a problem hiding this comment.
Already declared below.
| "k8s.io/kops/pkg/apis/kops" |
olemarkus
force-pushed
the
no-node-deletion
branch
2 times, most recently
from
85e2116
to
7f3f67d
Compare
olemarkus
force-pushed
the
no-node-deletion
branch
from
7f3f67d
to
9824636
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman 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 |
k8s-ci-robot
added
the
approved
…289-origin-release-1.22 Automated cherry pick of #13289: Only delete node object on GCE
…289-origin-release-1.23 Automated cherry pick of #13289: Only delete node object on GCE
I don't see why the GCE CCM shouldn't have deleted the node object before the new kublet registers given that the NodeLifecycle controller runs every 40 seconds ... But until we can test, we keep this for GCE.
/cc @justinsb @hakman @johngmyers
fixes #13259