feat: improve Operator performance by using caching to reduce api call

and network impact (work based on opendatahub-io#1189) - secret: default application namespace + other default ones + istio cert - configmap: all - namespace: all - ingressctrler: "default" one - deployment: default application namespaces + default namespaces Signed-off-by: Wen Zhou <wenzhou@redhat.com>
zdtsw-forking · Aug 19, 2024 · fa66b99 · fa66b99
1 parent a88cef0
commit fa66b99
Show file tree

Hide file tree

Showing 24 changed files with 438 additions and 2,219 deletions.
diff --git a/bundle/manifests/opendatahub-operator-webhook-service_v1_service.yaml b/bundle/manifests/opendatahub-operator-webhook-service_v1_service.yaml
@@ -3,14 +3,11 @@ kind: Service
 metadata:
   annotations:
     service.beta.openshift.io/inject-cabundle: "true"
-    service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-controller-webhook-cert
+    service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-webhook-cert
   creationTimestamp: null
   labels:
-    app.kubernetes.io/component: webhook
     app.kubernetes.io/created-by: opendatahub-operator
-    app.kubernetes.io/instance: webhook-service
     app.kubernetes.io/managed-by: kustomize
-    app.kubernetes.io/name: service
     app.kubernetes.io/part-of: opendatahub-operator
   name: opendatahub-operator-webhook-service
 spec:

diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml
@@ -1758,7 +1758,7 @@ spec:
               - name: cert
                 secret:
                   defaultMode: 420
-                  secretName: opendatahub-operator-controller-webhook-cert
+                  secretName: opendatahub-operator-controller-manager-service-cert
     strategy: deployment
   installModes:
   - supported: false

diff --git a/components/kserve/kserve_config_handler.go b/components/kserve/kserve_config_handler.go
@@ -58,7 +58,7 @@ func (k *Kserve) setDefaultDeploymentMode(ctx context.Context, cli client.Client
 		Name:      KserveConfigMapName,
 	}, inferenceServiceConfigMap)
 	if err != nil {
-		return fmt.Errorf("error getting configmap 'inferenceservice-config'. %w", err)
+		return fmt.Errorf("error getting configmap %v: %w", KserveConfigMapName, err)
 	}
 
 	// set data.deploy.defaultDeploymentMode to the model specified in the Kserve spec

diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml
@@ -7,7 +7,7 @@ resources:
 - bases/features.opendatahub.io_featuretrackers.yaml
 #+kubebuilder:scaffold:crdkustomizeresource
 
-patches:
+# patches:
 # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix.
 # patches here are for enabling the conversion webhook for each CRD
 #- patches/webhook_in_dscinitiatlizations.yaml

diff --git a/config/default/manager_webhook_patch.yaml b/config/default/manager_webhook_patch.yaml
@@ -20,4 +20,4 @@ spec:
       - name: cert
         secret:
           defaultMode: 420
-          secretName: opendatahub-operator-controller-webhook-cert
+          secretName: opendatahub-operator-webhook-service
diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -1062,6 +1062,7 @@ rules:
   - consoles
   verbs:
   - delete
+  - get
   - list
   - patch
   - watch

diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml
@@ -6,4 +6,4 @@ commonAnnotations:
   service.beta.openshift.io/inject-cabundle: "true"
 
 configurations:
-- kustomizeconfig.yaml
+- kustomizeconfig.yaml
diff --git a/config/webhook/kustomizeconfig.yaml b/config/webhook/kustomizeconfig.yaml
@@ -4,18 +4,11 @@ nameReference:
 - kind: Service
   version: v1
   fieldSpecs:
-  - kind: MutatingWebhookConfiguration
-    group: admissionregistration.k8s.io
-    path: webhooks/clientConfig/service/name
   - kind: ValidatingWebhookConfiguration
     group: admissionregistration.k8s.io
     path: webhooks/clientConfig/service/name
 
 namespace:
-- kind: MutatingWebhookConfiguration
-  group: admissionregistration.k8s.io
-  path: webhooks/clientConfig/service/namespace
-  create: true
 - kind: ValidatingWebhookConfiguration
   group: admissionregistration.k8s.io
   path: webhooks/clientConfig/service/namespace

diff --git a/config/webhook/service.yaml b/config/webhook/service.yaml
@@ -1,18 +1,13 @@
-
 apiVersion: v1
 kind: Service
 metadata:
   labels:
