Add a default NodeLabel with the InstanceGroup name

[georgebuckerfield]

As requested in #2999, this change just auto-populates new InstanceGroup specs with a default node label containing the name of the instance group. It would be really useful for those of us managing environments with multiple instance groups.

It allows an admin to easily view the instance groups using kubectl:

kubectl get nodes --label-columns kops.k8s.io/instancegroup
NAME                                           STATUS         AGE       VERSION   INSTANCEGROUP
ip-172-20-108-120.eu-west-1.compute.internal   Ready,node     3m        v1.7.4    xtra-large
ip-172-20-117-133.eu-west-1.compute.internal   Ready,master   14m       v1.7.4    master-eu-west-1c
ip-172-20-32-139.eu-west-1.compute.internal    Ready,master   14m       v1.7.4    master-eu-west-1a
ip-172-20-32-92.eu-west-1.compute.internal     Ready,node     12m       v1.7.4    nodes
ip-172-20-67-184.eu-west-1.compute.internal    Ready,master   13m       v1.7.4    master-eu-west-1b

[georgebuckerfield]

/retest

[k8s-ci-robot]

@georgebuckerfield: you can't request testing unless you are a kubernetes member.

In response to this:

/retest

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.

[justinsb]

/retest

[justinsb]

So I think you need to update the tests. I think the approach looks good.

If we do this using this PR, this label will effectively be mandatory. Another option is to make it so that when you create an instancegroup, we auto-populate the nodeLabels in the spec. Then it would be explicit, and easily changed / removed.

I do think it's a nice feature though, and labels should be additive, so I'm OK with having them always there. My bigger concern is that instance groups might well map to something in the machine API, and then we would want a different label.

cc @pipejakob any thoughts on this wrt the Machines API?

[georgebuckerfield]

Thanks @justinsb! Yes, I'd missed that PopulateInstanceGroupSpec gets called by kops edit ig etc. I've re-written so that the logic is moved into pkg/apis/kops/instancegroup.go (let me know if this isn't the best place for it) and called by cmd/kops/create_cluster.go and cmd/kops/create_ig.go. This means a user can remove the labels by editing the IG if they wish. Also working on fixing the tests.

[justinsb]

/ok-to-test

/lgtm

Looks great - thanks @georgebuckerfield

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb

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

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

• OWNERS [justinsb]

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

[k8s-github-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue.