Fix an issue with verify_client.sh #805
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/S
k8s-ci-robot
added
size/XS
| @@ -101,6 +100,7 @@ clientset: ## Generate a typed clientset | |||
| --listers-package sigs.k8s.io/cluster-api/pkg/client/listers_generated \ | |||
| --output-package sigs.k8s.io/cluster-api/pkg/client/informers_generated \ | |||
| --go-header-file=./hack/boilerplate.go.txt | |||
| $(MAKE) gazelle | |||
There was a problem hiding this comment.
Why not keep this with generate? Bazel files might need to be updated after go generate
There was a problem hiding this comment.
ok, we need it at both places to make Makefile target idempotent, the ci script verify_clientset.sh call make clientset, i will update the PR, thanks
There was a problem hiding this comment.
I was seeing issues with calling gazelle at the end of clientset where it was generating the wrong paths in the bazel build files, which was the reason for moving it to the generate target.
There was a problem hiding this comment.
@detiber i replied in another code line thread, thanks
no need to copy files back from tmp folder. also CI will call make clientset, so we need to run make gazelle on clientset target.
|
|
||
| echo "diffing ${DIFFROOT} against freshly generated codegen" | ||
| ret=0 | ||
| diff -Naupr "${DIFFROOT}" "${TMP_DIFFROOT}" -x '*.bazel' -x 'BUILD' || ret=$? | ||
| cp -a "${TMP_DIFFROOT}"/* "${DIFFROOT}" |
There was a problem hiding this comment.
@detiber i think this is line that causing issue you are seeing (correct me if not the same issue),
make gazelle will generate bazel under the _tmp, then it copy files from _tmp to main repo,
then main repo got the bazel files which point to _tmp which is wrong.
what i found is: we don't need to copy those clientset files from _tmp to main repo.
(i guess originally been done this way because: make gazelle is not called after clientset been generated, but thats only my guess)
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: figo, vincepri 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
no need to copy files back from tmp folder.
also CI will call make clientset, so we need to
run make gazelle on clientset target.
What this PR does / why we need it:
run
script/ci-test.shfailed, the current ci reporting pass only becausebazel is not available at CI image, bazel been skipped, fix is opened at kubernetes/test-infra#11642,
cc @vincepri @detiber