Adjust env tests for RayCluster controller

.github/workflows/component_tests.yaml

@@ -0,0 +1,43 @@
+name: Component tests
+
+on:
+  pull_request:
+    branches:
+      - main
+      - 'release-*'
+    paths-ignore:
+      - 'docs/**'
+      - '**.adoc'
+      - '**.md'
+      - 'LICENSE'
+  push:
+    branches:
+      - main
+      - 'release-*'
+    paths-ignore:
+      - 'docs/**'
+      - '**.adoc'
+      - '**.md'
+      - 'LICENSE'
+
+concurrency:
+  group: ${{ github.head_ref }}-${{ github.workflow }}
+  cancel-in-progress: true
+
+jobs:
+  kubernetes-component:
+
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Set Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: v1.20
+
+      - name: Run component tests
+        run: |
+          make test-component

Makefile

@@ -205,6 +205,7 @@ $(LOCALBIN):
 KUSTOMIZE ?= $(LOCALBIN)/kustomize
 YQ ?= $(LOCALBIN)/yq
 CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
+GINKGO ?= $(LOCALBIN)/ginkgo
 ENVTEST ?= $(LOCALBIN)/setup-envtest
 OPENSHIFT-GOIMPORTS ?= $(LOCALBIN)/openshift-goimports
 OPERATOR_SDK ?= $(LOCALBIN)/operator-sdk
@@ -241,6 +242,11 @@ controller-gen: $(CONTROLLER_GEN) ## Download controller-gen locally if necessar
 $(CONTROLLER_GEN): $(LOCALBIN)
 	test -s $(LOCALBIN)/controller-gen || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION)
 
+.PHONY: ginkgo
+ginkgo: $(GINKGO) ## Download ginkgo locally if necessary.
+$(GINKGO): $(LOCALBIN)
+	test -s $(LOCALBIN)/ginkgo || GOBIN=$(LOCALBIN) go install github.com/onsi/ginkgo/v2/ginkgo
+
 .PHONY: install-yq
 install-yq: $(YQ) ## Download yq locally if necessary
 $(YQ): $(LOCALBIN)
@@ -350,6 +356,10 @@ catalog-push: ## Push a catalog image.
 test-unit: manifests fmt vet envtest ## Run unit tests.
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $(go list ./... | grep -v /test/) -coverprofile cover.out
 
+.PHONY: test-component
+test-component: envtest ginkgo ## Run component tests.
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" $(GINKGO) -v ./pkg/controllers/
+
 .PHONY: test-e2e
 test-e2e: manifests fmt vet ## Run e2e tests.
 	go test -timeout 30m -v ./test/e2e

pkg/controllers/raycluster_controller.go

