Skip to content

Conversation

@tthvo
Copy link

@tthvo tthvo commented May 29, 2025

What this PR does / why we need it:

This added Make targets to install tooling binaries (i.e. controller-gen, conversion-gen and kustomize) to a pinned version if missing. I think it's a good practice to do so for various reasons:

  • Make commands run out of box without remembering to install required tools such as controller-gen and kustomize.
  • Pinned version allows the behaviors of the command to stay consistent, especially with more recent-versioned tools might introduce breaking changes.
  • Special use-case: Some projects (i.e. https://github.com/openshift/installer) clones this nutanix capi provider (this) repository to generate the infrastructure manifests. Thus, it would be great if we don't have keep track of what tools are required.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

N/A. I haven't created one yet since it's pretty small.

How Has This Been Tested?:

I ran the make targets that requires those tools, for example:

make release-manifests:

$ make release-manifests 
GOBIN=/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin go install sigs.k8s.io/kustomize/kustomize/[email protected]
GOBIN=/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin go install sigs.k8s.io/controller-tools/cmd/[email protected]
/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin/controller-gen "crd:crdVersions=v1" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin/kustomize build templates/base > templates/cluster-template.yaml
/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin/kustomize build templates/csi > templates/cluster-template-csi.yaml
/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin/kustomize build templates/csi3 > templates/cluster-template-csi3.yaml
/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin/kustomize build templates/clusterclass > templates/cluster-template-clusterclass.yaml
/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin/kustomize build templates/topology > templates/cluster-template-topology.yaml
/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin/kustomize build templates/image-lookup/ > templates/cluster-template-image-lookup.yaml
mkdir -p /home/thvo/workspace/cluster-api-provider-nutanix/out
/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin/kustomize build config/default > /home/thvo/workspace/cluster-api-provider-nutanix/out/infrastructure-components.yaml
cp templates/cluster-template*.yaml /home/thvo/workspace/cluster-api-provider-nutanix/out
cp /home/thvo/workspace/cluster-api-provider-nutanix/metadata.yaml /home/thvo/workspace/cluster-api-provider-nutanix/out/metadata.yaml

make controller-gen (i.e. the tooling target itself):

$ make controller-gen 
GOBIN=/home/thvo/workspace/cluster-api-provider-nutanix/hack/tools/bin go install sigs.k8s.io/controller-tools/cmd/[email protected]

make help:

$ make help
...output-omitted...
Tooling Binaries
  tools-bin-dir    Location to install tooling binaries.
  controller-gen   Install controller-gen if missing.
  conversion-gen   Install conversion-gen if missing.
  kustomize        Install kustomize if missing.
...output-omitted...

Special notes for your reviewer:

N/A

Release note:


This added Make targets to install tooling binaries (i.e.
controller-gen, conversion-gen and kustomize) to a pinned version if
missing.
@patrickdillon
Copy link

/cc @thunderboltsid @yanhua121
Can you all take a look or recommend other potential reviewers?

@thunderboltsid
Copy link
Contributor

thunderboltsid commented May 30, 2025

/cc @thunderboltsid @yanhua121 Can you all take a look or recommend other potential reviewers?

We use devbox for tracking tooling dependencies and corresponding versions. I'd rather not have this managed in two places. See https://github.com/nutanix-cloud-native/cluster-api-provider-nutanix/blob/main/devbox.lock

@thunderboltsid
Copy link
Contributor

thunderboltsid commented May 30, 2025

@tthvo we actually used to do something similar to what the change proposes and removed it about a year ago in favor of devbox in #358

@tthvo
Copy link
Author

tthvo commented May 30, 2025

Ah I see, I didn't realize we are using dev-box here 😅 For our purpose, the old way is easier but that's not enough to justify this change for everyone else.

I am closing this. Thanks @thunderboltsid!

@tthvo tthvo closed this May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants