tektoncd / pipeline

A cloud-native Pipeline resource.
https://tekton.dev
Apache License 2.0
8.41k stars 1.77k forks source link

script mode is broken by stepTemplate #1647

Closed afrittoli closed 4 years ago

afrittoli commented 4 years ago

Expected Behavior

I can use script mode in a step and use stepTemplate as well.

Actual Behavior

When a stepTemplate is added to the Task, the place-scripts container disappears, and entrypoint in step container runs /bin/sh instead of the script.

Steps to Reproduce the Problem

  1. I'm using the following task:

    apiVersion: tekton.dev/v1alpha1
    kind: Task
    metadata:
    name: create-draft-release
    spec:
    inputs:
    params:
    - name: package
      description: package (and its children) under test
    - name: release-name
      description: The name of the release (e.g. Cat + Robot for pipeline)
    - name: release-tag
      description: Release number and git tag to be applied (e.g. 0.888.1, no 'v')
    resources:
    - name: source
      type: git
    stepTemplate:
    env:
      - name: GITHUB_TOKEN
        valueFrom:
          secretKeyRef:
            name: github-token
            key: GITHUB_TOKEN
    steps:
    - name: authors-list
      image: gcr.io/tekton-releases/dogfooding/hub
      workingdir: $(inputs.resources.source.path)
      script: |
        #!/usr/bin/env bash
        set -ex
    
        hub pr list --state merged -f "%au %mI%n" -L 50 -o updated | \
          egrep -v '(2019-07|2019-08-14|2019-08-13)' | \
          awk '{ print "- @"$1 }' | sort -u > $HOME/authors.md
  2. I run the task with tkn:

    tkn task start create-draft-release -p package=foo -p release-name=bar -p release-tag=zyx -i source=pipelines-master -n tekton-release
  3. The resulting pod (status excluded):

    apiVersion: v1
    kind: Pod
    metadata:
    annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"tekton.dev/v1alpha1","kind":"Task","metadata":{"annotations":{},"name":"create-draft-release","namespace":"tekton-release"},"spec":{"inputs":{"params":[{"description":"package (and its children) under test","name":"package"},{"description":"The name of the release (e.g. Cat + Robot for pipeline)","name":"release-name"},{"description":"Release number and git tag to be applied (e.g. 0.888.1, no 'v')","name":"release-tag"}],"resources":[{"name":"source","type":"git"}]},"stepTemplate":{"env":[{"name":"GITHUB_TOKEN","valueFrom":{"secretKeyRef":{"key":"GITHUB_TOKEN","name":"github-token"}}}]},"steps":[{"image":"gcr.io/tekton-releases/dogfooding/hub","name":"authors-list","script":"#!/usr/bin/env bash\nset -ex\necho \"Hello, there!\"\n\ngit remote -v\ngit log -10\ngit fetch --all\nhub pr list --state merged -f \"%au %mI%n\" -L 50 -o updated | \\\n  egrep -v '(2019-07|2019-08-14|2019-08-13)' | \\\n  awk '{ print \"- @\"$1 }' | sort -u \u003e $HOME/authors.md\ncat $HOME/authors.md\n","workingdir":"$(inputs.resources.source.path)"}]}}
    kubernetes.io/psp: ibm-privileged-psp
    tekton.dev/ready: READY
    creationTimestamp: "2019-11-29T10:31:50Z"
    labels:
    app.kubernetes.io/managed-by: tekton-pipelines
    tekton.dev/task: create-draft-release
    tekton.dev/taskRun: create-draft-release-run-vzlv8
    name: create-draft-release-run-vzlv8-pod-33f4e4
    namespace: tekton-release
    ownerReferences:
    - apiVersion: tekton.dev/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: TaskRun
    name: create-draft-release-run-vzlv8
    uid: 538b11bb-1293-11ea-9f24-720b50a90936
    resourceVersion: "48461394"
    selfLink: /api/v1/namespaces/tekton-release/pods/create-draft-release-run-vzlv8-pod-33f4e4
    uid: 743e92bc-1293-11ea-b708-72c26db2b027
    spec:
    containers:
    - args:
    - -wait_file
    - /tekton/downward/ready
    - -wait_file_content
    - -post_file
    - /tekton/tools/0
    - -entrypoint
    - /ko-app/git-init
    - --
    - -url
    - 'https://github.com/tektoncd/pipeline '
    - -revision
    - master
    - -path
    - /workspace/source
    command:
    - /tekton/tools/entrypoint
    env:
    - name: HOME
      value: /tekton/home
    - name: TEKTON_RESOURCE_NAME
      value: pipelines-master
    - name: GITHUB_TOKEN
      valueFrom:
        secretKeyRef:
          key: GITHUB_TOKEN
          name: github-token
    image: us.icr.io/knative/git-init-4874978a9786b6625dd8b6ef2a21aa70@sha256:51020534aebff6e58f76d5ebef55527a58b72d70b4cd9eb8c2c1ba01b76cbe7f
    imagePullPolicy: IfNotPresent
    name: step-git-source-pipelines-master-nrnzg
    resources:
      requests:
        cpu: "0"
        ephemeral-storage: "0"
        memory: "0"
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /tekton/tools
      name: tools
    - mountPath: /tekton/downward
      name: downward
    - mountPath: /workspace
      name: workspace
    - mountPath: /tekton/home
      name: tekton-home
    - mountPath: /builder/home
      name: tekton-home
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: default-token-qxnbv
      readOnly: true
    workingDir: /workspace
    - args:
    - -wait_file
    - /tekton/tools/0
    - -post_file
    - /tekton/tools/1
    - -entrypoint
    - /bin/sh
    - --
    command:
    - /tekton/tools/entrypoint
    env:
    - name: HOME
      value: /tekton/home
    - name: GITHUB_TOKEN
      valueFrom:
        secretKeyRef:
          key: GITHUB_TOKEN
          name: github-token
    image: gcr.io/tekton-releases/dogfooding/hub@sha256:12bdb576b17fd863774c33d44990b0fa3a94d50f66d393d829376228f82139ba
    imagePullPolicy: IfNotPresent
    name: step-authors-list
    resources:
      requests:
        cpu: "0"
        ephemeral-storage: "0"
        memory: "0"
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /tekton/tools
      name: tools
    - mountPath: /workspace
      name: workspace
    - mountPath: /tekton/home
      name: tekton-home
    - mountPath: /builder/home
      name: tekton-home
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: default-token-qxnbv
      readOnly: true
    workingDir: /workspace/source
    dnsPolicy: ClusterFirst
    enableServiceLinks: true
    imagePullSecrets:
    - name: default-us-icr-io
    - name: ibm-cr-apikey
    initContainers:
    - args:
    - -basic-docker=ibm-cr-api-us-south=us.icr.io
    command:
    - /ko-app/creds-init
    env:
    - name: HOME
      value: /tekton/home
    image: us.icr.io/knative/creds-init-c761f275af7b3d8bea9d50cc6cb0106f@sha256:5a0f2f05f0b14dea88cc9a6074058a9fb96ff79fc1c303051ddd336f0292ef91
    imagePullPolicy: IfNotPresent
    name: credential-initializer-f5pjm
    resources: {}
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /workspace
      name: workspace
    - mountPath: /tekton/home
      name: tekton-home
    - mountPath: /builder/home
      name: tekton-home
    - mountPath: /var/build-secrets/ibm-cr-api-us-south
      name: secret-volume-ibm-cr-api-us-south-9mt72
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: default-token-qxnbv
      readOnly: true
    - args:
    - -c
    - mkdir -p /workspace/source
    command:
    - sh
    image: busybox
    imagePullPolicy: Always
    name: working-dir-initializer-qqhrz
    resources: {}
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /workspace
      name: workspace
    - mountPath: /tekton/home
      name: tekton-home
    - mountPath: /builder/home
      name: tekton-home
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: default-token-qxnbv
      readOnly: true
    workingDir: /workspace
    - command:
    - cp
    - /ko-app/entrypoint
    - /tekton/tools/entrypoint
    image: us.icr.io/knative/entrypoint-bff0a22da108bc2f16c818c97641a296@sha256:22f463e52704b917b322594fb293de0f733207cbc558f00e4942e3efe32d8f6c
    imagePullPolicy: IfNotPresent
    name: place-tools
    resources: {}
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /tekton/tools
      name: tools
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: default-token-qxnbv
      readOnly: true
    nodeName: 10.168.226.182
    priority: 0
    restartPolicy: Never
    schedulerName: default-scheduler
    securityContext: {}
    serviceAccount: default
    serviceAccountName: default
    terminationGracePeriodSeconds: 30
    tolerations:
    - effect: NoExecute
    key: node.kubernetes.io/not-ready
    operator: Exists
    tolerationSeconds: 600
    - effect: NoExecute
    key: node.kubernetes.io/unreachable
    operator: Exists
    tolerationSeconds: 600
    volumes:
    - emptyDir: {}
    name: workspace
    - emptyDir: {}
    name: tekton-home
    - name: secret-volume-ibm-cr-api-us-south-9mt72
    secret:
      defaultMode: 420
      secretName: ibm-cr-api-us-south
    - emptyDir: {}
    name: tools
    - downwardAPI:
      defaultMode: 420
      items:
      - fieldRef:
          apiVersion: v1
          fieldPath: metadata.annotations['tekton.dev/ready']
        path: ready
    name: downward
    - name: default-token-qxnbv
    secret:
      defaultMode: 420
      secretName: default-token-qxnbv

    Additional Info

vdemeester commented 4 years ago

/kind bug /cc @ImJasonH

afrittoli commented 4 years ago

I have an idea of what may be happening. MakePod applies the template merge first, and the script to container then. However the template merge code does a three-ways merge between the template, an empty container and the original step, into a container. Finally the merge code brings back the step with steps[i] = Step{Container: *merged}. Since "script" is not a container field, it's dropped along the way.

One possible way to solve this would be to resolve the script to regular containers first and do the merge then. The side effect would be that the template would be applied to the container that generates the script too, but that's probably ok, and there may be ways to avoid that.

The other alternative is in the merge code to take in account the 'Script' part explicitly.

bobcatfish commented 4 years ago

(p.s. beautifully written issue @afrittoli :heart_eyes: !!)