Skip to content
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

chore: use setup-envtest to bootstrap EnvTest #431

Merged
merged 1 commit into from
Dec 20, 2022

Conversation

erikgb
Copy link
Contributor

@erikgb erikgb commented Nov 28, 2022

What this PR does / why we need it:

This replaces the shell script installation of etcd and kube-apiserver with setup-envtest.

Also cleaning out some docs that are outdated by this change.

Upgrading to K8s version 1.25.0, which is the latest release of kubebuilder-tools.

Fixes #428

@erikgb erikgb force-pushed the chore/setup-envtest branch 4 times, most recently from c27a621 to fd2b3be Compare November 28, 2022 19:49
@erikgb erikgb changed the title WIP: chore: use setup-envtest to bootstrap testEnv chore: use setup-envtest to bootstrap testEnv Nov 28, 2022
@erikgb erikgb changed the title chore: use setup-envtest to bootstrap testEnv chore: use setup-envtest to bootstrap EnvTest Nov 28, 2022
@kensipe
Copy link
Member

kensipe commented Nov 28, 2022

I'm confused why this is passing tests... but it could be platform differences. I am unable to run integration-tests nor e2e.
the e2e fails with ARM issues:

unable to fetch checksum for requested version: unable fetch metadata for kubebuilder-tools-1.19.2-darwin-arm64.tar.gz -- got status "404 Not Found" from GCS
KUBEBUILDER_ASSETS="" ./hack/run-e2e-tests.sh

I will look into ARM options for around this timeframe. This likely predates Mac M1.
If we are unable to find a solution, we may need to hold on this and find a more modern solution (later version of k8s)

@kensipe
Copy link
Member

kensipe commented Nov 28, 2022

this is part of the issue with having the build tool manage these details. It is great for evergreen, leading edge dev. That said, I'm a fail of env-test (which was influx around M1 and over multiple versions of k8s). I think most of that has been worked out. I should add, I had issues local with integration tests as well... but for the same reason.

@kensipe
Copy link
Member

kensipe commented Nov 28, 2022

btw

./bin/setup-envtest list
(available)  v1.25.0  darwin/arm64
(available)  v1.24.2  darwin/arm64
(available)  v1.24.1  darwin/arm64

😞

@erikgb
Copy link
Contributor Author

erikgb commented Nov 28, 2022

Maybe upgrade K8s to 1.24.2 before merging this then?

@kensipe
Copy link
Member

kensipe commented Dec 7, 2022

@erikgb for awareness... I'm working to upgrade our CRDs to V1 then moving to the latest version of testenv. Once this is in place, I believe we will be ready to merge this.

@erikgb erikgb force-pushed the chore/setup-envtest branch 2 times, most recently from ec6b2a6 to b398a5d Compare December 10, 2022 14:11
@erikgb
Copy link
Contributor Author

erikgb commented Dec 10, 2022

@kensipe Thanks for fixing the requirements to go beyond K8s version 1.19! Somehow I cannot run the integration tests locally now, even with K8s version 1.19.2. It just hangs, and that worked before rebasing the PR. 🤔 But CI is happy, so I suggest moving forward. Curious if running the integration tests works for you now?

@kensipe
Copy link
Member

kensipe commented Dec 20, 2022

the integration test works fine for me... reviewing again now. but looks good

Copy link
Member

@kensipe kensipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @erikgb for this contribution... it is really great! After (needed) updates to the CRDs etc... this works great. On M1+ k8s 1.24+ is needed... making the 1.25.0 a great starting point. I've added a comment to address but will merge with your attribution and mod tomorrow. I wanted to make sure you had a day to respond (which given the holiday season may be somewhat short response time).

Makefile Outdated

# Run e2e tests
.PHONY: e2e-test
e2e-test: cli
./hack/run-e2e-tests.sh
e2e-test: cli envtest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the e2e-test is not a rerun of the integration tests... I think there was some confusion around this. The e2e tests should NOT be dependent on the envtest. e2e requires docker + kind. Although the refactor of the circleci -> git action lost that.... which is why we continue to have a circleci task along with the github actions. I'm not sure we need to solve all of that in this PR but we should revert this portion. I will wait a day for a response... if none, I will merge and mod, then create a PR for the e2e to be on github actions (assuming it's possible... It will be an area I will need to research).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the e2e-test make target.

@kensipe
Copy link
Member

kensipe commented Dec 20, 2022

well... It looks like unintentionally mislead... I'm not sure when the e2e changed. it is using envtest but in a dockerized env. that doesn't seem necessary and we do want a test env with kind for a real e2e. I can see why there was confusion. At this point we should remove the e2e and create a todo to add in a real e2e. I have to assume that we had the real e2e on the kudo side... it seems best to disconnect from kudo and create our own e2e. more clean up to do, but will result in a cleaner project. Please remove the e2e target... but as previously mentioned... I'm happy to do that.

Signed-off-by: Erik Godding Boye <egboye@gmail.com>
@erikgb
Copy link
Contributor Author

erikgb commented Dec 20, 2022

@kensipe I've removed the e2e-test make target, but unsure where to put the TODO for migrating the e2e tests to real e2e tests using kind. Please advice!

@kensipe
Copy link
Member

kensipe commented Dec 20, 2022

kind of you sir! I will create a github issue to capture it and schedule into a milestone

Copy link
Member

@kensipe kensipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@kensipe kensipe merged commit d44691a into kudobuilder:main Dec 20, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GH Action Upgrade test K8S Version
3 participants