fix(cache): update to use google/cloud-sdk:alpine instead of alpine f…

…or mutating cached steps. Fixes kubeflow#4099 (kubeflow#5184) * set up cluster * allow to set with env variable * fix the market place * updated manifests * Added default, still need to fix how env is set in test * test alpine agin * added test * updated the image * deleted * change image by misstake * updated after feedback * deleted * smaller image * added to the config.json * adjust to new updates for config handling * Updated image to use latest
algs · Mar 13, 2021 · 9c10d4f · 9c10d4f
1 parent 6ebff17
commit 9c10d4f
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 5 deletions.
diff --git a/backend/src/apiserver/config/config.json b/backend/src/apiserver/config/config.json
@@ -16,5 +16,7 @@
   "InitConnectionTimeout": "6m",
   "DefaultPipelineRunnerServiceAccount": "pipeline-runner",
   "CacheEnabled": "true",
-  "CRON_SCHEDULE_TIMEZONE": "UTC"
+  "CRON_SCHEDULE_TIMEZONE": "UTC",
+  "CACHE_IMAGE": "gcr.io/google-containers/busybox"
+
 }
diff --git a/backend/src/cache/server/mutation.go b/backend/src/cache/server/mutation.go
@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"os"
 	"strconv"
 	"strings"
 
