From 7ebf64d5a8cbdae13a5cdf251f03ca830cd1ef4c Mon Sep 17 00:00:00 2001 From: mishraprafful Date: Sat, 16 Nov 2024 20:23:29 +0100 Subject: [PATCH 1/6] feat: add generated names for resources --- .../controllers/notebook_controller.go | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/components/notebook-controller/controllers/notebook_controller.go b/components/notebook-controller/controllers/notebook_controller.go index 362da150c59..b499a7aa26a 100644 --- a/components/notebook-controller/controllers/notebook_controller.go +++ b/components/notebook-controller/controllers/notebook_controller.go @@ -16,7 +16,12 @@ limitations under the License. package controllers import ( + "base64" "context" + "hash" + "hash/fnv" + + "encoding/base64" "encoding/json" "fmt" "os" @@ -381,7 +386,7 @@ func generateStatefulSet(instance *v1beta1.Notebook) *appsv1.StatefulSet { ss := &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ - Name: instance.Name, + GenerateName: fmt.Sprintf("%s-%s-", instance.Name, computeObjectHash(fnv.New32a(), instance)), Namespace: instance.Namespace, }, Spec: appsv1.StatefulSetSpec{ @@ -454,13 +459,15 @@ func generateService(instance *v1beta1.Notebook) *corev1.Service { // Define the desired Service object port := DefaultContainerPort containerPorts := instance.Spec.Template.Spec.Containers[0].Ports + hasher := fnv.New32a() + objHash := computeObjectHash(hasher, instance) if containerPorts != nil { port = int(containerPorts[0].ContainerPort) } svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s%s-%s", namePrefix, instance.Namespace, instance.Name), Namespace: instance.Namespace, + GenerateName: fmt.Sprintf("%s-%s-", instance.Name, objHash), }, Spec: corev1.ServiceSpec{ Type: "ClusterIP", @@ -488,6 +495,8 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu namespace := instance.Namespace clusterDomain := "cluster.local" prefix := fmt.Sprintf("/notebook/%s/%s/", namespace, name) + hasher := fnv.New32a() + objHash := computeObjectHash(hasher, instance) // unpack annotations from Notebook resource annotations := make(map[string]string) @@ -509,7 +518,7 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu vsvc := &unstructured.Unstructured{} vsvc.SetAPIVersion("networking.istio.io/v1alpha3") vsvc.SetKind("VirtualService") - vsvc.SetName(virtualServiceName(name, namespace)) + vsvc.SetGenerateName(fmt.Sprintf("%s-%s-", instance.Name, objHash)) vsvc.SetNamespace(namespace) istioHost := os.Getenv("ISTIO_HOST") @@ -788,3 +797,9 @@ func (r *NotebookReconciler) SetupWithManager(mgr ctrl.Manager) error { return nil } + +// computeObjectHash takes any Kubernetes object as input and computes a hash for it. +func computeObjectHash(hasher hash.Hash, obj interface{}) string { + hasher.Reset() + return base64.URLEncoding.EncodeToString(hasher.Sum(nil)) +} From 8423a72005a2c61301c86b5e6fa1c98cb283b3f6 Mon Sep 17 00:00:00 2001 From: mishraprafful Date: Sat, 16 Nov 2024 23:12:26 +0100 Subject: [PATCH 2/6] feat: add owner reference based get for service, virtualservice, and statefulset --- .../controllers/notebook_controller.go | 87 +++++++++++++------ 1 file changed, 62 insertions(+), 25 deletions(-) diff --git a/components/notebook-controller/controllers/notebook_controller.go b/components/notebook-controller/controllers/notebook_controller.go index b499a7aa26a..44e5e570c0f 100644 --- a/components/notebook-controller/controllers/notebook_controller.go +++ b/components/notebook-controller/controllers/notebook_controller.go @@ -16,12 +16,11 @@ limitations under the License. package controllers import ( - "base64" "context" + "encoding/base64" "hash" "hash/fnv" - "encoding/base64" "encoding/json" "fmt" "os" @@ -155,9 +154,23 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } // Check if the StatefulSet already exists foundStateful := &appsv1.StatefulSet{} + namespacedStatefulSets := &appsv1.StatefulSetList{} justCreated := false - err := r.Get(ctx, types.NamespacedName{Name: ss.Name, Namespace: ss.Namespace}, foundStateful) - if err != nil && apierrs.IsNotFound(err) { + + err := r.List(ctx, namespacedStatefulSets, client.InNamespace(ss.Namespace)) + if err != nil { + log.Error(err, "error listing StatefulSets") + return ctrl.Result{}, err + } + + for _, sts := range namespacedStatefulSets.Items { + if metav1.IsControlledBy(&sts, instance) { + foundStateful = &sts + break + } + } + + if foundStateful.Name == "" && foundStateful.Namespace == "" { log.Info("Creating StatefulSet", "namespace", ss.Namespace, "name", ss.Name) r.Metrics.NotebookCreation.WithLabelValues(ss.Namespace).Inc() err = r.Create(ctx, ss) @@ -167,10 +180,8 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c r.Metrics.NotebookFailCreation.WithLabelValues(ss.Namespace).Inc() return ctrl.Result{}, err } - } else if err != nil { - log.Error(err, "error getting Statefulset") - return ctrl.Result{}, err } + // Update the foundStateful object and write the result back if there are any changes if !justCreated && reconcilehelper.CopyStatefulSetFields(ss, foundStateful) { log.Info("Updating StatefulSet", "namespace", ss.Namespace, "name", ss.Name) @@ -189,9 +200,22 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // Check if the Service already exists foundService := &corev1.Service{} + namespacedServices := &corev1.ServiceList{} justCreated = false - err = r.Get(ctx, types.NamespacedName{Name: service.Name, Namespace: service.Namespace}, foundService) - if err != nil && apierrs.IsNotFound(err) { + err = r.List(ctx, namespacedServices, client.InNamespace(ss.Namespace)) + if err != nil { + log.Error(err, "error listing Services") + return ctrl.Result{}, err + } + + for _, ser := range namespacedServices.Items { + if metav1.IsControlledBy(&ser, instance) { + foundService = &ser + break + } + } + + if foundService.Name == "" && foundService.Namespace == "" { log.Info("Creating Service", "namespace", service.Namespace, "name", service.Name) err = r.Create(ctx, service) justCreated = true @@ -199,9 +223,6 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c log.Error(err, "unable to create Service") return ctrl.Result{}, err } - } else if err != nil { - log.Error(err, "error getting Service") - return ctrl.Result{}, err } // Update the foundService object and write the result back if there are any changes if !justCreated && reconcilehelper.CopyServiceFields(service, foundService) { @@ -486,9 +507,6 @@ func generateService(instance *v1beta1.Notebook) *corev1.Service { return svc } -func virtualServiceName(kfName string, namespace string) string { - return fmt.Sprintf("%s%s-%s", namePrefix, namespace, kfName) -} func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructured, error) { name := instance.Name @@ -606,26 +624,45 @@ func (r *NotebookReconciler) reconcileVirtualService(instance *v1beta1.Notebook) } // Check if the virtual service already exists. foundVirtual := &unstructured.Unstructured{} - justCreated := false foundVirtual.SetAPIVersion("networking.istio.io/v1alpha3") foundVirtual.SetKind("VirtualService") - err = r.Get(context.TODO(), types.NamespacedName{Name: virtualServiceName(instance.Name, - instance.Namespace), Namespace: instance.Namespace}, foundVirtual) - if err != nil && apierrs.IsNotFound(err) { - log.Info("Creating virtual service", "namespace", instance.Namespace, "name", - virtualServiceName(instance.Name, instance.Namespace)) + namespacedVirtualServices := &unstructured.UnstructuredList{} + justCreated := false + + // List the VirtualServices in the given namespace + err = r.List( + context.TODO(), + namespacedVirtualServices, + client.InNamespace(instance.Namespace), + ) + if err != nil { + log.Error(err, "error listing Virtual Services") + return err + } + + // Manually filter the resources by kind and apiVersion + for _, virSer := range namespacedVirtualServices.Items { + // Check if the resource's kind and apiVersion match + if virSer.GetKind() == "VirtualService" && virSer.GetAPIVersion() == "networking.istio.io/v1alpha3" { + // If the resource is controlled by the instance, assign it to foundVirtual + if metav1.IsControlledBy(&virSer, instance) { + foundVirtual = &virSer + break + } + } + } + + if foundVirtual.GetName() == "" && foundVirtual.GetNamespace() == "" { + log.Info("Creating virtual service", "namespace", instance.Namespace, "notebook-name", instance.Name) err = r.Create(context.TODO(), virtualService) justCreated = true if err != nil { return err } - } else if err != nil { - return err } if !justCreated && reconcilehelper.CopyVirtualService(virtualService, foundVirtual) { - log.Info("Updating virtual service", "namespace", instance.Namespace, "name", - virtualServiceName(instance.Name, instance.Namespace)) + log.Info("Updating virtual service", "namespace", instance.Namespace, "notebook-name", instance.Name) err = r.Update(context.TODO(), foundVirtual) if err != nil { return err From f0a0da8517bf17aca09db1fe10bc4a10ffa075c3 Mon Sep 17 00:00:00 2001 From: mishraprafful Date: Sat, 16 Nov 2024 23:28:17 +0100 Subject: [PATCH 3/6] fix: set group version kind --- .../controllers/notebook_controller.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/components/notebook-controller/controllers/notebook_controller.go b/components/notebook-controller/controllers/notebook_controller.go index 44e5e570c0f..1324e33ffc1 100644 --- a/components/notebook-controller/controllers/notebook_controller.go +++ b/components/notebook-controller/controllers/notebook_controller.go @@ -38,6 +38,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/tools/record" @@ -627,6 +628,13 @@ func (r *NotebookReconciler) reconcileVirtualService(instance *v1beta1.Notebook) foundVirtual.SetAPIVersion("networking.istio.io/v1alpha3") foundVirtual.SetKind("VirtualService") namespacedVirtualServices := &unstructured.UnstructuredList{} + namespacedVirtualServices.SetGroupVersionKind( + schema.GroupVersionKind{ + Group: "networking.istio.io", + Version: "v1alpha3", + Kind: "VirtualService", + }, +) justCreated := false // List the VirtualServices in the given namespace From 38bf868e39923e67f238f0f86b01373a3648c3fb Mon Sep 17 00:00:00 2001 From: mishraprafful Date: Sat, 16 Nov 2024 23:58:32 +0100 Subject: [PATCH 4/6] fix: remove hashing --- .../controllers/notebook_controller.go | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/components/notebook-controller/controllers/notebook_controller.go b/components/notebook-controller/controllers/notebook_controller.go index 1324e33ffc1..44c4b2adb5b 100644 --- a/components/notebook-controller/controllers/notebook_controller.go +++ b/components/notebook-controller/controllers/notebook_controller.go @@ -17,9 +17,6 @@ package controllers import ( "context" - "encoding/base64" - "hash" - "hash/fnv" "encoding/json" "fmt" @@ -58,7 +55,8 @@ const AnnotationRewriteURI = "notebooks.kubeflow.org/http-rewrite-uri" const AnnotationHeadersRequestSet = "notebooks.kubeflow.org/http-headers-request-set" const PrefixEnvVar = "NB_PREFIX" -const namePrefix = "notebook-" +const namePrefix = "nb-" +const generatedSuffixLength = 5 const maxNameLength = 63 // The default fsGroup of PodSecurityContext. @@ -118,9 +116,9 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } // Make sure the prefix doesn't cause the derived resource names to get too long - if len(nbName)+len(involvedNotebookKey.Namespace)+len(namePrefix) > maxNameLength { + if len(nbName)+generatedSuffixLength+len(namePrefix) > maxNameLength { return reconcile.Result{}, - fmt.Errorf("notebook name must not be longer than %d characters as notebook resources are prefixed with %s%s", maxNameLength-len(namePrefix), namePrefix, involvedNotebookKey.Namespace) + fmt.Errorf("notebook name must not be longer than %d characters as notebook resources are prefixed with %s%s", maxNameLength-(len(namePrefix)+generatedSuffixLength), namePrefix, involvedNotebookKey.Namespace) } // re-emit the event in the Notebook CR @@ -408,7 +406,7 @@ func generateStatefulSet(instance *v1beta1.Notebook) *appsv1.StatefulSet { ss := &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("%s-%s-", instance.Name, computeObjectHash(fnv.New32a(), instance)), + GenerateName: fmt.Sprintf("%s-%s-", namePrefix, instance.Name), Namespace: instance.Namespace, }, Spec: appsv1.StatefulSetSpec{ @@ -481,15 +479,14 @@ func generateService(instance *v1beta1.Notebook) *corev1.Service { // Define the desired Service object port := DefaultContainerPort containerPorts := instance.Spec.Template.Spec.Containers[0].Ports - hasher := fnv.New32a() - objHash := computeObjectHash(hasher, instance) + if containerPorts != nil { port = int(containerPorts[0].ContainerPort) } svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: instance.Namespace, - GenerateName: fmt.Sprintf("%s-%s-", instance.Name, objHash), + GenerateName: fmt.Sprintf("%s-%s-", namePrefix, instance.Name), }, Spec: corev1.ServiceSpec{ Type: "ClusterIP", @@ -514,8 +511,6 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu namespace := instance.Namespace clusterDomain := "cluster.local" prefix := fmt.Sprintf("/notebook/%s/%s/", namespace, name) - hasher := fnv.New32a() - objHash := computeObjectHash(hasher, instance) // unpack annotations from Notebook resource annotations := make(map[string]string) @@ -537,7 +532,7 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu vsvc := &unstructured.Unstructured{} vsvc.SetAPIVersion("networking.istio.io/v1alpha3") vsvc.SetKind("VirtualService") - vsvc.SetGenerateName(fmt.Sprintf("%s-%s-", instance.Name, objHash)) + vsvc.SetGenerateName(fmt.Sprintf("%s-%s-", namePrefix, instance.Name)) vsvc.SetNamespace(namespace) istioHost := os.Getenv("ISTIO_HOST") @@ -843,8 +838,3 @@ func (r *NotebookReconciler) SetupWithManager(mgr ctrl.Manager) error { return nil } -// computeObjectHash takes any Kubernetes object as input and computes a hash for it. -func computeObjectHash(hasher hash.Hash, obj interface{}) string { - hasher.Reset() - return base64.URLEncoding.EncodeToString(hasher.Sum(nil)) -} From 6b054027d1fca45c359d7a9120900f377eb70c35 Mon Sep 17 00:00:00 2001 From: mishraprafful Date: Sun, 17 Nov 2024 00:29:13 +0100 Subject: [PATCH 5/6] fix: correct service name for virtual service --- .../controllers/notebook_controller.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/components/notebook-controller/controllers/notebook_controller.go b/components/notebook-controller/controllers/notebook_controller.go index 44c4b2adb5b..2def79da81c 100644 --- a/components/notebook-controller/controllers/notebook_controller.go +++ b/components/notebook-controller/controllers/notebook_controller.go @@ -55,7 +55,7 @@ const AnnotationRewriteURI = "notebooks.kubeflow.org/http-rewrite-uri" const AnnotationHeadersRequestSet = "notebooks.kubeflow.org/http-headers-request-set" const PrefixEnvVar = "NB_PREFIX" -const namePrefix = "nb-" +const namePrefix = "nb" const generatedSuffixLength = 5 const maxNameLength = 63 @@ -238,10 +238,11 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c log.Error(err, "unable to delete obsolete Service") return ctrl.Result{}, err } + // Reconcile virtual service if we use ISTIO. if os.Getenv("USE_ISTIO") == "true" { - err = r.reconcileVirtualService(instance) + err = r.reconcileVirtualService(instance, foundService.Name) if err != nil { return ctrl.Result{}, err } @@ -506,7 +507,7 @@ func generateService(instance *v1beta1.Notebook) *corev1.Service { } -func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructured, error) { +func generateVirtualService(instance *v1beta1.Notebook, serviceName string) (*unstructured.Unstructured, error) { name := instance.Name namespace := instance.Namespace clusterDomain := "cluster.local" @@ -527,7 +528,7 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu if clusterDomainFromEnv, ok := os.LookupEnv("CLUSTER_DOMAIN"); ok { clusterDomain = clusterDomainFromEnv } - service := fmt.Sprintf("%s.%s.svc.%s", name, namespace, clusterDomain) + service := fmt.Sprintf("%s.%s.svc.%s", serviceName, namespace, clusterDomain) vsvc := &unstructured.Unstructured{} vsvc.SetAPIVersion("networking.istio.io/v1alpha3") @@ -608,9 +609,9 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu } -func (r *NotebookReconciler) reconcileVirtualService(instance *v1beta1.Notebook) error { +func (r *NotebookReconciler) reconcileVirtualService(instance *v1beta1.Notebook, serviceName string) error { log := r.Log.WithValues("notebook", instance.Namespace) - virtualService, err := generateVirtualService(instance) + virtualService, err := generateVirtualService(instance, serviceName) if err != nil { log.Info("Unable to generate VirtualService...", err) return err From 6eac47bfb9981e902a2e48187b97b556e511b6f6 Mon Sep 17 00:00:00 2001 From: mishraprafful Date: Sun, 17 Nov 2024 21:21:44 +0100 Subject: [PATCH 6/6] fix: add test sts name check owner reference --- .../controllers/notebook_controller.go | 49 +++++++++---------- .../notebook_controller_bdd_test.go | 17 ++++--- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/components/notebook-controller/controllers/notebook_controller.go b/components/notebook-controller/controllers/notebook_controller.go index 2def79da81c..d48b25c65b9 100644 --- a/components/notebook-controller/controllers/notebook_controller.go +++ b/components/notebook-controller/controllers/notebook_controller.go @@ -206,14 +206,14 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c log.Error(err, "error listing Services") return ctrl.Result{}, err } - + for _, ser := range namespacedServices.Items { if metav1.IsControlledBy(&ser, instance) { foundService = &ser break } } - + if foundService.Name == "" && foundService.Namespace == "" { log.Info("Creating Service", "namespace", service.Namespace, "name", service.Name) err = r.Create(ctx, service) @@ -232,13 +232,12 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{}, err } } - + err = deleteObsoleteService(ctx, r, instance) if err != nil { log.Error(err, "unable to delete obsolete Service") return ctrl.Result{}, err } - // Reconcile virtual service if we use ISTIO. if os.Getenv("USE_ISTIO") == "true" { @@ -408,7 +407,7 @@ func generateStatefulSet(instance *v1beta1.Notebook) *appsv1.StatefulSet { ss := &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ GenerateName: fmt.Sprintf("%s-%s-", namePrefix, instance.Name), - Namespace: instance.Namespace, + Namespace: instance.Namespace, }, Spec: appsv1.StatefulSetSpec{ Replicas: &replicas, @@ -486,7 +485,7 @@ func generateService(instance *v1beta1.Notebook) *corev1.Service { } svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Namespace: instance.Namespace, + Namespace: instance.Namespace, GenerateName: fmt.Sprintf("%s-%s-", namePrefix, instance.Name), }, Spec: corev1.ServiceSpec{ @@ -506,7 +505,6 @@ func generateService(instance *v1beta1.Notebook) *corev1.Service { return svc } - func generateVirtualService(instance *v1beta1.Notebook, serviceName string) (*unstructured.Unstructured, error) { name := instance.Name namespace := instance.Namespace @@ -625,12 +623,12 @@ func (r *NotebookReconciler) reconcileVirtualService(instance *v1beta1.Notebook, foundVirtual.SetKind("VirtualService") namespacedVirtualServices := &unstructured.UnstructuredList{} namespacedVirtualServices.SetGroupVersionKind( - schema.GroupVersionKind{ - Group: "networking.istio.io", - Version: "v1alpha3", - Kind: "VirtualService", - }, -) + schema.GroupVersionKind{ + Group: "networking.istio.io", + Version: "v1alpha3", + Kind: "VirtualService", + }, + ) justCreated := false // List the VirtualServices in the given namespace @@ -655,7 +653,7 @@ func (r *NotebookReconciler) reconcileVirtualService(instance *v1beta1.Notebook, } } } - + if foundVirtual.GetName() == "" && foundVirtual.GetNamespace() == "" { log.Info("Creating virtual service", "namespace", instance.Namespace, "notebook-name", instance.Name) err = r.Create(context.TODO(), virtualService) @@ -680,36 +678,36 @@ func deleteObsoleteService(ctx context.Context, r *NotebookReconciler, instance log := r.Log.WithValues("notebook", instance.Namespace) obsoleteServiceName := instance.Name obsoleteService := &corev1.Service{} - + err := r.Get(ctx, client.ObjectKey{Name: obsoleteServiceName, Namespace: instance.Namespace}, obsoleteService) if apierrs.IsNotFound(err) { - log.Info("Obsolete Service not found; nothing to delete", "namespace", instance.Namespace, "name", obsoleteServiceName) - return nil + log.Info("Obsolete Service not found; nothing to delete", "namespace", instance.Namespace, "name", obsoleteServiceName) + return nil } else if err != nil { - log.Error(err, "error getting obsolete service", "namespace", instance.Namespace, "name", obsoleteServiceName) - return err + log.Error(err, "error getting obsolete service", "namespace", instance.Namespace, "name", obsoleteServiceName) + return err } log.Info("Found obsolete Service", "namespace", obsoleteService.Namespace, "name", obsoleteService.Name) - + // Remove owner references obsoleteService.OwnerReferences = []metav1.OwnerReference{} err = r.Update(ctx, obsoleteService) if err != nil { - log.Error(err, "unable to update owner reference for obsolete Service", "namespace", obsoleteService.Namespace, "name", obsoleteService.Name) - return err + log.Error(err, "unable to update owner reference for obsolete Service", "namespace", obsoleteService.Namespace, "name", obsoleteService.Name) + return err } // Delete the obsolete service err = r.Delete(ctx, obsoleteService) if err != nil { - log.Error(err, "unable to delete obsolete Service", "namespace", obsoleteService.Namespace, "name", obsoleteService.Name) - return err + log.Error(err, "unable to delete obsolete Service", "namespace", obsoleteService.Namespace, "name", obsoleteService.Name) + return err } log.Info("Deleted obsolete Service", "namespace", obsoleteService.Namespace, "name", obsoleteService.Name) return nil - + } func isStsOrPodEvent(event *corev1.Event) bool { @@ -838,4 +836,3 @@ func (r *NotebookReconciler) SetupWithManager(mgr ctrl.Manager) error { return nil } - diff --git a/components/notebook-controller/controllers/notebook_controller_bdd_test.go b/components/notebook-controller/controllers/notebook_controller_bdd_test.go index 02d28211326..43454c72419 100644 --- a/components/notebook-controller/controllers/notebook_controller_bdd_test.go +++ b/components/notebook-controller/controllers/notebook_controller_bdd_test.go @@ -25,6 +25,7 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" nbv1beta1 "github.com/kubeflow/kubeflow/components/notebook-controller/api/v1beta1" ) @@ -74,15 +75,19 @@ var _ = Describe("Notebook controller", func() { */ By("By checking that the Notebook has statefulset") Eventually(func() (bool, error) { - sts := &appsv1.StatefulSet{ObjectMeta: metav1.ObjectMeta{ - Name: Name, - Namespace: Namespace, - }} - err := k8sClient.Get(ctx, notebookLookupKey, sts) + namespacedStatefulSets := &appsv1.StatefulSetList{} + + err := k8sClient.List(ctx, namespacedStatefulSets, client.InNamespace(Namespace)) if err != nil { return false, err } - return true, nil + + for _, sts := range namespacedStatefulSets.Items { + if metav1.IsControlledBy(&sts, createdNotebook) { + return true, nil + } + } + return false, nil }, timeout, interval).Should(BeTrue()) }) })