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

Make it possible to extract results from a container's stdout #2925

Closed
bobcatfish opened this issue Jul 9, 2020 · 27 comments · Fixed by #4882
Closed

Make it possible to extract results from a container's stdout #2925

bobcatfish opened this issue Jul 9, 2020 · 27 comments · Fixed by #4882
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@bobcatfish
Copy link
Collaborator

bobcatfish commented Jul 9, 2020

Feature request

I'd like to be able to create a Task which provides results with come from the output of a container, without having to capture that input and parse it in my Task.

Initially we could do this only when output is json formatted maybe?

For example do something like this:

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: boskos-acquire
spec:
    steps:
    - name: boskosctl
      image: gcr.io/k8s-staging-boskos/boskosctl@sha256:a7fc984732c5dd0b4e0fe0a92e2730fa4b6bddecd0f6f6c7c6b5501abe4ab105 
      args:
      - "acquire"
      - "--server-url=http://boskos.test-pods.svc.cluster.local"
      - "--owner-name=christie-test-boskos"
      - "--type=gke-project"
      - "--state=free"
      - "--target-state=busy"
     results:
     - name: project-id
       description: The name of the project acquired
       fromStep:
       - name: boskosctl
          jsonPath: .name

The fromStep above would express the jsonpath to the desired value to use as the result, which is expected to be in the stdout of the command.

Use case

I'm working on a boskos Task as part of the catalog test infrastructure. This is what I'm playing around with now:

apiVersion: tekton.dev/v1beta1
kind: TaskRun
metadata:
  generateName: boskosctl-test-
spec:
  taskSpec:
    steps:
    - image: gcr.io/k8s-staging-boskos/boskosctl@sha256:a7fc984732c5dd0b4e0fe0a92e2730fa4b6bddecd0f6f6c7c6b5501abe4ab105 
      args:
      - "acquire"
      - "--server-url=http://boskos.test-pods.svc.cluster.local"
      - "--owner-name=christie-test-boskos"
      - "--type=gke-project"
      - "--state=free"
      - "--target-state=busy"

This lets me acquire a boskos project which is great, but the only way to tell what project I've acquired is to look at the container logs, e.g.:

[build-step-unnamed-1] {"type":"gke-project","name":"tekton-prow-6","state":"busy","owner":"christie-test-boskos","lastupdate":"2020-07-09T20:02:41.730506184Z","userdata":{}}

So even tho the boskos folks provided this sweet boskosctl image with a reasonable cmd default, im gonna end up having to override the entrypoint and write a script (and hope that the image has bash in it!)

@bobcatfish bobcatfish added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 9, 2020
@bobcatfish
Copy link
Collaborator Author

bobcatfish commented Jul 9, 2020

OR another idea that might be much better:

If a step could access the stdout of the previous step, then I could easily have a second container in my task that parses out the project name, e.g. something like:

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: boskos-acquire
spec:
    steps:
    - name: boskosctl
      image: gcr.io/k8s-staging-boskos/boskosctl@sha256:a7fc984732c5dd0b4e0fe0a92e2730fa4b6bddecd0f6f6c7c6b5501abe4ab105 
      args:
      - "acquire"
      - "--server-url=http://boskos.test-pods.svc.cluster.local"
      - "--owner-name=christie-test-boskos"
      - "--type=gke-project"
      - "--state=free"
      - "--target-state=busy"
    - name: grab-project-id
      image: ubuntu
      script: |
        echo $(steps.boskosctl.stdout) | jq -r ".name" > /tekton/results/project-id
    results:
    - name: project-id
      description: The name of the project acquired

A couple of ways this could work:

  • Indicate via variable interpolation when a step would like to be provided with the stdout of a previous step
  • Always save the stdout of all steps on disk in a known location

@vdemeester
Copy link
Member

So even tho the boskos folks provided this sweet boskosctl image with a reasonable cmd default, im gonna end up having to override the entrypoint and write a script (and hope that the image has bash in it!)

If boskosctl was able to write the result to a file, you could do that and have an extra step (using whatever image you feel like using) to extract some information from that file to the results file. (checked the image, there is /bin/sh and bash too.).

* Indicate via variable interpolation when a step would like to be provided with the stdout of a previous step

This could be done the other way, aka specify for a step that you want the output to be written to a file so that any next step in that task can do whatever with it. Using variable interpolation to guess that would work too, I wonder which approach is the saner, but I lean towards having an explicit way to say "I want the output of this step to be written somewhere" (especially because this doesn't have any implication on the variable interpolation part, it's simple for the user to understand what it is doing).

* Always save the stdout of all steps on disk in a known location

I would definitely not want that 🙃 This could create a lot of I/O for all task/step execution where it will not be needed.

@vdemeester
Copy link
Member

