Skip to content

Commit c3189b5

Browse files
committed
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. Fixes #17739 Signed-off-by: Peter Kubicsek <[email protected]>
1 parent 3440db0 commit c3189b5

File tree

2 files changed

+129
-1
lines changed

2 files changed

+129
-1
lines changed

upup/pkg/fi/cloudup/template_functions.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ func (tf *TemplateFunctions) APIServerNodeRole() string {
499499
// HasHighlyAvailableControlPlane returns true of the cluster has more than one control plane node. False otherwise.
500500
func (tf *TemplateFunctions) HasHighlyAvailableControlPlane() bool {
501501
cp := 0
502-
for _, ig := range tf.InstanceGroups {
502+
for _, ig := range tf.AllInstanceGroups {
503503
if ig.Spec.Role == kops.InstanceGroupRoleControlPlane {
504504
cp++
505505
if cp > 1 {

upup/pkg/fi/cloudup/template_functions_test.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"reflect"
2222
"testing"
2323

24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2425
"k8s.io/kops/pkg/apis/kops"
2526
"k8s.io/kops/pkg/featureflag"
2627
"k8s.io/kops/upup/pkg/fi"
@@ -313,3 +314,130 @@ func TestKopsFeatureEnabled(t *testing.T) {
313314
})
314315
}
315316
}
317+
318+
func TestHasHighlyAvailableControlPlane(t *testing.T) {
319+
tests := []struct {
320+
name string
321+
allInstanceGroups []*kops.InstanceGroup
322+
instanceGroups []*kops.InstanceGroup
323+
expectedHA bool
324+
}{
325+
{
326+
name: "Single control plane node",
327+
allInstanceGroups: []*kops.InstanceGroup{
328+
{
329+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"},
330+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
331+
},
332+
{
333+
ObjectMeta: metav1.ObjectMeta{Name: "nodes"},
334+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode},
335+
},
336+
},
337+
instanceGroups: []*kops.InstanceGroup{
338+
{
339+
ObjectMeta: metav1.ObjectMeta{Name: "nodes"},
340+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode},
341+
},
342+
},
343+
expectedHA: false,
344+
},
345+
{
346+
name: "Multiple control plane nodes",
347+
allInstanceGroups: []*kops.InstanceGroup{
348+
{
349+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"},
350+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
351+
},
352+
{
353+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1b"},
354+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
355+
},
356+
{
357+
ObjectMeta: metav1.ObjectMeta{Name: "nodes"},
358+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode},
359+
},
360+
},
361+
instanceGroups: []*kops.InstanceGroup{
362+
{
363+
ObjectMeta: metav1.ObjectMeta{Name: "nodes"},
364+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode},
365+
},
366+
},
367+
expectedHA: true,
368+
},
369+
{
370+
name: "Multiple control plane nodes with filtered instance groups (regression test)",
371+
allInstanceGroups: []*kops.InstanceGroup{
372+
{
373+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"},
374+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
375+
},
376+
{
377+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1b"},
378+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
379+
},
380+
{
381+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1c"},
382+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
383+
},
384+
{
385+
ObjectMeta: metav1.ObjectMeta{Name: "nodes"},
386+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode},
387+
},
388+
},
389+
instanceGroups: []*kops.InstanceGroup{
390+
{
391+
ObjectMeta: metav1.ObjectMeta{Name: "nodes"},
392+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleNode},
393+
},
394+
},
395+
expectedHA: true,
396+
},
397+
{
398+
name: "Three control plane nodes",
399+
allInstanceGroups: []*kops.InstanceGroup{
400+
{
401+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"},
402+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
403+
},
404+
{
405+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1b"},
406+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
407+
},
408+
{
409+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1c"},
410+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
411+
},
412+
},
413+
instanceGroups: []*kops.InstanceGroup{
414+
{
415+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1a"},
416+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
417+
},
418+
{
419+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1b"},
420+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
421+
},
422+
{
423+
ObjectMeta: metav1.ObjectMeta{Name: "master-us-east-1c"},
424+
Spec: kops.InstanceGroupSpec{Role: kops.InstanceGroupRoleControlPlane},
425+
},
426+
},
427+
expectedHA: true,
428+
},
429+
}
430+
431+
for _, tc := range tests {
432+
t.Run(tc.name, func(t *testing.T) {
433+
tf := &TemplateFunctions{}
434+
tf.AllInstanceGroups = tc.allInstanceGroups
435+
tf.InstanceGroups = tc.instanceGroups
436+
437+
actual := tf.HasHighlyAvailableControlPlane()
438+
if actual != tc.expectedHA {
439+
t.Errorf("expected HA to be %t, got %t", tc.expectedHA, actual)
440+
}
441+
})
442+
}
443+
}

0 commit comments

Comments
 (0)