@@ -53,6 +53,8 @@ type RayClusterReconciler struct {
 	Scheme      *runtime.Scheme
 	CookieSalt  string
 	Config      *config.CodeFlareOperatorConfiguration
+	// Used for envtest to pass OpenShift flag
+	isOpenShiftByConfiguration bool
 }
 
 const (
@@ -103,7 +105,7 @@ func (r *RayClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	isLocalInteractive := annotationBoolVal(ctx, &cluster, "sdk.codeflare.dev/local_interactive", false)
 	ingressDomain := "" // FIX - CFO will retrieve it.
-	isOpenShift, ingressHost := getClusterType(ctx, r.kubeClient, &cluster, ingressDomain)
+	isOpenShift, ingressHost := getClusterType(ctx, r, &cluster, ingressDomain)
 
 	if cluster.ObjectMeta.DeletionTimestamp.IsZero() {
 		if !controllerutil.ContainsFinalizer(&cluster, oAuthFinalizer) {

pkg/controllers/raycluster_controller_test.go

@@ -17,8 +17,6 @@ limitations under the License.
 package controllers
 
 import (
-	"context"
-	"math/rand"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -29,158 +27,150 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 
 	routev1 "github.com/openshift/api/route/v1"
 )
 
-func stringInList(l []string, s string) bool {
-	for _, i := range l {
-		if i == s {
-			return true
-		}
-	}
-	return false
-}
-
-var letters = []rune("abcdefghijklmnopqrstuvwxyz")
-var r = rand.New(rand.NewSource(time.Now().UnixNano()))
-
-func randSeq(n int) string {
-	b := make([]rune, n)
-	for i := range b {
-		b[i] = letters[r.Intn(len(letters))]
-	}
-	return string(b)
-}
-
 var _ = Describe("RayCluster controller", func() {
 	Context("RayCluster controller test", func() {
 		var rayClusterName = "test-raycluster"
-		var typeNamespaceName types.NamespacedName
-		ctx := context.Background()
-		BeforeEach(func() {
-			By("Generate random number so each run is creating unique")
-			rString := randSeq(10)
-			rayClusterName = rayClusterName + "-" + rString
-			typeNamespaceName = types.NamespacedName{Name: rayClusterName, Namespace: rayClusterName}
+		var namespaceName string
+		BeforeEach(func(ctx SpecContext) {
 			By("Creating a namespace for running the tests.")
 			namespace := &corev1.Namespace{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: rayClusterName,
+					GenerateName: "test-",
 				},
 			}
-			var err error
-			err = k8sClient.Create(ctx, namespace)
-			Expect(err).To(Not(HaveOccurred()))
+			namespace, err := k8sClient.CoreV1().Namespaces().Create(ctx, namespace, metav1.CreateOptions{})
+			Expect(err).NotTo(HaveOccurred())
+			DeferCleanup(func(ctx SpecContext) {
+				err := k8sClient.CoreV1().Namespaces().Delete(ctx, namespace.Name, metav1.DeleteOptions{})
+				Expect(err).To(Not(HaveOccurred()))
+			})
+			namespaceName = namespace.Name
 
 			By("creating a basic instance of the RayCluster CR")
 			raycluster := &rayv1.RayCluster{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      rayClusterName,
-					Namespace: rayClusterName,
+					Namespace: namespace.Name,
 				},
 				Spec: rayv1.RayClusterSpec{
 					HeadGroupSpec: rayv1.HeadGroupSpec{
 						Template: corev1.PodTemplateSpec{
 							Spec: corev1.PodSpec{
-								Containers: []corev1.Container{
-									corev1.Container{},
-								},
+								Containers: []corev1.Container{},
 							},
 						},
 						RayStartParams: map[string]string{},
 					},
 				},
 			}
-			err = k8sClient.Get(ctx, typeNamespaceName, &rayv1.RayCluster{})
-			Expect(errors.IsNotFound(err)).To(Equal(true))
-			err = k8sClient.Create(ctx, raycluster)
+			_, err = rayClient.RayV1().RayClusters(namespace.Name).Create(ctx, raycluster, metav1.CreateOptions{})
 			Expect(err).To(Not(HaveOccurred()))
 		})
 
-		AfterEach(func() {
-			By("removing the instance of the RayCluster used")
-			// err := clientSet.CoreV1().Namespaces().Delete(ctx, RayClusterName, metav1.DeleteOptions{})
-			foundRayCluster := rayv1.RayCluster{}
-			err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
-			if err != nil {
-				Expect(errors.IsNotFound(err)).To(Equal(true))
-			} else {
+		AfterEach(func(ctx SpecContext) {
+			By("removing instances of the RayClusters used")
+			rayClusters, err := rayClient.RayV1().RayClusters(namespaceName).List(ctx, metav1.ListOptions{})
+			Expect(err).To(Not(HaveOccurred()))
+
+			for _, rayCluster := range rayClusters.Items {
+				err = rayClient.RayV1().RayClusters(namespaceName).Delete(ctx, rayCluster.Name, metav1.DeleteOptions{})
 				Expect(err).To(Not(HaveOccurred()))
-				_ = k8sClient.Delete(ctx, &foundRayCluster)
 			}
-			Eventually(func() bool {
-				err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
-				return errors.IsNotFound(err)
-			}, SpecTimeout(time.Second*10)).Should(Equal(true))
-		})
 
-		It("should have oauth finalizer set", func() {
-			foundRayCluster := rayv1.RayCluster{}
-			Eventually(func() bool {
-				err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
-				Expect(err).To(Not(HaveOccurred()))
-				return stringInList(foundRayCluster.Finalizers, oAuthFinalizer)
-			}, SpecTimeout(time.Second*10)).Should(Equal(true))
+			Eventually(func() ([]rayv1.RayCluster, error) {
+				rayClusters, err := rayClient.RayV1().RayClusters(namespaceName).List(ctx, metav1.ListOptions{})
+				return rayClusters.Items, err
+			}).WithTimeout(time.Second * 10).Should(BeEmpty())
 		})
 
-		It("should create all oauth resources", func() {
-			Eventually(func() error {
-				foundRayCluster := rayv1.RayCluster{}
-				err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
-				if err != nil {
-					return err
-				}
-				err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthSecretNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Secret{})
-				if err != nil {
-					return err
-				}
-				err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthServiceNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Service{})
-				if err != nil {
-					return err
-				}
-				err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &corev1.ServiceAccount{})
-				if err != nil {
-					return err
-				}
-				err = k8sClient.Get(ctx, types.NamespacedName{Name: crbNameFromCluster(&foundRayCluster)}, &rbacv1.ClusterRoleBinding{})
-				if err != nil {
-					return err
-				}
-				err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &routev1.Route{})
-				if err != nil {
-					return err
-				}
-				return nil
-			}, SpecTimeout(time.Second*10)).Should(Not(HaveOccurred()))
-		})
+		It("should have oauth finalizer set", func(ctx SpecContext) {
+			Eventually(func() ([]string, error) {
+				foundRayCluster, err := rayClient.RayV1().RayClusters(namespaceName).Get(ctx, rayClusterName, metav1.GetOptions{})
+				return foundRayCluster.Finalizers, err
+			}).WithTimeout(time.Second * 10).Should(ContainElement(oAuthFinalizer))
+		}, SpecTimeout(time.Second*10))
 
-		It("should set owner references for all resources", func() {
-			foundRayCluster := rayv1.RayCluster{}
-			err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
-			Expect(err).ToNot(HaveOccurred())
-			err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthSecretNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Secret{})
-			Expect(err).To(Not(HaveOccurred()))
-			err = k8sClient.Get(ctx, types.NamespacedName{Name: oauthServiceNameFromCluster(&foundRayCluster), Namespace: foundRayCluster.Namespace}, &corev1.Service{})
+		It("should create all oauth resources", func(ctx SpecContext) {
+			foundRayCluster, err := rayClient.RayV1().RayClusters(namespaceName).Get(ctx, rayClusterName, metav1.GetOptions{})
 			Expect(err).To(Not(HaveOccurred()))
-			err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &corev1.ServiceAccount{})
-			Expect(err).To(Not(HaveOccurred()))
-			err = k8sClient.Get(ctx, types.NamespacedName{Name: crbNameFromCluster(&foundRayCluster)}, &rbacv1.ClusterRoleBinding{})
-			Expect(err).To(Not(HaveOccurred()))
-			err = k8sClient.Get(ctx, types.NamespacedName{Name: foundRayCluster.Name, Namespace: foundRayCluster.Namespace}, &routev1.Route{})
+
+			Eventually(func() (*corev1.Secret, error) {
+				return k8sClient.CoreV1().Secrets(namespaceName).Get(ctx, oauthSecretNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).ShouldNot(BeNil())
+			Eventually(func() (*corev1.Service, error) {
+				return k8sClient.CoreV1().Services(namespaceName).Get(ctx, oauthServiceNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).ShouldNot(BeNil())
+			Eventually(func() (*corev1.ServiceAccount, error) {
+				return k8sClient.CoreV1().ServiceAccounts(namespaceName).Get(ctx, oauthServiceAccountNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).ShouldNot(BeNil())
+			Eventually(func() (*rbacv1.ClusterRoleBinding, error) {
+				return k8sClient.RbacV1().ClusterRoleBindings().Get(ctx, crbNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).ShouldNot(BeNil())
+			Eventually(func() (*routev1.Route, error) {
+				return routeClient.RouteV1().Routes(namespaceName).Get(ctx, dashboardNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).ShouldNot(BeNil())
+		})
+
+		It("should set owner references for all resources", func(ctx SpecContext) {
+			foundRayCluster, err := rayClient.RayV1().RayClusters(namespaceName).Get(ctx, rayClusterName, metav1.GetOptions{})
 			Expect(err).To(Not(HaveOccurred()))
+
+			Eventually(func() (*corev1.Secret, error) {
+				return k8sClient.CoreV1().Secrets(namespaceName).Get(ctx, oauthSecretNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceKind, Equal("RayCluster")))
+			Eventually(func() (*corev1.Secret, error) {
+				return k8sClient.CoreV1().Secrets(namespaceName).Get(ctx, oauthSecretNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceName, Equal(foundRayCluster.Name)))
+			Eventually(func() (*corev1.Service, error) {
+				return k8sClient.CoreV1().Services(namespaceName).Get(ctx, oauthServiceNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceKind, Equal("RayCluster")))
+			Eventually(func() (*corev1.Service, error) {
+				return k8sClient.CoreV1().Services(namespaceName).Get(ctx, oauthServiceNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceName, Equal(foundRayCluster.Name)))
+			Eventually(func() (*corev1.ServiceAccount, error) {
+				return k8sClient.CoreV1().ServiceAccounts(namespaceName).Get(ctx, oauthServiceAccountNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceKind, Equal("RayCluster")))
+			Eventually(func() (*corev1.ServiceAccount, error) {
+				return k8sClient.CoreV1().ServiceAccounts(namespaceName).Get(ctx, oauthServiceAccountNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceName, Equal(foundRayCluster.Name)))
+			Eventually(func() (*rbacv1.ClusterRoleBinding, error) {
+				return k8sClient.RbacV1().ClusterRoleBindings().Get(ctx, crbNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceKind, Equal("RayCluster")))
+			Eventually(func() (*rbacv1.ClusterRoleBinding, error) {
+				return k8sClient.RbacV1().ClusterRoleBindings().Get(ctx, crbNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceName, Equal(foundRayCluster.Name)))
+			Eventually(func() (*routev1.Route, error) {
+				return routeClient.RouteV1().Routes(namespaceName).Get(ctx, dashboardNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceKind, Equal("RayCluster")))
+			Eventually(func() (*routev1.Route, error) {
+				return routeClient.RouteV1().Routes(namespaceName).Get(ctx, dashboardNameFromCluster(foundRayCluster), metav1.GetOptions{})
+			}).WithTimeout(time.Second * 10).Should(WithTransform(OwnerReferenceName, Equal(foundRayCluster.Name)))
 		})
 
-		It("should remove CRB when the RayCluster is deleted", func() {
-			foundRayCluster := rayv1.RayCluster{}
-			err := k8sClient.Get(ctx, typeNamespaceName, &foundRayCluster)
+		It("should remove CRB when the RayCluster is deleted", func(ctx SpecContext) {
+			foundRayCluster, err := rayClient.RayV1().RayClusters(namespaceName).Get(ctx, rayClusterName, metav1.GetOptions{})
 			Expect(err).To(Not(HaveOccurred()))
-			err = k8sClient.Delete(ctx, &foundRayCluster)
+
+			err = rayClient.RayV1().RayClusters(namespaceName).Delete(ctx, foundRayCluster.Name, metav1.DeleteOptions{})
 			Expect(err).To(Not(HaveOccurred()))
-			Eventually(func() bool {
-				return errors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: crbNameFromCluster(&foundRayCluster)}, &rbacv1.ClusterRoleBinding{}))
-			}, SpecTimeout(time.Second*10)).Should(Equal(true))
+
+			Eventually(func() error {
+				_, err := k8sClient.RbacV1().ClusterRoleBindings().Get(ctx, crbNameFromCluster(foundRayCluster), metav1.GetOptions{})
+				return err
+			}).WithTimeout(time.Second * 10).Should(Satisfy(errors.IsNotFound))
 		})
+
 	})
 })
+
+func OwnerReferenceKind(meta metav1.Object) string {
+	return meta.GetOwnerReferences()[0].Kind
+}
+
+func OwnerReferenceName(meta metav1.Object) string {
+	return meta.GetOwnerReferences()[0].Name
+}