Fix few typos and simplify error returns, remove redundant types (#131)

Signed-off-by: Oleg Atamanenko <[email protected]>

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 {

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

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
 }

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)

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)
 }

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

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) {

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

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{