diff --git a/apis/components/v1/modelcontroller_types.go b/apis/components/v1/modelcontroller_types.go index 933d41b5542..e1cc5702f74 100644 --- a/apis/components/v1/modelcontroller_types.go +++ b/apis/components/v1/modelcontroller_types.go @@ -51,8 +51,8 @@ type ModelController struct { // ModelControllerSpec defines the desired state of ModelController type ModelControllerSpec struct { //ModelMeshServing DSCModelMeshServing `json:"modelMeshServing,omitempty"` - Kserve *ModelControllerKerveSpec `json:"kserve,omitempty"` - ModelMeshServing *ModelControllerMMSpec `json:"modelMeshServing,omitempty"` + Kserve *ModelControllerKerveSpec `json:"kserve,omitempty"` + ModelMeshServing *ModelControllerMMSpec `json:"modelMeshServing,omitempty"` } // a mini version of the DSCKserve only keep devflags and management spec @@ -62,7 +62,7 @@ type ModelControllerKerveSpec struct { } // a mini version of the DSCModelMeshServing only keep devflags and management spec -type ModelControllerMMSpec struct { +type ModelControllerMMSpec struct { ManagementState operatorv1.ManagementState `json:"managementState,omitempty"` common.DevFlagsSpec `json:",inline"` } diff --git a/controllers/components/kserve/kserve_controller.go b/controllers/components/kserve/kserve_controller.go index 66b9b04fcb8..da7d838e642 100644 --- a/controllers/components/kserve/kserve_controller.go +++ b/controllers/components/kserve/kserve_controller.go @@ -151,7 +151,6 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. deploy.WithCache(), )). WithAction(security.NewUpdatePodSecurityRoleBindingAction(serviceAccounts)). - WithAction(patchOwnerReference). WithAction(updatestatus.NewAction()). // must be the final action WithAction(gc.NewAction()). diff --git a/controllers/components/kserve/kserve_controller_actions.go b/controllers/components/kserve/kserve_controller_actions.go index 5453011efdf..9bf9130cd47 100644 --- a/controllers/components/kserve/kserve_controller_actions.go +++ b/controllers/components/kserve/kserve_controller_actions.go @@ -9,11 +9,8 @@ import ( operatorv1 "github.com/openshift/api/operator/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - k8serr "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" @@ -261,49 +258,3 @@ func customizeKserveConfigMap(ctx context.Context, rr *odhtypes.ReconciliationRe return nil } - -func patchOwnerReference(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - k, ok := rr.Instance.(*componentsv1.Kserve) - if !ok { - return fmt.Errorf("resource instance %v is not a componentsv1.Kserve)", rr.Instance) - } - - l := logf.FromContext(ctx) - mc := &componentsv1.ModelController{} - if err := rr.Client.Get(ctx, client.ObjectKey{Name: componentsv1.ModelControllerInstanceName}, mc); err != nil { - if k8serr.IsNotFound(err) { - return odherrors.NewStopError("ModelController CR not exist: %v", err) - } - return odherrors.NewStopError("failed to get ModelController CR: %v", err) - } - - owners := []metav1.OwnerReference{} - for _, o := range mc.GetOwnerReferences() { - if o.UID == k.GetUID() { - return nil // kserve already as owner to modelcontroller, early exit - } - if o.Kind != componentsv1.KserveKind { // TODO: a workaround to ensure no old UID exist, this should be moved into finalizer later - owners = append(owners, o) - } - } - owners = append(owners, metav1.OwnerReference{ - Kind: componentsv1.KserveKind, - APIVersion: componentsv1.GroupVersion.String(), - Name: componentsv1.KserveInstanceName, - UID: k.GetUID(), - BlockOwnerDeletion: ptr.To(false), - Controller: ptr.To(false), - }, - ) - mc.SetOwnerReferences(owners) - mc.SetManagedFields(nil) // remove managed fields to avoid conflicts when SSA apply - opt := &client.PatchOptions{ - Force: ptr.To(true), - FieldManager: componentsv1.ModelControllerInstanceName, - } - if err := rr.Client.Patch(ctx, mc, client.Apply, opt); err != nil { - return fmt.Errorf("error update ownerreference for CR %s : %w", mc.GetName(), err) - } - l.Info("Update Ownerreference on kserve change") - return nil -} diff --git a/controllers/components/modelmeshserving/modelmeshserving_actions.go b/controllers/components/modelmeshserving/modelmeshserving_actions.go index aa6063c9c13..c1a276a0e76 100644 --- a/controllers/components/modelmeshserving/modelmeshserving_actions.go +++ b/controllers/components/modelmeshserving/modelmeshserving_actions.go @@ -21,14 +21,7 @@ import ( "fmt" "strings" - k8serr "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" - componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1" - odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" ) @@ -78,55 +71,3 @@ func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { // TODO: Implement devflags logmode logic return nil } - -func patchOwnerReference(ctx context.Context, rr *odhtypes.ReconciliationRequest) error { - mm, ok := rr.Instance.(*componentsv1.ModelMeshServing) - if !ok { - return fmt.Errorf("resource instance %v is not a componentsv1.ModelMeshServing)", rr.Instance) - } - - l := logf.FromContext(ctx) - mc := &componentsv1.ModelController{} - if err := rr.Client.Get(ctx, client.ObjectKey{Name: componentsv1.ModelControllerInstanceName}, mc); err != nil { - if k8serr.IsNotFound(err) { - return odherrors.NewStopError("ModelController CR not exist: %v", err) - } - return odherrors.NewStopError("failed to get ModelController CR: %v", err) - } - l.Info("Get WEN CR", "ModelController", mc) - for _, owners := range mc.GetOwnerReferences() { - if owners.UID == mm.GetUID() { - return nil // modelmesh already as owner to modelcontroller, early exit - } - } - - owners := []metav1.OwnerReference{} - for _, o := range mc.GetOwnerReferences() { - if o.UID == mm.GetUID() { - return nil // same modelmesh already as owner to modelcontroller, early exit - } - if o.Kind != componentsv1.ModelMeshServingKind { // TODO: a workaround to ensure no old UID exist, this should be moved into finalizer later - owners = append(owners, o) - } - } - owners = append(owners, metav1.OwnerReference{ - Kind: componentsv1.ModelMeshServingKind, - APIVersion: componentsv1.GroupVersion.String(), - Name: componentsv1.ModelMeshServingInstanceName, - UID: mm.GetUID(), - BlockOwnerDeletion: ptr.To(false), - Controller: ptr.To(false), - }, - ) - mc.SetOwnerReferences(owners) - mc.SetManagedFields(nil) // remove managed fields to avoid conflicts when SSA apply - opt := &client.PatchOptions{ - Force: ptr.To(true), - FieldManager: componentsv1.ModelControllerInstanceName, - } - if err := rr.Client.Patch(ctx, mc, client.Apply, opt); err != nil { - return fmt.Errorf("error update ownerreference for CR %s : %w", mc.GetName(), err) - } - l.Info("Update Ownerreference on modelmesh change") - return nil -} diff --git a/controllers/components/modelmeshserving/modelmeshserving_controller.go b/controllers/components/modelmeshserving/modelmeshserving_controller.go index b14c28eb058..ff342e518f1 100644 --- a/controllers/components/modelmeshserving/modelmeshserving_controller.go +++ b/controllers/components/modelmeshserving/modelmeshserving_controller.go @@ -78,7 +78,6 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl. WithAction(deploy.NewAction( deploy.WithCache(), )). - WithAction(patchOwnerReference). WithAction(updatestatus.NewAction()). WithAction(gc.NewAction()). Build(ctx) // include GenerationChangedPredicate no need set in each Owns() above diff --git a/tests/e2e/modelcontroller_test.go b/tests/e2e/modelcontroller_test.go index 4519c2726e0..747685255f3 100644 --- a/tests/e2e/modelcontroller_test.go +++ b/tests/e2e/modelcontroller_test.go @@ -126,14 +126,8 @@ func (tc *ModelControllerTestCtx) testModelControllerAvaile() error { } func (tc *ModelControllerTestCtx) testOwnerReferences() error { - // TODO: need to add test case for this later - // can be 1( when just created and no other compoment enabled yet) 2 (only one dependent component) or even 3 (enable both components) - // if len(tc.testModelControllerInstance.OwnerReferences) != 0 { - // return errors.New("expect CR has ownerreferences set") - // } - // Test ModelController CR ownerref - if tc.testModelControllerInstance.OwnerReferences[0].Kind != dscKind { // TODO: we might need to udpate this once we do not set DSC as ownerref + if tc.testModelControllerInstance.OwnerReferences[0].Kind != dscKind { return fmt.Errorf("expected ownerreference DataScienceCluster not found. Got ownereferrence: %v", tc.testModelControllerInstance.OwnerReferences[0].Kind) }