-    app.kubernetes.io/name: service
-    app.kubernetes.io/instance: webhook-service
-    app.kubernetes.io/component: webhook
     app.kubernetes.io/created-by: opendatahub-operator
     app.kubernetes.io/part-of: opendatahub-operator
     app.kubernetes.io/managed-by: kustomize
   name: webhook-service
-  namespace: system
   annotations:
-    service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-controller-webhook-cert
+    service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-webhook-cert
 spec:
   ports:
     - port: 443

diff --git a/controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go b/controllers/certconfigmapgenerator/certconfigmapgenerator_controller.go
@@ -18,7 +18,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	annotation "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
@@ -37,8 +36,8 @@ func (r *CertConfigmapGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) er
 	r.Log.Info("Adding controller for Configmap Generation.")
 	return ctrl.NewControllerManagedBy(mgr).
 		Named("cert-configmap-generator-controller").
-		Watches(&source.Kind{Type: &corev1.ConfigMap{}}, handler.EnqueueRequestsFromMapFunc(r.watchTrustedCABundleConfigMapResource), builder.WithPredicates(ConfigMapChangedPredicate)).
-		Watches(&source.Kind{Type: &corev1.Namespace{}}, handler.EnqueueRequestsFromMapFunc(r.watchNamespaceResource), builder.WithPredicates(NamespaceCreatedPredicate)).
+		Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.watchTrustedCABundleConfigMapResource), builder.WithPredicates(ConfigMapChangedPredicate)).
+		Watches(&corev1.Namespace{}, handler.EnqueueRequestsFromMapFunc(r.watchNamespaceResource), builder.WithPredicates(NamespaceCreatedPredicate)).
 		Complete(r)
 }
 
@@ -97,7 +96,7 @@ func (r *CertConfigmapGeneratorReconciler) Reconcile(ctx context.Context, req ct
 	return ctrl.Result{}, nil
 }
 
