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

feat: Supports image pull secrets #276

Merged
Merged
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
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -1065,6 +1065,13 @@ spec:
type: string
httpDataEndpoint:
type: string
imagePullSecrets:
items:
properties:
name:
type: string
type: object
type: array
labels:
additionalProperties:
type: string
Expand Down
9 changes: 8 additions & 1 deletion config/crd/bases/serving.kserve.io_servingruntimes.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -1065,6 +1065,13 @@ spec:
type: string
httpDataEndpoint:
type: string
imagePullSecrets:
items:
properties:
name:
type: string
type: object
type: array
labels:
additionalProperties:
type: string
Expand Down
4 changes: 3 additions & 1 deletion controllers/modelmesh/modelmesh.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const logYaml = false

const ModelMeshEtcdPrefix = "mm"

//Models a deployment
// Models a deployment
type Deployment struct {
ServiceName string
ServicePort uint16
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -126,6 +127,7 @@ func (m *Deployment) Apply(ctx context.Context) error {
m.configureRuntimePodSpecAnnotations,
m.configureRuntimePodSpecLabels,
m.ensureMMContainerIsLast,
m.configureRuntimePodSpecImagePullSecrets,
); tErr != nil {
return tErr
}
Expand Down
11 changes: 11 additions & 0 deletions controllers/modelmesh/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
79 changes: 79 additions & 0 deletions controllers/modelmesh/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,3 +504,82 @@ 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[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")
})
}
23 changes: 23 additions & 0 deletions controllers/modelmesh/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
1 change: 1 addition & 0 deletions controllers/servingruntime_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions docs/configuration/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ type Config struct {
RuntimePodLabels map[string]string
RuntimePodAnnotations map[string]string

ImagePullSecrets []corev1.LocalObjectReference

// For internal use only
InternalModelMeshEnvVars EnvVarList
}
Expand Down
18 changes: 18 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}