I think we also need to gauge the number of cases where this is required and where it would be a pain to do it otherwise (aka when you don't have a wait to write to a file in the image, using a shell or something else).

@bobcatfish
Copy link
Collaborator Author

Oh yeah, that's a good point, I guess it is a convention for a lot of tools to output to a file based on a flag 🤔

However I'd say it's even more common for tools to output to stdout and call it a day - aka relying on "text streams" so it feels to me like there are probably a lot of cases where capturing stdout will be the only way to get the output.

I would definitely not want that 🙃 This could create a lot of I/O for all task/step execution where it will not be needed.

Maybe a way we could adjust this would be to allow steps to opt into this.

Personally the option I currently like the best is to go the variable substitution route :D

@ghost
Copy link

ghost commented Jul 13, 2020

I actually really really like both the initial proposal - generate results from a docker image that I don't control but I know where the interesting data is - and the subsequent proposal - access stdout from previous steps so I can do cool stuff with it. I think these might be worth separating into two issues and pursuing both?

After reading through I think there might also be an alternative for accessing the stdout inspired by the fromStep idea: What if Step 2 specified a filter expression for Step 1 to run over its stdout and pass the matches along? That way we avoid writing all of Step 1 stdout to disk unless the user explicitly supplies like .* as a regex filter or something.

@dlorenc
Copy link
Contributor

dlorenc commented Jul 16, 2020

One more possible ux: https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-output-parameter

GitHub actions does something similar with special tokens in the output to denote a value. Probably a bit simpler than json, unless your tool already happens to output things in json. So maybe a tossup?

@tekton-robot
Copy link
Collaborator

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 15, 2020
@tekton-robot
Copy link
Collaborator

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

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.

@chhsia0
Copy link
Contributor

chhsia0 commented Aug 15, 2020

/remove-lifecycle rotten

@tekton-robot tekton-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 15, 2020
@chhsia0
Copy link
Contributor

chhsia0 commented Aug 15, 2020

/reopen

@tekton-robot
Copy link
Collaborator

@chhsia0: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

/reopen

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.

@chhsia0
Copy link
Contributor

chhsia0 commented Aug 15, 2020

@bobcatfish Would you mind reopening this issue?

@vdemeester
Copy link
Member

/remove-lifecycle rotten
/remove-lifecycle stale
/reopen

@tekton-robot tekton-robot reopened this Aug 17, 2020
@tekton-robot
Copy link
Collaborator

@vdemeester: Reopened this issue.

In response to this:

/remove-lifecycle rotten
/remove-lifecycle stale
/reopen

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.

@stuarthu
Copy link

maybe you can do sth like:

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: boskos-acquire
spec:
    volumes:
      - name: <mountname>
         emptyDir: {}
    steps:
    - name: boskosctl
      image: gcr.io/k8s-staging-boskos/boskosctl@sha256:a7fc984732c5dd0b4e0fe0a92e2730fa4b6bddecd0f6f6c7c6b5501abe4ab105 
      command: [sh]
      args:
      - -c
      - <boskosctl command> > <mountpath/redirectfile>
     volumeMounts:
       - name: <mountname>
          mountPath: <mountpath>
     results:
     - name: project-id
       description: The name of the project acquired
       fromStep:
       - name: boskosctl
          jsonPath: .name

@afrittoli
Copy link
Member

Oh yeah, that's a good point, I guess it is a convention for a lot of tools to output to a file based on a flag 🤔

However I'd say it's even more common for tools to output to stdout and call it a day - aka relying on "text streams" so it feels to me like there are probably a lot of cases where capturing stdout will be the only way to get the output.

If a tool works with output streams, it's really important that it does not pollute such stream, mixing output, logs and stderr al together, because doing renders the stream un-parsable at best.

From a Tekton POV I think it would be nice to provide an option to capture stdout to file, if the tool does not support it via a flag. The output is already captured by the pod, but AFAIU that includes stdout and stderr mixed together, so it may not provide what we need; we could write stdout to a file, but I would make that an opt-in, because other steps may write long logs to stdout, and we don't necessarily want to capture that on an extra file.

I would definitely not want that 🙃 This could create a lot of I/O for all task/step execution where it will not be needed.

Maybe a way we could adjust this would be to allow steps to opt into this.

Personally the option I currently like the best is to go the variable substitution route :D

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 17, 2021
@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 19, 2021
@Alveel
Copy link

Alveel commented Apr 14, 2021

/remove-lifecycle rotten

Also quite interested in these features.

@tekton-robot tekton-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Apr 14, 2021
@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 13, 2021
@muxmuse
Copy link

muxmuse commented Aug 10, 2021

  • The container contract suggest specifying a command in favor of a script
  • An image used in a step usually provides one single command. Using a script makes further assumptions about the image (e.g. bash/sh available)

That's why I think the issue should stay open.

/remove-lifecycle stale

@tekton-robot tekton-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 10, 2021
@jdurzi
Copy link

jdurzi commented Sep 15, 2021

For containers without a shell, that also lack an option to output to a file, this would be incredibly appreciated! We can't run certain tools in Tekton due to the lack of this feature (without hacky workarounds, of course)

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 14, 2021
@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 13, 2022
@tekton-robot
Copy link
Collaborator

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
10 participants