-func (r *CertConfigmapGeneratorReconciler) watchNamespaceResource(a client.Object) []reconcile.Request {
+func (r *CertConfigmapGeneratorReconciler) watchNamespaceResource(_ context.Context, a client.Object) []reconcile.Request {
 	namespace, isNamespaceObject := a.(*corev1.Namespace)
 	if !isNamespaceObject {
 		return nil
@@ -108,7 +107,7 @@ func (r *CertConfigmapGeneratorReconciler) watchNamespaceResource(a client.Objec
 	return nil
 }
 
-func (r *CertConfigmapGeneratorReconciler) watchTrustedCABundleConfigMapResource(a client.Object) []reconcile.Request {
+func (r *CertConfigmapGeneratorReconciler) watchTrustedCABundleConfigMapResource(_ context.Context, a client.Object) []reconcile.Request {
 	if a.GetName() == trustedcabundle.CAConfigMapName {
 		r.Log.Info("Cert configmap has been updated, start reconcile")
 		return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: a.GetName(), Namespace: a.GetNamespace()}}}

diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go
@@ -48,7 +48,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
@@ -438,77 +437,116 @@ func (r *DataScienceClusterReconciler) SetupWithManager(ctx context.Context, mgr
 		For(&dscv1.DataScienceCluster{}).
 		Owns(&corev1.Namespace{}).
 		Owns(&corev1.Secret{}).
-		Owns(&corev1.ConfigMap{}, builder.WithPredicates(configMapPredicates)).
-		Owns(&networkingv1.NetworkPolicy{}, builder.WithPredicates(networkpolicyPredicates)).
-		Owns(&rbacv1.Role{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRolePredicates))).
-		Owns(&rbacv1.RoleBinding{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
-		Owns(&rbacv1.ClusterRole{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRolePredicates))).
-		Owns(&rbacv1.ClusterRoleBinding{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
+		Owns(
+			&corev1.ConfigMap{},
+			builder.WithPredicates(configMapPredicates),
+		).
+		Owns(
+			&networkingv1.NetworkPolicy{},
+			builder.WithPredicates(networkpolicyPredicates),
+		).
+		Owns(
+			&rbacv1.Role{},
+			builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRolePredicates))).
+		Owns(
+			&rbacv1.RoleBinding{},
+			builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
+		Owns(
+			&rbacv1.ClusterRole{},
+			builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRolePredicates))).
+		Owns(
+			&rbacv1.ClusterRoleBinding{},
+			builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
 		Owns(&appsv1.Deployment{}).
 		Owns(&corev1.PersistentVolumeClaim{}).
-		Owns(&corev1.Service{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshGeneralPredicates))).
+		Owns(
+			&corev1.Service{},
+			builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshGeneralPredicates))).
 		Owns(&appsv1.StatefulSet{}).
 		Owns(&imagev1.ImageStream{}).
 		Owns(&buildv1.BuildConfig{}).
 		Owns(&apiregistrationv1.APIService{}).
 		Owns(&networkingv1.Ingress{}).
 		Owns(&admissionregistrationv1.MutatingWebhookConfiguration{}).
-		Owns(&admissionregistrationv1.ValidatingWebhookConfiguration{}, builder.WithPredicates(modelMeshwebhookPredicates)).
-		Owns(&corev1.ServiceAccount{}, builder.WithPredicates(saPredicates)).
-		Watches(&source.Kind{Type: &dsciv1.DSCInitialization{}}, handler.EnqueueRequestsFromMapFunc(r.watchDataScienceClusterForDSCI(ctx))).
-		Watches(&source.Kind{Type: &corev1.ConfigMap{}}, handler.EnqueueRequestsFromMapFunc(r.watchDataScienceClusterResources(ctx)), builder.WithPredicates(configMapPredicates)).
-		Watches(&source.Kind{Type: &apiextensionsv1.CustomResourceDefinition{}}, handler.EnqueueRequestsFromMapFunc(r.watchDataScienceClusterResources(ctx)),
-			builder.WithPredicates(argoWorkflowCRDPredicates)).
-		Watches(&source.Kind{Type: &corev1.Secret{}}, handler.EnqueueRequestsFromMapFunc(r.watchDefaultIngressSecret(ctx)), builder.WithPredicates(defaultIngressCertSecretPredicates)).
+		Owns(
+			&admissionregistrationv1.ValidatingWebhookConfiguration{},
+			builder.WithPredicates(modelMeshwebhookPredicates),
+		).
+		Owns(
+			&corev1.ServiceAccount{},
+			builder.WithPredicates(saPredicates),
+		).
+		Watches(
+			&dsciv1.DSCInitialization{},
+			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
+				return r.watchDataScienceClusterForDSCI(ctx, a)
+			},
+			)).
+		Watches(
+			&corev1.ConfigMap{},
+			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
+				return r.watchDataScienceClusterResources(ctx, a)
+			}),
+			builder.WithPredicates(configMapPredicates),
+		).
+		Watches(
+			&apiextensionsv1.CustomResourceDefinition{},
+			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
+				return r.watchDataScienceClusterResources(ctx, a)
+			}),
+			builder.WithPredicates(argoWorkflowCRDPredicates),
+		).
+		Watches(
+			&corev1.Secret{},
+			handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
+				return r.watchDefaultIngressSecret(ctx, a)
+			}),
+			builder.WithPredicates(defaultIngressCertSecretPredicates)).
 		// this predicates prevents meaningless reconciliations from being triggered
 		WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, predicate.LabelChangedPredicate{})).
 		Complete(r)
 }
 
