From cedec59f72d4e83ade59c509bb7266bca58aa6fc Mon Sep 17 00:00:00 2001 From: Ellis Tarn Date: Fri, 4 Oct 2019 14:57:57 +0800 Subject: [PATCH] Migrated to PodMutator from DeploymentMutator to avoid blocking k8s deployments before KFServing is installed (#400) --- pkg/constants/constants.go | 2 +- .../storage_initializer_injector_test.go | 803 ------------------ .../accelerator_injector.go | 13 +- .../accelerator_injector_test.go | 86 +- .../admission/{deployment => pod}/mutator.go | 24 +- .../storage_initializer_injector.go | 27 +- .../pod/storage_initializer_injector_test.go | 726 ++++++++++++++++ .../{deployment => pod}/suite_test.go | 2 +- pkg/webhook/webhook.go | 10 +- 9 files changed, 794 insertions(+), 899 deletions(-) delete mode 100644 pkg/webhook/admission/deployment/storage_initializer_injector_test.go rename pkg/webhook/admission/{deployment => pod}/accelerator_injector.go (73%) rename pkg/webhook/admission/{deployment => pod}/accelerator_injector_test.go (52%) rename pkg/webhook/admission/{deployment => pod}/mutator.go (79%) rename pkg/webhook/admission/{deployment => pod}/storage_initializer_injector.go (89%) create mode 100644 pkg/webhook/admission/pod/storage_initializer_injector_test.go rename pkg/webhook/admission/{deployment => pod}/suite_test.go (98%) diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 7647cced8cc..a1bdff96957 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -67,7 +67,7 @@ var ( KFServiceMutatingWebhookConfigName = strings.Join([]string{KFServiceName, KFServingAPIGroupName}, ".") KFServiceValidatingWebhookName = strings.Join([]string{KFServiceName, WebhookServerName, "validator"}, ".") KFServiceDefaultingWebhookName = strings.Join([]string{KFServiceName, WebhookServerName, "defaulter"}, ".") - DeploymentMutatorWebhookName = strings.Join([]string{KFServiceName, WebhookServerName, "deployment-mutator"}, ".") + PodMutatorWebhookName = strings.Join([]string{KFServiceName, WebhookServerName, "pod-mutator"}, ".") WebhookFailurePolicy = v1beta1.Fail ) diff --git a/pkg/webhook/admission/deployment/storage_initializer_injector_test.go b/pkg/webhook/admission/deployment/storage_initializer_injector_test.go deleted file mode 100644 index 9d0088fbb03..00000000000 --- a/pkg/webhook/admission/deployment/storage_initializer_injector_test.go +++ /dev/null @@ -1,803 +0,0 @@ -/* -Copyright 2019 kubeflow.org. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ -package deployment - -import ( - "context" - "strings" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/kubeflow/kfserving/pkg/constants" - "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials" - "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials/gcs" - "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials/s3" - "github.com/onsi/gomega" - appsv1 "k8s.io/api/apps/v1" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func TestStorageInitializerInjector(t *testing.T) { - scenarios := map[string]struct { - original *appsv1.Deployment - expected *appsv1.Deployment - }{ - "MissingAnnotations": { - original: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{}, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - }, - }, - }, - }, - }, - }, - expected: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{}, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - }, - }, - }, - }, - }, - }, - }, - "AlreadyInjected": { - original: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - }, - }, - InitContainers: []v1.Container{ - { - Name: "storage-initializer", - }, - }, - }, - }, - }, - }, - expected: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - }, - }, - InitContainers: []v1.Container{ - { - Name: "storage-initializer", - }, - }, - }, - }, - }, - }, - }, - "StorageInitializerInjected": { - original: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - }, - }, - }, - }, - }, - }, - expected: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - ReadOnly: true, - }, - }, - }, - }, - InitContainers: []v1.Container{ - { - Name: "storage-initializer", - Image: StorageInitializerContainerImage + ":" + StorageInitializerContainerImageVersion, - Args: []string{"gs://foo", constants.DefaultModelLocalMountPath}, - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - Volumes: []v1.Volume{ - { - Name: "kfserving-provision-location", - VolumeSource: v1.VolumeSource{ - EmptyDir: &v1.EmptyDirVolumeSource{}, - }, - }, - }, - }, - }, - }, - }, - }, - "StorageInitializerInjectedAndMountsPvc": { - original: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - }, - }, - }, - }, - }, - }, - expected: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - ReadOnly: true, - }, - }, - }, - }, - InitContainers: []v1.Container{ - { - Name: "storage-initializer", - Image: StorageInitializerContainerImage + ":" + StorageInitializerContainerImageVersion, - Args: []string{"/mnt/pvc/some/path/on/pvc", constants.DefaultModelLocalMountPath}, - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-pvc-source", - MountPath: "/mnt/pvc", - ReadOnly: true, - }, - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - Volumes: []v1.Volume{ - { - Name: "kfserving-pvc-source", - VolumeSource: v1.VolumeSource{ - PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{ - ClaimName: "mypvcname", - ReadOnly: false, - }, - }, - }, - { - Name: "kfserving-provision-location", - VolumeSource: v1.VolumeSource{ - EmptyDir: &v1.EmptyDirVolumeSource{}, - }, - }, - }, - }, - }, - }, - }, - }, - } - - for name, scenario := range scenarios { - injector := &StorageInitializerInjector{ - credentialBuilder: credentials.NewCredentialBulder(c, &v1.ConfigMap{ - Data: map[string]string{}, - }), - } - if err := injector.InjectStorageInitializer(scenario.original); err != nil { - t.Errorf("Test %q unexpected result: %s", name, err) - } - if diff := cmp.Diff(scenario.expected.Spec, scenario.original.Spec); diff != "" { - t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) - } - } -} - -func TestStorageInitializerFailureCases(t *testing.T) { - scenarios := map[string]struct { - original *appsv1.Deployment - expectedErrorPrefix string - }{ - "MissingUserContainer": { - original: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "random-container", - }, - }, - }, - }, - }, - }, - expectedErrorPrefix: "Invalid configuration: cannot find container", - }, - } - - for name, scenario := range scenarios { - injector := &StorageInitializerInjector{ - credentialBuilder: credentials.NewCredentialBulder(c, &v1.ConfigMap{ - Data: map[string]string{}, - }), - } - if err := injector.InjectStorageInitializer(scenario.original); err != nil { - if !strings.HasPrefix(err.Error(), scenario.expectedErrorPrefix) { - t.Errorf("Test %q unexpected failure [%s], expected: %s", name, err.Error(), scenario.expectedErrorPrefix) - } - } else { - t.Errorf("Test %q should have failed with: %s", name, scenario.expectedErrorPrefix) - } - } -} - -func TestCustomSpecStorageUriInjection(t *testing.T) { - scenarios := map[string]struct { - original *appsv1.Deployment - expectedStorageUriEnvVariable *v1.EnvVar - }{ - "CustomSpecStorageUriSet": { - original: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - v1.Container{ - Name: "user-container", - Env: []v1.EnvVar{ - v1.EnvVar{ - Name: constants.CustomSpecStorageUriEnvVarKey, - Value: "pvc://mypvcname/some/path/on/pvc", - }, - }, - }, - }, - }, - }, - }, - }, - expectedStorageUriEnvVariable: &v1.EnvVar{ - Name: constants.CustomSpecStorageUriEnvVarKey, - Value: constants.DefaultModelLocalMountPath, - }, - }, - "CustomSpecStorageUriEmpty": { - original: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - v1.Container{ - Name: "user-container", - Env: []v1.EnvVar{ - v1.EnvVar{ - Name: constants.CustomSpecStorageUriEnvVarKey, - Value: "", - }, - }, - }, - }, - }, - }, - }, - }, - expectedStorageUriEnvVariable: &v1.EnvVar{ - Name: constants.CustomSpecStorageUriEnvVarKey, - Value: "", - }, - }, - "CustomSpecStorageUriNotSet": { - original: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - v1.Container{ - Name: "user-container", - Env: []v1.EnvVar{ - v1.EnvVar{ - Name: "TestRandom", - Value: "val", - }, - }, - }, - }, - }, - }, - }, - }, - expectedStorageUriEnvVariable: nil, - }, - } - - for name, scenario := range scenarios { - injector := &StorageInitializerInjector{ - credentialBuilder: credentials.NewCredentialBulder(c, &v1.ConfigMap{ - Data: map[string]string{}, - }), - } - if err := injector.InjectStorageInitializer(scenario.original); err != nil { - t.Errorf("Test %q unexpected result: %s", name, err) - } - - var originalEnvVar *v1.EnvVar - for _, envVar := range scenario.original.Spec.Template.Spec.Containers[0].Env { - if envVar.Name == constants.CustomSpecStorageUriEnvVarKey { - originalEnvVar = &envVar - } - } - if diff := cmp.Diff(scenario.expectedStorageUriEnvVariable, originalEnvVar); diff != "" { - t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) - } - } -} - -func makeDeployment() *appsv1.Deployment { - return &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - }, - }, - }, - }, - }, - } -} - -func TestCredentialInjection(t *testing.T) { - g := gomega.NewGomegaWithT(t) - scenarios := map[string]struct { - sa *v1.ServiceAccount - secret *v1.Secret - original *appsv1.Deployment - expected *appsv1.Deployment - }{ - "Test s3 secrets injection": { - sa: &v1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "default", - Namespace: "default", - }, - Secrets: []v1.ObjectReference{ - { - Name: "s3-secret", - Namespace: "default", - }, - }, - }, - secret: &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "s3-secret", - Namespace: "default", - Annotations: map[string]string{ - s3.KFServiceS3SecretEndpointAnnotation: "s3.aws.com", - }, - }, - Data: map[string][]byte{ - "awsAccessKeyID": {}, - "awsSecretAccessKey": {}, - }, - }, - original: makeDeployment(), - expected: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - ReadOnly: true, - }, - }, - }, - }, - InitContainers: []v1.Container{ - { - Name: "storage-initializer", - Image: StorageInitializerContainerImage + ":" + StorageInitializerContainerImageVersion, - Args: []string{"gs://foo", constants.DefaultModelLocalMountPath}, - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - }, - }, - Env: []v1.EnvVar{ - { - Name: s3.AWSAccessKeyId, - ValueFrom: &v1.EnvVarSource{ - SecretKeyRef: &v1.SecretKeySelector{ - LocalObjectReference: v1.LocalObjectReference{ - Name: "s3-secret", - }, - Key: "awsAccessKeyID", - }, - }, - }, - { - Name: s3.AWSSecretAccessKey, - ValueFrom: &v1.EnvVarSource{ - SecretKeyRef: &v1.SecretKeySelector{ - LocalObjectReference: v1.LocalObjectReference{ - Name: "s3-secret", - }, - Key: "awsSecretAccessKey", - }, - }, - }, - { - Name: s3.S3Endpoint, - Value: "s3.aws.com", - }, - { - Name: s3.AWSEndpointUrl, - Value: "https://s3.aws.com", - }, - }, - }, - }, - Volumes: []v1.Volume{ - v1.Volume{ - Name: "kfserving-provision-location", - VolumeSource: v1.VolumeSource{ - EmptyDir: &v1.EmptyDirVolumeSource{}, - }, - }, - }, - }, - }, - }, - }, - }, - "Test GCS secrets injection": { - sa: &v1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "default", - Namespace: "default", - }, - Secrets: []v1.ObjectReference{ - { - Name: "user-gcp-sa", - Namespace: "default", - }, - }, - }, - secret: &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "user-gcp-sa", - Namespace: "default", - }, - Data: map[string][]byte{ - "gcloud-application-credentials.json": {}, - }, - }, - original: makeDeployment(), - expected: &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - v1.Container{ - Name: "user-container", - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - ReadOnly: true, - }, - }, - }, - }, - InitContainers: []v1.Container{ - { - Name: "storage-initializer", - Image: StorageInitializerContainerImage + ":" + StorageInitializerContainerImageVersion, - Args: []string{"gs://foo", constants.DefaultModelLocalMountPath}, - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - }, - { - Name: gcs.GCSCredentialVolumeName, - ReadOnly: true, - MountPath: gcs.GCSCredentialVolumeMountPath, - }, - }, - Env: []v1.EnvVar{ - { - Name: gcs.GCSCredentialEnvKey, - Value: gcs.GCSCredentialVolumeMountPath + "gcloud-application-credentials.json", - }, - }, - }, - }, - Volumes: []v1.Volume{ - { - Name: "kfserving-provision-location", - VolumeSource: v1.VolumeSource{ - EmptyDir: &v1.EmptyDirVolumeSource{}, - }, - }, - { - Name: gcs.GCSCredentialVolumeName, - VolumeSource: v1.VolumeSource{ - Secret: &v1.SecretVolumeSource{ - SecretName: "user-gcp-sa", - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - var configMap = &v1.ConfigMap{ - Data: map[string]string{ - "credentials": `{ - "gcs" : {"gcsCredentialFileName": "gcloud-application-credentials.json"}, - "s3" : { - "s3AccessKeyIDName": "awsAccessKeyID", - "s3SecretAccessKeyName": "awsSecretAccessKey" - } - }`, - }, - } - - builder := credentials.NewCredentialBulder(c, configMap) - for name, scenario := range scenarios { - g.Expect(c.Create(context.TODO(), scenario.sa)).NotTo(gomega.HaveOccurred()) - g.Expect(c.Create(context.TODO(), scenario.secret)).NotTo(gomega.HaveOccurred()) - - injector := &StorageInitializerInjector{ - credentialBuilder: builder, - } - if err := injector.InjectStorageInitializer(scenario.original); err != nil { - t.Errorf("Test %q unexpected failure [%s]", name, err.Error()) - } - if diff := cmp.Diff(scenario.expected.Spec, scenario.original.Spec); diff != "" { - t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) - } - - g.Expect(c.Delete(context.TODO(), scenario.sa)).NotTo(gomega.HaveOccurred()) - g.Expect(c.Delete(context.TODO(), scenario.secret)).NotTo(gomega.HaveOccurred()) - } -} - -func TestStorageInitializerConfigmap(t *testing.T) { - scenarios := map[string]struct { - original *appsv1.Deployment - expected *appsv1.Deployment - }{ - "StorageInitializerConfig": { - original: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - }, - }, - }, - }, - }, - }, - expected: &appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "user-container", - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - ReadOnly: true, - }, - }, - }, - }, - InitContainers: []v1.Container{ - { - Name: "storage-initializer", - Image: "kfserving/storage-initializer@sha256:xxx", - Args: []string{"gs://foo", constants.DefaultModelLocalMountPath}, - VolumeMounts: []v1.VolumeMount{ - { - Name: "kfserving-provision-location", - MountPath: constants.DefaultModelLocalMountPath, - }, - }, - }, - }, - Volumes: []v1.Volume{ - { - Name: "kfserving-provision-location", - VolumeSource: v1.VolumeSource{ - EmptyDir: &v1.EmptyDirVolumeSource{}, - }, - }, - }, - }, - }, - }, - }, - }, - } - - for name, scenario := range scenarios { - injector := &StorageInitializerInjector{ - credentialBuilder: credentials.NewCredentialBulder(c, &v1.ConfigMap{ - Data: map[string]string{}, - }), - config: &StorageInitializerConfig{ - Image: "kfserving/storage-initializer@sha256:xxx", - }, - } - if err := injector.InjectStorageInitializer(scenario.original); err != nil { - t.Errorf("Test %q unexpected result: %s", name, err) - } - if diff := cmp.Diff(scenario.expected.Spec, scenario.original.Spec); diff != "" { - t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) - } - } -} diff --git a/pkg/webhook/admission/deployment/accelerator_injector.go b/pkg/webhook/admission/pod/accelerator_injector.go similarity index 73% rename from pkg/webhook/admission/deployment/accelerator_injector.go rename to pkg/webhook/admission/pod/accelerator_injector.go index e1f5896006c..c890fafd8f8 100644 --- a/pkg/webhook/admission/deployment/accelerator_injector.go +++ b/pkg/webhook/admission/pod/accelerator_injector.go @@ -13,13 +13,12 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -package deployment +package pod import ( "github.com/kubeflow/kfserving/pkg/constants" "github.com/kubeflow/kfserving/pkg/utils" - - appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" ) // These constants are used for detecting and applying GPU selectors @@ -28,10 +27,10 @@ const ( NvidiaGPUTaintValue = "present" ) -func InjectGKEAcceleratorSelector(deployment *appsv1.Deployment) error { - if gpuSelector, ok := deployment.Annotations[constants.KFServiceGKEAcceleratorAnnotationKey]; ok { - deployment.Spec.Template.Spec.NodeSelector = utils.Union( - deployment.Spec.Template.Spec.NodeSelector, +func InjectGKEAcceleratorSelector(pod *v1.Pod) error { + if gpuSelector, ok := pod.Annotations[constants.KFServiceGKEAcceleratorAnnotationKey]; ok { + pod.Spec.NodeSelector = utils.Union( + pod.Spec.NodeSelector, map[string]string{GkeAcceleratorNodeSelector: gpuSelector}, ) } diff --git a/pkg/webhook/admission/deployment/accelerator_injector_test.go b/pkg/webhook/admission/pod/accelerator_injector_test.go similarity index 52% rename from pkg/webhook/admission/deployment/accelerator_injector_test.go rename to pkg/webhook/admission/pod/accelerator_injector_test.go index 955f3610090..fe773b65995 100644 --- a/pkg/webhook/admission/deployment/accelerator_injector_test.go +++ b/pkg/webhook/admission/pod/accelerator_injector_test.go @@ -13,14 +13,13 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -package deployment +package pod import ( "testing" "github.com/google/go-cmp/cmp" "github.com/kubeflow/kfserving/pkg/constants" - appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,83 +27,67 @@ import ( func TestAcceleratorInjector(t *testing.T) { scenarios := map[string]struct { - original *appsv1.Deployment - expected *appsv1.Deployment + original *v1.Pod + expected *v1.Pod }{ "AddGPUSelector": { - original: &appsv1.Deployment{ + original: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "deployment", Annotations: map[string]string{ constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-v100", }, }, - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{{ - Resources: v1.ResourceRequirements{ - Limits: v1.ResourceList{constants.NvidiaGPUResourceType: resource.MustParse("1")}, - }, - }}, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{constants.NvidiaGPUResourceType: resource.MustParse("1")}, }, - }, + }}, }, }, - expected: &appsv1.Deployment{ + expected: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "deployment", Annotations: map[string]string{ constants.KFServiceGKEAcceleratorAnnotationKey: "nvidia-tesla-v100", }, }, - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - NodeSelector: map[string]string{ - GkeAcceleratorNodeSelector: "nvidia-tesla-v100", - }, - Containers: []v1.Container{{ - Resources: v1.ResourceRequirements{ - Limits: v1.ResourceList{constants.NvidiaGPUResourceType: resource.MustParse("1")}, - }, - }}, - }, + Spec: v1.PodSpec{ + NodeSelector: map[string]string{ + GkeAcceleratorNodeSelector: "nvidia-tesla-v100", }, + Containers: []v1.Container{{ + Resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{constants.NvidiaGPUResourceType: resource.MustParse("1")}, + }, + }}, }, }, }, "DoNotAddGPUSelector": { - original: &appsv1.Deployment{ + original: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "deployment", }, - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{{ - Resources: v1.ResourceRequirements{ - Limits: v1.ResourceList{constants.NvidiaGPUResourceType: resource.MustParse("1")}, - }, - }}, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{constants.NvidiaGPUResourceType: resource.MustParse("1")}, }, - }, + }}, }, }, - expected: &appsv1.Deployment{ + expected: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "deployment", }, - Spec: appsv1.DeploymentSpec{ - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{{ - Resources: v1.ResourceRequirements{ - Limits: v1.ResourceList{constants.NvidiaGPUResourceType: resource.MustParse("1")}, - }, - }}, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{constants.NvidiaGPUResourceType: resource.MustParse("1")}, }, - }, + }}, }, }, }, @@ -114,17 +97,16 @@ func TestAcceleratorInjector(t *testing.T) { InjectGKEAcceleratorSelector(scenario.original) // cmd.Diff complains on ResourceList when Nvidia is key. Objects are explicitly compared if diff := cmp.Diff( - scenario.expected.Spec.Template.Spec.NodeSelector, - scenario.original.Spec.Template.Spec.NodeSelector, + scenario.expected.Spec.NodeSelector, + scenario.original.Spec.NodeSelector, ); diff != "" { t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) } if diff := cmp.Diff( - scenario.expected.Spec.Template.Spec.Tolerations, - scenario.original.Spec.Template.Spec.Tolerations, + scenario.expected.Spec.Tolerations, + scenario.original.Spec.Tolerations, ); diff != "" { t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) } - } } diff --git a/pkg/webhook/admission/deployment/mutator.go b/pkg/webhook/admission/pod/mutator.go similarity index 79% rename from pkg/webhook/admission/deployment/mutator.go rename to pkg/webhook/admission/pod/mutator.go index a7706849994..5a49cd4b6b9 100644 --- a/pkg/webhook/admission/deployment/mutator.go +++ b/pkg/webhook/admission/pod/mutator.go @@ -13,7 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -package deployment +package pod import ( "context" @@ -21,11 +21,9 @@ import ( "fmt" "net/http" - logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" - - appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" k8types "k8s.io/apimachinery/pkg/types" + "k8s.io/klog" "github.com/kubeflow/kfserving/pkg/constants" "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials" @@ -35,8 +33,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) -var log = logf.Log.WithName("kfserving-admission-mutator") - // Mutator is a webhook that injects incoming pods type Mutator struct { Client client.Client @@ -45,24 +41,24 @@ type Mutator struct { // Handle decodes the incoming Pod and executes mutation logic. func (mutator *Mutator) Handle(ctx context.Context, req types.Request) types.Response { - deployment := &appsv1.Deployment{} + pod := &v1.Pod{} - if err := mutator.Decoder.Decode(req, deployment); err != nil { + if err := mutator.Decoder.Decode(req, pod); err != nil { return admission.ErrorResponse(http.StatusBadRequest, err) } configMap := &v1.ConfigMap{} err := mutator.Client.Get(context.TODO(), k8types.NamespacedName{Name: constants.KFServiceConfigMapName, Namespace: constants.KFServingNamespace}, configMap) if err != nil { - log.Error(err, "Failed to find config map", "name", constants.KFServiceConfigMapName) + klog.Error(err, "Failed to find config map", "name", constants.KFServiceConfigMapName) return admission.ErrorResponse(http.StatusInternalServerError, err) } - if err := mutator.mutate(deployment, configMap); err != nil { + if err := mutator.mutate(pod, configMap); err != nil { return admission.ErrorResponse(http.StatusInternalServerError, err) } - patch, err := json.Marshal(deployment) + patch, err := json.Marshal(pod) if err != nil { return admission.ErrorResponse(http.StatusInternalServerError, err) } @@ -70,7 +66,7 @@ func (mutator *Mutator) Handle(ctx context.Context, req types.Request) types.Res return third_party.PatchResponseFromRaw(req.AdmissionRequest.Object.Raw, patch) } -func (mutator *Mutator) mutate(deployment *appsv1.Deployment, configMap *v1.ConfigMap) error { +func (mutator *Mutator) mutate(pod *v1.Pod, configMap *v1.ConfigMap) error { credentialBuilder := credentials.NewCredentialBulder(mutator.Client, configMap) var storageInitializerConfig StorageInitializerConfig @@ -85,13 +81,13 @@ func (mutator *Mutator) mutate(deployment *appsv1.Deployment, configMap *v1.Conf config: &storageInitializerConfig, } - mutators := []func(deployment *appsv1.Deployment) error{ + mutators := []func(pod *v1.Pod) error{ InjectGKEAcceleratorSelector, storageInitializer.InjectStorageInitializer, } for _, mutator := range mutators { - if err := mutator(deployment); err != nil { + if err := mutator(pod); err != nil { return err } } diff --git a/pkg/webhook/admission/deployment/storage_initializer_injector.go b/pkg/webhook/admission/pod/storage_initializer_injector.go similarity index 89% rename from pkg/webhook/admission/deployment/storage_initializer_injector.go rename to pkg/webhook/admission/pod/storage_initializer_injector.go index 9ccaead14bd..634ce967e4c 100644 --- a/pkg/webhook/admission/deployment/storage_initializer_injector.go +++ b/pkg/webhook/admission/pod/storage_initializer_injector.go @@ -11,7 +11,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package deployment +package pod import ( "fmt" @@ -20,7 +20,6 @@ import ( "github.com/kubeflow/kfserving/pkg/constants" "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials" - appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" ) @@ -48,19 +47,15 @@ type StorageInitializerInjector struct { // 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 // support INIT containers: https://github.com/knative/serving/issues/4307 -func (mi *StorageInitializerInjector) InjectStorageInitializer(deployment *appsv1.Deployment) error { - - annotations := deployment.Spec.Template.ObjectMeta.Annotations - podSpec := &deployment.Spec.Template.Spec - +func (mi *StorageInitializerInjector) InjectStorageInitializer(pod *v1.Pod) error { // Only inject if the required annotations are set - srcURI, ok := annotations[constants.StorageInitializerSourceUriInternalAnnotationKey] + srcURI, ok := pod.ObjectMeta.Annotations[constants.StorageInitializerSourceUriInternalAnnotationKey] if !ok { return nil } // Dont inject if InitContianer already injected - for _, container := range podSpec.InitContainers { + for _, container := range pod.Spec.InitContainers { if strings.Compare(container.Name, StorageInitializerContainerName) == 0 { return nil } @@ -68,9 +63,9 @@ func (mi *StorageInitializerInjector) InjectStorageInitializer(deployment *appsv // Find the knative user-container (this is the model inference server) var userContainer *v1.Container - for idx, container := range podSpec.Containers { + for idx, container := range pod.Spec.Containers { if strings.Compare(container.Name, UserContainerName) == 0 { - userContainer = &podSpec.Containers[idx] + userContainer = &pod.Spec.Containers[idx] break } } @@ -161,20 +156,20 @@ func (mi *StorageInitializerInjector) InjectStorageInitializer(deployment *appsv } // Add volumes to the PodSpec - podSpec.Volumes = append(podSpec.Volumes, podVolumes...) + pod.Spec.Volumes = append(pod.Spec.Volumes, podVolumes...) // Inject credentials if err := mi.credentialBuilder.CreateSecretVolumeAndEnv( - deployment.Namespace, - podSpec.ServiceAccountName, + pod.Namespace, + pod.Spec.ServiceAccountName, initContainer, - &podSpec.Volumes, + &pod.Spec.Volumes, ); err != nil { return err } // Add init container to the spec - podSpec.InitContainers = append(podSpec.InitContainers, *initContainer) + pod.Spec.InitContainers = append(pod.Spec.InitContainers, *initContainer) return nil } diff --git a/pkg/webhook/admission/pod/storage_initializer_injector_test.go b/pkg/webhook/admission/pod/storage_initializer_injector_test.go new file mode 100644 index 00000000000..b1966ea4ec0 --- /dev/null +++ b/pkg/webhook/admission/pod/storage_initializer_injector_test.go @@ -0,0 +1,726 @@ +/* +Copyright 2019 kubeflow.org. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package pod + +import ( + "context" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/kubeflow/kfserving/pkg/constants" + "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials" + "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials/gcs" + "github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials/s3" + "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestStorageInitializerInjector(t *testing.T) { + scenarios := map[string]struct { + original *v1.Pod + expected *v1.Pod + }{ + "MissingAnnotations": { + original: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + }, + }, + }, + }, + expected: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + }, + }, + }, + }, + }, + "AlreadyInjected": { + original: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + }, + }, + InitContainers: []v1.Container{ + { + Name: "storage-initializer", + }, + }, + }, + }, + expected: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + }, + }, + InitContainers: []v1.Container{ + { + Name: "storage-initializer", + }, + }, + }, + }, + }, + "StorageInitializerInjected": { + original: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + }, + }, + }, + }, + expected: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + ReadOnly: true, + }, + }, + }, + }, + InitContainers: []v1.Container{ + { + Name: "storage-initializer", + Image: StorageInitializerContainerImage + ":" + StorageInitializerContainerImageVersion, + Args: []string{"gs://foo", constants.DefaultModelLocalMountPath}, + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + Volumes: []v1.Volume{ + { + Name: "kfserving-provision-location", + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{}, + }, + }, + }, + }, + }, + }, + "StorageInitializerInjectedAndMountsPvc": { + original: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + }, + }, + }, + }, + expected: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + ReadOnly: true, + }, + }, + }, + }, + InitContainers: []v1.Container{ + { + Name: "storage-initializer", + Image: StorageInitializerContainerImage + ":" + StorageInitializerContainerImageVersion, + Args: []string{"/mnt/pvc/some/path/on/pvc", constants.DefaultModelLocalMountPath}, + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-pvc-source", + MountPath: "/mnt/pvc", + ReadOnly: true, + }, + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + Volumes: []v1.Volume{ + { + Name: "kfserving-pvc-source", + VolumeSource: v1.VolumeSource{ + PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvcname", + ReadOnly: false, + }, + }, + }, + { + Name: "kfserving-provision-location", + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{}, + }, + }, + }, + }, + }, + }, + } + + for name, scenario := range scenarios { + injector := &StorageInitializerInjector{ + credentialBuilder: credentials.NewCredentialBulder(c, &v1.ConfigMap{ + Data: map[string]string{}, + }), + } + if err := injector.InjectStorageInitializer(scenario.original); err != nil { + t.Errorf("Test %q unexpected result: %s", name, err) + } + if diff := cmp.Diff(scenario.expected.Spec, scenario.original.Spec); diff != "" { + t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) + } + } +} + +func TestStorageInitializerFailureCases(t *testing.T) { + scenarios := map[string]struct { + original *v1.Pod + expectedErrorPrefix string + }{ + "MissingUserContainer": { + original: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "random-container", + }, + }, + }, + }, + expectedErrorPrefix: "Invalid configuration: cannot find container", + }, + } + + for name, scenario := range scenarios { + injector := &StorageInitializerInjector{ + credentialBuilder: credentials.NewCredentialBulder(c, &v1.ConfigMap{ + Data: map[string]string{}, + }), + } + if err := injector.InjectStorageInitializer(scenario.original); err != nil { + if !strings.HasPrefix(err.Error(), scenario.expectedErrorPrefix) { + t.Errorf("Test %q unexpected failure [%s], expected: %s", name, err.Error(), scenario.expectedErrorPrefix) + } + } else { + t.Errorf("Test %q should have failed with: %s", name, scenario.expectedErrorPrefix) + } + } +} + +func TestCustomSpecStorageUriInjection(t *testing.T) { + scenarios := map[string]struct { + original *v1.Pod + expectedStorageUriEnvVariable *v1.EnvVar + }{ + "CustomSpecStorageUriSet": { + original: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + v1.Container{ + Name: "user-container", + Env: []v1.EnvVar{ + v1.EnvVar{ + Name: constants.CustomSpecStorageUriEnvVarKey, + Value: "pvc://mypvcname/some/path/on/pvc", + }, + }, + }, + }, + }, + }, + expectedStorageUriEnvVariable: &v1.EnvVar{ + Name: constants.CustomSpecStorageUriEnvVarKey, + Value: constants.DefaultModelLocalMountPath, + }, + }, + "CustomSpecStorageUriEmpty": { + original: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + v1.Container{ + Name: "user-container", + Env: []v1.EnvVar{ + v1.EnvVar{ + Name: constants.CustomSpecStorageUriEnvVarKey, + Value: "", + }, + }, + }, + }, + }, + }, + expectedStorageUriEnvVariable: &v1.EnvVar{ + Name: constants.CustomSpecStorageUriEnvVarKey, + Value: "", + }, + }, + "CustomSpecStorageUriNotSet": { + original: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "pvc://mypvcname/some/path/on/pvc", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + v1.Container{ + Name: "user-container", + Env: []v1.EnvVar{ + v1.EnvVar{ + Name: "TestRandom", + Value: "val", + }, + }, + }, + }, + }, + }, + expectedStorageUriEnvVariable: nil, + }, + } + + for name, scenario := range scenarios { + injector := &StorageInitializerInjector{ + credentialBuilder: credentials.NewCredentialBulder(c, &v1.ConfigMap{ + Data: map[string]string{}, + }), + } + if err := injector.InjectStorageInitializer(scenario.original); err != nil { + t.Errorf("Test %q unexpected result: %s", name, err) + } + + var originalEnvVar *v1.EnvVar + for _, envVar := range scenario.original.Spec.Containers[0].Env { + if envVar.Name == constants.CustomSpecStorageUriEnvVarKey { + originalEnvVar = &envVar + } + } + if diff := cmp.Diff(scenario.expectedStorageUriEnvVariable, originalEnvVar); diff != "" { + t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) + } + } +} + +func makePod() *v1.Pod { + return &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + }, + }, + }, + } +} + +func TestCredentialInjection(t *testing.T) { + g := gomega.NewGomegaWithT(t) + scenarios := map[string]struct { + sa *v1.ServiceAccount + secret *v1.Secret + original *v1.Pod + expected *v1.Pod + }{ + "Test s3 secrets injection": { + sa: &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: "default", + }, + Secrets: []v1.ObjectReference{ + { + Name: "s3-secret", + Namespace: "default", + }, + }, + }, + secret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "s3-secret", + Namespace: "default", + Annotations: map[string]string{ + s3.KFServiceS3SecretEndpointAnnotation: "s3.aws.com", + }, + }, + Data: map[string][]byte{ + "awsAccessKeyID": {}, + "awsSecretAccessKey": {}, + }, + }, + original: makePod(), + expected: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + ReadOnly: true, + }, + }, + }, + }, + InitContainers: []v1.Container{ + { + Name: "storage-initializer", + Image: StorageInitializerContainerImage + ":" + StorageInitializerContainerImageVersion, + Args: []string{"gs://foo", constants.DefaultModelLocalMountPath}, + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + }, + }, + Env: []v1.EnvVar{ + { + Name: s3.AWSAccessKeyId, + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "s3-secret", + }, + Key: "awsAccessKeyID", + }, + }, + }, + { + Name: s3.AWSSecretAccessKey, + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "s3-secret", + }, + Key: "awsSecretAccessKey", + }, + }, + }, + { + Name: s3.S3Endpoint, + Value: "s3.aws.com", + }, + { + Name: s3.AWSEndpointUrl, + Value: "https://s3.aws.com", + }, + }, + }, + }, + Volumes: []v1.Volume{ + v1.Volume{ + Name: "kfserving-provision-location", + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{}, + }, + }, + }, + }, + }, + }, + "Test GCS secrets injection": { + sa: &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: "default", + }, + Secrets: []v1.ObjectReference{ + { + Name: "user-gcp-sa", + Namespace: "default", + }, + }, + }, + secret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "user-gcp-sa", + Namespace: "default", + }, + Data: map[string][]byte{ + "gcloud-application-credentials.json": {}, + }, + }, + original: makePod(), + expected: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + v1.Container{ + Name: "user-container", + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + ReadOnly: true, + }, + }, + }, + }, + InitContainers: []v1.Container{ + { + Name: "storage-initializer", + Image: StorageInitializerContainerImage + ":" + StorageInitializerContainerImageVersion, + Args: []string{"gs://foo", constants.DefaultModelLocalMountPath}, + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + }, + { + Name: gcs.GCSCredentialVolumeName, + ReadOnly: true, + MountPath: gcs.GCSCredentialVolumeMountPath, + }, + }, + Env: []v1.EnvVar{ + { + Name: gcs.GCSCredentialEnvKey, + Value: gcs.GCSCredentialVolumeMountPath + "gcloud-application-credentials.json", + }, + }, + }, + }, + Volumes: []v1.Volume{ + { + Name: "kfserving-provision-location", + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{}, + }, + }, + { + Name: gcs.GCSCredentialVolumeName, + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: "user-gcp-sa", + }, + }, + }, + }, + }, + }, + }, + } + + var configMap = &v1.ConfigMap{ + Data: map[string]string{ + "credentials": `{ + "gcs" : {"gcsCredentialFileName": "gcloud-application-credentials.json"}, + "s3" : { + "s3AccessKeyIDName": "awsAccessKeyID", + "s3SecretAccessKeyName": "awsSecretAccessKey" + } + }`, + }, + } + + builder := credentials.NewCredentialBulder(c, configMap) + for name, scenario := range scenarios { + g.Expect(c.Create(context.TODO(), scenario.sa)).NotTo(gomega.HaveOccurred()) + g.Expect(c.Create(context.TODO(), scenario.secret)).NotTo(gomega.HaveOccurred()) + + injector := &StorageInitializerInjector{ + credentialBuilder: builder, + } + if err := injector.InjectStorageInitializer(scenario.original); err != nil { + t.Errorf("Test %q unexpected failure [%s]", name, err.Error()) + } + if diff := cmp.Diff(scenario.expected.Spec, scenario.original.Spec); diff != "" { + t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) + } + + g.Expect(c.Delete(context.TODO(), scenario.sa)).NotTo(gomega.HaveOccurred()) + g.Expect(c.Delete(context.TODO(), scenario.secret)).NotTo(gomega.HaveOccurred()) + } +} + +func TestStorageInitializerConfigmap(t *testing.T) { + scenarios := map[string]struct { + original *v1.Pod + expected *v1.Pod + }{ + "StorageInitializerConfig": { + original: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + }, + }, + }, + }, + expected: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + constants.StorageInitializerSourceUriInternalAnnotationKey: "gs://foo", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "user-container", + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + ReadOnly: true, + }, + }, + }, + }, + InitContainers: []v1.Container{ + { + Name: "storage-initializer", + Image: "kfserving/storage-initializer@sha256:xxx", + Args: []string{"gs://foo", constants.DefaultModelLocalMountPath}, + VolumeMounts: []v1.VolumeMount{ + { + Name: "kfserving-provision-location", + MountPath: constants.DefaultModelLocalMountPath, + }, + }, + }, + }, + Volumes: []v1.Volume{ + { + Name: "kfserving-provision-location", + VolumeSource: v1.VolumeSource{ + EmptyDir: &v1.EmptyDirVolumeSource{}, + }, + }, + }, + }, + }, + }, + } + + for name, scenario := range scenarios { + injector := &StorageInitializerInjector{ + credentialBuilder: credentials.NewCredentialBulder(c, &v1.ConfigMap{ + Data: map[string]string{}, + }), + config: &StorageInitializerConfig{ + Image: "kfserving/storage-initializer@sha256:xxx", + }, + } + if err := injector.InjectStorageInitializer(scenario.original); err != nil { + t.Errorf("Test %q unexpected result: %s", name, err) + } + if diff := cmp.Diff(scenario.expected.Spec, scenario.original.Spec); diff != "" { + t.Errorf("Test %q unexpected result (-want +got): %v", name, diff) + } + } +} diff --git a/pkg/webhook/admission/deployment/suite_test.go b/pkg/webhook/admission/pod/suite_test.go similarity index 98% rename from pkg/webhook/admission/deployment/suite_test.go rename to pkg/webhook/admission/pod/suite_test.go index 752016de664..418547e32b5 100644 --- a/pkg/webhook/admission/deployment/suite_test.go +++ b/pkg/webhook/admission/pod/suite_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package deployment +package pod import ( "os" diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 38fd3cff5ea..315358d7e1f 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -19,10 +19,10 @@ package webhook import ( "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha2" "github.com/kubeflow/kfserving/pkg/constants" - "github.com/kubeflow/kfserving/pkg/webhook/admission/deployment" "github.com/kubeflow/kfserving/pkg/webhook/admission/kfservice" + "github.com/kubeflow/kfserving/pkg/webhook/admission/pod" "k8s.io/api/admissionregistration/v1beta1" - v1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -116,7 +116,7 @@ func register(manager manager.Manager, server *webhook.Server) error { }, }, }, &admission.Webhook{ - Name: constants.DeploymentMutatorWebhookName, + Name: constants.PodMutatorWebhookName, FailurePolicy: &constants.WebhookFailurePolicy, Type: webhooktypes.WebhookTypeMutating, Rules: []v1beta1.RuleWithOperations{{ @@ -127,11 +127,11 @@ func register(manager manager.Manager, server *webhook.Server) error { Rule: v1beta1.Rule{ APIGroups: []string{v1.GroupName}, APIVersions: []string{v1.SchemeGroupVersion.Version}, - Resources: []string{"deployments"}, + Resources: []string{"pods"}, }, }}, Handlers: []admission.Handler{ - &deployment.Mutator{ + &pod.Mutator{ Client: manager.GetClient(), Decoder: manager.GetAdmissionDecoder(), },