Skip to content

Commit

Permalink
[Cherry pick] Set the volume mount's readonly annotation based on the…
Browse files Browse the repository at this point in the history 
… ISVC annotation (#413)

* Set the volume mount's readonly annotation based on the ISVC annotation

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>

* fixes

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>

* Cleanup

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>

* Add test case where readonly is unset, check values

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>

* Use StorageInitializerVolumeName constant

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>

* Set the readonly value for the storage-initializer

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>

* Add tests for direct pvc volume mount use case

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>

---------

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
Co-authored-by: Spolti <fspolti@redhat.com>
  • Loading branch information
@hdefazio @spolti
hdefazio and spolti authored Sep 23, 2024
1 parent 81e044d commit a292015
Show file tree
Hide file tree
Showing 4 changed files with 453 additions and 4 deletions.
1 change: 1 addition & 0 deletions pkg/constants/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ var (
KServeContainerPrometheusPathKey = "prometheus.kserve.io/path"
PrometheusPortAnnotationKey = "prometheus.io/port"
PrometheusPathAnnotationKey = "prometheus.io/path"
StorageReadonlyAnnotationKey = "storage.kserve.io/readonly"
DefaultPrometheusPath = "/metrics"
QueueProxyAggregatePrometheusMetricsPort = 9088
DefaultPodPrometheusPort = "9091"
Expand Down
153 changes: 153 additions & 0 deletions pkg/controller/v1beta1/inferenceservice/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1873,6 +1873,159 @@ var _ = Describe("v1beta1 inference service controller", func() {
})
})

Context("When creating inference service with storage.kserve.io/readonly", func() {
defaultIsvc := func(namespace string, name string, storageUri string) *v1beta1.InferenceService {
predictor := v1beta1.PredictorSpec{
ComponentExtensionSpec: v1beta1.ComponentExtensionSpec{
MinReplicas: v1beta1.GetIntReference(1),
MaxReplicas: 3,
},
Tensorflow: &v1beta1.TFServingSpec{
PredictorExtensionSpec: v1beta1.PredictorExtensionSpec{
StorageURI: &storageUri,
RuntimeVersion: proto.String("1.14.0"),
Container: v1.Container{
Name: constants.InferenceServiceContainerName,
Resources: defaultResource,
VolumeMounts: []v1.VolumeMount{
{Name: "predictor-volume"},
},
},
},
},
}
isvc := &v1beta1.InferenceService{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},

Spec: v1beta1.InferenceServiceSpec{
Predictor: predictor,
},
}
return isvc
}

createServingRuntime := func(namespace string, name string) *v1alpha1.ServingRuntime {
// Define and create serving runtime
servingRuntime := &v1alpha1.ServingRuntime{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: v1alpha1.ServingRuntimeSpec{
SupportedModelFormats: []v1alpha1.SupportedModelFormat{
{
Name: "tensorflow",
Version: proto.String("1"),
AutoSelect: proto.Bool(true),
},
},
ServingRuntimePodSpec: v1alpha1.ServingRuntimePodSpec{
Containers: []v1.Container{
{
Name: constants.InferenceServiceContainerName,
Image: "tensorflow/serving:1.14.0",
Command: []string{"/usr/bin/tensorflow_model_server"},
Args: []string{
"--port=9000",
"--rest_api_port=8080",
"--model_base_path=/mnt/models",
"--rest_api_timeout_in_ms=60000",
},
Resources: defaultResource,
},
},
ImagePullSecrets: []v1.LocalObjectReference{
{Name: "sr-image-pull-secret"},
},
},
Disabled: proto.Bool(false),
},
}
Expect(k8sClient.Create(ctx, servingRuntime)).NotTo(HaveOccurred())
return servingRuntime
}

createInferenceServiceConfigMap := func() *v1.ConfigMap {
configMap := &v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: constants.InferenceServiceConfigMapName,
Namespace: constants.KServeNamespace,
},
Data: configs,
}

Expect(k8sClient.Create(context.TODO(), configMap)).NotTo(gomega.HaveOccurred())
return configMap
}

It("should have the readonly annotation set to true in the knative serving pod spec", func() {
configMap := createInferenceServiceConfigMap()
defer k8sClient.Delete(ctx, configMap)

serviceName := "readonly-true-isvc"
serviceNamespace := "default"
var expectedRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: serviceName, Namespace: serviceNamespace}}
var serviceKey = expectedRequest.NamespacedName
var storageUri = "s3://test/mnist/export"

servingRuntime := createServingRuntime(serviceKey.Namespace, "tf-serving")
defer k8sClient.Delete(ctx, servingRuntime)

