Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor modelcontroller DevFlags handling #1435

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions apis/components/v1alpha1/modelcontroller_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
78 changes: 35 additions & 43 deletions controllers/components/modelcontroller/modelcontroller_actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
1 change: 0 additions & 1 deletion docs/api-overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions pkg/resources/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
"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) {
Expand Down Expand Up @@ -298,3 +300,13 @@

return nil
}

func HasDevFlags(in common.WithDevFlags) bool {
if in == nil {
return false
}

Check warning on line 307 in pkg/resources/resources.go

View check run for this annotation

Codecov / codecov/patch

pkg/resources/resources.go#L304-L307

Added lines #L304 - L307 were not covered by tests

df := in.GetDevFlags()

return df != nil && len(df.Manifests) != 0

Check warning on line 311 in pkg/resources/resources.go

View check run for this annotation

Codecov / codecov/patch

pkg/resources/resources.go#L309-L311

Added lines #L309 - L311 were not covered by tests
}
Loading