Skip to content

Commit

Permalink
revert: patchownerreference()
Browse files Browse the repository at this point in the history
- till we know more what we want to do, we keep modelcontroller ownereference to only DSC

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
  • Loading branch information
zdtsw committed Dec 3, 2024
1 parent 225dfe5 commit b6958de
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 120 deletions.
6 changes: 3 additions & 3 deletions apis/components/v1/modelcontroller_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"`
}
Expand Down
1 change: 0 additions & 1 deletion controllers/components/kserve/kserve_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()).
Expand Down
49 changes: 0 additions & 49 deletions controllers/components/kserve/kserve_controller_actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 1 addition & 7 deletions tests/e2e/modelcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit b6958de

Please sign in to comment.