Skip to content

Commit

Permalink
fix: handle stale specDefaults
Browse files Browse the repository at this point in the history
Signed-off-by: Dhiraj Bokde <[email protected]>
  • Loading branch information
dhirajsb committed Nov 8, 2024
1 parent 223a739 commit 00d0683
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 6 deletions.
30 changes: 25 additions & 5 deletions api/v1alpha1/modelregistry_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,15 @@ func (r *ModelRegistry) Default() {
// and it removes default runtime values that match default runtime properties set in the operator
// since they are redundant as custom property values.
func (r *ModelRegistry) cleanupRuntimeDefaults() {
// if specDefaults hasn't been set for new MRs or all properties were set in a previous version
if r.Status.SpecDefaults != "" && r.Status.SpecDefaults != "{}" {
// nothing to do as model registries with specDefaults set
// will only have custom properties set by users
return
// also check if any runtime default properties will be set in the next reconcile
// this is required because specDefaults could be stale
deepCopy := r.DeepCopy() // do not modify this registry
if deepCopy.RuntimeDefaults() {
// model registry has custom values set for runtime properties
return
}
}

// check grpc image against operator default grpc image repo
Expand Down Expand Up @@ -208,58 +213,73 @@ func (r *ModelRegistry) cleanupRuntimeDefaults() {
}
}

// RuntimeDefaults sets default values from the operator environment, which could change at runtime
func (r *ModelRegistry) RuntimeDefaults() {
// RuntimeDefaults sets default values from the operator environment, which could change at runtime.
// Returns true if any properties were set to a default value, false if all default values are overridden.
func (r *ModelRegistry) RuntimeDefaults() bool {
modelregistrylog.Info("runtime defaults", "name", r.Name)

modified := false

if r.Spec.Grpc.Resources == nil {
r.Spec.Grpc.Resources = config.MlmdGRPCResourceRequirements.DeepCopy()
modified = true
}
if len(r.Spec.Grpc.Image) == 0 {
r.Spec.Grpc.Image = config.GetStringConfigWithDefault(config.GrpcImage, config.DefaultGrpcImage)
modified = true
}

if r.Spec.Rest.Resources == nil {
r.Spec.Rest.Resources = config.MlmdRestResourceRequirements.DeepCopy()
modified = true
}
if len(r.Spec.Rest.Image) == 0 {
r.Spec.Rest.Image = config.GetStringConfigWithDefault(config.RestImage, config.DefaultRestImage)
modified = true
}

// istio defaults
if r.Spec.Istio != nil {
// set default audiences
if len(r.Spec.Istio.Audiences) == 0 {
r.Spec.Istio.Audiences = config.GetDefaultAudiences()
modified = true
}
// set default authprovider
if len(r.Spec.Istio.AuthProvider) == 0 {
r.Spec.Istio.AuthProvider = config.GetDefaultAuthProvider()
modified = true
}
// set default authconfig labels
if len(r.Spec.Istio.AuthConfigLabels) == 0 {
r.Spec.Istio.AuthConfigLabels = config.GetDefaultAuthConfigLabels()
modified = true
}

if r.Spec.Istio.Gateway != nil {
// set default domain
if len(r.Spec.Istio.Gateway.Domain) == 0 {
r.Spec.Istio.Gateway.Domain = config.GetDefaultDomain()
modified = true
}

// set default cert
if r.Spec.Istio.Gateway.Rest.TLS != nil && r.Spec.Istio.Gateway.Rest.TLS.Mode != DefaultTlsMode &&
(r.Spec.Istio.Gateway.Rest.TLS.CredentialName == nil || len(*r.Spec.Istio.Gateway.Rest.TLS.CredentialName) == 0) {
cert := config.GetDefaultCert()
r.Spec.Istio.Gateway.Rest.TLS.CredentialName = &cert
modified = true
}
if r.Spec.Istio.Gateway.Grpc.TLS != nil && r.Spec.Istio.Gateway.Grpc.TLS.Mode != DefaultTlsMode &&
(r.Spec.Istio.Gateway.Grpc.TLS.CredentialName == nil || len(*r.Spec.Istio.Gateway.Grpc.TLS.CredentialName) == 0) {
cert := config.GetDefaultCert()
r.Spec.Istio.Gateway.Grpc.TLS.CredentialName = &cert
modified = true
}
}
}

return modified
}

// ValidateRegistry validates registry spec
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/modelregistry_controller_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"bufio"
"context"
"fmt"
jsonpatch "github.com/evanphx/json-patch/v5"
"github.com/evanphx/json-patch/v5"
"github.com/go-logr/logr"
authorino "github.com/kuadrant/authorino/api/v1beta2"
modelregistryv1alpha1 "github.com/opendatahub-io/model-registry-operator/api/v1alpha1"
Expand Down

0 comments on commit 00d0683

Please sign in to comment.