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

Can't upgrade karpenter-crd from 1.0.8 -> 1.1.1 #7619

Open
raidlman opened this issue Jan 21, 2025 · 1 comment
Open

Can't upgrade karpenter-crd from 1.0.8 -> 1.1.1 #7619

raidlman opened this issue Jan 21, 2025 · 1 comment
Labels
bug Something isn't working needs-triage Issues that need to be triaged

Comments

@raidlman
Copy link

Description

Observed Behavior:

When trying to upgrade from karpenter-crd from 1.0.8 to 1.1.1 I get the following error messages:

CustomResourceDefinition.apiextensions.k8s.io "ec2nodeclasses.karpenter.k8s.aws" is invalid: [spec.conversion.strategy: Required value, spec.conversion.webhookClientConfig: Forbidden: should not be set when strategy is not set to Webhook]
CustomResourceDefinition.apiextensions.k8s.io "nodepools.karpenter.sh" is invalid: [spec.conversion.strategy: Required value, spec.conversion.webhookClientConfig: Forbidden: should not be set when strategy is not set to Webhook]
CustomResourceDefinition.apiextensions.k8s.io "nodeclaims.karpenter.sh" is invalid: [spec.conversion.strategy: Required value, spec.conversion.webhookClientConfig: Forbidden: should not be set when strategy is not set to Webhook]

We're using ArgoCD to manage the installation. I can see, that in the desired chart still is spec.conversion.webhookClientConfig.caBundle and spec.conversion.webhookClientConfig.service.path present.

Expected Behavior:

Since webhooks were removed in the last release, there shouldn't be any dependencies left. spec.conversion.webhookClientConfig.caBundle and spec.conversion.webhookClientConfig.service.path should be removed (I guess).

Reproduction Steps (Please include YAML):

Versions:

  • Chart Version: 1.1.1
  • Kubernetes Version (kubectl version):
Client Version: v1.31.3
Kustomize Version: v5.4.2
Server Version: v1.31.4-eks-2d5f260
  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@raidlman raidlman added bug Something isn't working needs-triage Issues that need to be triaged labels Jan 21, 2025
@jortkoopmans
Copy link

jortkoopmans commented Jan 21, 2025

I have seen this too, the workaround for me was to perform a replace sync. Slight difference from your case, I use the karpenter-crds chart (where the same error occurs) separately from the karpenter installation and I was coming from v1.0.6. Note that I normally use argocd serversideapply on these crds (this might matter for the argocd merge strategy).

I'm assuming that the cause of the issue is that the previously used webhook for doing the v1beta1 resource conversion, which argocd is seeing on the crd, is going through argocd merge, resulting in argocd (partially?) preserving the conversion block which is invalid. Of course karpenter then does not have the webhook endpoint anymore either on v1.1. But to be clear, the 1.1 crds are not having the conversion.webhook (you can check the chart files). Therefore it's perhaps more of an ArgoCD issue, than Karpenter.
EDIT: the most likely culprits are the injected caBundle and service.path.

Originally (<1.1), this originates from the chart snippet that only adds that to the CRD based on the chart settings;

{{- if .Values.webhook.enabled }} 
  conversion:
    strategy: Webhook
    webhook:
      conversionReviewVersions:
        - v1beta1
        - v1
      clientConfig:
        service:
          name: {{ .Values.webhook.serviceName }}
          namespace: {{ .Values.webhook.serviceNamespace | default .Release.Namespace }}
          port: {{ .Values.webhook.port }}
{{- end }}

I have not tried this, but perhaps you could disable the webhook on the old chart, before performing the upgrade (or just use replace like I did). Hope it helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-triage Issues that need to be triaged
Projects
None yet
Development

No branches or pull requests

2 participants