// Define InferenceService
isvc := defaultIsvc(serviceKey.Namespace, serviceKey.Name, storageUri)
isvc.Annotations = map[string]string{}
isvc.Annotations[constants.StorageReadonlyAnnotationKey] = "true"
Expect(k8sClient.Create(context.TODO(), isvc)).NotTo(gomega.HaveOccurred())
defer k8sClient.Delete(ctx, isvc)

// Knative service
actualService := &knservingv1.Service{}
predictorServiceKey := types.NamespacedName{Name: constants.PredictorServiceName(serviceKey.Name),
Namespace: serviceKey.Namespace}
Eventually(func() error { return k8sClient.Get(context.TODO(), predictorServiceKey, actualService) }, timeout).
Should(Succeed())

// Check readonly value
Expect(actualService.Spec.Template.Annotations[constants.StorageReadonlyAnnotationKey]).
To(Equal("true"))
})

It("should have the readonly annotation set to false in the knative serving pod spec", func() {
configMap := createInferenceServiceConfigMap()
defer k8sClient.Delete(ctx, configMap)

serviceName := "readonly-false-isvc"
serviceNamespace := "default"
var expectedRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: serviceName, Namespace: serviceNamespace}}
var serviceKey = expectedRequest.NamespacedName
var storageUri = "s3://test/mnist/export"

servingRuntime := createServingRuntime(serviceKey.Namespace, "tf-serving")
defer k8sClient.Delete(ctx, servingRuntime)

// Define InferenceService
isvc := defaultIsvc(serviceKey.Namespace, serviceKey.Name, storageUri)
isvc.Annotations = map[string]string{}
isvc.Annotations[constants.StorageReadonlyAnnotationKey] = "false"
Expect(k8sClient.Create(context.TODO(), isvc)).NotTo(gomega.HaveOccurred())
defer k8sClient.Delete(ctx, isvc)

// Knative service
actualService := &knservingv1.Service{}
predictorServiceKey := types.NamespacedName{Name: constants.PredictorServiceName(serviceKey.Name),
Namespace: serviceKey.Namespace}
Eventually(func() error { return k8sClient.Get(context.TODO(), predictorServiceKey, actualService) }, timeout).
Should(Succeed())

// Check readonly value
Expect(actualService.Spec.Template.Annotations[constants.StorageReadonlyAnnotationKey]).
To(Equal("false"))
})
})

Context("When creating an inference service with invalid Storage URI", func() {
It("Should fail with reason ModelLoadFailed", func() {
serviceName := "servingruntime-test"
Expand Down
17 changes: 13 additions & 4 deletions pkg/webhook/admission/pod/storage_initializer_injector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (mi *StorageInitializerInjector) InjectModelcar(pod *v1.Pod) error {

// InjectStorageInitializer injects an init container to provision model data
// for the serving container in a unified way across storage tech by injecting
// a provisioning INIT container. This is a work around because KNative does not
// a provisioning INIT container. This is a workaround because KNative does not
// support INIT containers: https://github.com/knative/serving/issues/4307
func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod) error {
// Only inject if the required annotations are set
Expand All @@ -216,6 +216,15 @@ func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod) erro
}
}

// Update volume mount's readonly annotation based on the ISVC annotation
isvcReadonlyStringFlag := true
isvcReadonlyString, ok := pod.ObjectMeta.Annotations[constants.StorageReadonlyAnnotationKey]
if ok {
if isvcReadonlyString == "false" {
isvcReadonlyStringFlag = false
}
}

// Find the kserve-container (this is the model inference server) and transformer container
userContainer := getContainerWithName(pod, constants.InferenceServiceContainerName)
transformerContainer := getContainerWithName(pod, constants.TransformerContainerName)
Expand Down Expand Up @@ -259,7 +268,7 @@ func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod) erro
MountPath: constants.DefaultModelLocalMountPath,
// only path to volume's root ("") or folder is supported
SubPath: pvcPath,
ReadOnly: true,
ReadOnly: isvcReadonlyStringFlag,
}

// Check if PVC source URIs is already mounted
Expand Down Expand Up @@ -305,7 +314,7 @@ func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod) erro
pvcSourceVolumeMount := v1.VolumeMount{
Name: PvcSourceMountName,
MountPath: PvcSourceMountPath,
ReadOnly: true,
ReadOnly: isvcReadonlyStringFlag,
}
storageInitializerMounts = append(storageInitializerMounts, pvcSourceVolumeMount)

Expand Down Expand Up @@ -368,7 +377,7 @@ func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod) erro
sharedVolumeReadMount := v1.VolumeMount{
Name: StorageInitializerVolumeName,
MountPath: constants.DefaultModelLocalMountPath,
ReadOnly: true,
ReadOnly: isvcReadonlyStringFlag,
}
userContainer.VolumeMounts = append(userContainer.VolumeMounts, sharedVolumeReadMount)
if transformerContainer != nil {
Expand Down
Loading

0 comments on commit a292015

Please sign in to comment.