Generate all Seed CRDs to example #4854

timebertt · 2021-10-15T09:06:53Z

How to categorize this PR?

/area dev-productivity
/kind enhancement

What this PR does / why we need it:

With this PR, make generate-extensions-crds is replaced by a reusable hack script generate-seed-crds.sh, that can generate all of our CRDs (extensions, druid, resources) or selected groups of them using controller-gen.

g/g now uses this script on make generate to generate all seed CRDs to example/seed-crds and the resource-manager CRDs to example/resource-manager.
extensions can then leverage this script as well to generate the example CRDs in order to provide a simple way for developers to use the current CRDs on a development cluster (e.g. ls example/20-crd-* | xargs -I {} kubectl apply -f {}). See timebertt/gardener-extension-provider-aws@a686de3

Which issue(s) this PR fixes:
Part of gardener/gardener-extension-provider-aws#430

Special notes for your reviewer:

Currently I'm missing CRDs for VPA, HVPA and DNS{Provider,Owner,Entry}.
I think, the HVPA and DNS CRDs are not strictly required as they are also not available in https://github.com/gardener/gardener-extension-provider-aws/tree/master/example.
However, there is https://github.com/gardener/gardener-extension-provider-aws/blob/master/example/20-crd-vpa.yaml but I'm not sure how we can generate it and if it's actually needed.
Any input is appreciated!

Release note:

CRDs that are installed by Gardener on a Seed cluster are now generated to `example/seed-crds`. This allows to quickly apply all Seed CRDs for development purposes.

A new hack script `generate-seed-crds.sh` was added, that can generate all of Gardener's Seed CRDs using `controller-gen`. See [this file](https://github.com/gardener/gardener/blob/master/example/seed-crds/doc.go) for an example usage. Make sure to add `controller-gen` to the list of requirements and `example` to the list of generated paths.

`hack/{generate,generate-parallel.sh}` don't set `GO111MODULE=off` anymore as they used to. This was done to speed up generation with `k8s.io/code-generator`. If your repo reuses these scripts to generate code using `k8s.io/code-generator` you might want to consider setting `GO111MODULE=off` explicitly in `hack/update-codegen.sh`.

timebertt · 2021-10-15T09:14:08Z

Hmm, check-generate fails:

Generate github.com/gardener/gardener/example/resource-manager
controller-gen not available, please install it first by running 'make install-requirements'
example/resource-manager/doc.go:15: running "../../hack/generate-seed-crds.sh": exit status 1
Generate github.com/gardener/gardener/example/seed-crds
controller-gen not available, please install it first by running 'make install-requirements'
example/seed-crds/doc.go:15: running "../../hack/generate-seed-crds.sh": exit status 1

However, controller-gen should actually be installed. I'm confused 🤔

timuthy · 2021-10-15T09:52:51Z

hack/generate-seed-crds.sh

+  fi
+
+  echo "Generating CRDs for $group group"
+  controller-gen crd paths="$package_path" output:crd:dir="$output_dir" output:stdout


What do you think about using a controller-gen bin from a dedicated bin directory as realized here. In the past, we had issues that different projects use different versions of contorller-gen and also long as we don't execute these commands in a dedicated container, we can at least place them in a git-ignored directory.

That's a great idea. Let's do it similarly to the yq binary, that we already install in such a way here.
I thought about doing this actually, but I didn't want to change the way controller-gen is installed in the same PR.
But now that you propose it, let's do it directly :)

Done in a new commit.
In a follow-up PR, we could get rid of the $YQ variable in start-gardenlet (and rather use the changed PATH):

gardener/hack/local-development/start-gardenlet

Line 34 in 27c2a5e

YQ="$REPO_ROOT/hack/tools/bin/yq"

and install all other go dependency binaries in the same way as well:

gardener/Makefile

Lines 180 to 184 in 27c2a5e

@go install -mod=vendor github.com/onsi/ginkgo/ginkgo

@go install -mod=vendor github.com/ahmetb/gen-crd-api-reference-docs

@go install -mod=vendor github.com/golang/mock/mockgen

@go install -mod=vendor sigs.k8s.io/controller-runtime/tools/setup-envtest

@go install -mod=vendor sigs.k8s.io/controller-tools/cmd/controller-gen

timebertt · 2021-10-15T13:27:07Z

Weird, check-generate still fails. This time, with a different error though:

Generate github.com/gardener/gardener/example/seed-crds
Generating CRDs for extensions.gardener.cloud group
Generating CRDs for resources.gardener.cloud group
cannot find package "github.com/gardener/etcd-druid/api/v1alpha1" in any of:
	/usr/local/go/src/github.com/gardener/etcd-druid/api/v1alpha1 (from $GOROOT)
	/go/src/github.com/gardener/etcd-druid/api/v1alpha1 (from $GOPATH)

However, when I hijack the build container, I can run the following successfully:

