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

[Karpenter Add-on][BREAKING CHANGE] Update for v0.34.0 #940

Merged
merged 18 commits into from
Mar 11, 2024

Conversation

youngjeong46
Copy link
Collaborator

@youngjeong46 youngjeong46 commented Mar 1, 2024

Issue #, if available: #924

Description of changes:
This PR changes the Karpenter add-on to support up to v0.34.0 (v0.35.0 just released as of the day of the PR, so it will be following as another PR). This is considered a breaking change:

  • Refactored properties for the add-on to better reflect the parameters needed for the NodePool and the EC2NodeClass.
  • Ability to deploy the CRDs based on the versions: v0.32.x and up will enforce v1beta1 CRDs (NodePool and EC2NodeClass), while below versions will enforce v1alpha5 CRDs (Provisioner and AWSNodeTemplate)
  • Both parameters are optional to be used for either the alpha or the beta CRDS. However, there are error checks implemented in case the wrong one is used (i.e. using consolidation with v1beta1 CRDs or disruption with v1alpha5 CRDs)
  • Addon will no longer support any versions below v0.21.x, as provided in the compatibility matrix from the official website here. Version Map is used to issue a warning if incompatible version is used for the Kubernetes version, but will proceed with deployment.
  • Additional unit tests
  • Doc revisions

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@elamaran11 elamaran11 linked an issue Mar 1, 2024 that may be closed by this pull request
1 task
@elamaran11
Copy link
Collaborator

@youngjeong46 Nice PR, lof of work. Does this also fix #859 and #893 Issues?

@youngjeong46
Copy link
Collaborator Author

@youngjeong46 Nice PR, lof of work. Does this also fix #859 and #893 Issues?

No, not yet. I'm trying to figure out the exact ask (at least for 893). Given this PR, it would most likely be a fast follow (along with 0.35 update that came out last night).

Copy link
Collaborator

@shapirov103 shapirov103 left a comment

Choose a reason for hiding this comment

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

@youngjeong46 tremendous work, a lot of changes and new features.
Please see my review comments. I will then kick off a test on 1.28. The move the default EKS version to 1.29 in a separate PR and retest before release on 1.29.
Great work!

docs/addons/karpenter.md Outdated Show resolved Hide resolved
docs/addons/karpenter.md Outdated Show resolved Hide resolved
docs/addons/karpenter.md Outdated Show resolved Hide resolved
docs/addons/karpenter.md Outdated Show resolved Hide resolved
docs/addons/karpenter.md Outdated Show resolved Hide resolved
lib/addons/karpenter/index.ts Outdated Show resolved Hide resolved
lib/addons/karpenter/index.ts Outdated Show resolved Hide resolved
lib/addons/karpenter/index.ts Outdated Show resolved Hide resolved
lib/builders/windows-builder.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@shapirov103 shapirov103 left a comment

Choose a reason for hiding this comment

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

See my last comment, I will kick off e2e once addressed since code changes invalidate the e2e approval.

lib/spi/types.ts Outdated Show resolved Hide resolved
lib/spi/types.ts Outdated Show resolved Hide resolved
@shapirov103
Copy link
Collaborator

/do-e2e-tests

Copy link

@aws-ia-ci aws-ia-ci left a comment

Choose a reason for hiding this comment

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

end to end tests failed. A maintainer can provide more details.

@shapirov103
Copy link
Collaborator

@youngjeong46 getting failure on jest test:

FAIL test/karpenter.test.ts

214 | ● Test suite failed to run
215 |  
216 | test/karpenter.test.ts:7:10 - error TS2724: '"../lib/spi"' has no exported member named 'reqValues'. Did you mean 'Values'?
217 |  
218 | 7 import { reqValues } from "../lib/spi";

@shapirov103
Copy link
Collaborator

/do-e2e-tests

Copy link

@aws-ia-ci aws-ia-ci left a comment

Choose a reason for hiding this comment

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

end to end tests passed

Copy link
Collaborator

@shapirov103 shapirov103 left a comment

Choose a reason for hiding this comment

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

LGTM

@shapirov103 shapirov103 merged commit 247aaf2 into main Mar 11, 2024
1 of 2 checks passed
@shapirov103 shapirov103 deleted the feat/karpenter-0.34 branch March 11, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Karpenter] Update for 0.34 Beta CRDs
5 participants