bootstrap improvements

controllers/providers/kubernetes/utils.go

@@ -24,10 +24,12 @@ import (
 	"strings"
 
 	"github.com/ghodss/yaml"
+	"github.com/keikoproj/instance-manager/api/v1alpha1"
 	"github.com/keikoproj/instance-manager/controllers/common"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
@@ -40,6 +42,16 @@ type KubernetesClientSet struct {
 	KubeDynamic dynamic.Interface
 }
 
+func GetUnstructuredInstanceGroup(instanceGroup *v1alpha1.InstanceGroup) (*unstructured.Unstructured, error) {
+	var obj = &unstructured.Unstructured{}
+	content, err := runtime.DefaultUnstructuredConverter.ToUnstructured(instanceGroup)
+	if err != nil {
+		return obj, err
+	}
+	obj.Object = content
+	return obj, nil
+}
+
 func IsDesiredNodesReady(kube kubernetes.Interface, instanceIds []string, desiredCount int) (bool, error) {
 	nodes, err := kube.CoreV1().Nodes().List(metav1.ListOptions{})
 	if err != nil {

controllers/provisioners/eks/create.go

@@ -58,7 +58,7 @@ func (ctx *EksInstanceGroupContext) Create() error {
 	return nil
 }
 
-func (ctx EksInstanceGroupContext) CreateScalingGroup() error {
+func (ctx *EksInstanceGroupContext) CreateScalingGroup() error {
 	var (
 		instanceGroup = ctx.GetInstanceGroup()
 		spec          = instanceGroup.GetEKSSpec()

controllers/provisioners/eks/delete.go

@@ -17,7 +17,6 @@ package eks
 
 import (
 	"github.com/keikoproj/instance-manager/api/v1alpha1"
-	"github.com/keikoproj/instance-manager/controllers/common"
 	"github.com/pkg/errors"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -38,8 +37,10 @@ func (ctx *EksInstanceGroupContext) Delete() error {
 		return errors.Wrap(err, "failed to delete scaling group")
 	}
 
-	// if scaling group is deleted, defer removal from aws-auth
-	defer common.RemoveAuthConfigMap(ctx.KubernetesClient.Kubernetes, []string{roleARN})
+	// if scaling group is deleted, remove the role from aws-auth if it's not in use by other groups
+	if err := ctx.RemoveAuthRole(roleARN); err != nil {
+		return errors.Wrap(err, "failed to remove auth role")
+	}
 
 	// delete launchconfig
 	err = ctx.DeleteLaunchConfiguration()

controllers/provisioners/eks/delete_test.go

@@ -18,12 +18,16 @@ package eks
 import (
 	"testing"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/aws/aws-sdk-go/service/iam"
+	awsauth "github.com/keikoproj/aws-auth/pkg/mapper"
 	"github.com/keikoproj/instance-manager/api/v1alpha1"
+	kubeprovider "github.com/keikoproj/instance-manager/controllers/providers/kubernetes"
 	"github.com/onsi/gomega"
 	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestDeletePositive(t *testing.T) {
@@ -122,5 +126,71 @@ func TestDeleteAutoScalingGroupNegative(t *testing.T) {
 	g.Expect(err).To(gomega.HaveOccurred())
 	g.Expect(ctx.GetState()).To(gomega.Equal(v1alpha1.ReconcileDeleting))
 	asgMock.DeleteAutoScalingGroupErr = nil
+}
+
+func TestRemoveAuthRoleNegative(t *testing.T) {
+	var (
+		g       = gomega.NewGomegaWithT(t)
+		k       = MockKubernetesClientSet()
+		ig      = MockInstanceGroup()
+		ig2     = MockInstanceGroup()
+		asgMock = NewAutoScalingMocker()
+		iamMock = NewIamMocker()
+	)
+
+	w := MockAwsWorker(asgMock, iamMock)
+	ctx := MockContext(ig, k, w)
+
+	// two instancegroups with same role arn
+	ig.Status.NodesArn = "same-role"
+	ig2.Name = "instance-group-2"
+	ig2.Namespace = "different-namespace"
+	ig2.Status.NodesArn = "same-role"
+
+	igObj, err := kubeprovider.GetUnstructuredInstanceGroup(ig)
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+	ig2Obj, err := kubeprovider.GetUnstructuredInstanceGroup(ig2)
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+
+	_, err = ctx.KubernetesClient.KubeDynamic.Resource(v1alpha1.GroupVersionResource).Namespace("instance-manager").Create(igObj, metav1.CreateOptions{})
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+
+	_, err = ctx.KubernetesClient.KubeDynamic.Resource(v1alpha1.GroupVersionResource).Namespace(ig2.Namespace).Create(ig2Obj, metav1.CreateOptions{})
+	g.Expect(err).NotTo(gomega.HaveOccurred())
 
+	ctx.SetDiscoveredState(&DiscoveredState{
+		IAMRole: &iam.Role{
+			Arn: aws.String("same-role"),
+		},
+		ScalingGroup: &autoscaling.Group{},
+	})
+
+	ctx.BootstrapNodes()
+
+	// Only one role is added to aws-auth
+	auth, _, err := awsauth.ReadAuthMap(k.Kubernetes)
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+	g.Expect(len(auth.MapRoles)).To(gomega.Equal(1))
+
+	// after delete, the role should not be deleted
+	err = ctx.Delete()
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+	g.Expect(ctx.GetState()).To(gomega.Equal(v1alpha1.ReconcileDeleting))
+
+	auth, _, err = awsauth.ReadAuthMap(k.Kubernetes)
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+	g.Expect(len(auth.MapRoles)).To(gomega.Equal(1))
+
+	// this time the role should be successfully removed
+	err = ctx.KubernetesClient.KubeDynamic.Resource(v1alpha1.GroupVersionResource).Namespace(ig2.Namespace).Delete(ig2.Name, &metav1.DeleteOptions{})
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+
+	err = ctx.Delete()
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+	g.Expect(ctx.GetState()).To(gomega.Equal(v1alpha1.ReconcileDeleting))
+
+	auth, _, err = awsauth.ReadAuthMap(k.Kubernetes)
+	t.Log(auth)
+	g.Expect(err).NotTo(gomega.HaveOccurred())
+	g.Expect(len(auth.MapRoles)).To(gomega.Equal(0))
 }

controllers/provisioners/eks/eks.go

@@ -16,6 +16,8 @@ limitations under the License.
 package eks
 
 import (
+	"sync"
+
 	"github.com/go-logr/logr"
 	"github.com/keikoproj/instance-manager/api/v1alpha1"
 	awsprovider "github.com/keikoproj/instance-manager/controllers/providers/aws"
@@ -71,6 +73,7 @@ type EksDefaultConfiguration struct {
 }
 
 type EksInstanceGroupContext struct {
+	sync.Mutex
 	InstanceGroup    *v1alpha1.InstanceGroup
 	KubernetesClient kubeprovider.KubernetesClientSet
 	AwsWorker        awsprovider.AwsWorker

controllers/provisioners/eks/helpers.go

@@ -24,10 +24,13 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/keikoproj/instance-manager/api/v1alpha1"
+	"github.com/keikoproj/instance-manager/controllers/common"
 	awsprovider "github.com/keikoproj/instance-manager/controllers/providers/aws"
 	kubeprovider "github.com/keikoproj/instance-manager/controllers/providers/kubernetes"
 	"github.com/keikoproj/instance-manager/controllers/provisioners"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
 func (ctx *EksInstanceGroupContext) GetLaunchConfigurationInput(name string) *autoscaling.CreateLaunchConfigurationInput {
@@ -329,3 +332,39 @@ func (ctx *EksInstanceGroupContext) GetManagedPoliciesList(additionalPolicies []
 	}
 	return managedPolicies
 }
+
+func (ctx *EksInstanceGroupContext) RemoveAuthRole(arn string) error {
+	ctx.Lock()
+	defer ctx.Unlock()
+
+	var instanceGroup = ctx.GetInstanceGroup()
+	var list = &unstructured.UnstructuredList{}
+	var sharedGroups = make([]string, 0)
+
+	list, err := ctx.KubernetesClient.KubeDynamic.Resource(v1alpha1.GroupVersionResource).List(metav1.ListOptions{})
+	if err != nil {
+		return err
+	}
+
+	// find objects which share the same nodesInstanceRoleArn
+	for _, obj := range list.Items {
+		if val, ok, _ := unstructured.NestedString(obj.Object, "status", "nodesInstanceRoleArn"); ok {
+			if strings.EqualFold(arn, val) {
+				sharedGroups = append(sharedGroups, obj.GetName())
+			}
+		}
+	}
+
+	// If there are other instance groups using the same role we should not remove it from aws-auth
+	if len(sharedGroups) > 1 {
+		ctx.Log.Info(
+			"skipping removal of auth role, is used by another instancegroup",
+			"instancegroup", instanceGroup.GetName(),
+			"arn", arn,
+			"conflict", strings.Join(sharedGroups, ","),
+		)
+		return nil
+	}
+
+	return common.RemoveAuthConfigMap(ctx.KubernetesClient.Kubernetes, []string{arn})
+}

controllers/provisioners/eks/update.go

@@ -63,6 +63,13 @@ func (ctx *EksInstanceGroupContext) Update() error {
 		return errors.Wrap(err, "failed to update scaling group")
 	}
 
+	// we should try to bootstrap the role before we wait for nodes to be ready
+	// to avoid getting locked if someone made a manual change to aws-auth
+	err = ctx.BootstrapNodes()
+	if err != nil {
+		ctx.Log.Error(err, "failed to bootstrap role", "instancegroup", instanceGroup.GetName())
+	}
+
 	// update readiness conditions
 	nodesReady := ctx.UpdateNodeReadyCondition()
 	if nodesReady {

controllers/provisioners/eks/upgrade.go

@@ -74,10 +74,16 @@ func (ctx *EksInstanceGroupContext) UpgradeNodes() error {
 
 func (ctx *EksInstanceGroupContext) BootstrapNodes() error {
 	var (
-		state   = ctx.GetDiscoveredState()
-		role    = state.GetRole()
-		roleARN = aws.StringValue(role.Arn)
+		state         = ctx.GetDiscoveredState()
+		instanceGroup = ctx.GetInstanceGroup()
+		role          = state.GetRole()
+		roleARN       = aws.StringValue(role.Arn)
 	)
+	ctx.Log.Info("bootstrapping arn to aws-auth", "instancegroup", instanceGroup.GetName(), "arn", roleARN)
+
+	// lock to guarantee Upsert and Remove cannot conflict when roles are shared between instancegroups
+	ctx.Lock()
+	defer ctx.Unlock()
 
 	return common.UpsertAuthConfigMap(ctx.KubernetesClient.Kubernetes, []string{roleARN})
 }