test(ws): enhance e2e test setup and cleanup (#39)

* test(ws): enhance e2e test setup and cleanup

Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>

* code review fixes

Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>

* remove env variable from e2e test GHA

Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>

* lint fixes

Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>

* mathew updates 1

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

---------

Signed-off-by: Adem Baccara <71262172+Adembc@users.noreply.github.com>
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
Co-authored-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>

.github/workflows/controller-tests.yaml

@@ -80,7 +80,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

workspaces/controller/Makefile

@@ -63,9 +63,12 @@ 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
 
+# 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: manifests generate fmt vet ## Run the e2e tests. Expected an isolated environment using Kind.
-	@$(prompt_for_e2e_test_execution)
 	@command -v kind >/dev/null 2>&1 || { \
 		echo "Kind is not installed. Please install Kind manually."; \
 		exit 1; \
@@ -206,25 +209,3 @@ mv $(1) $(1)-$(3) ;\
 } ;\
 ln -sf $(1)-$(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

workspaces/controller/test/e2e/e2e_suite_test.go

@@ -18,15 +18,89 @@ package e2e
 
 import (
 	"fmt"
+	"os"
+	"os/exec"
 	"testing"
 
+	"github.com/kubeflow/notebooks/workspaces/controller/test/utils"
+
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
-// Run e2e tests using the Ginkgo runner.
+var (
+	// These variables are useful to avoid re-installation and conflicts:
+	//  - PROMETHEUS_INSTALL_SKIP=true: Skips Prometheus installation during test setup.
+	//  - CERT_MANAGER_INSTALL_SKIP=true: Skips CertManager installation during test setup.
+	skipCertManagerInstall = os.Getenv("CERT_MANAGER_INSTALL_SKIP") == "true"
+	// skipPrometheusInstall  = os.Getenv("PROMETHEUS_INSTALL_SKIP") == "true"
+
+	// isCertManagerAlreadyInstalled will be set true when CertManager CRDs be found on the cluster
+	isCertManagerAlreadyInstalled = false
+
+	// isPrometheusOperatorAlreadyInstalled will be set true when prometheus CRDs be found on the cluster
+	// isPrometheusOperatorAlreadyInstalled = 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())
+
+	// TODO: enable Prometheus installation once we start using it
+	// 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")
+	// 	}
+	// }
+	// By("checking that prometheus is running")
+	// Expect(utils.WaitPrometheusOperatorRunning()).To(Succeed(), "Prometheus Operator is not running")
+
+	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")
+		}
+	}
+	By("checking that cert manager is running")
+	Expect(utils.WaitCertManagerRunning()).To(Succeed(), "CertManager is not running")
+})
+
+var _ = AfterSuite(func() {
+
+	// if !skipPrometheusInstall && !isPrometheusOperatorAlreadyInstalled {
+	// 	By("uninstalling Prometheus Operator")
+	// 	_, _ = fmt.Fprintf(GinkgoWriter, "Uninstalling Prometheus Operator...\n")
+	// 	utils.UninstallPrometheusOperator()
+	// }
+
+	if !skipCertManagerInstall && !isCertManagerAlreadyInstalled {
+		By("uninstalling CertManager")
+		_, _ = fmt.Fprintf(GinkgoWriter, "Uninstalling CertManager...\n")
+		utils.UninstallCertManager()
+	}
+})

workspaces/controller/test/e2e/e2e_test.go

@@ -66,97 +66,39 @@ 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")
 		cmd := exec.Command("kubectl", "create", "ns", controllerNamespace)
-		_, _ = utils.Run(cmd)
+		_, _ = utils.Run(cmd) // ignore errors because namespace may already exist
 
 		By("creating the workspace namespace")
 		cmd = exec.Command("kubectl", "create", "ns", workspaceNamespace)
-		_, _ = utils.Run(cmd)
+		_, _ = utils.Run(cmd) // ignore errors because namespace may already exist
 
 		By("creating common workspace resources")
 		cmd = exec.Command("kubectl", "apply",
 			"-k", filepath.Join(projectDir, "config/samples/common"),
 			"-n", workspaceNamespace,
 		)
