{"action":"created","issue":{"url":"https://api.github.com/repos/knative/build-pipeline/issues/200","repository_url":"https://api.github.com/repos/knative/build-pipeline","labels_url":"https://api.github.com/repos/knative/build-pipeline/issues/200/labels{/name}","comments_url":"https://api.github.com/repos/knative/build-pipeline/issues/200/comments","events_url":"https://api.github.com/repos/knative/build-pipeline/issues/200/events","html_url":"https://github.com/knative/build-pipeline/issues/200","id":375265621,"node_id":"MDU6SXNzdWUzNzUyNjU2MjE=","number":200,"title":"Ability to use same Pipeline with different Resources","user":{"login":"bobcatfish","id":432502,"node_id":"MDQ6VXNlcjQzMjUwMg==","avatar_url":"https://avatars2.githubusercontent.com/u/432502?v=4","gravatar_id":"","url":"https://api.github.com/users/bobcatfish","html_url":"https://github.com/bobcatfish","followers_url":"https://api.github.com/users/bobcatfish/followers","following_url":"https://api.github.com/users/bobcatfish/following{/other_user}","gists_url":"https://api.github.com/users/bobcatfish/gists{/gist_id}","starred_url":"https://api.github.com/users/bobcatfish/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bobcatfish/subscriptions","organizations_url":"https://api.github.com/users/bobcatfish/orgs","repos_url":"https://api.github.com/users/bobcatfish/repos","events_url":"https://api.github.com/users/bobcatfish/events{/privacy}","received_events_url":"https://api.github.com/users/bobcatfish/received_events","type":"User","site_admin":false},"labels":[{"id":1089279988,"node_id":"MDU6TGFiZWwxMDg5Mjc5OTg4","url":"https://api.github.com/repos/knative/build-pipeline/labels/design","name":"design","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":{"login":"bobcatfish","id":432502,"node_id":"MDQ6VXNlcjQzMjUwMg==","avatar_url":"https://avatars2.githubusercontent.com/u/432502?v=4","gravatar_id":"","url":"https://api.github.com/users/bobcatfish","html_url":"https://github.com/bobcatfish","followers_url":"https://api.github.com/users/bobcatfish/followers","following_url":"https://api.github.com/users/bobcatfish/following{/other_user}","gists_url":"https://api.github.com/users/bobcatfish/gists{/gist_id}","starred_url":"https://api.github.com/users/bobcatfish/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bobcatfish/subscriptions","organizations_url":"https://api.github.com/users/bobcatfish/orgs","repos_url":"https://api.github.com/users/bobcatfish/repos","events_url":"https://api.github.com/users/bobcatfish/events{/privacy}","received_events_url":"https://api.github.com/users/bobcatfish/received_events","type":"User","site_admin":false},"assignees":[{"login":"bobcatfish","id":432502,"node_id":"MDQ6VXNlcjQzMjUwMg==","avatar_url":"https://avatars2.githubusercontent.com/u/432502?v=4","gravatar_id":"","url":"https://api.github.com/users/bobcatfish","html_url":"https://github.com/bobcatfish","followers_url":"https://api.github.com/users/bobcatfish/followers","following_url":"https://api.github.com/users/bobcatfish/following{/other_user}","gists_url":"https://api.github.com/users/bobcatfish/gists{/gist_id}","starred_url":"https://api.github.com/users/bobcatfish/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bobcatfish/subscriptions","organizations_url":"https://api.github.com/users/bobcatfish/orgs","repos_url":"https://api.github.com/users/bobcatfish/repos","events_url":"https://api.github.com/users/bobcatfish/events{/privacy}","received_events_url":"https://api.github.com/users/bobcatfish/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/knative/build-pipeline/milestones/2","html_url":"https://github.com/knative/build-pipeline/milestone/2","labels_url":"https://api.github.com/repos/knative/build-pipeline/milestones/2/labels","id":3792285,"node_id":"MDk6TWlsZXN0b25lMzc5MjI4NQ==","number":2,"title":"0.0.1 Alpha release","description":"The goal of this milestone is to get the project to the point that it's stable enough that folks can actually use it.","creator":{"login":"bobcatfish","id":432502,"node_id":"MDQ6VXNlcjQzMjUwMg==","avatar_url":"https://avatars2.githubusercontent.com/u/432502?v=4","gravatar_id":"","url":"https://api.github.com/users/bobcatfish","html_url":"https://github.com/bobcatfish","followers_url":"https://api.github.com/users/bobcatfish/followers","following_url":"https://api.github.com/users/bobcatfish/following{/other_user}","gists_url":"https://api.github.com/users/bobcatfish/gists{/gist_id}","starred_url":"https://api.github.com/users/bobcatfish/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bobcatfish/subscriptions","organizations_url":"https://api.github.com/users/bobcatfish/orgs","repos_url":"https://api.github.com/users/bobcatfish/repos","events_url":"https://api.github.com/users/bobcatfish/events{/privacy}","received_events_url":"https://api.github.com/users/bobcatfish/received_events","type":"User","site_admin":false},"open_issues":28,"closed_issues":5,"state":"open","created_at":"2018-11-03T00:02:14Z","updated_at":"2018-11-10T01:48:31Z","due_on":"2018-12-14T08:00:00Z","closed_at":null},"comments":7,"created_at":"2018-10-30T00:03:03Z","updated_at":"2018-11-10T02:19:01Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"(Discovered by @pivotal-nader-ziada while working on #177)\r\n\r\nThis issue is about deciding on the solution we prefer (see \"possible solutions\" below) and implementing the change.\r\n\r\n## Expected Behavior\r\n\r\nA user should be able to take an existing `Pipeline` and run it against their own setup without\r\nmodifying the `Pipeline` itself, meaning using their own:\r\n\r\n1. GitHub fork / branch (e.g. for PR testing)\r\n2. Image registry\r\n3. k8s cluster (to deploy to)\r\n\r\n## Actual Behavior\r\n\r\nIn the current implementation, to change (3), a user would create their own `PipelineParams`, so they would not need to change the `Pipeline`.\r\n\r\nBut to change (1) or (2), they would have to create new `Resources` with their specific configuration. Since `Resources` are bound in the `Pipeline`, this means they would need to change the `Pipeline` as well.\r\n\r\nWe are considering moving `clusters` out of `PipelineParams` and into a `Resource` type (#177), but this would mean that for a user to use their own cluster, they would again need to change the `Pipeline`. \r\n\r\n## Possible solutions\r\n\r\n@pivotal-nader-ziada and I have prototyped 7 possible solutions in yaml [in this branch](https://github.com/bobcatfish/build-pipeline/tree/pipeline_reuse/examples_pipeline_reuse).\r\n\r\n1. We expand `PipelineParams` to include a generic list of keys and values, which can be templated\r\n   inside all types (e.g. `Pipelines`, `Resources`, etc.).\r\n\r\n   See [example_option_1](https://github.com/bobcatfish/build-pipeline/tree/pipeline_reuse/examples_pipeline_reuse/example_option_1):\r\n\r\n   * `pipelines/kritis-resources.yaml` contains templating for values like URL and revision\r\n     in the `Resources`\r\n\r\n ```yaml\r\napiVersion: pipeline.knative.dev/v1alpha1\r\nkind: PipelineResource\r\nmetadata:\r\n  name: kritis-resources-git\r\n  namespace: default\r\nspec:\r\n  type: git\r\n  params:\r\n  - name: url\r\n    value: ${url}\r\n  - name: revision\r\n    value: ${revision}\r\n```\r\n\r\n   * `pipelineparams.yaml` contains the values for these params\r\n\r\n```yaml\r\n    params:\r\n    - name: resources.kritis-resources-git.url\r\n      value: https://github.com/grafeas/kritis\r\n    - name: resources.kritis-resources-git.revision\r\n      value: master\r\n    - name: resources.kritis-resources-test-git.url\r\n      value: https://github.com/grafeas/kritis-test\r\n    - name: resources.kritis-resources-image.url\r\n      value: gcr.io/staging-image\r\n    - name: resoruces.kritis-test-cluster.endpoint\r\n      value: 'https://prod.gke.corp.com'\r\n```\r\n\r\n   _This is very similar to option 3, the difference is that in this version we still have\r\n   `Resource` CRDs, which would need to be updated on any changes to `PipelineParams`.\r\n\r\n2. `PipelineParams` can override values inside of `Resources`.\r\n\r\n   _This would be basically the same as option 1, except limited to `Resources` instead of working\r\n   for any CRD._\r\n\r\n3. Instead of declaring `Resources` as separate CRDs, we define them all inside `PipelineParams`\r\n\r\n   See [example_option_3](https://github.com/bobcatfish/build-pipeline/tree/pipeline_reuse/examples_pipeline_reuse/example_option_3):\r\n\r\n   * `pipelineparams.yaml` now contains all Resource definitions\r\n\r\n```yaml\r\n    resources:\r\n      - name: kritis-resources-git\r\n        spec:\r\n          type: git\r\n          params:\r\n          - name: url\r\n            value: https://github.com/grafeas/kritis\r\n          - name: revision\r\n            value: master\r\n      - name: kritis-resources-test-git\r\n        spec:    \r\n          type: git\r\n          params:\r\n          - name: revision\r\n            value: master\r\n          - name: url\r\n            value: https://github.com/grafeas/kritis-test  \r\n      - name: kritis-resources-image\r\n        spec:  \r\n          type: image\r\n          params:\r\n          - name: url\r\n            value: gcr.io/staging-image\r\n      - name: kritis-test-cluster\r\n        spec:  \r\n          type: cluster\r\n          params:\r\n          - name: endpoint\r\n            value: 'https://prod.gke.corp.com' \r\n```\r\n\r\n   * Resources are no longer their own type of CRD\r\n\r\n4. Instead of binding `Resources` in `Pipeline`, we bind them in `PipelineRun` (and `TaskRun`)\r\n\r\n   See [example_option_4](https://github.com/bobcatfish/build-pipeline/tree/pipeline_reuse/examples_pipeline_reuse/example_option_4):\r\n\r\n   * `kritis-pipeline-run.yaml` now contains all resource bindings\r\n\r\n```yaml\r\n    taskSourceBindings:\r\n    - name: unit-test-kritis\r\n      inputSourceBindings:\r\n      - key: workspace # bind to the name in the task\r\n        resourceRef:\r\n          name: kritis-resources-git\r\n    - name: push-kritis\r\n      inputSourceBindings:\r\n        - key: workspace # bind to the name in the task\r\n          resourceRef:\r\n            name: kritis-resources-git\r\n      outputSourceBindings:\r\n        - key: builtImage # bind to the name in the task\r\n          resourceRef:\r\n            name: kritis-resources-image\r\n```\r\n\r\n   * `pipeline/kritis.yaml` still contains the `passedConstraints` so that it\r\n     can express the order\r\n\r\n```yaml\r\n    tasks:\r\n      - name: unit-test-kritis          # 1.  Run unit Tests\r\n        taskRef:\r\n          name: make\r\n        params:\r\n          - name: makeTarget\r\n            value: test\r\n      - name: push-kritis               # 2.  Build And Push Tests\r\n        taskRef:\r\n          name: build-push\r\n        inputSourceBindings:\r\n          - key: workspace\r\n            passedConstraints: [unit-test-kritis]\r\n        params:\r\n          - name: pathToDockerfile\r\n            value: deploy/Dockerfile\r\n      - name: deploy-test-env           # 3. Finally Deploy to Test environment\r\n        taskRef:\r\n          name: deploy-with-helm\r\n        inputSourceBindings:\r\n          - key: workspace\r\n            passedConstraints: [push-kritis]\r\n```\r\n\r\n   Cons:\r\n\r\n   - Resources are now handled in both the Pipeline _and_ the PipelineRun\r\n\r\n5. A combination of (4) + (2): if you _want_ to override a value, you can do that by binding an overridden resource in (4).\r\n\r\n   _We ruled this one out because we feel it is too confusing._\r\n\r\n\r\n6. No `PipelineParams`; people need to change the Pipeline. Move the ServiceAccount and Result store information into the Pipeline itself.\r\n\r\n   See [example_option_6](https://github.com/bobcatfish/build-pipeline/tree/pipeline_reuse/examples_pipeline_reuse/example_option_6):\r\n\r\n   * The `PipelineParams` are gone, values are moved into `pipeline/kritis.yaml`\r\n\r\n```yaml\r\nspec:\r\n    tasks:\r\n    ....\r\n    serviceAccount: 'demoServiceAccount'\r\n    results:\r\n        runs:\r\n          name: 'runsBucket'\r\n          type: 'gcs'\r\n          url: 'gcs://somebucket/results/runs'\r\n        logs:\r\n          name: 'logBucket'\r\n          type: 'gcs'\r\n          url: 'gcs://somebucket/results/logs'\r\n        tests:\r\n          name: 'testBucket'\r\n          type: 'gcs'\r\n          url: 'gcs://somebucket/results/tests'        \r\n```\r\n\r\n   * The `PipelineRun` in `invocations/kritis-pipeline-run.yaml` now has very\r\n    little in it.\r\n\r\n```yaml\r\napiVersion: pipeline.knative.dev/v1alpha1\r\nkind: PipelineRun\r\nmetadata:\r\n  name: kritis-pipeline-run-12321312984\r\n  namespace: default\r\nspec:\r\n    pipelineRef:\r\n        name: kritis-pipeline\r\n    triggerRef:\r\n        type: manual\r\n```\r\n\r\n   Cons:\r\n\r\n   - Still need to change 2 things: create a Resource with the configuration you\r\n  want, and change the Pipeline to use that Resoruce\r\n\r\n7. Either (1) or (2) but instead of using `PipelineParams`, use `PipelineRun`\r\n   and `TaskRun`."},"comment":{"url":"https://api.github.com/repos/knative/build-pipeline/issues/comments/437550658","html_url":"https://github.com/knative/build-pipeline/issues/200#issuecomment-437550658","issue_url":"https://api.github.com/repos/knative/build-pipeline/issues/200","id":437550658,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNzU1MDY1OA==","user":{"login":"bobcatfish","id":432502,"node_id":"MDQ6VXNlcjQzMjUwMg==","avatar_url":"https://avatars2.githubusercontent.com/u/432502?v=4","gravatar_id":"","url":"https://api.github.com/users/bobcatfish","html_url":"https://github.com/bobcatfish","followers_url":"https://api.github.com/users/bobcatfish/followers","following_url":"https://api.github.com/users/bobcatfish/following{/other_user}","gists_url":"https://api.github.com/users/bobcatfish/gists{/gist_id}","starred_url":"https://api.github.com/users/bobcatfish/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bobcatfish/subscriptions","organizations_url":"https://api.github.com/users/bobcatfish/orgs","repos_url":"https://api.github.com/users/bobcatfish/repos","events_url":"https://api.github.com/users/bobcatfish/events{/privacy}","received_events_url":"https://api.github.com/users/bobcatfish/received_events","type":"User","site_admin":false},"created_at":"2018-11-10T02:19:01Z","updated_at":"2018-11-10T02:19:01Z","author_association":"CONTRIBUTOR","body":"@pivotal-nader-ziada I'm also leaning toward (4) now that we talk about it more\r\n\r\nDo you have any thoughts on how we can improve this situation where we now have to talk about Resources in both the Pipeline (b/c we have to express order) AND the PipelineRun?\r\n\r\nFor example in the Pipeline:\r\n```\r\n      - name: push-kritis               # 2.  Build And Push Tests\r\n        taskRef:\r\n          name: build-push\r\n         inputSourceBindings:\r\n          - key: workspace\r\n            passedConstraints: [unit-test-kritis]\r\n        params:\r\n          - name: pathToDockerfile\r\n            value: deploy/Dockerfile\r\n```\r\n\r\nAnd in the PipelineRun:\r\n```yaml\r\n - name: push-kritis\r\n      inputSourceBindings:\r\n        - key: workspace # bind to the name in the task\r\n          resourceRef:\r\n            name: kritis-resources-git\r\n      outputSourceBindings:\r\n        - key: builtImage # bind to the name in the task\r\n          resourceRef:\r\n            name: kritis-resources-image\r\n```\r\n\r\nMy gut reaction is to suggest that we kill two birds with one stone, and tackle #137 by making the `passedConstraint` an attribute of the Task instead of the Resource, e.g. the first snippet in the above example would become:\r\n\r\n```yaml\r\n      - name: push-kritis               # 2.  Build And Push Tests\r\n        taskRef:\r\n          name: build-push\r\n          passedConstraints: [unit-test-kritis] \r\n        params:\r\n          - name: pathToDockerfile\r\n            value: deploy/Dockerfile\r\n```\r\n\r\nThis way the Pipeline itself only expresses the ordering.\r\n\r\nNot sure if the Params should stay in the Pipeline in this design - @dlorenc has pointed out that params are looking more and more like Resources also."}}
{"actor":{"display_login":"bobcatfish"}}