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.

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

add kserve raw reconciler #250

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion controllers/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ const (
LabelEnableAuth = "enable-auth"
LabelEnableRoute = "enable-route"

CapabilityServiceMeshAuthorization = "CapabilityServiceMeshAuthorization"
CapabilityServiceMeshAuthorization = "CapabilityServiceMeshAuthorization"
InferenceServiceDeploymentModeAnnotation = "serving.kserve.io/deploymentMode"
KserveConfigMapName = "inferenceservice-config"
KServeWithServiceMeshComponent = "kserve-service-mesh"
)

// model registry
Expand Down
3 changes: 2 additions & 1 deletion controllers/inferenceservice_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
kservev1alpha1 "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
authorinov1beta2 "github.com/kuadrant/authorino/api/v1beta2"
"github.com/opendatahub-io/odh-model-controller/controllers/constants"
"github.com/opendatahub-io/odh-model-controller/controllers/reconcilers"
"github.com/opendatahub-io/odh-model-controller/controllers/utils"
routev1 "github.com/openshift/api/route/v1"
Expand Down Expand Up @@ -172,7 +173,7 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager)
}
}))

kserveWithMeshEnabled, kserveWithMeshEnabledErr := utils.VerifyIfComponentIsEnabled(context.Background(), mgr.GetClient(), utils.KServeWithServiceMeshComponent)
kserveWithMeshEnabled, kserveWithMeshEnabledErr := utils.VerifyIfComponentIsEnabled(context.Background(), mgr.GetClient(), constants.KServeWithServiceMeshComponent)
if kserveWithMeshEnabledErr != nil {
r.log.V(1).Error(kserveWithMeshEnabledErr, "could not determine if kserve have service mesh enabled")
}
Expand Down
98 changes: 91 additions & 7 deletions controllers/kserve_inferenceservice_controller_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package controllers

