From efe1d6c1e261e584f1c8ce7bd68b04ca49a3de4a Mon Sep 17 00:00:00 2001 From: Filinto Duran Date: Tue, 7 Jan 2025 11:09:00 -0600 Subject: [PATCH] add image pull policy (#1462) * add image pull policy Signed-off-by: Filinto Duran * add allowed values Signed-off-by: Filinto Duran * feedback refactor allowed values name Signed-off-by: Filinto Duran * add unit tests Signed-off-by: Filinto Duran * lint Signed-off-by: Filinto Duran * lint Signed-off-by: Filinto Duran * more lint Signed-off-by: Filinto Duran * more lint Signed-off-by: Filinto Duran --------- Signed-off-by: Filinto Duran Co-authored-by: Anton Troshin Co-authored-by: Mike Nguyen --- pkg/kubernetes/run.go | 2 +- pkg/runfileconfig/run_file_config.go | 5 +- pkg/runfileconfig/run_file_config_parser.go | 11 +++++ .../run_file_config_parser_test.go | 49 +++++++++++++++++++ ...un_config_container_image_pull_policy.yaml | 24 +++++++++ ...g_container_image_pull_policy_invalid.yaml | 24 +++++++++ 6 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 pkg/runfileconfig/testdata/test_run_config_container_image_pull_policy.yaml create mode 100644 pkg/runfileconfig/testdata/test_run_config_container_image_pull_policy_invalid.yaml diff --git a/pkg/kubernetes/run.go b/pkg/kubernetes/run.go index 6a7199d64..b8869916e 100644 --- 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 index 50f17a977..575237600 100644 --- 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 index 207bfd2fa..c5a857b3e 100644 --- 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 index 8fd009e73..b2df4dd17 100644 --- 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 new file mode 100644 index 000000000..606296e2f --- /dev/null +++ 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 new file mode 100644 index 000000000..26c5c6b4c --- /dev/null +++ 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