kubernetes.io/cluster/CLUSTER_NAME tag missing from EIPs, NAT GWs, S3 buckets and R53 zones

[joelddiaz]

Version

$ openshift-install version
./bin/openshift-install v0.1.0
Terraform v0.11.8

Platform (aws|libvirt|openshift):

aws

What happened?

After installation, I see some objects are tagged with kubernetes.io/cluster/CLUSTER_NAME and some aren't. Specifically, I've noticed that EIPs, NAT Gateways, S3 buckets, and Route53 zones are missing the above tag.

What you expected to happen?

I expect the tags applied to objects during installation to be consistent.

How to reproduce it (as minimally and precisely as possible)?

1) install a cluster as documented
2) query the tags on any of the EIPs, NAT Gateways, S3 buckets, and Route53 zones that were created during installation.
  For example: aws ec2 describe-nat-gateways --region us-east-1 --nat-gateway-ids nat-00f483ee692b50d54 | jq -r '.NatGateways[0].Tags'
[
  {
    "Key": "tectonicClusterID",
    "Value": "17993b0f-7370-4ee1-9e28-209db9a321d9"
  }
]

[wking]

I expect the tags applied to objects during installation to be consistent.

I haven't found upstream docs for the tag, but this suggests that tag is for resources we expect Kubernetes to take over and manage for us. Is that true for all of the resources which currently lack this tag? I think we need to find upstream docs for the tag, and then make the argument for each resource individually.

[joelddiaz]

@wking who could we talk to that could shed some light on the appropriateness of adding the kubernetes.io/cluster/CLUSTER_X tag to objects not intended for kubernetes usage.

really, what i'm looking for is "one true tag" so that i could search and see everything in AWS related to CLUSTER_X.

[wking]

really, what i'm looking for is "one true tag" so that i could search and see everything in AWS related to CLUSTER_X.

I think tectonicClusterID is that tag. Are there resources missing that tag?

[joelddiaz]

yes. any object created by the cluster during runtime (eg PVs, security groups, ELBs) don't get the tectonicClusterID tag.

[wking]

any object created by the cluster during runtime (eg PVs, security groups, ELBs) don't get the tectonicClusterID tag.

So what do we need to update so that those get tagged? I guess other repos should be pulling the tag information out of the install-config we push into cluster-config-v1 in the kube-system namespace? We probably want to push a helper function those external projects can vendor that combines UserTags with ClusterID (and anything else?) so they don't have to code that up themselves.

[wking]

This (I think) is setting the tag for workers created by the machine-API operator. Do you know who's creating the untagged resources? If not, can you give me more information about them, e.g. what ELB is untagged?

[joelddiaz]

@wking for the PVs, it's the kubernetes storage class provisioner

@ironcladlou can you give some details on which code is creating the ELBs/security groups that weren't being found by the uninstaller?

[ironcladlou]

To reproduce, create a service like:

kind: Service
apiVersion: v1
metadata:
  name: cloud-service
spec:
  type: LoadBalancer
  selector:
    app: some-app
  externalTrafficPolicy: Local
  ports:
  - name: http
    protocol: TCP
    port: 80
    targetPort: http

Here's a starting point upstream for understanding how k8s creates the resources in a given cloud provider to manifest such a service.

[wking]

