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

feat: Add support for CAPX #384

Closed
wants to merge 36 commits into from

Conversation

deepakm-ntnx
Copy link
Contributor

@deepakm-ntnx deepakm-ntnx commented Feb 20, 2024

to regenerate example Cluster with topology yamls

 make examples.sync

To run unit tests

 make test

To deploy cre controller as well as nutanix provider and cluster class of nutanix

make dev.run-on-kind 

To deploy nutanix cluster with topology with cre annotations

export KUBECONFIG=/go/src/github.com/deepakm-ntnx/capi-runtime-extensions/.local/kind/capi-runtime-extensions-dev/kubeconfig
clusterctl generate cluster nutanix-quick-start-helm-addon-cilium \
  --from examples/capi-quick-start/nutanix-cluster-cilium-helm-addon.yaml \
  --kubernetes-version v1.29.1 \
  --worker-machine-count 1 | \
  kubectl apply --server-side -f -

New clusterctl version available: v1.6.1 -> v1.6.2
sigs.k8s.io/cluster-api
Warning: Cluster refers to ClusterClass test-cre in the topology but it does not exist. Cluster topology has not been fully validated. The ClusterClass must be created to reconcile the Cluster
cluster.cluster.x-k8s.io/nutanix-quick-start-helm-addon-cilium serverside-applied

@deepakm-ntnx deepakm-ntnx marked this pull request as draft February 20, 2024 19:25
@deepakm-ntnx deepakm-ntnx changed the title [WIP] added nutanix support feat[WIP] added nutanix support Feb 21, 2024
@deepakm-ntnx deepakm-ntnx changed the title feat[WIP] added nutanix support feat: Add support for CAPX Feb 21, 2024
@github-actions github-actions bot added feature and removed feature labels Feb 21, 2024
api/v1alpha1/nutanix_clusterconfig_types.go Outdated Show resolved Hide resolved
cmd/main.go Outdated Show resolved Hide resolved
api/v1alpha1/nutanix_clusterconfig_types.go Show resolved Hide resolved
Comment on lines 637 to 528
postKubeadmCommands:
- echo export KUBECONFIG=/etc/kubernetes/admin.conf >> /root/.bashrc
- echo "after kubeadm call" > /var/log/postkubeadm.log
preKubeadmCommands:
- echo "before kubeadm call" > /var/log/prekubeadm.log
- hostnamectl set-hostname "{{ ds.meta_data.hostname }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might have to be modified to accommodate 1.29 nutanix-cloud-native/cluster-api-provider-nutanix#382

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently we are using alpha1 version. if we cut alpha2, then latest changes will show up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handled in d2iq-labs#6

metadata:
labels:
cluster.x-k8s.io/provider: infrastructure-nutanix
name: ${CLUSTER_NAME}-kcfg-0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I know this is the same as the original template but why do we append a -0 on this particular resource and not others?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is autogenerated, lets fix it in the main code if needed

namespace: ${NAMESPACE}
spec:
template:
spec:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should ideally also add:

  • project
  • additionalCategories
  • opus

Copy link
Contributor Author

@deepakm-ntnx deepakm-ntnx Feb 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assuming you mean GPUs. yes we need to figure out how to add required vs optional fields so that user can choose either way. this can be added in subsequent PRs

@@ -40,6 +40,8 @@ type ClusterConfigSpec struct {
AWS *AWSSpec `json:"aws,omitempty"`
// +optional
Docker *DockerSpec `json:"docker,omitempty"`
// +optional
Nutanix *NutanixSpec `json:"nutanix,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the reviewing easier what do you think about opening a PR with just API changes for only the required input?

SystemDiskSize string `json:"systemDiskSize"`

// List of GPU devices that need to be added to the machines.
GPUs []NutanixGPU `json:"gpus,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the GPU API should be added later after testing just a basic cluster?

@jimmidyson
Copy link
Member

I noticed conflicts so I rebased this and fixed up (force pushed) @deepakm-ntnx. Please pull latest changes locally.

@jimmidyson
Copy link
Member

Fixed up conflicts again after repo rename.

@deepakm-ntnx
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants