Skip to content

Commit

Permalink
chore: don't emit disruption events for non-karpenter nodes (#1644)
Browse files Browse the repository at this point in the history
  • Loading branch information
njtran authored Sep 5, 2024
1 parent 75fcd2a commit 140d35b
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
4 changes: 2 additions & 2 deletions pkg/controllers/disruption/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1492,7 +1492,7 @@ var _ = Describe("Candidate Filtering", func() {
Expect(cluster.Nodes()).To(HaveLen(1))
_, err := disruption.NewCandidate(ctx, env.Client, recorder, fakeClock, cluster.Nodes()[0], pdbLimits, nodePoolMap, nodePoolInstanceTypeMap, queue, disruption.GracefulDisruptionClass)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal("state node doesn't contain both a node and a nodeclaim"))
Expect(err.Error()).To(Equal("state node does not have a nodeclaim representation"))
})
It("should not consider candidate that has just a NodeClaim representation", func() {
nodeClaim, _ := test.NodeClaimAndNode(v1.NodeClaim{
Expand All @@ -1511,7 +1511,7 @@ var _ = Describe("Candidate Filtering", func() {
Expect(cluster.Nodes()).To(HaveLen(1))
_, err := disruption.NewCandidate(ctx, env.Client, recorder, fakeClock, cluster.Nodes()[0], pdbLimits, nodePoolMap, nodePoolInstanceTypeMap, queue, disruption.GracefulDisruptionClass)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Equal("state node doesn't contain both a node and a nodeclaim"))
Expect(err.Error()).To(Equal("state node does not have a node representation"))
})
It("should not consider candidates that are nominated", func() {
nodeClaim, node := test.NodeClaimAndNode(v1.NodeClaim{
Expand Down
5 changes: 4 additions & 1 deletion pkg/controllers/disruption/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ func NewCandidate(ctx context.Context, kubeClient client.Client, recorder events
var err error
var pods []*corev1.Pod
if err = node.ValidateNodeDisruptable(ctx, kubeClient); err != nil {
recorder.Publish(disruptionevents.Blocked(node.Node, node.NodeClaim, err.Error())...)
// Only emit an event if the NodeClaim is not nil, ensuring that we only emit events for Karpenter-managed nodes
if node.NodeClaim != nil {
recorder.Publish(disruptionevents.Blocked(node.Node, node.NodeClaim, err.Error())...)
}
return nil, err
}
// If the orchestration queue is already considering a candidate we want to disrupt, don't consider it a candidate.
Expand Down
7 changes: 5 additions & 2 deletions pkg/controllers/state/statenode.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,11 @@ func (in *StateNode) Pods(ctx context.Context, kubeClient client.Client) ([]*cor
//
//nolint:gocyclo
func (in *StateNode) ValidateNodeDisruptable(ctx context.Context, kubeClient client.Client) error {
if in.Node == nil || in.NodeClaim == nil {
return fmt.Errorf("state node doesn't contain both a node and a nodeclaim")
if in.NodeClaim == nil {
return fmt.Errorf("state node does not have a nodeclaim representation")
}
if in.Node == nil {
return fmt.Errorf("state node does not have a node representation")
}
if !in.Initialized() {
return fmt.Errorf("state node isn't initialized")
Expand Down

0 comments on commit 140d35b

Please sign in to comment.