Skip to content

🌱 Add prometheus to e2e workflow #1928

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🌱 Add prometheus to e2e workflow #1928

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions Makefile
View file
Original file line number Diff line number Diff line change
@@ -252,6 +252,24 @@ test-e2e: KUSTOMIZE_BUILD_DIR := config/overlays/e2e
test-e2e: GO_BUILD_EXTRA_FLAGS := -cover
test-e2e: run image-registry e2e e2e-coverage kind-clean #HELP Run e2e test suite on local kind cluster

.PHONY: test-e2e-metrics
test-e2e-metrics: KIND_CLUSTER_NAME := operator-controller-e2e
test-e2e-metrics: KUSTOMIZE_BUILD_DIR := config/overlays/e2e
test-e2e-metrics: GO_BUILD_EXTRA_FLAGS := -cover
test-e2e-metrics: run image-registry prometheus e2e e2e-coverage kind-clean #HELP Run e2e test suite on local kind cluster with prometheus installed

.PHONY: prometheus
prometheus: PROMETHEUS_NAMESPACE := prometheus
prometheus: TMPDIR := $(shell mktemp -d)
prometheus: LATEST := $(shell curl -s https://api.github.com/repos/prometheus-operator/prometheus-operator/releases/latest | jq -cr .tag_name)
prometheus: ## Deploy Prometheus into 'prometheus' namespace
trap 'echo "Cleaning up $(TMPDIR)"; rm -rf "$(TMPDIR)"' EXIT; \
kubectl create ns $(PROMETHEUS_NAMESPACE); \
curl -s "https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/refs/tags/$(LATEST)/kustomization.yaml" > "$(TMPDIR)/kustomization.yaml" ; \
curl -s "https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/refs/tags/$(LATEST)/bundle.yaml" > "$(TMPDIR)/bundle.yaml" ; \
(cd $(TMPDIR) && $(KUSTOMIZE) edit set namespace $(PROMETHEUS_NAMESPACE)) && kubectl create -k "$(TMPDIR)" ; \
kubectl wait --for=condition=Ready pods -n $(PROMETHEUS_NAMESPACE) -l app.kubernetes.io/name=prometheus-operator
Comment on lines +264 to +271
Copy link
Member

Choose a reason for hiding this comment

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

Did you consider adding the prometheus setup/teardown into the e2e's test_main.go? https://github.com/openshift/operator-framework-operator-controller/blob/355dcf40d5fcb4d9e93ab312a376c4e80d46ba76/test/e2e/e2e_suite_test.go#L31-L40

That would keep the Makefile a bit cleaner, and it would be a bit easier to deal with the tmpdir cleanup.

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 think this would go against our convention of setting up and tearing down everything in the Makefile. If people want to make changes to the e2e environment, they're going to be looking for things in here, and not in the go test files IMO.

Wouldn't that also add prometheus to every test run? I was hoping not to do that. I assume there's probably ways to turn it off/on in the go test calls, but then that adds more complication, and then we'll need to add additional targets in the Makefile for that too.

If your goal is just to keep the Makefile clean, I would be happy to compromise by adding a script instead and calling it from the Makefile. It's mainly just that putting it into the go test files doesn't feel right to me.

Copy link
Member

@joelanford joelanford Apr 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this would go against our convention of setting up and tearing down everything in the Makefile. If people want to make changes to the e2e environment, they're going to be looking for things in here, and not in the go test files IMO.

That's fair according to how everything else works. If we do want to reorganize and move setup/teardown out of Makefile, that does feel like a separate PR.

Wouldn't that also add prometheus to every test run? I was hoping not to do that. I assume there's probably ways to turn it off/on in the go test calls, but then that adds more complication, and then we'll need to add additional targets in the Makefile for that too.

That could be managed with an environment variable. However, once we get to the point where prometheus and alert manager are actually used in tests, I assume we'll want prometheus unconditionally enabled.

If your goal is just to keep the Makefile clean, I would be happy to compromise by adding a script instead and calling it from the Makefile. It's mainly just that putting it into the go test files doesn't feel right to me.

No need for a script for this PR. If folks are worried about the Makefile sprawl, let's tackle that separately.


.PHONY: extension-developer-e2e
extension-developer-e2e: KUSTOMIZE_BUILD_DIR := config/overlays/cert-manager
extension-developer-e2e: KIND_CLUSTER_NAME := operator-controller-ext-dev-e2e