@@ -133,9 +134,15 @@ func MutatePodIfCached(req *v1beta1.AdmissionRequest, clientMgr ClientManagerInt
 		labels[MetadataExecutionIDKey] = getValueFromSerializedMap(cachedExecution.ExecutionOutput, MetadataExecutionIDKey)
 		labels[MetadataWrittenKey] = "true"
 
+		// Image selected from Google Container Register(gcr) for it small size, gcr since there
+		// is not image pull rate limit. For more info see issue: https://github.com/kubeflow/pipelines/issues/4099
+		image := "gcr.io/google-containers/busybox"
+		if v, ok := os.LookupEnv("CACHE_IMAGE"); ok {
+			image = v
+		}
 		dummyContainer := corev1.Container{
 			Name:    "main",
-			Image:   "alpine",
+			Image:   image,
 			Command: []string{`echo`, `"This step output is taken from cache."`},
 		}
 		dummyContainers := []corev1.Container{

diff --git a/backend/src/cache/server/mutation_test.go b/backend/src/cache/server/mutation_test.go
@@ -17,6 +17,7 @@ package server
 import (
 	"bytes"
 	"encoding/json"
+	"os"
 	"testing"
 
 	"github.com/kubeflow/pipelines/backend/src/cache/model"
@@ -152,13 +153,48 @@ func TestMutatePodIfCachedWithCacheEntryExist(t *testing.T) {
 
 	patchOperation, err := MutatePodIfCached(&fakeAdmissionRequest, fakeClientManager)
 	assert.Nil(t, err)
+
 	require.NotNil(t, patchOperation)
 	require.Equal(t, 3, len(patchOperation))
 	require.Equal(t, patchOperation[0].Op, OperationTypeReplace)
 	require.Equal(t, patchOperation[1].Op, OperationTypeAdd)
 	require.Equal(t, patchOperation[2].Op, OperationTypeAdd)
 }
 
+func TestDefaultImage(t *testing.T) {
+	executionCache := &model.ExecutionCache{
+		ExecutionCacheKey: "f5fe913be7a4516ebfe1b5de29bcb35edd12ecc776b2f33f10ca19709ea3b2f0",
+		ExecutionOutput:   "testOutput",
+		ExecutionTemplate: `{"container":{"command":["echo", "Hello"],"image":"python:3.7"}}`,
+		MaxCacheStaleness: -1,
+	}
+	fakeClientManager.CacheStore().CreateExecutionCache(executionCache)
+
+	patchOperation, err := MutatePodIfCached(&fakeAdmissionRequest, fakeClientManager)
+	assert.Nil(t, err)
+	container := patchOperation[0].Value.([]corev1.Container)[0]
+	require.Equal(t, "gcr.io/google-containers/busybox", container.Image)
+}
+
+func TestSetImage(t *testing.T) {
+	testImage := "testimage"
+	os.Setenv("CACHE_IMAGE", testImage)
+	defer os.Unsetenv("CACHE_IMAGE")
+
+	executionCache := &model.ExecutionCache{
+		ExecutionCacheKey: "f5fe913be7a4516ebfe1b5de29bcb35edd12ecc776b2f33f10ca19709ea3b2f0",
+		ExecutionOutput:   "testOutput",
+		ExecutionTemplate: `{"container":{"command":["echo", "Hello"],"image":"python:3.7"}}`,
+		MaxCacheStaleness: -1,
+	}
+	fakeClientManager.CacheStore().CreateExecutionCache(executionCache)
+
+	patchOperation, err := MutatePodIfCached(&fakeAdmissionRequest, fakeClientManager)
+	assert.Nil(t, err)
+	container := patchOperation[0].Value.([]corev1.Container)[0]
+	assert.Equal(t, testImage, container.Image)
+}
+
 func TestMutatePodIfCachedWithTeamplateCleanup(t *testing.T) {
 	executionCache := &model.ExecutionCache{
 		ExecutionCacheKey: "f5fe913be7a4516ebfe1b5de29bcb35edd12ecc776b2f33f10ca19709ea3b2f0",

diff --git a/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/cache.yaml b/manifests/gcp_marketplace/chart/kubeflow-pipelines/templates/cache.yaml
@@ -190,6 +190,7 @@ data:
   mysql_driver: "mysql"
   mysql_host: "mysql"
   mysql_port: "3306"
+  cache_image: "gcr.io/google-containers/busybox"
 ---
 apiVersion: apps/v1
 kind: Deployment
@@ -231,6 +232,11 @@ spec:
           - name: DBCONFIG_PASSWORD
             value: ''
           {{ end }}
+          - name: CACHE_IMAGE
+            valueFrom:
+              configMapKeyRef:
+                name: cache-configmap
+                key: cache_image
           - name: DBCONFIG_DRIVER
             valueFrom:
               configMapKeyRef:

diff --git a/manifests/gcp_marketplace/test/snapshot-base.yaml b/manifests/gcp_marketplace/test/snapshot-base.yaml
@@ -221,6 +221,7 @@ data:
   mysql_driver: "mysql"
   mysql_host: "mysql"
   mysql_port: "3306"
+  cache_image: "gcr.io/google-containers/busybox"
 ---
 apiVersion: apps/v1
 kind: Deployment
@@ -250,7 +251,11 @@ spec:
             value: 'root'
           - name: DBCONFIG_PASSWORD
             value: ''
-
+          - name: CACHE_IMAGE
+            valueFrom:
+              configMapKeyRef:
+                name: cache-configmap
+                key: cache_image
           - name: DBCONFIG_DRIVER
             valueFrom:
               configMapKeyRef:

diff --git a/manifests/gcp_marketplace/test/snapshot-managed-storage-with-db-prefix.yaml b/manifests/gcp_marketplace/test/snapshot-managed-storage-with-db-prefix.yaml
@@ -229,6 +229,7 @@ data:
   mysql_driver: "mysql"
   mysql_host: "mysql"
   mysql_port: "3306"
+  cache_image: "gcr.io/google-containers/busybox"
 ---
 apiVersion: apps/v1
 kind: Deployment
@@ -263,8 +264,12 @@ spec:
             valueFrom:
               secretKeyRef:
                 name: mysql-credential
-                key: password
-
+                key: password        
+          - name: CACHE_IMAGE
+            valueFrom:
+              configMapKeyRef:
+                name: cache-configmap
+                key: cache_image
           - name: DBCONFIG_DRIVER
             valueFrom:
               configMapKeyRef:

diff --git a/manifests/kustomize/base/cache/cache-deployment.yaml b/manifests/kustomize/base/cache/cache-deployment.yaml
@@ -18,6 +18,11 @@ spec:
       - name: server
         image: gcr.io/ml-pipeline/cache-server:dummy
         env:
+          - name: CACHE_IMAGE
+            valueFrom:
+              configMapKeyRef:
+                name: pipeline-install-config
+                key: cacheImage
           - name: DBCONFIG_DRIVER
             value: mysql
           - name: DBCONFIG_DB_NAME

diff --git a/manifests/kustomize/base/installs/generic/pipeline-install-config.yaml b/manifests/kustomize/base/installs/generic/pipeline-install-config.yaml
@@ -24,3 +24,9 @@ data:
   ## Feature stage:
   ## [Alpha](https://github.com/kubeflow/pipelines/blob/07328e5094ac2981d3059314cc848fbb71437a76/docs/release/feature-stages.md#alpha)
   cronScheduleTimezone: "UTC"
+
+  ## cacheImage is the image that the mutating webhook will use to patch
+  ## cached steps with. Will be used to echo a message announcing that 
+  ## the cached step result will be used. If not set it will default to 
+  ## 'gcr.io/google-containers/busybox'
+  cacheImage: "gcr.io/google-containers/busybox"