-
Notifications
You must be signed in to change notification settings - Fork 104
⚠️ Split Helm chart into operator and providers charts with optional dependency #832
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for kubernetes-sigs-cluster-api-operator ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2ee1de5
to
51174e4
Compare
Hey @kahirokunn, can you PTAL at failing tests? Thanks |
Thanks for taking a look! Before I dive into fixing the failing tests, I wanted to check if this feature/approach is something you'd be interested in merging. I don't want to spend time on test fixes if the overall direction isn't what the project needs. Would love to hear your thoughts on the concept first - any initial feedback would be super helpful! |
It makes sense in general, but providers chart seem to be a better fit as an operator chart dependency. This, depending on the configuration, may make change non-breaking |
18080c7
to
b54f2ad
Compare
b5a4c5c
to
679bf1f
Compare
@Danil-Grigorev I tried changing it, but how does it look? 👀 |
5b0b9a5
to
86cb427
Compare
86cb427
to
1d115b9
Compare
Separate cluster-api-operator into two charts to fix webhook timing issues: - cluster-api-operator: operator deployment only - cluster-api-operator-providers: provider Custom Resources with optional operator dependency This ensures webhook readiness before applying provider CRs, preventing "no endpoints available" errors during installation. The providers chart includes cluster-api-operator as a conditional dependency (install: true by default), maintaining backward compatibility while allowing flexible deployment scenarios: **Recommended: Two-step installation (no errors):** ```sh helm install capi-operator capi-operator/cluster-api-operator \ --create-namespace -n capi-operator-system --wait helm install capi-providers capi-operator/cluster-api-operator-providers \ -n capi-operator-system --set cluster-api-operator.install=false \ --set infrastructure.docker.enabled=true ``` **Backward compatibility: Single-step installation (may require retry):** ```sh helm install capi-providers capi-operator/cluster-api-operator-providers \ --create-namespace -n capi-operator-system \ --set infrastructure.docker.enabled=true helm upgrade --install capi-providers capi-operator/cluster-api-operator-providers \ -n capi-operator-system --set infrastructure.docker.enabled=true ``` Signed-off-by: kahirokunn <[email protected]>
Update hack/chart-update/main.go to process both cluster-api-operator and cluster-api-operator-providers charts when updating index.yaml. This ensures all charts are properly registered in the helm repository index during the release process. Signed-off-by: kahirokunn <[email protected]>
Providers can now define their own configSecret: core: cluster-api: configSecret: name: core-secret namespace: capi-system If not specified, providers will use the global configSecret. Signed-off-by: kahirokunn <[email protected]>
Add a new GitHub Actions workflow for smoke testing. Signed-off-by: kahirokunn <[email protected]>
1d115b9
to
486dade
Compare
…sues This commit fixes intermittent test failures in the helm e2e tests caused by inconsistent whitespace handling between Helm output and expected manifest files. Signed-off-by: kahirokunn <[email protected]>
/test pull-cluster-api-operator-test-main |
appVersion: "0.0.0" | ||
dependencies: | ||
- name: cluster-api-operator | ||
repository: file://../cluster-api-operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably won’t work outside of locally built chart. On the second thought, having charts separated would be simpler from release perspective
Fixes: #534
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR fixes the flaky Helm installation issue where provider Custom Resources fail to install due to webhook validation errors. The root cause is that provider CRs are being applied at the same time as the operator deployment, before the webhook service is ready.
Problem:
When installing the cluster-api-operator Helm chart, users frequently encounter errors like:
Solution:
Split the Helm chart into two separate charts:
cluster-api-operator
- Contains only the operator deployment and its resources (providers removed)cluster-api-operator-providers
- Contains all provider Custom Resources with optional operator dependencyWhich issue(s) this PR fixes:
Fixes #534
Special notes for your reviewer:
cluster-api-operator
chart no longer includes provider CRs. Users must now use thecluster-api-operator-providers
chart to install providers.For new users - Two-step installation (recommended, no errors):
Single-step installation (backward compatibility, may require retry):
helm install capi-providers capi-operator/cluster-api-operator-providers \ --create-namespace -n capi-operator-system \ --set infrastructure.docker.enabled=true # If above fails, retry with upgrade helm upgrade --install capi-providers capi-operator/cluster-api-operator-providers \ -n capi-operator-system --set infrastructure.docker.enabled=true
Release note: