exit nodeup gracefully if server already exists in k8s

[zetaab]

Original issue for this is #15057 but it was reverted due to other issues in #15129

some discussion available in #15114 (review)

I have verified that this solution works in OpenStack. When existing instance tries to join again to cluster (it does that when rebooting server OR restarting kops-configuration.service):

kops-controller log:

kops-controller-t4b8j kops-controller I0212 14:11:02.063861       1 server.go:148] 10.2.96.248:59480: instance already exists in kubernetes cluster

node log

Feb 12 14:11:03 nodes-helpa-fctnib nodeup[669]: I0212 14:11:03.187949     669 client.go:156] bootstrap request responded with code 204
Feb 12 14:11:03 nodes-helpa-fctnib systemd[1]: kops-configuration.service: Succeeded.
Feb 12 14:11:03 nodes-helpa-fctnib systemd[1]: Finished Run kOps bootstrap (nodeup).

So it works like planned. The only possible problem in this solution is that if someone do have older than 455 days https://github.com/kubernetes/kops/blob/master/cmd/kops-controller/pkg/server/server.go#L188 kubelet, the cert will expiry and it cannot renew the cert because it will be still part of the Kubernetes cluster (node is maybe in NotReady state). Currently the workaround for that is delete the node manually kubectl delete node... and restart it in cloudprovider (or kops-configuration.service in node itself). After that the node can request the new certificate. I am just hoping that people could update their clusters in time and they should NOT have 455 day old nodes in their cluster.

cc @justinsb @hakman

[hakman]

/hold for 1-2 days for more feedback

[justinsb]

Nit-picking, but we could also do http.StatusConflict - that's what I've seen from GCP APIs at least.

[justinsb]

One nit pick on the http status code, but I like this approach - a specific response, and exit with 0 👍

[justinsb]

On the expiry side, I think we should figure out what behaviour we want here when the node already exists. IIUC this is mostly a security defense-in-depth, so as we beef up auth here (e.g. the node callback ) then we might not need it at all. Whatever we decide, I'd like for us to try to converge the various clouds so they have similar behvaior here.

Anyway, I don't think my nit on the status code is blocking ....

/lgtm

[hakman]

On the expiry side, I think we should figure out what behaviour we want here when the node already exists. IIUC this is mostly a security defense-in-depth, so as we beef up auth here (e.g. the node callback ) then we might not need it at all. Whatever we decide, I'd like for us to try to converge the various clouds so they have similar behvaior here.

It is also nice to know that the node config is sent only once and we don't update it dynamically on reboot, if something changes in the IG config.

[k8s-ci-robot]

[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:

• OWNERS [hakman]

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