Bug 2032926: Bump envtest version#154
Conversation
|
Please add appropriate PR descriptions to all PRs, even if they are minimal. I can see what you are doing with this, but I don't know why. |
Fedosin
force-pushed
the
envtest
branch
7 times, most recently
from
25d1427 to
e21989a
Compare
| unit: | ||
| hack/unit-tests.sh | ||
| unit: envtest | ||
| PATH=$(PATH):$(KUBEBUILDER_ASSETS) go test ./... -coverprofile cover.out |
There was a problem hiding this comment.
It looks like to override the default binaries path (/usr/local/kubebuilder/bin) we need to set the KUBEBUILDER_ASSETS env var rather than adding its value to the PATH env.
ref: https://github.com/kubernetes-sigs/controller-runtime/blob/11917e9d90625b51e5dab16b4cf20934971e2f08/pkg/envtest/server.go#L44
| PATH=$(PATH):$(KUBEBUILDER_ASSETS) go test ./... -coverprofile cover.out | |
| PATH="$(PATH)" KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test ./... -coverprofile cover.out |
damdo
left a comment
damdo
left a comment
There was a problem hiding this comment.
I tried to fix the unit tests that are failing for this PR, locally, and with the two changes I'm suggesting above, they are now passing for me.
| ENVTEST = $(shell pwd)/bin/setup-envtest | ||
| .PHONY: envtest | ||
| envtest: ## Download envtest-setup locally if necessary. | ||
| $(call go-get-tool,$(ENVTEST),sigs.k8s.io/controller-runtime/tools/setup-envtest@latest) |
There was a problem hiding this comment.
Is it worth pinning setup-envtest here to the current controller-runtime version we use in go.mod (i.e. v0.9.6)? Maybe we could have it in a variable at the top next to the ENVTEST_K8S_VERSION one.
There was a problem hiding this comment.
Currently kubebuilder generates it with latest for all new projects. I don't think we should change this pattern.
Fedosin
force-pushed
the
envtest
branch
8 times, most recently
from
f00c084 to
0d5fe9a
Compare
|
/test unit |
openshift-ci
Bot
added
bugzilla/severity-high
|
@Fedosin: This pull request references Bugzilla bug 2032926, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
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. |
Instead of a script envtest started shipping as a compiled binary, so we have to updated our Makefile accordingly. This bump is required to port to k8s 1.23, because unit tests fail otherwise.
|
/lgtm |
elmiko
left a comment
elmiko
left a comment
There was a problem hiding this comment.
i gave this a test drive locally, everything worked as expected, thanks Mike!
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: elmiko The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
@Fedosin: Some pull requests linked via external trackers have merged: The following pull requests linked via external trackers have not merged: These pull request must merge or be unlinked from the Bugzilla bug in order for it to move to the next state. Once unlinked, request a bug refresh with Bugzilla bug 2032926 has not been moved to the MODIFIED state. DetailsIn response to this:
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. |
|
@Fedosin: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
5 participants
Instead of a script, envtest started shipping as a compiled binary, so we have to updated our Makefile accordingly.
This bump is required to port CCCMO to k8s 1.23, because unit tests fail otherwise.