import (
"github.com/opendatahub-io/odh-model-controller/controllers/constants"
utils "github.com/opendatahub-io/odh-model-controller/controllers/utils"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"strings"
Expand Down Expand Up @@ -39,14 +40,18 @@ var _ = Describe("The KServe Dashboard reconciler", func() {
return servingRuntime
}

createInferenceService := func(namespace, name string, path string) *kservev1beta1.InferenceService {
createInferenceService := func(namespace, name string, path string, deploymentMode utils.IsvcDeploymentMode) *kservev1beta1.InferenceService {
inferenceService := &kservev1beta1.InferenceService{}
err := convertToStructuredResource(path, inferenceService)
Expect(err).NotTo(HaveOccurred())
inferenceService.SetNamespace(namespace)
if len(name) != 0 {
inferenceService.Name = name
}
if deploymentMode == utils.RawDeployment {
inferenceService.ObjectMeta.Annotations = map[string]string{}
inferenceService.Annotations[constants.InferenceServiceDeploymentModeAnnotation] = string(utils.RawDeployment)
}
if err := cli.Create(ctx, inferenceService); err != nil && !errors.IsAlreadyExists(err) {
Fail(err.Error())
}
Expand All @@ -64,10 +69,10 @@ var _ = Describe("The KServe Dashboard reconciler", func() {

})

When("deploying a Kserve model", func() {
When("deploying a Kserve Serverless model", func() {
It("if the runtime is supported for metrics, it should create a configmap with prometheus queries", func() {
_ = createServingRuntime(testNs, KserveServingRuntimePath1)
_ = createInferenceService(testNs, KserveOvmsInferenceServiceName, KserveInferenceServicePath1)
_ = createInferenceService(testNs, KserveOvmsInferenceServiceName, KserveInferenceServicePath1, utils.Serverless)

metricsConfigMap, err := waitForConfigMap(cli, testNs, KserveOvmsInferenceServiceName+constants.KserveMetricsConfigMapNameSuffix, 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -89,7 +94,78 @@ var _ = Describe("The KServe Dashboard reconciler", func() {

It("if the runtime is not supported for metrics, it should create a configmap with the unsupported config", func() {
_ = createServingRuntime(testNs, UnsupprtedMetricsServingRuntimePath)
_ = createInferenceService(testNs, UnsupportedMetricsInferenceServiceName, UnsupportedMetricsInferenceServicePath)
_ = createInferenceService(testNs, UnsupportedMetricsInferenceServiceName, UnsupportedMetricsInferenceServicePath, utils.Serverless)

metricsConfigMap, err := waitForConfigMap(cli, testNs, UnsupportedMetricsInferenceServiceName+constants.KserveMetricsConfigMapNameSuffix, 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expect(metricsConfigMap).NotTo(BeNil())

expectedmetricsConfigMap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: UnsupportedMetricsInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix,
Namespace: testNs,
},
Data: map[string]string{
"supported": "false",
},
}
Expect(compareConfigMap(metricsConfigMap, expectedmetricsConfigMap)).Should(BeTrue())
})

It("if the isvc does not have a runtime specified, an unsupported metrics configmap should be created", func() {
_ = createInferenceService(testNs, NilRuntimeInferenceServiceName, NilRuntimeInferenceServicePath, utils.Serverless)

metricsConfigMap, err := waitForConfigMap(cli, testNs, NilRuntimeInferenceServiceName+constants.KserveMetricsConfigMapNameSuffix, 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expect(metricsConfigMap).NotTo(BeNil())

expectedmetricsConfigMap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: NilRuntimeInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix,
Namespace: testNs,
},
Data: map[string]string{
"supported": "false",
},
}
Expect(compareConfigMap(metricsConfigMap, expectedmetricsConfigMap)).Should(BeTrue())
})
})

When("deploying a Kserve RawDeployment model", func() {
It("if the runtime is supported for metrics, it should create a configmap with prometheus queries and create a metrics service and servicemonitor", func() {
_ = createServingRuntime(testNs, KserveServingRuntimePath1)
_ = createInferenceService(testNs, KserveOvmsInferenceServiceName, KserveInferenceServicePath1, utils.RawDeployment)

metricsConfigMap, err := waitForConfigMap(cli, testNs, KserveOvmsInferenceServiceName+constants.KserveMetricsConfigMapNameSuffix, 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expect(metricsConfigMap).NotTo(BeNil())

finaldata := substituteVariablesInQueries(constants.OvmsMetricsData, testNs, KserveOvmsInferenceServiceName, constants.IntervalValue)
expectedmetricsConfigMap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: KserveOvmsInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix,
Namespace: testNs,
},
Data: map[string]string{
"supported": "true",
"metrics": finaldata,
},
}
Expect(compareConfigMap(metricsConfigMap, expectedmetricsConfigMap)).Should(BeTrue())

metricsService, err := waitForService(cli, testNs, KserveOvmsInferenceServiceName+"-metrics", 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expect(metricsService).NotTo(BeNil())

metricsServiceMonitor, err := waitForServiceMonitor(cli, testNs, KserveOvmsInferenceServiceName+"-metrics", 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expect(metricsServiceMonitor).NotTo(BeNil())
})

It("if the runtime is not supported for metrics, it should create a configmap with the unsupported config and create a metrics service and servicemonitor", func() {
_ = createServingRuntime(testNs, UnsupprtedMetricsServingRuntimePath)
_ = createInferenceService(testNs, UnsupportedMetricsInferenceServiceName, UnsupportedMetricsInferenceServicePath, utils.RawDeployment)

metricsConfigMap, err := waitForConfigMap(cli, testNs, UnsupportedMetricsInferenceServiceName+constants.KserveMetricsConfigMapNameSuffix, 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -105,10 +181,18 @@ var _ = Describe("The KServe Dashboard reconciler", func() {
},
}
Expect(compareConfigMap(metricsConfigMap, expectedmetricsConfigMap)).Should(BeTrue())

metricsService, err := waitForService(cli, testNs, UnsupportedMetricsInferenceServiceName+"-metrics", 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expect(metricsService).NotTo(BeNil())

metricsServiceMonitor, err := waitForServiceMonitor(cli, testNs, UnsupportedMetricsInferenceServiceName+"-metrics", 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expect(metricsServiceMonitor).NotTo(BeNil())
})

It("if the isvc does not have a runtime specified, an unsupported metrics configmap should be created", func() {
_ = createInferenceService(testNs, NilRuntimeInferenceServiceName, NilRuntimeInferenceServicePath)
_ = createInferenceService(testNs, NilRuntimeInferenceServiceName, NilRuntimeInferenceServicePath, utils.RawDeployment)

metricsConfigMap, err := waitForConfigMap(cli, testNs, NilRuntimeInferenceServiceName+constants.KserveMetricsConfigMapNameSuffix, 30, 1*time.Second)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -130,7 +214,7 @@ var _ = Describe("The KServe Dashboard reconciler", func() {
When("deleting the deployed models", func() {
It("it should delete the associated configmap", func() {
_ = createServingRuntime(testNs, KserveServingRuntimePath1)
OvmsInferenceService := createInferenceService(testNs, KserveOvmsInferenceServiceName, KserveInferenceServicePath1)
OvmsInferenceService := createInferenceService(testNs, KserveOvmsInferenceServiceName, KserveInferenceServicePath1, utils.Serverless)

Expect(cli.Delete(ctx, OvmsInferenceService)).Should(Succeed())
Eventually(func() error {
Expand All @@ -141,7 +225,7 @@ var _ = Describe("The KServe Dashboard reconciler", func() {
}, timeout, interval).ShouldNot(Succeed())

_ = createServingRuntime(testNs, UnsupprtedMetricsServingRuntimePath)
SklearnInferenceService := createInferenceService(testNs, UnsupportedMetricsInferenceServiceName, UnsupportedMetricsInferenceServicePath)
SklearnInferenceService := createInferenceService(testNs, UnsupportedMetricsInferenceServiceName, UnsupportedMetricsInferenceServicePath, utils.RawDeployment)

Expect(cli.Delete(ctx, SklearnInferenceService)).Should(Succeed())
Eventually(func() error {
Expand Down
32 changes: 10 additions & 22 deletions controllers/reconcilers/kserve_metrics_dashboard_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"strings"

"github.com/go-logr/logr"
kservev1alpha1 "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
"github.com/opendatahub-io/odh-model-controller/controllers/comparators"
"github.com/opendatahub-io/odh-model-controller/controllers/constants"
Expand Down Expand Up @@ -93,37 +92,26 @@ func (r *KserveMetricsDashboardReconciler) Reconcile(ctx context.Context, log lo

func (r *KserveMetricsDashboardReconciler) createDesiredResource(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) (*corev1.ConfigMap, error) {

var err error
var servingRuntime string
runtime := &kservev1alpha1.ServingRuntime{}
supported := false
// resolve SR
isvcRuntime := isvc.Spec.Predictor.Model.Runtime
if isvcRuntime == nil {
runtime, err = utils.FindSupportingRuntimeForISvc(ctx, r.client, log, isvc)
if err != nil {
if errwrap.Contains(err, constants.NoSuitableRuntimeError) {
configmap, err := r.createConfigMap(isvc, false, log)
if err != nil {
return nil, err
}
return configmap, nil
isvcRuntime, err := utils.FindSupportingRuntimeForISvc(ctx, r.client, log, isvc)
if err != nil {
if errwrap.Contains(err, constants.NoSuitableRuntimeError) {
configmap, err := r.createConfigMap(isvc, false, log)
if err != nil {
return nil, err
}
return nil, err
}
} else {
if err := r.client.Get(ctx, types.NamespacedName{Name: *isvcRuntime, Namespace: isvc.Namespace}, runtime); err != nil {
log.Error(err, "Could not determine servingruntime for isvc")
return nil, err
return configmap, nil
}
return nil, err
}

if (runtime.Spec.Containers == nil) || (len(runtime.Spec.Containers) < 1) {
if (isvcRuntime.Spec.Containers == nil) || (len(isvcRuntime.Spec.Containers) < 1) {
log.V(1).Info("Could not determine runtime image")
supported = false
}

servingRuntimeImage := runtime.Spec.Containers[0].Image
servingRuntimeImage := isvcRuntime.Spec.Containers[0].Image
re := regexp.MustCompile(`/([^/@]+)[@:]`)
findImageName := re.FindStringSubmatch(servingRuntimeImage)
// sanity check for regex match, will fall back to a known string that will lead to a configmap for unsupported metrics
Expand Down
24 changes: 19 additions & 5 deletions controllers/reconcilers/kserve_raw_inferenceservice_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package reconcilers

import (
"context"
"github.com/hashicorp/go-multierror"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to the next group of imports.


"github.com/go-logr/logr"
kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1"
Expand All @@ -26,16 +27,29 @@ import (
var _ Reconciler = (*KserveRawInferenceServiceReconciler)(nil)

type KserveRawInferenceServiceReconciler struct {
client client.Client
client client.Client
subResourceReconcilers []SubResourceReconciler
}

func NewKServeRawInferenceServiceReconciler(client client.Client) *KserveRawInferenceServiceReconciler {

subResourceReconciler := []SubResourceReconciler{
NewKServeRawMetricsServiceReconciler(client),
NewRawKServeMetricsServiceMonitorReconciler(client),
NewKserveMetricsDashboardReconciler(client),
}

return &KserveRawInferenceServiceReconciler{
client: client,
client: client,
subResourceReconcilers: subResourceReconciler,
}
}

func (r *KserveRawInferenceServiceReconciler) Reconcile(_ context.Context, log logr.Logger, _ *kservev1beta1.InferenceService) error {
log.V(1).Info("No Reconciliation to be done for inferenceservice as it is using RawDeployment mode")
return nil
func (r *KserveRawInferenceServiceReconciler) Reconcile(ctx context.Context, log logr.Logger, isvc *kservev1beta1.InferenceService) error {
var reconcileErrors *multierror.Error
for _, reconciler := range r.subResourceReconcilers {
reconcileErrors = multierror.Append(reconcileErrors, reconciler.Reconcile(ctx, log, isvc))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also will need to implement OnDeletionOfKserveInferenceService and CleanupNamespaceIfNoKserveIsvcExists.

Copy link
Contributor Author

@VedantMahabaleshwarkar VedantMahabaleshwarkar Aug 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep those changes are made already :D, they will be a part of the next set of commits

}

return reconcileErrors.ErrorOrNil()
}
Loading