Install kustomize if not present

[toonsevrin]

Resolves #1426

As long as the kustomize binary is already present in the PATH, the makefile behavior will be identical to before this patch (See the which statement). In my opinion this is perfect backwards compatibility and this patch should be included in v2.

Note that the test continuous-integration/travis-ci/pr will fail due to the Makefile being changed.

Notes:

• I have not yet tested this (or written automated tests for this)
• Neither controller-gen nor kustomize are currently checksummed (this is a small vulnerability, but was already present): See scaffolded controller-gen dependency is not checksummed #1429. This should be patched in a separate PR.

[k8s-ci-robot]

Hi @toonsevrin. 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.

[toonsevrin]

I am now testing this in a virtual machine.

/hold

[toonsevrin]

✔️ I've tested this on fresh ubuntu vms. It seems to work as expected.

I tested creating and installing an API with and without kustomize in the PATH. Without kustomize in the path, "go get kustomize" is ran, with kustomize nothing is ran.

Please confirm (and perhaps test some more paths) and merge.

[toonsevrin]

/hold cancel

[Adirio]

The approach is the same as the one used for controller-gen currently. In that sense it looks ok.

golden_script.sh needs to be run to update the Makefiles under the testdata directory in order to pass the Travis check.

@mengqiy @DirectXMan12 what do you think of this?

[toonsevrin]

All right I'll run goldenscript and squash.

[toonsevrin]

@Adirio can you or someone else generate the golden_script for me? I receive the following error during the test:

go test ./pkg/... ./cmd/... -coverprofile cover.out
...
2020/03/12 17:33:22 failed to start the controlplane. retried 5 times: fork/exec /usr/local/kubebuilder/bin/etcd: no such file or directory
...

Not sure where it gets the idea that I have a /usr/local/kubebuilder folder :P

EDIT: I'm installing the latest etcd and the kubernetes api binaries in this directory just to get this to work quickly...
EDIT 2: Five hours later: fatal error: runtime: out of memory goddamnit

[Adirio]

Sorry it is generate_golden.sh.

I can't run it because I'm on Windows.

[toonsevrin]

@Adirio no worries, I've started a new vm with 4 times the RAM. Should complete this time.

As I'm on 1Mbps expect this to take a while 😆

[toonsevrin]

@Adirio looks like an unexpected exception with the test occured, are you able to restart it?
/test all

[k8s-ci-robot]

@toonsevrin: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

@Adirio looks like the test froze, are you able to restart it?
/test all

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.

[Adirio]

@Adirio looks like an unexpected exception with the test occured, are you able to restart it?

Close the PR and reopen it.

[Adirio]

/ok-to-test
This will trigger the rest of the tests, but you still have to reopen for Travis

[camilamacedo86]

Hi @toonsevrin,

(For the next times)
Instead of run the golden script manually I'd recommend use the make generate. Note that it will ensure that the script will be executed with the correct versions required and is easier as well.

c/c @Adirio

[camilamacedo86]

Hi @toonsevrin,

Great contribution. Your solution for me make totally sense and you are following the current design/approach that is already applied for another dependence.

You just need squash the commits for we are able to do it. Could you please check it?

HI @Adirio,

It does not change the design and etc. Do you see any objection/reason for we not move forward here?

[camilamacedo86]

/lgtm cancel

Reason: Missing squash the commits.

[camilamacedo86]

Just missing squash the commits.
Otherwise, I do not see any reason we do not move forward with this solution. All shows fine.

[Adirio]

#1371 suggested to check for the dependency instead of installing it. One of the approaches should be followed by I'm not sure which of them feels better. Will keep the discussion over there.

[Adirio]

/hold until #1371 discussion is settled

[toonsevrin]

@camilamacedo86 squashed 🎉

@Adirio Imo the kustomize dependency should automatically be installed just like any other internal dependency (kustomize is no longer used by developers as they can apply -k). In an ideal world, it is installed in a path that is not accessible externally to the makefile (that way the external environment is not influenced). But as that is a bit perfectionistic (and is not done with controller-gen), I think my PR is superior to a simple check (and once we integrate checksums it will be safer as well, as users can trust us to peer-review the dependency)

[camilamacedo86]

It shows great for me.
Tested locally well.
Well done 👍

[camilamacedo86]

/approve

[Adirio]

Let's see what do the owners think and we can remove the hold label.

[toonsevrin]

Looks like the makefile makes sure to set this

[mengqiy]

/lgtm
/approve
/hold cancel
IMO this improvement is fine, since 1) it doesn't break any backward compatibility. 2) it respects it if users have already installed the kustomize binary. 3) it doesn't mess up go.mod, since it's done in a tmp dir 4) It deterministic what version of kustomize is installed if the installation is triggered.

Sorry for the delay.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, mengqiy, toonsevrin

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 [camilamacedo86,mengqiy]

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