Skip to content

Commit 25db80a

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3264 from helio/fix-asgNeedsUpdates
fix: asgNeedsUpdates invalid condition (pointers)
2 parents 5d4cd12 + 831193e commit 25db80a

File tree

2 files changed

+217
-1
lines changed

2 files changed

+217
-1
lines changed

‎exp/controllers/awsmachinepool_controller.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,11 @@ func (r *AWSMachinePoolReconciler) reconcileTags(machinePoolScope *scope.Machine
510510

511511
// asgNeedsUpdates compares incoming AWSMachinePool and compares against existing ASG.
512512
func asgNeedsUpdates(machinePoolScope *scope.MachinePoolScope, existingASG *expinfrav1.AutoScalingGroup) bool {
513-
if machinePoolScope.MachinePool.Spec.Replicas != nil && machinePoolScope.MachinePool.Spec.Replicas != existingASG.DesiredCapacity {
513+
if machinePoolScope.MachinePool.Spec.Replicas != nil {
514+
if existingASG.DesiredCapacity == nil || *machinePoolScope.MachinePool.Spec.Replicas != *existingASG.DesiredCapacity {
515+
return true
516+
}
517+
} else if existingASG.DesiredCapacity != nil {
514518
return true
515519
}
516520

‎exp/controllers/awsmachinepool_controller_test.go

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"fmt"
2424
"testing"
2525

26+
"github.com/go-logr/logr"
2627
"github.com/golang/mock/gomock"
2728
. "github.com/onsi/gomega"
2829
"github.com/pkg/errors"
@@ -363,3 +364,214 @@ func setupCluster(clusterName string) (*scope.ClusterScope, error) {
363364
Client: client,
364365
})
365366
}
367+
368+
func Test_asgNeedsUpdates(t *testing.T) {
369+
type args struct {
370+
machinePoolScope *scope.MachinePoolScope
371+
existingASG *expinfrav1.AutoScalingGroup
372+
}
373+
tests := []struct {
374+
name string
375+
args args
376+
want bool
377+
}{
378+
{
379+
name: "replicas != asg.desiredCapacity",
380+
args: args{
381+
machinePoolScope: &scope.MachinePoolScope{
382+
MachinePool: &expclusterv1.MachinePool{
383+
Spec: expclusterv1.MachinePoolSpec{
384+
Replicas: pointer.Int32(0),
385+
},
386+
},
387+
},
388+
existingASG: &expinfrav1.AutoScalingGroup{
389+
DesiredCapacity: pointer.Int32(1),
390+
},
391+
},
392+
want: true,
393+
},
394+
{
395+
name: "replicas (nil) != asg.desiredCapacity",
396+
args: args{
397+
machinePoolScope: &scope.MachinePoolScope{
398+
MachinePool: &expclusterv1.MachinePool{
399+
Spec: expclusterv1.MachinePoolSpec{
400+
Replicas: nil,
401+
},
402+
},
403+
},
404+
existingASG: &expinfrav1.AutoScalingGroup{
405+
DesiredCapacity: pointer.Int32(1),
406+
},
407+
},
408+
want: true,
409+
},
410+
{
411+
name: "replicas != asg.desiredCapacity (nil)",
412+
args: args{
413+
machinePoolScope: &scope.MachinePoolScope{
414+
MachinePool: &expclusterv1.MachinePool{
415+
Spec: expclusterv1.MachinePoolSpec{
416+
Replicas: pointer.Int32(0),
417+
},
418+
},
419+
},
420+
existingASG: &expinfrav1.AutoScalingGroup{
421+
DesiredCapacity: nil,
422+
},
423+
},
424+
want: true,
425+
},
426+
{
427+
name: "maxSize != asg.maxSize",
428+
args: args{
429+
machinePoolScope: &scope.MachinePoolScope{
430+
MachinePool: &expclusterv1.MachinePool{
431+
Spec: expclusterv1.MachinePoolSpec{
432+
Replicas: pointer.Int32(1),
433+
},
434+
},
435+
AWSMachinePool: &expinfrav1.AWSMachinePool{
436+
Spec: expinfrav1.AWSMachinePoolSpec{
437+
MaxSize: 1,
438+
},
439+
},
440+
},
441+
existingASG: &expinfrav1.AutoScalingGroup{
442+
DesiredCapacity: pointer.Int32(1),
443+
MaxSize: 2,
444+
},
445+
},
446+
want: true,
447+
},
448+
{
449+
name: "minSize != asg.minSize",
450+
args: args{
451+
machinePoolScope: &scope.MachinePoolScope{
452+
MachinePool: &expclusterv1.MachinePool{
453+
Spec: expclusterv1.MachinePoolSpec{
454+
Replicas: pointer.Int32(1),
455+
},
456+
},
457+
AWSMachinePool: &expinfrav1.AWSMachinePool{
458+
Spec: expinfrav1.AWSMachinePoolSpec{
459+
MaxSize: 2,
460+
MinSize: 0,
461+
},
462+
},
463+
},
464+
existingASG: &expinfrav1.AutoScalingGroup{
465+
DesiredCapacity: pointer.Int32(1),
466+
MaxSize: 2,
467+
MinSize: 1,
468+
},
469+
},
470+
want: true,
471+
},
472+
{
473+
name: "capacityRebalance != asg.capacityRebalance",
474+
args: args{
475+
machinePoolScope: &scope.MachinePoolScope{
476+
MachinePool: &expclusterv1.MachinePool{
477+
Spec: expclusterv1.MachinePoolSpec{
478+
Replicas: pointer.Int32(1),
479+
},
480+
},
481+
AWSMachinePool: &expinfrav1.AWSMachinePool{
482+
Spec: expinfrav1.AWSMachinePoolSpec{
483+
MaxSize: 2,
484+
MinSize: 0,
485+
CapacityRebalance: true,
486+
},
487+
},
488+
},
489+
existingASG: &expinfrav1.AutoScalingGroup{
490+
DesiredCapacity: pointer.Int32(1),
491+
MaxSize: 2,
492+
MinSize: 0,
493+
CapacityRebalance: false,
494+
},
495+
},
496+
want: true,
497+
},
498+
{
499+
name: "MixedInstancesPolicy != asg.MixedInstancesPolicy",
500+
args: args{
501+
machinePoolScope: &scope.MachinePoolScope{
502+
MachinePool: &expclusterv1.MachinePool{
503+
Spec: expclusterv1.MachinePoolSpec{
504+
Replicas: pointer.Int32(1),
505+
},
506+
},
507+
AWSMachinePool: &expinfrav1.AWSMachinePool{
508+
Spec: expinfrav1.AWSMachinePoolSpec{
509+
MaxSize: 2,
510+
MinSize: 0,
511+
CapacityRebalance: true,
512+
MixedInstancesPolicy: &expinfrav1.MixedInstancesPolicy{
513+
InstancesDistribution: &expinfrav1.InstancesDistribution{
514+
OnDemandAllocationStrategy: expinfrav1.OnDemandAllocationStrategyPrioritized,
515+
},
516+
Overrides: nil,
517+
},
518+
},
519+
},
520+
Logger: logr.Discard(),
521+
},
522+
existingASG: &expinfrav1.AutoScalingGroup{
523+
DesiredCapacity: pointer.Int32(1),
524+
MaxSize: 2,
525+
MinSize: 0,
526+
CapacityRebalance: true,
527+
MixedInstancesPolicy: &expinfrav1.MixedInstancesPolicy{},
528+
},
529+
},
530+
want: true,
531+
},
532+
{
533+
name: "all matches",
534+
args: args{
535+
machinePoolScope: &scope.MachinePoolScope{
536+
MachinePool: &expclusterv1.MachinePool{
537+
Spec: expclusterv1.MachinePoolSpec{
538+
Replicas: pointer.Int32(1),
539+
},
540+
},
541+
AWSMachinePool: &expinfrav1.AWSMachinePool{
542+
Spec: expinfrav1.AWSMachinePoolSpec{
543+
MaxSize: 2,
544+
MinSize: 0,
545+
CapacityRebalance: true,
546+
MixedInstancesPolicy: &expinfrav1.MixedInstancesPolicy{
547+
InstancesDistribution: &expinfrav1.InstancesDistribution{
548+
OnDemandAllocationStrategy: expinfrav1.OnDemandAllocationStrategyPrioritized,
549+
},
550+
Overrides: nil,
551+
},
552+
},
553+
},
554+
},
555+
existingASG: &expinfrav1.AutoScalingGroup{
556+
DesiredCapacity: pointer.Int32(1),
557+
MaxSize: 2,
558+
MinSize: 0,
559+
CapacityRebalance: true,
560+
MixedInstancesPolicy: &expinfrav1.MixedInstancesPolicy{
561+
InstancesDistribution: &expinfrav1.InstancesDistribution{
562+
OnDemandAllocationStrategy: expinfrav1.OnDemandAllocationStrategyPrioritized,
563+
},
564+
Overrides: nil,
565+
},
566+
},
567+
},
568+
want: false,
569+
},
570+
}
571+
for _, tt := range tests {
572+
t.Run(tt.name, func(t *testing.T) {
573+
g := NewWithT(t)
574+
g.Expect(asgNeedsUpdates(tt.args.machinePoolScope, tt.args.existingASG)).To(Equal(tt.want))
575+
})
576+
}
577+
}

0 commit comments

Comments
 (0)