Parameter substitution not working in ClusterTask.spec.steps.imagePullPolicy

[itewk]

Expected Behavior

This should work:

apiVersion: tekton.dev/v1beta1
kind: ClusterTask
metadata:
  name: tssc-setup
spec:
  params:
  - name: ciWorkersImageRegesitryURI
    type: string
    default: quay.io
  - name: ciWorkersImageRepositoryName
    type: string
    default: tssc
  - name: ciWorkersImageTag
    type: string
    default: latest
  - name: ciWorkersImagePullPolicy
    type: string
    default: IfNotPresent
  steps:
  - name: create-python-venv
    image: $(params.ciWorkersImageRegesitryURI)/$(params.ciWorkersImageRepositoryName)/tssc-base:$(params.ciWorkersImageTag)
    imagePullPolicy: $(params.ciWorkersImageRegesitryURI)
    script: |
      #!/usr/bin/env bash
      echo hello world

Actual Behavior

Get the following error:

status:
  completionTime: '2020-10-21T15:58:44Z'
  conditions:
    - lastTransitionTime: '2020-10-21T15:58:44Z'
      message: TaskRun tssc-tekton-pipeline-maven-8xnhn-tssc-setup-225w4 has failed
      reason: Failed
      status: 'False'
      type: Succeeded
  startTime: '2020-10-21T15:58:44Z'
  taskRuns:
    tssc-tekton-pipeline-maven-8xnhn-tssc-setup-225w4:
      pipelineTaskName: tssc-setup
      status:
        conditions:
          - lastTransitionTime: '2020-10-21T15:58:44Z'
            message: >-
              Missing or invalid Task ian-tekton-test2/tssc-setup: Pod
              "tssc-tekton-pipeline-maven-8xnhn-tssc-setup-225w4-pod-sn8w2" is
              invalid: spec.containers[0].imagePullPolicy: Unsupported value:
              "$(params.ciWorkersImagePullPolicy)": supported values: "Always",
              "IfNotPresent", "Never"
            reason: CouldntGetTask
            status: 'False'
            type: Succeeded
        podName: ''
        startTime: '2020-10-21T15:58:44Z'

It would apear from the error that for some reason variable substitution is not happenin gin the ClusterTask.spec.steps.imagePullPolicy value. I would have assumed that could do variable substitution in anything under ClusterTask.spec.steps.

Steps to Reproduce the Problem

1. Create a cluster task with a step defining imagePullPolicy with a avlue from a parameter
2. try to run the step

Additional Info

• Kubernetes version:

  Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.8",   GitCommit:"9f2892aab98fe339f3bd70e3c470144299398ace", GitTreeState:"clean", BuildDate:"2020-08-13T16:12:48Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"18+", GitVersion:"v1.18.3+012b3ec", GitCommit:"012b3ec", GitTreeState:"clean", BuildDate:"2020-07-24T07:23:10Z", GoVersion:"go1.13.4", Compiler:"gc", Platform:"linux/amd64"}

• Tekton Pipeline version:

  Output of tkn version or kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'

Installed via OpenShift Pipelines Operator, can't find a tekton-pipelines-controller

OpenShift Pipelines Operator: 1.1.1
OpenShift-Pipelines: v0.14.3
OpenShift-Pipelines-Triggers: v0.6.1
OpenShift-Pipelines-ClusterTasks: v0.14

[itewk]

it would seem the problem has to do with kube trying to validate the resulting container spec before the variable substituion happens....

[vdemeester]

@itewk I think we do not support variable interpolation in imagePullPolicy yet 😅 This needs to be fixed indeed 👼

[itewk]

@vdemeester is there a list somewhere of which paramters do support variable interpolation?

[ghost]

https://github.com/tektoncd/pipeline/blob/master/docs/variables.md#fields-that-accept-variable-substitutions should list all the places you can put variables at the moment.

[itewk]

@sbwsg sweet. I guess this should be changed to an RFE then?

[ghost]

/kind feature
/remove-kind bug

[bobcatfish]

Hey @rinckm and all,

I want to explore this a bit before we add it since it seems to me like ImagePullPolicy is a very k8s specific thing and while we do add k8s specific stuff into our API I want to make sure we really need to before we do (https://github.com/tektoncd/community/blob/master/design-principles.md#conformance)

@itewk I think we do not support variable interpolation in imagePullPolicy yet 😅 This needs to be fixed indeed 👼

@vdemeester it sounds like you think we SHOULD replace this - do we replace other container spec fields?

[vdemeester]

I want to explore this a bit before we add it since it seems to me like ImagePullPolicy is a very k8s specific thing and while we do add k8s specific stuff into our API I want to make sure we really need to before we do (https://github.com/tektoncd/community/blob/master/design-principles.md#conformance)

I would say, yes and no (on imagePullPolicy being "k8s specific stuff"). The field itself is k8s specific but the "feature" is more than k8s specific. It is tied to any container runtime (aka what should the runtime do in case of the image is present in its cache or not, …) and could probably be part of the conformance / api spec.

@itewk I think we do not support variable interpolation in imagePullPolicy yet sweat_smile This needs to be fixed indeed angel

@vdemeester it sounds like you think we SHOULD replace this - do we replace other container spec fields?

The variable interpolation is selective and I am pretty sure we don't go through the imagePullPolicy field. It's more or less a one-liner to fix this though (see #3488)

[vdemeester]

@bobcatfish See container_replacements.go for what we support in variable interpolation.

[bobcatfish]

Is it possible to understand the use case a bit more where you need to use this value inside your Tasks @rinckm? (Want to make sure there isn't some other feature we could provide instead that would solve your problem even better)

[itewk]

@bobcatfish my use case is being able to control at runtime when calling a pipeline whether to used the cached images or pull the latest. This comes in most handy when doing intigration testing of the workflow itself and testing with updated task container images.

This is likley a crazy ask, that i am guessing has already been discussed somewhere, but why isn't there variable substitution in every field within spec.steps? that would mean you arn't doing any codeing specific to k8s api and would mean when other fields are added arn't having to maitain and remember a complicated list of which fields do and do not accept variable substitution

[rinckm]

I want to allow that a user of the Task can specify the image and the corresponding imagePullPolicy.
If the user specifies a released image he may want to use the imagePullPolicy 'IfNotPresent' to speed up things.
If the user specifies a image with tag 'latest' he may want to use the imagePullPolicy 'Always' to really get the latest version.

[bobcatfish]

Thanks for explaining your use cases @itewk and @rinckm , that makes perfect sense to me! I also looked at your examples more closely and I had this backwards 🤦‍♀️ - I thought we were adding a variable that allows you to grab the ImagePullPolicy that was already set (e.g. something like "echo $(steps.imagePullPolicy)") but now i understand you want to SET the ImagePullPolicy at runtime which makes way more sense XD. (I can also see why you were confused by me saying this is "k8s specific" @vdemeester 🙏 😆 )

[bobcatfish]

@bobcatfish my use case is being able to control at runtime when calling a pipeline whether to used the cached images or pull the latest.

This is maybe different than what you're asking for but it also kinda sounds like you might want to be able to specify a step template at runtime 🤔 I was hoping that podTemplate would let you set the ImagePullPolicy at runtime but it doesn't since that's at the level of the pod and not the individual containers/steps

[itewk]

@rinckm thank you