Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: reallowing nodeclaim zones to fix node/nodeclaim creations for workloads with zone affinity constraints #340

Merged
merged 1 commit into from
May 11, 2024

Conversation

comtalyst
Copy link
Collaborator

@comtalyst comtalyst commented May 11, 2024

Fixes #339

Description
See the issue.
This change should not undo the continuous drift issue that #274 fixes, as adding zones requirements is still prohibited on the nodepool CRD, which is the (intentionally) user-facing interface.
Current drift logic also "ignore" requirements that exist in NodeClaims, but not NodePools.

How was this change tested?

  • Manual inspection from reproduction
  • NAP E2Es
  • Check-ins here

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note

- Fix the issue where Karpenter/NAP sometimes cannot create nodes/nodeclaims for workloads with zone affinity constraints

@comtalyst comtalyst requested a review from tallaxes May 11, 2024 00:40
@coveralls
Copy link

Pull Request Test Coverage Report for Build 9039702148

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.774%

Totals Coverage Status
Change from base Build 8994810202: 0.0%
Covered Lines: 36279
Relevant Lines: 37105

💛 - Coveralls

Copy link
Collaborator Author

@comtalyst comtalyst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/test

Copy link
Collaborator

@tallaxes tallaxes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

@tallaxes tallaxes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (pending passing tests)

@tallaxes tallaxes merged commit cd89504 into main May 11, 2024
19 checks passed
@tallaxes tallaxes deleted the comtalyst/fix-zone-label-not-allowed-on-nodeclaims branch May 11, 2024 16:45
@tallaxes tallaxes added area/provisioning Issues or PRs related to provisioning (instance provider) area/availability-zones Issues or PRs related to availability zones area/api Issues or PRs related to the APIs labels May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Issues or PRs related to the APIs area/availability-zones Issues or PRs related to availability zones area/provisioning Issues or PRs related to provisioning (instance provider)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Karpenter sometimes cannot create nodes/NodeClaims for workloads with zone affinity constraints
3 participants