chore: Add status conditions on nodeClass

pkg/apis/v1/ec2nodeclass_status.go

@@ -15,10 +15,17 @@ limitations under the License.
 package v1
 
 import (
-	op "github.com/awslabs/operatorpkg/status"
+	"github.com/awslabs/operatorpkg/status"
 	v1 "k8s.io/api/core/v1"
 )
 
+const (
+	ConditionTypeSubnetsReady         = "SubnetsReady"
+	ConditionTypeSecurityGroupsReady  = "SecurityGroupsReady"
+	ConditionTypeAMIsReady            = "AMIsReady"
+	ConditionTypeInstanceProfileReady = "InstanceProfileReady"
+)
+
 // Subnet contains resolved Subnet selector values utilized for node launch
 type Subnet struct {
 	// ID of the subnet
@@ -74,22 +81,22 @@ type EC2NodeClassStatus struct {
 	InstanceProfile string `json:"instanceProfile,omitempty"`
 	// Conditions contains signals for health and readiness
 	// +optional
-	Conditions []op.Condition `json:"conditions,omitempty"`
+	Conditions []status.Condition `json:"conditions,omitempty"`
 }
 
-const (
-	// 	ConditionTypeNodeClassReady = "Ready" condition indicates that subnets, security groups, AMIs and instance profile for nodeClass were resolved
-	ConditionTypeNodeClassReady = "Ready"
-)
-
-func (in *EC2NodeClass) StatusConditions() op.ConditionSet {
-	return op.NewReadyConditions(ConditionTypeNodeClassReady).For(in)
+func (in *EC2NodeClass) StatusConditions() status.ConditionSet {
+	return status.NewReadyConditions(
+		ConditionTypeAMIsReady,
+		ConditionTypeSubnetsReady,
+		ConditionTypeSecurityGroupsReady,
+		ConditionTypeInstanceProfileReady,
+	).For(in)
 }
 
-func (in *EC2NodeClass) GetConditions() []op.Condition {
+func (in *EC2NodeClass) GetConditions() []status.Condition {
 	return in.Status.Conditions
 }
 
-func (in *EC2NodeClass) SetConditions(conditions []op.Condition) {
+func (in *EC2NodeClass) SetConditions(conditions []status.Condition) {
 	in.Status.Conditions = conditions
 }

pkg/apis/v1beta1/ec2nodeclass_status.go

@@ -19,6 +19,13 @@ import (
 	v1 "k8s.io/api/core/v1"
 )
 
+const (
+	ConditionTypeSubnetsReady         = "SubnetsReady"
+	ConditionTypeSecurityGroupsReady  = "SecurityGroupsReady"
+	ConditionTypeAMIsReady            = "AMIsReady"
+	ConditionTypeInstanceProfileReady = "InstanceProfileReady"
+)
+
 // Subnet contains resolved Subnet selector values utilized for node launch
 type Subnet struct {
 	// ID of the subnet
@@ -78,7 +85,12 @@ type EC2NodeClassStatus struct {
 }
 
 func (in *EC2NodeClass) StatusConditions() status.ConditionSet {
-	return status.NewReadyConditions().For(in)
+	return status.NewReadyConditions(
+		ConditionTypeAMIsReady,
+		ConditionTypeSubnetsReady,
+		ConditionTypeSecurityGroupsReady,
+		ConditionTypeInstanceProfileReady,
+	).For(in)
 }
 
 func (in *EC2NodeClass) GetConditions() []status.Condition {

pkg/controllers/nodeclass/status/ami.go

@@ -41,6 +41,7 @@ func (a *AMI) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (r
 	}
 	if len(amis) == 0 {
 		nodeClass.Status.AMIs = nil
+		nodeClass.StatusConditions().SetFalse(v1beta1.ConditionTypeAMIsReady, "AMINotFound", "AMISelector did not match any AMIs")
 		return reconcile.Result{}, nil
 	}
 	nodeClass.Status.AMIs = lo.Map(amis, func(ami amifamily.AMI, _ int) v1beta1.AMI {
@@ -60,5 +61,6 @@ func (a *AMI) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (r
 			Requirements: reqs,
 		}
 	})
+	nodeClass.StatusConditions().SetTrue(v1beta1.ConditionTypeAMIsReady)
 	return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil
 }

pkg/controllers/nodeclass/status/ami_test.go

@@ -205,6 +205,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
 				},
 			},
 		}))
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeAMIsReady)).To(BeTrue())
 	})
 	It("should resolve amiSelector AMis and requirements into status when all SSM aliases don't resolve", func() {
 		version := lo.Must(awsEnv.VersionProvider.Get(ctx))
@@ -283,6 +284,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
 				},
 			},
 		}))
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeAMIsReady)).To(BeTrue())
 	})
 	It("Should resolve a valid AMI selector", func() {
 		ExpectApplied(ctx, env.Client, nodeClass)
@@ -302,5 +304,13 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
 				},
 			},
 		))
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeAMIsReady)).To(BeTrue())
+	})
+	It("should get error when resolving AMIs and have status condition set to false", func() {
+		awsEnv.EC2API.NextError.Set(fmt.Errorf("unable to resolve AMI"))
+		ExpectApplied(ctx, env.Client, nodeClass)
+		ExpectObjectReconcileFailed(ctx, env.Client, statusController, nodeClass)
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeAMIsReady)).To(BeFalse())
 	})
 })

