Skip to content

Commit

Permalink
Show KServe defaultDeploymentMode in DSC status
Browse files Browse the repository at this point in the history
JIRA: https://issues.redhat.com/browse/RHOAIENG-16240

The dashboard would like to be able to read this value, and should
only read it from the status.

The value for the status is always read from the KServe ConfigMap,
rather than ever assuming that what is set in the DSC, or what the
operator defaults to, is correct: this is because that ConfigMap can
be annotated to be unmanaged by the operator, in which case a user may
have changed the value to something else, and the status should
reflect this reality.
  • Loading branch information
grdryn committed Dec 30, 2024
1 parent 9b75bac commit 3635ba3
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 11 deletions.
3 changes: 3 additions & 0 deletions apis/components/v1alpha1/kserve_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ type KserveSpec struct {

// KserveCommonStatus defines the shared observed state of Kserve
type KserveCommonStatus struct {
// DefaultDeploymentMode is the value of the defaultDeploymentMode field
// as read from the "deploy" JSON in the inferenceservice-config ConfigMap
DefaultDeploymentMode string `json:"defaultDeploymentMode,omitempty"`
}

// KserveStatus defines the observed state of Kserve
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,11 @@ spec:
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
defaultDeploymentMode:
description: |-
DefaultDeploymentMode is the value of the defaultDeploymentMode field
as read from the "deploy" JSON in the inferenceservice-config ConfigMap
type: string
observedGeneration:
format: int64
type: integer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,11 @@ spec:
kserve:
description: Kserve component status.
properties:
defaultDeploymentMode:
description: |-
DefaultDeploymentMode is the value of the defaultDeploymentMode field
as read from the "deploy" JSON in the inferenceservice-config ConfigMap
type: string
managementState:
description: |-
Set to one of the following values:
Expand Down
26 changes: 26 additions & 0 deletions controllers/components/kserve/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package kserve

import (
"encoding/json"
"fmt"

corev1 "k8s.io/api/core/v1"
)

// ConfigMap Keys.
const (
DeployConfigName = "deploy"
IngressConfigKeyName = "ingress"
)

type DeployConfig struct {
DefaultDeploymentMode string `json:"defaultDeploymentMode,omitempty"`
}

func getDeployConfig(cm *corev1.ConfigMap) (*DeployConfig, error) {
deployConfig := DeployConfig{}
if err := json.Unmarshal([]byte(cm.Data[DeployConfigName]), &deployConfig); err != nil {
return nil, fmt.Errorf("error retrieving value for key '%s' from ConfigMap %s. %w", DeployConfigName, cm.Name, err)
}
return &deployConfig, nil
}
1 change: 1 addition & 0 deletions controllers/components/kserve/kserve_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
WithAction(deploy.NewAction(
deploy.WithCache(),
)).
WithAction(setStatusFields).
WithAction(updatestatus.NewAction()).
// must be the final action
WithAction(gc.NewAction()).
Expand Down
21 changes: 21 additions & 0 deletions controllers/components/kserve/kserve_controller_actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
Expand Down Expand Up @@ -262,3 +263,23 @@ func customizeKserveConfigMap(ctx context.Context, rr *odhtypes.ReconciliationRe

return nil
}

func setStatusFields(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)
}

kserveConfigMap := corev1.ConfigMap{}
if err := rr.Client.Get(ctx, client.ObjectKey{Name: kserveConfigMapName, Namespace: rr.DSCI.Spec.ApplicationsNamespace}, &kserveConfigMap); err != nil {
return err
}

ddm, err := getDefaultDeploymentMode(&kserveConfigMap)
if err != nil {
return err
}

k.Status.DefaultDeploymentMode = ddm
return nil
}
30 changes: 19 additions & 11 deletions controllers/components/kserve/kserve_support.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,24 +148,32 @@ func removeServerlessFeatures(ctx context.Context, cli client.Client, k *compone
return serverlessFeatures.Delete(ctx, cli)
}

func getDefaultDeploymentMode(inferenceServiceConfigMap *corev1.ConfigMap) (string, error) {
deployConfig, err := getDeployConfig(inferenceServiceConfigMap)
if err != nil {
return "", err
}

return deployConfig.DefaultDeploymentMode, nil
}

