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.

Sign up for GitHub

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

WIP: Remove FeatureTracker code from Kserve reconciler #1521

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

WIP: Remove FeatureTracker code from Kserve reconciler #1521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e8bb43
Add tests around FTs in Kserve before removal
grdryn Feb 25, 2025
17b7a7d
Remove FeatureTracker code from Kserve reconciler
grdryn Jan 20, 2025
d35d619
Deduplicate KnativeServing template in Kserve
grdryn Feb 19, 2025
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
58 changes: 16 additions & 42 deletions controllers/components/kserve/kserve_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@ import (

componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
featuresv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/template"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/clusterrole"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/generation"
Expand All @@ -53,8 +52,6 @@ import (

// NewComponentReconciler creates a ComponentReconciler for the Dashboard API.
func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.Manager) error {
ownedViaFTMapFunc := ownedViaFT(mgr.GetClient())

_, err := reconciler.ReconcilerFor(mgr, &componentApi.Kserve{}).
// operands - owned
Owns(&corev1.Secret{}).
Expand All @@ -69,12 +66,20 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
// changes. The compareHashPredicate ensures that we don't needlessly enqueue
// requests if there are no changes that we don't care about.
Owns(&templatev1.Template{}, reconciler.WithPredicates(hash.Updated())).
Owns(&featuresv1.FeatureTracker{}).
Owns(&networkingv1.NetworkPolicy{}).
Owns(&monitoringv1.ServiceMonitor{}).
Owns(&admissionregistrationv1.MutatingWebhookConfiguration{}).
Owns(&admissionregistrationv1.ValidatingWebhookConfiguration{}).
Owns(&appsv1.Deployment{}, reconciler.WithPredicates(resources.NewDeploymentPredicate())).

// operands - dynamically owned
OwnsGVK(gvk.Gateway, reconciler.Dynamic(ifGVKInstalled(gvk.Gateway))).
OwnsGVK(gvk.EnvoyFilter, reconciler.Dynamic(ifGVKInstalled(gvk.EnvoyFilter))).
OwnsGVK(gvk.KnativeServing, reconciler.Dynamic(ifGVKInstalled(gvk.KnativeServing))).
OwnsGVK(gvk.ServiceMeshMember, reconciler.Dynamic(ifGVKInstalled(gvk.ServiceMeshMember))).
OwnsGVK(gvk.AuthorizationPolicy, reconciler.Dynamic(ifGVKInstalled(gvk.AuthorizationPolicy))).
OwnsGVK(gvk.AuthorizationPolicyv1beta1, reconciler.Dynamic(ifGVKInstalled(gvk.AuthorizationPolicyv1beta1))).

// operands - watched
//
// By default the Watches functions adds:
Expand Down Expand Up @@ -111,51 +116,20 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
reconciler.WithEventHandler(handlers.ToNamed(componentApi.KserveInstanceName)),
reconciler.WithPredicates(predicate.Or(generation.New(), resources.DSCIReadiness)),
).
// operands - dynamically watched
//
// A watch will be created dynamically for these kinds, if they exist on the cluster
// (they come from ServiceMesh and Serverless operators).
//
// They're owned by FeatureTrackers, which are owned by a Kserve; so there's a
// custom event mapper to enqueue a reconcile request for a Kserve object, if
// applicable.
//
// They also don't have the "partOf" label that Watches expects in the
// implicit predicate, so the simpler "DefaultPredicate" is also added.
WatchesGVK(
gvk.KnativeServing,
reconciler.Dynamic(ifGVKInstalled(gvk.KnativeServing)),
reconciler.WithEventMapper(ownedViaFTMapFunc),
reconciler.WithPredicates(predicates.DefaultPredicate)).
WatchesGVK(
gvk.ServiceMeshMember,
reconciler.Dynamic(ifGVKInstalled(gvk.ServiceMeshMember)),
reconciler.WithEventMapper(ownedViaFTMapFunc),
reconciler.WithPredicates(predicates.DefaultPredicate)).
WatchesGVK(
gvk.EnvoyFilter,
reconciler.Dynamic(ifGVKInstalled(gvk.EnvoyFilter)),
reconciler.WithEventMapper(ownedViaFTMapFunc),
reconciler.WithPredicates(predicates.DefaultPredicate)).
WatchesGVK(
gvk.AuthorizationPolicy,
reconciler.Dynamic(ifGVKInstalled(gvk.AuthorizationPolicy)),
reconciler.WithEventMapper(ownedViaFTMapFunc),
reconciler.WithPredicates(predicates.DefaultPredicate)).
WatchesGVK(
gvk.Gateway,
reconciler.Dynamic(ifGVKInstalled(gvk.Gateway)),
reconciler.WithEventMapper(ownedViaFTMapFunc),
reconciler.WithPredicates(predicates.DefaultPredicate)).

// actions
WithAction(checkPreConditions).
WithAction(initialize).
WithAction(devFlags).
WithAction(releases.NewAction()).
WithAction(removeLegacyFeatureTrackerOwnerRef).
WithAction(configureServerless).
WithAction(configureServiceMesh).
WithAction(template.NewAction(
template.WithCache(),
template.WithDataFn(getTemplateData),
)).
WithAction(rmFTOwnerRefs).
WithAction(deleteFeatureTrackers).
WithAction(kustomize.NewAction(
kustomize.WithCache(),
// These are the default labels added by the legacy deploy method
Expand Down
127 changes: 100 additions & 27 deletions controllers/components/kserve/kserve_controller_actions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"slices"
"strings"

operatorv1 "github.com/openshift/api/operator/v1"
Expand All @@ -24,7 +25,6 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions"
odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
)
Expand Down Expand Up @@ -142,32 +142,59 @@ func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
return nil
}

func removeLegacyFeatureTrackerOwnerRef(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
func deleteFeatureTrackers(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
ftNames := []string{
rr.DSCI.Spec.ApplicationsNamespace + "-serverless-serving-deployment",
rr.DSCI.Spec.ApplicationsNamespace + "-serverless-net-istio-secret-filtering",
rr.DSCI.Spec.ApplicationsNamespace + "-serverless-serving-gateways",
rr.DSCI.Spec.ApplicationsNamespace + "-kserve-external-authz",
}

for _, ftName := range ftNames {
obj := &featuresv1.FeatureTracker{}
err := rr.Client.Get(ctx, client.ObjectKey{Name: ftName}, obj)
switch {
case k8serr.IsNotFound(err):
for _, n := range ftNames {
ft := featuresv1.FeatureTracker{}
err := rr.Client.Get(ctx, client.ObjectKey{Name: n}, &ft)
if k8serr.IsNotFound(err) {
continue
case err != nil:
return fmt.Errorf("error while retrieving FeatureTracker %s: %w", ftName, err)
} else if err != nil {
return fmt.Errorf("failed to lookup FeatureTracker %s: %w", ft.GetName(), err)
}

if err := resources.RemoveOwnerReferences(ctx, rr.Client, obj, isLegacyOwnerRef); err != nil {
return err
err = rr.Client.Delete(ctx, &ft, client.PropagationPolicy(metav1.DeletePropagationForeground))
if k8serr.IsNotFound(err) {
continue
} else if err != nil {
return fmt.Errorf("failed to delete FeatureTracker %s: %w", ft.GetName(), err)
}
}

return nil
}

func rmFTOwnerRefs(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
hasFTOwner := func(or metav1.OwnerReference) bool { return or.Kind == gvk.FeatureTracker.Kind }

for _, res := range rr.Resources {
current := resources.GvkToUnstructured(res.GroupVersionKind())

lookupErr := rr.Client.Get(ctx, client.ObjectKeyFromObject(&res), current)
switch {
case k8serr.IsNotFound(lookupErr):
continue
case lookupErr != nil:
return fmt.Errorf("failed to lookup object %s/%s: %w", res.GetNamespace(), res.GetName(), lookupErr)
default:
ors := slices.DeleteFunc(current.GetOwnerReferences(), hasFTOwner)

if len(ors) < len(current.GetOwnerReferences()) {
if err := resources.RemoveOwnerReferences(ctx, rr.Client, current, hasFTOwner); err != nil {
return fmt.Errorf("failed to remove FeatureTracker owner: %w", err)
}
}
}
}
return nil
}

func configureServerless(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
k, ok := rr.Instance.(*componentApi.Kserve)
if !ok {
Expand All @@ -183,9 +210,6 @@ func configureServerless(ctx context.Context, rr *odhtypes.ReconciliationRequest

case operatorv1.Removed: // we remove serving CR
logger.Info("existing Serverless CR (owned by operator) will be removed")
if err := removeServerlessFeatures(ctx, rr.Client, k, &rr.DSCI.Spec); err != nil {
return err
}

case operatorv1.Managed: // standard workflow to create CR
if rr.DSCI.Spec.ServiceMesh == nil {
Expand All @@ -199,34 +223,83 @@ func configureServerless(ctx context.Context, rr *odhtypes.ReconciliationRequest
"as it is required by the KServe serving field", rr.DSCI.Spec.ServiceMesh.ManagementState)
}

serverlessFeatures := feature.ComponentFeaturesHandler(rr.Instance, componentName, rr.DSCI.Spec.ApplicationsNamespace, configureServerlessFeatures(&rr.DSCI.Spec, k))
err := createServingCertResource(ctx, cli, &rr.DSCI.Spec, k)
if err != nil {
return fmt.Errorf("unable to create serverless serving certificate secret: %w", err)
}

if err := serverlessFeatures.Apply(ctx, cli); err != nil {
return err
templates := []odhtypes.TemplateInfo{
{
FS: resourcesFS,
Path: "resources/serving-install/service-mesh-subscription.tmpl.yaml",
},
{
FS: resourcesFS,
Path: "resources/serving-install/knative-serving.tmpl.yaml",
},
{
FS: resourcesFS,
Path: "resources/servicemesh/routing/istio-ingress-gateway.tmpl.yaml",
},
{
FS: resourcesFS,
Path: "resources/servicemesh/routing/istio-kserve-local-gateway.tmpl.yaml",
},
{
FS: resourcesFS,
Path: "resources/servicemesh/routing/istio-local-gateway.yaml",
},
{
FS: resourcesFS,
Path: "resources/servicemesh/routing/kserve-local-gateway-svc.tmpl.yaml",
},
{
FS: resourcesFS,
Path: "resources/servicemesh/routing/local-gateway-svc.tmpl.yaml",
},
}

rr.Templates = append(rr.Templates, templates...)
}
return nil
}

func configureServiceMesh(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
k, ok := rr.Instance.(*componentApi.Kserve)
if !ok {
return fmt.Errorf("resource instance %v is not a componentApi.Kserve)", rr.Instance)
}

cli := rr.Client

if rr.DSCI.Spec.ServiceMesh != nil {
if rr.DSCI.Spec.ServiceMesh.ManagementState == operatorv1.Managed {
serviceMeshInitializer := feature.ComponentFeaturesHandler(k, componentName, rr.DSCI.Spec.ApplicationsNamespace, defineServiceMeshFeatures(ctx, cli, &rr.DSCI.Spec))
return serviceMeshInitializer.Apply(ctx, cli)
templates := []odhtypes.TemplateInfo{
{
FS: resourcesFS,
Path: "resources/servicemesh/activator-envoyfilter.tmpl.yaml",
},
{
FS: resourcesFS,
Path: "resources/servicemesh/envoy-oauth-temp-fix.tmpl.yaml",
},
{
FS: resourcesFS,
Path: "resources/servicemesh/kserve-predictor-authorizationpolicy.tmpl.yaml",
},
{
FS: resourcesFS,
Path: "resources/servicemesh/kserve-inferencegraph-envoyfilter.tmpl.yaml",
},
{
FS: resourcesFS,
Path: "resources/servicemesh/kserve-inferencegraph-authorizationpolicy.tmpl.yaml",
},
}

rr.Templates = append(rr.Templates, templates...)

return nil
}
if rr.DSCI.Spec.ServiceMesh.ManagementState == operatorv1.Unmanaged {
return nil
}
}

return removeServiceMeshConfigurations(ctx, cli, k, &rr.DSCI.Spec)
return nil
}

func customizeKserveConfigMap(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
Expand Down
Loading