From 0ab7273f3e1d2ce83432a6b819f6ed179d04aa34 Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Fri, 31 Mar 2023 20:32:32 +0200 Subject: [PATCH 01/10] feat: disables features conflicting with Istio In order to handle creation of Notebooks which should belong to the service mesh we have to ensure this controller does not create unnecessary resources. For this purpose, when opendatahub.io/service-mesh annotation is present on the Notebook resource it will make sure that: - no OAuth Proxy related resources are created - no routes are created It will fail admission of a notebook pod for the injection of proxy sidecars if annotation opendatahub.io/service-mesh is present and set to true as well. I made few minor adjustements in the test code as I needed to re-use several resources, such as NetworkPolicy. --- .../controllers/notebook_controller.go | 69 ++- .../controllers/notebook_controller_test.go | 483 ++++++++++-------- .../controllers/notebook_network.go | 34 +- .../controllers/notebook_oauth.go | 4 +- .../controllers/notebook_webhook.go | 13 +- .../controllers/suite_test.go | 27 +- 6 files changed, 366 insertions(+), 264 deletions(-) diff --git a/components/odh-notebook-controller/controllers/notebook_controller.go b/components/odh-notebook-controller/controllers/notebook_controller.go index cabf4b67975..b9b2401071b 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller.go +++ b/components/odh-notebook-controller/controllers/notebook_controller.go @@ -40,6 +40,7 @@ import ( const ( AnnotationInjectOAuth = "notebooks.opendatahub.io/inject-oauth" + AnnotationServiceMesh = "opendatahub.io/service-mesh" AnnotationValueReconciliationLock = "odh-notebook-controller-lock" AnnotationLogoutUrl = "notebooks.opendatahub.io/oauth-logout-url" ) @@ -79,6 +80,17 @@ func OAuthInjectionIsEnabled(meta metav1.ObjectMeta) bool { } } +// ServiceMeshIsEnabled returns true if the notebook should be part of +// the service mesh. +func ServiceMeshIsEnabled(meta metav1.ObjectMeta) bool { + if meta.Annotations[AnnotationServiceMesh] != "" { + result, _ := strconv.ParseBool(meta.Annotations[AnnotationServiceMesh]) + return result + } else { + return false + } +} + // ReconciliationLockIsEnabled returns true if the reconciliation lock // annotation is present in the notebook. func ReconciliationLockIsEnabled(meta metav1.ObjectMeta) bool { @@ -151,37 +163,38 @@ func (r *OpenshiftNotebookReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, err } - // Create the objects required by the OAuth proxy sidecar (see - // notebook_oauth.go file) - if OAuthInjectionIsEnabled(notebook.ObjectMeta) { - // Call the OAuth Service Account reconciler - err = r.ReconcileOAuthServiceAccount(notebook, ctx) - if err != nil { - return ctrl.Result{}, err - } + if !ServiceMeshIsEnabled(notebook.ObjectMeta) { + // Create the objects required by the OAuth proxy sidecar (see notebook_oauth.go file) + if OAuthInjectionIsEnabled(notebook.ObjectMeta) { - // Call the OAuth Service reconciler - err = r.ReconcileOAuthService(notebook, ctx) - if err != nil { - return ctrl.Result{}, err - } + err = r.ReconcileOAuthServiceAccount(notebook, ctx) + if err != nil { + return ctrl.Result{}, err + } - // Call the OAuth Secret reconciler - err = r.ReconcileOAuthSecret(notebook, ctx) - if err != nil { - return ctrl.Result{}, err - } + // Call the OAuth Service reconciler + err = r.ReconcileOAuthService(notebook, ctx) + if err != nil { + return ctrl.Result{}, err + } - // Call the OAuth Route reconciler - err = r.ReconcileOAuthRoute(notebook, ctx) - if err != nil { - return ctrl.Result{}, err - } - } else { - // Call the route reconciler (see notebook_route.go file) - err = r.ReconcileRoute(notebook, ctx) - if err != nil { - return ctrl.Result{}, err + // Call the OAuth Secret reconciler + err = r.ReconcileOAuthSecret(notebook, ctx) + if err != nil { + return ctrl.Result{}, err + } + + // Call the OAuth Route reconciler + err = r.ReconcileOAuthRoute(notebook, ctx) + if err != nil { + return ctrl.Result{}, err + } + } else { + // Call the route reconciler (see notebook_route.go file) + err = r.ReconcileRoute(notebook, ctx) + if err != nil { + return ctrl.Result{}, err + } } } diff --git a/components/odh-notebook-controller/controllers/notebook_controller_test.go b/components/odh-notebook-controller/controllers/notebook_controller_test.go index 869068945b3..5782d09aee4 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller_test.go +++ b/components/odh-notebook-controller/controllers/notebook_controller_test.go @@ -17,8 +17,10 @@ package controllers import ( "context" + "github.com/onsi/gomega/format" "io/ioutil" netv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/api/resource" "strings" "time" @@ -26,7 +28,6 @@ import ( . "github.com/onsi/gomega" routev1 "github.com/openshift/api/route/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" @@ -40,29 +41,17 @@ import ( var _ = Describe("The Openshift Notebook controller", func() { // Define utility constants for testing timeouts/durations and intervals. const ( - timeout = time.Second * 10 - interval = time.Second * 2 + duration = 10 * time.Second + interval = 2 * time.Second ) - Context("When creating a Notebook", func() { + When("Creating a Notebook", func() { const ( Name = "test-notebook" Namespace = "default" ) - notebook := &nbv1.Notebook{ - ObjectMeta: metav1.ObjectMeta{ - Name: Name, - Namespace: Namespace, - }, - Spec: nbv1.NotebookSpec{ - Template: nbv1.NotebookTemplateSpec{ - Spec: corev1.PodSpec{Containers: []corev1.Container{{ - Name: Name, - Image: "registry.redhat.io/ubi8/ubi:latest", - }}}}, - }, - } + notebook := createNotebook(Name, Namespace) expectedRoute := routev1.Route{ ObjectMeta: metav1.ObjectMeta{ @@ -105,7 +94,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, route) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookRoutes(*route, expectedRoute)).Should(BeTrue()) }) @@ -123,7 +112,7 @@ var _ = Describe("The Openshift Notebook controller", func() { return "", err } return route.Spec.To.Name, nil - }, timeout, interval).Should(Equal(Name)) + }, duration, interval).Should(Equal(Name)) Expect(CompareNotebookRoutes(*route, expectedRoute)).Should(BeTrue()) }) @@ -136,7 +125,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, route) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookRoutes(*route, expectedRoute)).Should(BeTrue()) }) @@ -165,29 +154,17 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, notebook) - }, timeout, interval).Should(HaveOccurred()) + }, duration, interval).Should(HaveOccurred()) }) }) - Context("When creating a Notebook, test Networkpolicies", func() { + When("Creating a Notebook, test Networkpolicies", func() { const ( Name = "test-notebook-np" Namespace = "default" ) - notebook := &nbv1.Notebook{ - ObjectMeta: metav1.ObjectMeta{ - Name: Name, - Namespace: Namespace, - }, - Spec: nbv1.NotebookSpec{ - Template: nbv1.NotebookTemplateSpec{ - Spec: corev1.PodSpec{Containers: []corev1.Container{{ - Name: Name, - Image: "registry.redhat.io/ubi8/ubi:latest", - }}}}, - }, - } + notebook := createNotebook(Name, Namespace) npProtocol := corev1.ProtocolTCP testPodNamespace := "redhat-ods-applications" @@ -221,7 +198,8 @@ var _ = Describe("The Openshift Notebook controller", func() { From: []netv1.NetworkPolicyPeer{ { // Since for unit tests we do not have context, - // namespace will fallback to test pod namespace if run in CI or `redhat-ods-applications` if run locally + // namespace will fallback to test pod namespace + // if run in CI or `redhat-ods-applications` if run locally NamespaceSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "kubernetes.io/metadata.name": testPodNamespace, @@ -236,34 +214,8 @@ var _ = Describe("The Openshift Notebook controller", func() { }, }, } - expectedNotebookOAuthNetworkPolicy := netv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: notebook.Name + "-oauth-np", - Namespace: notebook.Namespace, - }, - Spec: netv1.NetworkPolicySpec{ - PodSelector: metav1.LabelSelector{ - MatchLabels: map[string]string{ - "notebook-name": notebook.Name, - }, - }, - Ingress: []netv1.NetworkPolicyIngressRule{ - { - Ports: []netv1.NetworkPolicyPort{ - { - Protocol: &npProtocol, - Port: &intstr.IntOrString{ - IntVal: NotebookOAuthPort, - }, - }, - }, - }, - }, - PolicyTypes: []netv1.PolicyType{ - netv1.PolicyTypeIngress, - }, - }, - } + + expectedNotebookOAuthNetworkPolicy := createOAuthNetworkPolicy(notebook.Name, notebook.Namespace, npProtocol, NotebookOAuthPort) notebookNetworkPolicy := &netv1.NetworkPolicy{} notebookOAuthNetworkPolicy := &netv1.NetworkPolicy{} @@ -279,15 +231,16 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name + "-ctrl-np", Namespace: Namespace} return cli.Get(ctx, key, notebookNetworkPolicy) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookNetworkPolicies(*notebookNetworkPolicy, expectedNotebookNetworkPolicy)).Should(BeTrue()) By("By checking that the controller has created Network policy to allow all requests on OAuth port") Eventually(func() error { key := types.NamespacedName{Name: Name + "-oauth-np", Namespace: Namespace} return cli.Get(ctx, key, notebookOAuthNetworkPolicy) - }, timeout, interval).ShouldNot(HaveOccurred()) - Expect(CompareNotebookNetworkPolicies(*notebookOAuthNetworkPolicy, expectedNotebookOAuthNetworkPolicy)).Should(BeTrue()) + }, duration, interval).Should(Succeed()) + Expect(CompareNotebookNetworkPolicies(*notebookOAuthNetworkPolicy, expectedNotebookOAuthNetworkPolicy)). + To(BeTrue(), "Expected :%v\n, Got: %v", format.Object(expectedNotebookOAuthNetworkPolicy, 1), format.Object(notebookOAuthNetworkPolicy, 1)) }) It("Should reconcile the Network policies when modified", func() { @@ -304,7 +257,7 @@ var _ = Describe("The Openshift Notebook controller", func() { return "", err } return string(notebookNetworkPolicy.Spec.PolicyTypes[0]), nil - }, timeout, interval).Should(Equal("Ingress")) + }, duration, interval).Should(Equal("Ingress")) Expect(CompareNotebookNetworkPolicies(*notebookNetworkPolicy, expectedNotebookNetworkPolicy)).Should(BeTrue()) }) @@ -317,7 +270,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name + "-oauth-np", Namespace: Namespace} return cli.Get(ctx, key, notebookOAuthNetworkPolicy) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookNetworkPolicies(*notebookOAuthNetworkPolicy, expectedNotebookOAuthNetworkPolicy)).Should(BeTrue()) }) @@ -339,51 +292,44 @@ var _ = Describe("The Openshift Notebook controller", func() { By("By deleting the recently created Notebook") Expect(cli.Delete(ctx, notebook)).Should(Succeed()) - time.Sleep(interval) By("By checking that the Notebook is deleted") Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, notebook) - }, timeout, interval).Should(HaveOccurred()) - time.Sleep(interval) + }, duration, interval).Should(HaveOccurred()) }) }) - Context("When creating a Notebook with the OAuth annotation enabled", func() { + When("Creating a Notebook with OAuth", func() { const ( Name = "test-notebook-oauth" Namespace = "default" ) - notebook := &nbv1.Notebook{ - ObjectMeta: metav1.ObjectMeta{ - Name: Name, - Namespace: Namespace, - Labels: map[string]string{ - "app.kubernetes.io/instance": Name, - }, - Annotations: map[string]string{ - "notebooks.opendatahub.io/inject-oauth": "true", - "notebooks.opendatahub.io/foo": "bar", - "notebooks.opendatahub.io/oauth-logout-url": "https://example.notebook-url/notebook/" + Namespace + "/" + Name, - }, - }, - Spec: nbv1.NotebookSpec{ - Template: nbv1.NotebookTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{{ - Name: Name, - Image: "registry.redhat.io/ubi8/ubi:latest", - }}, - Volumes: []corev1.Volume{ - { - Name: "notebook-data", - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: Name + "-data", - }, + notebook := createNotebook(Name, Namespace) + notebook.SetLabels(map[string]string{ + "app.kubernetes.io/instance": Name, + }) + notebook.SetAnnotations(map[string]string{ + "notebooks.opendatahub.io/inject-oauth": "true", + "notebooks.opendatahub.io/foo": "bar", + "notebooks.opendatahub.io/oauth-logout-url": "https://example.notebook-url/notebook/" + Namespace + "/" + Name, + }) + notebook.Spec = nbv1.NotebookSpec{ + Template: nbv1.NotebookTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: Name, + Image: "registry.redhat.io/ubi8/ubi:latest", + }}, + Volumes: []corev1.Volume{ + { + Name: "notebook-data", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: Name + "-data", }, }, }, @@ -415,89 +361,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Name: Name, Image: "registry.redhat.io/ubi8/ubi:latest", }, - { - Name: "oauth-proxy", - Image: OAuthProxyImage, - ImagePullPolicy: corev1.PullAlways, - Env: []corev1.EnvVar{{ - Name: "NAMESPACE", - ValueFrom: &corev1.EnvVarSource{ - FieldRef: &corev1.ObjectFieldSelector{ - FieldPath: "metadata.namespace", - }, - }, - }}, - Args: []string{ - "--provider=openshift", - "--https-address=:8443", - "--http-address=", - "--openshift-service-account=" + Name, - "--cookie-secret-file=/etc/oauth/config/cookie_secret", - "--cookie-expire=24h0m0s", - "--tls-cert=/etc/tls/private/tls.crt", - "--tls-key=/etc/tls/private/tls.key", - "--upstream=http://localhost:8888", - "--upstream-ca=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt", - "--email-domain=*", - "--skip-provider-button", - `--openshift-sar={"verb":"get","resource":"notebooks","resourceAPIGroup":"kubeflow.org",` + - `"resourceName":"` + Name + `","namespace":"$(NAMESPACE)"}`, - "--logout-url=https://example.notebook-url/notebook/" + Namespace + "/" + Name, - }, - Ports: []corev1.ContainerPort{{ - Name: OAuthServicePortName, - ContainerPort: 8443, - Protocol: corev1.ProtocolTCP, - }}, - LivenessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/oauth/healthz", - Port: intstr.FromString(OAuthServicePortName), - Scheme: corev1.URISchemeHTTPS, - }, - }, - InitialDelaySeconds: 30, - TimeoutSeconds: 1, - PeriodSeconds: 5, - SuccessThreshold: 1, - FailureThreshold: 3, - }, - ReadinessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/oauth/healthz", - Port: intstr.FromString(OAuthServicePortName), - Scheme: corev1.URISchemeHTTPS, - }, - }, - InitialDelaySeconds: 5, - TimeoutSeconds: 1, - PeriodSeconds: 5, - SuccessThreshold: 1, - FailureThreshold: 3, - }, - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - "cpu": resource.MustParse("100m"), - "memory": resource.MustParse("64Mi"), - }, - Limits: corev1.ResourceList{ - "cpu": resource.MustParse("100m"), - "memory": resource.MustParse("64Mi"), - }, - }, - VolumeMounts: []corev1.VolumeMount{ - { - Name: "oauth-config", - MountPath: "/etc/oauth/config", - }, - { - Name: "tls-certificates", - MountPath: "/etc/tls/private", - }, - }, - }, + createOAuthContainer(Name, Namespace), }, Volumes: []corev1.Volume{ { @@ -553,7 +417,7 @@ var _ = Describe("The Openshift Notebook controller", func() { return false } return CompareNotebooks(*notebook, expectedNotebook) - }, timeout, interval).Should(BeTrue()) + }, duration, interval).Should(BeTrue()) }) It("Should reconcile the Notebook when modified", func() { @@ -568,31 +432,19 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, notebook) - }, timeout, interval).Should(Succeed()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebooks(*notebook, expectedNotebook)).Should(BeTrue()) }) serviceAccount := &corev1.ServiceAccount{} - expectedServiceAccount := corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: Name, - Namespace: Namespace, - Labels: map[string]string{ - "notebook-name": Name, - }, - Annotations: map[string]string{ - "serviceaccounts.openshift.io/oauth-redirectreference.first": "" + - `{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"` + Name + `"}}`, - }, - }, - } + expectedServiceAccount := createOAuthServiceAccount(Name, Namespace) It("Should create a Service Account for the notebook", func() { By("By checking that the controller has created the Service Account") Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, serviceAccount) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookServiceAccounts(*serviceAccount, expectedServiceAccount)).Should(BeTrue()) }) @@ -605,7 +457,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, serviceAccount) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookServiceAccounts(*serviceAccount, expectedServiceAccount)).Should(BeTrue()) }) @@ -636,7 +488,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name + "-tls", Namespace: Namespace} return cli.Get(ctx, key, service) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookServices(*service, expectedService)).Should(BeTrue()) }) @@ -649,7 +501,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name + "-tls", Namespace: Namespace} return cli.Get(ctx, key, service) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookServices(*service, expectedService)).Should(BeTrue()) }) @@ -660,7 +512,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name + "-oauth-config", Namespace: Namespace} return cli.Get(ctx, key, secret) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) By("By checking that the cookie secret format is correct") Expect(len(secret.Data["cookie_secret"])).Should(Equal(32)) @@ -675,7 +527,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name + "-oauth-config", Namespace: Namespace} return cli.Get(ctx, key, secret) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) }) route := &routev1.Route{} @@ -712,7 +564,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, route) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookRoutes(*route, expectedRoute)).Should(BeTrue()) }) @@ -725,7 +577,7 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, route) - }, timeout, interval).ShouldNot(HaveOccurred()) + }, duration, interval).Should(Succeed()) Expect(CompareNotebookRoutes(*route, expectedRoute)).Should(BeTrue()) }) @@ -743,7 +595,7 @@ var _ = Describe("The Openshift Notebook controller", func() { return "", err } return route.Spec.To.Name, nil - }, timeout, interval).Should(Equal(Name + "-tls")) + }, duration, interval).Should(Equal(Name + "-tls")) Expect(CompareNotebookRoutes(*route, expectedRoute)).Should(BeTrue()) }) @@ -781,7 +633,222 @@ var _ = Describe("The Openshift Notebook controller", func() { Eventually(func() error { key := types.NamespacedName{Name: Name, Namespace: Namespace} return cli.Get(ctx, key, notebook) - }, timeout, interval).Should(HaveOccurred()) + }, duration, interval).Should(HaveOccurred()) }) }) + + When("Creating notebook as part of Service Mesh", func() { + + const ( + name = "test-notebook-mesh" + namespace = "mesh-ns" + ) + testNamespaces = append(testNamespaces, namespace) + + notebookOAuthNetworkPolicy := createOAuthNetworkPolicy(name, namespace, corev1.ProtocolTCP, NotebookOAuthPort) + + It("Should not add OAuth sidecar", func() { + notebook := createNotebook(name, namespace) + notebook.SetAnnotations(map[string]string{AnnotationServiceMesh: "true"}) + ctx := context.Background() + Expect(cli.Create(ctx, notebook)).Should(Succeed()) + + actualNotebook := &nbv1.Notebook{} + Eventually(func() error { + key := types.NamespacedName{Name: name, Namespace: namespace} + return cli.Get(ctx, key, actualNotebook) + }, duration, interval).Should(Succeed()) + + Expect(actualNotebook.Spec.Template.Spec.Containers).To(Not(ContainElement(createOAuthContainer(name, namespace)))) + }) + + It("Should not define OAuth network policy", func() { + policies := &netv1.NetworkPolicyList{} + Eventually(func() error { + return cli.List(context.Background(), policies, client.InNamespace(namespace)) + }, duration, interval).Should(Succeed()) + + Expect(policies.Items).To(Not(ContainElement(notebookOAuthNetworkPolicy))) + }) + + It("Should not create routes", func() { + routes := &routev1.RouteList{} + Eventually(func() error { + return cli.List(context.Background(), routes, client.InNamespace(namespace)) + }, duration, interval).Should(Succeed()) + + Expect(routes.Items).To(BeEmpty()) + }) + + It("Should not create OAuth Service Account", func() { + oauthServiceAccount := createOAuthServiceAccount(name, namespace) + + serviceAccounts := &corev1.ServiceAccountList{} + Eventually(func() error { + return cli.List(context.Background(), serviceAccounts, client.InNamespace(namespace)) + }, duration, interval).Should(Succeed()) + + Expect(serviceAccounts.Items).ToNot(ContainElement(oauthServiceAccount)) + }) + + It("Should not create OAuth secret", func() { + secrets := &corev1.SecretList{} + Eventually(func() error { + return cli.List(context.Background(), secrets, client.InNamespace(namespace)) + }, duration, interval).Should(Succeed()) + + Expect(secrets.Items).To(BeEmpty()) + }) + + }) + }) + +func createNotebook(name, namespace string) *nbv1.Notebook { + return &nbv1.Notebook{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: nbv1.NotebookSpec{ + Template: nbv1.NotebookTemplateSpec{ + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: name, + Image: "registry.redhat.io/ubi8/ubi:latest", + }}}}, + }, + } +} + +func createOAuthServiceAccount(name, namespace string) corev1.ServiceAccount { + return corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{ + "notebook-name": name, + }, + Annotations: map[string]string{ + "serviceaccounts.openshift.io/oauth-redirectreference.first": "" + + `{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"` + name + `"}}`, + }, + }, + } +} + +func createOAuthContainer(name, namespace string) corev1.Container { + return corev1.Container{ + Name: "oauth-proxy", + Image: OAuthProxyImage, + ImagePullPolicy: corev1.PullAlways, + Env: []corev1.EnvVar{{ + Name: "NAMESPACE", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.namespace", + }, + }, + }}, + Args: []string{ + "--provider=openshift", + "--https-address=:8443", + "--http-address=", + "--openshift-service-account=" + name, + "--cookie-secret-file=/etc/oauth/config/cookie_secret", + "--cookie-expire=24h0m0s", + "--tls-cert=/etc/tls/private/tls.crt", + "--tls-key=/etc/tls/private/tls.key", + "--upstream=http://localhost:8888", + "--upstream-ca=/var/run/secrets/kubernetes.io/serviceaccount/ca.crt", + "--email-domain=*", + "--skip-provider-button", + `--openshift-sar={"verb":"get","resource":"notebooks","resourceAPIGroup":"kubeflow.org",` + + `"resourceName":"` + name + `","namespace":"$(NAMESPACE)"}`, + "--logout-url=https://example.notebook-url/notebook/" + namespace + "/" + name, + }, + Ports: []corev1.ContainerPort{{ + Name: OAuthServicePortName, + ContainerPort: 8443, + Protocol: corev1.ProtocolTCP, + }}, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/oauth/healthz", + Port: intstr.FromString(OAuthServicePortName), + Scheme: corev1.URISchemeHTTPS, + }, + }, + InitialDelaySeconds: 30, + TimeoutSeconds: 1, + PeriodSeconds: 5, + SuccessThreshold: 1, + FailureThreshold: 3, + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/oauth/healthz", + Port: intstr.FromString(OAuthServicePortName), + Scheme: corev1.URISchemeHTTPS, + }, + }, + InitialDelaySeconds: 5, + TimeoutSeconds: 1, + PeriodSeconds: 5, + SuccessThreshold: 1, + FailureThreshold: 3, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "cpu": resource.MustParse("100m"), + "memory": resource.MustParse("64Mi"), + }, + Limits: corev1.ResourceList{ + "cpu": resource.MustParse("100m"), + "memory": resource.MustParse("64Mi"), + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "oauth-config", + MountPath: "/etc/oauth/config", + }, + { + Name: "tls-certificates", + MountPath: "/etc/tls/private", + }, + }, + } +} + +func createOAuthNetworkPolicy(name, namespace string, npProtocol corev1.Protocol, port int32) netv1.NetworkPolicy { + return netv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: name + "-oauth-np", + Namespace: namespace, + }, + Spec: netv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "notebook-name": name, + }, + }, + Ingress: []netv1.NetworkPolicyIngressRule{ + { + Ports: []netv1.NetworkPolicyPort{ + { + Protocol: &npProtocol, + Port: &intstr.IntOrString{ + IntVal: port, + }, + }, + }, + }, + }, + PolicyTypes: []netv1.PolicyType{ + netv1.PolicyTypeIngress, + }, + }, + } +} diff --git a/components/odh-notebook-controller/controllers/notebook_network.go b/components/odh-notebook-controller/controllers/notebook_network.go index 1f0a07abdc4..74c7dcf8cd0 100644 --- a/components/odh-notebook-controller/controllers/notebook_network.go +++ b/components/odh-notebook-controller/controllers/notebook_network.go @@ -37,15 +37,14 @@ const ( NotebookPort = 8888 ) -// ReconcileNetworkPolicies will manage the network policies reconciliation -// required by the notebook +// ReconcileAllNetworkPolicies will manage the network policies reconciliation +// required by the notebook. func (r *OpenshiftNotebookReconciler) ReconcileAllNetworkPolicies(notebook *nbv1.Notebook, ctx context.Context) error { // Initialize logger format log := r.Log.WithValues("notebook", notebook.Name, "namespace", notebook.Namespace) // Generate the desired Network Policies desiredNotebookNetworkPolicy := NewNotebookNetworkPolicy(notebook) - desiredOAuthNetworkPolicy := NewOAuthNetworkPolicy(notebook) // Create Network Policies if they do not already exist err := r.reconcileNetworkPolicy(desiredNotebookNetworkPolicy, ctx, notebook) @@ -54,35 +53,38 @@ func (r *OpenshiftNotebookReconciler) ReconcileAllNetworkPolicies(notebook *nbv1 return err } - err = r.reconcileNetworkPolicy(desiredOAuthNetworkPolicy, ctx, notebook) - if err != nil { - log.Error(err, "error creating Notebook OAuth network policy") - return err + if !ServiceMeshIsEnabled(notebook.ObjectMeta) { + desiredOAuthNetworkPolicy := NewOAuthNetworkPolicy(notebook) + err = r.reconcileNetworkPolicy(desiredOAuthNetworkPolicy, ctx, notebook) + if err != nil { + log.Error(err, "error creating Notebook OAuth network policy") + return err + } } return nil } -func (r *OpenshiftNotebookReconciler) reconcileNetworkPolicy(desirednetworkPolicy *netv1.NetworkPolicy, ctx context.Context, notebook *nbv1.Notebook) error { +func (r *OpenshiftNotebookReconciler) reconcileNetworkPolicy(desiredNetworkPolicy *netv1.NetworkPolicy, ctx context.Context, notebook *nbv1.Notebook) error { // Create the Network Policy if it does not already exist foundNetworkPolicy := &netv1.NetworkPolicy{} justCreated := false err := r.Get(ctx, types.NamespacedName{ - Name: desirednetworkPolicy.GetName(), + Name: desiredNetworkPolicy.GetName(), Namespace: notebook.GetNamespace(), }, foundNetworkPolicy) if err != nil { if apierrs.IsNotFound(err) { - r.Log.Info("Creating Network Policy", "name", desirednetworkPolicy.Name) + r.Log.Info("Creating Network Policy", "name", desiredNetworkPolicy.Name) // Add .metatada.ownerReferences to the Network Policy to be deleted by // the Kubernetes garbage collector if the notebook is deleted - err = ctrl.SetControllerReference(notebook, desirednetworkPolicy, r.Scheme) + err = ctrl.SetControllerReference(notebook, desiredNetworkPolicy, r.Scheme) if err != nil { return err } // Create the NetworkPolicy in the Openshift cluster - err = r.Create(ctx, desirednetworkPolicy) + err = r.Create(ctx, desiredNetworkPolicy) if err != nil && !apierrs.IsAlreadyExists(err) { return err } @@ -93,21 +95,21 @@ func (r *OpenshiftNotebookReconciler) reconcileNetworkPolicy(desirednetworkPolic } // Reconcile the NetworkPolicy spec if it has been manually modified - if !justCreated && !CompareNotebookNetworkPolicies(*desirednetworkPolicy, *foundNetworkPolicy) { + if !justCreated && !CompareNotebookNetworkPolicies(*desiredNetworkPolicy, *foundNetworkPolicy) { r.Log.Info("Reconciling Network policy", "name", foundNetworkPolicy.Name) // Retry the update operation when the ingress controller eventually // updates the resource version field err := retry.RetryOnConflict(retry.DefaultRetry, func() error { // Get the last route revision if err := r.Get(ctx, types.NamespacedName{ - Name: desirednetworkPolicy.Name, + Name: desiredNetworkPolicy.Name, Namespace: notebook.Namespace, }, foundNetworkPolicy); err != nil { return err } // Reconcile labels and spec field - foundNetworkPolicy.Spec = desirednetworkPolicy.Spec - foundNetworkPolicy.ObjectMeta.Labels = desirednetworkPolicy.ObjectMeta.Labels + foundNetworkPolicy.Spec = desiredNetworkPolicy.Spec + foundNetworkPolicy.ObjectMeta.Labels = desiredNetworkPolicy.ObjectMeta.Labels return r.Update(ctx, foundNetworkPolicy) }) if err != nil { diff --git a/components/odh-notebook-controller/controllers/notebook_oauth.go b/components/odh-notebook-controller/controllers/notebook_oauth.go index 803c531c1ef..e67b7683dd3 100644 --- a/components/odh-notebook-controller/controllers/notebook_oauth.go +++ b/components/odh-notebook-controller/controllers/notebook_oauth.go @@ -255,8 +255,8 @@ func NewNotebookOAuthRoute(notebook *nbv1.Notebook) *routev1.Route { return route } -// Reconcile will manage the creation, update and deletion of the OAuth route -// when the notebook is reconciled +// ReconcileOAuthRoute will manage the creation, update and deletion of the OAuth route +// when the notebook is reconciled. func (r *OpenshiftNotebookReconciler) ReconcileOAuthRoute( notebook *nbv1.Notebook, ctx context.Context) error { return r.reconcileRoute(notebook, ctx, NewNotebookOAuthRoute) diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index a695a289ad7..b858404ec43 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -45,13 +45,13 @@ type NotebookWebhook struct { var proxyEnvVars = make(map[string]string, 4) -// InjectReconciliationLock injects the kubefllow notebook controller culling +// InjectReconciliationLock injects the kubeflow notebook controller culling // stop annotation to explicitly start the notebook pod when the ODH notebook -// controller finishes the reconciliation. Otherwise a race condition may happen +// controller finishes the reconciliation. Otherwise, a race condition may happen // while mounting the notebook service account pull secret into the pod. // // The ODH notebook controller will remove this annotation when the first -// reconcilitation is completed (see RemoveReconciliationLock). +// reconciliation is completed (see RemoveReconciliationLock). func InjectReconciliationLock(meta *metav1.ObjectMeta) error { if meta.Annotations != nil { meta.Annotations[culler.STOP_ANNOTATION] = AnnotationValueReconciliationLock @@ -231,7 +231,7 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm return admission.Errored(http.StatusBadRequest, err) } - // Inject the the reconciliation lock only on new notebook creation + // Inject the reconciliation lock only on new notebook creation if req.Operation == admissionv1.Create { err = InjectReconciliationLock(¬ebook.ObjectMeta) if err != nil { @@ -239,8 +239,11 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm } } - // Inject the OAuth proxy if the annotation is present + // Inject the OAuth proxy if the annotation is present but only if Service Mesh is disabled if OAuthInjectionIsEnabled(notebook.ObjectMeta) { + if ServiceMeshIsEnabled(notebook.ObjectMeta) { + return admission.Denied(fmt.Sprintf("Cannot have both %s and %s set to true. Pick one.", AnnotationServiceMesh, AnnotationInjectOAuth)) + } err = InjectOAuthProxy(notebook, w.OAuthConfig) if err != nil { return admission.Errored(http.StatusInternalServerError, err) diff --git a/components/odh-notebook-controller/controllers/suite_test.go b/components/odh-notebook-controller/controllers/suite_test.go index c486653fc72..be1353c1d26 100644 --- a/components/odh-notebook-controller/controllers/suite_test.go +++ b/components/odh-notebook-controller/controllers/suite_test.go @@ -19,7 +19,9 @@ import ( "context" "crypto/tls" "fmt" + v1 "k8s.io/api/core/v1" netv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "net" "path/filepath" "testing" @@ -52,11 +54,17 @@ import ( // +kubebuilder:docs-gen:collapse=Imports var ( - cfg *rest.Config - cli client.Client - envTest *envtest.Environment - ctx context.Context - cancel context.CancelFunc + cfg *rest.Config + cli client.Client + envTest *envtest.Environment + ctx context.Context + cancel context.CancelFunc + testNamespaces = []string{} +) + +const ( + timeout = time.Second * 10 + interval = time.Second * 2 ) func TestAPIs(t *testing.T) { @@ -164,6 +172,15 @@ var _ = BeforeSuite(func() { cli = mgr.GetClient() Expect(cli).ToNot(BeNil()) + for _, namespace := range testNamespaces { + ns := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + Expect(cli.Create(ctx, ns)).To(Succeed()) + } + }, 60) var _ = AfterSuite(func() { From 7f83b003520676c67a93bcd43e85a6c34e58f82e Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Tue, 16 May 2023 13:55:36 +0200 Subject: [PATCH 02/10] chore: renames helper to be part of _test One reason being IDEs such as GoLand have troubles recongizing private funcs, but it is mainly due to the fact that it is a purely test code --- .../odh-notebook-controller/e2e/{helper.go => helper_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename components/odh-notebook-controller/e2e/{helper.go => helper_test.go} (100%) diff --git a/components/odh-notebook-controller/e2e/helper.go b/components/odh-notebook-controller/e2e/helper_test.go similarity index 100% rename from components/odh-notebook-controller/e2e/helper.go rename to components/odh-notebook-controller/e2e/helper_test.go From ce4bfe6ce54a844205b7d73602d5be8b1c9dc396 Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Tue, 16 May 2023 14:12:04 +0200 Subject: [PATCH 03/10] chore: switches to os.ReadFile from deprecated ioutil counterpart --- .../odh-notebook-controller/controllers/notebook_network.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/odh-notebook-controller/controllers/notebook_network.go b/components/odh-notebook-controller/controllers/notebook_network.go index 74c7dcf8cd0..4a42e5ceacf 100644 --- a/components/odh-notebook-controller/controllers/notebook_network.go +++ b/components/odh-notebook-controller/controllers/notebook_network.go @@ -17,9 +17,9 @@ package controllers import ( "context" - "io/ioutil" corev1 "k8s.io/api/core/v1" netv1 "k8s.io/api/networking/v1" + "os" "reflect" "strings" @@ -212,7 +212,7 @@ func NewOAuthNetworkPolicy(notebook *nbv1.Notebook) *netv1.NetworkPolicy { func getControllerNamespace() string { // TODO:Add env variable that stores namespace for both controllers. - if data, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil { + if data, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace"); err == nil { if ns := strings.TrimSpace(string(data)); len(ns) > 0 { return ns } From 9d6774eac2309252ee779647a1ccb6b3ac5bf165 Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Wed, 17 May 2023 14:36:42 +0200 Subject: [PATCH 04/10] feat: enables test filtering for OSSM and OAuth scenarios --- .../e2e/helper_test.go | 86 +++++++++++++++++++ .../e2e/notebook_controller_setup_test.go | 20 +++-- .../e2e/notebook_creation_test.go | 3 +- .../e2e/notebook_deletion_test.go | 3 +- 4 files changed, 105 insertions(+), 7 deletions(-) diff --git a/components/odh-notebook-controller/e2e/helper_test.go b/components/odh-notebook-controller/e2e/helper_test.go index 93c5869b893..1c02987c83b 100644 --- a/components/odh-notebook-controller/e2e/helper_test.go +++ b/components/odh-notebook-controller/e2e/helper_test.go @@ -228,3 +228,89 @@ func setupThothMinimalOAuthNotebook() notebookContext { } return thothMinimalOAuthNbContext } + +func setupThothMinimalServiceMeshNotebook() notebookContext { + testNotebookName := "thoth-minimal-service-mesh-notebook" + + testNotebook := &nbv1.Notebook{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"notebooks.opendatahub.io/service-mesh": "true"}, + Name: testNotebookName, + Namespace: notebookTestNamespace, + }, + Spec: nbv1.NotebookSpec{ + Template: nbv1.NotebookTemplateSpec{ + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "thoth-minimal-service-mesh-notebook", + Image: "quay.io/thoth-station/s2i-minimal-notebook:v0.2.2", + WorkingDir: "/opt/app-root/src", + Ports: []v1.ContainerPort{ + { + Name: "notebook-port", + ContainerPort: 8888, + Protocol: "TCP", + }, + }, + EnvFrom: []v1.EnvFromSource{}, + Env: []v1.EnvVar{ + { + Name: "JUPYTER_NOTEBOOK_PORT", + Value: "8888", + }, + { + Name: "NOTEBOOK_ARGS", + Value: "--ServerApp.port=8888 --NotebookApp.token='' --NotebookApp.password='' --ServerApp.base_url=/notebook/" + notebookTestNamespace + "/" + testNotebookName, + }, + }, + Resources: v1.ResourceRequirements{ + Limits: map[v1.ResourceName]resource.Quantity{ + v1.ResourceCPU: resource.MustParse("1"), + v1.ResourceMemory: resource.MustParse("1Gi"), + }, + Requests: map[v1.ResourceName]resource.Quantity{ + v1.ResourceCPU: resource.MustParse("1"), + v1.ResourceMemory: resource.MustParse("1Gi"), + }, + }, + LivenessProbe: &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + HTTPGet: &v1.HTTPGetAction{ + Path: "/notebook/" + notebookTestNamespace + "/" + testNotebookName + "/api", + Port: intstr.FromString("notebook-port"), + Scheme: "HTTP", + }, + }, + InitialDelaySeconds: 5, + TimeoutSeconds: 1, + PeriodSeconds: 5, + SuccessThreshold: 1, + FailureThreshold: 3, + }, + }, + }, + }, + }, + }, + } + + thothMinimalServiceMeshNbContext := notebookContext{ + nbObjectMeta: &testNotebook.ObjectMeta, + nbSpec: &testNotebook.Spec, + deploymentMode: ServiceMesh, + } + return thothMinimalServiceMeshNbContext +} + +func filterTestNotebooks(notebooks []notebookContext, mode DeploymentMode) []notebookContext { + var filtered []notebookContext + for _, notebook := range notebooks { + if notebook.deploymentMode == mode { + filtered = append(filtered, notebook) + } + } + + return filtered +} diff --git a/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go b/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go index 484701dd933..befa4b310cf 100644 --- a/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go +++ b/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go @@ -38,7 +38,7 @@ type testContext struct { customClient client.Client // namespace for running the tests testNamespace string - // time rquired to create a resource + // time required to create a resource resourceCreationTimeout time.Duration // time interval to check for resource creation resourceRetryInterval time.Duration @@ -48,6 +48,15 @@ type testContext struct { ctx context.Context } +// DeploymentMode indicates what infra scenarios should be verified by the test +// with default being OAuthProxy scenario +type DeploymentMode int + +const ( + OAuthProxy DeploymentMode = iota + ServiceMesh +) + // notebookContext holds information about test notebook // Any notebook that needs to be added to the e2e test suite should be defined in // the notebookContext struct. @@ -55,14 +64,15 @@ type notebookContext struct { // metadata for Notebook object nbObjectMeta *metav1.ObjectMeta // metadata for Notebook Spec - nbSpec *nbv1.NotebookSpec + nbSpec *nbv1.NotebookSpec + deploymentMode DeploymentMode } func NewTestContext() (*testContext, error) { // GetConfig(): If KUBECONFIG env variable is set, it is used to create // the client, else the inClusterConfig() is used. - // Lastly if none of the them are set, it uses $HOME/.kube/config to create the client. + // Lastly if none of them are set, it uses $HOME/.kube/config to create the client. config, err := ctrlruntime.GetConfig() if err != nil { return nil, fmt.Errorf("error creating the config object %v", err) @@ -80,7 +90,7 @@ func NewTestContext() (*testContext, error) { } // Setup all test Notebooks - testNotebooksContextList := []notebookContext{setupThothMinimalOAuthNotebook()} + testNotebooksContextList := []notebookContext{setupThothMinimalOAuthNotebook(), setupThothMinimalServiceMeshNotebook()} return &testContext{ cfg: config, @@ -116,7 +126,7 @@ func TestE2ENotebookController(t *testing.T) { func TestMain(m *testing.M) { // call flag.Parse() here if TestMain uses flags flag.StringVar(¬ebookTestNamespace, "nb-namespace", - "e2e-notebook-controller", "Custom namespace where the notebook contollers are deployed") + "e2e-notebook-controller", "Custom namespace where the notebook controllers are deployed") flag.BoolVar(&skipDeletion, "skip-deletion", false, "skip deletion of the controllers") flag.Parse() os.Exit(m.Run()) diff --git a/components/odh-notebook-controller/e2e/notebook_creation_test.go b/components/odh-notebook-controller/e2e/notebook_creation_test.go index 1643fe803bf..acad2aa4464 100644 --- a/components/odh-notebook-controller/e2e/notebook_creation_test.go +++ b/components/odh-notebook-controller/e2e/notebook_creation_test.go @@ -21,7 +21,8 @@ import ( func creationTestSuite(t *testing.T) { testCtx, err := NewTestContext() require.NoError(t, err) - for _, nbContext := range testCtx.testNotebooks { + oauthNotebooks := filterTestNotebooks(testCtx.testNotebooks, OAuthProxy) + for _, nbContext := range oauthNotebooks { // prepend Notebook name to every subtest t.Run(nbContext.nbObjectMeta.Name, func(t *testing.T) { t.Run("Creation of Notebook instance", func(t *testing.T) { diff --git a/components/odh-notebook-controller/e2e/notebook_deletion_test.go b/components/odh-notebook-controller/e2e/notebook_deletion_test.go index d5cee95e6e1..1e44fb2cd07 100644 --- a/components/odh-notebook-controller/e2e/notebook_deletion_test.go +++ b/components/odh-notebook-controller/e2e/notebook_deletion_test.go @@ -20,7 +20,8 @@ import ( func deletionTestSuite(t *testing.T) { testCtx, err := NewTestContext() require.NoError(t, err) - for _, nbContext := range testCtx.testNotebooks { + oauthNotebooks := filterTestNotebooks(testCtx.testNotebooks, OAuthProxy) + for _, nbContext := range oauthNotebooks { // prepend Notebook name to every subtest t.Run(nbContext.nbObjectMeta.Name, func(t *testing.T) { t.Run("Notebook Deletion", func(t *testing.T) { From 20f39d1554eda2661e6b20656826239ec128ae99 Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Tue, 23 May 2023 13:39:15 +0200 Subject: [PATCH 05/10] feat(e2e): introduces cli flag to run selected type of e2e tests --- components/odh-notebook-controller/Makefile | 10 +++++-- .../e2e/helper_test.go | 4 +-- .../e2e/notebook_controller_setup_test.go | 27 ++++++++++++++++--- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/components/odh-notebook-controller/Makefile b/components/odh-notebook-controller/Makefile index e88ab9d2541..c935d05a093 100644 --- a/components/odh-notebook-controller/Makefile +++ b/components/odh-notebook-controller/Makefile @@ -168,9 +168,15 @@ undeploy: setup undeploy-kf ## Undeploy controller from the Openshift cluster. undeploy-dev: setup undeploy-kf ## Undeploy controller from the Openshift cluster. $(KUSTOMIZE) build config/development | oc delete --ignore-not-found=true -f - +e2e-test-%: + $(eval deploymentMode:=$(subst e2e-test-,,$@)) + go test ./e2e/ -run ^TestE2ENotebookController -v \ + --nb-namespace=${K8S_NAMESPACE} \ + --deploymentMode=$(deploymentMode) \ + ${E2E_TEST_FLAGS} + .PHONY: e2e-test -e2e-test: ## Run e2e tests for the controller - go test ./e2e/ -run ^TestE2ENotebookController -v --nb-namespace=${K8S_NAMESPACE} ${E2E_TEST_FLAGS} +e2e-test: e2e-test-oauth e2e-test-service-mesh ## Run e2e tests for the controller .PHONY: run-ci-e2e-tests run-ci-e2e-tests: diff --git a/components/odh-notebook-controller/e2e/helper_test.go b/components/odh-notebook-controller/e2e/helper_test.go index 1c02987c83b..37e121f7968 100644 --- a/components/odh-notebook-controller/e2e/helper_test.go +++ b/components/odh-notebook-controller/e2e/helper_test.go @@ -98,14 +98,14 @@ func (tc *testContext) curlNotebookEndpoint(nbMeta metav1.ObjectMeta) (*http.Res tr := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } - client := &http.Client{Transport: tr} + httpClient := &http.Client{Transport: tr} req, err := http.NewRequest("GET", notebookEndpoint, nil) if err != nil { return nil, err } - return client.Do(req) + return httpClient.Do(req) } func (tc *testContext) rolloutDeployment(depMeta metav1.ObjectMeta) error { diff --git a/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go b/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go index befa4b310cf..294c9167f41 100644 --- a/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go +++ b/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go @@ -25,6 +25,7 @@ import ( var ( notebookTestNamespace string skipDeletion bool + deploymentMode DeploymentMode scheme = runtime.NewScheme() ) @@ -49,7 +50,7 @@ type testContext struct { } // DeploymentMode indicates what infra scenarios should be verified by the test -// with default being OAuthProxy scenario +// with default being OAuthProxy scenario. type DeploymentMode int const ( @@ -57,6 +58,25 @@ const ( ServiceMesh ) +var modes = [...]string{"oauth", "service-mesh"} + +// Implementing flag.Value funcs, so we can use DeploymentMode as flag. + +func (d *DeploymentMode) String() string { + return modes[*d] +} + +func (d *DeploymentMode) Set(s string) error { + for i := range modes { + if modes[i] == s { + *d = DeploymentMode(i) + return nil + } + } + + return errors.Errorf("Unknown deployment mode %s. Try any of these %v", s, modes) +} + // notebookContext holds information about test notebook // Any notebook that needs to be added to the e2e test suite should be defined in // the notebookContext struct. @@ -104,7 +124,7 @@ func NewTestContext() (*testContext, error) { }, nil } -// TestKFNBC sets up the testing suite for KFNBC. +// TestE2ENotebookController sets up the testing suite for KFNBC. func TestE2ENotebookController(t *testing.T) { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) @@ -124,10 +144,11 @@ func TestE2ENotebookController(t *testing.T) { } func TestMain(m *testing.M) { - // call flag.Parse() here if TestMain uses flags flag.StringVar(¬ebookTestNamespace, "nb-namespace", "e2e-notebook-controller", "Custom namespace where the notebook controllers are deployed") flag.BoolVar(&skipDeletion, "skip-deletion", false, "skip deletion of the controllers") + flag.Var(&deploymentMode, "deploymentMode", "sets deployment mode") flag.Parse() + os.Exit(m.Run()) } From 03539ce090bb75b806a5e0317981f3121fdef6e5 Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Sat, 3 Jun 2023 13:25:10 +0200 Subject: [PATCH 06/10] feat: introduces sevice-mesh e2e tests --- .../config/base/kustomization.yaml | 2 +- .../overlays/service-mesh/cert-secret.yaml | 9 + .../overlays/service-mesh/gateway-route.yaml | 19 ++ .../config/overlays/service-mesh/gateway.yaml | 18 ++ .../overlays/service-mesh/kustomization.yaml | 48 +++ .../config/overlays/service-mesh/smm.yaml | 8 + components/odh-notebook-controller/Makefile | 39 ++- .../config/base/kustomization.yaml | 2 +- .../e2e/helper_test.go | 18 +- .../e2e/notebook_controller_setup_test.go | 5 +- .../e2e/notebook_controller_test.go | 10 +- .../e2e/notebook_creation_test.go | 20 +- .../e2e/notebook_deletion_test.go | 9 +- .../e2e/scripts/README.md | 23 ++ .../e2e/scripts/func-kiali.sh | 106 +++++++ .../e2e/scripts/func-sm.sh | 295 ++++++++++++++++++ .../e2e/scripts/install-ossm-release.sh | 156 +++++++++ .../run-e2e-test-service-mesh.sh | 23 ++ 18 files changed, 781 insertions(+), 29 deletions(-) create mode 100644 components/notebook-controller/config/overlays/service-mesh/cert-secret.yaml create mode 100644 components/notebook-controller/config/overlays/service-mesh/gateway-route.yaml create mode 100644 components/notebook-controller/config/overlays/service-mesh/gateway.yaml create mode 100644 components/notebook-controller/config/overlays/service-mesh/kustomization.yaml create mode 100644 components/notebook-controller/config/overlays/service-mesh/smm.yaml create mode 100644 components/odh-notebook-controller/e2e/scripts/README.md create mode 100644 components/odh-notebook-controller/e2e/scripts/func-kiali.sh create mode 100755 components/odh-notebook-controller/e2e/scripts/func-sm.sh create mode 100755 components/odh-notebook-controller/e2e/scripts/install-ossm-release.sh create mode 100755 components/odh-notebook-controller/run-e2e-test-service-mesh.sh diff --git a/components/notebook-controller/config/base/kustomization.yaml b/components/notebook-controller/config/base/kustomization.yaml index 438b00a0946..b50b7dbac80 100644 --- a/components/notebook-controller/config/base/kustomization.yaml +++ b/components/notebook-controller/config/base/kustomization.yaml @@ -4,4 +4,4 @@ resources: - ../default images: - name: docker.io/kubeflownotebookswg/notebook-controller - newTag: v1.6.1 + newTag: latest diff --git a/components/notebook-controller/config/overlays/service-mesh/cert-secret.yaml b/components/notebook-controller/config/overlays/service-mesh/cert-secret.yaml new file mode 100644 index 00000000000..4f5367f68c9 --- /dev/null +++ b/components/notebook-controller/config/overlays/service-mesh/cert-secret.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Secret +metadata: + name: odh-dashboard-cert + namespace: odh-notebook-controller-system +type: kubernetes.io/tls +data: + tls.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURIVENDQWdXZ0F3SUJBZ0lVZlpOenNGV29JRkxPYXB3aTNwMkx4VDNIWC9Bd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0hqRWNNQm9HQTFVRUF3d1RZMkV1WVhCd2N5MWpjbU11ZEdWemRHbHVaekFlRncweU16QXlNRE14TkRReQpNVGxhRncweU5EQXlNRE14TkRReU1UbGFNQnN4R1RBWEJnTlZCQU1NRUdGd2NITXRZM0pqTG5SbGMzUnBibWN3CmdnRWlNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0SUJEd0F3Z2dFS0FvSUJBUURCYmJ6Ritxa3lFZ0pUak1xSENHZTgKdnkzcTgwaTZPa1ByMlZmWm04THdxT3BJcDJxOWduZFF4MCtqaDI0UjhqLzgwQmVxMEJtc1FFNCsyTURXcmUzdQpkY2pBTVlSZWN2aE5DOVduQmlSZy9USVZYNEQya0hnV05XSS8zMDI4elVEZmlhMm1NbnFwNWFaVWUwcitndm5SClRRaFdUR0UrMStETTYrNUZySzB0NUhMeTZLLzJvRTBmb09SUG9wNzZyeEEzU3J6Rnd3N1gyZ2p0ckM0TjRJQlUKcE02OGtZUE9JZkk3bW9tV055bnRPeVQ5OGpUSThYY0VGeGp1L2NjcEczUi9aVmRiM1pXRUljeDJhd2ljZmFweAovMWNWRittNWkzSnRVRjBING5ZSG8yaXdFTlpmTTJZdHJNR2ZYaFBqdzBqWis0NFlNU0h4djdmdUttOGgybWxOCkFnTUJBQUdqVmpCVU1Ba0dBMVVkRXdRQ01BQXdDd1lEVlIwUEJBUURBZ1hnTUIwR0ExVWRKUVFXTUJRR0NDc0cKQVFVRkJ3TUNCZ2dyQmdFRkJRY0RBVEFiQmdOVkhSRUVGREFTZ2hCaGNIQnpMV055WXk1MFpYTjBhVzVuTUEwRwpDU3FHU0liM0RRRUJDd1VBQTRJQkFRRERKVE1vTVdTczZweHk0eHJTK2pFTnZrVkxDbHpabWZQbWx6MEZFY2JzCm9ZVW4vcGdQRmNQVms2QTdKZ204d2E3dG5Pc280U1JRWG5ReXFUYXNkZXJmZ3V2MWhLNkdpaGRzeTI1UncxYXIKaW5OSk93b1RXRG1aVVpTMmp3NElUUWlib2wzZUR4RzNNVXR1UnRzZG1WZFFkL3RreTdDZWxROFc1VDRqLytyMQpFeXRzWTJVYnIrcVdkcUxWZ0tjbXJnL0NiVzFhWVlOVlljVXlYa3d4ZGFRa09SaXBRd2NjaVJid3oyWERiN3VuCk56K29oSXVUMm01elFJLzkxRWlPVkp3MjFiR3oxWGg1Z2VKdnJsUG0ydjZPbkcvTFJ3bW9iWGl5Yk8wdzJTQkIKRUZnOURTYkF2NFdGcjR2ZUJ3akIwSXhlaW55Z2FLTnNSVm05U2w1eS9oaDkKLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo= + tls.key: LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVktLS0tLQpNSUlFcEFJQkFBS0NBUUVBd1cyOHhmcXBNaElDVTR6S2h3aG52TDh0NnZOSXVqcEQ2OWxYMlp2QzhLanFTS2RxCnZZSjNVTWRQbzRkdUVmSS8vTkFYcXRBWnJFQk9QdGpBMXEzdDduWEl3REdFWG5MNFRRdlZwd1lrWVAweUZWK0EKOXBCNEZqVmlQOTlOdk0xQTM0bXRwako2cWVXbVZIdEsvb0w1MFUwSVZreGhQdGZnek92dVJheXRMZVJ5OHVpdgo5cUJOSDZEa1Q2S2UrcThRTjBxOHhjTU8xOW9JN2F3dURlQ0FWS1RPdkpHRHppSHlPNXFKbGpjcDdUc2svZkkwCnlQRjNCQmNZN3YzSEtSdDBmMlZYVzkyVmhDSE1kbXNJbkgycWNmOVhGUmZwdVl0eWJWQmRCK0oyQjZOb3NCRFcKWHpObUxhekJuMTRUNDhOSTJmdU9HREVoOGIrMzdpcHZJZHBwVFFJREFRQUJBb0lCQVFDejRhekRaUGVDSS9OYgo1YnZXMWY4N0xZT3pVdXBZbmFUYXFiWWtIZEd0WXpqMXRoUHpCMmlVaTdaSk9zSW5HR1ZmWTlvT3RSYWE5UGFQClJaNFlSNG5VMEY2UU5ieUc1VjU2c0QzUjVVbGhsVFhGWUpxYk1nRXJqaHUva0poSHM0M1lGTDZUcDdBaFhmdFAKNTVUM21iQmZiOGNJRW1JQlFsdkIxc3N3cW9RbS93V3UyT1ZjZnBFRlRvN2N3eVptVVZHYnEwbmpiR01rbUE4bwpoMkVvUEtPV1FDckhQK0hKanBzSHhqbUdJZ3ZTdGVHOEpPcnBvVWVKQW0wdzVRYUVxeW9HSUJ0QzdmZnhvYlZSCkNOU1pETUNGMFh3RjFXWDJ2NFBqam83bTBIVU9iVHFXZlBzRXdRYTNSNWZON05HZ2FNMnRDTWpVUlZhYlBZZHkKcFZmcWdPc2hBb0dCQU9aNEEvdG1PdDFNbktLMG81dmh1TXpvMkhWN3djbmE3U0ljRURqODB4cTJzeTlkL3VXSgpqZTVlRTNHUDZDcXRQOXhBdFZDSUJwRXg4eDJjRFJ3dnRxcEFKNmgrSHRTcnUyOC9Oa2VndElxT3poL0dSZXlzClFRSHRZYmlTMEZSekNMTGpEcGpVb3BjY1llYzBpSDlycVA0Q1NDQnQ1WUtYcHJOYUYzNDNvaW4xQW9HQkFOYmIKUi9Ob2lncDNOZnpxV292QUdyT1IzZkxNd2RReklFbytTSHdSY1o1WDY2VXhqd3c2UUlTRlp5SEk2TXhMMXlpNQpUOTFNdHpIV2hIcnZJVWJySDBlRm1jN2sxSWRQY0ppd2xXU3dkWHYveS9kK21RUDRKT1BBMzB2cGRBUmtnVE9FCjQrUDlrQzlySjJGMkZaLzVGdy9aVzZ6QUZ5WFRDdW9TeGx5YjN2TDVBb0dCQUtLci81T0pHdTlzemZxQ0tpRXkKOTYrYWduNmFOYlIybEg1STlLSmt3ZFRQTkRhd3orUFFiWi9jUXprYTdES0RTdG41eW9EbklrdUZ5Q1lVS2FURgpnTmMycFVkbWpmaHFwc2ZsQkRrV2s1aGhKOWlCcUlWZktCdG1KRjJWTXZzSW54RTA5dTZrMTRaMWdCMGpsVnpxCjdzTXJkU0Yrc0VxM0kvRGdIRWo0bDd1cEFvR0FjaUdIaGZBcEs4Z0pnTEVJcWlYQXlWU1ozc2tQeVdYaktDMFAKbWdBMko1T3lsRXpRSFFHd2xmUzdSUUlSVDd5VnJZZEt1bFp2RmVWSytIYWdhYWlxTS9idkxpejJERzZSZERxUgpFU3gvTEFCRVc5TCsrMUhNWHNOc21rbUhuSEc3QkIvNlluaW1KOW8yMEJuSEFQUnpZTExvZE1xUlFVRnJFYzRwCldyWmQ1eGtDZ1lCd0ZETFVjZWNhRjRUcUZ5QnNhSE5KeisraUROUm1GeWMyNytGL3czQUE1ejRVMFIxVEdIM1EKOHNjeVhpNDdQUzF2Qy9uWklnOFpyVzhqL2lJVEIyR0JTQVNORU9hNzFxRWRkUUJyaFZ6cXE5aGVUMC8yNjVTMgpmaEhjQlhyQlk4OHBiRGUxSFI4d3k4OXBic2FtQXJNRTE5T0p3dUdVd05CcllHd2tZaTZmRXc9PQotLS0tLUVORCBSU0EgUFJJVkFURSBLRVktLS0tLQo= \ No newline at end of file diff --git a/components/notebook-controller/config/overlays/service-mesh/gateway-route.yaml b/components/notebook-controller/config/overlays/service-mesh/gateway-route.yaml new file mode 100644 index 00000000000..877adcb043b --- /dev/null +++ b/components/notebook-controller/config/overlays/service-mesh/gateway-route.yaml @@ -0,0 +1,19 @@ +kind: Route +apiVersion: route.openshift.io/v1 +metadata: + name: opendatahub-odh-gateway + namespace: istio-system + labels: + maistra.io/gateway-name: odh-gateway + maistra.io/gateway-namespace: odh-notebook-controller-system +spec: + host: opendatahub.apps-crc.testing + to: + kind: Service + name: istio-ingressgateway + weight: 100 + port: + targetPort: https + tls: + termination: passthrough + wildcardPolicy: None \ No newline at end of file diff --git a/components/notebook-controller/config/overlays/service-mesh/gateway.yaml b/components/notebook-controller/config/overlays/service-mesh/gateway.yaml new file mode 100644 index 00000000000..8161458e006 --- /dev/null +++ b/components/notebook-controller/config/overlays/service-mesh/gateway.yaml @@ -0,0 +1,18 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: odh-gateway + namespace: odh-notebook-controller-system +spec: + selector: + istio: ingressgateway + servers: + - port: + number: 443 + name: https + protocol: HTTPS + tls: + mode: SIMPLE + credentialName: odh-dashboard-cert + hosts: + - "*" diff --git a/components/notebook-controller/config/overlays/service-mesh/kustomization.yaml b/components/notebook-controller/config/overlays/service-mesh/kustomization.yaml new file mode 100644 index 00000000000..544ec932752 --- /dev/null +++ b/components/notebook-controller/config/overlays/service-mesh/kustomization.yaml @@ -0,0 +1,48 @@ +--- +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +resources: + - ../openshift + - gateway.yaml + - gateway-route.yaml + - cert-secret.yaml + - smm.yaml +namespace: odh-notebook-controller-system + +configMapGenerator: + - name: config + behavior: merge + literals: + - USE_ISTIO=true + - ISTIO_GATEWAY=odh-notebook-controller-system/odh-gateway + +patchesJson6902: + - patch: |- + - op: replace + path: /metadata/namespace + value: istio-system + target: + group: route.openshift.io + kind: Route + version: v1 + namespace: odh-notebook-controller-system + name: opendatahub-odh-gateway + - patch: |- + - op: replace + path: /metadata/namespace + value: istio-system + target: + kind: Secret + version: v1 + namespace: odh-notebook-controller-system + name: odh-dashboard-cert + - patch: |- + - op: replace + path: /spec/controlPlaneRef/namespace + value: istio-system + target: + group: maistra.io + version: v1 + kind: ServiceMeshMember + name: default + diff --git a/components/notebook-controller/config/overlays/service-mesh/smm.yaml b/components/notebook-controller/config/overlays/service-mesh/smm.yaml new file mode 100644 index 00000000000..e775d36ea4f --- /dev/null +++ b/components/notebook-controller/config/overlays/service-mesh/smm.yaml @@ -0,0 +1,8 @@ +apiVersion: maistra.io/v1 +kind: ServiceMeshMember +metadata: + name: default +spec: + controlPlaneRef: + namespace: odh-notebook-controller-system + name: custom diff --git a/components/odh-notebook-controller/Makefile b/components/odh-notebook-controller/Makefile index c935d05a093..879a2db5f11 100644 --- a/components/odh-notebook-controller/Makefile +++ b/components/odh-notebook-controller/Makefile @@ -136,7 +136,30 @@ setup: manifests kustomize ## Replace Kustomize manifests with your environment sed -i'' -e 's,newName: .*,newName: '"${IMG}"',' ./config/base/kustomization.yaml sed -i'' -e 's,newTag: .*,newTag: '"${TAG}"',' ./config/base/kustomization.yaml -.PHONE: deploy-kf +.PHONY: setup-service-mesh +setup-service-mesh: kustomize ## Replace Kustomize manifests with your environment configuration. + sed -i'' -e 's,namespace: .*,namespace: '"${K8S_NAMESPACE}"',' \ + ../notebook-controller/config/overlays/service-mesh/smm.yaml + sed -i'' -e 's,namespace: .*,namespace: '"${K8S_NAMESPACE}"',' \ + ../notebook-controller/config/overlays/service-mesh/kustomization.yaml + sed -i'' -e 's,newName: .*,newName: '"${KF_IMG}"',' \ + ../notebook-controller/config/overlays/service-mesh/kustomization.yaml + sed -i'' -e 's,newTag: .*,newTag: '"${KF_TAG}"',' \ + ../notebook-controller/config/overlays/service-mesh/kustomization.yaml + sed -i'' -e 's,host: .*,host: opendatahub.'"$(shell kubectl get ingress.config.openshift.io cluster -o 'jsonpath={.spec.domain}')"',' \ + ../notebook-controller/config/overlays/service-mesh/gateway-route.yaml + +.PHONY: deploy-service-mesh +deploy-service-mesh: ## Deploy Service Mesh to the Openshift cluster. + ./e2e/scripts/install-ossm-release.sh install-operators + ./e2e/scripts/install-ossm-release.sh install-smcp + +.PHONY: undeploy-service-mesh +undeploy-service-mesh: ## Undeploy Service Mesh and related operators + ./e2e/scripts/install-ossm-release.sh delete-smcp + ./e2e/scripts/install-ossm-release.sh delete-operators + +.PHONY: deploy-kf deploy-kf: setup-kf ## Deploy kubeflow controller to the Openshift cluster. $(KUSTOMIZE) build ../notebook-controller/config/overlays/openshift | oc apply -f - @@ -144,6 +167,14 @@ deploy-kf: setup-kf ## Deploy kubeflow controller to the Openshift cluster. deploy: setup deploy-kf ## Deploy controller to the Openshift cluster. $(KUSTOMIZE) build config/base | oc apply -f - +.PHONY: deploy-with-mesh +deploy-with-mesh: setup-service-mesh deploy + $(KUSTOMIZE) build ../notebook-controller/config/overlays/service-mesh | oc apply -f - + +.PHONY: undeploy-with-mesh +undeploy-with-mesh: setup-service-mesh undeploy + $(KUSTOMIZE) build ../notebook-controller/config/overlays/service-mesh | oc delete --ignore-not-found=true -f - + .PHONY: deploy-dev deploy-dev: setup deploy-kf ## Deploy controller to the Openshift cluster. $(KUSTOMIZE) build config/development | oc apply -f - @@ -176,12 +207,16 @@ e2e-test-%: ${E2E_TEST_FLAGS} .PHONY: e2e-test -e2e-test: e2e-test-oauth e2e-test-service-mesh ## Run e2e tests for the controller +e2e-test: e2e-test-oauth ## Run e2e tests for the controller with oauth proxy .PHONY: run-ci-e2e-tests run-ci-e2e-tests: bash ./run-e2e-test.sh +.PHONY: run-ci-e2e-tests-service-mesh +run-ci-e2e-service-mesh-tests: + bash ./run-e2e-test-service-mesh.sh + ##@ Build Dependencies ## Location to install dependencies to diff --git a/components/odh-notebook-controller/config/base/kustomization.yaml b/components/odh-notebook-controller/config/base/kustomization.yaml index 10887962092..6afdeebdd4f 100644 --- a/components/odh-notebook-controller/config/base/kustomization.yaml +++ b/components/odh-notebook-controller/config/base/kustomization.yaml @@ -6,4 +6,4 @@ resources: images: - name: quay.io/opendatahub/odh-notebook-controller newName: quay.io/opendatahub/odh-notebook-controller - newTag: latest + newTag: v1.6.1-2-5-gac345842 diff --git a/components/odh-notebook-controller/e2e/helper_test.go b/components/odh-notebook-controller/e2e/helper_test.go index 37e121f7968..792187e4a69 100644 --- a/components/odh-notebook-controller/e2e/helper_test.go +++ b/components/odh-notebook-controller/e2e/helper_test.go @@ -50,9 +50,13 @@ func (tc *testContext) waitForControllerDeployment(name string, replicas int32) func (tc *testContext) getNotebookRoute(nbMeta *metav1.ObjectMeta) (*routev1.Route, error) { nbRouteList := routev1.RouteList{} - opts := []client.ListOption{ - client.InNamespace(nbMeta.Namespace), - client.MatchingLabels{"notebook-name": nbMeta.Name}} + + var opts []client.ListOption + if deploymentMode == ServiceMesh { + opts = append(opts, client.MatchingLabels{"maistra.io/gateway-name": "odh-gateway"}) + } else { + opts = append(opts, client.MatchingLabels{"notebook-name": nbMeta.Name}) + } err := wait.Poll(tc.resourceRetryInterval, tc.resourceCreationTimeout, func() (done bool, err error) { routeErr := tc.customClient.List(tc.ctx, &nbRouteList, opts...) if routeErr != nil { @@ -70,7 +74,7 @@ func (tc *testContext) getNotebookRoute(nbMeta *metav1.ObjectMeta) (*routev1.Rou return &nbRouteList.Items[0], err } -func (tc *testContext) getNotebookNetworkpolicy(nbMeta *metav1.ObjectMeta, name string) (*netv1.NetworkPolicy, error) { +func (tc *testContext) getNotebookNetworkPolicy(nbMeta *metav1.ObjectMeta, name string) (*netv1.NetworkPolicy, error) { nbNetworkPolicy := &netv1.NetworkPolicy{} err := wait.Poll(tc.resourceRetryInterval, tc.resourceCreationTimeout, func() (done bool, err error) { np, npErr := tc.kubeClient.NetworkingV1().NetworkPolicies(nbMeta.Namespace).Get(tc.ctx, name, metav1.GetOptions{}) @@ -93,7 +97,7 @@ func (tc *testContext) curlNotebookEndpoint(nbMeta metav1.ObjectMeta) (*http.Res } // Access the Notebook endpoint using http request notebookEndpoint := "https://" + nbRoute.Spec.Host + "/notebook/" + - nbRoute.Namespace + "/" + nbRoute.Name + "/api" + nbMeta.Namespace + "/" + nbMeta.Name + "/api" tr := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, @@ -235,7 +239,7 @@ func setupThothMinimalServiceMeshNotebook() notebookContext { testNotebook := &nbv1.Notebook{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"notebooks.opendatahub.io/service-mesh": "true"}, + Annotations: map[string]string{"opendatahub.io/service-mesh": "true"}, Name: testNotebookName, Namespace: notebookTestNamespace, }, @@ -304,7 +308,7 @@ func setupThothMinimalServiceMeshNotebook() notebookContext { return thothMinimalServiceMeshNbContext } -func filterTestNotebooks(notebooks []notebookContext, mode DeploymentMode) []notebookContext { +func notebooksForScenario(notebooks []notebookContext, mode DeploymentMode) []notebookContext { var filtered []notebookContext for _, notebook := range notebooks { if notebook.deploymentMode == mode { diff --git a/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go b/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go index 294c9167f41..d5d0f052566 100644 --- a/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go +++ b/components/odh-notebook-controller/e2e/notebook_controller_setup_test.go @@ -60,8 +60,7 @@ const ( var modes = [...]string{"oauth", "service-mesh"} -// Implementing flag.Value funcs, so we can use DeploymentMode as flag. - +// Implementing flag.Value funcs, so we can use DeploymentMode as a CLI flag. func (d *DeploymentMode) String() string { return modes[*d] } @@ -129,7 +128,7 @@ func TestE2ENotebookController(t *testing.T) { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(nbv1.AddToScheme(scheme)) - utilruntime.Must(routev1.AddToScheme(scheme)) + utilruntime.Must(routev1.Install(scheme)) utilruntime.Must(netv1.AddToScheme(scheme)) // individual test suites after the operator is running diff --git a/components/odh-notebook-controller/e2e/notebook_controller_test.go b/components/odh-notebook-controller/e2e/notebook_controller_test.go index c1ddc8cfd39..5e0b80f91da 100644 --- a/components/odh-notebook-controller/e2e/notebook_controller_test.go +++ b/components/odh-notebook-controller/e2e/notebook_controller_test.go @@ -25,15 +25,7 @@ func (tc *testContext) testKubeflowNotebookController(t *testing.T) { for _, configmap := range configMaplist.Items { if strings.Contains(configmap.Name, "notebook-controller-config") { notebookConfigFound = true - // Verify if the configmap has key ADD_FSGROUP with value 'false' - require.EqualValuesf(t, "false", configmap.Data["ADD_FSGROUP"], - "error getting ADD_FSGROUP in the configmap") - // Verify if the configmap has key USE_ISTIO with value 'false' - require.EqualValuesf(t, "false", configmap.Data["USE_ISTIO"], - "error getting USE_ISTIO in the configmap") - // Verify if the configmap has key ISTIO_GATEWAY with value 'kubeflow/kubeflow-gateway' - require.EqualValuesf(t, "kubeflow/kubeflow-gateway", - configmap.Data["ISTIO_GATEWAY"], "error getting ISTIO_GATEWAY in the configmap") + break } } diff --git a/components/odh-notebook-controller/e2e/notebook_creation_test.go b/components/odh-notebook-controller/e2e/notebook_creation_test.go index acad2aa4464..2eb46013406 100644 --- a/components/odh-notebook-controller/e2e/notebook_creation_test.go +++ b/components/odh-notebook-controller/e2e/notebook_creation_test.go @@ -21,8 +21,8 @@ import ( func creationTestSuite(t *testing.T) { testCtx, err := NewTestContext() require.NoError(t, err) - oauthNotebooks := filterTestNotebooks(testCtx.testNotebooks, OAuthProxy) - for _, nbContext := range oauthNotebooks { + notebooksForSelectedDeploymentMode := notebooksForScenario(testCtx.testNotebooks, deploymentMode) + for _, nbContext := range notebooksForSelectedDeploymentMode { // prepend Notebook name to every subtest t.Run(nbContext.nbObjectMeta.Name, func(t *testing.T) { t.Run("Creation of Notebook instance", func(t *testing.T) { @@ -30,11 +30,17 @@ func creationTestSuite(t *testing.T) { require.NoError(t, err, "error creating Notebook object ") }) t.Run("Notebook Route Validation", func(t *testing.T) { + if deploymentMode == ServiceMesh { + t.Skipf("Skipping as it's not relevant for Service Mesh scenario") + } err = testCtx.testNotebookRouteCreation(nbContext.nbObjectMeta) require.NoError(t, err, "error testing Route for Notebook ") }) t.Run("Notebook Network Policies Validation", func(t *testing.T) { + if deploymentMode == ServiceMesh { + t.Skipf("Skipping as it's not relevant for Service Mesh scenario") + } err = testCtx.testNetworkPolicyCreation(nbContext.nbObjectMeta) require.NoError(t, err, "error testing Network Policies for Notebook ") }) @@ -43,14 +49,20 @@ func creationTestSuite(t *testing.T) { err = testCtx.testNotebookValidation(nbContext.nbObjectMeta) require.NoError(t, err, "error testing StatefulSet for Notebook ") }) + t.Run("Notebook OAuth sidecar Validation", func(t *testing.T) { + if deploymentMode == ServiceMesh { + t.Skipf("Skipping as it's not relevant for Service Mesh scenario") + } err = testCtx.testNotebookOAuthSidecar(nbContext.nbObjectMeta) require.NoError(t, err, "error testing sidecar for Notebook ") }) + t.Run("Verify Notebook Traffic", func(t *testing.T) { err = testCtx.testNotebookTraffic(nbContext.nbObjectMeta) require.NoError(t, err, "error testing Notebook traffic ") }) + t.Run("Verify Notebook Culling", func(t *testing.T) { err = testCtx.testNotebookCulling(nbContext.nbObjectMeta) require.NoError(t, err, "error testing Notebook culling ") @@ -117,7 +129,7 @@ func (tc *testContext) testNotebookRouteCreation(nbMeta *metav1.ObjectMeta) erro func (tc *testContext) testNetworkPolicyCreation(nbMeta *metav1.ObjectMeta) error { // Test Notebook Network Policy that allows access only to Notebook Controller - notebookNetworkPolicy, err := tc.getNotebookNetworkpolicy(nbMeta, nbMeta.Name+"-ctrl-np") + notebookNetworkPolicy, err := tc.getNotebookNetworkPolicy(nbMeta, nbMeta.Name+"-ctrl-np") if err != nil { return fmt.Errorf("error getting network policy for Notebook %v: %v", notebookNetworkPolicy.Name, err) } @@ -143,7 +155,7 @@ func (tc *testContext) testNetworkPolicyCreation(nbMeta *metav1.ObjectMeta) erro } // Test Notebook Network policy that allows all requests on Notebook OAuth port - notebookOAuthNetworkPolicy, err := tc.getNotebookNetworkpolicy(nbMeta, nbMeta.Name+"-oauth-np") + notebookOAuthNetworkPolicy, err := tc.getNotebookNetworkPolicy(nbMeta, nbMeta.Name+"-oauth-np") if err != nil { return fmt.Errorf("error getting network policy for Notebook OAuth port %v: %v", notebookOAuthNetworkPolicy.Name, err) } diff --git a/components/odh-notebook-controller/e2e/notebook_deletion_test.go b/components/odh-notebook-controller/e2e/notebook_deletion_test.go index 1e44fb2cd07..f33de4786b3 100644 --- a/components/odh-notebook-controller/e2e/notebook_deletion_test.go +++ b/components/odh-notebook-controller/e2e/notebook_deletion_test.go @@ -20,8 +20,8 @@ import ( func deletionTestSuite(t *testing.T) { testCtx, err := NewTestContext() require.NoError(t, err) - oauthNotebooks := filterTestNotebooks(testCtx.testNotebooks, OAuthProxy) - for _, nbContext := range oauthNotebooks { + notebooksForSelectedDeploymentMode := notebooksForScenario(testCtx.testNotebooks, deploymentMode) + for _, nbContext := range notebooksForSelectedDeploymentMode { // prepend Notebook name to every subtest t.Run(nbContext.nbObjectMeta.Name, func(t *testing.T) { t.Run("Notebook Deletion", func(t *testing.T) { @@ -73,6 +73,11 @@ func (tc *testContext) testNotebookResourcesDeletion(nbMeta *metav1.ObjectMeta) return fmt.Errorf("unable to delete Statefulset %s :%v ", nbMeta.Name, err) } + if deploymentMode == ServiceMesh { + // Subsequent resources are create for deployments with oauth-proxy + return nil + } + // Verify Notebook Route is deleted nbRouteLookupKey := types.NamespacedName{Name: nbMeta.Name, Namespace: tc.testNamespace} nbRoute := &routev1.Route{} diff --git a/components/odh-notebook-controller/e2e/scripts/README.md b/components/odh-notebook-controller/e2e/scripts/README.md new file mode 100644 index 00000000000..37a3f656e73 --- /dev/null +++ b/components/odh-notebook-controller/e2e/scripts/README.md @@ -0,0 +1,23 @@ +# Installing The Latest OSSM Release + +> **_NOTE:_** Scripts are derived from https://gitlab.cee.redhat.com/istio/kiali-qe/kiali-dev-tools/-/tree/master/install-scripts. + +To install the latest release of OSSM (including Kiali), use the script `install-ossm-release.sh`. This will install the latest released images from the public Red Hat repository. In other words, this will install exactly what customers are installing. + +Here's what you need to do in order to install OSSM. + +First, install a default OpenShift cluster and make sure you do not already have Istio, Service Mesh, or Kiali installed. + +Second, log into this cluster as a cluster admin user via 'oc'. + +Now install the OSSM operators: + +``` +./install-ossm-release.sh --enable-kiali true install-operators +``` + +Once the operators have been given time to start up, now install a control plane with Istio and Kiali: + +``` +./install-ossm-release.sh --enable-kiali true install-smcp +``` diff --git a/components/odh-notebook-controller/e2e/scripts/func-kiali.sh b/components/odh-notebook-controller/e2e/scripts/func-kiali.sh new file mode 100644 index 00000000000..6d065871852 --- /dev/null +++ b/components/odh-notebook-controller/e2e/scripts/func-kiali.sh @@ -0,0 +1,106 @@ +#!/bin/bash + +########################################################## +# +# Functions for managing Kiali installs. +# +########################################################## + +set -u + +install_kiali_operator() { + local kiali_subscription_source="${1}" + + echo "Installing the Kiali Operator" + cat <& /dev/null ; do echo -n '.'; sleep 1; done + ${OC} wait --for condition=established crd/kialis.kiali.io + + if ! ${OC} get namespace ${control_plane_namespace} >& /dev/null; then + echo "Creating control plane namespace: ${control_plane_namespace}" + ${OC} create namespace ${control_plane_namespace} + fi + + ${OC} apply -n ${control_plane_namespace} -f https://raw.githubusercontent.com/kiali/kiali-operator/master/deploy/kiali/kiali_cr_minimal.yaml +} + +delete_kiali_operator() { + local abort_operation="false" + for cr in \ + $(${OC} get kiali --all-namespaces -o custom-columns=NS:.metadata.namespace,N:.metadata.name --no-headers | sed 's/ */:/g' ) + do + abort_operation="true" + local res_namespace=$(echo ${cr} | cut -d: -f1) + local res_name=$(echo ${cr} | cut -d: -f2) + echo "A Kiali CR named [${res_name}] in namespace [${res_namespace}] still exists." + done + if [ "${abort_operation}" == "true" ]; then + echo "Aborting" + exit 1 + fi + + echo "Unsubscribing from the Kiali Operator" + ${OC} delete subscription --namespace openshift-operators kiali-ossm + + echo "Deleting OLM CSVs which uninstalled the Kiali Operator and its related resources" + for csv in $(${OC} get csv --all-namespaces --no-headers -o custom-columns=NS:.metadata.namespace,N:.metadata.name | sed 's/ */:/g' | grep kiali-operator) + do + ${OC} delete csv -n $(echo -n $csv | cut -d: -f1) $(echo -n $csv | cut -d: -f2) + done + + echo "Delete Kiali CRDs" + ${OC} get crds -o name | grep '.*\.kiali\.io' | xargs -r -n 1 ${OC} delete +} + +delete_kiali_cr() { + echo "Deleting all Kiali CRs in the cluster" + for cr in $(${OC} get kiali --all-namespaces -o custom-columns=NS:.metadata.namespace,N:.metadata.name --no-headers | sed 's/ */:/g' ) + do + local res_namespace=$(echo ${cr} | cut -d: -f1) + local res_name=$(echo ${cr} | cut -d: -f2) + ${OC} delete -n ${res_namespace} kiali ${res_name} + done +} + +status_kiali_operator() { + echo + echo "===== KIALI OPERATOR SUBSCRIPTION" + if ${OC} get subscription --namespace openshift-operators kiali-ossm >& /dev/null ; then + echo "A Subscription exists for the Kiali Operator" + ${OC} get subscription --namespace openshift-operators kiali-ossm + echo + echo "===== KIALI OPERATOR POD" + local op_name="$(${OC} get pod --namespace openshift-operators -o name | grep kiali)" + [ ! -z "${op_name}" ] && ${OC} get --namespace openshift-operators ${op_name} || echo "There is no pod" + else + echo "There is no Subscription for the Kiali Operator" + fi +} + +status_kiali_cr() { + echo + echo "===== KIALI CRs" + if [ "$(${OC} get kiali --all-namespaces 2> /dev/null | wc -l)" -gt "0" ] ; then + echo "One or more Kiali CRs exist in the cluster" + ${OC} get kiali --all-namespaces + else + echo "There are no Kiali CRs in the cluster" + fi +} diff --git a/components/odh-notebook-controller/e2e/scripts/func-sm.sh b/components/odh-notebook-controller/e2e/scripts/func-sm.sh new file mode 100755 index 00000000000..89449184bcd --- /dev/null +++ b/components/odh-notebook-controller/e2e/scripts/func-sm.sh @@ -0,0 +1,295 @@ +#!/bin/bash + +########################################################## +# +# Functions for managing Service Mesh installs. +# +########################################################## + +set -u + +install_servicemesh_operators() { + local servicemesh_subscription_source="${1}" + local elasticsearch_subscription_channel="stable" + + echo "Installing the Service Mesh Operators" + cat <& /dev/null ; do echo -n '.'; sleep 1; done + ${OC} wait --for condition=established crd/${crd} + done + + echo -n "Waiting for Service Mesh operator deployment to be created..." + while ! ${OC} get deployment -n openshift-operators -o name | grep istio >& /dev/null ; do echo -n '.'; sleep 1; done + echo "done." + servicemesh_deployment="$(${OC} get deployment -n openshift-operators -o name | grep istio)" + + echo -n "Waiting for Jaeger operator deployment to be created..." + while ! ${OC} get deployment -n openshift-operators -o name | grep jaeger >& /dev/null ; do echo -n '.'; sleep 1; done + echo "done." + jaeger_deployment="$(${OC} get deployment -n openshift-operators -o name | grep jaeger)" + + echo "Waiting for operator deployments to start..." + for op in ${servicemesh_deployment} ${jaeger_deployment} + do + echo -n "Waiting for ${op} to be ready..." + readyReplicas="0" + while [ "$?" != "0" -o "$readyReplicas" == "0" ] + do + sleep 1 + echo -n '.' + readyReplicas="$(${OC} get ${op} -n openshift-operators -o jsonpath='{.status.readyReplicas}' 2> /dev/null)" + done + echo "done." + done + + echo "Wait for the servicemesh operator to be Ready." + ${OC} wait --for condition=Ready $(${OC} get pod -n openshift-operators -l name=istio-operator -o name) --timeout 300s -n openshift-operators + echo "done." + + echo "Wait for the servicemesh validating webhook to be created." + while [ "$(${OC} get validatingwebhookconfigurations -o name | grep servicemesh)" == "" ]; do echo -n '.'; sleep 5; done + echo "done." + + echo "Wait for the servicemesh mutating webhook to be created." + while [ "$(${OC} get mutatingwebhookconfigurations -o name | grep servicemesh)" == "" ]; do echo -n '.'; sleep 5; done + echo "done." + + if ! ${OC} get namespace ${control_plane_namespace} >& /dev/null; then + echo "Creating control plane namespace: ${control_plane_namespace}" + ${OC} create namespace ${control_plane_namespace} + fi + + echo "Installing SMCP" + if [ "${smcp_yaml_file}" == "" ]; then + smcp_yaml_file="/tmp/maistra-smcp.yaml" + cat < ${smcp_yaml_file} +apiVersion: maistra.io/v2 +kind: ServiceMeshControlPlane +metadata: + name: custom +spec: + version: ${smcp_version} + proxy: + injection: + autoInject: true + gateways: + openshiftRoute: + enabled: false + tracing: + type: Jaeger + addons: + jaeger: + name: jaeger + install: {} + grafana: + enabled: true + install: {} + kiali: + name: kiali + enabled: ${enable_kiali} + install: {} + prometheus: + enabled: true +--- +apiVersion: maistra.io/v1 +kind: ServiceMeshMemberRoll +metadata: + name: default +spec: + members: [] +EOM + fi + + while ! ${OC} apply -n ${control_plane_namespace} -f ${smcp_yaml_file} + do + echo "WARNING: Failed to apply [${smcp_yaml_file}] to namespace [${control_plane_namespace}] - will retry in 5 seconds to see if the error condition clears up..." + sleep 5 + done + echo "[${smcp_yaml_file}] has been successfully applied to namespace [${control_plane_namespace}]." +} + +delete_servicemesh_operators() { + local abort_operation="false" + for cr in \ + $(${OC} get smm --all-namespaces -o custom-columns=K:.kind,NS:.metadata.namespace,N:.metadata.name --no-headers | sed 's/ */:/g' ) \ + $(${OC} get smmr --all-namespaces -o custom-columns=K:.kind,NS:.metadata.namespace,N:.metadata.name --no-headers | sed 's/ */:/g' ) \ + $(${OC} get smcp --all-namespaces -o custom-columns=K:.kind,NS:.metadata.namespace,N:.metadata.name --no-headers | sed 's/ */:/g' ) + do + abort_operation="true" + local res_kind=$(echo ${cr} | cut -d: -f1) + local res_namespace=$(echo ${cr} | cut -d: -f2) + local res_name=$(echo ${cr} | cut -d: -f3) + echo "A [${res_kind}] resource named [${res_name}] in namespace [${res_namespace}] still exists. You must delete it first." + done + if [ "${abort_operation}" == "true" ]; then + echo "Aborting" + exit 1 + fi + + echo "Unsubscribing from the Service Mesh operators" + for sub in $(${OC} get subscriptions -n openshift-operators -o name | grep -E 'servicemesh|jaeger|elasticsearch') + do + ${OC} delete -n openshift-operators ${sub} + done + + echo "Deleting OLM CSVs which uninstalls the operators and their related resources" + for csv in $(${OC} get csv --all-namespaces --no-headers -o custom-columns=NS:.metadata.namespace,N:.metadata.name | sed 's/ */:/g' | grep -E 'servicemesh|jaeger|elasticsearch') + do + ${OC} delete csv -n $(echo -n $csv | cut -d: -f1) $(echo -n $csv | cut -d: -f2) + done + + echo "Deleting any Istio clusterroles/bindings that are getting left behind" + for r in \ + $(${OC} get clusterrolebindings -o name | grep -E 'istio') \ + $(${OC} get clusterroles -o name | grep -E 'istio') + do + ${OC} delete ${r} + done + + echo "Delete Istio service accounts, configmaps, secrets that are getting left behind" + for r in \ + $(${OC} get sa -n openshift-operators -o name | grep -E 'istio') \ + $(${OC} get configmaps -n openshift-operators -o name | grep -E 'istio') \ + $(${OC} get secrets -n openshift-operators -o name | grep -E 'istio') + do + ${OC} delete -n openshift-operators ${r} + done + + # See: https://docs.openshift.com/container-platform/4.7/service_mesh/v2x/removing-ossm.html + echo "Clean up validating webhooks" + ${OC} delete validatingwebhookconfiguration/openshift-operators.servicemesh-resources.maistra.io + #${OC} delete validatingwebhookconfiguration/istiod-istio-system + echo "Clean up mutating webhooks" + ${OC} delete mutatingwebhookconfigurations/openshift-operators.servicemesh-resources.maistra.io + #${OC} delete mutatingwebhookconfigurations/istio-sidecar-injector + echo "Clean up services" + ${OC} delete -n openshift-operators svc maistra-admission-controller + echo "Clean up deamonsets" + ${OC} delete -n openshift-operators daemonset/istio-node + #${OC} delete -n kube-system daemonset/istio-cni-node + echo "Clean up some more clusterroles/bindings" + ${OC} delete clusterrole/istio-admin clusterrole/istio-cni clusterrolebinding/istio-cni + ${OC} delete clusterrole istio-view istio-edit + echo "Clean up some security related things from the operator" + ${OC} delete -n openshift-operators configmap/maistra-operator-cabundle + ${OC} delete -n openshift-operators secret/maistra-operator-serving-cert + echo "Clean up Jaeger" + ${OC} delete clusterrole jaegers.jaegertracing.io-v1-admin jaegers.jaegertracing.io-v1-crdview jaegers.jaegertracing.io-v1-edit jaegers.jaegertracing.io-v1-view + echo "Clean up CNI" + ${OC} delete cm -n openshift-operators istio-cni-config + ${OC} delete sa -n openshift-operators istio-cni + echo "Delete the CRDs" + ${OC} get crds -o name | grep '.*\.istio\.io' | xargs -r -n 1 ${OC} delete + ${OC} get crds -o name | grep '.*\.maistra\.io' | xargs -r -n 1 ${OC} delete + ${OC} get crds -o name | grep '.*\.jaegertracing\.io' | xargs -r -n 1 ${OC} delete + #${OC} get crds -o name | grep '.*\.logging\.openshift\.io' | xargs -r -n 1 ${OC} delete +} + +delete_smcp() { + echo "Deleting all SMCP, SMMR, and SMM CRs (if they exist) which uninstalls all the Service Mesh components" + local doomed_namespaces="" + for cr in \ + $(${OC} get smm --all-namespaces -o custom-columns=K:.kind,NS:.metadata.namespace,N:.metadata.name --no-headers | sed 's/ */:/g' ) \ + $(${OC} get smmr --all-namespaces -o custom-columns=K:.kind,NS:.metadata.namespace,N:.metadata.name --no-headers | sed 's/ */:/g' ) \ + $(${OC} get smcp --all-namespaces -o custom-columns=K:.kind,NS:.metadata.namespace,N:.metadata.name --no-headers | sed 's/ */:/g' ) + do + local res_kind=$(echo ${cr} | cut -d: -f1) + local res_namespace=$(echo ${cr} | cut -d: -f2) + local res_name=$(echo ${cr} | cut -d: -f3) + ${OC} delete -n ${res_namespace} ${res_kind} ${res_name} + doomed_namespaces="$(echo ${res_namespace} ${doomed_namespaces} | tr ' ' '\n' | sort -u)" + done + + echo "Deleting the control plane namespaces" + for ns in ${doomed_namespaces} + do + ${OC} delete namespace ${ns} + done +} + +status_servicemesh_operators() { + echo + echo "===== SERVICEMESH OPERATORS SUBSCRIPTIONS" + local all_subs="$(${OC} get subscriptions -n openshift-operators -o name | grep -E 'servicemesh|jaeger|elasticsearch')" + if [ ! -z "${all_subs}" ]; then + ${OC} get --namespace openshift-operators ${all_subs} + echo + echo "===== SERVICEMESH OPERATORS PODS" + local all_pods="$(${OC} get pods -n openshift-operators -o name | grep -E 'istio|jaeger|elasticsearch')" + [ ! -z "${all_pods}" ] && ${OC} get --namespace openshift-operators ${all_pods} || echo "There are no pods" + else + echo "There are no Subscriptions for the Service Mesh Operators" + fi +} + +status_smcp() { + echo + echo "===== SMCPs" + if [ "$(${OC} get smcp --all-namespaces 2> /dev/null | wc -l)" -gt "0" ] ; then + echo "One or more SMCPs exist in the cluster" + ${OC} get smcp --all-namespaces + echo + for cr in \ + $(${OC} get smcp --all-namespaces -o custom-columns=NS:.metadata.namespace,N:.metadata.name --no-headers | sed 's/ */:/g' ) + do + local res_namespace=$(echo ${cr} | cut -d: -f1) + local res_name=$(echo ${cr} | cut -d: -f2) + echo -n "SMCP [${res_name}] in namespace [${res_namespace}]: " + if [ "$(${OC} get smcp ${res_name} -n ${res_namespace} -o jsonpath='{.spec.addons.kiali.enabled}')" == "true" ]; then + echo "Kiali is enabled" + else + echo "Kiali is NOT enabled" + fi + done + else + echo "There are no SMCPs in the cluster" + fi +} diff --git a/components/odh-notebook-controller/e2e/scripts/install-ossm-release.sh b/components/odh-notebook-controller/e2e/scripts/install-ossm-release.sh new file mode 100755 index 00000000000..68a0e9ad9ce --- /dev/null +++ b/components/odh-notebook-controller/e2e/scripts/install-ossm-release.sh @@ -0,0 +1,156 @@ +#!/bin/bash + +########################################################## +# +# This installs the latest public release of Service Mesh an OpenShift cluster. +# +# The operators are installed through OLM. +# +########################################################## + +set -u + +# Change to the directory where this script is +SCRIPT_ROOT="$( cd "$(dirname "$0")" ; pwd -P )" +cd ${SCRIPT_ROOT} + +# get function definitions +source ${SCRIPT_ROOT}/func-sm.sh +source ${SCRIPT_ROOT}/func-kiali.sh + +DEFAULT_CONTROL_PLANE_NAMESPACE="istio-system" +DEFAULT_ENABLE_KIALI="false" +DEFAULT_OC="oc" +DEFAULT_SMCP_VERSION="v2.3" + +_CMD="" +while [[ $# -gt 0 ]]; do + key="$1" + case $key in + + # COMMANDS + + install-operators) _CMD="install-operators" ; shift ;; + install-smcp) _CMD="install-smcp" ; shift ;; + delete-operators) _CMD="delete-operators" ; shift ;; + delete-smcp) _CMD="delete-smcp" ; shift ;; + status) _CMD="status" ; shift ;; + + # OPTIONS + + -c|--client) OC="${2}" ; shift;shift ;; + -cpn|--control-plane-namespace) CONTROL_PLANE_NAMESPACE="${2}" ; shift;shift ;; + -ek|--enable-kiali) ENABLE_KIALI="${2}" ; shift;shift ;; + -smcpv|--smcp-version) SMCP_VERSION="${2}" ; shift;shift ;; + + # HELP + + -h|--help) + cat < + A filename or path to the 'oc' client. + Default: ${DEFAULT_OC} + + -cpn|--control-plane-namespace + The name of the control plane namespace if the SMCP is to be installed. + Default: ${DEFAULT_CONTROL_PLANE_NAMESPACE} + + -ek|--enable-kiali + If true, and you elect to install-operators, the Kiali operator is installed + with the rest of the Service Mesh operators. + If true, and you elect to install-smcp, the SMCP will be configured to tell + Service Mesh to create and manage its own Kiali CR. + This is ignored when deleting operators (i.e. regardless of this setting, all + operators are deleted, Kiali operator included). + Default: ${DEFAULT_ENABLE_KIALI} + + -smcpv|--smcp-version + The version of the SMCP that will be created. + This defines the version of the control plane that will be installed. + This is only used with the "install-smcp" command. + Default: ${DEFAULT_SMCP_VERSION} +The command must be one of: + + * install-operators: Install the Service Mesh operators and (if --enable-kiali is "true") the Kiali operator. + * install-smcp: Install the SMCP (you must first have installed the operators) + * delete-operators: Delete the Service Mesh and Kiali operators (you must first delete all SMCPs and Kiali CRs manually). + * delete-smcp: Delete the Service Mesh custom resources. + * status: Provides details about resources that have been installed. + +HELPMSG + exit 1 + ;; + *) + echo "ERROR: Unknown argument [$key]. Aborting." + exit 1 + ;; + esac +done + +# Setup environment + +CONTROL_PLANE_NAMESPACE="${CONTROL_PLANE_NAMESPACE:-${DEFAULT_CONTROL_PLANE_NAMESPACE}}" +ENABLE_KIALI="${ENABLE_KIALI:-${DEFAULT_ENABLE_KIALI}}" +OC="${OC:-${DEFAULT_OC}}" +SMCP_VERSION="${SMCP_VERSION:-${DEFAULT_SMCP_VERSION}}" + +echo CONTROL_PLANE_NAMESPACE=$CONTROL_PLANE_NAMESPACE +echo ENABLE_KIALI=$ENABLE_KIALI +echo OC=${OC} +echo SMCP_VERSION=${SMCP_VERSION} + +# Make sure we are logged in +if ! which ${OC} >& /dev/null; then + echo "ERROR: The client is not valid [${OC}]. Use --client to specify a valid path to 'oc'." + exit 1 +fi +if ! ${OC} whoami >& /dev/null; then + echo "ERROR: You are not logged into the OpenShift cluster. Use '${OC} login' to log into a cluster and then retry." + exit 1 +fi + +# Process the command +if [ "${_CMD}" == "install-operators" ]; then + + if [ "${ENABLE_KIALI}" == "true" ]; then + install_kiali_operator "redhat-operators" + fi + install_servicemesh_operators "redhat-operators" + +elif [ "${_CMD}" == "install-smcp" ]; then + + if [ "${ENABLE_KIALI}" == "true" ] && ! ${OC} get crd kialis.kiali.io >& /dev/null; then + echo "Cannot install the SMCP with Kiali enabled because Kiali Operator is not installed." + exit 1 + fi + + install_smcp "${CONTROL_PLANE_NAMESPACE}" "${ENABLE_KIALI}" "${SMCP_VERSION}" "" + +elif [ "${_CMD}" == "delete-operators" ]; then + + delete_servicemesh_operators + delete_kiali_operator + +elif [ "${_CMD}" == "delete-smcp" ]; then + + delete_smcp + +elif [ "${_CMD}" == "status" ]; then + + status_servicemesh_operators + status_kiali_operator + status_smcp + status_kiali_cr + +else + echo "ERROR: Missing or unknown command. See --help for usage." + exit 1 +fi diff --git a/components/odh-notebook-controller/run-e2e-test-service-mesh.sh b/components/odh-notebook-controller/run-e2e-test-service-mesh.sh new file mode 100755 index 00000000000..db4c6d0eb4a --- /dev/null +++ b/components/odh-notebook-controller/run-e2e-test-service-mesh.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash + +TEST_NAMESPACE="odh-notebook-controller-system" + +# setup and deploy the controller +oc new-project $TEST_NAMESPACE -n $TEST_NAMESPACE --skip-config-write + +IFS=':' read -r -a CTRL_IMG <<< "${ODH_NOTEBOOK_CONTROLLER_IMAGE}" +export IMG="${CTRL_IMG[0]}" +export TAG="${CTRL_IMG[1]}" +IFS=':' read -r -a KF_NBC_IMG <<< "${KF_NOTEBOOK_CONTROLLER}" +export KF_IMG="${KF_NBC_IMG[0]}" +export KF_TAG="${KF_NBC_IMG[1]}" +export K8S_NAMESPACE=$TEST_NAMESPACE + +make deploy-service-mesh deploy-with-mesh + +# run e2e tests +make e2e-test-service-mesh + +# cleanup deployment +make undeploy-with-mesh undeploy-service-mesh +oc delete project $TEST_NAMESPACE -n $TEST_NAMESPACE From 59041b2c9e14514ec824afa1d0dcd5ac99345505 Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Mon, 5 Jun 2023 13:58:05 +0200 Subject: [PATCH 07/10] feat(e2e): tests for relevant network policy creation and deletion --- .../e2e/notebook_creation_test.go | 58 ++++++++++++------- .../e2e/notebook_deletion_test.go | 57 +++++++++++------- 2 files changed, 73 insertions(+), 42 deletions(-) diff --git a/components/odh-notebook-controller/e2e/notebook_creation_test.go b/components/odh-notebook-controller/e2e/notebook_creation_test.go index 2eb46013406..c94b2536e5e 100644 --- a/components/odh-notebook-controller/e2e/notebook_creation_test.go +++ b/components/odh-notebook-controller/e2e/notebook_creation_test.go @@ -38,9 +38,6 @@ func creationTestSuite(t *testing.T) { }) t.Run("Notebook Network Policies Validation", func(t *testing.T) { - if deploymentMode == ServiceMesh { - t.Skipf("Skipping as it's not relevant for Service Mesh scenario") - } err = testCtx.testNetworkPolicyCreation(nbContext.nbObjectMeta) require.NoError(t, err, "error testing Network Policies for Notebook ") }) @@ -128,56 +125,73 @@ func (tc *testContext) testNotebookRouteCreation(nbMeta *metav1.ObjectMeta) erro } func (tc *testContext) testNetworkPolicyCreation(nbMeta *metav1.ObjectMeta) error { - // Test Notebook Network Policy that allows access only to Notebook Controller - notebookNetworkPolicy, err := tc.getNotebookNetworkPolicy(nbMeta, nbMeta.Name+"-ctrl-np") + err := tc.ensureNetworkPolicyAllowingAccessToOnlyNotebookControllerExists(nbMeta) if err != nil { - return fmt.Errorf("error getting network policy for Notebook %v: %v", notebookNetworkPolicy.Name, err) + return err } - if len(notebookNetworkPolicy.Spec.PolicyTypes) == 0 || notebookNetworkPolicy.Spec.PolicyTypes[0] != netv1.PolicyTypeIngress { + if deploymentMode == ServiceMesh { + // Scenario below is not part of service mesh deployment + return nil + } + return tc.ensureOAuthNetworkPolicyExists(nbMeta, err) +} + +func (tc *testContext) ensureOAuthNetworkPolicyExists(nbMeta *metav1.ObjectMeta, err error) error { + // Test Notebook Network policy that allows all requests on Notebook OAuth port + notebookOAuthNetworkPolicy, err := tc.getNotebookNetworkPolicy(nbMeta, nbMeta.Name+"-oauth-np") + if err != nil { + return fmt.Errorf("error getting network policy for Notebook OAuth port %v: %v", notebookOAuthNetworkPolicy.Name, err) + } + + if len(notebookOAuthNetworkPolicy.Spec.PolicyTypes) == 0 || notebookOAuthNetworkPolicy.Spec.PolicyTypes[0] != netv1.PolicyTypeIngress { return fmt.Errorf("invalid policy type. Expected value :%v", netv1.PolicyTypeIngress) } - if len(notebookNetworkPolicy.Spec.Ingress) == 0 { + if len(notebookOAuthNetworkPolicy.Spec.Ingress) == 0 { return fmt.Errorf("invalid network policy, should contain ingress rule") - } else if len(notebookNetworkPolicy.Spec.Ingress[0].Ports) != 0 { + } else if len(notebookOAuthNetworkPolicy.Spec.Ingress[0].Ports) != 0 { isNotebookPort := false - for _, notebookport := range notebookNetworkPolicy.Spec.Ingress[0].Ports { - if notebookport.Port.IntVal == 8888 { + for _, notebookport := range notebookOAuthNetworkPolicy.Spec.Ingress[0].Ports { + if notebookport.Port.IntVal == 8443 { isNotebookPort = true } } if !isNotebookPort { return fmt.Errorf("invalid Network Policy comfiguration") } - } else if len(notebookNetworkPolicy.Spec.Ingress[0].From) == 0 { - return fmt.Errorf("invalid Network Policy comfiguration") } + return err +} - // Test Notebook Network policy that allows all requests on Notebook OAuth port - notebookOAuthNetworkPolicy, err := tc.getNotebookNetworkPolicy(nbMeta, nbMeta.Name+"-oauth-np") +func (tc *testContext) ensureNetworkPolicyAllowingAccessToOnlyNotebookControllerExists(nbMeta *metav1.ObjectMeta) error { + // Test Notebook Network Policy that allows access only to Notebook Controller + notebookNetworkPolicy, err := tc.getNotebookNetworkPolicy(nbMeta, nbMeta.Name+"-ctrl-np") if err != nil { - return fmt.Errorf("error getting network policy for Notebook OAuth port %v: %v", notebookOAuthNetworkPolicy.Name, err) + return fmt.Errorf("error getting network policy for Notebook %v: %v", notebookNetworkPolicy.Name, err) } - if len(notebookOAuthNetworkPolicy.Spec.PolicyTypes) == 0 || notebookOAuthNetworkPolicy.Spec.PolicyTypes[0] != netv1.PolicyTypeIngress { + if len(notebookNetworkPolicy.Spec.PolicyTypes) == 0 || notebookNetworkPolicy.Spec.PolicyTypes[0] != netv1.PolicyTypeIngress { return fmt.Errorf("invalid policy type. Expected value :%v", netv1.PolicyTypeIngress) } - if len(notebookOAuthNetworkPolicy.Spec.Ingress) == 0 { + if len(notebookNetworkPolicy.Spec.Ingress) == 0 { return fmt.Errorf("invalid network policy, should contain ingress rule") - } else if len(notebookOAuthNetworkPolicy.Spec.Ingress[0].Ports) != 0 { + } else if len(notebookNetworkPolicy.Spec.Ingress[0].Ports) != 0 { isNotebookPort := false - for _, notebookport := range notebookOAuthNetworkPolicy.Spec.Ingress[0].Ports { - if notebookport.Port.IntVal == 8443 { + for _, notebookport := range notebookNetworkPolicy.Spec.Ingress[0].Ports { + if notebookport.Port.IntVal == 8888 { isNotebookPort = true } } if !isNotebookPort { return fmt.Errorf("invalid Network Policy comfiguration") } + } else if len(notebookNetworkPolicy.Spec.Ingress[0].From) == 0 { + return fmt.Errorf("invalid Network Policy comfiguration") } - return err + + return nil } func (tc *testContext) testNotebookValidation(nbMeta *metav1.ObjectMeta) error { diff --git a/components/odh-notebook-controller/e2e/notebook_deletion_test.go b/components/odh-notebook-controller/e2e/notebook_deletion_test.go index f33de4786b3..541c0edd349 100644 --- a/components/odh-notebook-controller/e2e/notebook_deletion_test.go +++ b/components/odh-notebook-controller/e2e/notebook_deletion_test.go @@ -3,6 +3,8 @@ package e2e import ( "fmt" netv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" "log" "testing" @@ -73,6 +75,28 @@ func (tc *testContext) testNotebookResourcesDeletion(nbMeta *metav1.ObjectMeta) return fmt.Errorf("unable to delete Statefulset %s :%v ", nbMeta.Name, err) } + // Verify Notebook Network Policies are deleted + nbNetworkPolicyList := netv1.NetworkPolicyList{} + opts := filterServiceMeshManagedPolicies(nbMeta) + err = wait.Poll(tc.resourceRetryInterval, tc.resourceCreationTimeout, func() (done bool, err error) { + nperr := tc.customClient.List(tc.ctx, &nbNetworkPolicyList, opts...) + if nperr != nil { + if errors.IsNotFound(nperr) { + return true, nil + } + log.Printf("Failed to get Network policies for %v", nbMeta.Name) + return false, err + + } + if len(nbNetworkPolicyList.Items) == 0 { + return true, nil + } + return false, nil + }) + if err != nil { + return fmt.Errorf("unable to delete Network policies for %s : %v", nbMeta.Name, err) + } + if deploymentMode == ServiceMesh { // Subsequent resources are create for deployments with oauth-proxy return nil @@ -97,29 +121,22 @@ func (tc *testContext) testNotebookResourcesDeletion(nbMeta *metav1.ObjectMeta) return fmt.Errorf("unable to delete Route %s : %v", nbMeta.Name, err) } - // Verify Notebook Network Policies are deleted - nbNetworkPolicyList := netv1.NetworkPolicyList{} - opts := []client.ListOption{ - client.InNamespace(nbMeta.Namespace)} - err = wait.Poll(tc.resourceRetryInterval, tc.resourceCreationTimeout, func() (done bool, err error) { - nperr := tc.customClient.List(tc.ctx, &nbNetworkPolicyList, opts...) - if nperr != nil { - if errors.IsNotFound(nperr) { - return true, nil - } - log.Printf("Failed to get Network policies for %v", nbMeta.Name) - return false, err + return nil +} - } - if len(nbNetworkPolicyList.Items) == 0 { - return true, nil - } - return false, nil - }) +func filterServiceMeshManagedPolicies(nbMeta *metav1.ObjectMeta) []client.ListOption { + labelSelectorReq, err := labels.NewRequirement("app.kubernetes.io/managed-by", selection.NotIn, []string{"maistra-istio-operator"}) if err != nil { - return fmt.Errorf("unable to delete Network policies for %s : %v", nbMeta.Name, err) + log.Fatal(err) + } + + notManagedByMeshLabel := labels.NewSelector() + notManagedByMeshLabel = notManagedByMeshLabel.Add(*labelSelectorReq) + + return []client.ListOption{ + client.InNamespace(nbMeta.Namespace), + client.MatchingLabelsSelector{Selector: notManagedByMeshLabel}, } - return nil } func (tc *testContext) isNotebookCRD() error { From 28ba6bc1fd97fdb74c3fa6b8d950fe0718e23a1a Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Mon, 5 Jun 2023 14:00:29 +0200 Subject: [PATCH 08/10] chore: improves readibility of assertions depending on deployment scenario --- .../e2e/notebook_creation_test.go | 8 ++--- .../e2e/notebook_deletion_test.go | 35 +++++++++---------- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/components/odh-notebook-controller/e2e/notebook_creation_test.go b/components/odh-notebook-controller/e2e/notebook_creation_test.go index c94b2536e5e..f5c7391117e 100644 --- a/components/odh-notebook-controller/e2e/notebook_creation_test.go +++ b/components/odh-notebook-controller/e2e/notebook_creation_test.go @@ -130,11 +130,11 @@ func (tc *testContext) testNetworkPolicyCreation(nbMeta *metav1.ObjectMeta) erro return err } - if deploymentMode == ServiceMesh { - // Scenario below is not part of service mesh deployment - return nil + if deploymentMode == OAuthProxy { + return tc.ensureOAuthNetworkPolicyExists(nbMeta, err) } - return tc.ensureOAuthNetworkPolicyExists(nbMeta, err) + + return nil } func (tc *testContext) ensureOAuthNetworkPolicyExists(nbMeta *metav1.ObjectMeta, err error) error { diff --git a/components/odh-notebook-controller/e2e/notebook_deletion_test.go b/components/odh-notebook-controller/e2e/notebook_deletion_test.go index 541c0edd349..2d34eef86d7 100644 --- a/components/odh-notebook-controller/e2e/notebook_deletion_test.go +++ b/components/odh-notebook-controller/e2e/notebook_deletion_test.go @@ -97,28 +97,25 @@ func (tc *testContext) testNotebookResourcesDeletion(nbMeta *metav1.ObjectMeta) return fmt.Errorf("unable to delete Network policies for %s : %v", nbMeta.Name, err) } - if deploymentMode == ServiceMesh { - // Subsequent resources are create for deployments with oauth-proxy - return nil - } + if deploymentMode == OAuthProxy { + // Verify Notebook Route is deleted + nbRouteLookupKey := types.NamespacedName{Name: nbMeta.Name, Namespace: tc.testNamespace} + nbRoute := &routev1.Route{} + err = wait.Poll(tc.resourceRetryInterval, tc.resourceCreationTimeout, func() (done bool, err error) { + err = tc.customClient.Get(tc.ctx, nbRouteLookupKey, nbRoute) + if err != nil { + if errors.IsNotFound(err) { + return true, nil + } + log.Printf("Failed to get %s Route", nbMeta.Name) + return false, err - // Verify Notebook Route is deleted - nbRouteLookupKey := types.NamespacedName{Name: nbMeta.Name, Namespace: tc.testNamespace} - nbRoute := &routev1.Route{} - err = wait.Poll(tc.resourceRetryInterval, tc.resourceCreationTimeout, func() (done bool, err error) { - err = tc.customClient.Get(tc.ctx, nbRouteLookupKey, nbRoute) - if err != nil { - if errors.IsNotFound(err) { - return true, nil } - log.Printf("Failed to get %s Route", nbMeta.Name) - return false, err - + return false, nil + }) + if err != nil { + return fmt.Errorf("unable to delete Route %s : %v", nbMeta.Name, err) } - return false, nil - }) - if err != nil { - return fmt.Errorf("unable to delete Route %s : %v", nbMeta.Name, err) } return nil From 3fead380c230866d4232b978e17b03d872c8d850 Mon Sep 17 00:00:00 2001 From: Bartosz Majsak Date: Mon, 5 Jun 2023 14:17:09 +0200 Subject: [PATCH 09/10] chore: reverts image tags --- components/notebook-controller/config/base/kustomization.yaml | 2 +- .../odh-notebook-controller/config/base/kustomization.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/components/notebook-controller/config/base/kustomization.yaml b/components/notebook-controller/config/base/kustomization.yaml index b50b7dbac80..438b00a0946 100644 --- a/components/notebook-controller/config/base/kustomization.yaml +++ b/components/notebook-controller/config/base/kustomization.yaml @@ -4,4 +4,4 @@ resources: - ../default images: - name: docker.io/kubeflownotebookswg/notebook-controller - newTag: latest + newTag: v1.6.1 diff --git a/components/odh-notebook-controller/config/base/kustomization.yaml b/components/odh-notebook-controller/config/base/kustomization.yaml index 6afdeebdd4f..10887962092 100644 --- a/components/odh-notebook-controller/config/base/kustomization.yaml +++ b/components/odh-notebook-controller/config/base/kustomization.yaml @@ -6,4 +6,4 @@ resources: images: - name: quay.io/opendatahub/odh-notebook-controller newName: quay.io/opendatahub/odh-notebook-controller - newTag: v1.6.1-2-5-gac345842 + newTag: latest From 9a88950ae0cd249089e6d48e4328ee00ac632bc4 Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Wed, 14 Jun 2023 16:49:43 +0200 Subject: [PATCH 10/10] chore(make): allows to set K8S_NS as env variable --- components/odh-notebook-controller/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/odh-notebook-controller/Makefile b/components/odh-notebook-controller/Makefile index 879a2db5f11..676428a132f 100644 --- a/components/odh-notebook-controller/Makefile +++ b/components/odh-notebook-controller/Makefile @@ -12,7 +12,7 @@ CONTAINER_ENGINE ?= podman ENVTEST_K8S_VERSION = 1.23 # Kubernetes configuration -K8S_NAMESPACE = odh-notebook-controller-system +K8S_NAMESPACE ?= odh-notebook-controller-system # Webhook configuration WEBHOOK_PORT = 8443 @@ -202,7 +202,7 @@ undeploy-dev: setup undeploy-kf ## Undeploy controller from the Openshift cluste e2e-test-%: $(eval deploymentMode:=$(subst e2e-test-,,$@)) go test ./e2e/ -run ^TestE2ENotebookController -v \ - --nb-namespace=${K8S_NAMESPACE} \ + --nb-namespace=$(K8S_NAMESPACE) \ --deploymentMode=$(deploymentMode) \ ${E2E_TEST_FLAGS}