Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task results do not get passed to conditions #2336

Closed
Tracked by #87
drewbutlerbb4 opened this issue Apr 6, 2020 · 16 comments
Closed
Tracked by #87

Task results do not get passed to conditions #2336

drewbutlerbb4 opened this issue Apr 6, 2020 · 16 comments
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@drewbutlerbb4
Copy link

Expected Behavior

Conditions receive Task results as condition parameters.

Actual Behavior

Task results do not get passed properly

Containers:
  step-condition-check-condition-1:
    Container ID:  containerd://4c1637ec361f853159205654e67425ca7ef4b14b3da8f1b3b6316a539e12d21e
    Image:         python:alpine3.6
    Image ID:      docker.io/library/python@sha256:766a961bf699491995cc29e20958ef11fd63741ff41dcc70ec34355b39d52971
    Port:          <none>
    Host Port:     <none>
    Command:
      /tekton/tools/entrypoint
    Args:
      -wait_file
      /tekton/downward/ready
      -wait_file_content
      -post_file
      /tekton/tools/0
      -termination_path
      /tekton/termination
      -entrypoint
      sh
      --
      -c
      EXITCODE=$(python -c "import sys; input1=str.rstrip(sys.argv[1]); input2=str.rstrip(sys.argv[2]); print(0) if (input1 == 'heads') else print(1)" '$(tasks.flip-coin.results.output)' 'heads'); exit $EXITCODE

Steps to Reproduce the Problem

Can be reproduced from this yaml

apiVersion: tekton.dev/v1alpha1
kind: Condition
metadata:
  name: condition-1
spec:
  check:
    args:
    - EXITCODE=$(python -c "import sys; input1=str.rstrip(sys.argv[1]); input2=str.rstrip(sys.argv[2]); print(0) if (input1 == 'heads') else
      print(1)" '$(params.flip-coin)' 'heads'); exit $EXITCODE
    command:
    - sh
    - -c
    image: python:alpine3.6
  params:
  - name: flip-coin
---
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: flip-coin
spec:
  results:
  - description: /tmp/output
    name: output
  steps:
  - args:
    - python -c "import random; result = 'heads' if random.randint(0,1) == 0 else
      'tails'; result='heads'; print(result)" | tee $(results.output.path)
    command:
    - sh
    - -c
    image: python:alpine3.6
    name: flip-coin
---
apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: condition-check
spec:
  params:
  - name: flip-coin
  results:
  - description: /tmp/output
    name: output
  steps:
  - args:
    - EXITCODE=$(python -c "import sys; input1=str.rstrip(sys.argv[1]); input2=str.rstrip(sys.argv[2]); print(input1) if (input1 == 'heads') else
      print(input1)" '$(params.flip-coin)' 'heads'); echo $EXITCODE | tee $(results.output.path)
    command:
    - sh
    - -c
    image: python:alpine3.6
---
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: generate-random-number
spec:
  results:
  - description: /tmp/output
    name: output
  steps:
  - args:
    - python -c "import random; print(random.randint($0, $1))" | tee $2
    - '0'
    - '9'
    - $(results.output.path)
    command:
    - sh
    - -c
    image: python:alpine3.6
    name: generate-random-number
---
apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  annotations:
    pipelines.kubeflow.org/pipeline_spec: '{"description": "Shows how to use dsl.Condition().",
      "name": "Conditional execution pipeline"}'
  name: flip-cond-gen
spec:
  params: []
  tasks:
  - name: flip-coin
    params: []
    taskRef:
      name: flip-coin
  - name: condition-check
    params:
      - name: flip-coin
        value: $(tasks.flip-coin.results.output)
    taskRef:
      name: condition-check
  - conditions:
    - conditionRef: condition-1
      params:
      - name: flip-coin
        value: $(tasks.flip-coin.results.output)
    name: generate-random-number
    params: []
    taskRef:
      name: generate-random-number

Additional Info

  • Kubernetes version:

    Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"14", GitVersion:"v1.14.8", GitCommit:"211047e9a1922595eaa3a1127ed365e9299a6c23", GitTreeState:"clean", BuildDate:"2019-10-15T12:11:03Z", GoVersion:"go1.12.10", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"16", GitVersion:"v1.16.8+IKS", GitCommit:"c6910e70b932ed9ffe5baedade2274a72925660f", GitTreeState:"clean", BuildDate:"2020-03-13T14:13:41Z", GoVersion:"go1.13.8", Compiler:"gc", Platform:"linux/amd64"}
  • Tekton Pipeline version:

    Client version: 0.8.0
    Pipeline version: v0.11.0-rc2