pkg/controllers/nodeclass/status/instanceprofile.go

@@ -39,6 +39,6 @@ func (ip *InstanceProfile) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2
 	} else {
 		nodeClass.Status.InstanceProfile = lo.FromPtr(nodeClass.Spec.InstanceProfile)
 	}
-
+	nodeClass.StatusConditions().SetTrue(v1beta1.ConditionTypeInstanceProfileReady)
 	return reconcile.Result{}, nil
 }

pkg/controllers/nodeclass/status/instanceprofile_test.go

@@ -17,6 +17,7 @@ package status_test
 import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/iam"
+	"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
 	"github.com/samber/lo"
 
 	"github.com/aws/karpenter-provider-aws/pkg/fake"
@@ -43,6 +44,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
 
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
 		Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName))
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeInstanceProfileReady)).To(BeTrue())
 	})
 	It("should add the role to the instance profile when it exists without a role", func() {
 		awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{
@@ -62,6 +64,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
 
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
 		Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName))
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeInstanceProfileReady)).To(BeTrue())
 	})
 	It("should update the role for the instance profile when the wrong role exists", func() {
 		awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{
@@ -86,6 +89,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
 
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
 		Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName))
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeInstanceProfileReady)).To(BeTrue())
 	})
 	It("should not call CreateInstanceProfile or AddRoleToInstanceProfile when instance profile exists with correct role", func() {
 		awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{
@@ -110,6 +114,8 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
 
 		Expect(awsEnv.IAMAPI.CreateInstanceProfileBehavior.Calls()).To(BeZero())
 		Expect(awsEnv.IAMAPI.AddRoleToInstanceProfileBehavior.Calls()).To(BeZero())
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeInstanceProfileReady)).To(BeTrue())
 	})
 	It("should resolve the specified instance profile into the status when using instanceProfile field", func() {
 		nodeClass.Spec.Role = ""
@@ -119,6 +125,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
 
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
 		Expect(nodeClass.Status.InstanceProfile).To(Equal(lo.FromPtr(nodeClass.Spec.InstanceProfile)))
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeInstanceProfileReady)).To(BeTrue())
 	})
 	It("should not call the the IAM API when specifying an instance profile", func() {
 		nodeClass.Spec.Role = ""
@@ -128,5 +135,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
 
 		Expect(awsEnv.IAMAPI.CreateInstanceProfileBehavior.Calls()).To(BeZero())
 		Expect(awsEnv.IAMAPI.AddRoleToInstanceProfileBehavior.Calls()).To(BeZero())
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
+		Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeInstanceProfileReady)).To(BeTrue())
 	})
 })

pkg/controllers/nodeclass/status/launchtemplate_test.go

@@ -16,6 +16,7 @@ package status_test
 
 import (
 	"github.com/aws/aws-sdk-go/service/eks"
+	"github.com/awslabs/operatorpkg/status"
 	"github.com/samber/lo"
 
 	"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
@@ -70,6 +71,8 @@ var _ = Describe("NodeClass Launch Template CIDR Resolution Controller", func()
 		ExpectApplied(ctx, env.Client, nodeClass)
 		ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
 		Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("10.100.0.0/16"))
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
+		Expect(nodeClass.StatusConditions().IsTrue(status.ConditionReady)).To(BeTrue())
 	})
 	It("should resolve cluster CIDR for IPv6 clusters", func() {
 		awsEnv.EKSAPI.DescribeClusterBehavior.Output.Set(&eks.DescribeClusterOutput{
@@ -83,5 +86,7 @@ var _ = Describe("NodeClass Launch Template CIDR Resolution Controller", func()
 		ExpectApplied(ctx, env.Client, nodeClass)
 		ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
 		Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("2001:db8::/64"))
+		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
+		Expect(nodeClass.StatusConditions().IsTrue(status.ConditionReady)).To(BeTrue())
 	})
 })

