-
Notifications
You must be signed in to change notification settings - Fork 289
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
Update and fix e2e-vault #234
Update and fix e2e-vault #234
Conversation
Welcome @mitsutaka! |
Hi @mitsutaka. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
Makefile
Outdated
.PHONY: e2e-bootstrap | ||
e2e-bootstrap: install-helm | ||
apt-get update && apt-get install bats && apt-get install gettext-base -y | ||
# Download and install Vault | ||
curl -LO https://releases.hashicorp.com/vault/${VAULT_VERSION}/vault_${VAULT_VERSION}_linux_amd64.zip && unzip vault_${VAULT_VERSION}_linux_amd64.zip && chmod +x vault && mv vault /usr/local/bin/ | ||
command -v vault || (curl -LO https://releases.hashicorp.com/vault/${VAULT_VERSION}/vault_${VAULT_VERSION}_linux_amd64.zip && unzip vault_${VAULT_VERSION}_linux_amd64.zip && chmod +x vault && mv vault /usr/local/bin/) |
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.
We should check the version here as well.
Makefile
Outdated
@@ -91,8 +91,9 @@ endif | |||
.PHONY: setup-kind | |||
setup-kind: | |||
# Download and install kind | |||
curl -L https://github.com/kubernetes-sigs/kind/releases/download/v${KIND_VERSION}/kind-linux-amd64 --output kind && chmod +x kind && mv kind /usr/local/bin/ | |||
command -v kind || (curl -L https://github.com/kubernetes-sigs/kind/releases/download/v${KIND_VERSION}/kind-linux-amd64 --output kind && chmod +x kind && mv kind /usr/local/bin/) |
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.
Check the version
/ok-to-test |
@mitsutaka Thank you for the PR! 🎉 Looks great! |
In PR kubernetes-sigs/secrets-store-csi-driver#234, we are going to update kind. Since kind is no longer supported `kind get kubeconfig-path`, kubectl context is automatically loaded and kubeconfig will be saved at `$HOME/.kube/config`.
01ea8e5
to
bafbe51
Compare
* Update latest tools for e2e. Install commands if it does not exist. * Fix wrong usage of arithmetic binary operators usage in bats it made wrong assets. * Update latest usage of `kubectl exec`.
bafbe51
to
c269bd0
Compare
/retest |
Going to run windows tests too for my sanity as there are changes in Makefile /test pull-secrets-store-csi-driver-e2e-windows |
Thank you for the PR, @mitsutaka!
This will be a great improvement. I think it'll also make it easier for contributors to add new test cases and ensure complete coverage for all features added. Would you be interested in starting a proposal on a new test framework that doesn't depend on bats? We are in the process of setting up community call for the project - https://kubernetes.slack.com/archives/C013PUP2WRK/p1592844755016800 and this will be a great topic for discussion. We can get input for all other contributors on the proposal before implementing it. Let us know how we can help. |
/lgtm |
Thanks @aramase and @ritazh for reviewing! I’m interested in building a new test framework because I am going to use secret-store-csi in production cluster. I would like to improve reliability and stability with contributors. Candidate time is midnight in my time. I might be hard to join but I would follow your discussions if summary is posted in somewhere. |
@mitsutaka Feel free to start a discussion either in a new GH issue or on slack or we can also review and collaborate in google docs. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mitsutaka, ritazh 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 |
In PR kubernetes-sigs/secrets-store-csi-driver#234, we are going to update kind. Since kind is no longer supported `kind get kubeconfig-path`, kubectl context is automatically loaded and kubeconfig will be saved at `$HOME/.kube/config`.
What this PR does / why we need it:
kubectl exec
to dismiss the deprecated warning.Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #
Special notes for your reviewer:
As a long-term improvement, I'm thinking to replace bats based e2e with
onsi/ginkgo
to avoid using bash that is hard to notice.