From 965d5d8608931e758eed1e4b496e75e10c65c015 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 11 Jan 2023 23:58:37 -0800 Subject: [PATCH 01/14] Make boilerplate.py smarter about generated Don't just grep for DO NOT EDIT - anchor it in something that looks like a comment and alone on a line. Also ignore __* dirs Prevent it from triggering on update-generated-swagger-docs (hack, but better than before) --- hack/boilerplate/boilerplate.py | 26 ++++++++++---------------- hack/update-generated-swagger-docs.sh | 22 ++++++++++++---------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/hack/boilerplate/boilerplate.py b/hack/boilerplate/boilerplate.py index 89224186aca..eaf758b9fe5 100755 --- a/hack/boilerplate/boilerplate.py +++ b/hack/boilerplate/boilerplate.py @@ -62,10 +62,6 @@ def get_refs(): def is_generated_file(filename, data, regexs): - for d in skipped_ungenerated_files: - if d in filename: - return False - p = regexs["generated"] return p.search(data) @@ -151,21 +147,15 @@ def file_extension(filename): return os.path.splitext(filename)[1].split(".")[-1].lower() -skipped_dirs = ['third_party', '_gopath', '_output', '.git', 'cluster/env.sh', - "vendor", "test/e2e/generated/bindata.go", "hack/boilerplate/test", - "staging/src/k8s.io/kubectl/pkg/generated/bindata.go"] - -# list all the files contain 'DO NOT EDIT', but are not generated -skipped_ungenerated_files = [ - 'hack/update-generated-swagger-docs.sh', - 'hack/boilerplate/boilerplate.py' -] +skipped_names = ['third_party', '_gopath', '_output', '.git', 'cluster/env.sh', + "vendor", "test/e2e/generated/bindata.go", "hack/boilerplate/test", + "staging/src/k8s.io/kubectl/pkg/generated/bindata.go"] def normalize_files(files): newfiles = [] for pathname in files: - if any(x in pathname for x in skipped_dirs): + if any(x in pathname for x in skipped_names): continue newfiles.append(pathname) for i, pathname in enumerate(newfiles): @@ -184,9 +174,13 @@ def get_files(extensions): # as we would prune these later in normalize_files(). But doing it # cuts down the amount of filesystem walking we do and cuts down # the size of the file list - for d in skipped_dirs: + for d in skipped_names: if d in dirs: dirs.remove(d) + for d in dirs: + # dirs that start with __ are ignored + if re.match("^__", d): + dirs.remove(d) for name in walkfiles: pathname = os.path.join(root, name) @@ -222,7 +216,7 @@ def get_regexs(): # strip #!.* from scripts regexs["shebang"] = re.compile(r"^(#!.*\n)\n*", re.MULTILINE) # Search for generated files - regexs["generated"] = re.compile('DO NOT EDIT') + regexs["generated"] = re.compile(r"^[/*#]+ +.* DO NOT EDIT\.$", re.MULTILINE) return regexs diff --git a/hack/update-generated-swagger-docs.sh b/hack/update-generated-swagger-docs.sh index dedf8e7db42..545f7f0a850 100755 --- a/hack/update-generated-swagger-docs.sh +++ b/hack/update-generated-swagger-docs.sh @@ -41,19 +41,21 @@ gen_types_swagger_doc() { { echo -e "$(cat hack/boilerplate/boilerplate.generatego.txt)\n" echo "package ${group_version##*/}" + # Indenting here prevents the boilerplate checker from thinking this file + # is generated - gofmt will fix the indents anyway. cat < "${TMPFILE}" From 7c262b901fe2013d8a9d22e3ff0b5d58135452e9 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 19 Jan 2023 18:30:09 -0800 Subject: [PATCH 02/14] Set GOCACHE and GOMODCACHE If these are not set, set them. This ensures that any subsequent scripts we call (which may call setup_env again) use the same values. --- hack/lib/golang.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh index e74a090ccac..806da41142f 100755 --- a/hack/lib/golang.sh +++ b/hack/lib/golang.sh @@ -521,7 +521,10 @@ kube::golang::setup_env() { kube::golang::create_gopath_tree export GOPATH="${KUBE_GOPATH}" - export GOCACHE="${KUBE_GOPATH}/cache" + # If these are not set, set them now. This ensures that any subsequent + # scripts we run (which may call this function again) use the same values. + export GOCACHE="${GOCACHE:-"${KUBE_GOPATH}/cache/build"}" + export GOMODCACHE="${GOMODCACHE:-"${KUBE_GOPATH}/cache/mod"}" # Make sure our own Go binaries are in PATH. export PATH="${KUBE_GOPATH}/bin:${PATH}" From 04c80d0a230239127df03360915e686dc4474c77 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 19 Jan 2023 18:25:24 -0800 Subject: [PATCH 03/14] Simplify verify-codegen to use worktrees --- hack/verify-codegen.sh | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/hack/verify-codegen.sh b/hack/verify-codegen.sh index 7fd0a19a7ad..2442e2a6d3a 100755 --- a/hack/verify-codegen.sh +++ b/hack/verify-codegen.sh @@ -26,21 +26,27 @@ set -o pipefail KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. source "${KUBE_ROOT}/hack/lib/init.sh" +kube::util::ensure_clean_working_dir + +# This sets up the environment, like GOCACHE, which keeps the worktree cleaner. kube::golang::setup_env -# call verify on sub-project for now -# -# Note: these must be before the main script call because the later calls the sub-project's -# update-codegen.sh scripts. We wouldn't see any error on changes then. -export CODEGEN_PKG=./vendor/k8s.io/code-generator -vendor/k8s.io/code-generator/hack/verify-codegen.sh -vendor/k8s.io/kube-aggregator/hack/verify-codegen.sh -vendor/k8s.io/sample-apiserver/hack/verify-codegen.sh -vendor/k8s.io/sample-controller/hack/verify-codegen.sh -vendor/k8s.io/apiextensions-apiserver/hack/verify-codegen.sh -vendor/k8s.io/metrics/hack/verify-codegen.sh +_tmpdir="$(kube::realpath "$(mktemp -d -t "$(basename "$0").XXXXXX")")" +git worktree add -f -q "${_tmpdir}" HEAD +kube::util::trap_add "git worktree remove -f ${_tmpdir}" EXIT +cd "${_tmpdir}" -# This won't actually update anything because of --verify-only, but it tells -# the openapi tool to verify against the real filenames. +# Update generated code export UPDATE_API_KNOWN_VIOLATIONS=true -"${KUBE_ROOT}/hack/update-codegen.sh" --verify-only "$@" +hack/update-codegen.sh "$@" + +# Test for diffs +diffs=$(git status --porcelain | wc -l) +if [[ ${diffs} -gt 0 ]]; then + git status >&2 + git diff >&2 + echo "Generated files need to be updated" >&2 + echo "Please run 'hack/update-codegen.sh'" >&2 + exit 1 +fi +echo "Generated files are up to date" From b201c08cead67602ac6656cb15bda98d684e3bce Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 4 Jan 2023 23:32:44 -0800 Subject: [PATCH 04/14] Simplify find in codegen The `find` tool has hard to comprehend syntax and does not consider things excluded by .gitignore. I keep tripping over this in my own repos, where I have __stuff which gets found. This converts update-codegen to use `git ls-files` in a seemingly equivalent way (`-cmo --exclude-standard`). I verified it finds the same set of files as before. This also drops some obsolete filtering. Also hide grep errors for not-found files, which can happen if a file is removed but git ls-files still knows it. Re-running update-codegen shows no diffs. This will make subsequent changes easier. --- hack/update-codegen.sh | 87 ++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 58 deletions(-) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 01e2d4528a9..64dbd1f690a 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -42,65 +42,40 @@ if [[ "${DBG_CODEGEN}" == 1 ]]; then kube::log::status "DBG: starting generated_files" fi -# This is a partial 'find' command. The caller is expected to pass the -# remaining arguments. -# -# Example: -# kfind -type f -name foobar.go -function kfind() { - # We want to include the "special" vendor directories which are actually - # part of the Kubernetes source tree (./staging/*) but we need them to be - # named as their ./vendor/* equivalents. Also, we do not want all of - # ./vendor nor ./hack/tools/vendor nor even all of ./vendor/k8s.io. - find -H . \ - \( \ - -not \( \ - \( \ - -name '_*' -o \ - -name '.[^.]*' -o \ - \( \ - -name 'vendor' \ - -type d \ - \) -o \ - \( \ - -name 'testdata' \ - -type d \ - \) \ - \) -prune \ - \) \ - \) \ - "$@" \ - | sed 's|^./staging/src|vendor|' +function git_find() { + # Similar to find but faster and easier to understand. We want to include + # modified and untracked files because this might be running against code + # which is not tracked by git yet. + git ls-files -cmo --exclude-standard "$@" } -function find_all_go_dirs() { - kfind -type f -name \*.go \ - | sed 's|/[^/]*$||' \ - | sed 's|^./||' \ - | LC_ALL=C sort -u +function git_grep() { + # We want to include modified and untracked files because this might be + # running against code which is not tracked by git yet. + git grep --untracked "$@" } -# This variable holds a list of every directory that contains Go files in this -# project. Other rules and variables can use this as a starting point to -# reduce filesystem accesses. -if [[ "${DBG_CODEGEN}" == 1 ]]; then - kube::log::status "DBG: finding all *.go dirs" -fi -ALL_GO_DIRS=() -kube::util::read-array ALL_GO_DIRS < <(find_all_go_dirs) -if [[ "${DBG_CODEGEN}" == 1 ]]; then - kube::log::status "DBG: found ${#ALL_GO_DIRS[@]} *.go dirs" -fi - # Generate a list of all files that have a `+k8s:` comment-tag. This will be # used to derive lists of files/dirs for generation tools. +# +# We want to include the "special" vendor directories which are actually part +# of the Kubernetes source tree (staging/*) but we need them to be named as +# their vendor/* equivalents. We do not want all of vendor nor +# hack/tools/vendor nor even all of vendor/k8s.io - just the subset that lives +# in staging. if [[ "${DBG_CODEGEN}" == 1 ]]; then kube::log::status "DBG: finding all +k8s: tags" fi ALL_K8S_TAG_FILES=() kube::util::read-array ALL_K8S_TAG_FILES < <( - find "${ALL_GO_DIRS[@]}" -maxdepth 1 -type f -name \*.go -print0 \ - | xargs -0 grep --color=never -l '^// *+k8s:') + git_grep -l \ + -e '^// *+k8s:' `# match +k8s: tags` \ + -- \ + ':!:vendor/*' `# not under vendor` \ + ':!:*/testdata/*' `# not under any testdata` \ + ':(glob)**/*.go' `# in any *.go file` \ + | sed 's|^staging/src|vendor|' `# see comments above` \ + ) if [[ "${DBG_CODEGEN}" == 1 ]]; then kube::log::status "DBG: found ${#ALL_K8S_TAG_FILES[@]} +k8s: tagged files" fi @@ -593,14 +568,12 @@ function codegen::applyconfigs() { local applyconfigurationgen applyconfigurationgen=$(kube::util::find-binary "applyconfiguration-gen") - # because client-gen doesn't do policy/v1alpha1, we have to skip it too local ext_apis=() kube::util::read-array ext_apis < <( - cd "${KUBE_ROOT}/staging/src" - find k8s.io/api -name types.go -print0 \ - | xargs -0 -n1 dirname \ - | grep -v pkg.apis.policy.v1alpha1 \ - | LC_ALL=C sort -u) + cd "${KUBE_ROOT}/staging/src" + git_find -z ':(glob)k8s.io/api/**/types.go' \ + | xargs -0 -n1 dirname \ + | LC_ALL=C sort -u) ext_apis+=("k8s.io/apimachinery/pkg/apis/meta/v1") kube::log::status "Generating apply-config code for ${#ext_apis[@]} targets" @@ -681,7 +654,7 @@ function codegen::listers() { local ext_apis=() kube::util::read-array ext_apis < <( cd "${KUBE_ROOT}/staging/src" - find k8s.io/api -name types.go -print0 \ + git_find -z ':(glob)k8s.io/api/**/types.go' \ | xargs -0 -n1 dirname \ | LC_ALL=C sort -u) @@ -712,13 +685,11 @@ function codegen::informers() { local informergen informergen=$(kube::util::find-binary "informer-gen") - # because client-gen doesn't do policy/v1alpha1, we have to skip it too local ext_apis=() kube::util::read-array ext_apis < <( cd "${KUBE_ROOT}/staging/src" - find k8s.io/api -name types.go -print0 \ + git_find -z ':(glob)k8s.io/api/**/types.go' \ | xargs -0 -n1 dirname \ - | grep -v pkg.apis.policy.v1alpha1 \ | LC_ALL=C sort -u) kube::log::status "Generating informer code for ${#ext_apis[@]} targets" From 8704337395002309b2497b131a8e5dbba9b9b6d6 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 4 Jan 2023 16:54:26 -0800 Subject: [PATCH 05/14] Codegen: rm generated proto files before regen --- hack/update-generated-protobuf.sh | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/hack/update-generated-protobuf.sh b/hack/update-generated-protobuf.sh index 01b826ab2a0..76ba550dcad 100755 --- a/hack/update-generated-protobuf.sh +++ b/hack/update-generated-protobuf.sh @@ -27,12 +27,24 @@ KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. # source tree. This is managed in kube::build::copy_output in build/common.sh. # If the output set is changed update that function. -APIROOTS=${APIROOTS:-$(git grep --files-with-matches -e '// +k8s:protobuf-gen=package' cmd pkg staging | \ - xargs -n 1 dirname | \ - sed 's,^,k8s.io/kubernetes/,;s,k8s.io/kubernetes/staging/src/,,' | \ - sort | uniq +APIROOTS=${APIROOTS:-$( \ + git grep --untracked --null -l \ + -e '// +k8s:protobuf-gen=package' \ + -- \ + cmd pkg staging \ + | xargs -0 -n1 dirname \ + | sed 's,^,k8s.io/kubernetes/,;s,k8s.io/kubernetes/staging/src/,,' \ + | sort -u )} -"${KUBE_ROOT}/build/run.sh" hack/update-generated-protobuf-dockerized.sh "${APIROOTS}" "$@" +function git_find() { + # Similar to find but faster and easier to understand. We want to include + # modified and untracked files because this might be running against code + # which is not tracked by git yet. + git ls-files -cmo --exclude-standard ':!:vendor/*' "$@" +} -# ex: ts=2 sw=2 et filetype=sh +git_find -z ':(glob)**/generated.proto' | xargs -0 rm -f +git_find -z ':(glob)**/generated.pb.go' | xargs -0 rm -f + +"${KUBE_ROOT}/build/run.sh" hack/update-generated-protobuf-dockerized.sh "${APIROOTS}" "$@" From 07f7941de6beafb77561ad44a68b2118b65c3e36 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 4 Jan 2023 16:00:04 -0800 Subject: [PATCH 06/14] Codegen: rm deepcopy files before regen --- hack/update-codegen.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 64dbd1f690a..9d464be9298 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -184,6 +184,8 @@ function codegen::deepcopy() { done fi + git_find -z ':(glob)**'/"${output_base}.go" | xargs -0 rm -f + ./hack/run-in-gopath.sh "${gen_deepcopy_bin}" \ --v "${KUBE_VERBOSE}" \ --logtostderr \ From 3f0c3f33caa45cff7a4598e989c7a98bf4abc5d4 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 4 Jan 2023 15:58:21 -0800 Subject: [PATCH 07/14] Codegen: rm prerelease files before regen --- hack/update-codegen.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 9d464be9298..29cc94d3db0 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -125,6 +125,8 @@ function codegen::prerelease() { done fi + git_find -z ':(glob)**'/"${output_base}.go" | xargs -0 rm -f + ./hack/run-in-gopath.sh "${gen_prerelease_bin}" \ --v "${KUBE_VERBOSE}" \ --logtostderr \ From e149f79d4eee68f7b8456868d12686a503280924 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 4 Jan 2023 16:00:36 -0800 Subject: [PATCH 08/14] Codegen: rm defaults files before regen --- hack/update-codegen.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 29cc94d3db0..4b0a0f01df7 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -254,6 +254,8 @@ function codegen::defaults() { done fi + git_find -z ':(glob)**'/"${output_base}.go" | xargs -0 rm -f + ./hack/run-in-gopath.sh "${gen_defaulter_bin}" \ --v "${KUBE_VERBOSE}" \ --logtostderr \ From 7f87ecfb9a1ecd4267622b4de474a7b7494ad6b0 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 4 Jan 2023 16:01:25 -0800 Subject: [PATCH 09/14] Codegen: rm conversion files before regen --- hack/update-codegen.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 4b0a0f01df7..e8c2933ab18 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -333,6 +333,8 @@ function codegen::conversions() { done fi + git_find -z ':(glob)**'/"${output_base}.go" | xargs -0 rm -f + ./hack/run-in-gopath.sh "${gen_conversion_bin}" \ --v "${KUBE_VERBOSE}" \ --logtostderr \ From 6aea6fe86ba6d29d38dd7e52ced042cb7ca450a0 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 4 Jan 2023 16:03:42 -0800 Subject: [PATCH 10/14] Codegen: rm openapi files before regen --- hack/update-codegen.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index e8c2933ab18..7ead9d63d5c 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -501,6 +501,8 @@ function codegen::openapi() { "${apimachinery_dirs[@]}" ) + git_find -z ':(glob)**'/"${output_base}.go" | xargs -0 rm -f + for prefix in "${targets[@]}"; do local report_file="${OUT_DIR}/${prefix}_violations.report" # When UPDATE_API_KNOWN_VIOLATIONS is set to be true, let the generator to write From b852b365553dd379122c76a0d96a73ce5a6dae3e Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 4 Jan 2023 21:41:01 -0800 Subject: [PATCH 11/14] Codegen: rm applyconfig files before regen --- hack/update-codegen.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 7ead9d63d5c..d6112d00efa 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -594,6 +594,12 @@ function codegen::applyconfigs() { done fi + git_grep -l --null \ + -e '^// Code generated by applyconfiguration-gen. DO NOT EDIT.$' \ + -- \ + ':(glob)staging/src/k8s.io/client-go/**/*.go' \ + | xargs -0 rm -f + "${applyconfigurationgen}" \ --openapi-schema <("${modelsschema}") \ --go-header-file "${BOILERPLATE_FILENAME}" \ From bb53ed4ff6d278121495a93504f77ba890ba33aa Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 5 Jan 2023 00:05:17 -0800 Subject: [PATCH 12/14] Codegen: rm client files before regen --- hack/update-codegen.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index d6112d00efa..9ba1646fbb2 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -646,6 +646,12 @@ function codegen::clients() { done fi + git_grep -l --null \ + -e '^// Code generated by client-gen. DO NOT EDIT.$' \ + -- \ + ':(glob)staging/src/k8s.io/client-go/**/*.go' \ + | xargs -0 rm -f + "${clientgen}" \ --go-header-file "${BOILERPLATE_FILENAME}" \ --output-base "${KUBE_ROOT}/vendor" \ From 9a11efa7196d926173267785a030501294a18a14 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 5 Jan 2023 00:07:15 -0800 Subject: [PATCH 13/14] Codegen: rm lister files before regen --- hack/update-codegen.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 9ba1646fbb2..bad3cb2e3b7 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -688,6 +688,12 @@ function codegen::listers() { done fi + git_grep -l --null \ + -e '^// Code generated by lister-gen. DO NOT EDIT.$' \ + -- \ + ':(glob)staging/src/k8s.io/client-go/**/*.go' \ + | xargs -0 rm -f + "${listergen}" \ --go-header-file "${BOILERPLATE_FILENAME}" \ --output-base "${KUBE_ROOT}/vendor" \ From ac90c60cff833666335313564fd81aa920d7d3f1 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 5 Jan 2023 00:09:16 -0800 Subject: [PATCH 14/14] Codegen: rm informer files before regen --- hack/update-codegen.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index bad3cb2e3b7..a5cbcb5a86f 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -728,6 +728,12 @@ function codegen::informers() { done fi + git_grep -l --null \ + -e '^// Code generated by informer-gen. DO NOT EDIT.$' \ + -- \ + ':(glob)staging/src/k8s.io/client-go/**/*.go' \ + | xargs -0 rm -f + "${informergen}" \ --go-header-file "${BOILERPLATE_FILENAME}" \ --output-base "${KUBE_ROOT}/vendor" \