pkg/controllers/nodeclass/status/readiness.go

@@ -33,22 +33,6 @@ type Readiness struct {
 }
 
 func (n Readiness) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) {
-	if len(nodeClass.Status.AMIs) == 0 {
-		nodeClass.StatusConditions().SetFalse(status.ConditionReady, "NodeClassNotReady", "Failed to resolve AMIs")
-		return reconcile.Result{}, nil
-	}
-	if len(nodeClass.Status.Subnets) == 0 {
-		nodeClass.StatusConditions().SetFalse(status.ConditionReady, "NodeClassNotReady", "Failed to resolve subnets")
-		return reconcile.Result{}, nil
-	}
-	if len(nodeClass.Status.SecurityGroups) == 0 {
-		nodeClass.StatusConditions().SetFalse(status.ConditionReady, "NodeClassNotReady", "Failed to resolve security groups")
-		return reconcile.Result{}, nil
-	}
-	if len(nodeClass.Status.InstanceProfile) == 0 {
-		nodeClass.StatusConditions().SetFalse(status.ConditionReady, "NodeClassNotReady", "Failed to resolve instance profile")
-		return reconcile.Result{}, nil
-	}
 	// A NodeClass that uses AL2023 requires the cluster CIDR for launching nodes.
 	// To allow Karpenter to be used for Non-EKS clusters, resolving the Cluster CIDR
 	// will not be done at startup but instead in a reconcile loop.
@@ -58,6 +42,5 @@ func (n Readiness) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClas
 			return reconcile.Result{}, fmt.Errorf("failed to detect the cluster CIDR, %w", err)
 		}
 	}
-	nodeClass.StatusConditions().SetTrue(status.ConditionReady)
 	return reconcile.Result{}, nil
 }

pkg/controllers/nodeclass/status/readiness_test.go

@@ -51,7 +51,7 @@ var _ = Describe("NodeClass Status Condition Controller", func() {
 		ExpectApplied(ctx, env.Client, nodeClass)
 		ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
-		Expect(nodeClass.Status.Conditions).To(HaveLen(1))
+		Expect(nodeClass.Status.Conditions).To(HaveLen(5))
 		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).IsTrue()).To(BeTrue())
 	})
 	It("should update status condition as Not Ready", func() {
@@ -65,6 +65,6 @@ var _ = Describe("NodeClass Status Condition Controller", func() {
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
 
 		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).IsFalse()).To(BeTrue())
-		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).Message).To(Equal("Failed to resolve security groups"))
+		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).Message).To(Equal("SecurityGroupsReady=False"))
 	})
 })

pkg/controllers/nodeclass/status/securitygroup.go

@@ -39,6 +39,7 @@ func (sg *SecurityGroup) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2No
 	}
 	if len(securityGroups) == 0 && len(nodeClass.Spec.SecurityGroupSelectorTerms) > 0 {
 		nodeClass.Status.SecurityGroups = nil
+		nodeClass.StatusConditions().SetFalse(v1beta1.ConditionTypeSecurityGroupsReady, "SecurityGroupsNotFound", "SecurityGroupSelector did not match any SecurityGroups")
 		return reconcile.Result{}, nil
 	}
 	sort.Slice(securityGroups, func(i, j int) bool {
@@ -50,5 +51,6 @@ func (sg *SecurityGroup) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2No
 			Name: *securityGroup.GroupName,
 		}
 	})
+	nodeClass.StatusConditions().SetTrue(v1beta1.ConditionTypeSecurityGroupsReady)
 	return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil
 }

pkg/controllers/nodeclass/status/securitygroup_test.go

@@ -15,8 +15,6 @@ limitations under the License.
 package status_test
 
 import (
-	"github.com/awslabs/operatorpkg/status"
-
 	"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
 	"github.com/aws/karpenter-provider-aws/pkg/test"
 
@@ -65,6 +63,7 @@ var _ = Describe("NodeClass Security Group Status Controller", func() {
 				Name: "securityGroup-test3",
 			},
 		}))
