Merge branch 'incubation' into authorino-rebase-bup

Signed-off-by: Edgar Hernández <[email protected]>
opendatahub-io · Feb 19, 2024 · abe7daa · abe7daa
2 parents c60ea4b + e7e3982
commit abe7daa
Show file tree

Hide file tree

Showing 9 changed files with 80 additions and 95 deletions.
diff --git a/bundle/manifests/dscinitialization.opendatahub.io_dscinitializations.yaml b/bundle/manifests/dscinitialization.opendatahub.io_dscinitializations.yaml
@@ -153,6 +153,7 @@ spec:
                   field.
                 properties:
                   customCABundle:
+                    default: ""
                     description: A custom CA bundle that will be available for  all  components
                       in the Data Science Cluster(DSC). This bundle will be stored
                       in odh-trusted-ca-bundle ConfigMap .data.odh-ca-bundle.crt .
@@ -168,6 +169,7 @@ spec:
                     pattern: ^(Managed|Unmanaged|Force|Removed)$
                     type: string
                 required:
+                - customCABundle
                 - managementState
                 type: object
             required:

diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml
@@ -1911,24 +1911,19 @@ spec:
     - v1
     containerPort: 443
     deploymentName: opendatahub-operator-controller-manager
-    failurePolicy: Ignore
+    failurePolicy: Fail
     generateName: operator.opendatahub.io
     rules:
     - apiGroups:
       - datasciencecluster.opendatahub.io
-      apiVersions:
-      - v1
-      operations:
-      - CREATE
-      resources:
-      - datascienceclusters
-    - apiGroups:
       - dscinitialization.opendatahub.io
       apiVersions:
       - v1
       operations:
       - CREATE
+      - UPDATE
       resources:
+      - datascienceclusters
       - dscinitializations
     sideEffects: None
     targetPort: 9443

diff --git a/config/crd/bases/dscinitialization.opendatahub.io_dscinitializations.yaml b/config/crd/bases/dscinitialization.opendatahub.io_dscinitializations.yaml
@@ -154,6 +154,7 @@ spec:
                   field.
                 properties:
                   customCABundle:
+                    default: ""
                     description: A custom CA bundle that will be available for  all  components
                       in the Data Science Cluster(DSC). This bundle will be stored
                       in odh-trusted-ca-bundle ConfigMap .data.odh-ca-bundle.crt .
@@ -169,6 +170,7 @@ spec:
                     pattern: ^(Managed|Unmanaged|Force|Removed)$
                     type: string
                 required:
+                - customCABundle
                 - managementState
                 type: object
             required:

