From d91693a3bbf03a58adb54263ca551a1e0fba2b24 Mon Sep 17 00:00:00 2001 From: Adem Baccara <71262172+Adembc@users.noreply.github.com> Date: Mon, 2 Sep 2024 20:06:03 +0100 Subject: [PATCH 1/4] test(ws): enhance e2e test setup and cleanup Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com> --- workspaces/controller/Makefile | 22 +++-- .../controller/test/e2e/e2e_suite_test.go | 70 +++++++++++++++- workspaces/controller/test/e2e/e2e_test.go | 55 +++++------- workspaces/controller/test/utils/utils.go | 84 ++++++++++++++++++- 4 files changed, 189 insertions(+), 42 deletions(-) diff --git a/workspaces/controller/Makefile b/workspaces/controller/Makefile index 6ce3d20e..c2078c31 100644 --- a/workspaces/controller/Makefile +++ b/workspaces/controller/Makefile @@ -63,11 +63,23 @@ vet: ## Run go vet against code. test: manifests generate fmt vet envtest ## Run tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out -# Utilize Kind or modify the e2e tests to load the image locally, enabling compatibility with other vendors. -.PHONY: test-e2e # Run the e2e tests against a Kind k8s instance that is spun up. -test-e2e: - @$(prompt_for_e2e_test_execution) - go test ./test/e2e/ -v -ginkgo.v +# TODO(user): To use a different vendor for e2e tests, modify the setup under 'tests/e2e'. +# The default setup assumes Kind is pre-installed and builds/loads the Manager Docker image locally. +# Prometheus and CertManager are installed by default; skip with: +# - PROMETHEUS_INSTALL_SKIP=true +# - CERT_MANAGER_INSTALL_SKIP=true +.PHONY: test-e2e +test-e2e: ## Run the e2e tests. Expected an isolated environment using Kind. + @command -v kind >/dev/null 2>&1 || { \ + echo "Kind is not installed. Please install Kind manually."; \ + exit 1; \ + } + @kind get clusters | grep -q 'kind' || { \ + echo "No Kind cluster is running. Please start a Kind cluster before running the e2e tests."; \ + exit 1; \ + } + PROMETHEUS_INSTALL_SKIP=true go test ./test/e2e/ -v -ginkgo.v + .PHONY: lint lint: golangci-lint ## Run golangci-lint linter & yamllint $(GOLANGCI_LINT) run diff --git a/workspaces/controller/test/e2e/e2e_suite_test.go b/workspaces/controller/test/e2e/e2e_suite_test.go index 39723f14..940ef84c 100644 --- a/workspaces/controller/test/e2e/e2e_suite_test.go +++ b/workspaces/controller/test/e2e/e2e_suite_test.go @@ -18,15 +18,83 @@ package e2e import ( "fmt" + "github.com/kubeflow/notebooks/workspaces/controller/test/utils" + "os" + "os/exec" "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) -// Run e2e tests using the Ginkgo runner. +var ( + // Optional Environment Variables: + // - PROMETHEUS_INSTALL_SKIP=true: Skips Prometheus Operator installation during test setup. + // - CERT_MANAGER_INSTALL_SKIP=true: Skips CertManager installation during test setup. + // These variables are useful if Prometheus or CertManager is already installed, avoiding + // re-installation and conflicts. + skipPrometheusInstall = os.Getenv("PROMETHEUS_INSTALL_SKIP") == "true" + skipCertManagerInstall = os.Getenv("CERT_MANAGER_INSTALL_SKIP") == "true" + // isPrometheusOperatorAlreadyInstalled will be set true when prometheus CRDs be found on the cluster + isPrometheusOperatorAlreadyInstalled = false + // isCertManagerAlreadyInstalled will be set true when CertManager CRDs be found on the cluster + isCertManagerAlreadyInstalled = false +) + +// TestE2E runs the end-to-end (e2e) test suite for the project. These tests execute in an isolated, +// temporary environment to validate project changes with the purposed to be used in CI jobs. +// The default setup requires Kind, builds/loads the Manager Docker image locally, and installs +// CertManager and Prometheus. func TestE2E(t *testing.T) { RegisterFailHandler(Fail) fmt.Fprintf(GinkgoWriter, "Starting workspace-controller suite\n") RunSpecs(t, "e2e suite") } + +var _ = BeforeSuite(func() { + By("building the controller image") + cmd := exec.Command("make", "docker-build", fmt.Sprintf("IMG=%s", controllerImage)) + _, err := utils.Run(cmd) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + By("loading the controller image on Kind") + err = utils.LoadImageToKindClusterWithName(controllerImage) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + // The tests-e2e are intended to run on a temporary cluster that is created and destroyed for testing. + // To prevent errors when tests run in environments with Prometheus or CertManager already installed, + // we check for their presence before execution. + // Setup Prometheus and CertManager before the suite if not skipped and if not already installed + if !skipPrometheusInstall { + By("checking if prometheus is installed already") + isPrometheusOperatorAlreadyInstalled = utils.IsPrometheusCRDsInstalled() + if !isPrometheusOperatorAlreadyInstalled { + _, _ = fmt.Fprintf(GinkgoWriter, "Installing Prometheus Operator...\n") + Expect(utils.InstallPrometheusOperator()).To(Succeed(), "Failed to install Prometheus Operator") + } else { + _, _ = fmt.Fprintf(GinkgoWriter, "WARNING: Prometheus Operator is already installed. Skipping installation...\n") + } + } + if !skipCertManagerInstall { + By("checking if cert manager is installed already") + isCertManagerAlreadyInstalled = utils.IsCertManagerCRDsInstalled() + if !isCertManagerAlreadyInstalled { + _, _ = fmt.Fprintf(GinkgoWriter, "Installing CertManager...\n") + Expect(utils.InstallCertManager()).To(Succeed(), "Failed to install CertManager") + } else { + _, _ = fmt.Fprintf(GinkgoWriter, "WARNING: CertManager is already installed. Skipping installation...\n") + } + } +}) + +var _ = AfterSuite(func() { + // Teardown Prometheus and CertManager after the suite if not skipped and if they were not already installed + if !skipPrometheusInstall && !isPrometheusOperatorAlreadyInstalled { + _, _ = fmt.Fprintf(GinkgoWriter, "Uninstalling Prometheus Operator...\n") + utils.UninstallPrometheusOperator() + } + if !skipCertManagerInstall && !isCertManagerAlreadyInstalled { + _, _ = fmt.Fprintf(GinkgoWriter, "Uninstalling CertManager...\n") + utils.UninstallCertManager() + } +}) diff --git a/workspaces/controller/test/e2e/e2e_test.go b/workspaces/controller/test/e2e/e2e_test.go index 52c7493b..4decdcba 100644 --- a/workspaces/controller/test/e2e/e2e_test.go +++ b/workspaces/controller/test/e2e/e2e_test.go @@ -66,9 +66,6 @@ var ( var _ = Describe("controller", Ordered, func() { BeforeAll(func() { - By("installing the cert-manager") - Expect(utils.InstallCertManager()).To(Succeed()) - projectDir, _ = utils.GetProjectDir() By("creating the controller namespace") @@ -85,6 +82,15 @@ var _ = Describe("controller", Ordered, func() { "-n", workspaceNamespace, ) _, _ = utils.Run(cmd) + + By("installing CRDs") + cmd = exec.Command("make", "install") + _, _ = utils.Run(cmd) + + By("deploying the controller-manager") + cmd = exec.Command("make", "deploy", fmt.Sprintf("IMG=%s", controllerImage)) + _, _ = utils.Run(cmd) + }) AfterAll(func() { @@ -101,6 +107,10 @@ var _ = Describe("controller", Ordered, func() { ) _, _ = utils.Run(cmd) + By("deleting the controller") + cmd = exec.Command("make", "undeploy") + _, _ = utils.Run(cmd) + By("deleting common workspace resources") cmd = exec.Command("kubectl", "delete", "-k", filepath.Join(projectDir, "config/samples/common"), @@ -116,16 +126,10 @@ var _ = Describe("controller", Ordered, func() { cmd = exec.Command("kubectl", "delete", "ns", workspaceNamespace) _, _ = utils.Run(cmd) - By("deleting the controller") - cmd = exec.Command("make", "undeploy") - _, _ = utils.Run(cmd) - By("deleting CRDs") cmd = exec.Command("make", "uninstall") _, _ = utils.Run(cmd) - By("uninstalling the cert-manager bundle") - utils.UninstallCertManager() }) Context("Operator", func() { @@ -134,29 +138,10 @@ var _ = Describe("controller", Ordered, func() { var controllerPodName string var err error - By("building the controller image") - cmd := exec.Command("make", "docker-build", fmt.Sprintf("IMG=%s", controllerImage)) - _, err = utils.Run(cmd) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - - By("loading the controller image on Kind") - err = utils.LoadImageToKindClusterWithName(controllerImage) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - - By("installing CRDs") - cmd = exec.Command("make", "install") - _, err = utils.Run(cmd) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - - By("deploying the controller-manager") - cmd = exec.Command("make", "deploy", fmt.Sprintf("IMG=%s", controllerImage)) - _, err = utils.Run(cmd) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - By("validating that the controller-manager pod is running as expected") verifyControllerUp := func() error { // Get controller pod name - cmd = exec.Command("kubectl", "get", "pods", + cmd := exec.Command("kubectl", "get", "pods", "-l", "control-plane=controller-manager", "-n", controllerNamespace, "-o", "go-template={{ range .items }}"+ @@ -192,7 +177,7 @@ var _ = Describe("controller", Ordered, func() { By("creating an instance of the WorkspaceKind CR") EventuallyWithOffset(1, func() error { - cmd = exec.Command("kubectl", "apply", + cmd := exec.Command("kubectl", "apply", "-f", filepath.Join(projectDir, "config/samples/jupyterlab_v1beta1_workspacekind.yaml"), ) _, err = utils.Run(cmd) @@ -201,7 +186,7 @@ var _ = Describe("controller", Ordered, func() { By("creating an instance of the Workspace CR") EventuallyWithOffset(1, func() error { - cmd = exec.Command("kubectl", "apply", + cmd := exec.Command("kubectl", "apply", "-f", filepath.Join(projectDir, "config/samples/jupyterlab_v1beta1_workspace.yaml"), "-n", workspaceNamespace, ) @@ -211,7 +196,7 @@ var _ = Describe("controller", Ordered, func() { By("validating that the workspace has 'Running' state") verifyWorkspaceState := func() error { - cmd = exec.Command("kubectl", "get", "workspaces", + cmd := exec.Command("kubectl", "get", "workspaces", workspaceName, "-n", workspaceNamespace, "-o", "jsonpath={.status.state}", @@ -237,7 +222,7 @@ var _ = Describe("controller", Ordered, func() { By("validating that the workspace pod is running as expected") verifyWorkspacePod := func() error { // Get workspace pod name - cmd = exec.Command("kubectl", "get", "pods", + cmd := exec.Command("kubectl", "get", "pods", "-l", fmt.Sprintf("notebooks.kubeflow.org/workspace-name=%s", workspaceName), "-n", workspaceNamespace, "-o", "go-template={{ range .items }}"+ @@ -348,7 +333,7 @@ var _ = Describe("controller", Ordered, func() { By("failing to delete an in-use WorkspaceKind") EventuallyWithOffset(1, func() error { - cmd = exec.Command("kubectl", "delete", "workspacekind", workspaceKindName) + cmd := exec.Command("kubectl", "delete", "workspacekind", workspaceKindName) _, err := utils.Run(cmd) return err }, timeout, interval).ShouldNot(Succeed()) @@ -362,7 +347,7 @@ var _ = Describe("controller", Ordered, func() { By("deleting an unused WorkspaceKind") EventuallyWithOffset(1, func() error { - cmd = exec.Command("kubectl", "delete", "workspacekind", workspaceKindName) + cmd := exec.Command("kubectl", "delete", "workspacekind", workspaceKindName) _, err := utils.Run(cmd) return err }, timeout, interval).Should(Succeed()) diff --git a/workspaces/controller/test/utils/utils.go b/workspaces/controller/test/utils/utils.go index 6641e4e3..0665fdd7 100644 --- a/workspaces/controller/test/utils/utils.go +++ b/workspaces/controller/test/utils/utils.go @@ -26,8 +26,13 @@ import ( ) const ( - // use LTS version of cert-manager + // use LTS version of prometheus-operator + prometheusOperatorVersion = "v0.72.0" + prometheusOperatorURL = "https://github.com/prometheus-operator/prometheus-operator/" + + "releases/download/%s/bundle.yaml" + + // use LTS version of cert-manager certManagerVersion = "v1.12.13" certManagerURLTmpl = "https://github.com/jetstack/cert-manager/releases/download/%s/cert-manager.yaml" ) @@ -56,6 +61,50 @@ func Run(cmd *exec.Cmd) ([]byte, error) { return output, nil } +// UninstallPrometheusOperator uninstalls the prometheus +func UninstallPrometheusOperator() { + url := fmt.Sprintf(prometheusOperatorURL, prometheusOperatorVersion) + cmd := exec.Command("kubectl", "delete", "-f", url) + if _, err := Run(cmd); err != nil { + warnError(err) + } +} + +// InstallPrometheusOperator installs the prometheus Operator to be used to export the enabled metrics. +func InstallPrometheusOperator() error { + url := fmt.Sprintf(prometheusOperatorURL, prometheusOperatorVersion) + cmd := exec.Command("kubectl", "create", "-f", url) + _, err := Run(cmd) + return err +} + +// IsPrometheusCRDsInstalled checks if any Prometheus CRDs are installed +// by verifying the existence of key CRDs related to Prometheus. +func IsPrometheusCRDsInstalled() bool { + // List of common Prometheus CRDs + prometheusCRDs := []string{ + "prometheuses.monitoring.coreos.com", + "prometheusrules.monitoring.coreos.com", + "prometheusagents.monitoring.coreos.com", + } + + cmd := exec.Command("kubectl", "get", "crds", "-o", "custom-columns=NAME:.metadata.name") + output, err := Run(cmd) + if err != nil { + return false + } + crdList := GetNonEmptyLines(string(output)) + for _, crd := range prometheusCRDs { + for _, line := range crdList { + if strings.Contains(line, crd) { + return true + } + } + } + + return false +} + // UninstallCertManager uninstalls the cert manager func UninstallCertManager() { url := fmt.Sprintf(certManagerURLTmpl, certManagerVersion) @@ -84,6 +133,39 @@ func InstallCertManager() error { return err } +// IsCertManagerCRDsInstalled checks if any Cert Manager CRDs are installed +// by verifying the existence of key CRDs related to Cert Manager. +func IsCertManagerCRDsInstalled() bool { + // List of common Cert Manager CRDs + certManagerCRDs := []string{ + "certificates.cert-manager.io", + "issuers.cert-manager.io", + "clusterissuers.cert-manager.io", + "certificaterequests.cert-manager.io", + "orders.acme.cert-manager.io", + "challenges.acme.cert-manager.io", + } + + // Execute the kubectl command to get all CRDs + cmd := exec.Command("kubectl", "get", "crds") + output, err := Run(cmd) + if err != nil { + return false + } + + // Check if any of the Cert Manager CRDs are present + crdList := GetNonEmptyLines(string(output)) + for _, crd := range certManagerCRDs { + for _, line := range crdList { + if strings.Contains(line, crd) { + return true + } + } + } + + return false +} + // LoadImageToKindClusterWithName loads a local docker image to the kind cluster func LoadImageToKindClusterWithName(name string) error { var cluster string From f403d9c0028ca2e8fdd5245c8c12fbe7c78357a5 Mon Sep 17 00:00:00 2001 From: Adem Baccara <71262172+Adembc@users.noreply.github.com> Date: Mon, 2 Sep 2024 20:11:41 +0100 Subject: [PATCH 2/4] remove prompt for e2e test execution Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com> --- workspaces/controller/Makefile | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/workspaces/controller/Makefile b/workspaces/controller/Makefile index c2078c31..cb978aed 100644 --- a/workspaces/controller/Makefile +++ b/workspaces/controller/Makefile @@ -207,26 +207,4 @@ echo "Downloading $${package}" ;\ GOBIN=$(LOCALBIN) go install $${package} ;\ mv "$$(echo "$(1)" | sed "s/-$(3)$$//")" $(1) ;\ } -endef - -define prompt_for_e2e_test_execution - if [ "$$(echo "$(KUBEFLOW_TEST_PROMPT)" | tr '[:upper:]' '[:lower:]')" = "false" ]; then \ - echo "Skipping E2E test confirmation prompt (KUBEFLOW_TEST_PROMPT is set to false)"; \ - else \ - current_k8s_context=$$(kubectl config current-context); \ - echo "================================ WARNING ================================"; \ - echo "E2E tests use your current Kubernetes context!"; \ - echo "This will DELETE EXISTING RESOURCES such as cert-manager!"; \ - echo "Current context: '$$current_k8s_context'"; \ - echo "========================================================================="; \ - echo "Proceed with E2E tests? (yes/NO)"; \ - read user_confirmation; \ - case $$user_confirmation in \ - [yY] | [yY][eE][sS] ) \ - echo "Running E2E tests...";; \ - [nN] | [nN][oO] | * ) \ - echo "Aborting E2E tests..."; \ - exit 1; \ - esac \ - fi -endef +endef \ No newline at end of file From 40e4c7b7b20838f1e53dd016481447da5800850f Mon Sep 17 00:00:00 2001 From: Adem Baccara <71262172+Adembc@users.noreply.github.com> Date: Tue, 10 Sep 2024 08:06:20 +0100 Subject: [PATCH 3/4] code review fixes Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com> --- workspaces/controller/Makefile | 1 + workspaces/controller/test/utils/utils.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/workspaces/controller/Makefile b/workspaces/controller/Makefile index cb978aed..f46f3a51 100644 --- a/workspaces/controller/Makefile +++ b/workspaces/controller/Makefile @@ -78,6 +78,7 @@ test-e2e: ## Run the e2e tests. Expected an isolated environment using Kind. echo "No Kind cluster is running. Please start a Kind cluster before running the e2e tests."; \ exit 1; \ } + # TODO: set PROMETHEUS_INSTALL_SKIP to `false` or remove `PROMETHEUS_INSTALL_SKIP=true` if we start using Prometheus PROMETHEUS_INSTALL_SKIP=true go test ./test/e2e/ -v -ginkgo.v .PHONY: lint diff --git a/workspaces/controller/test/utils/utils.go b/workspaces/controller/test/utils/utils.go index 0665fdd7..a23ebf17 100644 --- a/workspaces/controller/test/utils/utils.go +++ b/workspaces/controller/test/utils/utils.go @@ -88,7 +88,7 @@ func IsPrometheusCRDsInstalled() bool { "prometheusagents.monitoring.coreos.com", } - cmd := exec.Command("kubectl", "get", "crds", "-o", "custom-columns=NAME:.metadata.name") + cmd := exec.Command("kubectl", "get", "crds", "-o", "name") output, err := Run(cmd) if err != nil { return false @@ -147,7 +147,7 @@ func IsCertManagerCRDsInstalled() bool { } // Execute the kubectl command to get all CRDs - cmd := exec.Command("kubectl", "get", "crds") + cmd := exec.Command("kubectl", "get", "crds", "-o", "name") output, err := Run(cmd) if err != nil { return false From 21f6551b1179e9e886bf9af166ec038b78e44952 Mon Sep 17 00:00:00 2001 From: Adem Baccara <71262172+Adembc@users.noreply.github.com> Date: Tue, 10 Sep 2024 19:48:14 +0100 Subject: [PATCH 4/4] remove env variable from e2e test GHA Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com> --- .github/workflows/controller-tests.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/controller-tests.yaml b/.github/workflows/controller-tests.yaml index 2fe08c5b..f8bca40c 100644 --- a/.github/workflows/controller-tests.yaml +++ b/.github/workflows/controller-tests.yaml @@ -46,7 +46,5 @@ jobs: cache-dependency-path: workspaces/controller/go.sum - name: Run e2e tests - env: - KUBEFLOW_TEST_PROMPT: "false" working-directory: workspaces/controller run: make test-e2e