From e6786c2ee0da168d5890682a743815bca0c323e8 Mon Sep 17 00:00:00 2001 From: Peter Kubicsek Date: Mon, 3 Nov 2025 19:10:43 +0100 Subject: [PATCH] Fix HasHighlyAvailableControlPlane to use AllInstanceGroups When using 'kops update cluster' with --instance-group or --instance-group-roles filters, the HasHighlyAvailableControlPlane function was incorrectly using the filtered InstanceGroups list instead of AllInstanceGroups. This caused cluster-wide controllers like aws-load-balancer-controller and node-termination-handler to incorrectly downscale replicas from 2 to 1 when updating a specific instance group in an HA cluster. The fix changes HasHighlyAvailableControlPlane to use AllInstanceGroups since HA status is a cluster-wide property, not specific to filtered instance groups. Signed-off-by: Peter Kubicsek --- upup/pkg/fi/cloudup/template_functions.go | 2 +- .../pkg/fi/cloudup/template_functions_test.go | 128 ++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/upup/pkg/fi/cloudup/template_functions.go b/upup/pkg/fi/cloudup/template_functions.go index fb7538319cf32..5bd6189cbf81d 100644 --- a/upup/pkg/fi/cloudup/template_functions.go +++ b/upup/pkg/fi/cloudup/template_functions.go @@ -499,7 +499,7 @@ func (tf *TemplateFunctions) APIServerNodeRole() string { // HasHighlyAvailableControlPlane returns true of the cluster has more than one control plane node. False otherwise. func (tf *TemplateFunctions) HasHighlyAvailableControlPlane() bool { cp := 0 - for _, ig := range tf.InstanceGroups { + for _, ig := range tf.AllInstanceGroups { if ig.Spec.Role == kops.InstanceGroupRoleControlPlane { cp++ if cp > 1 { diff --git a/upup/pkg/fi/cloudup/template_functions_test.go b/upup/pkg/fi/cloudup/template_functions_test.go index 0f9d5f8ae96d1..64dd0cd50f3af 100644 --- a/upup/pkg/fi/cloudup/template_functions_test.go +++ b/upup/pkg/fi/cloudup/template_functions_test.go @@ -21,6 +21,7 @@ import ( "reflect" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/featureflag" "k8s.io/kops/upup/pkg/fi" @@ -313,3 +314,130 @@ func TestKopsFeatureEnabled(t *testing.T) { }) } } + +func TestHasHighlyAvailableControlPlane(t *testing.T) { + tests := []struct { + name string + allInstanceGroups []*kops.InstanceGroup + instanceGroups []*kops.InstanceGroup + expectedHA bool + }{ + { + name: "Single control plane node", + allInstanceGroups: []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "nodes"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode}, + }, + }, + instanceGroups: []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{Name: "nodes"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode}, + }, + }, + expectedHA: false, + }, + { + name: "Multiple control plane nodes", + allInstanceGroups: []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1b"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "nodes"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode}, + }, + }, + instanceGroups: []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{Name: "nodes"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode}, + }, + }, + expectedHA: true, + }, + { + name: "Multiple control plane nodes with filtered instance groups (regression test)", + allInstanceGroups: []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1b"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1c"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "nodes"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode}, + }, + }, + instanceGroups: []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{Name: "nodes"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode}, + }, + }, + expectedHA: true, + }, + { + name: "Three control plane nodes", + allInstanceGroups: []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1b"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1c"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + }, + instanceGroups: []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1b"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1c"}, + Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane}, + }, + }, + expectedHA: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tf := &TemplateFunctions{} + tf.AllInstanceGroups = tc.allInstanceGroups + tf.InstanceGroups = tc.instanceGroups + + actual := tf.HasHighlyAvailableControlPlane() + if actual != tc.expectedHA { + t.Errorf("expected HA to be %t, got %t", tc.expectedHA, actual) + } + }) + } +}