-
Notifications
You must be signed in to change notification settings - Fork 32
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
🐛 Check cluster is running for non-standalone Make targets #437
🐛 Check cluster is running for non-standalone Make targets #437
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #437 +/- ##
=======================================
Coverage 38.23% 38.23%
=======================================
Files 15 15
Lines 1224 1224
=======================================
Hits 468 468
Misses 706 706
Partials 50 50 ☔ View full report in Codecov by Sentry. |
Makefile
Outdated
@@ -102,7 +102,7 @@ FOCUS := $(if $(TEST),-v -focus "$(TEST)") | |||
ifeq ($(origin E2E_FLAGS), undefined) | |||
E2E_FLAGS := | |||
endif | |||
test-e2e: $(GINKGO) ## Run the e2e tests | |||
test-e2e: check-kind-cluster $(GINKGO) ## Run the e2e tests |
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 check might be a bit too presumptuous. For example, someone running test-e2e
might want to run the tests after running kind create cluster --name=foobar
and tilt up
.
It may even be that someone wants to run the e2es against a non-kind cluster that already has OLM deployed.
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.
Changed to make it more generic like this:
.PHONY: check-cluster
check-cluster:
if ! kubectl config current-context >/dev/null 2>&1; then \
echo "Error: Could not get current Kubernetes context. Use 'run' or 'e2e' targets first."; \
exit 1; \
fi
wdyt?
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.
okay I changed the output to simply be:
echo "Error: Could not get current Kubernetes context. Maybe use 'run' or 'e2e' targets first?";
and check-cluster
is called now from test-e2e
, install
and kind-load
all of which really need some cluster running or they produce a non-descriptive and cryptic error message.
Signed-off-by: Brett Tofel <[email protected]>
Co-authored-by: Jordan Keister <[email protected]>
Co-authored-by: Jordan Keister <[email protected]>
Signed-off-by: Brett Tofel <[email protected]>
Signed-off-by: Brett Tofel <[email protected]>
7564eb8
to
2f186ef
Compare
Hi @bentito If you see the logs https://github.com/operator-framework/catalogd/actions/runs/11819448861/job/32929383546 (ci job of this PR)you will see that this check is wrong and is always failing: I do not see the need for this check. c/c @grokspawn |
)" This reverts commit b30eb4a.
)" This reverts commit b30eb4a.
We have several non-standlone Make targets
The above targets rely on Kind to be running.
Unfortunately, currently, errors returned from the Go code are pretty cryptic. Better would be an explicit catch and error when a Kind cluster is not found when these targets are run.
Currently we have:
wha?
but it would be better to have: