Use Security Groups status for node launch

pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml

@@ -520,6 +520,10 @@ spec:
                 description: InstanceProfile contains the resolved instance profile
                   for the role
                 type: string
+              observedGeneration:
+                description: The generation observed by the nodeclass controller.
+                format: int64
+                type: integer
               securityGroups:
                 description: |-
                   SecurityGroups contains the current Security Groups values that are available to the

pkg/apis/v1beta1/ec2nodeclass_status.go

@@ -68,4 +68,7 @@ type EC2NodeClassStatus struct {
 	// InstanceProfile contains the resolved instance profile for the role
 	// +optional
 	InstanceProfile string `json:"instanceProfile,omitempty"`
+	// The generation observed by the nodeclass controller.
+	// +optional
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }

pkg/cloudprovider/drift.go

@@ -54,7 +54,7 @@ func (c *CloudProvider) isNodeClassDrifted(ctx context.Context, nodeClaim *corev
 	if err != nil {
 		return "", fmt.Errorf("calculating ami drift, %w", err)
 	}
-	securitygroupDrifted, err := c.areSecurityGroupsDrifted(ctx, instance, nodeClass)
+	securitygroupDrifted, err := c.areSecurityGroupsDrifted(instance, nodeClass)
 	if err != nil {
 		return "", fmt.Errorf("calculating securitygroup drift, %w", err)
 	}
@@ -118,12 +118,11 @@ func (c *CloudProvider) isSubnetDrifted(ctx context.Context, instance *instance.
 
 // Checks if the security groups are drifted, by comparing the security groups returned from the SecurityGroupProvider
 // to the ec2 instance security groups
-func (c *CloudProvider) areSecurityGroupsDrifted(ctx context.Context, ec2Instance *instance.Instance, nodeClass *v1beta1.EC2NodeClass) (cloudprovider.DriftReason, error) {
-	securitygroup, err := c.securityGroupProvider.List(ctx, nodeClass)
-	if err != nil {
-		return "", err
+func (c *CloudProvider) areSecurityGroupsDrifted(ec2Instance *instance.Instance, nodeClass *v1beta1.EC2NodeClass) (cloudprovider.DriftReason, error) {
+	if nodeClass.Generation != nodeClass.Status.ObservedGeneration {
+		return "", fmt.Errorf("waiting until nodeclass is updated")
 	}
-	securityGroupIds := sets.New(lo.Map(securitygroup, func(sg *ec2.SecurityGroup, _ int) string { return aws.StringValue(sg.GroupId) })...)
+	securityGroupIds := sets.New(lo.Map(nodeClass.Status.SecurityGroups, func(sg v1beta1.SecurityGroup, _ int) string { return sg.ID })...)
 	if len(securityGroupIds) == 0 {
 		return "", fmt.Errorf("no security groups are discovered")
 	}

pkg/controllers/nodeclass/status/controller.go

@@ -94,6 +94,7 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeCl
 	}
 
 	if !equality.Semantic.DeepEqual(stored, nodeClass) {
+		nodeClass.Status.ObservedGeneration = nodeClass.Generation
 		if err := c.kubeClient.Status().Patch(ctx, nodeClass, client.MergeFrom(stored)); err != nil {
 			errs = multierr.Append(errs, client.IgnoreNotFound(err))
 		}

pkg/controllers/nodeclass/status/securitygroup.go

@@ -50,5 +50,5 @@ func (sg *SecurityGroup) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2No
 			Name: *securityGroup.GroupName,
 		}
 	})
-	return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil
+	return reconcile.Result{RequeueAfter: time.Minute}, nil
 }

pkg/providers/instance/suite_test.go

@@ -26,16 +26,19 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1"
 	corecloudprovider "sigs.k8s.io/karpenter/pkg/cloudprovider"
 	"sigs.k8s.io/karpenter/pkg/events"
+	"sigs.k8s.io/karpenter/pkg/operator/controller"
 	coreoptions "sigs.k8s.io/karpenter/pkg/operator/options"
 	"sigs.k8s.io/karpenter/pkg/operator/scheme"
 	coretest "sigs.k8s.io/karpenter/pkg/test"
 
 	"github.com/aws/karpenter-provider-aws/pkg/apis"
 	"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
 	"github.com/aws/karpenter-provider-aws/pkg/cloudprovider"
+	"github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status"
 	"github.com/aws/karpenter-provider-aws/pkg/fake"
 	"github.com/aws/karpenter-provider-aws/pkg/operator/options"
 	"github.com/aws/karpenter-provider-aws/pkg/providers/instance"
@@ -51,6 +54,7 @@ var ctx context.Context
 var env *coretest.Environment
 var awsEnv *test.Environment
 var cloudProvider *cloudprovider.CloudProvider
+var statusController controller.Controller
 
 func TestAWS(t *testing.T) {
 	ctx = TestContextWithLogger(t)
@@ -65,6 +69,14 @@ var _ = BeforeSuite(func() {
 	awsEnv = test.NewEnvironment(ctx, env)
 	cloudProvider = cloudprovider.New(awsEnv.InstanceTypesProvider, awsEnv.InstanceProvider, events.NewRecorder(&record.FakeRecorder{}),
 		env.Client, awsEnv.AMIProvider, awsEnv.SecurityGroupProvider, awsEnv.SubnetProvider)
+	statusController = status.NewController(
+		env.Client,
+		awsEnv.SubnetProvider,
+		awsEnv.SecurityGroupProvider,
+		awsEnv.AMIProvider,
+		awsEnv.InstanceProfileProvider,
+		awsEnv.LaunchTemplateProvider,
+	)
 })
 
 var _ = AfterSuite(func() {
@@ -111,6 +123,8 @@ var _ = Describe("InstanceProvider", func() {
 	})
 	It("should return an ICE error when all attempted instance types return an ICE error", func() {
 		ExpectApplied(ctx, env.Client, nodeClaim, nodePool, nodeClass)
+		ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass))
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
 		awsEnv.EC2API.InsufficientCapacityPools.Set([]fake.CapacityPool{
 			{CapacityType: corev1beta1.CapacityTypeOnDemand, InstanceType: "m5.xlarge", Zone: "test-zone-1a"},
 			{CapacityType: corev1beta1.CapacityTypeOnDemand, InstanceType: "m5.xlarge", Zone: "test-zone-1b"},