/tmp/build/86a4261d# cd /go/src/github.com/gardener/gardener/
/go/src/github.com/gardener/gardener# ls -l hack/tools/bin
total 20024
-rwxr-xr-x 1 root root 20501205 Oct 15 13:12 controller-gen
/go/src/github.com/gardener/gardener# hack/tools/bin/controller-gen --version
Version: v0.7.0
/go/src/github.com/gardener/gardener# export PATH=$PWD/hack/tools/bin:$PATH
/go/src/github.com/gardener/gardener# go generate ./example/...
Generating CRDs for resources.gardener.cloud group
Generating CRDs for extensions.gardener.cloud group
Generating CRDs for resources.gardener.cloud group
Generating CRDs for druid.gardener.cloud group
/go/src/github.com/gardener/gardener# go list -f '{{ .Dir }}' github.com/gardener/etcd-druid/api/v1alpha1
/go/src/github.com/gardener/gardener/vendor/github.com/gardener/etcd-druid/api/v1alpha1

This is exactly how it should look like. I'm confused again ... 🤔

EDIT:
Actually, make check-generate also fails with the same error on my machine, although make generate works perfectly fine.

timuthy · 2021-10-15T14:23:33Z

Weird, check-generate still fails. This time, with a different error though:

For the Druid I recall that we first needed to cd into the submodule dir before we call controller-gen https://github.com/gardener/etcd-druid/blob/5c908089a87215c73710ee15f1a12e1397153486/Makefile#L68

timebertt · 2021-10-15T14:42:14Z

I think, I found the problem:

In hack/generate.sh and hack/generate-parallel.sh we set GO111MODULE=off:

gardener/hack/generate-parallel.sh

Lines 37 to 39 in 10fe751

    
           # We need to explicitly pass GO111MODULE=off to k8s.io/code-generator as it is significantly slower otherwise, 
        
           # see https://github.com/kubernetes/code-generator/issues/100. 
        
           GO111MODULE=off parallel --will-cite echo Generate {}';' go generate -mod=vendor {} ::: ${PACKAGES[@]}

However, we don't even call k8s.io/code-generator in there – we only call it in hack/update-{codegen,protobuf}.sh which is invoked from make generate.

Because of GO111MODULE=off, the go list call in hack/generate-seed-crds.sh doesn't look at the vendored version of etcd-druid, but only looks for the package from GOROOT and GOPATH:

cannot find package "github.com/gardener/etcd-druid/api/v1alpha1" in any of:
	/usr/local/go/src/github.com/gardener/etcd-druid/api/v1alpha1 (from $GOROOT)
	/go/src/github.com/gardener/etcd-druid/api/v1alpha1 (from $GOPATH)

It wasn't failing on my machine before, because it just used my local clone of etcd-druid inside GOPATH.
After deleting the clone, make generate and make check-generate started failing with the same error.

Long story short

I think we have to remove GO111MODULE=off in hack/{generate,generate-parallell}.sh (it's not used in g/g)
In #3489, @stoyanr probably added it because the scripts are used in our extensions to call update-codegen.sh (see #3489 (comment)).
I would vote for a) setting GO111MODULE=off in the scripts where it's needed (hack/update-{codegen,protobuf}.sh) or b) explicitly calling hack/update-codegen.sh with GO111MODULE=off from the Makefile in extensions.

timebertt · 2021-10-15T14:47:47Z

For now, I have simply dropped GO111MODULE=off in hack/{generate,generate-parallell}.sh and generated the etcd druid with the correct API version. I also added a breaking release note for dependencies reusing the hack scripts.

timebertt · 2021-10-18T05:39:35Z

Rebased, now that #4822 is merged.

timebertt · 2021-10-18T09:31:35Z

It would be good to include this in v1.34.0, given that #4851 is already merged (which will require extensions to update their CRDs, gardener/gardener-extension-provider-aws#430). With this hack script, extension can easily regenerate new versions of their CRDs without much effort.
/milestone v1.34
cc @plkokanov

timuthy · 2021-10-18T11:59:50Z

Hm, make generate does not work for me because of some problems with the mockgen

pkg/utils/secrets/mock/doc.go:15: running "mockgen": exit status 1
Generate github.com/gardener/gardener/pkg/apis/seedmanagement/v1alpha1
I1018 13:18:37.623181    5072 main.go:127] parsing go packages in directory .
I1018 13:20:51.474834    5072 main.go:229] using package=github.com/gardener/gardener/pkg/apis/seedmanagement/v1alpha1
I1018 13:20:51.507099    5072 main.go:165] written to ../../../../hack/api-reference/seedmanagement.md
make: *** [generate] Error 40

Do you have the same issue?

timebertt · 2021-10-18T12:45:06Z

This seems to be related to #4570.
I've seen it with my PR, but also on master before.

timuthy

/lgtm

plkokanov · 2021-10-18T15:08:37Z

/lgtm

both make generate and make check-generate worked for me without a problem, further points to #4570 being the problem you guys encountered.

* Generate all Seed CRDs to example * Install controller-gen to tools bin dir * Fix check-generate * Set GO111MODULE=off in the scripts where it's needed

timebertt requested a review from a team as a code owner October 15, 2021 09:06

gardener-robot added area/dev-productivity Developer productivity related (how to improve development) kind/enhancement Enhancement, improvement, extension needs/review size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 15, 2021

