Migrate code-gen to kube_codegen.sh

[fmuyassarov]

Migrate to kube_codegen.sh and bump k8s libraries along the way to the same version as k8s.io/code-generator.

I decided to wait for #406 to land but it should not block you from giving review on this.

[klihub]

Could you alter the split into two commits like this:

1. import new deps + update Makefile+any other tooling necessary
2. regenerate everything (make generate) and commit the resulting changes

It is much easier for us to review (review first commit, skim through resulting changes in the second), much easier for you to rebase (just drop the tip-most commit, rerun make generate the commit the changes) and much easier for me to test it on top of my tree (just cherry-pick for first commit with the tooling changes and run make generate). With the current version, the last two of these will cause unavoidable conflicts because the relevant changes are not split from the side-effects.

[fmuyassarov]

Could you alter the split into two commits like this:

1. import new deps + update Makefile+any other tooling necessary

2. regenerate everything (`make generate`) and commit the resulting changes

It is much easier for us to review (review first commit, skim through resulting changes in the second), much easier for you to rebase (just drop the tip-most commit, rerun make generate the commit the changes) and much easier for me to test it on top of my tree (just cherry-pick for first commit with the tooling changes and run make generate). With the current version, the last two of these will cause unavoidable conflicts because the relevant changes are not split from the side-effects.

Sure, I will do as you suggested here because I currently it is messed up :)

[fmuyassarov]

@klihub PTAL. Updated the commits.

[klihub]

@fmuyassarov When I try a make generate with this, I get this failure:

klitkey1-mobl1 generators-v2 $ make generate
/home/kli/src/work/devel/nri-plugins/metrics/generators-v2/build-aux/bin/controller-gen \
        crd:generateEmbeddedObjectMeta=true output:crd:artifacts:config=config/crd/bases\
        paths="./pkg/apis/..."
/home/kli/src/work/devel/nri-plugins/metrics/generators-v2/build-aux/bin/controller-gen \
        rbac:roleName=manager-role crd paths="./pkg/apis/..." output:crd:artifacts:config=config/crd/bases
/home/kli/src/work/devel/nri-plugins/metrics/generators-v2/build-aux/bin/controller-gen object:headerFile="scripts/hack/boilerplate.go.txt" paths="./pkg/apis/..."
./scripts/hack/update_codegen.sh go
Generating client code for 1 targets
F1115 11:08:28.607834 2185078 main.go:65] Error: failed making a parser: error(s) in "/u/src/kli/work/devel/nri-plugins/metrics/generators-v2/pkg/apis/config/v1alpha1":
-: directory /u/src/kli/work/devel/nri-plugins/metrics/generators-v2/pkg/apis/config/v1alpha1 outside main module or its selected dependencies
Cleaning up temporary vendor directory...
make: *** [Makefile:505: generate-clients] Error 255

[klihub]

@fmuyassarov I still get the same failure with this latest version.

[klihub]

Can you test on your side with my proposed one-liner change ?

With that in place now make generate works for me. I wonder if this then breaks for you, since the original version worked with $REPO_ROOT as the current working directory...

[klihub]

This really LGTM now. I have a few small nits. Some of the shell-quoting ones should be taken with a grain of salt though, especially if shellcheck disagrees with me.

Now works also for me.

[marquiz]

I think this could be merged. Preferably with klihubs comments addressed.

Now that I looked at this: just a general note that my personal feeling is that the Makefile is getting somewhat hairy'ish with all the phony rules and army of variables (like CLIENT_GEN_VERSION has no effect if CLIENT_GEN is modified and such). I'd find it easier to follow a script (á la scripts/hack/update_codegen.sh) that just does everything (installs tools, runs them, cleanup if needed) in sequential order. But not much to do with this PR...

[fmuyassarov]

I think this could be merged. Preferably with klihubs comments addressed.

Now that I looked at this: just a general note that my personal feeling is that the Makefile is getting somewhat hairy'ish with all the phony rules and army of variables (like CLIENT_GEN_VERSION has no effect if CLIENT_GEN is modified and such). I'd find it easier to follow a script (á la scripts/hack/update_codegen.sh) that just does everything (installs tools, runs them, cleanup if needed) in sequential order. But not much to do with this PR...

This a valid point but I would prefer to do that clean up as a follow up.

[marquiz]

This a valid point but I would prefer to do that clean up as a follow up.

Yeah, was just a note, food for thought... And I'm not sure that everybody agrees...

[fmuyassarov]

rebased....

[fmuyassarov]

@klihub This is now rebased on top of your merged patches.

[klihub]

This a valid point but I would prefer to do that clean up as a follow up.

Yeah, was just a note, food for thought... And I'm not sure that everybody agrees...

Yes, as long as we think it through beforehand and make such changes so that 1) we can still pass the tooling version(s) to be used from the Makefile rule to the script, and 2) the script is smart enough to not re-clone and re-compile the tooling binaries if I run twice make generate without overriding the tooling versions.

But maybe I'd prefer to let things settle now for a while and go with these changes before we start again touching this stuff...