-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add script in the repo to setup envtest #1608
feat: add script in the repo to setup envtest #1608
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86 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 |
38b5fbc
to
e296f12
Compare
/assign @pwittrock |
scripts/setup_envtest_bins.sh
Outdated
# In this way, to have the kube-apiserver is required to build it locally | ||
# if the project is cloned locally already do nothing | ||
if [ ! -d $GOPATH/src/k8s.io/kubernetes ]; then | ||
git clone https://github.com/kubernetes/kubernetes $GOPATH/src/k8s.io/kubernetes --depth=1 -b v1.18.2 |
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.
should this use the K8S_VER env var?
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.
good catcher.
scripts/setup_envtest_bins.sh
Outdated
# To use envtest is required to have etcd, kube-apiserver and kubetcl binaries installed locally. | ||
# This script will create the directory testbin and perform this setup for linux or mac os x envs in | ||
|
||
K8S_VER=v1.18.2 |
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.
This value should be a script parameter ideally.
e296f12
to
c87bc6a
Compare
c87bc6a
to
28e92a5
Compare
# This script will create the directory testbin and perform this setup for linux or mac os x envs in | ||
|
||
# Kubernetes version e.g v1.18.2 | ||
K8S_VER=$1 |
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.
As a follow up consider defaulting these if they are unset. Maybe keep as env var
/lgtm |
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.
This needs documentation @camilamacedo86.
@estroz I still think that we need the new target in the Makefile that will call this script as you suggested. |
Users shouldn't have to download kubebuilder's source to get this file. Its download and use should be documented in the quickstart. See #1600 (review). Edit: I may be misunderstanding the problem this script is trying to solve. I did not realize upstream server binaries were being packaged with the kubebuilder binary. |
This pr is just for we add the script that will be used in the solution to set up the env test locally for we are able to close #1599. See: