diff --git a/controllers/helpers.go b/controllers/helpers.go index 7dc64cd5..3b8a09b6 100644 --- a/controllers/helpers.go +++ b/controllers/helpers.go @@ -24,8 +24,8 @@ func getMaxUnavailable(strategy upgrademgrv1alpha1.UpdateStrategy, totalNodes in if strategy.MaxUnavailable.Type == 0 { maxUnavailable = int(strategy.MaxUnavailable.IntVal) } else if strategy.MaxUnavailable.Type == 1 { - strVallue := strategy.MaxUnavailable.StrVal - intValue, _ := strconv.Atoi(strings.Trim(strVallue, "%")) + strValue := strategy.MaxUnavailable.StrVal + intValue, _ := strconv.Atoi(strings.Trim(strValue, "%")) maxUnavailable = int(float32(intValue) / float32(100) * float32(totalNodes)) } // setting maxUnavailable to total number of nodes when maxUnavailable is greater than total node count @@ -34,7 +34,7 @@ func getMaxUnavailable(strategy upgrademgrv1alpha1.UpdateStrategy, totalNodes in maxUnavailable, totalNodes, totalNodes) maxUnavailable = totalNodes } - // maxUnavailable has to be atleast 1 when there are nodes in the ASG + // maxUnavailable has to be at least 1 when there are nodes in the ASG if totalNodes > 0 && maxUnavailable < 1 { maxUnavailable = 1 } @@ -43,10 +43,8 @@ func getMaxUnavailable(strategy upgrademgrv1alpha1.UpdateStrategy, totalNodes in func isNodeReady(node corev1.Node) bool { for _, condition := range node.Status.Conditions { - if condition.Type == corev1.NodeReady { - if condition.Status == corev1.ConditionTrue { - return true - } + if condition.Type == corev1.NodeReady && condition.Status == corev1.ConditionTrue { + return true } } return false @@ -72,14 +70,13 @@ func getInServiceIds(instances []*autoscaling.Instance) []string { return list } -func getGroupInstanceState(group *autoscaling.Group, id string) (string, error) { +func getGroupInstanceState(group *autoscaling.Group, instanceID string) (string, error) { for _, instance := range group.Instances { - if aws.StringValue(instance.InstanceId) != id { - continue + if aws.StringValue(instance.InstanceId) == instanceID { + return aws.StringValue(instance.LifecycleState), nil } - return aws.StringValue(instance.LifecycleState), nil } - return "", errors.Errorf("could not get instance group state, instance %v not found", id) + return "", errors.Errorf("could not get instance group state, instance %v not found", instanceID) } func isInServiceLifecycleState(state string) bool { @@ -97,10 +94,7 @@ func tagEC2instance(instanceID, tagKey, tagValue string, client ec2iface.EC2API) }, } _, err := client.CreateTags(input) - if err != nil { - return err - } - return nil + return err } func getTaggedInstances(tagKey, tagValue string, client ec2iface.EC2API) ([]string, error) { @@ -123,10 +117,7 @@ func getTaggedInstances(tagKey, tagValue string, client ec2iface.EC2API) ([]stri } return page.NextToken != nil }) - if err != nil { - return instances, err - } - return instances, nil + return instances, err } func contains(s []string, e string) bool { diff --git a/controllers/helpers_test.go b/controllers/helpers_test.go index 3648279b..a8f828b9 100644 --- a/controllers/helpers_test.go +++ b/controllers/helpers_test.go @@ -82,19 +82,19 @@ func TestGetInServiceCount(t *testing.T) { func TestGetInServiceIds(t *testing.T) { g := gomega.NewGomegaWithT(t) tt := map[*autoscaling.Instance][]string{ - &autoscaling.Instance{InstanceId: aws.String("i-1"), LifecycleState: aws.String(autoscaling.LifecycleStateInService)}: []string{"i-1"}, - &autoscaling.Instance{InstanceId: aws.String("i-2"), LifecycleState: aws.String(autoscaling.LifecycleStateDetached)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-3"), LifecycleState: aws.String(autoscaling.LifecycleStateDetaching)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-4"), LifecycleState: aws.String(autoscaling.LifecycleStateEnteringStandby)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-5"), LifecycleState: aws.String(autoscaling.LifecycleStatePending)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-6"), LifecycleState: aws.String(autoscaling.LifecycleStatePendingProceed)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-7"), LifecycleState: aws.String(autoscaling.LifecycleStatePendingWait)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-8"), LifecycleState: aws.String(autoscaling.LifecycleStateQuarantined)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-9"), LifecycleState: aws.String(autoscaling.LifecycleStateStandby)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-10"), LifecycleState: aws.String(autoscaling.LifecycleStateTerminated)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-11"), LifecycleState: aws.String(autoscaling.LifecycleStateTerminating)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-12"), LifecycleState: aws.String(autoscaling.LifecycleStateTerminatingProceed)}: []string{}, - &autoscaling.Instance{InstanceId: aws.String("i-13"), LifecycleState: aws.String(autoscaling.LifecycleStateTerminatingWait)}: []string{}, + &autoscaling.Instance{InstanceId: aws.String("i-1"), LifecycleState: aws.String(autoscaling.LifecycleStateInService)}: {"i-1"}, + &autoscaling.Instance{InstanceId: aws.String("i-2"), LifecycleState: aws.String(autoscaling.LifecycleStateDetached)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-3"), LifecycleState: aws.String(autoscaling.LifecycleStateDetaching)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-4"), LifecycleState: aws.String(autoscaling.LifecycleStateEnteringStandby)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-5"), LifecycleState: aws.String(autoscaling.LifecycleStatePending)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-6"), LifecycleState: aws.String(autoscaling.LifecycleStatePendingProceed)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-7"), LifecycleState: aws.String(autoscaling.LifecycleStatePendingWait)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-8"), LifecycleState: aws.String(autoscaling.LifecycleStateQuarantined)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-9"), LifecycleState: aws.String(autoscaling.LifecycleStateStandby)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-10"), LifecycleState: aws.String(autoscaling.LifecycleStateTerminated)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-11"), LifecycleState: aws.String(autoscaling.LifecycleStateTerminating)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-12"), LifecycleState: aws.String(autoscaling.LifecycleStateTerminatingProceed)}: {}, + &autoscaling.Instance{InstanceId: aws.String("i-13"), LifecycleState: aws.String(autoscaling.LifecycleStateTerminatingWait)}: {}, } // test every condition diff --git a/controllers/launch_definition.go b/controllers/launch_definition.go index f68e5b99..4fb902be 100644 --- a/controllers/launch_definition.go +++ b/controllers/launch_definition.go @@ -20,9 +20,8 @@ func NewLaunchDefinition(asg *autoscaling.Group) *launchDefinition { if template == nil && asg.MixedInstancesPolicy != nil { template = asg.MixedInstancesPolicy.LaunchTemplate.LaunchTemplateSpecification } - l := &launchDefinition{ + return &launchDefinition{ launchConfigurationName: asg.LaunchConfigurationName, launchTemplate: template, } - return l } diff --git a/controllers/node_selector.go b/controllers/node_selector.go index 8e4b93b5..8d2cd241 100644 --- a/controllers/node_selector.go +++ b/controllers/node_selector.go @@ -9,10 +9,7 @@ type NodeSelector interface { SelectNodesForRestack(state ClusterState) []*autoscaling.Instance } -func getNodeSelector( - asg *autoscaling.Group, - ruObj *upgrademgrv1alpha1.RollingUpgrade, -) NodeSelector { +func getNodeSelector(asg *autoscaling.Group, ruObj *upgrademgrv1alpha1.RollingUpgrade) NodeSelector { switch ruObj.Spec.Strategy.Type { case upgrademgrv1alpha1.UniformAcrossAzUpdateStrategy: return NewUniformAcrossAzNodeSelector(asg, ruObj) diff --git a/controllers/random_node_selector.go b/controllers/random_node_selector.go index e89cd7f6..0916de45 100644 --- a/controllers/random_node_selector.go +++ b/controllers/random_node_selector.go @@ -12,10 +12,7 @@ type RandomNodeSelector struct { asg *autoscaling.Group } -func NewRandomNodeSelector( - asg *autoscaling.Group, - ruObj *upgrademgrv1alpha1.RollingUpgrade, -) *RandomNodeSelector { +func NewRandomNodeSelector(asg *autoscaling.Group, ruObj *upgrademgrv1alpha1.RollingUpgrade) *RandomNodeSelector { maxUnavailable := getMaxUnavailable(ruObj.Spec.Strategy, len(asg.Instances)) log.Printf("Max unavailable calculated for %s is %d", ruObj.Name, maxUnavailable) return &RandomNodeSelector{ @@ -25,9 +22,6 @@ func NewRandomNodeSelector( } } -func (selector *RandomNodeSelector) SelectNodesForRestack( - state ClusterState, -) []*autoscaling.Instance { - return getNextAvailableInstances(selector.ruObj.Spec.AsgName, - selector.maxUnavailable, selector.asg.Instances, state) +func (selector *RandomNodeSelector) SelectNodesForRestack(state ClusterState) []*autoscaling.Instance { + return getNextAvailableInstances(selector.ruObj.Spec.AsgName, selector.maxUnavailable, selector.asg.Instances, state) } diff --git a/controllers/rollingupgrade_controller.go b/controllers/rollingupgrade_controller.go index c5fbcefd..689a31e1 100644 --- a/controllers/rollingupgrade_controller.go +++ b/controllers/rollingupgrade_controller.go @@ -1024,7 +1024,6 @@ func (r *RollingUpgradeReconciler) UpdateInstanceEager( // Drain and wait for draining node. r.DrainTerminate(ruObj, nodeName, targetInstanceID, KubeCtlCall, ch) - } func (r *RollingUpgradeReconciler) DrainTerminate( @@ -1049,7 +1048,6 @@ func (r *RollingUpgradeReconciler) DrainTerminate( ch <- err return } - } // UpdateInstance runs the rolling upgrade on one instance from an autoscaling group diff --git a/controllers/rollingupgrade_controller_test.go b/controllers/rollingupgrade_controller_test.go index db6f6822..74ea7d79 100644 --- a/controllers/rollingupgrade_controller_test.go +++ b/controllers/rollingupgrade_controller_test.go @@ -900,7 +900,7 @@ func TestFinishExecutionCompleted(t *testing.T) { g.Expect(ruObj.Status.TotalProcessingTime).To(gomega.Not(gomega.BeNil())) g.Expect(ruObj.Status.Conditions).To(gomega.Equal( []upgrademgrv1alpha1.RollingUpgradeCondition{ - upgrademgrv1alpha1.RollingUpgradeCondition{ + { Type: upgrademgrv1alpha1.UpgradeComplete, Status: corev1.ConditionTrue, }, @@ -937,7 +937,7 @@ func TestFinishExecutionError(t *testing.T) { g.Expect(ruObj.Status.TotalProcessingTime).To(gomega.Not(gomega.BeNil())) g.Expect(ruObj.Status.Conditions).To(gomega.Equal( []upgrademgrv1alpha1.RollingUpgradeCondition{ - upgrademgrv1alpha1.RollingUpgradeCondition{ + { Type: upgrademgrv1alpha1.UpgradeComplete, Status: corev1.ConditionTrue, }, @@ -2229,7 +2229,7 @@ func TestValidateruObjStrategyAfterSettingDefaultsWithOnlyMaxUnavailable(t *test rcRollingUpgrade.setDefaultsForRollingUpdateStrategy(ruObj) error := rcRollingUpgrade.validateRollingUpgradeObj(ruObj) - g.Expect(error).To((gomega.BeNil())) + g.Expect(error).To(gomega.BeNil()) } func TestRunRestackNoNodeInAsg(t *testing.T) { diff --git a/controllers/uniform_across_az_node_selector.go b/controllers/uniform_across_az_node_selector.go index 6b8c2b54..bf2640a1 100644 --- a/controllers/uniform_across_az_node_selector.go +++ b/controllers/uniform_across_az_node_selector.go @@ -17,10 +17,7 @@ type UniformAcrossAzNodeSelector struct { asg *autoscaling.Group } -func NewUniformAcrossAzNodeSelector( - asg *autoscaling.Group, - ruObj *upgrademgrv1alpha1.RollingUpgrade, -) *UniformAcrossAzNodeSelector { +func NewUniformAcrossAzNodeSelector(asg *autoscaling.Group, ruObj *upgrademgrv1alpha1.RollingUpgrade) *UniformAcrossAzNodeSelector { // find total number of nodes in each AZ azNodeCounts := make(map[string]*azNodesCountState) @@ -45,9 +42,7 @@ func NewUniformAcrossAzNodeSelector( } } -func (selector *UniformAcrossAzNodeSelector) SelectNodesForRestack( - state ClusterState, -) []*autoscaling.Instance { +func (selector *UniformAcrossAzNodeSelector) SelectNodesForRestack(state ClusterState) []*autoscaling.Instance { var instances []*autoscaling.Instance // Fetch instances to update from each instance group diff --git a/main.go b/main.go index 4c07604c..883217d2 100644 --- a/main.go +++ b/main.go @@ -49,10 +49,10 @@ var ( ) var ( - CacheDefaultTTL time.Duration = time.Second * 0 - DescribeAutoScalingGroupsTTL time.Duration = 60 * time.Second - CacheMaxItems int64 = 5000 - CacheItemsToPrune uint32 = 500 + CacheDefaultTTL = time.Second * 0 + DescribeAutoScalingGroupsTTL = 60 * time.Second + CacheMaxItems int64 = 5000 + CacheItemsToPrune uint32 = 500 ) var DefaultRetryer = client.DefaultRetryer{