diff --git a/controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go b/controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go
@@ -49,7 +49,7 @@ func (r *CertConfigmapGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) er
 // ca bundle in every new namespace created.
 func (r *CertConfigmapGeneratorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	// Request includes namespace that is newly created or where odh-trusted-ca-bundle configmap is updated.
-	r.Log.Info("Reconciling certConfigMapGenerator.", " CertConfigMapGenerator Request.Namespace", req.NamespacedName)
+	r.Log.Info("Reconciling certConfigMapGenerator.", "CertConfigMapGenerator Request.Namespace", req.NamespacedName)
 	// Get namespace instance
 	userNamespace := &corev1.Namespace{}
 	err := r.Client.Get(ctx, client.ObjectKey{Name: req.Namespace}, userNamespace)
@@ -71,9 +71,6 @@ func (r *CertConfigmapGeneratorReconciler) Reconcile(ctx context.Context, req ct
 		return ctrl.Result{}, nil
 	case 1:
 		dsciInstance = &dsciInstances.Items[0]
-	default:
-		message := "only one instance of DSCInitialization object is allowed"
-		return ctrl.Result{}, errors.New(message)
 	}
 
 	if dsciInstance.Spec.TrustedCABundle.ManagementState != operatorv1.Managed {

diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go
@@ -81,7 +81,7 @@ const (
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
-func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:gocyclo,maintidx
+func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:gocyclo
 	r.Log.Info("Reconciling DataScienceCluster resources", "Request.Name", req.Name)
 
 	instances := &dsc.DataScienceClusterList{}
@@ -137,19 +137,6 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 		return reconcile.Result{Requeue: true}, nil
 	}
 
-	if len(instances.Items) > 1 {
-		message := fmt.Sprintf("only one instance of DataScienceCluster object is allowed. Update existing instance %s", req.Name)
-		err := errors.New(message)
-		_ = r.reportError(err, instance, message)
-
-		_, _ = r.updateStatus(ctx, instance, func(saved *dsc.DataScienceCluster) {
-			status.SetErrorCondition(&saved.Status.Conditions, status.DuplicateDataScienceCluster, message)
-			saved.Status.Phase = status.PhaseError
-		})
-
-		return ctrl.Result{}, err
-	}
-
 	// Verify a valid DSCInitialization instance is created
 	dsciInstances := &dsci.DSCInitializationList{}
 	err = r.Client.List(ctx, dsciInstances)
@@ -161,7 +148,7 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 	}
 
 	// Update phase to error state if DataScienceCluster is created without valid DSCInitialization
-	switch len(dsciInstances.Items) {
+	switch len(dsciInstances.Items) { // only handle number as 0 or 1, others won't be existed since webhook block creation
 	case 0:
 		reason := status.ReconcileFailed
 		message := "Failed to get a valid DSCInitialization instance, please create a DSCI instance"
@@ -179,14 +166,6 @@ func (r *DataScienceClusterReconciler) Reconcile(ctx context.Context, req ctrl.R
 	case 1:
 		dscInitializationSpec := dsciInstances.Items[0].Spec
 		dscInitializationSpec.DeepCopyInto(r.DataScienceCluster.DSCISpec)
-	default:
-		message := "only one instance of DSCInitialization object is allowed"
-		_, _ = r.updateStatus(ctx, instance, func(saved *dsc.DataScienceCluster) {
-			status.SetErrorCondition(&saved.Status.Conditions, status.DuplicateDSCInitialization, message)
-			saved.Status.Phase = status.PhaseError
-		})
-
-		return ctrl.Result{}, errors.New(message)
 	}
 
 	if instance.ObjectMeta.DeletionTimestamp.IsZero() {

diff --git a/controllers/dscinitialization/dscinitialization_controller.go b/controllers/dscinitialization/dscinitialization_controller.go
@@ -19,8 +19,6 @@ package dscinitialization
 
 import (
 	"context"
-	"errors"
-	"fmt"
 	"path/filepath"
 	"reflect"
 
@@ -90,33 +88,11 @@ func (r *DSCInitializationReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	}
 
 	var instance *dsciv1.DSCInitialization
-	switch {
+	switch { // only handle number as 0 or 1, others won't be existed since webhook block creation
 	case len(instances.Items) == 0:
 		return ctrl.Result{}, nil
 	case len(instances.Items) == 1:
 		instance = &instances.Items[0]
-	case len(instances.Items) > 1:
-		// find out the one by created timestamp and use it as the default one
-		earliestDSCI := &instances.Items[0]
-		for _, instance := range instances.Items {
-			currentDSCI := instance
-			if currentDSCI.CreationTimestamp.Before(&earliestDSCI.CreationTimestamp) {
-				earliestDSCI = &currentDSCI
-			}
-		}
-		message := fmt.Sprintf("only one instance of DSCInitialization object is allowed. Please delete other instances than %s", earliestDSCI.Name)
-		// update all instances Message and Status
-		for _, deletionInstance := range instances.Items {
-			deletionInstance := deletionInstance
-			if deletionInstance.Name != earliestDSCI.Name {
-				_, _ = r.updateStatus(ctx, &deletionInstance, func(saved *dsciv1.DSCInitialization) {
-					status.SetErrorCondition(&saved.Status.Conditions, status.DuplicateDSCInitialization, message)
-					saved.Status.Phase = status.PhaseError
-				})
-			}
-		}
-
-		return ctrl.Result{}, errors.New(message)
 	}
 
 	if instance.ObjectMeta.DeletionTimestamp.IsZero() {

diff --git a/controllers/dscinitialization/dscinitialization_test.go b/controllers/dscinitialization/dscinitialization_test.go
@@ -19,6 +19,7 @@ import (
 
 const (
 	workingNamespace     = "test-operator-ns"
+	applicationName      = "default-dsci"
 	applicationNamespace = "test-application-ns"
 	configmapName        = "odh-common-config"
 	monitoringNamespace  = "test-monitoring-ns"
@@ -28,10 +29,10 @@ const (
 var _ = Describe("DataScienceCluster initialization", func() {
 	Context("Creation of related resources", func() {
 		// must be default as instance name, or it will break
-		const applicationName = "default-dsci"
+
 		BeforeEach(func() {
 			// when
-			desiredDsci := createDSCI(applicationName, operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
+			desiredDsci := createDSCI(operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
 			Expect(k8sClient.Create(context.Background(), desiredDsci)).Should(Succeed())
 			foundDsci := &dsci.DSCInitialization{}
 			Eventually(dscInitializationIsReady(applicationName, workingNamespace, foundDsci)).
@@ -111,7 +112,7 @@ var _ = Describe("DataScienceCluster initialization", func() {
 		const applicationName = "default-dsci"
 		It("Should not create monitoring namespace if monitoring is disabled", func() {
 			// when
-			desiredDsci := createDSCI(applicationName, operatorv1.Removed, operatorv1.Managed, monitoringNamespace2)
+			desiredDsci := createDSCI(operatorv1.Removed, operatorv1.Managed, monitoringNamespace2)
 			Expect(k8sClient.Create(context.Background(), desiredDsci)).Should(Succeed())
 			foundDsci := &dsci.DSCInitialization{}
 			Eventually(dscInitializationIsReady(applicationName, workingNamespace, foundDsci)).
@@ -127,7 +128,7 @@ var _ = Describe("DataScienceCluster initialization", func() {
 		})
 		It("Should create default monitoring namespace if monitoring enabled", func() {
 			// when
-			desiredDsci := createDSCI(applicationName, operatorv1.Managed, operatorv1.Managed, monitoringNamespace2)
+			desiredDsci := createDSCI(operatorv1.Managed, operatorv1.Managed, monitoringNamespace2)
 			Expect(k8sClient.Create(context.Background(), desiredDsci)).Should(Succeed())
 			foundDsci := &dsci.DSCInitialization{}
 			Eventually(dscInitializationIsReady(applicationName, workingNamespace, foundDsci)).
@@ -148,21 +149,6 @@ var _ = Describe("DataScienceCluster initialization", func() {
 		AfterEach(cleanupResources)
 		const applicationName = "default-dsci"
 
-		It("Should not have more than one DSCI instance in the cluster", func() {
-
-			anotherApplicationName := "default2"
-			// given
-			desiredDsci := createDSCI(applicationName, operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
-			Expect(k8sClient.Create(context.Background(), desiredDsci)).Should(Succeed())
-			// when
-			desiredDsci2 := createDSCI(anotherApplicationName, operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
-			// then
-			Eventually(dscInitializationIsReady(anotherApplicationName, workingNamespace, desiredDsci2)).
-				WithTimeout(timeout).
-				WithPolling(interval).
-				Should(BeFalse())
-		})
-
 		It("Should not update rolebinding if it exists", func() {
 
 			// given
@@ -190,7 +176,7 @@ var _ = Describe("DataScienceCluster initialization", func() {
 				Should(BeTrue())
 
 			// when
-			desiredDsci := createDSCI(applicationName, operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
+			desiredDsci := createDSCI(operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
 			Expect(k8sClient.Create(context.Background(), desiredDsci)).Should(Succeed())
 			foundDsci := &dsci.DSCInitialization{}
 			Eventually(dscInitializationIsReady(applicationName, workingNamespace, foundDsci)).
@@ -230,7 +216,7 @@ var _ = Describe("DataScienceCluster initialization", func() {
 				Should(BeTrue())
 
 			// when
-			desiredDsci := createDSCI(applicationName, operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
+			desiredDsci := createDSCI(operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
 			Expect(k8sClient.Create(context.Background(), desiredDsci)).Should(Succeed())
 			foundDsci := &dsci.DSCInitialization{}
 			Eventually(dscInitializationIsReady(applicationName, workingNamespace, foundDsci)).
@@ -266,7 +252,7 @@ var _ = Describe("DataScienceCluster initialization", func() {
 				Should(BeTrue())
 
 			// when
-			desiredDsci := createDSCI(applicationName, operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
+			desiredDsci := createDSCI(operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
 			Expect(k8sClient.Create(context.Background(), desiredDsci)).Should(Succeed())
 			foundDsci := &dsci.DSCInitialization{}
 			Eventually(dscInitializationIsReady(applicationName, workingNamespace, foundDsci)).
@@ -340,14 +326,14 @@ func objectExists(ns string, name string, obj client.Object) func() bool { //nol
 	}
 }
 
-func createDSCI(appName string, enableMonitoring operatorv1.ManagementState, enableTrustedCABundle operatorv1.ManagementState, monitoringNS string) *dsci.DSCInitialization {
+func createDSCI(enableMonitoring operatorv1.ManagementState, enableTrustedCABundle operatorv1.ManagementState, monitoringNS string) *dsci.DSCInitialization {
 	return &dsci.DSCInitialization{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "DSCInitialization",
 			APIVersion: "v1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      appName,
+			Name:      applicationName,
 			Namespace: workingNamespace,
 		},
 		Spec: dsci.DSCInitializationSpec{

diff --git a/controllers/dscinitialization/utils.go b/controllers/dscinitialization/utils.go
@@ -238,7 +238,7 @@ func (r *DSCInitializationReconciler) reconcileDefaultNetworkPolicy(ctx context.
 			r.Log.Error(err, "error to set networkpolicy in applications namespace", "path", networkpolicyPath)
 			return err
 		}
-	} else { // Expected namespace for the given name
+	} else { // Expected namespace for the given name in ODH
 		desiredNetworkPolicy := &netv1.NetworkPolicy{
 			TypeMeta: metav1.TypeMeta{
 				Kind:       "NetworkPolicy",
@@ -255,7 +255,11 @@ func (r *DSCInitializationReconciler) reconcileDefaultNetworkPolicy(ctx context.
 				Ingress: []netv1.NetworkPolicyIngressRule{
 					{
 						From: []netv1.NetworkPolicyPeer{
-							{
+							{ /* allow ODH namespace <->ODH namespace:
+								- default notebook project: rhods-notebooks
+								- redhat-odh-monitoring
+								- redhat-odh-applications / opendatahub
+								*/
 								NamespaceSelector: &metav1.LabelSelector{ // AND logic
 									MatchLabels: map[string]string{
 										cluster.ODHGeneratedNamespaceLabel: "true",
@@ -266,7 +270,7 @@ func (r *DSCInitializationReconciler) reconcileDefaultNetworkPolicy(ctx context.
 					},
 					{ // OR logic
 						From: []netv1.NetworkPolicyPeer{
-							{ // need this for access dashboard
+							{ // need this to access external-> dashboard
 								NamespaceSelector: &metav1.LabelSelector{
 									MatchLabels: map[string]string{
 										"network.openshift.io/policy-group": "ingress",
@@ -277,7 +281,7 @@ func (r *DSCInitializationReconciler) reconcileDefaultNetworkPolicy(ctx context.
 					},
 					{ // OR logic for PSI
 						From: []netv1.NetworkPolicyPeer{
-							{ // need this to access dashboard
+							{ // need this to access external->dashboard
 								NamespaceSelector: &metav1.LabelSelector{
 									MatchLabels: map[string]string{
 										"kubernetes.io/metadata.name": "openshift-host-network",
@@ -286,6 +290,17 @@ func (r *DSCInitializationReconciler) reconcileDefaultNetworkPolicy(ctx context.
 							},
 						},
 					},
+					{
+						From: []netv1.NetworkPolicyPeer{
+							{ // need this for cluster-monitoring work: cluster-monitoring->ODH namespaces
+								NamespaceSelector: &metav1.LabelSelector{
+									MatchLabels: map[string]string{
+										"kubernetes.io/metadata.name": "openshift-monitoring",
+									},
+								},
+							},
+						},
+					},
 				},
 				PolicyTypes: []netv1.PolicyType{
 					netv1.PolicyTypeIngress,