Skip to content

Commit

Permalink
BREAKING: Add operatorpkg status conditions (#6180)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan-innis authored May 14, 2024
1 parent 47cc587 commit 2ace691
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 37 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
github.com/aws/aws-sdk-go v1.53.0
github.com/aws/karpenter-provider-aws/tools/kompat v0.0.0-20240410220356-6b868db24881
github.com/awslabs/amazon-eks-ami/nodeadm v0.0.0-20240229193347-cfab22a10647
github.com/awslabs/operatorpkg v0.0.0-20240502203521-a2115dcf4ac0
github.com/awslabs/operatorpkg v0.0.0-20240509171849-455f977cee10
github.com/go-logr/zapr v1.3.0
github.com/imdario/mergo v0.3.16
github.com/mitchellh/hashstructure/v2 v2.0.2
Expand All @@ -29,7 +29,7 @@ require (
k8s.io/utils v0.0.0-20240102154912-e7106e64919e
knative.dev/pkg v0.0.0-20231010144348-ca8c009405dd
sigs.k8s.io/controller-runtime v0.18.2
sigs.k8s.io/karpenter v0.36.1-0.20240510144555-8ed47ded76f6
sigs.k8s.io/karpenter v0.36.1-0.20240513160924-49ce67fb94eb
sigs.k8s.io/yaml v1.4.0
)

Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ github.com/aws/karpenter-provider-aws/tools/kompat v0.0.0-20240410220356-6b868db
github.com/aws/karpenter-provider-aws/tools/kompat v0.0.0-20240410220356-6b868db24881/go.mod h1:+Mk5k0b6HpKobxNq+B56DOhZ+I/NiPhd5MIBhQMSTSs=
github.com/awslabs/amazon-eks-ami/nodeadm v0.0.0-20240229193347-cfab22a10647 h1:8yRBVsjGmI7qQsPWtIrbWP+XfwHO9Wq7gdLVzjqiZFs=
github.com/awslabs/amazon-eks-ami/nodeadm v0.0.0-20240229193347-cfab22a10647/go.mod h1:9NafTAUHL0FlMeL6Cu5PXnMZ1q/LnC9X2emLXHsVbM8=
github.com/awslabs/operatorpkg v0.0.0-20240502203521-a2115dcf4ac0 h1:sLJ+JX6Yko4dUc5MfqwHGcC7yWQxgKwry1Nhh+bMw/E=
github.com/awslabs/operatorpkg v0.0.0-20240502203521-a2115dcf4ac0/go.mod h1:I7p/HTgsO8XwYbqBvtp37JMB0yFHrFSv3Pki4blv5HQ=
github.com/awslabs/operatorpkg v0.0.0-20240509171849-455f977cee10 h1:l9aKssdP1CaqzB/kRyMjYBgZxXnZlqYl0EhYT9iPBFY=
github.com/awslabs/operatorpkg v0.0.0-20240509171849-455f977cee10/go.mod h1:G4QVeP+gdcP8JtDDxelh58IlFbpKNJcS8/isVDMW2KY=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
Expand Down Expand Up @@ -761,8 +761,8 @@ sigs.k8s.io/controller-runtime v0.18.2 h1:RqVW6Kpeaji67CY5nPEfRz6ZfFMk0lWQlNrLql
sigs.k8s.io/controller-runtime v0.18.2/go.mod h1:tuAt1+wbVsXIT8lPtk5RURxqAnq7xkpv2Mhttslg7Hw=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
sigs.k8s.io/karpenter v0.36.1-0.20240510144555-8ed47ded76f6 h1:CRMU8LU/Fheac8loEbXR8Xmjnvi1Wa3zViN9CIIpo3o=
sigs.k8s.io/karpenter v0.36.1-0.20240510144555-8ed47ded76f6/go.mod h1:OQJz6dEZUXtaMspUemCRsNggUmIxeoUDfo2dPIuqUXM=
sigs.k8s.io/karpenter v0.36.1-0.20240513160924-49ce67fb94eb h1:5/97WIFCzlAHbpIHwARebNOQcCfCXnc/o6GmBXSxZTw=
sigs.k8s.io/karpenter v0.36.1-0.20240513160924-49ce67fb94eb/go.mod h1:R6JcpsnPV12yC0b/UxdlZB3rm1u0V403t/tmU+DzHJI=
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=
sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08=
sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
Expand Down
54 changes: 40 additions & 14 deletions pkg/apis/crds/karpenter.sh_nodeclaims.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -385,34 +385,60 @@ spec:
conditions:
description: Conditions contains signals for health and readiness
items:
description: |-
Condition defines a readiness condition for a Knative resource.
See: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties
description: Condition aliases the upstream type and adds additional helper methods
properties:
lastTransitionTime:
description: |-
LastTransitionTime is the last time the condition transitioned from one status to another.
We use VolatileTime in place of metav1.Time to exclude this from creating equality.Semantic
differences (all other things held constant).
lastTransitionTime is the last time the condition transitioned from one status to another.
This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable.
format: date-time
type: string
message:
description: A human readable message indicating details about the transition.
description: |-
message is a human readable message indicating details about the transition.
This may be an empty string.
maxLength: 32768
type: string
observedGeneration:
description: |-
observedGeneration represents the .metadata.generation that the condition was set based upon.
For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date
with respect to the current state of the instance.
format: int64
minimum: 0
type: integer
reason:
description: The reason for the condition's last transition.
type: string
severity:
description: |-
Severity with which to treat failures of this type of condition.
When this is not specified, it defaults to Error.
reason contains a programmatic identifier indicating the reason for the condition's last transition.
Producers of specific condition types may define expected values and meanings for this field,
and whether the values are considered a guaranteed API.
The value should be a CamelCase string.
This field may not be empty.
maxLength: 1024
minLength: 1
pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
type: string
status:
description: Status of the condition, one of True, False, Unknown.
description: status of the condition, one of True, False, Unknown.
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: Type of condition.
description: |-
type of condition in CamelCase or in foo.example.com/CamelCase.
---
Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be
useful (see .node.status.conditions), the ability to deconflict is important.
The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
maxLength: 316
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
type: string
required:
- lastTransitionTime
- message
- reason
- status
- type
type: object
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/v1beta1/ec2nodeclass_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ type EC2NodeClassStatus struct {
Conditions []op.Condition `json:"conditions,omitempty"`
}

var (
const (
// ConditionTypeNodeClassReady = "Ready" condition indicates that subnets, security groups, AMIs and instance profile for nodeClass were resolved
ConditionTypeNodeClassReady = "Ready"
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/nodeclass/hash/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (c *Controller) updateNodeClaimHash(ctx context.Context, nodeClass *v1beta1

// Any NodeClaim that is already drifted will remain drifted if the karpenter.k8s.aws/nodepool-hash-version doesn't match
// Since the hashing mechanism has changed we will not be able to determine if the drifted status of the NodeClaim has changed
if nc.StatusConditions().GetCondition(corev1beta1.Drifted) == nil {
if nc.StatusConditions().Get(corev1beta1.ConditionTypeDrifted) == nil {
nc.Annotations = lo.Assign(nc.Annotations, map[string]string{
v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash(),
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/nodeclass/hash/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ var _ = Describe("NodeClass Hash Controller", func() {
},
},
})
nodeClaim.StatusConditions().MarkTrue(corev1beta1.Drifted)
nodeClaim.StatusConditions().SetTrue(corev1beta1.ConditionTypeDrifted)
ExpectApplied(ctx, env.Client, nodeClass, nodeClaim)

ExpectReconcileSucceeded(ctx, hashController, client.ObjectKeyFromObject(nodeClass))
Expand Down
8 changes: 4 additions & 4 deletions test/pkg/debug/nodeclaim.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ func (c *NodeClaimController) Reconcile(ctx context.Context, req reconcile.Reque

func (c *NodeClaimController) GetInfo(nc *corev1beta1.NodeClaim) string {
return fmt.Sprintf("ready=%t launched=%t registered=%t initialized=%t",
nc.StatusConditions().IsHappy(),
nc.StatusConditions().GetCondition(corev1beta1.Launched).IsTrue(),
nc.StatusConditions().GetCondition(corev1beta1.Registered).IsTrue(),
nc.StatusConditions().GetCondition(corev1beta1.Initialized).IsTrue(),
nc.StatusConditions().Root().IsTrue(),
nc.StatusConditions().Get(corev1beta1.ConditionTypeLaunched).IsTrue(),
nc.StatusConditions().Get(corev1beta1.ConditionTypeRegistered).IsTrue(),
nc.StatusConditions().Get(corev1beta1.ConditionTypeInitialized).IsTrue(),
)
}

Expand Down
10 changes: 5 additions & 5 deletions test/pkg/environment/common/expectations.go
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ func (env *Environment) EventuallyExpectNodeClaimsReady(nodeClaims ...*corev1bet
for _, nc := range nodeClaims {
temp := &corev1beta1.NodeClaim{}
g.Expect(env.Client.Get(env.Context, client.ObjectKeyFromObject(nc), temp)).Should(Succeed())
g.Expect(temp.StatusConditions().IsHappy()).To(BeTrue())
g.Expect(temp.StatusConditions().Root().IsTrue()).To(BeTrue())
}
}).Should(Succeed())
}
Expand All @@ -684,7 +684,7 @@ func (env *Environment) EventuallyExpectExpired(nodeClaims ...*corev1beta1.NodeC
Eventually(func(g Gomega) {
for _, nc := range nodeClaims {
g.Expect(env.Client.Get(env, client.ObjectKeyFromObject(nc), nc)).To(Succeed())
g.Expect(nc.StatusConditions().GetCondition(corev1beta1.Expired).IsTrue()).To(BeTrue())
g.Expect(nc.StatusConditions().Get(corev1beta1.ConditionTypeExpired).IsTrue()).To(BeTrue())
}
}).Should(Succeed())
}
Expand All @@ -694,7 +694,7 @@ func (env *Environment) EventuallyExpectDrifted(nodeClaims ...*corev1beta1.NodeC
Eventually(func(g Gomega) {
for _, nc := range nodeClaims {
g.Expect(env.Client.Get(env, client.ObjectKeyFromObject(nc), nc)).To(Succeed())
g.Expect(nc.StatusConditions().GetCondition(corev1beta1.Drifted).IsTrue()).To(BeTrue())
g.Expect(nc.StatusConditions().Get(corev1beta1.ConditionTypeDrifted).IsTrue()).To(BeTrue())
}
}).Should(Succeed())
}
Expand All @@ -706,7 +706,7 @@ func (env *Environment) ConsistentlyExpectNodeClaimsNotDrifted(duration time.Dur
Consistently(func(g Gomega) {
for _, nc := range nodeClaims {
g.Expect(env.Client.Get(env, client.ObjectKeyFromObject(nc), nc)).To(Succeed())
g.Expect(nc.StatusConditions().GetCondition(corev1beta1.Drifted)).To(BeNil())
g.Expect(nc.StatusConditions().Get(corev1beta1.ConditionTypeDrifted)).To(BeNil())
}
}, duration).Should(Succeed())
}
Expand All @@ -716,7 +716,7 @@ func (env *Environment) EventuallyExpectEmpty(nodeClaims ...*corev1beta1.NodeCla
Eventually(func(g Gomega) {
for _, nc := range nodeClaims {
g.Expect(env.Client.Get(env, client.ObjectKeyFromObject(nc), nc)).To(Succeed())
g.Expect(nc.StatusConditions().GetCondition(corev1beta1.Empty).IsTrue()).To(BeTrue())
g.Expect(nc.StatusConditions().Get(corev1beta1.ConditionTypeEmpty).IsTrue()).To(BeTrue())
}
}).Should(Succeed())
}
Expand Down
4 changes: 2 additions & 2 deletions test/suites/drift/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,8 +705,8 @@ var _ = Describe("Drift", func() {
By("validating the drifted status condition has propagated")
Eventually(func(g Gomega) {
g.Expect(env.Client.Get(env, client.ObjectKeyFromObject(nodeClaim), nodeClaim)).To(Succeed())
g.Expect(nodeClaim.StatusConditions().GetCondition(corev1beta1.Drifted)).ToNot(BeNil())
g.Expect(nodeClaim.StatusConditions().GetCondition(corev1beta1.Drifted).IsTrue()).To(BeTrue())
g.Expect(nodeClaim.StatusConditions().Get(corev1beta1.ConditionTypeDrifted)).ToNot(BeNil())
g.Expect(nodeClaim.StatusConditions().Get(corev1beta1.ConditionTypeDrifted).IsTrue()).To(BeTrue())
}).Should(Succeed())

delete(pod.Annotations, corev1beta1.DoNotDisruptAnnotationKey)
Expand Down
6 changes: 3 additions & 3 deletions test/suites/nodeclaim/nodeclaim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,9 @@ var _ = Describe("StandaloneNodeClaim", func() {
Eventually(func(g Gomega) {
temp := &corev1beta1.NodeClaim{}
g.Expect(env.Client.Get(env.Context, client.ObjectKeyFromObject(nodeClaim), temp)).To(Succeed())
g.Expect(temp.StatusConditions().GetCondition(corev1beta1.Launched).IsTrue()).To(BeTrue())
g.Expect(temp.StatusConditions().GetCondition(corev1beta1.Registered).IsFalse()).To(BeTrue())
g.Expect(temp.StatusConditions().GetCondition(corev1beta1.Initialized).IsFalse()).To(BeTrue())
g.Expect(temp.StatusConditions().Get(corev1beta1.ConditionTypeLaunched).IsTrue()).To(BeTrue())
g.Expect(temp.StatusConditions().Get(corev1beta1.ConditionTypeRegistered).IsFalse()).To(BeTrue())
g.Expect(temp.StatusConditions().Get(corev1beta1.ConditionTypeInitialized).IsFalse()).To(BeTrue())
}).Should(Succeed())

// Expect that the nodeClaim is eventually de-provisioned due to the registration timeout
Expand Down
1 change: 1 addition & 0 deletions website/content/en/preview/upgrading/upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ WHEN CREATING A NEW SECTION OF THE UPGRADE GUIDANCE FOR NEWER VERSIONS, ENSURE T
### Upgrading to `0.37.0`+

* Karpenter updated the NodeClass controller naming in the following way: `nodeclass` -> `nodeclass.status`, `nodeclass.hash`, `nodeclass.termination`
* Karpenter's NodeClaim status conditions no longer include the `severity` field

### Upgrading to `0.36.0`+

Expand Down

0 comments on commit 2ace691

Please sign in to comment.