add image pull policy (#1462)

* add image pull policy Signed-off-by: Filinto Duran <[email protected]> * add allowed values Signed-off-by: Filinto Duran <[email protected]> * feedback refactor allowed values name Signed-off-by: Filinto Duran <[email protected]> * add unit tests Signed-off-by: Filinto Duran <[email protected]> * lint Signed-off-by: Filinto Duran <[email protected]> * lint Signed-off-by: Filinto Duran <[email protected]> * more lint Signed-off-by: Filinto Duran <[email protected]> * more lint Signed-off-by: Filinto Duran <[email protected]> --------- Signed-off-by: Filinto Duran <[email protected]> Co-authored-by: Anton Troshin <[email protected]> Co-authored-by: Mike Nguyen <[email protected]>
dapr · Jan 7, 2025 · efe1d6c · efe1d6c
1 parent dbbe022
commit efe1d6c
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 3 deletions.
diff --git a/pkg/kubernetes/run.go b/pkg/kubernetes/run.go
@@ -297,7 +297,7 @@ func createDeploymentConfig(client versioned.Interface, app runfileconfig.App) d
 						Name:            app.AppID,
 						Image:           app.ContainerImage,
 						Env:             getEnv(app),
-						ImagePullPolicy: corev1.PullAlways,
+						ImagePullPolicy: corev1.PullPolicy(app.ContainerImagePullPolicy),
 					},
 				},
 			},

diff --git a/pkg/runfileconfig/run_file_config.go b/pkg/runfileconfig/run_file_config.go
@@ -41,8 +41,9 @@ type RunFileConfig struct {
 
 // ContainerConfiguration represents the application container configuration parameters.
 type ContainerConfiguration struct {
-	ContainerImage string `yaml:"containerImage"`
-	CreateService  bool   `yaml:"createService"`
+	ContainerImage           string `yaml:"containerImage"`
+	ContainerImagePullPolicy string `yaml:"containerImagePullPolicy"`
+	CreateService            bool   `yaml:"createService"`
 }
 
 // App represents the configuration options for the apps in the run file.

diff --git a/pkg/runfileconfig/run_file_config_parser.go b/pkg/runfileconfig/run_file_config_parser.go
@@ -26,6 +26,8 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
+var imagePullPolicyValuesAllowed = []string{"Always", "Never", "IfNotPresent"}
+
 // Parse the provided run file into a RunFileConfig struct.
 func (a *RunFileConfig) parseAppsConfig(runFilePath string) error {
 	var err error
@@ -97,6 +99,15 @@ func (a *RunFileConfig) validateRunConfig(runFilePath string) error {
 		if len(strings.TrimSpace(a.Apps[i].ResourcesPath)) > 0 {
 			a.Apps[i].ResourcesPaths = append(a.Apps[i].ResourcesPaths, a.Apps[i].ResourcesPath)
 		}
+
+		// Check containerImagePullPolicy is valid.
+		if a.Apps[i].ContainerImagePullPolicy != "" {
+			if !utils.Contains(imagePullPolicyValuesAllowed, a.Apps[i].ContainerImagePullPolicy) {
+				return fmt.Errorf("invalid containerImagePullPolicy: %s, allowed values: %s", a.Apps[i].ContainerImagePullPolicy, strings.Join(imagePullPolicyValuesAllowed, ", "))
+			}
+		} else {
+			a.Apps[i].ContainerImagePullPolicy = "Always"
+		}
 	}
 	return nil
 }

diff --git a/pkg/runfileconfig/run_file_config_parser_test.go b/pkg/runfileconfig/run_file_config_parser_test.go
@@ -14,6 +14,7 @@ limitations under the License.
 package runfileconfig
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -32,6 +33,9 @@ var (
 	runFileForPrecedenceRuleDaprDir = filepath.Join(".", "testdata", "test_run_config_precedence_rule_dapr_dir.yaml")
 	runFileForLogDestination        = filepath.Join(".", "testdata", "test_run_config_log_destination.yaml")
 	runFileForMultiResourcePaths    = filepath.Join(".", "testdata", "test_run_config_multiple_resources_paths.yaml")
+
+	runFileForContainerImagePullPolicy        = filepath.Join(".", "testdata", "test_run_config_container_image_pull_policy.yaml")
+	runFileForContainerImagePullPolicyInvalid = filepath.Join(".", "testdata", "test_run_config_container_image_pull_policy_invalid.yaml")
 )
 
 func TestRunConfigFile(t *testing.T) {
@@ -251,6 +255,51 @@ func TestRunConfigFile(t *testing.T) {
 	})
 }
 
