-
Notifications
You must be signed in to change notification settings - Fork 22
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
CAPX clusterclass support #344
CAPX clusterclass support #344
Conversation
8594642
to
a0dab43
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #344 +/- ##
==========================================
+ Coverage 15.14% 15.21% +0.07%
==========================================
Files 17 18 +1
Lines 1208 1209 +1
==========================================
+ Hits 183 184 +1
Misses 1025 1025 ☔ View full report in Codecov by Sentry. |
Please note the TODO list in description. while adding e2e tests, it was discovered that we need to reorg the templates folder yaml with respect to bases else it casues issues in generate e2e templates. working on the same base - Cluster without topology - KubeadmControlPlane - KubeadmConfigTemplate - NutanixCluster - NutanixMachineTemplate - MachineDeployment - ConfigMap - Secret Overlay: Cluster class - Template patches - Add ClusterClass - Add KubeadmControlPlaneTemplate - Update Cluster with topology - Kustomize - Include everything from base - Drop NutanixCluster - Drop MachineDeployment - Drop KubeadmControlPlane |
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.
Thank you @deepakm-ntnx for this! Looking forward to getting this merged and CAPX supporting ClusterTopology!
The big thing that stands out is that the ClusterClass
definition and associated templates should be using variables and patches, rather than clusterctl
env var templating. This will allow configuration to be specified per-cluster rather than embedding in the CC at time of applying the template.
Oops sorry @deepakm-ntnx I didn't see the TODO about the clusterclass variables and patches and commented 🫣 |
415721e
to
fcaa087
Compare
50233dc
to
e188070
Compare
For anyone exercising this in a shared PC: Your cluster name must be unique. Here, it's determined by the |
7347c8f
to
c84bc1a
Compare
c84bc1a
to
e93fdd1
Compare
6720df1
to
a6f1a56
Compare
have kept the go/v3 kuberbuilder scffolding as is so reduce the code churn
this is done to improve maintainability
This is required as currently ccm-update.yaml vars are not getting replaced thru code.
3e3c76d
to
31a01eb
Compare
/retest |
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adiantum, deepakm-ntnx, dkoshkin, thunderboltsid The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
d5cf036
into
nutanix-cloud-native:main
What this PR does / why we need it:
This PR adds support for clusterclass in CAPX. More indepth details on need for clusterclass can be found here https://cluster-api.sigs.k8s.io/tasks/experimental-features/cluster-class/
have kept the go/v3 kuberbuilder scffolding as is to reduce the code churn
Done
Dev Steps:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
How Has This Been Tested?:
Currently its tested manually as mentioned above dev steps.
local test passed
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration and test output
Special notes for your reviewer:
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
Release note: