Skip to content

Commit

Permalink
Merge branch 'main' into add-unknown-ready-condition
Browse files Browse the repository at this point in the history
  • Loading branch information
engedaam authored Dec 11, 2024
2 parents 1f01c4c + ed92913 commit 1a198ed
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 25 deletions.
30 changes: 26 additions & 4 deletions .github/workflows/stale.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,39 @@ jobs:
if: github.repository == 'aws/karpenter-provider-aws'
name: Stale issue bot
steps:
# PR stale-out
- uses: actions/stale@28ca1036281a5e5922ead5184a1bbf96e5fc984e # v9.0.0
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
stale-issue-message: 'This issue has been inactive for 14 days. StaleBot will close this stale issue after 14 more days of inactivity.'
exempt-issue-labels: 'bug,chore,feature,documentation,testing,operational-excellence,automation,roadmap'
stale-issue-label: 'lifecycle/stale'
close-issue-label: 'lifecycle/closed'
only-issue-labels: 'ignore' # Ignore this step for Issues
stale-pr-message: 'This PR has been inactive for 14 days. StaleBot will close this stale PR after 14 more days of inactivity.'
exempt-pr-labels: 'blocked,needs-review,needs-design'
stale-pr-label: 'lifecycle/stale'
close-pr-label: 'lifecycle/closed'
days-before-stale: 14
days-before-close: 14
operations-per-run: 300
# Issue stale-out for "triage/needs-information"
- uses: actions/stale@28ca1036281a5e5922ead5184a1bbf96e5fc984e # v9.0.0
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
stale-issue-message: 'This issue has been inactive for 14 days. StaleBot will close this stale issue after 14 more days of inactivity.'
only-issue-labels: 'triage/needs-information'
stale-issue-label: 'lifecycle/stale'
close-issue-label: 'lifecycle/closed'
only-pr-labels: 'ignore' # Ignore this step for PRs
days-before-stale: 14
days-before-close: 14
operations-per-run: 300
# Issue stale-out for "triage/solved"
- uses: actions/stale@28ca1036281a5e5922ead5184a1bbf96e5fc984e # v9.0.0
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
stale-issue-message: 'This issue has been inactive for 7 days and is marked as "triage/solved". StaleBot will close this stale issue after 7 more days of inactivity.'
only-issue-labels: 'triage/solved'
stale-issue-label: 'lifecycle/stale'
close-issue-label: 'lifecycle/closed'
only-pr-labels: 'ignore' # Ignore this step for PRs
days-before-stale: 7
days-before-close: 7
operations-per-run: 300
1 change: 0 additions & 1 deletion pkg/apis/v1/ec2nodeclass_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const (
ConditionTypeSubnetsReady = "SubnetsReady"
ConditionTypeSecurityGroupsReady = "SecurityGroupsReady"
ConditionTypeAMIsReady = "AMIsReady"
ConditionTypeAMIsDeprecated = "AMIsDeprecated"
ConditionTypeInstanceProfileReady = "InstanceProfileReady"
)

Expand Down
11 changes: 0 additions & 11 deletions pkg/controllers/nodeclass/status/ami.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,6 @@ func (a *AMI) Reconcile(ctx context.Context, nodeClass *v1.EC2NodeClass) (reconc
}
})

// If deprecated AMIs are discovered set the AMIsDeprecated status condition
// If no deprecated AMIs are present, and previous status condition for AMIsDeprecated exists, remove the condition
hasDeprecatedAMIs := lo.Filter(nodeClass.Status.AMIs, func(ami v1.AMI, _ int) bool {
return ami.Deprecated
})
hasDeprecatedCondition := nodeClass.StatusConditions().Get(v1.ConditionTypeAMIsDeprecated) != nil
if len(hasDeprecatedAMIs) > 0 {
nodeClass.StatusConditions().SetTrue(v1.ConditionTypeAMIsDeprecated)
} else if hasDeprecatedCondition {
_ = nodeClass.StatusConditions().Clear(v1.ConditionTypeAMIsDeprecated)
}
nodeClass.StatusConditions().SetTrue(v1.ConditionTypeAMIsReady)
return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil
}
3 changes: 0 additions & 3 deletions pkg/controllers/nodeclass/status/ami_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,6 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
},
},
))
Expect(nodeClass.StatusConditions().IsTrue(v1.ConditionTypeAMIsDeprecated)).To(BeTrue())
Expect(nodeClass.StatusConditions().IsTrue(v1.ConditionTypeAMIsReady)).To(BeTrue())
})
It("should remove AMIDeprecated status condition when non deprecated AMIs are discovered", func() {
Expand Down Expand Up @@ -678,7 +677,6 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
},
))
// Checks if both AMIsReady and AMIsDeprecated status conditions are set
Expect(nodeClass.StatusConditions().IsTrue(v1.ConditionTypeAMIsDeprecated)).To(BeTrue())
Expect(nodeClass.StatusConditions().IsTrue(v1.ConditionTypeAMIsReady)).To(BeTrue())