gardener-robot-ci-1 added reviewed/ok-to-test and removed reviewed/ok-to-test labels Oct 15, 2021

timebertt mentioned this pull request Oct 15, 2021

Update example CRDs to apiextensions.k8s.io/v1 gardener/gardener-extension-provider-aws#430

Closed

timuthy reviewed Oct 15, 2021

View reviewed changes

gardener-robot-ci-3 added the reviewed/ok-to-test label Oct 15, 2021

gardener-robot-ci-2 removed the reviewed/ok-to-test label Oct 15, 2021

timebertt requested a review from timuthy October 15, 2021 12:51

timebertt force-pushed the enh/generate-seed-crds branch from a4cc6d1 to 692c828 Compare October 15, 2021 13:09

gardener-robot-ci-3 added the reviewed/ok-to-test label Oct 15, 2021

gardener-robot-ci-2 removed the reviewed/ok-to-test label Oct 15, 2021

timebertt force-pushed the enh/generate-seed-crds branch from 692c828 to 81a5f55 Compare October 15, 2021 14:43

gardener-robot-ci-3 added reviewed/ok-to-test and removed reviewed/ok-to-test labels Oct 15, 2021

timebertt added the merge/squash label Oct 15, 2021

timebertt force-pushed the enh/generate-seed-crds branch from b3d7735 to 5fc3b8b Compare October 15, 2021 15:02

gardener-robot-ci-2 added the reviewed/ok-to-test label Oct 15, 2021

gardener-robot-ci-1 removed the reviewed/ok-to-test label Oct 15, 2021

timebertt added 4 commits October 18, 2021 07:37

Generate all Seed CRDs to example

8db0878

Install controller-gen to tools bin dir

f526209

Fix check-generate

f1b7749

Set GO111MODULE=off in the scripts where it's needed

935b00b

timebertt force-pushed the enh/generate-seed-crds branch from 5fc3b8b to 935b00b Compare October 18, 2021 05:39

gardener-robot-ci-3 added the reviewed/ok-to-test label Oct 18, 2021

gardener-robot-ci-1 removed the reviewed/ok-to-test label Oct 18, 2021

gardener-robot added this to the v1.34 milestone Oct 18, 2021

timuthy approved these changes Oct 18, 2021

View reviewed changes

gardener-robot added reviewed/lgtm and removed needs/review labels Oct 18, 2021

plkokanov approved these changes Oct 18, 2021

View reviewed changes

timebertt merged commit 2d43446 into gardener:master Oct 19, 2021

timebertt deleted the enh/generate-seed-crds branch October 19, 2021 06:41

timebertt mentioned this pull request Oct 20, 2021

Build/install development tools to local dir #4879

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate all Seed CRDs to example #4854

Generate all Seed CRDs to example #4854

timebertt commented Oct 15, 2021 •

edited

timebertt commented Oct 15, 2021

timuthy Oct 15, 2021 •

edited

timebertt Oct 15, 2021

timebertt Oct 15, 2021

timebertt commented Oct 15, 2021 •

edited

timuthy commented Oct 15, 2021

timebertt commented Oct 15, 2021 •

edited

timebertt commented Oct 15, 2021

timebertt commented Oct 18, 2021

timebertt commented Oct 18, 2021 •

edited

timuthy commented Oct 18, 2021 •

edited

timebertt commented Oct 18, 2021

timuthy left a comment

plkokanov commented Oct 18, 2021

	@go install -mod=vendor github.com/onsi/ginkgo/ginkgo
	@go install -mod=vendor github.com/ahmetb/gen-crd-api-reference-docs
	@go install -mod=vendor github.com/golang/mock/mockgen
	@go install -mod=vendor sigs.k8s.io/controller-runtime/tools/setup-envtest
	@go install -mod=vendor sigs.k8s.io/controller-tools/cmd/controller-gen

Generate all Seed CRDs to example #4854

Generate all Seed CRDs to example #4854

Conversation

timebertt commented Oct 15, 2021 • edited

timebertt commented Oct 15, 2021

timuthy Oct 15, 2021 • edited

Choose a reason for hiding this comment

timebertt Oct 15, 2021

Choose a reason for hiding this comment

timebertt Oct 15, 2021

Choose a reason for hiding this comment

timebertt commented Oct 15, 2021 • edited

timuthy commented Oct 15, 2021

timebertt commented Oct 15, 2021 • edited

Long story short

timebertt commented Oct 15, 2021

timebertt commented Oct 18, 2021

timebertt commented Oct 18, 2021 • edited

timuthy commented Oct 18, 2021 • edited

timebertt commented Oct 18, 2021

timuthy left a comment

Choose a reason for hiding this comment

plkokanov commented Oct 18, 2021

timebertt commented Oct 15, 2021 •

edited

timuthy Oct 15, 2021 •

edited

timebertt commented Oct 15, 2021 •

edited

timebertt commented Oct 15, 2021 •

edited

timebertt commented Oct 18, 2021 •

edited

timuthy commented Oct 18, 2021 •

edited