func setDefaultDeploymentMode(inferenceServiceConfigMap *corev1.ConfigMap, defaultmode componentApi.DefaultDeploymentMode) error {
// set data.deploy.defaultDeploymentMode to the model specified in the Kserve spec
var deployData map[string]interface{}
if err := json.Unmarshal([]byte(inferenceServiceConfigMap.Data["deploy"]), &deployData); err != nil {
return fmt.Errorf("error retrieving value for key 'deploy' from configmap %s. %w", kserveConfigMapName, err)
deployData, err := getDeployConfig(inferenceServiceConfigMap)
if err != nil {
return err
}
modeFound := deployData["defaultDeploymentMode"]
if modeFound != string(defaultmode) {
deployData["defaultDeploymentMode"] = defaultmode

if deployData.DefaultDeploymentMode != string(defaultmode) {
deployData.DefaultDeploymentMode = string(defaultmode)
deployDataBytes, err := json.MarshalIndent(deployData, "", " ")
if err != nil {
return fmt.Errorf("could not set values in configmap %s. %w", kserveConfigMapName, err)
}
inferenceServiceConfigMap.Data["deploy"] = string(deployDataBytes)
inferenceServiceConfigMap.Data[DeployConfigName] = string(deployDataBytes)

var ingressData map[string]interface{}
if err = json.Unmarshal([]byte(inferenceServiceConfigMap.Data["ingress"]), &ingressData); err != nil {
return fmt.Errorf("error retrieving value for key 'ingress' from configmap %s. %w", kserveConfigMapName, err)
if err = json.Unmarshal([]byte(inferenceServiceConfigMap.Data[IngressConfigKeyName]), &ingressData); err != nil {
return fmt.Errorf("error retrieving value for key '%s' from configmap %s. %w", IngressConfigKeyName, kserveConfigMapName, err)
}
if defaultmode == componentApi.RawDeployment {
ingressData["disableIngressCreation"] = true
Expand All @@ -176,7 +184,7 @@ func setDefaultDeploymentMode(inferenceServiceConfigMap *corev1.ConfigMap, defau
if err != nil {
return fmt.Errorf("could not set values in configmap %s. %w", kserveConfigMapName, err)
}
inferenceServiceConfigMap.Data["ingress"] = string(ingressDataBytes)
inferenceServiceConfigMap.Data[IngressConfigKeyName] = string(ingressDataBytes)
}

return nil
Expand Down
4 changes: 4 additions & 0 deletions docs/api-overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,9 @@ _Appears in:_
- [DSCKserveStatus](#dsckservestatus)
- [KserveStatus](#kservestatus)

| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `defaultDeploymentMode` _string_ | DefaultDeploymentMode is the value of the defaultDeploymentMode field<br />as read from the "deploy" JSON in the inferenceservice-config ConfigMap | | |


#### KserveList
Expand Down Expand Up @@ -862,6 +865,7 @@ _Appears in:_
| `phase` _string_ | | | |
| `observedGeneration` _integer_ | | | |
| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.25/#condition-v1-meta) array_ | | | |
| `defaultDeploymentMode` _string_ | DefaultDeploymentMode is the value of the defaultDeploymentMode field<br />as read from the "deploy" JSON in the inferenceservice-config ConfigMap | | |


#### Kueue
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster {
ManagementState: operatorv1.Removed,
},
KserveCommonSpec: componentApi.KserveCommonSpec{
DefaultDeploymentMode: componentApi.Serverless,
Serving: infrav1.ServingSpec{
ManagementState: operatorv1.Managed,
Name: "knative-serving",
Expand Down
2 changes: 2 additions & 0 deletions tests/e2e/kserve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ func (k *KserveTestCtx) validateKserveInstance(t *testing.T) {
k.testDsc.Spec.Components.Kserve.Serving.IngressGateway.Certificate.Type),

jq.Match(`.status.phase == "%s"`, readyStatus),
jq.Match(`.status.defaultDeploymentMode == "%s"`, k.testDsc.Spec.Components.Kserve.DefaultDeploymentMode),
)),
))

Expand All @@ -86,6 +87,7 @@ func (k *KserveTestCtx) validateKserveInstance(t *testing.T) {
jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, modelcontroller.ReadyConditionType, metav1.ConditionTrue),
jq.Match(`.status.installedComponents."%s" == true`, kserve.LegacyComponentName),
jq.Match(`.status.components.%s.managementState == "%s"`, componentApi.KserveComponentName, operatorv1.Managed),
jq.Match(`.status.components.%s.defaultDeploymentMode == "%s"`, componentApi.KserveComponentName, k.testDsc.Spec.Components.Kserve.DefaultDeploymentMode),
)),
))

Expand Down

0 comments on commit 3635ba3

Please sign in to comment.