Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 11 additions & 20 deletions controllers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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) {
Expand All @@ -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 {
Expand Down
26 changes: 13 additions & 13 deletions controllers/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions controllers/launch_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
5 changes: 1 addition & 4 deletions controllers/node_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 3 additions & 9 deletions controllers/random_node_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)
}
2 changes: 0 additions & 2 deletions controllers/rollingupgrade_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -1049,7 +1048,6 @@ func (r *RollingUpgradeReconciler) DrainTerminate(
ch <- err
return
}

}

// UpdateInstance runs the rolling upgrade on one instance from an autoscaling group
Expand Down
6 changes: 3 additions & 3 deletions controllers/rollingupgrade_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 2 additions & 7 deletions controllers/uniform_across_az_node_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down