🌱 Install hack/tools with temporary go module

[kaitoii11]

What this PR does / why we need it:
Install hack/tools with temporary go modules.

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 #5569

[k8s-ci-robot]

Hi @kaitoii11. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vincepri]

/ok-to-test

[sbueringer]

I think we should now be able to drop hack/tools/tools.go. After make modules hack/tools/go.mod should then only contain dependencies of our own tools in hack/tools

[sbueringer]

@kaitoii11 I think this comment still applies

[kaitoii11]

-// Code generated by conversion-gen. DO NOT EDIT.
+// Code generated by conversion-gen-v0.23.0-alpha.4. DO NOT EDIT.

The test is failing because the naming convention changed for the command binary.
Do we want to use the symlink of the command to avoid the test fail or generate a newer version?

[sbueringer]

/test pull-cluster-api-e2e-main

EDIT: Okay that's a merge conflict not a test flake

[sbueringer]

@kaitoii11 I found a solution that I would be happy with

CONVERSION_GEN := $(abspath $(TOOLS_BIN_DIR)/$(CONVERSION_GEN_BIN)-$(CONVERSION_GEN_VER))
=>
CONVERSION_GEN := $(abspath $(TOOLS_BIN_DIR)/$(CONVERSION_GEN_BIN))

$(CONVERSION_GEN): # Build conversion-gen from tools
=>
## We are forcing a rebuilt of conversion-gen via PHONY so that we're always using an up-to-date version.
## We can't use a versioned name for the binary, because that would be reflected in generated files.
.PHONY: $(CONVERSION_GEN)
$(CONVERSION_GEN): # Build conversion-gen from tools

This way we are using the symlink generated by the go install script. That is enough because conversion-gen is
using path.Base(os.Args[0]) to replace GENERATOR_NAME in the generated by comment template (// Code generated by GENERATOR_NAME. DO NOT EDIT.) (https://github.com/kubernetes/gengo/blob/master/args/args.go#L121-L129).

This has the downside that on every conversion the binary has to be rebuilt, but it's very fast with the go cache and only takes a few seconds, which is way faster than the conversion generation itself.

/cc @CecileRobertMichon @vincepri WDYT?

[CecileRobertMichon]

This has the downside that on every conversion the binary has to be rebuilt, but it's very fast with the go cache and only takes a few seconds, which is way faster than the conversion generation itself.

SGTM

[sbueringer]

Thank you very much. Last round of nits from my side. Otherwise lgtm!!

[sbueringer]

@kaitoii11 Thank you very much!!

I know it has been a lot of work, probably more than expected, but thx for staying on it and getting it done! :)
It's really a great improvement.

/lgtm

/assign @fabriziopandini @CecileRobertMichon @vincepri

[CecileRobertMichon]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

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:

• OWNERS [CecileRobertMichon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment