From eb019dbb4e28fba1a243effeaa9d29a6a6f7213f Mon Sep 17 00:00:00 2001 From: jigisha620 Date: Mon, 8 Jul 2024 15:47:15 -0700 Subject: [PATCH] chore: Add status conditions on nodeClass --- pkg/apis/v1/ec2nodeclass_status.go | 29 ++++++++++++------- pkg/apis/v1beta1/ec2nodeclass_status.go | 14 ++++++++- pkg/controllers/nodeclass/status/ami.go | 2 ++ pkg/controllers/nodeclass/status/ami_test.go | 10 +++++++ .../nodeclass/status/instanceprofile.go | 2 +- .../nodeclass/status/instanceprofile_test.go | 9 ++++++ .../nodeclass/status/launchtemplate_test.go | 5 ++++ pkg/controllers/nodeclass/status/readiness.go | 17 ----------- .../nodeclass/status/readiness_test.go | 4 +-- .../nodeclass/status/securitygroup.go | 2 ++ .../nodeclass/status/securitygroup_test.go | 13 +++++---- pkg/controllers/nodeclass/status/subnet.go | 3 +- .../nodeclass/status/subnet_test.go | 14 +++++---- test/suites/ami/suite_test.go | 4 ++- .../integration/instance_profile_test.go | 5 +++- .../suites/integration/security_group_test.go | 4 ++- test/suites/integration/subnet_test.go | 4 ++- 17 files changed, 92 insertions(+), 49 deletions(-) diff --git a/pkg/apis/v1/ec2nodeclass_status.go b/pkg/apis/v1/ec2nodeclass_status.go index 6b551971ca08..6114d2cce35a 100644 --- a/pkg/apis/v1/ec2nodeclass_status.go +++ b/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 } diff --git a/pkg/apis/v1beta1/ec2nodeclass_status.go b/pkg/apis/v1beta1/ec2nodeclass_status.go index 9510e5b0567b..4731f5aeede8 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_status.go +++ b/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 { diff --git a/pkg/controllers/nodeclass/status/ami.go b/pkg/controllers/nodeclass/status/ami.go index baeba547d390..f8899660e0c8 100644 --- a/pkg/controllers/nodeclass/status/ami.go +++ b/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 } diff --git a/pkg/controllers/nodeclass/status/ami_test.go b/pkg/controllers/nodeclass/status/ami_test.go index a5213a0491b5..d395a91bfa9b 100644 --- a/pkg/controllers/nodeclass/status/ami_test.go +++ b/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()) }) }) diff --git a/pkg/controllers/nodeclass/status/instanceprofile.go b/pkg/controllers/nodeclass/status/instanceprofile.go index 10988803db5c..882cbc5e7ed7 100644 --- a/pkg/controllers/nodeclass/status/instanceprofile.go +++ b/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 } diff --git a/pkg/controllers/nodeclass/status/instanceprofile_test.go b/pkg/controllers/nodeclass/status/instanceprofile_test.go index 163e0b95e909..505d98768609 100644 --- a/pkg/controllers/nodeclass/status/instanceprofile_test.go +++ b/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()) }) }) diff --git a/pkg/controllers/nodeclass/status/launchtemplate_test.go b/pkg/controllers/nodeclass/status/launchtemplate_test.go index f5163d75df5f..67a38fe11451 100644 --- a/pkg/controllers/nodeclass/status/launchtemplate_test.go +++ b/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()) }) }) diff --git a/pkg/controllers/nodeclass/status/readiness.go b/pkg/controllers/nodeclass/status/readiness.go index 1aac2e87bf2a..06ce3bad486f 100644 --- a/pkg/controllers/nodeclass/status/readiness.go +++ b/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 } diff --git a/pkg/controllers/nodeclass/status/readiness_test.go b/pkg/controllers/nodeclass/status/readiness_test.go index 8bf678c4b3ad..2cb807c33b6c 100644 --- a/pkg/controllers/nodeclass/status/readiness_test.go +++ b/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")) }) }) diff --git a/pkg/controllers/nodeclass/status/securitygroup.go b/pkg/controllers/nodeclass/status/securitygroup.go index 764bf26969de..59817d4544b1 100644 --- a/pkg/controllers/nodeclass/status/securitygroup.go +++ b/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 } diff --git a/pkg/controllers/nodeclass/status/securitygroup_test.go b/pkg/controllers/nodeclass/status/securitygroup_test.go index 78a14476c9ce..4daf63852b6f 100644 --- a/pkg/controllers/nodeclass/status/securitygroup_test.go +++ b/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()) }) }) diff --git a/pkg/controllers/nodeclass/status/subnet.go b/pkg/controllers/nodeclass/status/subnet.go index 2f87638ca340..5d987eed74b0 100644 --- a/pkg/controllers/nodeclass/status/subnet.go +++ b/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 } diff --git a/pkg/controllers/nodeclass/status/subnet_test.go b/pkg/controllers/nodeclass/status/subnet_test.go index 5658e0fc7cce..b2e9e0c70535 100644 --- a/pkg/controllers/nodeclass/status/subnet_test.go +++ b/pkg/controllers/nodeclass/status/subnet_test.go @@ -17,8 +17,6 @@ package status_test import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" - "github.com/awslabs/operatorpkg/status" - "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" "github.com/aws/karpenter-provider-aws/pkg/test" @@ -75,6 +73,7 @@ var _ = Describe("NodeClass Subnet Status Controller", func() { ZoneID: "tstz1-1alocal", }, })) + Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeSubnetsReady)).To(BeTrue()) }) It("Should have the correct ordering for the Subnets", func() { awsEnv.EC2API.DescribeSubnetsOutput.Set(&ec2.DescribeSubnetsOutput{Subnets: []*ec2.Subnet{ @@ -102,6 +101,7 @@ var _ = Describe("NodeClass Subnet Status Controller", func() { ZoneID: "tstz1-1a", }, })) + Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeSubnetsReady)).To(BeTrue()) }) It("Should resolve a valid selectors for Subnet by tags", func() { nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ @@ -127,6 +127,7 @@ var _ = Describe("NodeClass Subnet Status Controller", func() { ZoneID: "tstz1-1b", }, })) + Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeSubnetsReady)).To(BeTrue()) }) It("Should resolve a valid selectors for Subnet by ids", func() { nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ @@ -144,6 +145,7 @@ var _ = Describe("NodeClass Subnet Status Controller", func() { ZoneID: "tstz1-1a", }, })) + Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeSubnetsReady)).To(BeTrue()) }) It("Should update Subnet status when the Subnet selector gets updated by tags", func() { ExpectApplied(ctx, env.Client, nodeClass) @@ -199,6 +201,7 @@ var _ = Describe("NodeClass Subnet Status Controller", func() { ZoneID: "tstz1-1b", }, })) + Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeSubnetsReady)).To(BeTrue()) }) It("Should update Subnet status when the Subnet selector gets updated by ids", func() { ExpectApplied(ctx, env.Client, nodeClass) @@ -242,6 +245,7 @@ var _ = Describe("NodeClass Subnet Status Controller", func() { ZoneID: "tstz1-1a", }, })) + Expect(nodeClass.StatusConditions().IsTrue(v1beta1.ConditionTypeSubnetsReady)).To(BeTrue()) }) It("Should not resolve a invalid selectors for Subnet", func() { nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ @@ -253,8 +257,7 @@ var _ = Describe("NodeClass Subnet Status Controller", func() { ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.Subnets).To(BeNil()) - Expect(nodeClass.StatusConditions().Get(status.ConditionReady).IsFalse()).To(BeTrue()) - Expect(nodeClass.StatusConditions().Get(status.ConditionReady).Message).To(Equal("Failed to resolve subnets")) + Expect(nodeClass.StatusConditions().Get(v1beta1.ConditionTypeSubnetsReady).IsFalse()).To(BeTrue()) }) It("Should not resolve a invalid selectors for an updated subnet selector", func() { ExpectApplied(ctx, env.Client, nodeClass) @@ -292,7 +295,6 @@ var _ = Describe("NodeClass Subnet Status Controller", func() { ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.Subnets).To(BeNil()) - Expect(nodeClass.StatusConditions().Get(status.ConditionReady).IsFalse()).To(BeTrue()) - Expect(nodeClass.StatusConditions().Get(status.ConditionReady).Message).To(Equal("Failed to resolve subnets")) + Expect(nodeClass.StatusConditions().Get(v1beta1.ConditionTypeSubnetsReady).IsFalse()).To(BeTrue()) }) }) diff --git a/test/suites/ami/suite_test.go b/test/suites/ami/suite_test.go index d6b6278866b2..35dc80071996 100644 --- a/test/suites/ami/suite_test.go +++ b/test/suites/ami/suite_test.go @@ -238,6 +238,7 @@ var _ = Describe("AMI", func() { nc := EventuallyExpectAMIsToExist(nodeClass) Expect(len(nc.Status.AMIs)).To(BeNumerically("==", 1)) Expect(nc.Status.AMIs[0].ID).To(Equal(customAMI)) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1beta1.ConditionTypeAMIsReady, Status: metav1.ConditionTrue}) ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionTrue}) }) It("should have ec2nodeClass status as not ready since AMI was not resolved", func() { @@ -247,7 +248,8 @@ var _ = Describe("AMI", func() { }, } env.ExpectCreated(nodeClass) - ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionFalse, Message: "Failed to resolve AMIs"}) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1beta1.ConditionTypeAMIsReady, Status: metav1.ConditionFalse, Message: "AMISelector did not match any AMIs"}) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionFalse, Message: "AMIsReady=False"}) }) }) diff --git a/test/suites/integration/instance_profile_test.go b/test/suites/integration/instance_profile_test.go index f994e9864877..68deef309752 100644 --- a/test/suites/integration/instance_profile_test.go +++ b/test/suites/integration/instance_profile_test.go @@ -16,6 +16,7 @@ package integration_test import ( "fmt" + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" "time" "github.com/awslabs/operatorpkg/status" @@ -86,11 +87,13 @@ var _ = Describe("InstanceProfile Generation", func() { instance := env.GetInstance(node.Name) Expect(instance.IamInstanceProfile).ToNot(BeNil()) Expect(lo.FromPtr(instance.IamInstanceProfile.Arn)).To(ContainSubstring(nodeClass.Status.InstanceProfile)) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1beta1.ConditionTypeInstanceProfileReady, Status: metav1.ConditionTrue}) ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionTrue}) }) It("should have the EC2NodeClass status as not ready since Instance Profile was not resolved", func() { nodeClass.Spec.Role = fmt.Sprintf("KarpenterNodeRole-%s", "invalidRole") env.ExpectCreated(nodeClass) - ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionFalse, Message: "Failed to resolve instance profile"}) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1beta1.ConditionTypeInstanceProfileReady, Status: metav1.ConditionUnknown}) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionUnknown}) }) }) diff --git a/test/suites/integration/security_group_test.go b/test/suites/integration/security_group_test.go index ecb365ff7aca..d981fc6ad156 100644 --- a/test/suites/integration/security_group_test.go +++ b/test/suites/integration/security_group_test.go @@ -78,6 +78,7 @@ var _ = Describe("SecurityGroups", func() { It("should update the EC2NodeClass status security groups", func() { env.ExpectCreated(nodeClass) EventuallyExpectSecurityGroups(env, nodeClass) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1beta1.ConditionTypeSecurityGroupsReady, Status: metav1.ConditionTrue}) ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionTrue}) }) @@ -88,7 +89,8 @@ var _ = Describe("SecurityGroups", func() { }, } env.ExpectCreated(nodeClass) - ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionFalse, Message: "Failed to resolve security groups"}) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1beta1.ConditionTypeSecurityGroupsReady, Status: metav1.ConditionFalse, Message: "SecurityGroupSelector did not match any SecurityGroups"}) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionFalse, Message: "SecurityGroupsReady=False"}) }) }) diff --git a/test/suites/integration/subnet_test.go b/test/suites/integration/subnet_test.go index 7eacc0ef28ec..821a94ebdb29 100644 --- a/test/suites/integration/subnet_test.go +++ b/test/suites/integration/subnet_test.go @@ -125,6 +125,7 @@ var _ = Describe("Subnets", func() { It("should have the NodeClass status for subnets", func() { env.ExpectCreated(nodeClass) EventuallyExpectSubnets(env, nodeClass) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1beta1.ConditionTypeSubnetsReady, Status: metav1.ConditionTrue}) ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionTrue}) }) It("should have the NodeClass status as not ready since subnets were not resolved", func() { @@ -134,7 +135,8 @@ var _ = Describe("Subnets", func() { }, } env.ExpectCreated(nodeClass) - ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionFalse, Message: "Failed to resolve subnets"}) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1beta1.ConditionTypeSubnetsReady, Status: metav1.ConditionFalse, Message: "SubnetSelector did not match any Subnets"}) + ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: status.ConditionReady, Status: metav1.ConditionFalse, Message: "SubnetsReady=False"}) }) })