Use kind instead of resource name for selectors

opendatahub-io · Nov 26, 2024 · eee0289 · eee0289
1 parent a1560ca
commit eee0289
Show file tree

Hide file tree

Showing 29 changed files with 166 additions and 166 deletions.
diff --git a/controllers/components/dashboard/dashboard_controller.go b/controllers/components/dashboard/dashboard_controller.go
@@ -44,11 +44,7 @@ import (
 func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
 	componentName := computeComponentName()
 
-	_, err := reconciler.ComponentReconcilerFor(
-		mgr,
-		componentsv1.DashboardInstanceName,
-		&componentsv1.Dashboard{},
-	).
+	_, err := reconciler.ComponentReconcilerFor(mgr, &componentsv1.Dashboard{}).
 		// operands - owned
 		Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.Secret{}).

diff --git a/controllers/components/dashboard/dashboard_controller_actions.go b/controllers/components/dashboard/dashboard_controller_actions.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 
 	routev1 "github.com/openshift/api/route/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -110,7 +111,7 @@ func updateStatus(ctx context.Context, rr *odhtypes.ReconciliationRequest) error
 		&rl,
 		client.InNamespace(rr.DSCI.Spec.ApplicationsNamespace),
 		client.MatchingLabels(map[string]string{
-			labels.ComponentPartOf: componentsv1.DashboardInstanceName,
+			labels.ComponentPartOf: strings.ToLower(componentsv1.DashboardKind),
 		}),
 	)
 

diff --git a/controllers/components/datasciencepipelines/datasciencepipelines_controller.go b/controllers/components/datasciencepipelines/datasciencepipelines_controller.go
@@ -48,11 +48,7 @@ var (
 )
 
 func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
-	_, err := reconciler.ComponentReconcilerFor(
-		mgr,
-		componentsv1.DataSciencePipelinesInstanceName,
-		&componentsv1.DataSciencePipelines{},
-	).
+	_, err := reconciler.ComponentReconcilerFor(mgr, &componentsv1.DataSciencePipelines{}).
 		// customized Owns() for Component with new predicates
 		Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.Secret{}).

diff --git a/controllers/components/kueue/kueue_controller.go b/controllers/components/kueue/kueue_controller.go
@@ -39,11 +39,7 @@ import (
 )
 
 func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
-	_, err := reconciler.ComponentReconcilerFor(
-		mgr,
-		componentsv1.KueueInstanceName,
-		&componentsv1.Kueue{},
-	).
+	_, err := reconciler.ComponentReconcilerFor(mgr, &componentsv1.Kueue{}).
 		// customized Owns() for Component with new predicates
 		Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.Secret{}).

diff --git a/controllers/components/modelregistry/modelregistry_controller.go b/controllers/components/modelregistry/modelregistry_controller.go
@@ -43,11 +43,7 @@ import (
 )
 
 func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
-	_, err := reconciler.ComponentReconcilerFor(
-		mgr,
-		componentsv1.ModelRegistryInstanceName,
-		&componentsv1.ModelRegistry{},
-	).
+	_, err := reconciler.ComponentReconcilerFor(mgr, &componentsv1.ModelRegistry{}).
 		Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.Secret{}).
 		Owns(&rbacv1.Role{}).

diff --git a/controllers/components/ray/ray_controller.go b/controllers/components/ray/ray_controller.go
@@ -37,11 +37,7 @@ import (
 )
 
 func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
-	_, err := reconciler.ComponentReconcilerFor(
-		mgr,
-		componentsv1.RayInstanceName,
-		&componentsv1.Ray{},
-	).
+	_, err := reconciler.ComponentReconcilerFor(mgr, &componentsv1.Ray{}).
 		// customized Owns() for Component with new predicates
 		Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.Secret{}).

diff --git a/controllers/components/trainingoperator/trainingoperator_controller.go b/controllers/components/trainingoperator/trainingoperator_controller.go
@@ -37,11 +37,7 @@ import (
 )
 
 func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
-	_, err := reconciler.ComponentReconcilerFor(
-		mgr,
-		componentsv1.TrainingOperatorInstanceName,
-		&componentsv1.TrainingOperator{},
-	).
+	_, err := reconciler.ComponentReconcilerFor(mgr, &componentsv1.TrainingOperator{}).
 		// customized Owns() for Component with new predicates
 		Owns(&corev1.ConfigMap{}).
 		Owns(&promv1.PodMonitor{}).

diff --git a/controllers/components/trustyai/trustyai_controller.go b/controllers/components/trustyai/trustyai_controller.go
@@ -36,11 +36,7 @@ import (
 )
 
 func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