+		Expect(nodeClass.StatusConditions().Get(v1beta1.ConditionTypeSecurityGroupsReady).IsTrue()).To(BeTrue())
 	})
 	It("Should resolve a valid selectors for Security Groups by tags", func() {
 		nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{
@@ -88,6 +87,7 @@ var _ = Describe("NodeClass Security Group Status Controller", func() {
 				Name: "securityGroup-test2",
 			},
 		}))
+		Expect(nodeClass.StatusConditions().Get(v1beta1.ConditionTypeSecurityGroupsReady).IsTrue()).To(BeTrue())
 	})
 	It("Should resolve a valid selectors for Security Groups by ids", func() {
 		nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{
@@ -104,6 +104,7 @@ var _ = Describe("NodeClass Security Group Status Controller", func() {
 				Name: "securityGroup-test1",
 			},
 		}))
+		Expect(nodeClass.StatusConditions().Get(v1beta1.ConditionTypeSecurityGroupsReady).IsTrue()).To(BeTrue())
 	})
 	It("Should update Security Groups status when the Security Groups selector gets updated by tags", func() {
 		ExpectApplied(ctx, env.Client, nodeClass)
@@ -145,6 +146,7 @@ var _ = Describe("NodeClass Security Group Status Controller", func() {
 				Name: "securityGroup-test2",
 			},
 		}))
+		Expect(nodeClass.StatusConditions().Get(v1beta1.ConditionTypeSecurityGroupsReady).IsTrue()).To(BeTrue())
 	})
 	It("Should update Security Groups status when the Security Groups selector gets updated by ids", func() {
 		ExpectApplied(ctx, env.Client, nodeClass)
@@ -179,6 +181,7 @@ var _ = Describe("NodeClass Security Group Status Controller", func() {
 				Name: "securityGroup-test1",
 			},
 		}))
+		Expect(nodeClass.StatusConditions().Get(v1beta1.ConditionTypeSecurityGroupsReady).IsTrue()).To(BeTrue())
 	})
 	It("Should not resolve a invalid selectors for Security Groups", func() {
 		nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{
@@ -190,8 +193,7 @@ var _ = Describe("NodeClass Security Group Status Controller", func() {
 		ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
 		Expect(nodeClass.Status.SecurityGroups).To(BeNil())
-		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).IsFalse()).To(BeTrue())
-		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).Message).To(Equal("Failed to resolve security groups"))
+		Expect(nodeClass.StatusConditions().Get(v1beta1.ConditionTypeSecurityGroupsReady).IsFalse()).To(BeTrue())
 	})
 	It("Should not resolve a invalid selectors for an updated Security Groups selector", func() {
 		ExpectApplied(ctx, env.Client, nodeClass)
@@ -221,7 +223,6 @@ var _ = Describe("NodeClass Security Group Status Controller", func() {
 		ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
 		nodeClass = ExpectExists(ctx, env.Client, nodeClass)
 		Expect(nodeClass.Status.SecurityGroups).To(BeNil())
-		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).IsFalse()).To(BeTrue())
-		Expect(nodeClass.StatusConditions().Get(status.ConditionReady).Message).To(Equal("Failed to resolve security groups"))
+		Expect(nodeClass.StatusConditions().Get(v1beta1.ConditionTypeSecurityGroupsReady).IsFalse()).To(BeTrue())
 	})
 })

pkg/controllers/nodeclass/status/subnet.go

@@ -39,6 +39,7 @@ func (s *Subnet) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass)
 	}
 	if len(subnets) == 0 {
 		nodeClass.Status.Subnets = nil
+		nodeClass.StatusConditions().SetFalse(v1beta1.ConditionTypeSubnetsReady, "SubnetsNotFound", "SubnetSelector did not match any Subnets")
 		return reconcile.Result{}, nil
 	}
 	sort.Slice(subnets, func(i, j int) bool {
@@ -54,6 +55,6 @@ func (s *Subnet) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass)
 			ZoneID: *ec2subnet.AvailabilityZoneId,
 		}
 	})
-
+	nodeClass.StatusConditions().SetTrue(v1beta1.ConditionTypeSubnetsReady)
 	return reconcile.Result{RequeueAfter: time.Minute}, nil
 }