From 8759ff84b5932d180735dff3c716fd313894f088 Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Mon, 16 Dec 2024 21:48:52 -0600 Subject: [PATCH 1/3] Add problematic test case for NTH's ManagedASGTag --- ...s_s3_object_cluster-completed.spec_content | 2 +- ...mplex.example.com-addons-bootstrap_content | 2 +- ...e-termination-handler.aws-k8s-1.11_content | 2 +- .../complex/in-legacy-v1alpha2.yaml | 2 ++ .../update_cluster/complex/in-v1alpha2.yaml | 2 ++ .../update_cluster/complex/kubernetes.tf | 32 +++++-------------- 6 files changed, 15 insertions(+), 27 deletions(-) diff --git a/tests/integration/update_cluster/complex/data/aws_s3_object_cluster-completed.spec_content b/tests/integration/update_cluster/complex/data/aws_s3_object_cluster-completed.spec_content index ebf7c5064d45a..f4819fc064fd9 100644 --- a/tests/integration/update_cluster/complex/data/aws_s3_object_cluster-completed.spec_content +++ b/tests/integration/update_cluster/complex/data/aws_s3_object_cluster-completed.spec_content @@ -235,7 +235,7 @@ spec: enableSpotInterruptionDraining: true enabled: true excludeFromLoadBalancers: true - managedASGTag: aws-node-termination-handler/managed + managedASGTag: kubernetes.io/cluster/complex.example.com memoryRequest: 64Mi podTerminationGracePeriod: -1 prometheusEnable: false diff --git a/tests/integration/update_cluster/complex/data/aws_s3_object_complex.example.com-addons-bootstrap_content b/tests/integration/update_cluster/complex/data/aws_s3_object_complex.example.com-addons-bootstrap_content index db241b0360900..73038598b5df7 100644 --- a/tests/integration/update_cluster/complex/data/aws_s3_object_complex.example.com-addons-bootstrap_content +++ b/tests/integration/update_cluster/complex/data/aws_s3_object_complex.example.com-addons-bootstrap_content @@ -41,7 +41,7 @@ spec: version: 9.99.0 - id: k8s-1.11 manifest: node-termination-handler.aws/k8s-1.11.yaml - manifestHash: 93627ba43aa9bca83bd85005d328f2b42c28a9cf0cad1c03375fa4e33d59a877 + manifestHash: 565c78752a2216a5bef8c63aff4fe1435e054689f565f587b375c593b346a411 name: node-termination-handler.aws prune: kinds: diff --git a/tests/integration/update_cluster/complex/data/aws_s3_object_complex.example.com-addons-node-termination-handler.aws-k8s-1.11_content b/tests/integration/update_cluster/complex/data/aws_s3_object_complex.example.com-addons-node-termination-handler.aws-k8s-1.11_content index 6f8bf4fed96a6..d09e9a32bdf0b 100644 --- a/tests/integration/update_cluster/complex/data/aws_s3_object_complex.example.com-addons-node-termination-handler.aws-k8s-1.11_content +++ b/tests/integration/update_cluster/complex/data/aws_s3_object_complex.example.com-addons-node-termination-handler.aws-k8s-1.11_content @@ -176,7 +176,7 @@ spec: - name: CHECK_TAG_BEFORE_DRAINING value: "true" - name: MANAGED_TAG - value: aws-node-termination-handler/managed + value: kubernetes.io/cluster/complex.example.com - name: USE_PROVIDER_ID value: "true" - name: DRY_RUN diff --git a/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml b/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml index 4c74928ddcbc4..ff038e96bbd3a 100644 --- a/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml +++ b/tests/integration/update_cluster/complex/in-legacy-v1alpha2.yaml @@ -67,6 +67,8 @@ spec: kubernetesVersion: v1.30.0 masterPublicName: api.complex.example.com networkCIDR: 172.20.0.0/16 + nodeTerminationHandler: + managedASGTag: kubernetes.io/cluster/complex.example.com additionalNetworkCIDRs: - 10.1.0.0/16 - 10.2.0.0/16 diff --git a/tests/integration/update_cluster/complex/in-v1alpha2.yaml b/tests/integration/update_cluster/complex/in-v1alpha2.yaml index c0c414ea44cf3..aba1d11fbb652 100644 --- a/tests/integration/update_cluster/complex/in-v1alpha2.yaml +++ b/tests/integration/update_cluster/complex/in-v1alpha2.yaml @@ -67,6 +67,8 @@ spec: kubernetesVersion: v1.30.0 masterPublicName: api.complex.example.com networkCIDR: 172.20.0.0/16 + nodeTerminationHandler: + managedASGTag: kubernetes.io/cluster/complex.example.com additionalNetworkCIDRs: - 10.1.0.0/16 - 10.2.0.0/16 diff --git a/tests/integration/update_cluster/complex/kubernetes.tf b/tests/integration/update_cluster/complex/kubernetes.tf index 385254fc74385..6efe3a4b840ce 100644 --- a/tests/integration/update_cluster/complex/kubernetes.tf +++ b/tests/integration/update_cluster/complex/kubernetes.tf @@ -140,11 +140,6 @@ resource "aws_autoscaling_group" "master-us-test-1a-masters-complex-example-com" propagate_at_launch = true value = "John Doe" } - tag { - key = "aws-node-termination-handler/managed" - propagate_at_launch = true - value = "" - } tag { key = "foo/bar" propagate_at_launch = true @@ -183,7 +178,7 @@ resource "aws_autoscaling_group" "master-us-test-1a-masters-complex-example-com" tag { key = "kubernetes.io/cluster/complex.example.com" propagate_at_launch = true - value = "owned" + value = "" } target_group_arns = [aws_lb_target_group.tcp-complex-example-com-vpjolq.id, aws_lb_target_group.tls-complex-example-com-5nursn.id] vpc_zone_identifier = [aws_subnet.us-test-1a-complex-example-com.id] @@ -218,11 +213,6 @@ resource "aws_autoscaling_group" "nodes-complex-example-com" { propagate_at_launch = true value = "John Doe" } - tag { - key = "aws-node-termination-handler/managed" - propagate_at_launch = true - value = "" - } tag { key = "foo/bar" propagate_at_launch = true @@ -246,7 +236,7 @@ resource "aws_autoscaling_group" "nodes-complex-example-com" { tag { key = "kubernetes.io/cluster/complex.example.com" propagate_at_launch = true - value = "owned" + value = "" } vpc_zone_identifier = [aws_subnet.us-test-1a-complex-example-com.id] } @@ -493,7 +483,6 @@ resource "aws_launch_template" "master-us-test-1a-masters-complex-example-com" { "KubernetesCluster" = "complex.example.com" "Name" = "master-us-test-1a.masters.complex.example.com" "Owner" = "John Doe" - "aws-node-termination-handler/managed" = "" "foo/bar" = "fib+baz" "k8s.io/cluster-autoscaler/node-template/label/kops.k8s.io/kops-controller-pki" = "" "k8s.io/cluster-autoscaler/node-template/label/node-role.kubernetes.io/control-plane" = "" @@ -501,7 +490,7 @@ resource "aws_launch_template" "master-us-test-1a-masters-complex-example-com" { "k8s.io/role/control-plane" = "1" "k8s.io/role/master" = "1" "kops.k8s.io/instancegroup" = "master-us-test-1a" - "kubernetes.io/cluster/complex.example.com" = "owned" + "kubernetes.io/cluster/complex.example.com" = "" } } tag_specifications { @@ -510,7 +499,6 @@ resource "aws_launch_template" "master-us-test-1a-masters-complex-example-com" { "KubernetesCluster" = "complex.example.com" "Name" = "master-us-test-1a.masters.complex.example.com" "Owner" = "John Doe" - "aws-node-termination-handler/managed" = "" "foo/bar" = "fib+baz" "k8s.io/cluster-autoscaler/node-template/label/kops.k8s.io/kops-controller-pki" = "" "k8s.io/cluster-autoscaler/node-template/label/node-role.kubernetes.io/control-plane" = "" @@ -518,14 +506,13 @@ resource "aws_launch_template" "master-us-test-1a-masters-complex-example-com" { "k8s.io/role/control-plane" = "1" "k8s.io/role/master" = "1" "kops.k8s.io/instancegroup" = "master-us-test-1a" - "kubernetes.io/cluster/complex.example.com" = "owned" + "kubernetes.io/cluster/complex.example.com" = "" } } tags = { "KubernetesCluster" = "complex.example.com" "Name" = "master-us-test-1a.masters.complex.example.com" "Owner" = "John Doe" - "aws-node-termination-handler/managed" = "" "foo/bar" = "fib+baz" "k8s.io/cluster-autoscaler/node-template/label/kops.k8s.io/kops-controller-pki" = "" "k8s.io/cluster-autoscaler/node-template/label/node-role.kubernetes.io/control-plane" = "" @@ -533,7 +520,7 @@ resource "aws_launch_template" "master-us-test-1a-masters-complex-example-com" { "k8s.io/role/control-plane" = "1" "k8s.io/role/master" = "1" "kops.k8s.io/instancegroup" = "master-us-test-1a" - "kubernetes.io/cluster/complex.example.com" = "owned" + "kubernetes.io/cluster/complex.example.com" = "" } user_data = filebase64("${path.module}/data/aws_launch_template_master-us-test-1a.masters.complex.example.com_user_data") } @@ -593,12 +580,11 @@ resource "aws_launch_template" "nodes-complex-example-com" { "KubernetesCluster" = "complex.example.com" "Name" = "nodes.complex.example.com" "Owner" = "John Doe" - "aws-node-termination-handler/managed" = "" "foo/bar" = "fib+baz" "k8s.io/cluster-autoscaler/node-template/label/node-role.kubernetes.io/node" = "" "k8s.io/role/node" = "1" "kops.k8s.io/instancegroup" = "nodes" - "kubernetes.io/cluster/complex.example.com" = "owned" + "kubernetes.io/cluster/complex.example.com" = "" } } tag_specifications { @@ -607,24 +593,22 @@ resource "aws_launch_template" "nodes-complex-example-com" { "KubernetesCluster" = "complex.example.com" "Name" = "nodes.complex.example.com" "Owner" = "John Doe" - "aws-node-termination-handler/managed" = "" "foo/bar" = "fib+baz" "k8s.io/cluster-autoscaler/node-template/label/node-role.kubernetes.io/node" = "" "k8s.io/role/node" = "1" "kops.k8s.io/instancegroup" = "nodes" - "kubernetes.io/cluster/complex.example.com" = "owned" + "kubernetes.io/cluster/complex.example.com" = "" } } tags = { "KubernetesCluster" = "complex.example.com" "Name" = "nodes.complex.example.com" "Owner" = "John Doe" - "aws-node-termination-handler/managed" = "" "foo/bar" = "fib+baz" "k8s.io/cluster-autoscaler/node-template/label/node-role.kubernetes.io/node" = "" "k8s.io/role/node" = "1" "kops.k8s.io/instancegroup" = "nodes" - "kubernetes.io/cluster/complex.example.com" = "owned" + "kubernetes.io/cluster/complex.example.com" = "" } user_data = filebase64("${path.module}/data/aws_launch_template_nodes.complex.example.com_user_data") } From 9eb3eabc06ab75093714485d68c81f6c1309fe6a Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Mon, 16 Dec 2024 21:50:19 -0600 Subject: [PATCH 2/3] Only set NTH ManagedASGTag label if it doesn't already exist --- pkg/model/context.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/model/context.go b/pkg/model/context.go index 625e938d101d5..7e3e7ea1c520f 100644 --- a/pkg/model/context.go +++ b/pkg/model/context.go @@ -161,7 +161,10 @@ func (b *KopsModelContext) CloudTagsForInstanceGroup(ig *kops.InstanceGroup) (ma // Apply NTH Labels nth := b.Cluster.Spec.CloudProvider.AWS.NodeTerminationHandler if nth.IsQueueMode() { - labels[fi.ValueOf(nth.ManagedASGTag)] = "" + k := fi.ValueOf(nth.ManagedASGTag) + if _, ok := labels[k]; !ok && k != "" { + labels[k] = "" + } } } From 6a0c41664ebe50a2e8ec34a9ac92733a865f2763 Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Mon, 16 Dec 2024 21:50:38 -0600 Subject: [PATCH 3/3] ./hack/update-expected.sh --- .../update_cluster/complex/kubernetes.tf | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/integration/update_cluster/complex/kubernetes.tf b/tests/integration/update_cluster/complex/kubernetes.tf index 6efe3a4b840ce..7303d9e77a528 100644 --- a/tests/integration/update_cluster/complex/kubernetes.tf +++ b/tests/integration/update_cluster/complex/kubernetes.tf @@ -178,7 +178,7 @@ resource "aws_autoscaling_group" "master-us-test-1a-masters-complex-example-com" tag { key = "kubernetes.io/cluster/complex.example.com" propagate_at_launch = true - value = "" + value = "owned" } target_group_arns = [aws_lb_target_group.tcp-complex-example-com-vpjolq.id, aws_lb_target_group.tls-complex-example-com-5nursn.id] vpc_zone_identifier = [aws_subnet.us-test-1a-complex-example-com.id] @@ -236,7 +236,7 @@ resource "aws_autoscaling_group" "nodes-complex-example-com" { tag { key = "kubernetes.io/cluster/complex.example.com" propagate_at_launch = true - value = "" + value = "owned" } vpc_zone_identifier = [aws_subnet.us-test-1a-complex-example-com.id] } @@ -490,7 +490,7 @@ resource "aws_launch_template" "master-us-test-1a-masters-complex-example-com" { "k8s.io/role/control-plane" = "1" "k8s.io/role/master" = "1" "kops.k8s.io/instancegroup" = "master-us-test-1a" - "kubernetes.io/cluster/complex.example.com" = "" + "kubernetes.io/cluster/complex.example.com" = "owned" } } tag_specifications { @@ -506,7 +506,7 @@ resource "aws_launch_template" "master-us-test-1a-masters-complex-example-com" { "k8s.io/role/control-plane" = "1" "k8s.io/role/master" = "1" "kops.k8s.io/instancegroup" = "master-us-test-1a" - "kubernetes.io/cluster/complex.example.com" = "" + "kubernetes.io/cluster/complex.example.com" = "owned" } } tags = { @@ -520,7 +520,7 @@ resource "aws_launch_template" "master-us-test-1a-masters-complex-example-com" { "k8s.io/role/control-plane" = "1" "k8s.io/role/master" = "1" "kops.k8s.io/instancegroup" = "master-us-test-1a" - "kubernetes.io/cluster/complex.example.com" = "" + "kubernetes.io/cluster/complex.example.com" = "owned" } user_data = filebase64("${path.module}/data/aws_launch_template_master-us-test-1a.masters.complex.example.com_user_data") } @@ -584,7 +584,7 @@ resource "aws_launch_template" "nodes-complex-example-com" { "k8s.io/cluster-autoscaler/node-template/label/node-role.kubernetes.io/node" = "" "k8s.io/role/node" = "1" "kops.k8s.io/instancegroup" = "nodes" - "kubernetes.io/cluster/complex.example.com" = "" + "kubernetes.io/cluster/complex.example.com" = "owned" } } tag_specifications { @@ -597,7 +597,7 @@ resource "aws_launch_template" "nodes-complex-example-com" { "k8s.io/cluster-autoscaler/node-template/label/node-role.kubernetes.io/node" = "" "k8s.io/role/node" = "1" "kops.k8s.io/instancegroup" = "nodes" - "kubernetes.io/cluster/complex.example.com" = "" + "kubernetes.io/cluster/complex.example.com" = "owned" } } tags = { @@ -608,7 +608,7 @@ resource "aws_launch_template" "nodes-complex-example-com" { "k8s.io/cluster-autoscaler/node-template/label/node-role.kubernetes.io/node" = "" "k8s.io/role/node" = "1" "kops.k8s.io/instancegroup" = "nodes" - "kubernetes.io/cluster/complex.example.com" = "" + "kubernetes.io/cluster/complex.example.com" = "owned" } user_data = filebase64("${path.module}/data/aws_launch_template_nodes.complex.example.com_user_data") }