-
Notifications
You must be signed in to change notification settings - Fork 1
Drop integration tests, add e2e tests with kind #7
Conversation
Makefile
Outdated
e2e-tests: | ||
kind delete cluster --name "ros-e2e" | ||
kind create cluster --name "ros-e2e" | ||
kubectl cluster-info --context kind-ros-e2e |
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.
gah external_ip succeds locally but it doesn't on the runner, have to check why
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.
wow, I mean, its cool but a bit overboard with a lot of functions that we dont need. Looks good to me thought, just a couple of nits and a (future?) implementation issue.
yup, I'm indeed not sure if just cut what we need here or just port it as a standalone lib. At the end, I see us reusing that code parts in the e2e tests and we could just reuse that code to quickly ramp-up without have to re-implement such helpers ourselves. |
8340963
to
6215a7b
Compare
- Also add nginx as ingress as is more compliant to kind - Move out some common code pieces - Address points from review Signed-off-by: Ettore Di Giacinto <edigiacinto@suse.com>
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.
2 comments a little bit late :) but as my bootstrapping tests conversion code to ginkgo as some similarities I will try to use the https://github.com/rancher-sandbox/ele-testhelpers lib.
In my current version I use some code from Epinio, but this lib could be more powerful. My main concern is to use kubectl
command and not API, which looks like to be the case in the lib.
|
||
var _ = Describe("MachineRegistration e2e tests", func() { | ||
k := kubectl.New() | ||
Context("registration", func() { |
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.
Is Context
really useful here? As there is only this one (or maybe more are planned?).
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.
I hoped there was more in the pipeline for that change at hand 😅 but this will get bigger I'm pretty sure..
Yes, that is done in purpose to avoid to pinpoint kubernetes api versions in all the code consuming it downstream. It uses also standalone kubectl so we can just setup matrix tests without caring about the api versions attached to the test code |
Yes, and it's better to use the command like a real user instead of the API for e2e tests IMO :-) |
Fixes: #2
Fixes: #6
Need also to cleanup a bit
Signed-off-by: Ettore Di Giacinto edigiacinto@suse.com