-		_, _ = utils.Run(cmd)
+		_, err := utils.Run(cmd)
-	})
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
-
-	AfterAll(func() {
-		By("deleting sample Workspace")
-		cmd := exec.Command("kubectl", "delete", "-f",
-			filepath.Join(projectDir, "config/samples/jupyterlab_v1beta1_workspace.yaml"),
-			"-n", workspaceNamespace,
-		)
-		_, _ = utils.Run(cmd)
-
-		By("deleting sample WorkspaceKind")
-		cmd = exec.Command("kubectl", "delete",
-			"-f", filepath.Join(projectDir, "config/samples/jupyterlab_v1beta1_workspacekind.yaml"),
-		)
-		_, _ = utils.Run(cmd)
-
-		By("deleting common workspace resources")
-		cmd = exec.Command("kubectl", "delete",
-			"-k", filepath.Join(projectDir, "config/samples/common"),
-			"-n", workspaceNamespace,
-		)
-		_, _ = utils.Run(cmd)
-
-		By("deleting controller namespace")
-		cmd = exec.Command("kubectl", "delete", "ns", controllerNamespace)
-		_, _ = utils.Run(cmd)
-
-		By("deleting workspace namespace")
-		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() {
-
-		It("should run successfully", 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)
-			Expect(err).NotTo(HaveOccurred())
-
-			By("loading the controller image on Kind")
-			err = utils.LoadImageToKindClusterWithName(controllerImage)
-			Expect(err).NotTo(HaveOccurred())
 
 		By("installing CRDs")
 		cmd = exec.Command("make", "install")
 		_, err = utils.Run(cmd)
-			Expect(err).NotTo(HaveOccurred())
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
 
 		By("deploying the controller-manager")
 		cmd = exec.Command("make", "deploy", fmt.Sprintf("IMG=%s", controllerImage))
 		_, err = utils.Run(cmd)
-			Expect(err).NotTo(HaveOccurred())
+		ExpectWithOffset(1, err).NotTo(HaveOccurred())
 
 		By("validating that the controller-manager pod is running as expected")
+		var controllerPodName string
 		verifyControllerUp := func(g Gomega) {
 			// 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 }}"+
@@ -185,30 +127,75 @@ var _ = Describe("controller", Ordered, func() {
 		}
 		Eventually(verifyControllerUp, timeout, interval).Should(Succeed())
 
-			By("creating an instance of WorkspaceKind")
+	})
-			createWorkspaceKindSample := func() error {
+
-				cmd = exec.Command("kubectl", "apply",
+	AfterAll(func() {
+		By("deleting sample Workspace")
+		cmd := exec.Command("kubectl", "delete", "-f",
+			filepath.Join(projectDir, "config/samples/jupyterlab_v1beta1_workspace.yaml"),
+			"-n", workspaceNamespace,
+		)
+		_, _ = utils.Run(cmd)
+
+		By("deleting sample WorkspaceKind")
+		cmd = exec.Command("kubectl", "delete",
 			"-f", filepath.Join(projectDir, "config/samples/jupyterlab_v1beta1_workspacekind.yaml"),
 		)
-				_, err = utils.Run(cmd)
+		_, _ = 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"),
+			"-n", workspaceNamespace,
+		)
+		_, _ = utils.Run(cmd)
+
+		By("deleting controller namespace")
+		cmd = exec.Command("kubectl", "delete", "ns", controllerNamespace)
+		_, _ = utils.Run(cmd)
+
+		By("deleting workspace namespace")
+		cmd = exec.Command("kubectl", "delete", "ns", workspaceNamespace)
+		_, _ = utils.Run(cmd)
+
+		By("deleting CRDs")
+		cmd = exec.Command("make", "uninstall")
+		_, _ = utils.Run(cmd)
+
+	})
+
+	Context("Operator", func() {
+
+		It("should run successfully", func() {
+
+			By("creating an instance of WorkspaceKind")
+			createWorkspaceKindSample := func() error {
+				cmd := exec.Command("kubectl", "apply",
+					"-f", filepath.Join(projectDir, "config/samples/jupyterlab_v1beta1_workspacekind.yaml"),
+				)
+				_, err := utils.Run(cmd)
 				return err
 			}
 			Eventually(createWorkspaceKindSample, timeout, interval).Should(Succeed())
 
 			By("creating an instance of Workspace")
 			createWorkspaceSample := func() error {
-				cmd = exec.Command("kubectl", "apply",
+				cmd := exec.Command("kubectl", "apply",
 					"-f", filepath.Join(projectDir, "config/samples/jupyterlab_v1beta1_workspace.yaml"),
 					"-n", workspaceNamespace,
 				)
-				_, err = utils.Run(cmd)
+				_, err := utils.Run(cmd)
 				return err
 			}
 			Eventually(createWorkspaceSample, timeout, interval).Should(Succeed())
 
 			By("validating that the workspace has 'Running' state")
 			verifyWorkspaceState := func(g Gomega) error {
-				cmd = exec.Command("kubectl", "get", "workspaces",
+				cmd := exec.Command("kubectl", "get", "workspaces",
 					workspaceName,
 					"-n", workspaceNamespace,
 					"-o", "jsonpath={.status.state}",
@@ -234,7 +221,7 @@ var _ = Describe("controller", Ordered, func() {
 			By("validating that the workspace pod is running as expected")
 			verifyWorkspacePod := func(g Gomega) {
 				// 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 }}"+
@@ -340,7 +327,7 @@ var _ = Describe("controller", Ordered, func() {
 
 			By("failing to delete an in-use WorkspaceKind")
 			deleteInUseWorkspaceKind := func() error {
-				cmd = exec.Command("kubectl", "delete", "workspacekind", workspaceKindName)
+				cmd := exec.Command("kubectl", "delete", "workspacekind", workspaceKindName)
 				_, err := utils.Run(cmd)
 				return err
 			}
@@ -356,7 +343,7 @@ var _ = Describe("controller", Ordered, func() {
 
 			By("deleting an unused WorkspaceKind")
 			deleteWorkspaceKind := func() error {
-				cmd = exec.Command("kubectl", "delete", "workspacekind", workspaceKindName)
+				cmd := exec.Command("kubectl", "delete", "workspacekind", workspaceKindName)
 				_, err := utils.Run(cmd)
 				return err
 			}

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,63 @@ func Run(cmd *exec.Cmd) (string, error) {
 	return string(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", "apply", "-f", url)
+	_, err := Run(cmd)
+	return err
+}
+
+// WaitPrometheusOperatorRunning waits for prometheus operator to be running, and returns an error if not.
+func WaitPrometheusOperatorRunning() error {
+	cmd := exec.Command("kubectl", "wait",
+		"deployment.apps",
+		"--for", "condition=Available",
+		"--selector", "app.kubernetes.io/name=prometheus-operator",
+		"--all-namespaces",
+		"--timeout", "5m",
+	)
+	_, 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", "name")
+	output, err := Run(cmd)
+	if err != nil {
+		return false
+	}
+	crdList := GetNonEmptyLines(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)
@@ -67,23 +129,89 @@ func UninstallCertManager() {
 
 // InstallCertManager installs the cert manager bundle.
 func InstallCertManager() error {
-	url := fmt.Sprintf(certManagerURLTmpl, certManagerVersion)
+	// remove any existing cert-manager leases
-	cmd := exec.Command("kubectl", "apply", "-f", url)
+	// NOTE: this is required to avoid issues where cert-manager is reinstalled quickly due to rerunning tests
-	if _, err := Run(cmd); err != nil {
+	cmd := exec.Command("kubectl", "delete",
+		"leases",
+		"--ignore-not-found",
+		"--namespace", "kube-system",
+		"cert-manager-controller",
+		"cert-manager-cainjector-leader-election",
+	)
+	_, err := Run(cmd)
+	if err != nil {
 		return err
 	}
-	// Wait for cert-manager-webhook to be ready, which can take time if cert-manager
+
-	// was re-installed after uninstalling on a cluster.
+	// install cert-manager
-	cmd = exec.Command("kubectl", "wait", "deployment.apps/cert-manager-webhook",
+	url := fmt.Sprintf(certManagerURLTmpl, certManagerVersion)
+	cmd = exec.Command("kubectl", "apply", "-f", url)
+	_, err = Run(cmd)
+	return err
+}
+
+// WaitCertManagerRunning waits for cert manager to be running, and returns an error if not.
+func WaitCertManagerRunning() error {
+
+	// Wait for the cert-manager Deployments to be Available
+	cmd := exec.Command("kubectl", "wait",
+		"deployment.apps",
 		"--for", "condition=Available",
-		"--namespace", "cert-manager",
+		"--selector", "app.kubernetes.io/instance=cert-manager",
+		"--all-namespaces",
 		"--timeout", "5m",
 	)
-
 	_, err := Run(cmd)
+	if err != nil {
 		return err
 	}
 
+	// Wait for the cert-manager Endpoints to be ready
+	// NOTE: the webhooks will not function correctly until this is ready
+	cmd = exec.Command("kubectl", "wait",
+		"endpoints",
+		"--for", "jsonpath=subsets[0].addresses[0].targetRef.kind=Pod",
+		"--selector", "app.kubernetes.io/instance=cert-manager",
+		"--all-namespaces",
+		"--timeout", "2m",
+	)
+	_, err = Run(cmd)
+	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", "-o", "name")
+	output, err := Run(cmd)
+	if err != nil {
+		return false
+	}
+
+	// Check if any of the Cert Manager CRDs are present
+	crdList := GetNonEmptyLines(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