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: Mitigate breaking changes to VPA upgrad v2.x #332

Closed
wants to merge 4 commits into from

Conversation

neelaruban
Copy link
Contributor

@neelaruban neelaruban commented Dec 12, 2023

What does this PR do?

🛑 Please open an issue first to discuss any significant work and flesh out details/direction - we would hate for your time to be wasted.
Consult the CONTRIBUTING guide for submitting pull-requests.

Motivation

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I ran pre-commit run -a with this PR

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

@neelaruban neelaruban requested a review from a team as a code owner December 12, 2023 06:24
@bryantbiggs
Copy link
Contributor

how has this been validated, are there any schema changes for the chart?

@neelaruban
Copy link
Contributor Author

how has this been validated, are there any schema changes for the chart?

the breaking changes introduced will affect existing implementation only during when the intent is to migrate seamlessly without any outages

the new version of the chart introduces new set of default values -

The certificate creation process was changed from using OpenSSL to kube-webhook-certgen to simplify the process. It still uses the same configuration keys (.Values.admissionController.certGen), which makes it impossible to reuse the values from a previous install.

You can mitigate this change by setting the correct image for the upgrade:

helm upgrade <release name> fairwinds-stable/vpa --version 2.0.0 --reuse-values \
  --set "admissionController.certGen.image.repository=registry.k8s.io/ingress-nginx/kube-webhook-certgen" \
  --set "admissionController.certGen.image.tag=v20230312-helm-chart-4.5.2-28-g66a760794"

these new values should be installed when it is installed fresh unless you are trying to do a in place replacement which should be done as shown above .

Furthermore, Previously, the webhook creation was handled by the admission controller itself. This had the downside that Helm is not in control of the resource and therefore required the cleanupOnDelete job.

This version disables the self-registration by the admission controller and creates the MutatingWebhookconfiguration using Helm.

You can either:

  • Migrate the MutatingWebhookconfiguration by:

    • adding the label app.kubernetes.io/managed-by: Helm
    • adding the annotation meta.helm.sh/release-name: <release name>
    • adding the annotation meta.helm.sh/release-namespace: <release namespace>
  • delete the configuration and it will be recreated by Helm

  • or keep the configuration as it is and Helm will ignore it. Execute the tests, to make sure everything works.

Also, the cleanupOnDelete configuration is obsolete.

Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Jan 17, 2024
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update VPA addon to v2.x
4 participants