The Wayback Machine - https://web.archive.org/web/20201020201712/https://github.com/openshift/pipelines-tutorial/pull/95
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

Adding VolumeClaimTemplate to pipelines-tutorial #95

Merged

Conversation

@VeereshAradhya
Copy link
Contributor

@VeereshAradhya VeereshAradhya commented Sep 7, 2020

  • Added little info about VolumeClaimTemplate and updated the tkn command in README.md to use volumeClaimTemplate
  • Updated demo.sh to use VolumeClaimTemplate for the vote-ui pipelinerun
Copy link
Contributor

@savitaashture savitaashture left a comment

Thank you 👍


```bash
$ tkn pipeline start build-and-deploy \
-w name=shared-workspace,claimName=source-pvc \
-w name=shared-workspace,volumeClaimTemplateFile=https://raw.githubusercontent.com/openshift/pipelines-tutorial/master/01_pipeline/03_persistent_volume_claim.yaml \

This comment has been minimized.

@savitaashture

savitaashture Sep 8, 2020
Contributor

Can we do the same changes here https://github.com/openshift/pipelines-tutorial/blob/master/02_pipelinerun/02_build_deploy_ui_pipelinerun.yaml#L17-L18 so that it will consistent while using tkn or yaml for vote-ui

@VeereshAradhya VeereshAradhya force-pushed the VeereshAradhya:volumeClaimTemplate-tut branch from 4ce2d2a to d05fb9f Sep 8, 2020
@piyush-garg
Copy link
Contributor

@piyush-garg piyush-garg commented Sep 8, 2020

I think we can use volumeClaimTemplate in both pipelinerun, this means we can remove PVC yaml also update the docs accordingly

@VeereshAradhya
Copy link
Contributor Author

@VeereshAradhya VeereshAradhya commented Sep 8, 2020

@piyush-garg The idea is to use both VolumeClaimTemplate and PersistentVolumeClaim in the tutorial and by doing that we are providing example usages of both VolumeClaimTemplate and PersistentVolumeClaim to the user

@khrm
Copy link
Contributor

@khrm khrm commented Sep 8, 2020

I think we try to keep tutorial simple. That's why we will move to single git repo example. So having only one approach is fine too.

@VeereshAradhya
Copy link
Contributor Author

@VeereshAradhya VeereshAradhya commented Sep 8, 2020

@Preeticp @savitaashture WDYT? If you all agree I will update the PR to contain only VolumeClaimTemplate

@savitaashture
Copy link
Contributor

@savitaashture savitaashture commented Sep 9, 2020

Actually it will be good if we show how we can use VolumeClaimTemplate and PersistentVolumeClaim
but if we are considering to keep tutorial simple then it make sense to use VolumeClaimTemplate 👍

+1 to use VolumeClaimTemplate

@VeereshAradhya VeereshAradhya force-pushed the VeereshAradhya:volumeClaimTemplate-tut branch from d05fb9f to c05b869 Sep 9, 2020
Copy link
Contributor

@savitaashture savitaashture left a comment

Looks good to me 👍

Minor change

@@ -45,7 +45,7 @@ The custom resources needed to define a pipeline are listed below:
In short, in order to create a pipeline, one does the following:
* Create custom or install [existing](https://github.com/tektoncd/catalog) reusable `Tasks`
* Create a `Pipeline` and `PipelineResources` to define your application's delivery pipeline
* Create a `PersistentVolumeClaim` to provide the volume/filesystem for pipeline execution
* Create a `PersistentVolumeClaim` to provide the volume/filesystem for pipeline execution or provide a `VolumeClaimTemplate` which creates a `PersistentVolumeClaim`

This comment has been minimized.

@savitaashture

savitaashture Sep 10, 2020
Contributor

I think we need to change this sentence bit because we are not creating PersistentVolumeClaim now

This comment has been minimized.

@VeereshAradhya

VeereshAradhya Sep 10, 2020
Author Contributor

It's just extra information. Anyway even VolumeClaimTemplate creates a PVC. I think it's ok to keep it :)

@piyush-garg
Copy link
Contributor

@piyush-garg piyush-garg commented Sep 10, 2020

/lgtm

@openshift-ci-robot
Copy link

@openshift-ci-robot openshift-ci-robot commented Sep 10, 2020

@piyush-garg: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

Copy link
Contributor

@savitaashture savitaashture left a comment

/lgtm

@openshift-ci-robot
Copy link

@openshift-ci-robot openshift-ci-robot commented Sep 16, 2020

@savitaashture: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

Copy link
Member

@vdemeester vdemeester left a comment

/lgtm

@openshift-ci-robot
Copy link

@openshift-ci-robot openshift-ci-robot commented Sep 16, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: piyush-garg, savitaashture, vdemeester, VeereshAradhya

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

@openshift-merge-robot openshift-merge-robot merged commit b2a4d5d into openshift:master Sep 16, 2020
1 check was pending
1 check was pending
tide Not mergeable. Needs approved, lgtm labels.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
You can’t perform that action at this time.