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

Link inputs and outputs without using volumes #617

Closed
bobcatfish opened this issue Mar 13, 2019 · 8 comments
Closed

Link inputs and outputs without using volumes #617

bobcatfish opened this issue Mar 13, 2019 · 8 comments
Labels
design This task is about creating and discussing a design help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. meaty-juicy-coding-work This task is mostly about implementation!!! And docs and tests of course but that's a given

Comments

@bobcatfish
Copy link
Collaborator

bobcatfish commented Mar 13, 2019

Expected Behavior

When a user wants to link Pipeline Task outputs and inputs, this should:

  • Not be significantly slower than executing a Pipeline where no outputs are linked.
  • Not require external resources such as PVCs or GCS/S3 buckets

Actual Behavior

There are currently two options for how outputs and inputs are linked:

  • By default, when this happens, a PVC is created so that the content of the PVC serves as the output of a Task, which can then be provided as an input to the next Task via the volume
  • Or you can configure pipelines to use GCS to store outputs - this means they output data would be uploaded at the end of Task's execution, and downloaded to serve as input for the next

Additional Info

@bobcatfish bobcatfish added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. design This task is about creating and discussing a design meaty-juicy-coding-work This task is mostly about implementation!!! And docs and tests of course but that's a given labels Mar 13, 2019
@bobcatfish
Copy link
Collaborator Author

One possible solution is to detect when this is happening, and schedule pods for Tasks with linked inputs/outputs onto the same nodes so they can share data directly on disk.

This would be a bit of a scheduling challenge; we may need to look into using a custom scheduler.

@dlorenc
Copy link
Contributor

dlorenc commented Mar 15, 2019

Hmm, I was sort of hoping that the k8s scheduler would be smart enough to allow us to schedule subsequent tasks on the same node as the previous one when there's a shared volume (in cases where this is possible). Maybe this is true and we just need to use the scheduler differently? If not a custom scheduler should do the trick.

@alxsbn
Copy link

alxsbn commented Jul 3, 2019

Not sure about this one, probably because caching (read persistence) is a core concept of ci/cd architectures. But did you looked at emptydir ?

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
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.

/lifecycle stale

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
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 tekton-robot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Aug 12, 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.

@bobcatfish
Copy link
Collaborator Author

These days the affinity assistant helps us a lot with this; also we are still trying to determine the fate of pipelineresources #1673

pradeepitm12 referenced this issue in openshift/tektoncd-pipeline Jan 27, 2021
The testbuilder `EventListenerTriggerBinding` allows users to specify both
`name` and `ref` fields. With #617, specifying both `name` and `ref` is no
longer allowed.

We fixed the builder usage for all the happy path tests to use `name` as an
empty string but did not do it for tests that are verifying error paths (such
as validation errors). So, these tests will now all fail at the same validation
check making most of these tests useless (causing the drop in coverage in

To fix, I changed the builder to only accept `ref` so that one cannot create
invalid structs. Later, we can migrate our tests to not use builders at all and
have our error tests verify the actual error string.

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
pradeepitm12 referenced this issue in openshift/tektoncd-pipeline Jan 27, 2021
This commit adds support for a more concise syntax for TriggerBindings embedded
in a Trigger or EventListener:

NEW SYNTAX:
```
bindings:
- ref: some-name
- name: commit_id # embedded binding
  value: "$(body.head_commit_id)
```

OLD SYNTAX
```
bindings:
- name: some-name
  spec:
    params:
    - name: commit_id # embedded binding
      value: "$(body.head_commit_id)
```

The old embedded syntax is deprecated and Triggers will convert any old style
embedded bindings to the new syntax via the MutatingAdmissionWebhook.

BREAKING CHANGE: With this change, users cannot specify both `name` and `ref`
for a single binding. Use `ref` to refer to a TriggerBinding resource and
`name` for embedded bindings.

Fixes #617

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design This task is about creating and discussing a design help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. meaty-juicy-coding-work This task is mostly about implementation!!! And docs and tests of course but that's a given
Projects
None yet
Development

No branches or pull requests

4 participants