fix: imagePullPolicy only changed to 'IfNotPresent' if 'Always' is specified.

integration/run_test.go

@@ -19,6 +19,7 @@ package integration
 import (
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -226,6 +227,58 @@ func TestRun(t *testing.T) {
 	}
 }
 
+func TestRunWithImagePullPolicy(t *testing.T) {
+	miniKubeRunArgs := []string{"image", "build", "testdata/image-pull-policy", "-f", "Dockerfile", "-t", "test-image:built-locally"}
+	imagePullPolicyTests := []struct {
+		description            string
+		skipBuildingLocalImage bool
+		dir                    string
+		pods                   []string
+		targetLog              string
+	}{
+		{
+			description: "'Never' image pull policy works if image is present locally",
+			dir:         "testdata/image-pull-policy/never",
+			pods:        []string{"getting-started"},
+		},
+		{
+			description: "'Always' doesn't actually pull and reads a local image that doesn't exist remotely",
+			dir:         "testdata/image-pull-policy/always",
+			pods:        []string{"getting-started"},
+		},
+		{
+			description:            "'IfNotPresent' pulls the remote image",
+			skipBuildingLocalImage: true,
+			dir:                    "testdata/image-pull-policy/if-not-present",
+			pods:                   []string{"getting-started"},
+		},
+	}
+	for _, test := range imagePullPolicyTests {
+		t.Run(test.description, func(t *testing.T) {
+			MarkIntegrationTest(t, CanRunWithoutGcp)
+
+			// kubectl uses the minikube context, so build the docker images within minikube
+			// to make them accessible.
+			if !test.skipBuildingLocalImage {
+				if err := exec.Command("minikube", miniKubeRunArgs...).Run(); err != nil {
+					t.Errorf("unexpected error while running minikube with args: %v, err: %v", miniKubeRunArgs, err)
+				}
+			}
+
+			ns, client := SetupNamespace(t)
+			args := []string{"--cache-artifacts=false"}
+			if test.dir == "" {
+				t.Fatalf("A skaffold directory is required")
+			}
+			skaffold.Run(args...).InDir(test.dir).InNs(ns.Name).RunOrFail(t)
+
+			client.WaitForPodsReady(test.pods...)
+
+			skaffold.Delete().InDir(test.dir).InNs(ns.Name).RunOrFail(t)
+		})
+	}
+}
+
 func TestRunTail(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
@@ -638,3 +691,7 @@ func TestRunKubectlDefaultNamespace(t *testing.T) {
 		})
 	}
 }
+
+func TestRunImagePullPolicy(t *testing.T) {
+
+}

integration/testdata/image-pull-policy/Dockerfile

@@ -0,0 +1,2 @@
+FROM gcr.io/google-containers/nginx@sha256:f49a843c290594dcf4d193535d1f4ba8af7d56cea2cf79d1e9554f077f1e7aaa
+RUN echo "hello world" > example.txt

integration/testdata/image-pull-policy/always/k8s-pod.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+  - name: nginx
+    image: test-image:built-locally
+    imagePullPolicy: Always

integration/testdata/image-pull-policy/always/skaffold.yaml

@@ -0,0 +1,7 @@
+apiVersion: skaffold/v4beta12
+kind: Config
+manifests:
+  rawYaml:
+  - k8s-*
+deploy:
+  kubectl: {}

integration/testdata/image-pull-policy/if-not-present/k8s-pod.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+  - name: nginx
+    image: gcr.io/google-containers/nginx@sha256:f49a843c290594dcf4d193535d1f4ba8af7d56cea2cf79d1e9554f077f1e7aaa
+    imagePullPolicy: IfNotPresent

integration/testdata/image-pull-policy/if-not-present/skaffold.yaml

@@ -0,0 +1,7 @@
+apiVersion: skaffold/v4beta12
+kind: Config
+manifests:
+  rawYaml:
+  - k8s-*
+deploy:
+  kubectl: {}

integration/testdata/image-pull-policy/never/k8s-pod.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+  - name: nginx
+    image: test-image:built-locally
+    imagePullPolicy: Never

integration/testdata/image-pull-policy/never/skaffold.yaml

@@ -0,0 +1,7 @@
+apiVersion: skaffold/v4beta12
+kind: Config
+manifests:
+  rawYaml:
+  - k8s-*
+deploy:
+  kubectl: {}

pkg/skaffold/kubernetes/manifest/image_pull_policy.go

@@ -18,6 +18,10 @@ package manifest
 
 import apimachinery "k8s.io/apimachinery/pkg/runtime/schema"
 
+const imagePullPolicyField = "imagePullPolicy"
+const imagePullPolicyAlways = "Always"
+const imagePullPolicyNever = "Never"
+
 // resourceSelectorImagePullPolicy selects PodSpecs for transforming the imagePullPolicy field
 // based on allowlist and denylist rules for their GroupKind and navigation path.
 type resourceSelectorImagePullPolicy struct{}
@@ -73,7 +77,6 @@ type imagePullPolicyReplacer struct{}
 
 // Visit sets the value of the "imagePullPolicy" field in a Kubernetes manifest to "Never".
 func (i *imagePullPolicyReplacer) Visit(gk apimachinery.GroupKind, navpath string, o map[string]interface{}, k string, v interface{}, rs ResourceSelector) bool {
-	const imagePullPolicyField = "imagePullPolicy"
 	if _, allowed := rs.allowByNavpath(gk, navpath, k); !allowed {
 		return true
 	}
@@ -83,6 +86,8 @@ func (i *imagePullPolicyReplacer) Visit(gk apimachinery.GroupKind, navpath strin
 	if _, ok := v.(string); !ok {
 		return true
 	}
-	o[imagePullPolicyField] = "Never"
+	if o[imagePullPolicyField] == imagePullPolicyAlways {
+		o[imagePullPolicyField] = imagePullPolicyNever
+	}
 	return false
 }

pkg/skaffold/kubernetes/manifest/image_pull_policy_test.go

@@ -50,7 +50,7 @@ spec:
   containers:
   - image: gcr.io/k8s-skaffold/example@sha256:81daf011d63b68cfa514ddab7741a1adddd59d3264118dfb0fd9266328bb8883
     name: if-not-present
-    imagePullPolicy: Never
+    imagePullPolicy: IfNotPresent
   - image: gcr.io/k8s-skaffold/example@sha256:81daf011d63b68cfa514ddab7741a1adddd59d3264118dfb0fd9266328bb8883
     name: always
     imagePullPolicy: Never