🌱 Revert "🐛 Check cluster is running for non-standalone Make targets (#…

[camilamacedo86]

Reverts #437

If we see the logs https://github.com/operator-framework/catalogd/actions/runs/11819448861/job/32929383546 (ci job of PR where the change was introduced), we can see that this check is wrong and is always failing:

PS.: I don't think there's a need for this check. Proof of that is that besides it does not work, nothing changed. Therefore, I do not see value in this one.
Instead, I would prefer the targets of this project to be as similar as possible to those of https://github.com/operator-framework/operator-controller so that life for us contributors is easier.

[camilamacedo86]

HI @joelanford

That job didn't fail.

Not because. As far as I can see, the check is wrong and not helpful for anything.
Just bring the noise in the command line every time we run.

Do you see any reason to keep it?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 38.23%. Comparing base (4dd7d59) to head (60a9dd3).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #465   +/-   ##
=======================================
  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.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[bentito]

HI @joelanford

That job didn't fail.

Not because. As far as I can see, the check is wrong and not helpful for anything. Just bring the noise in the command line every time we run.

Do you see any reason to keep it?

The check didn't fail AND reason it's needed:

The other targets that are calling this, they fail cryptically if there is no cluster available. Check the original PR for example. With this check you can at least get a nice error when those targets don't find the cluster they expect.

[bentito]

Just bring the noise in the command line every time we run.

no noise, only makes noise if cluster not found.

[camilamacedo86]

Hi @bentito

no noise, only makes noise if cluster not found.

So, is the cluster not found in our e2e tests?
Should it not be found in our e2e tests?

[m1kola]

Something is not right with the check - that is for sure. I currently do not have a cluster and the check returns 0 exit code (example below).

Also I agree that this would not be showing up in CI from the example above.

# make check-cluster
if ! kubectl config current-context >/dev/null 2>&1; then \
		echo "Error: Could not get current Kubernetes context. Maybe use 'run' or 'e2e' targets first?"; \
		exit 1; \
	fi
# echo $?
0

[bentito]

Something is not right with the check - that is for sure. I currently do not have a cluster and the check returns 0 exit code (example below).

Also I agree that this would not be showing up in CI from the example above.

# make check-cluster
if ! kubectl config current-context >/dev/null 2>&1; then \
		echo "Error: Could not get current Kubernetes context. Maybe use 'run' or 'e2e' targets first?"; \
		exit 1; \
	fi
# echo $?
0

Yeah seem like the logic got borked somewhere along the way:

#466 fixes it:

with that change it looks like this:

catalogd main $ make check-cluster
catalogd main $ kind delete clusters catalogd
Deleted nodes: ["catalogd-control-plane"]
Deleted clusters: ["catalogd"]
catalogd main $ make check-cluster
Error: Could not get current Kubernetes context. Maybe use 'run' or 'e2e' targets first?
make: *** [check-cluster] Error 1

[camilamacedo86]

Hi @bentito

Why do we need this check in the first place?
We have not it working well, and all is fine
We do not have it in the operator-controller; all works fine.
We are unable to run the staff without BINGO, so we cannot have the freedom of not using the targets

[camilamacedo86]

anyway #466 was merged so I will close this one
If I face issues locally and etc then we open another