// rediscover AMIs again and reconcile
Expand Down Expand Up @@ -740,7 +738,6 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
},
))
// Since all AMIs discovered are non deprecated, the status conditions should remove AMIsDeprecated and only set AMIsReady
Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeAMIsDeprecated)).To(BeNil())
Expect(nodeClass.StatusConditions().IsTrue(v1.ConditionTypeAMIsReady)).To(BeTrue())
})
})
Expand Down
1 change: 0 additions & 1 deletion test/suites/ami/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ var _ = Describe("AMI", func() {
Expect(len(nc.Status.AMIs)).To(BeNumerically("==", 1))
Expect(nc.Status.AMIs[0].Deprecated).To(BeTrue())
ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1.ConditionTypeAMIsReady, Status: metav1.ConditionTrue})
ExpectStatusConditions(env, env.Client, 1*time.Minute, nodeClass, status.Condition{Type: v1.ConditionTypeAMIsDeprecated, Status: metav1.ConditionTrue})
})
It("should prioritize launch with non-deprecated AMIs", func() {
nodeClass.Spec.AMIFamily = lo.ToPtr(v1.AMIFamilyAL2023)
Expand Down
5 changes: 2 additions & 3 deletions test/suites/drift/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,8 @@ var _ = Describe("Drift", func() {

By("validating the deprecated status condition has propagated")
Eventually(func(g Gomega) {
g.Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeAMIsDeprecated).IsTrue()).To(BeTrue())
g.Expect(env.Client.Get(env.Context, client.ObjectKeyFromObject(nodeClass), nodeClass)).Should(Succeed())
g.Expect(nodeClass.Status.AMIs[0].Deprecated).To(BeTrue())
g.Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeAMIsReady).IsTrue()).To(BeTrue())
}).Should(Succeed())

Expand All @@ -397,10 +398,8 @@ var _ = Describe("Drift", func() {
pod = env.EventuallyExpectHealthyPodCount(selector, numPods)[0]
env.ExpectInstance(pod.Spec.NodeName).To(HaveField("ImageId", HaveValue(Equal(amdAMI))))

By("validating the deprecated status condition has been removed")
Eventually(func(g Gomega) {
g.Expect(env.Client.Get(env.Context, client.ObjectKeyFromObject(nodeClass), nodeClass)).Should(Succeed())
g.Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeAMIsDeprecated)).To(BeNil())
g.Expect(nodeClass.StatusConditions().Get(v1.ConditionTypeAMIsReady).IsTrue()).To(BeTrue())
}).Should(Succeed())
})
Expand Down
3 changes: 1 addition & 2 deletions website/content/en/preview/concepts/nodeclasses.md
Original file line number Diff line number Diff line change
Expand Up @@ -1566,8 +1566,7 @@ NodeClasses have the following status conditions:
| SubnetsReady | Subnets are discovered. |
| SecurityGroupsReady | Security Groups are discovered. |
| InstanceProfileReady | Instance Profile is discovered. |
| AMIsReady | AMIs are discovered. |
| AMIsDeprecated | AMIs are discovered, but they are deprecated. Individual deprecated AMIs can be identified by reviewing the `status.amis`. |
| AMIsReady | AMIs are discovered. |
| Ready | Top level condition that indicates if the nodeClass is ready. If any of the underlying conditions is `False` then this condition is set to `False` and `Message` on the condition indicates the dependency that was not resolved. |

If a NodeClass is not ready, NodePools that reference it through their `nodeClassRef` will not be considered for scheduling.

0 comments on commit 1a198ed

Please sign in to comment.