kubernetes-sigs / karpenter

Karpenter is a Kubernetes Node Autoscaler built for flexibility, performance, and simplicity.
Apache License 2.0
637 stars 206 forks source link

fix: dedupe expiration reconciliations #1794

Closed jmdeal closed 3 weeks ago

jmdeal commented 3 weeks ago

Fixes #N/A

Description Currently, the expiration controller reconciles against all NodeClaim updates, and doesn't have a check to ensure that the NodeClaim hasn't already been terminated. This can result in duplicate expiration attempts against the same NodeClaim. The main problem with this (other than unnecessary deletes) is that we increment the expiration metric each time.

This change ensures we don't double count expirations through two mechanisms: a. Check that the NodeClaim's deletion timestamp is zero before proceeding with expiration. b. Only enqueue requests for NodeClaims when they are first added to the informer cache.

How was this change tested? make test

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

coveralls commented 3 weeks ago

Pull Request Test Coverage Report for Build 11619365060

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/controllers/nodeclaim/expiration/controller.go 7 9 77.78%
<!-- Total: 7 9 77.78% -->
Totals Coverage Status
Change from base Build 11603073442: 0.08%
Covered Lines: 8508
Relevant Lines: 10511

💛 - Coveralls
k8s-ci-robot commented 3 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jmdeal, njtran

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/karpenter/blob/main/OWNERS)~~ [njtran] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment