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.

Sign up for GitHub

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

Jump to bottom

Introduce workspaces to Pipelines. #1866

Merged
merged 1 commit into from Jan 22, 2020
Merged

Introduce workspaces to Pipelines. #1866

merged 1 commit into from Jan 22, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jan 13, 2020 •
edited by ghost

A workspace can be threaded through the tasks of a Pipeline, allowing easier
reuse and customisation of shared storage between tasks.

Workspace types have been added to Pipeline structs:

  • A Pipeline declares a list of workspace names that it requires be provided
    by a PipelineRun.
  • A PipelineTask declares a mapping from a Pipeline's workspace name to a
    workspace name declared by the Task it references (or embeds).
  • A PipelineRun declares the concrete workspace implementations - whether
    they be a PVC, emptyDir, configMap or secret - and passes those in with
    the name declared by the Pipeline it references (or embeds).

An example PipelineRun has been added to demonstrate usage of
workspaces in pipelines but I still need to add docs and tests.

TODOs:

  • Document that pipeline task ordering is not yet enforced when tasks share workspaces. Must use runAfter for now.
  • Document emptyDir not being shared across tasks in pipelinerun
  • Document configmap and secret workspace support in the pipelineruns doc.
  • Document configmap size limit. (Does secret have same size limit? Yes)
  • E2E test: check that validation fails when workspace provided more than once by pipelinerun
  • E2E test: check that submitting pipelinerun with missing workspace results in error

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

Release Notes

Pipelines and PipelineRuns can now declare workspaces to more easily thread shared storage through the tasks in a pipeline

@tekton-robot tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 13, 2020
@googlebot googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Jan 13, 2020
@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 13, 2020
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.9% 84.8% -3.0

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.9% 84.8% -3.0

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.9% 84.8% -3.0

chmouel
chmouel reviewed Jan 15, 2020
View reviewed changes
docs/pipelineruns.md Outdated Show resolved Hide resolved
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.9% 84.8% -3.0

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.9% 84.8% -3.0

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.9% 84.8% -3.0

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.9% 88.3% 0.4

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.9% 88.3% 0.4

@ghost ghost changed the title WIP Introduce workspaces to Pipelines. Introduce workspaces to Pipelines. Jan 17, 2020
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 17, 2020
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 88.2% 87.4% -0.8

pierretasci
pierretasci approved these changes Jan 17, 2020
View reviewed changes
Copy link
Contributor

@pierretasci pierretasci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, AFAIK though I think you still need an owner's review.

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.3% 86.6% -0.7

@tekton-robot tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 21, 2020
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.3% 86.6% -0.7

vdemeester
vdemeester reviewed Jan 21, 2020
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/meow

/cc @bobcatfish

@tekton-robot
Copy link
Collaborator

@vdemeester: cat image

In response to this:

/lgtm
/meow

/cc @bobcatfish

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.

@tekton-robot tekton-robot added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Jan 21, 2020
@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.3% 86.6% -0.7

This commit introduces workspaces for Pipelines.
0d07da4 
A workspace can be threaded through the tasks of a Pipeline, allowing easier
reuse and customisation of shared storage between tasks.

Workspace types have been added to Pipeline structs:

* A Pipeline declares a list of workspace names that it requires be provided
by a PipelineRun.
* A PipelineTask declares a mapping from a Pipeline's workspace name to a
workspace name declared by the Task it references (or embeds).
* A PipelineRun declares the concrete workspace implementations - whether
they be a PVC, emptyDir, configMap or secret - and passes those in with
the name declared by the Pipeline it references (or embeds).

An example PipelineRun has been added to demonstrate usage of the
workspaces in pipelines.

Outstanding TODOs:

* Add tests
@ghost
Copy link
Author

ghost commented Jan 21, 2020

OK, barring any last build failures or reviewer items I think this PR is in a finished state.

@tekton-robot
Copy link
Collaborator

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
test/builder/pipeline.go 87.3% 86.6% -0.7

@chmouel
Copy link
Member

chmouel commented Jan 22, 2020

nicely done @sbwsg 👍

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2020
vdemeester
vdemeester approved these changes Jan 22, 2020
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/meow

🎉

@tekton-robot
Copy link
Collaborator

@vdemeester: cat image

In response to this:

/lgtm
/meow

🎉

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.

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierretasci, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 22, 2020
@tekton-robot tekton-robot merged commit fd28f51 into tektoncd:master Jan 22, 2020
@vdemeester vdemeester mentioned this pull request Jan 22, 2020
Closed
@ghost
Copy link
Author

ghost commented Jan 22, 2020

🙏 thank you!

@ghost ghost deleted the pipeline-workspaces branch January 22, 2020 15:34
@vdemeester vdemeester mentioned this pull request Jan 27, 2020
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afrittoli afrittoli Awaiting requested review from afrittoli

@dibyom dibyom Awaiting requested review from dibyom

@bobcatfish bobcatfish Awaiting requested review from bobcatfish

Assignees

@vdemeester vdemeester

@chmouel chmouel

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tekton-robot @chmouel @vdemeester @pierretasci @googlebot