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

Feature flags to trim tekton results EOF newlines #3146

Closed
Tomcli opened this issue Aug 27, 2020 · 5 comments
Closed

Feature flags to trim tekton results EOF newlines #3146

Tomcli opened this issue Aug 27, 2020 · 5 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@Tomcli
Copy link
Contributor

Tomcli commented Aug 27, 2020

Feature request

Related #2936
Some of our users have pre-defined container that doesn't trim the outputs. It is quite troublesome to ask everyone to fix and rebuild their container especially when we are building a pipeline that's depended on each other's work. Is it possible to add a feature flag for Tekton to handle it? Thank you in advance.

Here is an example to reproduce this:

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: test-params
spec:
  params:
  - name: project_name
    value: 'a-project'
  - name: notebook_name
    value: 'a-notebook'
  pipelineSpec:
    params:
    - name: project_name
    - name: notebook_name
    tasks:
    - name: find-project
      params:
      - name: project_name
        value: $(params.project_name)
      taskSpec:
        params:
        - name: project_name
        results:
        - description: /tmp/outputs/project/data
          name: project
        steps:
        - command:
          - sh
          - -ex
          - -c
          - "echo \"$0\" > \"$1\""
          - $(inputs.params.project_name)
          - $(results.project.path)
          image: alpine:3.12
          name: main
    - name: find-asset
      params:
      - name: find-project-project
        value: $(tasks.find-project.results.project)
      - name: notebook_name
        value: $(params.notebook_name)
      taskSpec:
        params:
        - name: find-project-project
        - name: notebook_name
        steps:
        - command:
          - sh
          - -ex
          - -c
          - "echo \"'$0'\"\n echo \"'$1'\""
          - $(inputs.params.find-project-project)
          - $(inputs.params.notebook_name)
          image: alpine:3.12
          name: main

Tracking issue on our repo: kubeflow/kfp-tekton#273

/cc @pritidesai @animeshsingh

Use case

@Tomcli Tomcli added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 27, 2020
@ghost
Copy link

ghost commented Aug 28, 2020

Thanks for the issue report. This has also recently come up in the context of stdout / stderr capture (comment thread here).

I agree I think this would be useful. Here's the way it might work:

# in a TaskSpec
results:
- name: my-result
  description: Foo bar result description.
  trimWhitespace: true

I hesitate to add yet more flags to the entrypoint binary so I think right now my preference would be that the Tekton Controller is responsible for performing the trimming after the result is sent back to it but that's an implementation detail. I'll write a TEP for it.

@ghost ghost self-assigned this Aug 28, 2020
@XinruZhang
Copy link
Member

XinruZhang commented Sep 15, 2020

I'm interested in working on it and will work with @sbwsg together :)
/assign @XinruZhang

@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, 2020
@ghost
Copy link

ghost commented Dec 14, 2020

In summary the feedback on this feature's TEP boiled down to the fact that working around this problem is quite easy for users. Removing trailing spaces can be done in the Task directly or by adding a Task to their pipeline to perform string trimming. Therefore it doesn't seem like this is a feature the Pipelines project should pursue. That doesn't preclude this being something the Pipelines project takes on in future when a clear need presents itself that users are unable to work around, but right now we don't see evidence of that kind of problem.

/close

@tekton-robot
Copy link
Collaborator

@sbwsg: Closing this issue.

In response to this:

In summary the feedback on this feature's TEP boiled down to the fact that working around this problem is quite easy for users. Removing trailing spaces can be done in the Task directly or by adding a Task to their pipeline to perform string trimming. Therefore it doesn't seem like this is a feature the Pipelines project should pursue. That doesn't preclude this being something the Pipelines project takes on in future when a clear need presents itself that users are unable to work around, but right now we don't see evidence of that kind of problem.

/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.

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/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

3 participants