Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AWS subnet will not be used for new ELB if legacy KubernetesCluster tag appears before new kubernetes.io/cluster tag #64230

Closed
twilfong opened this issue May 23, 2018 · 1 comment · Fixed by #64231
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@twilfong
Copy link
Contributor

/kind bug

What happened:

When you create a new Service of type LoadBalancer in a Kubernetes (v1.9) cluster hosted on AWS with nodes spread across subnets in 3 AvailabilityZones, the new ELB that Kube creates is only enabled for 2 AZs. This happens even though all three subnets have the correct cluster tags, and the same list of tags other than the Name tag.

I've determined that, unfortunately, the order of tags matters. If a subnet has the KubernetesCluster tag, and this tag appears in the tag list before the kubernetes.io/cluster/X tag, then only this KubernetesCluster tag will be used to decide if this subnet is a candidate for use by the cluster. For subnets that are shared between multiple clusters, and which still have old legacy tags on them in addition to newer type of tag, this means the subnet can only be used by the cluster that matches the KubernetesCluster tag. However, if a newer kubernetes.io/cluster/X tag, where X matches the cluster name, appears first in the list, then that tag is used to determine subnet candidacy.

What you expected to happen:

For a Kubernetes (v1.9) cluster named "X", with nodes spread across subnets in 3 AvailabilityZones, when you create a new Service of type LoadBalancer, the new ELB that Kube creates is enabled for all 3 AZs if there is a subnet in each of those 3 AZs that contains a tag with key "kubernetes.io/cluster/X" and value "shared". Ordering of keys should not matter so long as one of the tag keys is "kubernetes.io/cluster/X".

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

Create 3 (public) subnets in 3 different AZs in an AWS VPC with the following tags:

Key Value
kubernetes.io/cluster/a.k8s.local shared
kubernetes.io/cluster/b.k8s.local shared
kubernetes.io/role/elb 1
KubernetesCluster a.k8s.local

However, in one of the 3 subnets above, ensure that KubernetesCluster tag appears first in the tag list, instead of last.

Spin up a new Kube cluster named "b.k8s.local" (via Kops or other means) that uses the above 3 public subnets.

Create a Service of type LoadBalancer in Kube cluster b.k8s.local and notice that the resulting ELB created in AWS is only associated with 2 of the 3 subnets, and that the one which is absent is the one that had the KubernetesCluster tag appearing in the tag list before the kubernetes.io/cluster/b.k8s.local tag.

Anything else we need to know?:

This happens because the hasClusterTag function in cloudprovider/providers/aws/tags.go checks to see if a tag in the list has legacy key value before checking for the newer key value.

I believe that If a subnet has any tags with key matching "kubernetes.io/cluster/*" then these tags should take precedence over the "KubernetesCluster" tag. I will make a pull request to enable this behavior.

If the intention is that KubernetesCluster tag should take precedence, then this is still a bug, because this tag will currently only take precedence if it appears before a matching "kubernetes.io/cluster/*" tag in the tag list.

Environment:

  • Kubernetes version: 1.9
  • Cloud provider or hardware configuration: AWS
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. kind/bug Categorizes issue or PR as related to a bug. labels May 23, 2018
@twilfong
Copy link
Contributor Author

/sig aws

@k8s-ci-robot k8s-ci-robot added sig/aws and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels May 23, 2018
k8s-github-robot pushed a commit that referenced this issue May 25, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Update function hasClusterTag to fix issue #64230

**What this PR does / why we need it**:

Fixes issue #64230, by changing function hasClusterTag, in aws/tags.go, to ensure that, when called with a list of tags containing a tag with a key which matches clusterTagKey, function will return true even if a tag with key TagNameKubernetesClusterLegacy also exists in the list with a value other than the ClusterID.

**Which issue(s) this PR fixes**:
Fixes #64230

**Special notes for your reviewer**:
Notes are in issue

**Release note**:
```release-note
NONE
```
wenjiaswe pushed a commit to wenjiaswe/kubernetes that referenced this issue Jun 19, 2018
Fixes issue kubernetes#64230, by changing function hasClusterTag, in aws/tags.go, to ensure that a list of tags containing a tag with a key which matches clusterTagKey will return true even if a TagNameKubernetesClusterLegacy tag also exists in the list with a value other than the ClusterID.

/sig aws
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants