-
Notifications
You must be signed in to change notification settings - Fork 530
build: Use setup-envtest to download later version of envtest #1519
build: Use setup-envtest to download later version of envtest #1519
Conversation
6065b42
to
8cd39f2
Compare
`controller-runtime` provides a convenient tool to download various versions of k8s. This commit switches to using that to ensure we use a single version for testing in unit tests and in e2e tests, namely v1.21.x right now so to keep the version stable while just changing the installation mechanism.
8cd39f2
to
7f406a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one question, otherwise lgtm
export KUBEBUILDER_ASSETS="${dest_dir}" | ||
echo "Setting to KUBEBUILDER_ASSETS ${dest_dir}" | ||
go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest | ||
source <(setup-envtest use -p env 1.21.x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question as in #1516:
Why do we need to setup envtest here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kubebuilder releases used to bundle kubebuilder
binary along with kube-apiserver
, etcd
, kubectl
, etc. These stopped happening a long time ago, with version 2.3.2 iirc of kubebuilder (the version used in this project) bundling k8s v1.16 - too old for us now. Recent versions of kubebuilder no longer do that, instead distributing k8s binaries per version. Although you can define the URL to download directly, using setup-envtest
is a convenience that allows you to not /discover know the URL and change the minor version to test against as this PR now does. It's just to make things easier tbh...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh you mean why here! Well strictly we don't I guess could remove it but it is just replacing what was here before downloading kubebuilder provided kube-apiserver, etc. I don't mind removing it if you think it's best to though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Yeah, with this line the behaviour would be retained. I'm fine either way.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jimmidyson, makkes 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 |
controller-runtime
provides a convenient tool to download various versions of k8s. This commit switches to using that to ensure we use a single version for testing in unit tests and in e2e tests, namely v1.21.x right now so to keep the version stable while just changing the installation mechanism.