From 0bc36e0a1717516a31bb7ca80ef3e01cee2c1664 Mon Sep 17 00:00:00 2001 From: Lize Cai Date: Mon, 7 Nov 2022 02:00:08 +0800 Subject: [PATCH] modelmesh supports image pull secrets Signed-off-by: Lize Cai --- ...ving.kserve.io_clusterservingruntimes.yaml | 9 ++- .../serving.kserve.io_servingruntimes.yaml | 9 ++- controllers/modelmesh/modelmesh.go | 4 +- controllers/modelmesh/runtime.go | 11 +++ controllers/modelmesh/runtime_test.go | 80 +++++++++++++++++++ controllers/modelmesh/util.go | 23 ++++++ controllers/servingruntime_controller.go | 1 + docs/configuration/README.md | 1 + go.mod | 2 +- go.sum | 2 + pkg/config/config.go | 2 + pkg/config/config_test.go | 18 +++++ 12 files changed, 158 insertions(+), 4 deletions(-) diff --git a/config/crd/bases/serving.kserve.io_clusterservingruntimes.yaml b/config/crd/bases/serving.kserve.io_clusterservingruntimes.yaml index c33c6f110..753cc15e5 100644 --- a/config/crd/bases/serving.kserve.io_clusterservingruntimes.yaml +++ b/config/crd/bases/serving.kserve.io_clusterservingruntimes.yaml @@ -1,4 +1,4 @@ -# Copied from https://github.com/kserve/kserve/blob/532989a2ccef63ff912008359540b0937596f55c/config/crd/serving.kserve.io_clusterservingruntimes.yaml +# Copied from https://github.com/kserve/kserve/blob/335dfbcc461a3b2127354aeb7df2414fb11ddfe3/config/crd/serving.kserve.io_clusterservingruntimes.yaml # apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition @@ -1065,6 +1065,13 @@ spec: type: string httpDataEndpoint: type: string + imagePullSecrets: + items: + properties: + name: + type: string + type: object + type: array labels: additionalProperties: type: string diff --git a/config/crd/bases/serving.kserve.io_servingruntimes.yaml b/config/crd/bases/serving.kserve.io_servingruntimes.yaml index 5b5284e20..cc96fb43b 100644 --- a/config/crd/bases/serving.kserve.io_servingruntimes.yaml +++ b/config/crd/bases/serving.kserve.io_servingruntimes.yaml @@ -1,4 +1,4 @@ -# Copied from https://github.com/kserve/kserve/blob/532989a2ccef63ff912008359540b0937596f55c/config/crd/serving.kserve.io_servingruntimes.yaml +# Copied from https://github.com/kserve/kserve/blob/335dfbcc461a3b2127354aeb7df2414fb11ddfe3/config/crd/serving.kserve.io_servingruntimes.yaml # apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition @@ -1065,6 +1065,13 @@ spec: type: string httpDataEndpoint: type: string + imagePullSecrets: + items: + properties: + name: + type: string + type: object + type: array labels: additionalProperties: type: string diff --git a/controllers/modelmesh/modelmesh.go b/controllers/modelmesh/modelmesh.go index bb020a2b5..ea0e70bc8 100644 --- a/controllers/modelmesh/modelmesh.go +++ b/controllers/modelmesh/modelmesh.go @@ -34,7 +34,7 @@ const logYaml = false const ModelMeshEtcdPrefix = "mm" -//Models a deployment +// Models a deployment type Deployment struct { ServiceName string ServicePort uint16 @@ -73,6 +73,7 @@ type Deployment struct { AnnotationConfigMap *corev1.ConfigMap AnnotationsMap map[string]string LabelsMap map[string]string + ImagePullSecrets []corev1.LocalObjectReference EnableAccessLogging bool Client client.Client } @@ -126,6 +127,7 @@ func (m *Deployment) Apply(ctx context.Context) error { m.configureRuntimePodSpecAnnotations, m.configureRuntimePodSpecLabels, m.ensureMMContainerIsLast, + m.configureRuntimePodSpecImagePullSecrets, ); tErr != nil { return tErr } diff --git a/controllers/modelmesh/runtime.go b/controllers/modelmesh/runtime.go index 106570501..896f35a72 100644 --- a/controllers/modelmesh/runtime.go +++ b/controllers/modelmesh/runtime.go @@ -372,3 +372,14 @@ func (m *Deployment) configureRuntimePodSpecLabels(deployment *appsv1.Deployment deployment.Spec.Template.Labels = Union(deployment.Spec.Template.Labels, m.LabelsMap, m.SRSpec.ServingRuntimePodSpec.Labels) return nil } + +func (m *Deployment) configureRuntimePodSpecImagePullSecrets(deployment *appsv1.Deployment) error { + // merging the image pull secrets from deploymentTemplate (if any), configMap and ServingRuntimePodSpec + deployment.Spec.Template.Spec.ImagePullSecrets = mergeImagePullSecrets( + deployment.Spec.Template.Spec.ImagePullSecrets, + m.ImagePullSecrets, + m.SRSpec.ServingRuntimePodSpec.ImagePullSecrets, + ) + + return nil +} diff --git a/controllers/modelmesh/runtime_test.go b/controllers/modelmesh/runtime_test.go index e7f74cd91..ec7ace4f3 100644 --- a/controllers/modelmesh/runtime_test.go +++ b/controllers/modelmesh/runtime_test.go @@ -504,3 +504,83 @@ func TestConfigureRuntimeLabels(t *testing.T) { assert.Equal(t, deploy.Spec.Template.Labels["cp4s-internet"], "allow") }) } + +func TestConfigureRuntimeImagePullSecrets(t *testing.T) { + t.Run("success-set-image-pull-secrets", func(t *testing.T) { + deploy := &appsv1.Deployment{} + sr := &kserveapi.ServingRuntime{} + imagePullSecretsData := []v1.LocalObjectReference{ + {Name: "config-image-pull-secret-1"}, + {Name: "config-image-pull-secret-2"}, + } + + m := Deployment{Owner: sr, ImagePullSecrets: imagePullSecretsData, SRSpec: &sr.Spec} + + err := m.configureRuntimePodSpecImagePullSecrets(deploy) + assert.Nil(t, err) + // assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets, imagePullSecretsData) + assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets[0].Name, "config-image-pull-secret-1") + assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets[1].Name, "config-image-pull-secret-2") + }) + + t.Run("success-no-image-pull-secrets", func(t *testing.T) { + deploy := &appsv1.Deployment{} + sr := &kserveapi.ServingRuntime{} + m := Deployment{Owner: sr, SRSpec: &sr.Spec} + + err := m.configureRuntimePodSpecImagePullSecrets(deploy) + assert.Nil(t, err) + assert.Empty(t, deploy.Spec.Template.Spec.ImagePullSecrets) + }) + + t.Run("success-set-image-pull-secrets-from-servingruntime-spec", func(t *testing.T) { + deploy := &appsv1.Deployment{} + sr := &kserveapi.ServingRuntime{ + Spec: kserveapi.ServingRuntimeSpec{ + ServingRuntimePodSpec: kserveapi.ServingRuntimePodSpec{ + ImagePullSecrets: []v1.LocalObjectReference{ + {Name: "sr-image-pull-secret-1"}, + {Name: "sr-image-pull-secret-2"}, + }, + }, + }, + } + + m := Deployment{Owner: sr, SRSpec: &sr.Spec} + + err := m.configureRuntimePodSpecImagePullSecrets(deploy) + assert.Nil(t, err) + assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets[0].Name, "sr-image-pull-secret-1") + assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets[1].Name, "sr-image-pull-secret-2") + }) + + t.Run("success-set-image-pull-secrets-from-config-and-servingruntime-spec", func(t *testing.T) { + deploy := &appsv1.Deployment{} + imagePullSecretsData := []v1.LocalObjectReference{ + {Name: "config-image-pull-secret-1"}, + {Name: "config-image-pull-secret-2"}, + {Name: "overlap-image-pull-secret"}, + } + sr := &kserveapi.ServingRuntime{ + Spec: kserveapi.ServingRuntimeSpec{ + ServingRuntimePodSpec: kserveapi.ServingRuntimePodSpec{ + ImagePullSecrets: []v1.LocalObjectReference{ + {Name: "overlap-image-pull-secret"}, + {Name: "sr-image-pull-secret-1"}, + {Name: "sr-image-pull-secret-2"}, + }, + }, + }, + } + + m := Deployment{Owner: sr, ImagePullSecrets: imagePullSecretsData, SRSpec: &sr.Spec} + + err := m.configureRuntimePodSpecImagePullSecrets(deploy) + assert.Nil(t, err) + assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets[0].Name, "config-image-pull-secret-1") + assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets[1].Name, "config-image-pull-secret-2") + assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets[2].Name, "overlap-image-pull-secret") + assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets[3].Name, "sr-image-pull-secret-1") + assert.Equal(t, deploy.Spec.Template.Spec.ImagePullSecrets[4].Name, "sr-image-pull-secret-2") + }) +} diff --git a/controllers/modelmesh/util.go b/controllers/modelmesh/util.go index 8180e51e4..4342e1cfd 100644 --- a/controllers/modelmesh/util.go +++ b/controllers/modelmesh/util.go @@ -135,3 +135,26 @@ func mountPoint(rts *kserveapi.ServingRuntimeSpec) (bool, string, error) { return false, "", nil } + +// mergeImagePullSecrets merge image pull secret lists and remove duplicates +func mergeImagePullSecrets(secrets ...[]corev1.LocalObjectReference) []corev1.LocalObjectReference { + imagePullSecrets := []corev1.LocalObjectReference{} + + // remove the duplicated secrets and keep the order of the secret in the list + for _, secret := range secrets { + for _, v := range secret { + exist := false + for _, ips := range imagePullSecrets { + if ips.Name == v.Name { + exist = true + break + } + } + if !exist { + imagePullSecrets = append(imagePullSecrets, corev1.LocalObjectReference{Name: v.Name}) + } + } + } + + return imagePullSecrets +} diff --git a/controllers/servingruntime_controller.go b/controllers/servingruntime_controller.go index 389750627..d433cbad9 100644 --- a/controllers/servingruntime_controller.go +++ b/controllers/servingruntime_controller.go @@ -258,6 +258,7 @@ func (r *ServingRuntimeReconciler) Reconcile(ctx context.Context, req ctrl.Reque Client: r.Client, AnnotationsMap: cfg.RuntimePodAnnotations, LabelsMap: cfg.RuntimePodLabels, + ImagePullSecrets: cfg.ImagePullSecrets, } // if the runtime is disabled, delete the deployment if spec.IsDisabled() || !spec.IsMultiModelRuntime() || !mmEnabled { diff --git a/docs/configuration/README.md b/docs/configuration/README.md index dcb2f32cf..51fa9eb79 100644 --- a/docs/configuration/README.md +++ b/docs/configuration/README.md @@ -52,6 +52,7 @@ The following parameters are currently supported. _Note_ the keys are expressed | `restProxy.port` | Port on which the REST proxy to serve REST requests | `8008` | | `runtimePodLabels` | `metadata.labels` to be added to all `ServingRuntime` pods | (\*\*\*\*\*) See default labels below | | `runtimePodAnnotations` | `metadata.annotations` to be added to all `ServingRuntime` pods | (\*\*\*\*\*) See default annotations below | +| `imagePullSecrets` | The image pull secrets to use for runtime Pods | | (\*) Currently requires a controller restart to take effect diff --git a/go.mod b/go.mod index bcaf8811a..3c1317171 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/go-logr/logr v1.2.3 github.com/golang/protobuf v1.5.2 github.com/google/go-cmp v0.5.8 - github.com/kserve/kserve v0.9.1-0.20221002134029-532989a2ccef + github.com/kserve/kserve v0.9.1-0.20221013012311-335dfbcc461a github.com/manifestival/controller-runtime-client v0.4.0 github.com/manifestival/manifestival v0.7.1 github.com/moverest/mnist v0.0.0-20160628192128-ec5d9d203b59 diff --git a/go.sum b/go.sum index 91a792d2d..1b1ea0618 100644 --- a/go.sum +++ b/go.sum @@ -503,6 +503,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kserve/kserve v0.9.1-0.20221002134029-532989a2ccef h1:K3KVPI2w6D4L2VZVDIV+8ArYD7Bo4G1bzUXieQhHX+I= github.com/kserve/kserve v0.9.1-0.20221002134029-532989a2ccef/go.mod h1:OgRx84UYmcNeM4Q6uuxHHypQRc8J5v5caGsuyAsbAWQ= +github.com/kserve/kserve v0.9.1-0.20221013012311-335dfbcc461a h1:gJwJj2hiZ+D0+C4FE9bMwHof8WdWYBjZw5H6F9ACLbY= +github.com/kserve/kserve v0.9.1-0.20221013012311-335dfbcc461a/go.mod h1:vC6wtbGo6xh9a445wVTTmiZbbF3TbXutIjgSeeH19u4= github.com/logrusorgru/aurora/v3 v3.0.0 h1:R6zcoZZbvVcGMvDCKo45A9U/lzYyzl5NfYIvznmDfE4= github.com/logrusorgru/aurora/v3 v3.0.0/go.mod h1:vsR12bk5grlLvLXAYrBsb5Oc/N+LxAlxggSjiwMnCUc= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= diff --git a/pkg/config/config.go b/pkg/config/config.go index b55da5c3f..d6d509901 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -84,6 +84,8 @@ type Config struct { RuntimePodLabels map[string]string RuntimePodAnnotations map[string]string + ImagePullSecrets []corev1.LocalObjectReference + // For internal use only InternalModelMeshEnvVars EnvVarList } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 4c2fd2928..4b4458f81 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -293,3 +293,21 @@ internalModelMeshEnvVars: t.Fatalf("Expected InternalModelMeshEnvVars to have env var with value [%s], but got [%s]", expectedValue, envvar.Value) } } + +func TestImagePullSecrets(t *testing.T) { + yaml := ` +imagePullSecrets: + - name: "config-image-pull-secret" +` + expectedSecretName := "config-image-pull-secret" + + conf, err := NewMergedConfigFromString(yaml) + if err != nil { + t.Fatal(err) + } + + secret := conf.ImagePullSecrets[0] + if secret.Name != expectedSecretName { + t.Fatalf("Expected ImagePullSecrets to have secret with name [%s], but got [%s]", expectedSecretName, secret.Name) + } +}