Repository: keikoproj/instance-manager (go)

Issue:
Add configurable prefix to stack name
The current stack name is constructed as:
CLUSTERNAME-NAMESPACE-GROUPNAME

We should change it to:

<CONFIGURABLE_PREFIX>-CLUSTERNAME-GROUPNAME

- Configurable prefix can be added to the controller configmap as an option `stackNamePrefix`.
- Constructed name must be < 63 characters or operation should fail.

Pull request description:
Cloudformation Stack Prefixes
Fixes #45 

This PR adds support for `stackNamePrefix` in the controller global config.
Created stack name is persisted in `.Status.StackName` and then only read from there.
Move `LoadControllerConfig` inside the provisioner as it's quite specific for the ekscf provisioner and that seem to make more sense inside rather then in the controller.

Future PRs will follow for documentation improvements and improvements to current unit tests which are becoming a bit brittle.

Tested this feature out manually with various scenarios, including changing the prefix and restarting the controller, everything works as expected since the stack name is now persisted in the status and is the source of truth for stack name

Patch:
diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go
index c7c4251..bda59ca 100644
--- a/api/v1alpha1/instancegroup_types.go
+++ b/api/v1alpha1/instancegroup_types.go
@@ -177,6 +177,7 @@ type EKSCFConfiguration struct {
 
 // InstanceGroupStatus defines the schema of resource Status
 type InstanceGroupStatus struct {
+	StackName                     string `json:"stackName,omitempty"`
 	CurrentState                  string `json:"currentState,omitempty"`
 	CurrentMin                    int    `json:"currentMin,omitempty"`
 	CurrentMax                    int    `json:"currentMax,omitempty"`
@@ -256,6 +257,14 @@ func (status *InstanceGroupStatus) SetActiveLaunchConfigurationName(name string)
 	status.ActiveLaunchConfigurationName = name
 }
 
+func (status *InstanceGroupStatus) GetStackName() string {
+	return status.StackName
+}
+
+func (status *InstanceGroupStatus) SetStackName(name string) {
+	status.StackName = name
+}
+
 func (status *InstanceGroupStatus) GetActiveScalingGroupName() string {
 	return status.ActiveScalingGroupName
 }
diff --git a/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml b/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
index 2b9dec3..8e11ae3 100644
--- a/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
+++ b/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
@@ -533,6 +533,8 @@ spec:
               type: string
             nodesInstanceRoleArn:
               type: string
+            stackName:
+              type: string
             strategyResourceName:
               type: string
             usingSpotRecommendation:
diff --git a/controllers/instancegroup_controller.go b/controllers/instancegroup_controller.go
index f295a12..8ed02b6 100644
--- a/controllers/instancegroup_controller.go
+++ b/controllers/instancegroup_controller.go
@@ -29,7 +29,6 @@ import (
 	"github.com/keikoproj/instance-manager/controllers/providers/aws"
 	"github.com/keikoproj/instance-manager/controllers/provisioners/ekscloudformation"
 	log "github.com/sirupsen/logrus"
-	yaml "gopkg.in/yaml.v2"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -199,7 +198,17 @@ func (r *InstanceGroupReconciler) ReconcileEKSCF(instanceGroup *v1alpha.Instance
 		KubeDynamic: dynClient,
 	}
 
-	defaultARNList, err := r.loadControllerConfiguration(instanceGroup)
+	if _, err := os.Stat(r.ControllerConfPath); os.IsNotExist(err) {
+		log.Errorf("controller config file not found: %v", err)
+		return err
+	}
+
+	controllerConfig, err := common.ReadFile(r.ControllerConfPath)
+	if err != nil {
+		return err
+	}
+
+	defaultConfiguration, err := ekscloudformation.LoadControllerConfiguration(instanceGroup, controllerConfig)
 	if err != nil {
 		log.Errorf("failed to load controller configuration: %v", err)
 		return err
@@ -211,11 +220,23 @@ func (r *InstanceGroupReconciler) ReconcileEKSCF(instanceGroup *v1alpha.Instance
 		return err
 	}
 
+	var stackName string
+	if defaultConfiguration.StackNamePrefix != "" {
+		stackName = fmt.Sprintf("%v-%v-%v", defaultConfiguration.StackNamePrefix, specConfig.GetClusterName(), instanceGroup.GetName())
+	} else {
+		stackName = fmt.Sprintf("%v-%v", specConfig.GetClusterName(), instanceGroup.GetName())
+	}
+
+	// set the stack name if it is unset
+	if instanceGroup.Status.GetStackName() == "" {
+		instanceGroup.Status.SetStackName(stackName)
+	}
+
 	awsWorker := aws.AwsWorker{
 		CfClient:     aws.GetAwsCloudformationClient(awsRegion),
 		AsgClient:    aws.GetAwsAsgClient(awsRegion),
 		EksClient:    aws.GetAwsEksClient(awsRegion),
-		StackName:    fmt.Sprintf("%v-%v-%v", specConfig.GetClusterName(), instanceGroup.GetNamespace(), instanceGroup.GetName()),
+		StackName:    instanceGroup.Status.GetStackName(),
 		TemplateBody: template,
 	}
 
@@ -225,7 +246,10 @@ func (r *InstanceGroupReconciler) ReconcileEKSCF(instanceGroup *v1alpha.Instance
 	}
 
 	ctx.ControllerRegion = awsRegion
-	ctx.DefaultARNList = defaultARNList
+	if len(defaultConfiguration.DefaultARNs) != 0 {
+		ctx.DefaultARNList = defaultConfiguration.DefaultARNs
+	}
+
 	ctx.TemplatePath = r.ControllerTemplatePath
 
 	// Init State is set when handling reconcile
@@ -322,41 +346,6 @@ func (r *InstanceGroupReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *InstanceGroupReconciler) loadControllerConfiguration(ig *v1alpha.InstanceGroup) ([]string, error) {
-	var defaultConfig ekscloudformation.EksCfDefaultConfiguration
-	var specConfig = &ig.Spec.EKSCFSpec.EKSCFConfiguration
-	var defaultARNs []string
-
-	if _, err := os.Stat(r.ControllerConfPath); os.IsNotExist(err) {
-		log.Errorf("controller config file not found: %v", err)
-		return nil, err
-	}
-
-	controllerConfig, err := common.ReadFile(r.ControllerConfPath)
-	if err != nil {
-		return nil, err
-	}
-
-	err = yaml.Unmarshal(controllerConfig, &defaultConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	if len(defaultConfig.DefaultSubnets) != 0 {
-		specConfig.SetSubnets(defaultConfig.DefaultSubnets)
-	}
-
-	if defaultConfig.EksClusterName != "" {
-		specConfig.SetClusterName(defaultConfig.EksClusterName)
-	}
-
-	if len(defaultConfig.DefaultARNs) != 0 {
-		defaultARNs = defaultConfig.DefaultARNs
-	}
-
-	return defaultARNs, nil
-}
-
 func (r *InstanceGroupReconciler) spotEventReconciler(obj handler.MapObject) []ctrl.Request {
 	unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.Object)
 	if err != nil {
diff --git a/controllers/provisioners/ekscloudformation/helpers.go b/controllers/provisioners/ekscloudformation/helpers.go
index 03eb4c1..f3889c9 100644
--- a/controllers/provisioners/ekscloudformation/helpers.go
+++ b/controllers/provisioners/ekscloudformation/helpers.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/keikoproj/instance-manager/api/v1alpha1"
 	"github.com/keikoproj/instance-manager/controllers/common"
+	yaml "gopkg.in/yaml.v2"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -96,10 +97,31 @@ func (ctx *EksCfInstanceGroupContext) reloadCloudformationConfiguration() error
 	if err != nil {
 		return err
 	}
+
 	ctx.AwsWorker.TemplateBody = template
 	return nil
 }
 
+func LoadControllerConfiguration(ig *v1alpha1.InstanceGroup, controllerConfig []byte) (EksCfDefaultConfiguration, error) {
+	var defaultConfig EksCfDefaultConfiguration
+	var specConfig = &ig.Spec.EKSCFSpec.EKSCFConfiguration
+
+	err := yaml.Unmarshal(controllerConfig, &defaultConfig)
+	if err != nil {
+		return defaultConfig, err
+	}
+
+	if len(defaultConfig.DefaultSubnets) != 0 {
+		specConfig.SetSubnets(defaultConfig.DefaultSubnets)
+	}
+
+	if defaultConfig.EksClusterName != "" {
+		specConfig.SetClusterName(defaultConfig.EksClusterName)
+	}
+
+	return defaultConfig, nil
+}
+
 func LoadCloudformationConfiguration(ig *v1alpha1.InstanceGroup, path string) (string, error) {
 	var renderBuffer bytes.Buffer
 
diff --git a/controllers/provisioners/ekscloudformation/types.go b/controllers/provisioners/ekscloudformation/types.go
index 264cce1..4782d70 100644
--- a/controllers/provisioners/ekscloudformation/types.go
+++ b/controllers/provisioners/ekscloudformation/types.go
@@ -41,9 +41,10 @@ type EksCfInstanceGroupContext struct {
 }
 
 type EksCfDefaultConfiguration struct {
-	DefaultSubnets []string `yaml:"defaultSubnets,omitempty"`
-	EksClusterName string   `yaml:"defaultClusterName,omitempty"`
-	DefaultARNs    []string `yaml:"defaultArns,omitempty"`
+	StackNamePrefix string   `yaml:"stackNamePrefix,omitempty"`
+	DefaultSubnets  []string `yaml:"defaultSubnets,omitempty"`
+	EksClusterName  string   `yaml:"defaultClusterName,omitempty"`
+	DefaultARNs     []string `yaml:"defaultArns,omitempty"`
 }
 
 func (ctx *EksCfInstanceGroupContext) GetInstanceGroup() *v1alpha1.InstanceGroup {
diff --git a/docs/04_instance-manager.yaml b/docs/04_instance-manager.yaml
index 23defc6..b029178 100644
--- a/docs/04_instance-manager.yaml
+++ b/docs/04_instance-manager.yaml
@@ -537,6 +537,8 @@ spec:
               type: string
             nodesInstanceRoleArn:
               type: string
+            stackName:
+              type: string
             strategyResourceName:
               type: string
             usingSpotRecommendation:
diff --git a/docs/examples/crd-argo.yaml b/docs/examples/crd-argo.yaml
index de2410a..32239df 100644
--- a/docs/examples/crd-argo.yaml
+++ b/docs/examples/crd-argo.yaml
@@ -10,7 +10,7 @@ spec:
       crdName: workflows
       statusJSONPath: .status.phase
       statusSuccessString: Succeeded
-      statusErrorString: Failed
+      statusFailureString: Failed
       concurrencyPolicy: Forbid
       spec: |
         apiVersion: argoproj.io/v1alpha1


Test patch:
diff --git a/controllers/provisioners/ekscloudformation/ekscloudformation_test.go b/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
index 6fc6641..44d370b 100644
--- a/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
+++ b/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
@@ -1166,3 +1166,41 @@ func TestStateGetter(t *testing.T) {
 		t.Fatalf("TestStateGetter: got %v, expected: %v", string(ctx.GetState()), string(v1alpha1.ReconcileReady))
 	}
 }
+
+func Test_ControllerConfigLoader(t *testing.T) {
+	ctx := getBasicContext(t, blankMocker)
+	payload := []byte(`
+stackNamePrefix: myOrg
+defaultSubnets:
+- subnet-12345678
+- subnet-23344567
+defaultClusterName: myEksCluster
+defaultArns:
+- MyARN-1
+- MyARN-2`)
+	ig := ctx.GetInstanceGroup()
+	specConfig := &ig.Spec.EKSCFSpec.EKSCFConfiguration
+	config, err := LoadControllerConfiguration(ig, payload)
+	if err != nil {
+		t.Fatal("Test_ControllerConfigLoader expected error not to have occured")
+	}
+
+	expectedConfig := EksCfDefaultConfiguration{
+		StackNamePrefix: "myOrg",
+		DefaultSubnets:  []string{"subnet-12345678", "subnet-23344567"},
+		EksClusterName:  "myEksCluster",
+		DefaultARNs:     []string{"MyARN-1", "MyARN-2"},
+	}
+
+	if !reflect.DeepEqual(config, expectedConfig) {
+		t.Fatalf("Test_ControllerConfigLoader: got %+v, expected: %+v", config, expectedConfig)
+	}
+
+	if specConfig.GetClusterName() != expectedConfig.EksClusterName {
+		t.Fatalf("Test_ControllerConfigLoader: got %v, expected: %v", specConfig.GetClusterName(), expectedConfig.EksClusterName)
+	}
+
+	if !reflect.DeepEqual(specConfig.GetSubnets(), expectedConfig.DefaultSubnets) {
+		t.Fatalf("Test_ControllerConfigLoader: got %v, expected: %v", specConfig.GetSubnets(), expectedConfig.DefaultSubnets)
+	}
+}
diff --git a/test-bdd/templates/instance-group-crd.yaml b/test-bdd/templates/instance-group-crd.yaml
index 1e1a66b..8722cc8 100644
--- a/test-bdd/templates/instance-group-crd.yaml
+++ b/test-bdd/templates/instance-group-crd.yaml
@@ -13,7 +13,7 @@ spec:
       crdName: rollingupgrades
       statusJSONPath: .status.currentStatus
       statusSuccessString: completed
-      statusErrorString: error
+      statusFailureString: error
       spec: |
         apiVersion: upgrademgr.keikoproj.io/v1alpha1
         kind: RollingUpgrade