diff --git a/docs/cmd/kn_container_add.md b/docs/cmd/kn_container_add.md index 0ba471626..01d5146e4 100644 --- a/docs/cmd/kn_container_add.md +++ b/docs/cmd/kn_container_add.md @@ -40,6 +40,7 @@ kn container add NAME --limit strings The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource limit, append "-" to the resource name, e.g. '--limit memory-'. --mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume. -p, --port string The port where application listens on, in the format 'NAME:PORT', where 'NAME' is optional. Examples: '--port h2c:8080' , '--port 8080'. + --pull-policy string Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'. --service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace. diff --git a/docs/cmd/kn_service_apply.md b/docs/cmd/kn_service_apply.md index 319bb2034..978cc4ec0 100644 --- a/docs/cmd/kn_service_apply.md +++ b/docs/cmd/kn_service_apply.md @@ -56,6 +56,7 @@ kn service apply s0 --filename my-svc.yml --no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) --no-wait Do not wait for 'service apply' operation to be completed. -p, --port string The port where application listens on, in the format 'NAME:PORT', where 'NAME' is optional. Examples: '--port h2c:8080' , '--port 8080'. + --pull-policy string Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'. --revision-name string The revision name to set. Must start with the service name and a dash as a prefix. Empty revision name will result in the server generating a name for the revision. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants (e.g. {{.Service}}-{{.Random 5}}-{{.Generation}}) diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 33ea8defa..1a0c8ca91 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -81,6 +81,7 @@ kn service create NAME --image IMAGE --no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) --no-wait Do not wait for 'service create' operation to be completed. -p, --port string The port where application listens on, in the format 'NAME:PORT', where 'NAME' is optional. Examples: '--port h2c:8080' , '--port 8080'. + --pull-policy string Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'. --revision-name string The revision name to set. Must start with the service name and a dash as a prefix. Empty revision name will result in the server generating a name for the revision. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants (e.g. {{.Service}}-{{.Random 5}}-{{.Generation}}) diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index 2736bfff7..8c37dee85 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -68,6 +68,7 @@ kn service update NAME --no-lock-to-digest Do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision) --no-wait Do not wait for 'service update' operation to be completed. -p, --port string The port where application listens on, in the format 'NAME:PORT', where 'NAME' is optional. Examples: '--port h2c:8080' , '--port 8080'. + --pull-policy string Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'. --revision-name string The revision name to set. Must start with the service name and a dash as a prefix. Empty revision name will result in the server generating a name for the revision. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants (e.g. {{.Service}}-{{.Random 5}}-{{.Generation}}) diff --git a/docs/cmd/kn_source_container_create.md b/docs/cmd/kn_source_container_create.md index f2c730eaa..93dd06955 100644 --- a/docs/cmd/kn_source_container_create.md +++ b/docs/cmd/kn_source_container_create.md @@ -30,6 +30,7 @@ kn source container create NAME --image IMAGE --sink SINK --mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume. -n, --namespace string Specify the namespace to operate in. -p, --port string The port where application listens on, in the format 'NAME:PORT', where 'NAME' is optional. Examples: '--port h2c:8080' , '--port 8080'. + --pull-policy string Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'. --service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace. diff --git a/docs/cmd/kn_source_container_update.md b/docs/cmd/kn_source_container_update.md index cff08abc9..95d96ef8f 100644 --- a/docs/cmd/kn_source_container_update.md +++ b/docs/cmd/kn_source_container_update.md @@ -30,6 +30,7 @@ kn source container update NAME --image IMAGE --mount stringArray Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. When a configmap or a secret is specified, a corresponding volume is automatically generated. You can use this flag multiple times. For unmounting a directory, append "-", e.g. --mount /mydir-, which also removes any auto-generated volume. -n, --namespace string Specify the namespace to operate in. -p, --port string The port where application listens on, in the format 'NAME:PORT', where 'NAME' is optional. Examples: '--port h2c:8080' , '--port 8080'. + --pull-policy string Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent --pull-secret string Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace. --request strings The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'. --service-account string Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace. diff --git a/pkg/kn/commands/service/configuration_edit_flags_test.go b/pkg/kn/commands/service/configuration_edit_flags_test.go new file mode 100644 index 000000000..b2cd8726f --- /dev/null +++ b/pkg/kn/commands/service/configuration_edit_flags_test.go @@ -0,0 +1,49 @@ +// Copyright © 2022 The Knative Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package service + +import ( + "testing" + + "gotest.tools/v3/assert" + "knative.dev/client/pkg/kn/commands" + "knative.dev/client/pkg/util" +) + +func TestApplyPullPolicyFlag(t *testing.T) { + var editFlags ConfigurationEditFlags + knParams := &commands.KnParams{} + cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams) + + editFlags.AddCreateFlags(cmd) + svc := createTestService("test-svc", []string{"test-svc-00001", "test-svc-00002"}, goodConditions()) + cmd.SetArgs([]string{"--pull-policy", "Always"}) + cmd.Execute() + err := editFlags.Apply(&svc, nil, cmd) + assert.NilError(t, err) +} + +func TestApplyPullPolicyFlagError(t *testing.T) { + var editFlags ConfigurationEditFlags + knParams := &commands.KnParams{} + cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams) + + editFlags.AddCreateFlags(cmd) + svc := createTestService("test-svc", []string{"test-svc-00001", "test-svc-00002"}, goodConditions()) + cmd.SetArgs([]string{"--pull-policy", "InvalidPolicy"}) + cmd.Execute() + err := editFlags.Apply(&svc, nil, cmd) + assert.Assert(t, util.ContainsAll(err.Error(), "invalid", "InvalidPolicy", "Valid arguments (case insensitive): Always | Never | IfNotPresent")) +} diff --git a/pkg/kn/flags/podspec.go b/pkg/kn/flags/podspec.go index bc7b6f329..ccd0b1eb6 100644 --- a/pkg/kn/flags/podspec.go +++ b/pkg/kn/flags/podspec.go @@ -28,13 +28,14 @@ import ( // PodSpecFlags to hold the container resource requirements values type PodSpecFlags struct { // Direct field manipulation - Image uniqueStringArg - Env []string - EnvFrom []string - EnvValueFrom []string - EnvFile string - Mount []string - Volume []string + Image uniqueStringArg + ImagePullPolicy string + Env []string + EnvFrom []string + EnvValueFrom []string + EnvFile string + Mount []string + Volume []string Command []string Arg []string @@ -129,6 +130,9 @@ func (p *PodSpecFlags) AddFlags(flagset *pflag.FlagSet) []string { flagset.VarP(&p.Image, "image", "", "Image to run.") flagNames = append(flagNames, "image") + flagset.StringVar(&p.ImagePullPolicy, "pull-policy", "", + "Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent") + flagset.StringVarP(&p.EnvFile, "env-file", "", "", "Path to a file containing environment variables (e.g. --env-file=/home/knative/service1/env).") flagNames = append(flagNames, "env-file") @@ -285,6 +289,14 @@ func (p *PodSpecFlags) ResolvePodSpec(podSpec *corev1.PodSpec, flags *pflag.Flag } } + if flags.Changed("pull-policy") { + + err = UpdateImagePullPolicy(podSpec, p.ImagePullPolicy) + if err != nil { + return err + } + } + requestsToRemove, limitsToRemove, err := p.Resources.Validate() if err != nil { return err diff --git a/pkg/kn/flags/podspec_helper.go b/pkg/kn/flags/podspec_helper.go index c35853cd1..f6ff0dfa3 100644 --- a/pkg/kn/flags/podspec_helper.go +++ b/pkg/kn/flags/podspec_helper.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/yaml" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" "knative.dev/client/pkg/util" ) @@ -301,6 +302,35 @@ func UpdateContainers(spec *corev1.PodSpec, containers []corev1.Container) { } } +// UpdateImagePullPolicy updates the pull policy for the given revision template +func UpdateImagePullPolicy(spec *corev1.PodSpec, imagePullPolicy string) error { + container := containerOfPodSpec(spec) + + if !isValidPullPolicy(imagePullPolicy) { + return fmt.Errorf("invalid --pull-policy %s. Valid arguments (case insensitive): Always | Never | IfNotPresent", imagePullPolicy) + } + container.ImagePullPolicy = getPolicy(imagePullPolicy) + return nil +} + +func getPolicy(policy string) v1.PullPolicy { + var ret v1.PullPolicy + switch strings.ToLower(policy) { + case "always": + ret = v1.PullAlways + case "ifnotpresent": + ret = v1.PullIfNotPresent + case "never": + ret = v1.PullNever + } + return ret +} + +func isValidPullPolicy(policy string) bool { + validPolicies := []string{string(v1.PullAlways), string(v1.PullNever), string(v1.PullIfNotPresent)} + return util.SliceContainsIgnoreCase(validPolicies, policy) +} + // ======================================================================================= func updateEnvVarsFromMap(env []corev1.EnvVar, toUpdate *util.OrderedMap) []corev1.EnvVar { updated := sets.NewString() diff --git a/pkg/kn/flags/podspec_helper_test.go b/pkg/kn/flags/podspec_helper_test.go index 5fbd60f95..e928cfb7a 100644 --- a/pkg/kn/flags/podspec_helper_test.go +++ b/pkg/kn/flags/podspec_helper_test.go @@ -892,3 +892,66 @@ containers: assert.Assert(t, err != nil) assert.Assert(t, util.ContainsAll(err.Error(), "no", "file", "directory")) } + +func TestUpdateImagePullPolicy(t *testing.T) { + policyMap := make(map[string][]string) + policyMap["Always"] = []string{"always", "ALWAYS", "Always"} + policyMap["Never"] = []string{"never", "NEVER", "Never"} + policyMap["IfNotPresent"] = []string{"ifnotpresent", "IFNOTPRESENT", "IfNotPresent"} + + for k, values := range policyMap { + expectedPodSpec := &corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "repo/user/imageID:tag", + ImagePullPolicy: corev1.PullPolicy(k), + Command: []string{"/app/start"}, + Args: []string{"myArg1"}, + Ports: []corev1.ContainerPort{ + { + ContainerPort: 8080, + }, + }, + }, + }, + } + for _, v := range values { + podSpec := &corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "repo/user/imageID:tag", + Command: []string{"/app/start"}, + Args: []string{"myArg1"}, + Ports: []corev1.ContainerPort{ + { + ContainerPort: 8080, + }, + }, + }, + }, + } + err := UpdateImagePullPolicy(podSpec, v) + assert.NilError(t, err, "update pull policy failed") + assert.DeepEqual(t, expectedPodSpec, podSpec) + } + } +} + +func TestUpdateImagePullPolicyError(t *testing.T) { + podSpec := &corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "repo/user/imageID:tag", + Command: []string{"/app/start"}, + Args: []string{"myArg1"}, + Ports: []corev1.ContainerPort{ + { + ContainerPort: 8080, + }, + }, + }, + }, + } + err := UpdateImagePullPolicy(podSpec, "InvalidPolicy") + assert.Assert(t, util.ContainsAll(err.Error(), "invalid --pull-policy", "Valid arguments", "Always | Never | IfNotPresent")) +} diff --git a/pkg/kn/flags/podspec_test.go b/pkg/kn/flags/podspec_test.go index 68a2ec2bd..bf90daf12 100644 --- a/pkg/kn/flags/podspec_test.go +++ b/pkg/kn/flags/podspec_test.go @@ -70,13 +70,14 @@ func TestPodSpecResolve(t *testing.T) { "--port", "8080", "--limit", "cpu=1000m", "--limit", "memory=1024Mi", "--cmd", "/app/start", "--arg", "myArg1", "--service-account", "foo-bar-account", "--mount", "/mount/path=volume-name", "--volume", "volume-name=cm:config-map-name", - "--env-from", "config-map:config-map-name", "--user", "1001"} + "--env-from", "config-map:config-map-name", "--user", "1001", "--pull-policy", "always"} expectedPodSpec := corev1.PodSpec{ Containers: []corev1.Container{ { - Image: "repo/user/imageID:tag", - Command: []string{"/app/start"}, - Args: []string{"myArg1"}, + Image: "repo/user/imageID:tag", + ImagePullPolicy: "Always", + Command: []string{"/app/start"}, + Args: []string{"myArg1"}, Ports: []corev1.ContainerPort{ { ContainerPort: 8080, @@ -291,6 +292,28 @@ func TestPodSpecResolveReturnError(t *testing.T) { assert.Assert(t, util.ContainsAll(out, "Invalid", "mount")) } +func TestPodSpecResolveReturnErrorPullPolicy(t *testing.T) { + outBuf := bytes.Buffer{} + flags := &PodSpecFlags{} + inputArgs := []string{"--pull-policy", "invalidPolicy"} + testCmd := &cobra.Command{ + Use: "test", + Run: func(cmd *cobra.Command, args []string) { + podSpec := &corev1.PodSpec{Containers: []corev1.Container{{}}} + err := flags.ResolvePodSpec(podSpec, cmd.Flags(), inputArgs) + fmt.Fprint(cmd.OutOrStdout(), "Return error: ", err) + }, + } + testCmd.SetOut(&outBuf) + + testCmd.SetArgs(inputArgs) + flags.AddFlags(testCmd.Flags()) + flags.AddCreateFlags(testCmd.Flags()) + testCmd.Execute() + out := outBuf.String() + assert.Assert(t, util.ContainsAll(out, "invalid --pull-policy", "Always | Never | IfNotPresent")) +} + func TestPodSpecResolveWithEnvFile(t *testing.T) { file, err := ioutil.TempFile("", "envfile.env") assert.NilError(t, err) diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index 5c5cc57e0..6c6b88032 100644 --- a/pkg/serving/config_changes.go +++ b/pkg/serving/config_changes.go @@ -22,12 +22,11 @@ import ( "strings" "time" + "knative.dev/client/pkg/kn/flags" "knative.dev/pkg/ptr" "knative.dev/serving/pkg/apis/autoscaling" servingconfig "knative.dev/serving/pkg/apis/config" servingv1 "knative.dev/serving/pkg/apis/serving/v1" - - "knative.dev/client/pkg/kn/flags" ) // VolumeSourceType is a type standing for enumeration of ConfigMap and Secret diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go index cb4e51170..e971d908f 100644 --- a/pkg/serving/config_changes_test.go +++ b/pkg/serving/config_changes_test.go @@ -151,6 +151,41 @@ func TestPinImageToDigestNilContainerStatus(t *testing.T) { assert.NilError(t, err) } +func TestPinImageToDigestNilContainerSpec(t *testing.T) { + template, _ := getRevisionTemplate() + template.Spec.Containers = nil + revision := &servingv1.Revision{} + revision.Spec = template.Spec + revision.ObjectMeta = template.ObjectMeta + revision.Status.ContainerStatuses = nil + err := PinImageToDigest(template, revision) + assert.ErrorContains(t, err, "no container given in current revision") +} + +func TestPinImageToDigestNilBaseRevisionContainerSpec(t *testing.T) { + template, _ := getRevisionTemplate() + revision := &servingv1.Revision{} + revision.Spec = template.Spec + revision.ObjectMeta = template.ObjectMeta + revision.Status.ContainerStatuses = nil + + revision.Spec.Containers = nil + err := PinImageToDigest(template, revision) + assert.ErrorContains(t, err, "no container found in base revision") +} + +func TestPinImageToDigestImageMismatch(t *testing.T) { + template, _ := getRevisionTemplate() + revision := &servingv1.Revision{} + revision.Spec = template.Spec + revision.ObjectMeta = template.ObjectMeta + revision.Status.ContainerStatuses = nil + + revision.Spec.Containers = []corev1.Container{{Image: "mock-image"}} + err := PinImageToDigest(template, revision) + assert.ErrorContains(t, err, "contains unexpected image") +} + func TestUpdateTimestampAnnotation(t *testing.T) { template, _ := getRevisionTemplate() UpdateTimestampAnnotation(template)