aws-quickstart / cdk-eks-blueprints

AWS Quick Start Team
Apache License 2.0
429 stars 190 forks source link

Bugfix -- add in the Beta1 CRD's on upgrade to v0.32 or higher. #963

Open jsamuel1 opened 3 months ago

jsamuel1 commented 3 months ago

Issue #, if available: Fixes Issue #962 Description of changes: Install the CRD's for Karpenter if installing addon 0.32 or higher.
This prevents upgrade failures if original install of the Helm chart was lower than 0.32.

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

jsamuel1 commented 3 months ago

Question for reviewers:

shapirov103 commented 2 months ago

@jsamuel1 thank you for the PR. We are validating the upgrade path and planning thus to be part of the follow-up patch release (potentially 1.14.2).

youngjeong46 commented 2 months ago

@jsamuel1 Thanks for raising the PR. Here are my initial thoughts on your questions (@shapirov103 please share your thoughts as well)

  • Are there cases where a user would not want the CRDs to be installed/managed by the addOn? (If yes, how would be give a clean way to opt-out?)

We have customers that have their own addon extensions for CRDs and CRD resources, and manage them through GitOps. We can provide them an opt out by using a flag to enable/disable CRD installations, and document that if they disable, they need to manage the CRD lifecycle.

  • Internet download of CRDs as published against the version, vs taking a point in time copy to put in this repo.

We tend to avoid doing this, as we want to leave the CRD management to Karpenter.

jsamuel1 commented 2 months ago

@jsamuel1 Thanks for raising the PR. Here are my initial thoughts on your questions (@shapirov103 please share your thoughts as well)

  • Are there cases where a user would not want the CRDs to be installed/managed by the addOn? (If yes, how would be give a clean way to opt-out?)

We have customers that have their own addon extensions for CRDs and CRD resources, and manage them through GitOps. We can provide them an opt out by using a flag to enable/disable CRD installations, and document that if they disable, they need to manage the CRD lifecycle.

created a new option manageCRDs, which defaults false.
Users should therefore opt-in if they are upgrading and require new CRDs.

  • Internet download of CRDs as published against the version, vs taking a point in time copy to put in this repo.

We tend to avoid doing this, as we want to leave the CRD management to Karpenter.

shapirov103 commented 1 month ago

@youngjeong46 please check the latest update with the opt-in support at the addon level. @jsamuel1 please resolve the conflict in the test.

jsamuel1 commented 1 month ago

@youngjeong46 please check the latest update with the opt-in support at the addon level. @jsamuel1 please resolve the conflict in the test.

Updated to resolve conflicts and to ensure test is correct after adding the installCRDs option.