@drewbutlerbb4
Copy link
Author

@Tomcli @afrittoli

@ghost ghost added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 6, 2020
@bobcatfish
Copy link
Collaborator

Ah so we're not applying the variable replacement of results inside of conditions? Whoops! Sounds like an easy fix for whoever is keen to work on this 🙏

@bobcatfish bobcatfish added the kind/bug Categorizes issue or PR as related to a bug. label Apr 8, 2020
@bobcatfish bobcatfish added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Apr 8, 2020
@skaegi
Copy link
Contributor

skaegi commented Apr 8, 2020

@othomann could you take a look.

@othomann
Copy link
Contributor

othomann commented Apr 8, 2020

I will take a look.

@othomann
Copy link
Contributor

othomann commented Apr 8, 2020

@drewbutlerbb4 how did you get the output for Containers: step-condition-check-condition-1:? I believe I have a fix, but I want to make sure it works ok.

@othomann
Copy link
Contributor

othomann commented Apr 8, 2020

I added a test to check that the Params inside the condition are substituted. So this test works now, but I want to make sure the output is good as well.

@drewbutlerbb4
Copy link
Author

To get the names of the pods from your pipelinerun:
kubectl get pods
Then to get the output:
kubectl describe pod flip-cond-gen-run-28gr7-generate-random-number-l8xcw-cond-2tf8f
(replace flip-cond-gen-run-28gr7-generate-random-number-l8xcw-cond-2tf8f with the name of the pods from your pipelinerun)

The output will be mixed in with other details about the pods. Does this answer your question?

We'd like to see $(params.flip-coin) replaced with the actual task output string instead of the $(tasks.flip-coin.results.output) that we get.

@othomann
Copy link
Contributor

othomann commented Apr 8, 2020

Yes, thanks. This answers my question. I finally got it. Working on it.

@othomann
Copy link
Contributor

othomann commented Apr 8, 2020

Now I have this:

  containers:
  - args:
    - -wait_file
    - /tekton/downward/ready
    - -wait_file_content
    - -post_file
    - /tekton/tools/0
    - -termination_path
    - /tekton/termination
    - -entrypoint
    - sh
    - --
    - -c
    - |-
      EXITCODE=$(python -c "import sys; input1=str.rstrip(sys.argv[1]); input2=str.rstrip(sys.argv[2]); print(0) if (input1 == 'heads') else print(1)" 'heads
      ' 'heads'); exit $EXITCODE

Preparing a PR for it.

@othomann
Copy link
Contributor

othomann commented Apr 8, 2020

Created #2354 to take care of this issue.

@vdemeester
Copy link
Member

@sbwsg @bobcatfish do we target 0.11.x for this fix ?

@ghost
Copy link

ghost commented Apr 9, 2020

We could, it's additive. But this isn't a bug fix (in my view), it's a new feature. We never suggested in any docs that results could be used anywhere other than task params.

Especially given conditions staying in v1alpha1 I dont feel that this is an urgent problem that we need to address in a patch release. Results also don't substitute in to PipelineResources, for example, but likewise I wouldn't rush to add them there either.

It might even be better if it rests in master for a bit to work out any issues before we cut a release with it?

@othomann
Copy link
Contributor

othomann commented Apr 9, 2020

Fixed by the merge of #2354. Scott, you can close it, I believe.

@vdemeester
Copy link
Member

@sbwsg fair :)
/close

@tekton-robot
Copy link
Collaborator

@vdemeester: Closing this issue.

In response to this:

@sbwsg fair :)
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@animeshsingh
Copy link

thanks @othomann !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

7 participants