Thanks for the code link :). It looks like folks creating services should be setting service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags on load-balancer services like that if they want them tagged. For anything we control, I think that means we want to add our cluster tags (#465, or something similar after install-config is dropped from the cluster). For things that other folks inject into the cluster, I dunno. Something like the old worker scale-down request we used to have? I'm ok leaving it up to the cluster admins to scale down anything extra they've added before coming back and calling openshift-install destroy-cluster, as long as we do a good job cleaning up our own stuff.

[joelddiaz]

@wking so i looked at the annotation and it does indeed work as advertised (it even tagged the security group associated with the load balancer as well). the problem is that end users are free to create the service load balancers, and they are free to leave them untagged. this of course complicates cleaning up the cluster for deprovisioning.

talking this over with some people it was suggested that having an admission server catch the service object creations could then add the annotations as needed. but this functionality doesn't exist today.

i also looked at the options for getting custom tagging working on the PV/storage classes. i didn't find anything like what is possible with the service load balancers.

[wking]

... the problem is that end users are free to create the service load balancers, and they are free to leave them untagged. this of course complicates cleaning up the cluster for deprovisioning.

This is not something we can solve completely via tagging. For example, someone with an AWS cluster might create GCE resources for some task. There's no way we're going to have our AWS-cleanup logic reap that for them. I think the solution to this sort of thing is to require users to spin down anything they've created on their own before they call our delete logic. Of course, the more we tag where our cleanup logic can find it, the less they'll have to manually spin down. So I'm in favor of comprehensive tagging. I just think getting user-created resources tagged is a nice-to-have and not a must-have.

On the kubernetes.io/cluster/ front, it looks like an AWS-ism (although that's not a problem if we're only looking at AWS cleanup ;). The tag is from kubernetes/kubernetes#41695. ELB tagging came in via kubernetes/kubernetes#45932. Based on that diff, I'd guess that tagging any resources we create with kubernetes.io/cluster will be ok, but:

• Kubernetes may create new EIPs, gateways, etc. without the tag, if it hasn't been specifically taught to apply the tag to that resource type yet. In this case, we're still back to "how do we reap these if the creator isn't tagging them?".

• The docs for the tag specify uniqueness within an AZ (which is clearly imprecise). The stale design-proposal suggests that the uniqueness constraint is really "within a VPC", which makes more sense. But things like S3 buckets and R53 zones do not live inside VPCs. What would the uniqueness constraint be then? Requiring cluster names to be unique within an AWS account seems overly strict, but we could always switch to using the UUID ClusterID as our kubernetes.io/cluster/ value, and in that case the uniqueness scope would no longer matter.

[joelddiaz]

This is not something we can solve completely via tagging. For example, someone with an AWS cluster might create GCE resources for some task. There's no way we're going to have our AWS-cleanup logic reap that for them. I think the solution to this sort of thing is to require users to spin down anything they've created on their own before they call our delete logic.

It does sound reasonable to insert a step to delete all non-protected projects/namespaces as an early step. But I think this does still leave us exposed if the cluster itself is dysfunctional (ie the PR that we're doing CI against broke the cluster in some strange way).

Kubernetes may create new EIPs, gateways, etc. without the tag, if it hasn't been specifically taught to apply the tag to that resource type yet. In this case, we're still back to "how do we reap these if the creator isn't tagging them?".

I certainly don't have a comprehensive (nor non-comprehensive ;) list of the kinds of objects the cluster can create during normal lifecycle, but I would treat the lack of tagging as a bug and get that component to behave like the rest of the system.

The docs for the tag specify uniqueness within an AZ (which is clearly imprecise). The stale design-proposal suggests that the uniqueness constraint is really "within a VPC", which makes more sense. But things like S3 buckets and R53 zones do not live inside VPCs. What would the uniqueness constraint be then? Requiring cluster names to be unique within an AWS account seems overly strict, but we could always switch to using the UUID ClusterID as our kubernetes.io/cluster/ value, and in that case the uniqueness scope would no longer matter.

I worry for the operations team that would have to look at UUID tags in the AWS console :). I suppose the names of the objects would help the 'browsing the AWS console' usecase, but using the UUID here would certainly avoid the naming collisions.

[ironcladlou]

So something to keep in mind is that very soon the cluster-ingress-operator is going to be enabled by default, which means that a LoadBalancer service is going to come out of the box with new AWS installs. This service will be untagged. Here's the service manifest. Questions:

1. Should the cluster-ingress-operator be actively annotating the service?
2. If so, from where should the value be obtained (e.g. install-config.clusterID in kube-system/configmaps/cluster-config-v1?)
3. Is there any precedent for this?