update: update GetManagementState return type and logic

- change return as string, used by NewCRObject() - move logic check on Managed, Removed, not set component and others into GetManagementState() Signed-off-by: Wen Zhou <[email protected]>
opendatahub-io · Nov 26, 2024 · 2fbf3bd · 2fbf3bd
1 parent 1ea21ad
commit 2fbf3bd
Show file tree

Hide file tree

Showing 9 changed files with 79 additions and 68 deletions.
diff --git a/controllers/components/dashboard/dashboard.go b/controllers/components/dashboard/dashboard.go
@@ -25,8 +25,16 @@ func (s *componentHandler) GetName() string {
 	return componentsv1.DashboardComponentName
 }
 
-func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) operatorv1.ManagementState {
-	return dsc.Spec.Components.Dashboard.ManagementState
+func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) string {
+	if s == nil || dsc.Spec.Components.Dashboard.ManagementState == operatorv1.Removed {
+		return string(operatorv1.Removed)
+	}
+	switch dsc.Spec.Components.Dashboard.ManagementState {
+	case operatorv1.Managed:
+		return string(dsc.Spec.Components.Dashboard.ManagementState)
+	default: // Force and Unmanaged case for unknown values, we do not support these yet
+		return "Unknown"
+	}
 }
 
 func (s *componentHandler) Init(platform cluster.Platform) error {
@@ -41,13 +49,7 @@ func (s *componentHandler) Init(platform cluster.Platform) error {
 
 func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) client.Object {
 	dashboardAnnotations := make(map[string]string)
-
-	switch dsc.Spec.Components.Dashboard.ManagementState {
-	case operatorv1.Managed, operatorv1.Removed:
-		dashboardAnnotations[annotations.ManagementStateAnnotation] = string(dsc.Spec.Components.Dashboard.ManagementState)
-	default: // Force and Unmanaged case for unknown values, we do not support these yet
-		dashboardAnnotations[annotations.ManagementStateAnnotation] = "Unknown"
-	}
+	dashboardAnnotations[annotations.ManagementStateAnnotation] = s.GetManagementState(dsc)
 
 	return client.Object(&componentsv1.Dashboard{
 		TypeMeta: metav1.TypeMeta{

diff --git a/controllers/components/datasciencepipelines/datasciencepipelines.go b/controllers/components/datasciencepipelines/datasciencepipelines.go
@@ -29,8 +29,16 @@ func (s *componentHandler) GetName() string {
 	return componentsv1.DashboardComponentName
 }
 
-func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) operatorv1.ManagementState {
-	return dsc.Spec.Components.Dashboard.ManagementState
+func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) string {
+	if s == nil || dsc.Spec.Components.Dashboard.ManagementState == operatorv1.Removed {
+		return string(operatorv1.Removed)
+	}
+	switch dsc.Spec.Components.Dashboard.ManagementState {
+	case operatorv1.Managed:
+		return string(dsc.Spec.Components.Dashboard.ManagementState)
+	default: // Force and Unmanaged case for unknown values, we do not support these yet
+		return "Unknown"
+	}
 }
 
 func (s *componentHandler) Init(platform cluster.Platform) error {
@@ -62,13 +70,7 @@ func (s *componentHandler) Init(platform cluster.Platform) error {
 
 func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) client.Object {
 	dataSciencePipelinesAnnotations := make(map[string]string)
-
-	switch dsc.Spec.Components.DataSciencePipelines.ManagementState {
-	case operatorv1.Managed, operatorv1.Removed:
-		dataSciencePipelinesAnnotations[annotations.ManagementStateAnnotation] = string(dsc.Spec.Components.DataSciencePipelines.ManagementState)
-	default: // Force and Unmanaged case for unknown values, we do not support these yet
-		dataSciencePipelinesAnnotations[annotations.ManagementStateAnnotation] = "Unknown"
-	}
+	dataSciencePipelinesAnnotations[annotations.ManagementStateAnnotation] = s.GetManagementState(dsc)
 
 	return client.Object(&componentsv1.DataSciencePipelines{
 		TypeMeta: metav1.TypeMeta{

diff --git a/controllers/components/kueue/kueue.go b/controllers/components/kueue/kueue.go
@@ -33,19 +33,21 @@ func (s *componentHandler) GetName() string {
 	return componentsv1.KueueComponentName
 }
 
-func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) operatorv1.ManagementState {
-	return dsc.Spec.Components.Kueue.ManagementState
-}
-
-func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) client.Object {
-	kueueAnnotations := make(map[string]string)
+func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) string {
+	if s == nil || dsc.Spec.Components.Kueue.ManagementState == operatorv1.Removed {
+		return string(operatorv1.Removed)
+	}
 	switch dsc.Spec.Components.Kueue.ManagementState {
-	case operatorv1.Managed, operatorv1.Removed:
-		kueueAnnotations[annotations.ManagementStateAnnotation] = string(dsc.Spec.Components.Kueue.ManagementState)
+	case operatorv1.Managed:
+		return string(dsc.Spec.Components.Kueue.ManagementState)
 	default: // Force and Unmanaged case for unknown values, we do not support these yet
-		kueueAnnotations[annotations.ManagementStateAnnotation] = "Unknown"
+		return "Unknown"
 	}
+}
 
+func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) client.Object {
+	kueueAnnotations := make(map[string]string)
+	kueueAnnotations[annotations.ManagementStateAnnotation] = s.GetManagementState(dsc)
 	return client.Object(&componentsv1.Kueue{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       componentsv1.KueueKind,

diff --git a/controllers/components/modelregistry/modelregistry.go b/controllers/components/modelregistry/modelregistry.go
@@ -25,8 +25,16 @@ func (s *componentHandler) GetName() string {
 	return componentsv1.ModelRegistryComponentName
 }
 
-func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) operatorv1.ManagementState {
-	return dsc.Spec.Components.ModelRegistry.ManagementState
+func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) string {
+	if s == nil || dsc.Spec.Components.ModelRegistry.ManagementState == operatorv1.Removed {
+		return string(operatorv1.Removed)
+	}
+	switch dsc.Spec.Components.ModelRegistry.ManagementState {
+	case operatorv1.Managed:
+		return string(dsc.Spec.Components.ModelRegistry.ManagementState)
+	default: // Force and Unmanaged case for unknown values, we do not support these yet
+		return "Unknown"
+	}
 }
 
 func (s *componentHandler) Init(_ cluster.Platform) error {
@@ -49,15 +57,7 @@ func (s *componentHandler) Init(_ cluster.Platform) error {
 
 func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) client.Object {
 	componentAnnotations := make(map[string]string)
-
-	switch dsc.Spec.Components.ModelRegistry.ManagementState {
-	case operatorv1.Managed, operatorv1.Removed:
-		componentAnnotations[annotations.ManagementStateAnnotation] = string(dsc.Spec.Components.ModelRegistry.ManagementState)
-	default:
-		// Force and Unmanaged case for unknown values, we do not support these yet
-		componentAnnotations[annotations.ManagementStateAnnotation] = "Unknown"
-	}
-
+	componentAnnotations[annotations.ManagementStateAnnotation] = s.GetManagementState(dsc)
 	return client.Object(&componentsv1.ModelRegistry{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       componentsv1.ModelRegistryKind,

diff --git a/controllers/components/ray/ray.go b/controllers/components/ray/ray.go
@@ -33,18 +33,20 @@ func (s *componentHandler) GetName() string {
 	return componentsv1.RayComponentName
 }
 
-func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) operatorv1.ManagementState {
-	return dsc.Spec.Components.Ray.ManagementState
-}
-
-func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) client.Object {
-	rayAnnotations := make(map[string]string)
+func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) string {
+	if s == nil || dsc.Spec.Components.Ray.ManagementState == operatorv1.Removed {
+		return string(operatorv1.Removed)
+	}
 	switch dsc.Spec.Components.Ray.ManagementState {
-	case operatorv1.Managed, operatorv1.Removed:
-		rayAnnotations[annotations.ManagementStateAnnotation] = string(dsc.Spec.Components.Ray.ManagementState)
+	case operatorv1.Managed:
+		return string(dsc.Spec.Components.Ray.ManagementState)
 	default: // Force and Unmanaged case for unknown values, we do not support these yet
-		rayAnnotations[annotations.ManagementStateAnnotation] = "Unknown"
+		return "Unknown"
 	}
+}
+func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) client.Object {
+	rayAnnotations := make(map[string]string)
+	rayAnnotations[annotations.ManagementStateAnnotation] = s.GetManagementState(dsc)
 
 	return client.Object(&componentsv1.Ray{
 		TypeMeta: metav1.TypeMeta{

diff --git a/controllers/components/trainingoperator/trainingoperator.go b/controllers/components/trainingoperator/trainingoperator.go
@@ -33,18 +33,20 @@ func (s *componentHandler) GetName() string {
 	return componentsv1.TrainingOperatorComponentName
 }
 
-func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) operatorv1.ManagementState {
-	return dsc.Spec.Components.TrainingOperator.ManagementState
-}
-
-func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) k8sclient.Object {
-	trainingoperatorAnnotations := make(map[string]string)
+func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) string {
+	if s == nil || dsc.Spec.Components.TrainingOperator.ManagementState == operatorv1.Removed {
+		return string(operatorv1.Removed)
+	}
 	switch dsc.Spec.Components.TrainingOperator.ManagementState {
-	case operatorv1.Managed, operatorv1.Removed:
-		trainingoperatorAnnotations[annotations.ManagementStateAnnotation] = string(dsc.Spec.Components.TrainingOperator.ManagementState)
+	case operatorv1.Managed:
+		return string(dsc.Spec.Components.TrainingOperator.ManagementState)
 	default: // Force and Unmanaged case for unknown values, we do not support these yet
-		trainingoperatorAnnotations[annotations.ManagementStateAnnotation] = "Unknown"
+		return "Unknown"
 	}
+}
+func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) k8sclient.Object {
+	trainingoperatorAnnotations := make(map[string]string)
+	trainingoperatorAnnotations[annotations.ManagementStateAnnotation] = s.GetManagementState(dsc)
 
 	return k8sclient.Object(&componentsv1.TrainingOperator{
 		TypeMeta: metav1.TypeMeta{

diff --git a/controllers/components/trustyai/trustyai.go b/controllers/components/trustyai/trustyai.go
@@ -39,19 +39,21 @@ func (s *componentHandler) GetName() string {
 	return componentsv1.TrustyAIComponentName
 }
 
-func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) operatorv1.ManagementState {
-	return dsc.Spec.Components.TrustyAI.ManagementState
-}
-
-func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) client.Object {
-	trustyaiAnnotations := make(map[string]string)
+func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) string {
+	if s == nil || dsc.Spec.Components.TrustyAI.ManagementState == operatorv1.Removed {
+		return string(operatorv1.Removed)
+	}
 	switch dsc.Spec.Components.TrustyAI.ManagementState {
-	case operatorv1.Managed, operatorv1.Removed:
-		trustyaiAnnotations[annotations.ManagementStateAnnotation] = string(dsc.Spec.Components.TrustyAI.ManagementState)
+	case operatorv1.Managed:
+		return string(dsc.Spec.Components.TrustyAI.ManagementState)
 	default: // Force and Unmanaged case for unknown values, we do not support these yet
-		trustyaiAnnotations[annotations.ManagementStateAnnotation] = "Unknown"
+		return "Unknown"
 	}
+}
 
+func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) client.Object {
+	trustyaiAnnotations := make(map[string]string)
+	trustyaiAnnotations[annotations.ManagementStateAnnotation] = s.GetManagementState(dsc)
 	return client.Object(&componentsv1.TrustyAI{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       componentsv1.TrustyAIKind,

diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go
@@ -288,7 +288,7 @@ func (r *DataScienceClusterReconciler) ReconcileComponent(
 		return instance, err
 	}
 
-	enabled := component.GetManagementState(instance) == operatorv1.Managed
+	enabled := component.GetManagementState(instance) == string(operatorv1.Managed)
 
 	// TODO: check component status before update DSC status to successful .GetStatus().Phase == "Ready"
 	r.Log.Info("component reconciled successfully: " + componentName)

diff --git a/pkg/componentsregistry/componentsregistry.go b/pkg/componentsregistry/componentsregistry.go
@@ -6,7 +6,6 @@ import (
 	"context"
 
 	"github.com/hashicorp/go-multierror"
-	operatorv1 "github.com/openshift/api/operator/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -21,7 +20,7 @@ type ComponentHandler interface {
 	// GetName and GetManagementState sound like pretty much the same across
 	// all components, but I could not find a way to avoid it
 	GetName() string
-	GetManagementState(dsc *dscv1.DataScienceCluster) operatorv1.ManagementState
+	GetManagementState(dsc *dscv1.DataScienceCluster) string
 	// NewCRObject constructs components specific Custom Resource
 	// e.g. Dashboard in datasciencecluster.opendatahub.io group
 	// It returns interface, but it simplifies DSC reconciler code a lot