-	_, err := reconciler.ComponentReconcilerFor(
-		mgr,
-		componentsv1.TrustyAIInstanceName,
-		&componentsv1.TrustyAI{},
-	).
+	_, err := reconciler.ComponentReconcilerFor(mgr, &componentsv1.TrustyAI{}).
 		// customized Owns() for Component with new predicates
 		Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.ServiceAccount{}).

diff --git a/pkg/controller/actions/deploy/action_deploy.go b/pkg/controller/actions/deploy/action_deploy.go
@@ -112,22 +112,15 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er
 		a.cache.Sync()
 	}
 
-	controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind)
-
-	deployHash, err := odhTypes.HashStr(rr)
+	kind, err := resources.KindForObject(rr.Client.Scheme(), rr.Instance)
 	if err != nil {
-		return fmt.Errorf("unable to compute reauest hash: %w", err)
+		return err
 	}
 
-	deployAnnotations := map[string]string{
-		annotations.ComponentGeneration: strconv.FormatInt(rr.Instance.GetGeneration(), 10),
-		annotations.ComponentHash:       deployHash,
-		annotations.PlatformType:        string(rr.Release.Name),
-		annotations.PlatformVersion:     rr.Release.Version.String(),
-	}
+	controllerName := strings.ToLower(kind)
 
 	for i := range rr.Resources {
-		ok, err := a.deploy(ctx, rr, rr.Resources[i], deployAnnotations)
+		ok, err := a.deploy(ctx, rr, rr.Resources[i])
 		if err != nil {
 			return fmt.Errorf("failure deploying %s: %w", rr.Resources[i], err)
 		}
@@ -144,7 +137,6 @@ func (a *Action) deploy(
 	ctx context.Context,
 	rr *odhTypes.ReconciliationRequest,
 	obj unstructured.Unstructured,
-	deployAnnotations map[string]string,
 ) (bool, error) {
 	current, lookupErr := a.lookup(ctx, rr.Client, obj)
 	if lookupErr != nil {
@@ -159,12 +151,21 @@ func (a *Action) deploy(
 
 	fo := a.fieldOwner
 	if fo == "" {
-		fo = rr.OwnerName
+		kind, err := resources.KindForObject(rr.Client.Scheme(), rr.Instance)
+		if err != nil {
+			return false, err
+		}
+
+		fo = strings.ToLower(kind)
 	}
 
 	resources.SetLabels(&obj, a.labels)
 	resources.SetAnnotations(&obj, a.annotations)
-	resources.SetAnnotations(&obj, deployAnnotations)
+	resources.SetAnnotation(&obj, annotations.InstanceGeneration, strconv.FormatInt(rr.Instance.GetGeneration(), 10))
+	resources.SetAnnotation(&obj, annotations.InstanceName, rr.Instance.GetName())
+	resources.SetAnnotation(&obj, annotations.InstanceUID, string(rr.Instance.GetUID()))
+	resources.SetAnnotation(&obj, annotations.PlatformType, string(rr.Release.Name))
+	resources.SetAnnotation(&obj, annotations.PlatformVersion, rr.Release.Version.String())
 
 	if resources.GetLabel(&obj, labels.ComponentPartOf) == "" && fo != "" {
 		resources.SetLabel(&obj, labels.ComponentPartOf, fo)

diff --git a/pkg/controller/actions/deploy/action_deploy_cache_test.go b/pkg/controller/actions/deploy/action_deploy_cache_test.go
@@ -40,6 +40,7 @@ func TestDeployWithCacheAction(t *testing.T) {
 	utilruntime.Must(corev1.AddToScheme(s))
 	utilruntime.Must(appsv1.AddToScheme(s))
 	utilruntime.Must(apiextensionsv1.AddToScheme(s))
+	utilruntime.Must(componentsv1.AddToScheme(s))
 
 	projectDir, err := envtestutil.FindProjectRoot()
 	g.Expect(err).NotTo(HaveOccurred())

diff --git a/pkg/controller/actions/deploy/action_deploy_test.go b/pkg/controller/actions/deploy/action_deploy_test.go
@@ -3,6 +3,7 @@ package deploy_test
 import (
 	"context"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/blang/semver/v4"
@@ -57,10 +58,9 @@ func TestDeployAction(t *testing.T) {
 	g.Expect(err).ShouldNot(HaveOccurred())
 
 	rr := types.ReconciliationRequest{
-		OwnerName: xid.New().String(),
-		Client:    cl,
-		DSCI:      &dsciv1.DSCInitialization{Spec: dsciv1.DSCInitializationSpec{ApplicationsNamespace: ns}},
-		DSC:       &dscv1.DataScienceCluster{},
+		Client: cl,
+		DSCI:   &dsciv1.DSCInitialization{Spec: dsciv1.DSCInitializationSpec{ApplicationsNamespace: ns}},
+		DSC:    &dscv1.DataScienceCluster{},
 		Instance: &componentsv1.Dashboard{
 			ObjectMeta: metav1.ObjectMeta{
 				Generation: 1,
@@ -80,13 +80,9 @@ func TestDeployAction(t *testing.T) {
 	err = cl.Get(ctx, client.ObjectKeyFromObject(obj1), obj1)
 	g.Expect(err).ShouldNot(HaveOccurred())
 
-	dh, err := types.HashStr(&rr)
-	g.Expect(err).ShouldNot(HaveOccurred())
-
 	g.Expect(obj1).Should(And(
-		jq.Match(`.metadata.labels."%s" == "%s"`, labels.ComponentPartOf, rr.OwnerName),
-		jq.Match(`.metadata.annotations."%s" == "%s"`, annotations.ComponentGeneration, strconv.FormatInt(rr.Instance.GetGeneration(), 10)),
-		jq.Match(`.metadata.annotations."%s" == "%s"`, annotations.ComponentHash, dh),
+		jq.Match(`.metadata.labels."%s" == "%s"`, labels.ComponentPartOf, strings.ToLower(componentsv1.DashboardKind)),
+		jq.Match(`.metadata.annotations."%s" == "%s"`, annotations.InstanceGeneration, strconv.FormatInt(rr.Instance.GetGeneration(), 10)),
 		jq.Match(`.metadata.annotations."%s" == "%s"`, annotations.PlatformVersion, "1.2.3"),
 		jq.Match(`.metadata.annotations."%s" == "%s"`, annotations.PlatformType, string(cluster.OpenDataHub)),
 	))

diff --git a/pkg/controller/actions/gc/action_gc.go b/pkg/controller/actions/gc/action_gc.go
@@ -12,8 +12,8 @@ import (
 
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions"
 	odhTypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
 	odhLabels "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/services/gc"
 )
 
@@ -83,19 +83,19 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er
 		return nil
 	}
 
-	controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind)
-
-	CyclesTotal.WithLabelValues(controllerName).Inc()
-
-	ch, err := odhTypes.HashStr(rr)
+	kind, err := resources.KindForObject(rr.Client.Scheme(), rr.Instance)
 	if err != nil {
 		return err
 	}
 
+	controllerName := strings.ToLower(kind)
+
+	CyclesTotal.WithLabelValues(controllerName).Inc()
+
 	selector := a.selector
 	if selector == nil {
 		selector = labels.SelectorFromSet(map[string]string{
-			odhLabels.ComponentPartOf: rr.OwnerName,
+			odhLabels.ComponentPartOf: strings.ToLower(kind),
 		})
 	}
 
@@ -107,11 +107,6 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er
 				return false, nil
 			}
 
-			objectHash := obj.GetAnnotations()[annotations.ComponentHash]
-			if objectHash != "" {
-				return objectHash != ch, nil
-			}
-
 			return a.predicateFn(rr, obj)
 		},
 	)

diff --git a/pkg/controller/actions/gc/action_gc_support.go b/pkg/controller/actions/gc/action_gc_support.go
@@ -18,9 +18,10 @@ func DefaultPredicate(rr *odhTypes.ReconciliationRequest, obj unstructured.Unstr
 
 	pv := resources.GetAnnotation(&obj, odhAnnotations.PlatformVersion)
 	pt := resources.GetAnnotation(&obj, odhAnnotations.PlatformType)
-	cg := resources.GetAnnotation(&obj, odhAnnotations.ComponentGeneration)
+	ig := resources.GetAnnotation(&obj, odhAnnotations.InstanceGeneration)
+	iu := resources.GetAnnotation(&obj, odhAnnotations.InstanceUID)
 
-	if pv == "" || pt == "" || cg == "" {
+	if pv == "" || pt == "" || ig == "" || iu == "" {
 		return false, nil
 	}
 
@@ -32,7 +33,11 @@ func DefaultPredicate(rr *odhTypes.ReconciliationRequest, obj unstructured.Unstr
 		return true, nil
 	}
 
-	g, err := strconv.Atoi(cg)
+	if iu != string(rr.Instance.GetUID()) {
+		return true, nil
+	}
+
+	g, err := strconv.Atoi(ig)
 	if err != nil {
 		return false, fmt.Errorf("cannot determine generation: %w", err)
 	}