Pagination & Launch config deletion (#115)

* fix deletions & add pagination

* Update cloud.go

* Update delete.go

* fix test

* tidy

* Update update.go

* Update update.go

* get input inside create launch config

controllers/common/utils.go

@@ -90,5 +90,5 @@ func ReadFile(path string) ([]byte, error) {
 
 func GetTimeString() string {
 	n := time.Now().UTC()
-	return fmt.Sprintf("%v%v%v%v%v%v", n.Year(), int(n.Month()), n.Day(), n.Hour(), n.Minute(), n.Second())
+	return n.Format("20060102150405")
 }

controllers/providers/aws/aws.go

@@ -494,12 +494,16 @@ func (w *AwsWorker) DescribeAutoscalingGroups() ([]*autoscaling.Group, error) {
 	return scalingGroups, nil
 }
 
-func (w *AwsWorker) DescribeAutoscalingLaunchConfigs() (autoscaling.DescribeLaunchConfigurationsOutput, error) {
-	out, err := w.AsgClient.DescribeLaunchConfigurations(&autoscaling.DescribeLaunchConfigurationsInput{})
+func (w *AwsWorker) DescribeAutoscalingLaunchConfigs() ([]*autoscaling.LaunchConfiguration, error) {
+	launchConfigurations := []*autoscaling.LaunchConfiguration{}
+	err := w.AsgClient.DescribeLaunchConfigurationsPages(&autoscaling.DescribeLaunchConfigurationsInput{}, func(page *autoscaling.DescribeLaunchConfigurationsOutput, lastPage bool) bool {
+		launchConfigurations = append(launchConfigurations, page.LaunchConfigurations...)
+		return page.NextToken != nil
+	})
 	if err != nil {
-		return autoscaling.DescribeLaunchConfigurationsOutput{}, err
+		return launchConfigurations, err
 	}
-	return *out, nil
+	return launchConfigurations, nil
 }
 
 func (w *AwsWorker) GetAutoscalingLaunchConfig(name string) (*autoscaling.LaunchConfiguration, error) {

controllers/provisioners/eks/cloud.go

@@ -17,6 +17,7 @@ package eks
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/keikoproj/instance-manager/api/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
@@ -27,6 +28,18 @@ import (
 	"github.com/pkg/errors"
 )
 
+type DiscoveredState struct {
+	Provisioned                   bool
+	NodesReady                    bool
+	OwnedScalingGroups            []*autoscaling.Group
+	ScalingGroup                  *autoscaling.Group
+	LaunchConfigurations          []*autoscaling.LaunchConfiguration
+	LaunchConfiguration           *autoscaling.LaunchConfiguration
+	ActiveLaunchConfigurationName string
+	IAMRole                       *iam.Role
+	InstanceProfile               *iam.InstanceProfile
+}
+
 func (ctx *EksInstanceGroupContext) CloudDiscovery() error {
 	var (
 		state         = ctx.GetDiscoveredState()
@@ -63,16 +76,22 @@ func (ctx *EksInstanceGroupContext) CloudDiscovery() error {
 	// find all owned scaling groups
 	ownedScalingGroups := ctx.findOwnedScalingGroups(scalingGroups)
 	state.SetOwnedScalingGroups(ownedScalingGroups)
-
 	// cache the scaling group we are reconciling for if it exists
 	targetScalingGroup := ctx.findTargetScalingGroup(ownedScalingGroups)
 
 	// if there is no scaling group found, it's deprovisioned
 	if targetScalingGroup == nil {
 		state.SetProvisioned(false)
-		// no need to look for launch configurations at this point
 		return nil
 	}
+
+	launchConfigurations, err := ctx.AwsWorker.DescribeAutoscalingLaunchConfigs()
+	if err != nil {
+		return errors.Wrap(err, "failed to describe autoscaling groups")
+	}
+
+	ctx.DiscoveredState.SetLaunchConfigurations(launchConfigurations)
+
 	state.SetProvisioned(true)
 	state.SetScalingGroup(targetScalingGroup)
 
@@ -87,22 +106,33 @@ func (ctx *EksInstanceGroupContext) CloudDiscovery() error {
 	}
 
 	// cache the launch configuration we are reconciling for if it exists
-	launchConfigName := aws.StringValue(targetScalingGroup.LaunchConfigurationName)
-	if launchConfigName != "" {
-		targetLaunchConfig, err := ctx.AwsWorker.GetAutoscalingLaunchConfig(launchConfigName)
-		if err != nil {
-			return errors.Wrap(err, "failed to describe autoscaling launch configurations")
-		}
-
-		if targetLaunchConfig == nil {
-			return nil
+	for _, lc := range launchConfigurations {
+		lcName := aws.StringValue(lc.LaunchConfigurationName)
+		if aws.StringValue(lc.LaunchConfigurationName) == aws.StringValue(targetScalingGroup.LaunchConfigurationName) {
+			state.SetLaunchConfiguration(lc)
+			state.SetActiveLaunchConfigurationName(lcName)
+			status.SetActiveLaunchConfigurationName(lcName)
 		}
+	}
 
-		var lcName = aws.StringValue(targetLaunchConfig.LaunchConfigurationName)
+	// delete old launch configurations
+	sortedConfigs := ctx.GetTimeSortedLaunchConfigurations()
+	var deletable []*autoscaling.LaunchConfiguration
+	if len(sortedConfigs) > defaultLaunchConfigurationRetention {
+		d := len(sortedConfigs) - defaultLaunchConfigurationRetention
+		deletable = sortedConfigs[:d]
+	}
 
-		state.SetLaunchConfiguration(targetLaunchConfig)
-		state.SetActiveLaunchConfigurationName(lcName)
-		status.SetActiveLaunchConfigurationName(lcName)
+	for _, d := range deletable {
+		name := aws.StringValue(d.LaunchConfigurationName)
+		if strings.EqualFold(name, state.GetActiveLaunchConfigurationName()) {
+			// never try to delete the active launch config
+			continue
+		}
+		ctx.Log.Info("deleting old launch configuration", "instancegroup", instanceGroup.GetName(), "name", name)
+		if err := ctx.AwsWorker.DeleteLaunchConfig(name); err != nil {
+			ctx.Log.Error(err, "failed to delete launch configuration", "instancegroup", instanceGroup.GetName(), "name", name)
+		}
 	}
 
 	if status.GetNodesReadyCondition() == corev1.ConditionTrue {
@@ -119,17 +149,6 @@ func (ctx *EksInstanceGroupContext) CloudDiscovery() error {
 	return nil
 }
 
-type DiscoveredState struct {
-	Provisioned                   bool
-	NodesReady                    bool
-	OwnedScalingGroups            []*autoscaling.Group
-	ScalingGroup                  *autoscaling.Group
-	LaunchConfiguration           *autoscaling.LaunchConfiguration
-	ActiveLaunchConfigurationName string
-	IAMRole                       *iam.Role
-	InstanceProfile               *iam.InstanceProfile
-}
-
 func (d *DiscoveredState) SetScalingGroup(asg *autoscaling.Group) {
 	if asg != nil {
 		d.ScalingGroup = asg
@@ -155,6 +174,12 @@ func (d *DiscoveredState) SetLaunchConfiguration(lc *autoscaling.LaunchConfigura
 func (d *DiscoveredState) GetLaunchConfiguration() *autoscaling.LaunchConfiguration {
 	return d.LaunchConfiguration
 }
+func (d *DiscoveredState) GetLaunchConfigurations() []*autoscaling.LaunchConfiguration {
+	return d.LaunchConfigurations
+}
+func (d *DiscoveredState) SetLaunchConfigurations(configs []*autoscaling.LaunchConfiguration) {
+	d.LaunchConfigurations = configs
+}
 func (d *DiscoveredState) SetActiveLaunchConfigurationName(name string) {
 	d.ActiveLaunchConfigurationName = name
 }

controllers/provisioners/eks/cloud_test.go

@@ -16,6 +16,7 @@ limitations under the License.
 package eks
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
@@ -209,3 +210,67 @@ func TestCloudDiscoverySpotPrice(t *testing.T) {
 	g.Expect(err).NotTo(gomega.HaveOccurred())
 	g.Expect(configuration.GetSpotPrice()).To(gomega.BeEmpty())
 }
+
+func TestLaunchConfigDeletion(t *testing.T) {
+	var (
+		g       = gomega.NewGomegaWithT(t)
+		k       = MockKubernetesClientSet()
+		ig      = MockInstanceGroup()
+		asgMock = NewAutoScalingMocker()
+		iamMock = NewIamMocker()
+	)
+
+	w := MockAwsWorker(asgMock, iamMock)
+	ctx := MockContext(ig, k, w)
+	configuration := ig.GetEKSConfiguration()
+
+	iamMock.Role = &iam.Role{
+		RoleName: aws.String("some-role"),
+		Arn:      aws.String("some-arn"),
+	}
+
+	iamMock.InstanceProfile = &iam.InstanceProfile{
+		InstanceProfileName: aws.String("some-profile"),
+	}
+
+	var (
+		clusterName           = "some-cluster"
+		resourceName          = "some-instance-group"
+		resourceNamespace     = "default"
+		ownedScalingGroupName = "scaling-group-1"
+		ownershipTag          = MockTagDescription(provisioners.TagClusterName, clusterName)
+		nameTag               = MockTagDescription(provisioners.TagInstanceGroupName, resourceName)
+		namespaceTag          = MockTagDescription(provisioners.TagInstanceGroupNamespace, resourceNamespace)
+	)
+
+	ig.SetName(resourceName)
+	ig.SetNamespace(resourceNamespace)
+	configuration.SetClusterName(clusterName)
+
+	asgMock.AutoScalingGroups = []*autoscaling.Group{
+		MockScalingGroup(ownedScalingGroupName, ownershipTag, nameTag, namespaceTag),
+	}
+
+	asgMock.LaunchConfigurations = []*autoscaling.LaunchConfiguration{
+		{
+			LaunchConfigurationName: aws.String(fmt.Sprintf("%v-123456", ctx.ResourcePrefix)),
+			CreatedTime:             aws.Time(time.Now()),
+		},
+		{
+			LaunchConfigurationName: aws.String(fmt.Sprintf("%v-123457", ctx.ResourcePrefix)),
+			CreatedTime:             aws.Time(time.Now().Add(time.Duration(-1) * time.Minute)),
+		},
+		{
+			LaunchConfigurationName: aws.String(fmt.Sprintf("%v-123458", ctx.ResourcePrefix)),
+			CreatedTime:             aws.Time(time.Now().Add(time.Duration(-3) * time.Minute)),
+		},
+		{
+			LaunchConfigurationName: aws.String(fmt.Sprintf("%v-123459", ctx.ResourcePrefix)),
+			CreatedTime:             aws.Time(time.Now().Add(time.Duration(-5) * time.Minute)),
+		},
+	}
+
+	err := ctx.CloudDiscovery()
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+	g.Expect(asgMock.DeleteLaunchConfigurationCallCount).To(gomega.Equal(2))
+}

controllers/provisioners/eks/create.go

@@ -42,7 +42,8 @@ func (ctx *EksInstanceGroupContext) Create() error {
 
 	// create launchconfig
 	if !state.HasLaunchConfiguration() {
-		err := ctx.CreateLaunchConfiguration()
+		lcName := fmt.Sprintf("%v-%v", ctx.ResourcePrefix, common.GetTimeString())
+		err := ctx.CreateLaunchConfiguration(lcName)
 		if err != nil {
 			return errors.Wrap(err, "failed to create launch configuration")
 		}
@@ -63,9 +64,8 @@ func (ctx *EksInstanceGroupContext) CreateScalingGroup() error {
 		instanceGroup = ctx.GetInstanceGroup()
 		spec          = instanceGroup.GetEKSSpec()
 		configuration = instanceGroup.GetEKSConfiguration()
-		clusterName   = configuration.GetClusterName()
 		state         = ctx.GetDiscoveredState()
-		asgName       = fmt.Sprintf("%v-%v-%v", clusterName, instanceGroup.GetNamespace(), instanceGroup.GetName())
+		asgName       = ctx.ResourcePrefix
 		tags          = ctx.GetAddedTags(asgName)
 	)
 
@@ -99,35 +99,30 @@ func (ctx *EksInstanceGroupContext) CreateScalingGroup() error {
 	return nil
 }
 
-func (ctx *EksInstanceGroupContext) CreateLaunchConfiguration() error {
+func (ctx *EksInstanceGroupContext) CreateLaunchConfiguration(name string) error {
 	var (
 		state         = ctx.GetDiscoveredState()
 		instanceGroup = ctx.GetInstanceGroup()
-		configuration = instanceGroup.GetEKSConfiguration()
 		status        = instanceGroup.GetStatus()
-		clusterName   = configuration.GetClusterName()
-		lcName        = fmt.Sprintf("%v-%v-%v-%v", clusterName, instanceGroup.GetNamespace(), instanceGroup.GetName(), common.GetTimeString())
-		lcInput       = ctx.GetLaunchConfigurationInput(lcName)
+		input         = ctx.GetLaunchConfigurationInput(name)
 	)
 
-	if aws.StringValue(lcInput.IamInstanceProfile) == "" {
+	if aws.StringValue(input.IamInstanceProfile) == "" {
 		return errors.Errorf("cannot create a launchconfiguration without iam instance profile")
 	}
 
-	err := ctx.AwsWorker.CreateLaunchConfig(lcInput)
+	err := ctx.AwsWorker.CreateLaunchConfig(input)
 	if err != nil {
 		return err
 	}
 
-	ctx.Log.Info("created launchconfig", "instancegroup", instanceGroup.GetName(), "launchconfig", lcName)
-
-	lc, err := ctx.AwsWorker.GetAutoscalingLaunchConfig(lcName)
+	ctx.Log.Info("created launchconfig", "instancegroup", instanceGroup.GetName(), "launchconfig", name)
+	lc, err := ctx.AwsWorker.GetAutoscalingLaunchConfig(name)
 	if err != nil {
 		return err
 	}
 
 	if lc != nil {
-		name := aws.StringValue(lc.LaunchConfigurationName)
 		status.SetActiveLaunchConfigurationName(name)
 		state.SetActiveLaunchConfigurationName(name)
 		state.SetLaunchConfiguration(lc)
@@ -141,9 +136,8 @@ func (ctx *EksInstanceGroupContext) CreateManagedRole() error {
 		instanceGroup      = ctx.GetInstanceGroup()
 		state              = ctx.GetDiscoveredState()
 		configuration      = instanceGroup.GetEKSConfiguration()
-		clusterName        = configuration.GetClusterName()
 		additionalPolicies = configuration.GetManagedPolicies()
-		roleName           = fmt.Sprintf("%v-%v-%v", clusterName, instanceGroup.GetNamespace(), instanceGroup.GetName())
+		roleName           = ctx.ResourcePrefix
 	)
 
 	if configuration.HasExistingRole() {

controllers/provisioners/eks/delete.go

@@ -16,6 +16,8 @@ limitations under the License.
 package eks
 
 import (
+	"strings"
+
 	"github.com/keikoproj/instance-manager/api/v1alpha1"
 	"github.com/pkg/errors"
 
@@ -81,18 +83,18 @@ func (ctx *EksInstanceGroupContext) DeleteLaunchConfiguration() error {
 	var (
 		state         = ctx.GetDiscoveredState()
 		instanceGroup = ctx.GetInstanceGroup()
-		lcName        = state.GetActiveLaunchConfigurationName()
 	)
 
-	if !state.HasLaunchConfiguration() {
-		return nil
-	}
-
-	err := ctx.AwsWorker.DeleteLaunchConfig(lcName)
-	if err != nil {
-		return err
+	for _, lc := range state.GetLaunchConfigurations() {
+		name := aws.StringValue(lc.LaunchConfigurationName)
+		if strings.HasPrefix(name, ctx.ResourcePrefix) {
+			err := ctx.AwsWorker.DeleteLaunchConfig(name)
+			if err != nil {
+				return err
+			}
+			ctx.Log.Info("deleted launch config", "instancegroup", instanceGroup.GetName(), "launchconfig", name)
+		}
 	}
-	ctx.Log.Info("deleted launch config", "instancegroup", instanceGroup.GetName(), "launchconfig", lcName)
 	return nil
 }
 

controllers/provisioners/eks/delete_test.go

@@ -16,6 +16,7 @@ limitations under the License.
 package eks
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -95,14 +96,17 @@ func TestDeleteLaunchConfigurationNegative(t *testing.T) {
 	ctx := MockContext(ig, k, w)
 
 	ctx.SetDiscoveredState(&DiscoveredState{
-		LaunchConfiguration: &autoscaling.LaunchConfiguration{},
+		LaunchConfigurations: []*autoscaling.LaunchConfiguration{
+			{
+				LaunchConfigurationName: aws.String(fmt.Sprintf("%v-123456", ctx.ResourcePrefix)),
+			},
+		},
 	})
 
 	asgMock.DeleteLaunchConfigurationErr = errors.New("some-error")
 	err := ctx.Delete()
 	g.Expect(err).To(gomega.HaveOccurred())
 	g.Expect(ctx.GetState()).To(gomega.Equal(v1alpha1.ReconcileDeleting))
-	asgMock.DeleteLaunchConfigurationErr = nil
 }
 
 func TestDeleteAutoScalingGroupNegative(t *testing.T) {