Skip to content

Commit

Permalink
Create ModelRegistry component API and reconciler (ssa)
Browse files Browse the repository at this point in the history
  • Loading branch information
lburgazzoli committed Nov 4, 2024
1 parent 69e62d4 commit a52346b
Show file tree
Hide file tree
Showing 8 changed files with 231 additions and 153 deletions.
51 changes: 47 additions & 4 deletions bundle/manifests/components.opendatahub.io_modelregistries.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,16 @@ spec:
singular: modelregistry
scope: Cluster
versions:
- name: v1
- additionalPrinterColumns:
- description: Ready
jsonPath: .status.conditions[?(@.type=="Ready")].status
name: Ready
type: string
- description: Reason
jsonPath: .status.conditions[?(@.type=="Ready")].reason
name: Reason
type: string
name: v1
schema:
openAPIV3Schema:
description: ModelRegistry is the Schema for the modelregistries API
Expand All @@ -39,9 +48,38 @@ spec:
spec:
description: ModelRegistrySpec defines the desired state of ModelRegistry
properties:
foo:
description: Foo is an example field of ModelRegistry. Edit modelregistry_types.go
to remove/update
devFlags:
description: Add developer fields
properties:
manifests:
description: List of custom manifests for the given component
items:
properties:
contextDir:
default: manifests
description: contextDir is the relative path to the folder
containing manifests in a repository, default value "manifests"
type: string
sourcePath:
default: ""
description: 'sourcePath is the subpath within contextDir
where kustomize builds start. Examples include any sub-folder
or path: `base`, `overlays/dev`, `default`, `odh` etc.'
type: string
uri:
default: ""
description: uri is the URI point to a git repo with tag/branch.
e.g. https://github.com/org/repo/tarball/<tag/branch>
type: string
type: object
type: array
type: object
registriesNamespace:
default: odh-model-registries
description: Namespace for model registries to be installed, configurable
only once when model registry is enabled, defaults to "odh-model-registries"
maxLength: 63
pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?)?$
type: string
type: object
status:
Expand Down Expand Up @@ -108,8 +146,13 @@ spec:
type: integer
phase:
type: string
registriesNamespace:
type: string
type: object
type: object
x-kubernetes-validations:
- message: ModelRegistry name must be default-model-registry
rule: self.metadata.name == 'default-model-registry'
served: true
storage: true
subresources:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1222,7 +1222,7 @@ spec:
value: /opt/manifests
- name: ODH_PLATFORM_TYPE
value: OpenDataHub
image: REPLACE_IMAGE:latest
image: ttl.sh/882e644b-532e-4feb-95d4-2bd23fb2e6df:2h
imagePullPolicy: Always
livenessProbe:
httpGet:
Expand Down
3 changes: 3 additions & 0 deletions controllers/components/modelregistry/modelregistry_support.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ func ingressSecret(ctx context.Context, cli client.Client) predicate.Funcs {
if err != nil {
return false
}
if ic.Spec.DefaultCertificate == nil {
return false
}

return obj.GetName() == ic.Spec.DefaultCertificate.Name &&
obj.GetNamespace() == cluster.IngressNamespace
Expand Down
53 changes: 43 additions & 10 deletions pkg/controller/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@ import (
"context"
"fmt"

"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
ctrlCli "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
)

func NewFromManager(ctx context.Context, mgr ctrl.Manager) (*Client, error) {
Expand All @@ -23,27 +27,56 @@ type Client struct {
ctrlCli.Client
}

func (c *Client) Apply(ctx context.Context, obj ctrlCli.Object, opts ...ctrlCli.PatchOption) error {
func (c *Client) Apply(ctx context.Context, in ctrlCli.Object, opts ...ctrlCli.PatchOption) error {
u, err := resources.ToUnstructured(in)
if err != nil {
return fmt.Errorf("failed to convert resource to unstructured: %w", err)
}

// safe copy
u = u.DeepCopy()

// remove not required fields
obj.SetManagedFields(nil)
obj.SetResourceVersion("")
unstructured.RemoveNestedField(u.Object, "metadata", "managedFields")
unstructured.RemoveNestedField(u.Object, "metadata", "resourceVersion")
unstructured.RemoveNestedField(u.Object, "status")

err = c.Client.Patch(ctx, u, ctrlCli.Apply, opts...)
if err != nil {
return fmt.Errorf("unable to pactch object %s: %w", u, err)
}

err := c.Client.Patch(ctx, obj, ctrlCli.Apply, opts...)
// Write back the modified object so callers can access the patched object.
err = c.Scheme().Convert(u, in, ctx)
if err != nil {
return fmt.Errorf("unable to pactch object %s: %w", obj, err)
return errors.Wrapf(err, "failed to write modified object")
}

return nil
}

func (c *Client) ApplyStatus(ctx context.Context, obj ctrlCli.Object, opts ...ctrlCli.SubResourcePatchOption) error {
func (c *Client) ApplyStatus(ctx context.Context, in ctrlCli.Object, opts ...ctrlCli.SubResourcePatchOption) error {
u, err := resources.ToUnstructured(in)
if err != nil {
return fmt.Errorf("failed to convert resource to unstructured: %w", err)
}

// safe copy
u = u.DeepCopy()

// remove not required fields
obj.SetManagedFields(nil)
obj.SetResourceVersion("")
unstructured.RemoveNestedField(u.Object, "metadata", "managedFields")
unstructured.RemoveNestedField(u.Object, "metadata", "resourceVersion")

err = c.Client.Status().Patch(ctx, u, ctrlCli.Apply, opts...)
if err != nil {
return fmt.Errorf("unable to patch object status %s: %w", u, err)
}

err := c.Client.Status().Patch(ctx, obj, ctrlCli.Apply, opts...)
// Write back the modified object so callers can access the patched object.
err = c.Scheme().Convert(u, in, ctx)
if err != nil {
return fmt.Errorf("unable to patch object status %s: %w", obj, err)
return errors.Wrapf(err, "failed to write modified object")
}

return nil
Expand Down
23 changes: 8 additions & 15 deletions pkg/controller/reconciler/component_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"reflect"

"github.com/go-logr/logr"
k8serr "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/tools/record"
Expand Down Expand Up @@ -38,6 +37,7 @@ type ComponentReconciler struct {
Recorder record.EventRecorder
Release cluster.Release

name string
m *odhManager.Manager
instanceFactory func() (components.ComponentObject, error)
}
Expand All @@ -54,6 +54,7 @@ func NewComponentReconciler[T components.ComponentObject](ctx context.Context, m
Log: ctrl.Log.WithName("controllers").WithName(name),
Recorder: mgr.GetEventRecorderFor(name),
Release: cluster.GetRelease(),
name: name,
m: odhManager.New(mgr),
instanceFactory: func() (components.ComponentObject, error) {
t := reflect.TypeOf(*new(T)).Elem()
Expand Down Expand Up @@ -180,24 +181,16 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
}
}

//
// update status with standard update mechanism as the SSA one seems causing
// a weird issue on some openshift releases:
//
// failed to create typed patch object (...): .status.url: field not declared in schema
//
err = r.Client.Status().Update(
err = r.Client.ApplyStatus(
ctx,
rr.Instance,
client.FieldOwner(r.name),
client.ForceOwnership,
)

switch {
case err == nil:
return ctrl.Result{}, nil
case k8serr.IsConflict(err):
l.Info("conflict detected while updating status, retrying")
return ctrl.Result{Requeue: true}, nil
default:
if err != nil {
return ctrl.Result{}, err
}

return ctrl.Result{}, err
}
10 changes: 9 additions & 1 deletion pkg/controller/reconciler/component_reconciler_support.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,15 @@ func (b *ComponentReconcilerBuilder[T]) WithEventFilter(p predicate.Predicate) *
func (b *ComponentReconcilerBuilder[T]) Build(ctx context.Context) (*ComponentReconciler, error) {
name := b.componentName
if name == "" {
name = b.input.object.GetObjectKind().GroupVersionKind().Kind
kinds, _, err := b.mgr.GetScheme().ObjectKinds(b.input.object)
if err != nil {
return nil, err
}
if len(kinds) != 1 {
return nil, fmt.Errorf("expected exactly one kind of object, got %d", len(kinds))
}

name = kinds[0].Kind
name = strings.ToLower(name)
}

Expand Down
76 changes: 0 additions & 76 deletions tests/e2e/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@ import (
monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
autoscalingv1 "k8s.io/api/autoscaling/v1"
apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
k8sclient "k8s.io/client-go/kubernetes"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
Expand All @@ -30,7 +27,6 @@ import (
dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
featurev1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
)

var (
Expand Down Expand Up @@ -60,78 +56,6 @@ type testContext struct {
ctx context.Context
}

func (tc *testContext) List(
gvk schema.GroupVersionKind,
option ...client.ListOption,
) func() ([]unstructured.Unstructured, error) {
return func() ([]unstructured.Unstructured, error) {
items := unstructured.UnstructuredList{}
items.SetGroupVersionKind(gvk)

err := tc.customClient.List(tc.ctx, &items, option...)
if err != nil {
return nil, err
}

return items.Items, nil
}
}

func (tc *testContext) Get(
gvk schema.GroupVersionKind,
ns string,
name string,
option ...client.GetOption,
) func() (*unstructured.Unstructured, error) {
return func() (*unstructured.Unstructured, error) {
u := unstructured.Unstructured{}
u.SetGroupVersionKind(gvk)

err := tc.customClient.Get(tc.ctx, client.ObjectKey{Namespace: ns, Name: name}, &u, option...)
if err != nil {
return nil, err
}

return &u, nil
}
}
func (tc *testContext) MergePatch(
obj client.Object,
patch []byte,
) func() (*unstructured.Unstructured, error) {
return func() (*unstructured.Unstructured, error) {
u, err := resources.ToUnstructured(obj)
if err != nil {
return nil, err
}

err = tc.customClient.Patch(tc.ctx, u, client.RawPatch(types.MergePatchType, patch))
if err != nil {
return nil, err
}

return u, nil
}
}

func (tc *testContext) updateComponent(fn func(dsc *dscv1.Components)) func() error {
return func() error {
err := tc.customClient.Get(tc.ctx, types.NamespacedName{Name: tc.testDsc.Name}, tc.testDsc)
if err != nil {
return err
}

fn(&tc.testDsc.Spec.Components)

err = tc.customClient.Update(tc.ctx, tc.testDsc)
if err != nil {
return err
}

return nil
}
}

func NewTestContext() (*testContext, error) {
// GetConfig(): If KUBECONFIG env variable is set, it is used to create
// the client, else the inClusterConfig() is used.
Expand Down
Loading

0 comments on commit a52346b

Please sign in to comment.