-func (r *DataScienceClusterReconciler) watchDataScienceClusterForDSCI(ctx context.Context) func(client.Object) []reconcile.Request {
-	return func(a client.Object) []reconcile.Request {
-		requestName, err := r.getRequestName(ctx)
-		if err != nil {
-			return nil
-		}
-		// When DSCI CR gets created, trigger reconcile function
-		if a.GetObjectKind().GroupVersionKind().Kind == "DSCInitialization" || a.GetName() == "default-dsci" {
-			return []reconcile.Request{{
-				NamespacedName: types.NamespacedName{Name: requestName},
-			}}
-		}
+func (r *DataScienceClusterReconciler) watchDataScienceClusterForDSCI(ctx context.Context, a client.Object) []reconcile.Request {
+	requestName, err := r.getRequestName(ctx)
+	if err != nil {
 		return nil
 	}
+	// When DSCI CR gets created, trigger reconcile function
+	if a.GetObjectKind().GroupVersionKind().Kind == "DSCInitialization" || a.GetName() == "default-dsci" {
+		return []reconcile.Request{{
+			NamespacedName: types.NamespacedName{Name: requestName},
+		}}
+	}
+	return nil
 }
 
-func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(ctx context.Context) func(client.Object) []reconcile.Request {
-	return func(a client.Object) []reconcile.Request {
-		requestName, err := r.getRequestName(ctx)
-		if err != nil {
-			return nil
-		}
+func (r *DataScienceClusterReconciler) watchDataScienceClusterResources(ctx context.Context, a client.Object) []reconcile.Request {
+	requestName, err := r.getRequestName(ctx)
+	if err != nil {
+		return nil
+	}
+
+	if a.GetObjectKind().GroupVersionKind().Kind == "CustomResourceDefinition" || a.GetName() == "ArgoWorkflowCRD" {
+		return []reconcile.Request{{
+			NamespacedName: types.NamespacedName{Name: requestName},
+		}}
+	}
 
-		if a.GetObjectKind().GroupVersionKind().Kind == "CustomResourceDefinition" || a.GetName() == "ArgoWorkflowCRD" {
+	// Trigger reconcile function when uninstall configmap is created
+	operatorNs, err := cluster.GetOperatorNamespace()
+	if err != nil {
+		return nil
+	}
+	if a.GetNamespace() == operatorNs {
+		cmLabels := a.GetLabels()
+		if val, ok := cmLabels[upgrade.DeleteConfigMapLabel]; ok && val == "true" {
 			return []reconcile.Request{{
 				NamespacedName: types.NamespacedName{Name: requestName},
 			}}
 		}
-
-		// Trigger reconcile function when uninstall configmap is created
-		operatorNs, err := cluster.GetOperatorNamespace()
-		if err != nil {
-			return nil
-		}
-		if a.GetNamespace() == operatorNs {
-			cmLabels := a.GetLabels()
-			if val, ok := cmLabels[upgrade.DeleteConfigMapLabel]; ok && val == "true" {
-				return []reconcile.Request{{
-					NamespacedName: types.NamespacedName{Name: requestName},
-				}}
-			}
-		}
-		return nil
 	}
+	return nil
 }
 
 func (r *DataScienceClusterReconciler) getRequestName(ctx context.Context) (string, error) {
@@ -543,25 +581,23 @@ var argoWorkflowCRDPredicates = predicate.Funcs{
 	},
 }
 
-func (r *DataScienceClusterReconciler) watchDefaultIngressSecret(ctx context.Context) func(client.Object) []reconcile.Request {
-	return func(a client.Object) []reconcile.Request {
-		requestName, err := r.getRequestName(ctx)
-		if err != nil {
-			return nil
-		}
-		// When ingress secret gets created/deleted, trigger reconcile function
-		ingressCtrl, err := cluster.FindAvailableIngressController(ctx, r.Client)
-		if err != nil {
-			return nil
-		}
-		defaultIngressSecretName := cluster.GetDefaultIngressCertSecretName(ingressCtrl)
-		if a.GetName() == defaultIngressSecretName && a.GetNamespace() == "openshift-ingress" {
-			return []reconcile.Request{{
-				NamespacedName: types.NamespacedName{Name: requestName},
-			}}
-		}
+func (r *DataScienceClusterReconciler) watchDefaultIngressSecret(ctx context.Context, a client.Object) []reconcile.Request {
+	requestName, err := r.getRequestName(ctx)
+	if err != nil {
 		return nil
 	}
+	// When ingress secret gets created/deleted, trigger reconcile function
+	ingressCtrl, err := cluster.FindAvailableIngressController(ctx, r.Client)
+	if err != nil {
+		return nil
+	}
+	defaultIngressSecretName := cluster.GetDefaultIngressCertSecretName(ingressCtrl)
+	if a.GetName() == defaultIngressSecretName && a.GetNamespace() == "openshift-ingress" {
+		return []reconcile.Request{{
+			NamespacedName: types.NamespacedName{Name: requestName},
+		}}
+	}
+	return nil
 }
 
 // defaultIngressCertSecretPredicates filters delete and create events to trigger reconcile when default ingress cert secret is expired

diff --git a/controllers/datasciencecluster/kubebuilder_rbac.go b/controllers/datasciencecluster/kubebuilder_rbac.go
@@ -97,7 +97,7 @@ package datasciencecluster
 
 // +kubebuilder:rbac:groups="apiregistration.k8s.io",resources=apiservices,verbs=create;delete;list;watch;update;patch;get
 
-// +kubebuilder:rbac:groups="operator.openshift.io",resources=consoles,verbs=list;watch;patch;delete
+// +kubebuilder:rbac:groups="operator.openshift.io",resources=consoles,verbs=get;list;watch;patch;delete
 // +kubebuilder:rbac:groups="operator.openshift.io",resources=ingresscontrollers,verbs=get;list;watch;patch;delete
 
 // +kubebuilder:rbac:groups="oauth.openshift.io",resources=oauthclients,verbs=create;delete;list;watch;update;patch;get
-Original file line number
+Diff line change
@@ Expand Up / @@ -1062,6 +1062,7 @@ rules: @@
       - consoles
       verbs:
       - delete
+      - get
       - list
       - patch
       - watch
@@ Expand Down @@