Skip to content

Commit

Permalink
chore: Add status conditions on nodeClass
Browse files Browse the repository at this point in the history
  • Loading branch information
jigisha620 committed Jul 8, 2024
1 parent e674d28 commit eb019db
Show file tree
Hide file tree
Showing 17 changed files with 92 additions and 49 deletions.
29 changes: 18 additions & 11 deletions pkg/apis/v1/ec2nodeclass_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
14 changes: 13 additions & 1 deletion pkg/apis/v1beta1/ec2nodeclass_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions pkg/controllers/nodeclass/status/ami.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
10 changes: 10 additions & 0 deletions pkg/controllers/nodeclass/status/ami_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand All @@ -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())
})
})
2 changes: 1 addition & 1 deletion pkg/controllers/nodeclass/status/instanceprofile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
9 changes: 9 additions & 0 deletions pkg/controllers/nodeclass/status/instanceprofile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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 = ""
Expand All @@ -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 = ""
Expand All @@ -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())
})
})
5 changes: 5 additions & 0 deletions pkg/controllers/nodeclass/status/launchtemplate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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())
})
})
17 changes: 0 additions & 17 deletions pkg/controllers/nodeclass/status/readiness.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
4 changes: 2 additions & 2 deletions pkg/controllers/nodeclass/status/readiness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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"))
})
})
2 changes: 2 additions & 0 deletions pkg/controllers/nodeclass/status/securitygroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
13 changes: 7 additions & 6 deletions pkg/controllers/nodeclass/status/securitygroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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())
})
})
3 changes: 2 additions & 1 deletion pkg/controllers/nodeclass/status/subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Loading

0 comments on commit eb019db

Please sign in to comment.