+func TestContainerImagePullPolicy(t *testing.T) {
+	testcases := []struct {
+		name                   string
+		runfFile               string
+		expectedPullPolicies   []string
+		expectedBadPolicyValue string
+		expectedErr            bool
+	}{
+		{
+			name:                 "default value is Always",
+			runfFile:             validRunFilePath,
+			expectedPullPolicies: []string{"Always", "Always"},
+			expectedErr:          false,
+		},
+		{
+			name:                 "custom value is respected",
+			runfFile:             runFileForContainerImagePullPolicy,
+			expectedPullPolicies: []string{"IfNotPresent", "Always"},
+			expectedErr:          false,
+		},
+		{
+			name:                   "invalid value is rejected",
+			runfFile:               runFileForContainerImagePullPolicyInvalid,
+			expectedPullPolicies:   []string{"Always", "Always"},
+			expectedBadPolicyValue: "Invalid",
+			expectedErr:            true,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			config := RunFileConfig{}
+			config.parseAppsConfig(tc.runfFile)
+			err := config.validateRunConfig(tc.runfFile)
+			if tc.expectedErr {
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), fmt.Sprintf("invalid containerImagePullPolicy: %s, allowed values: Always, Never, IfNotPresent", tc.expectedBadPolicyValue))
+				return
+			}
+			assert.Equal(t, tc.expectedPullPolicies[0], config.Apps[0].ContainerImagePullPolicy)
+			assert.Equal(t, tc.expectedPullPolicies[1], config.Apps[1].ContainerImagePullPolicy)
+		})
+	}
+}
+
 func TestMultiResourcePathsResolution(t *testing.T) {
 	config := RunFileConfig{}
 

diff --git a/pkg/runfileconfig/testdata/test_run_config_container_image_pull_policy.yaml b/pkg/runfileconfig/testdata/test_run_config_container_image_pull_policy.yaml
@@ -0,0 +1,24 @@
+version: 1
+common:
+  resourcesPath: ./app/resources
+  appProtocol: HTTP
+  appHealthProbeTimeout: 10
+  env:
+    DEBUG: false
+    tty: sts
+apps:
+  - appDirPath: ./webapp/
+    resourcesPath: ./resources
+    configFilePath: ./config.yaml
+    appPort: 8080
+    appHealthProbeTimeout: 1
+    containerImagePullPolicy: IfNotPresent
+    containerImage: ghcr.io/dapr/dapr-workflows-python-sdk:latest
+  - appID: backend
+    appDirPath: ./backend/
+    appProtocol: GRPC
+    appPort: 3000
+    unixDomainSocket: /tmp/test-socket
+    env:
+      DEBUG: true
+    containerImage: ghcr.io/dapr/dapr-workflows-csharp-sdk:latest
diff --git a/pkg/runfileconfig/testdata/test_run_config_container_image_pull_policy_invalid.yaml b/pkg/runfileconfig/testdata/test_run_config_container_image_pull_policy_invalid.yaml
@@ -0,0 +1,24 @@
+version: 1
+common:
+  resourcesPath: ./app/resources
+  appProtocol: HTTP
+  appHealthProbeTimeout: 10
+  env:
+    DEBUG: false
+    tty: sts
+apps:
+  - appDirPath: ./webapp/
+    resourcesPath: ./resources
+    configFilePath: ./config.yaml
+    appPort: 8080
+    appHealthProbeTimeout: 1
+    containerImagePullPolicy: Invalid
+    containerImage: ghcr.io/dapr/dapr-workflows-python-sdk:latest
+  - appID: backend
+    appDirPath: ./backend/
+    appProtocol: GRPC
+    appPort: 3000
+    unixDomainSocket: /tmp/test-socket
+    env:
+      DEBUG: true
+    containerImage: ghcr.io/dapr/dapr-workflows-csharp-sdk:latest