Refactor modelcontroller DevFlags handling (#1435)

- remove the need to lookup DSC
- remove .status.uri to avoid cluttering the API and add debug log
  statement to trace down the selected DevFlags

apis/components/v1alpha1/modelcontroller_types.go

@@ -50,7 +50,7 @@ type ModelController struct {
 
 // ModelControllerSpec defines the desired state of ModelController
 type ModelControllerSpec struct {
-	//ModelMeshServing DSCModelMeshServing `json:"modelMeshServing,omitempty"`
+	// ModelMeshServing DSCModelMeshServing `json:"modelMeshServing,omitempty"`
 	Kserve           *ModelControllerKerveSpec `json:"kserve,omitempty"`
 	ModelMeshServing *ModelControllerMMSpec    `json:"modelMeshServing,omitempty"`
 }
@@ -62,17 +62,23 @@ type ModelControllerKerveSpec struct {
 	common.DevFlagsSpec `json:",inline"`
 }
 
+func (s *ModelControllerKerveSpec) GetDevFlags() *common.DevFlags {
+	return s.DevFlags
+}
+
 // a mini version of the DSCModelMeshServing only keep devflags and management spec
 type ModelControllerMMSpec struct {
 	ManagementState     operatorv1.ManagementState `json:"managementState,omitempty"`
 	common.DevFlagsSpec `json:",inline"`
 }
 
+func (s *ModelControllerMMSpec) GetDevFlags() *common.DevFlags {
+	return s.DevFlags
+}
+
 // ModelControllerStatus defines the observed state of ModelController
 type ModelControllerStatus struct {
 	common.Status `json:",inline"`
-	// devflag's URI
-	URI string `json:"URI,omitempty"`
 }
 
 // +kubebuilder:object:root=true

bundle/manifests/components.platform.opendatahub.io_modelcontrollers.yaml

@@ -139,9 +139,6 @@ spec:
           status:
             description: ModelControllerStatus defines the observed state of ModelController
             properties:
-              URI:
-                description: devflag's URI
-                type: string
               conditions:
                 items:
                   description: Condition contains details for one aspect of the current

config/crd/bases/components.platform.opendatahub.io_modelcontrollers.yaml

@@ -139,9 +139,6 @@ spec:
           status:
             description: ModelControllerStatus defines the observed state of ModelController
             properties:
-              URI:
-                description: devflag's URI
-                type: string
               conditions:
                 items:
                   description: Condition contains details for one aspect of the current

controllers/components/modelcontroller/modelcontroller_actions.go

@@ -18,19 +18,20 @@ package modelcontroller
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"strings"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
+	"github.com/opendatahub-io/opendatahub-operator/v2/apis/common"
 	componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
-	dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
 	odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
 	odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
 )
 
-func initialize(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
+func initialize(_ context.Context, rr *odhtypes.ReconciliationRequest) error {
 	// early exist
 	mc, ok := rr.Instance.(*componentApi.ModelController)
 	if !ok {
@@ -62,52 +63,43 @@ func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
 	if !ok {
 		return fmt.Errorf("resource instance %v is not a componentApi.ModelController)", rr.Instance)
 	}
-	// since we do not initialize the rr with DSC CR any more, add this into function
-	dscl := dscv1.DataScienceClusterList{}
-	if err := rr.Client.List(ctx, &dscl); err != nil {
-		return err
-	}
-	if len(dscl.Items) != 1 {
-		return errors.New("unable to find DataScienceCluster CR")
+
+	l := logf.FromContext(ctx)
+
+	var df *common.DevFlags
+
+	ks := mc.Spec.Kserve
+	ms := mc.Spec.ModelMeshServing
+
+	switch {
+	case ks != nil && ks.ManagementState == operatorv1.Managed && resources.HasDevFlags(ks):
+		l.V(3).Info("Using DevFlags from KServe")
+		df = ks.GetDevFlags()
+	case ms != nil && ms.ManagementState == operatorv1.Managed && resources.HasDevFlags(ms):
+		l.V(3).Info("Using DevFlags from ModelMesh")
+		df = ms.GetDevFlags()
+	default:
+		return nil
 	}
-	// Get Kserve which can override Kserve devflags
-	k := &dscl.Items[0].Spec.Components.Kserve
-	if k.ManagementSpec.ManagementState != operatorv1.Managed || k.DevFlags == nil || len(k.DevFlags.Manifests) == 0 {
-		// Get ModelMeshServing if it is enabled and has devlfags
-		mm := &dscl.Items[0].Spec.Components.ModelMeshServing
-		if mm.ManagementSpec.ManagementState != operatorv1.Managed || mm.DevFlags == nil || len(mm.DevFlags.Manifests) == 0 {
-			// no need devflag, no need update status.uri
-			return nil
+
+	for _, subcomponent := range df.Manifests {
+		if !strings.Contains(subcomponent.URI, ComponentName) {
+			continue
 		}
 
-		for _, subcomponent := range mc.Spec.ModelMeshServing.DevFlags.Manifests {
-			if strings.Contains(subcomponent.URI, ComponentName) {
-				// update .status.uri and download odh-model-controller
-				mc.Status.URI = subcomponent.URI
-				if err := odhdeploy.DownloadManifests(ctx, ComponentName, subcomponent); err != nil {
-					return err
-				}
-				// If overlay is defined, update paths
-				if subcomponent.SourcePath != "" {
-					rr.Manifests[0].SourcePath = subcomponent.SourcePath
-				}
-			}
+		l.V(3).Info("Downloading manifests", "uri", subcomponent.URI)
+
+		if err := odhdeploy.DownloadManifests(ctx, ComponentName, subcomponent); err != nil {
+			return err
 		}
-		return nil
-	}
 
-	for _, subcomponent := range mc.Spec.Kserve.DevFlags.Manifests {
-		if strings.Contains(subcomponent.URI, ComponentName) {
-			// update .status.uri and download odh-model-controller
-			mc.Status.URI = subcomponent.URI
-			if err := odhdeploy.DownloadManifests(ctx, ComponentName, subcomponent); err != nil {
-				return err
-			}
-			// If overlay is defined, update paths
-			if subcomponent.SourcePath != "" {
-				rr.Manifests[0].SourcePath = subcomponent.SourcePath
-			}
+		// If overlay is defined, update paths
+		if subcomponent.SourcePath != "" {
+			rr.Manifests[0].SourcePath = subcomponent.SourcePath
 		}
+
+		break
 	}
+
 	return nil
 }

docs/api-overview.md

@@ -849,7 +849,6 @@ _Appears in:_
 | `phase` _string_ |  |  |  |
 | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#condition-v1-meta) array_ |  |  |  |
 | `observedGeneration` _integer_ |  |  |  |
-| `URI` _string_ | devflag's URI |  |  |
 
 
 #### ModelMeshServing

pkg/resources/resources.go

@@ -18,6 +18,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
+
+	"github.com/opendatahub-io/opendatahub-operator/v2/apis/common"
 )
 
 func ToUnstructured(obj any) (*unstructured.Unstructured, error) {
@@ -298,3 +300,13 @@ func EnsureGroupVersionKind(s *runtime.Scheme, obj client.Object) error {
 
 	return nil
 }
+
+func HasDevFlags(in common.WithDevFlags) bool {
+	if in == nil {
+		return false
+	}
+
+	df := in.GetDevFlags()
+
+	return df != nil && len(df.Manifests) != 0
+}