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

Add Image Output Resources #216

Open
bobcatfish opened this Issue Nov 2, 2018 · 5 comments

Comments

Projects
5 participants
@bobcatfish
Copy link
Contributor

bobcatfish commented Nov 2, 2018

Expected Behavior

If a Task declares that it produces an image output:

  1. The TaskRun controller should verify that the promised image was actually created and pushed to where it was supposed to be pushed to. If the Task does not push the image, the TaskRun should fail.
  2. The Image resource should be updated with the digest of the image created

This should leverage the design already fleshed out in knative build artifact design and the work being added to go-containerregistry in google/go-containerregistry#209 and google/go-containerregistry#261 (+ @jonjohnsonjr )

Actual Behavior

  • Currently there is no handling of outputs (only used as part of templating)
  • After #124 there should be hooks added to validate resources generically, and "file" resources will be validated, but no other types

Steps to Reproduce the Problem

  1. Create a Task that declares an image as an output, but don't actually create the image (e.g. it just runs echo)
  2. Create a TaskRun that uses that task
  3. Notice there is no error!

This means if you were relying on your pipeline to produce the image, but you made a mistake and the image was never published, you may never notice!

Additional Info

knative build artifact design

@bobcatfish bobcatfish added this to To do in Pipeline CRD via automation Nov 2, 2018

bobcatfish added a commit to bobcatfish/build-pipeline that referenced this issue Nov 2, 2018

Use image outputs in examples
While talking with @jonjohnsonjr about knative#216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (knative#108 - adding tests
for these examples - and knative#212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.

@bobcatfish bobcatfish added this to the 0.0.1 Alpha release milestone Nov 3, 2018

bobcatfish added a commit to bobcatfish/build-pipeline that referenced this issue Nov 7, 2018

Use image outputs in examples
While talking with @jonjohnsonjr about knative#216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (knative#108 - adding tests
for these examples - and knative#212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.

bobcatfish added a commit to bobcatfish/build-pipeline that referenced this issue Nov 7, 2018

Use image outputs in examples
While talking with @jonjohnsonjr about knative#216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (knative#108 - adding tests
for these examples - and knative#212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.

bobcatfish added a commit to bobcatfish/build-pipeline that referenced this issue Nov 7, 2018

Use image outputs in examples
While talking with @jonjohnsonjr about knative#216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (knative#108 - adding tests
for these examples - and knative#212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.

knative-prow-robot added a commit that referenced this issue Nov 7, 2018

Use image outputs in examples
While talking with @jonjohnsonjr about #216 I wanted to show him an
example of a Pipeline wherein there are Tasks that output Images
Resources, which are then used as inputs for subsequent Tasks. Our
examples Pipelines were meant to do that, but unfortunately when I
looked at them, I realized these were not actually hooked up properly.
The Tasks were not actually using the Images that were built by previous
Tasks, so I have updated them to do that.

I also discovered that the templating was incorrect (#108 - adding tests
for these examples - and #212 - making sure templated variables are
actually used - can't come quickly enough!) so I've updated that in the
examples as well.
@solsson

This comment has been minimized.

Copy link

solsson commented Dec 2, 2018

Maybe this belongs in #124, but it's specific to images. I'd like to offer two design ideas based on Source-to-URL use cases:

  • It's unclear to me if ImageResource's URL property should include tag or not. I assume it can, but my suggestion is to add an optional Tag property and validate that URL does not contain a :. Otherwise the ambiguity will probably lead to variations among Tasks, with some having tag as param instead.

    • Also the Tag property would be useful until we have tag-to-digest resolution, allowing Resources to set git revision and pipeline tag to the same git ref. That would let us deploy withou the caveat.
    • Even with tag-to-digest such a trick can be used to support concurrent builds.
  • For Tasks that consume the built image resource it can be useful to know the previous digest, maybe through an optional property PreviousDigest. The use case is to see, in multi-image builds like the current example, if an image has changed or not. If it hasn't maybe some resource intensive tasks like integration tests can be skipped, as can some redeployments. With build caching enabled and builds triggered on every push such checks could save lots of resources. I got the impression that for example Buildpack looks up the previous image for the URL+tag already.

afrittoli added a commit to afrittoli/build-pipeline that referenced this issue Jan 20, 2019

DNM Copy output resource to PVC only for storage output
At the moment the output_resource module unconditionally adds a
copy step for every output resource in the taskrun.
However not every output resource will generate content to be
copied. Limit the copy to PipelineResourceTypeStorage for now.

This is meant as a hotfix for bug knative#401 until an implementation
of the image output resource is available via knative#216. Until
image resource output is implemented, any consumer won't
have guarantee that the image has been actually built and pushed,
and won't know the hash of the created image.

afrittoli added a commit to afrittoli/build-pipeline that referenced this issue Jan 21, 2019

Onpy copy output resource to PVC for storage/git output
At the moment the output_resource module unconditionally adds a
copy step for every output resource in the taskrun.
However not every output resource will generate content to be
copied. Limit the copy to Storage and Git outoputs for now.

This is meant as a hotfix for bug knative#401 until an implementation
of the image output resource is available via knative#216. Until
image resource output is implemented, any consumer won't
have guarantee that the image has been actually built and pushed,
and won't know the hash of the created image.

afrittoli added a commit to afrittoli/build-pipeline that referenced this issue Jan 21, 2019

Onpy copy output resource to PVC for storage/git output
At the moment the output_resource module unconditionally adds a
copy step for every output resource in the taskrun.
However not every output resource will generate content to be
copied. Limit the copy to Storage and Git outoputs for now.

This is meant as a hotfix for bug knative#401 until an implementation
of the image output resource is available via knative#216. Until
image resource output is implemented, any consumer won't
have guarantee that the image has been actually built and pushed,
and won't know the hash of the created image.

knative-prow-robot added a commit that referenced this issue Jan 22, 2019

Onpy copy output resource to PVC for storage/git output
At the moment the output_resource module unconditionally adds a
copy step for every output resource in the taskrun.
However not every output resource will generate content to be
copied. Limit the copy to Storage and Git outoputs for now.

This is meant as a hotfix for bug #401 until an implementation
of the image output resource is available via #216. Until
image resource output is implemented, any consumer won't
have guarantee that the image has been actually built and pushed,
and won't know the hash of the created image.
@afrittoli

This comment has been minimized.

Copy link
Contributor

afrittoli commented Jan 23, 2019

If no-one else is looking at this I can give it a go and propose a design. Is there a special template to be used? Or any google doc should be fine?

@vdemeester

This comment has been minimized.

Copy link
Member

vdemeester commented Jan 24, 2019

@afrittoli I think any google doc should be fine at start, but we may want to have a special template in the near future (maybe we do cc @bobcatfish @ImJasonH ?)

@afrittoli

This comment has been minimized.

Copy link
Contributor

afrittoli commented Jan 24, 2019

@shashwathi @bobcatfish In the description of this issue it talks about generic verification hooks to be introduced by #124; even though #124 is closed it doesn't seem to me that they have been implemented yet. Is that right? Is there a design for those yet or anyone working on them, or shall I include that in the design for this #216?

zhyuri added a commit to zhyuri/build-pipeline that referenced this issue Jan 24, 2019

Onpy copy output resource to PVC for storage/git output
At the moment the output_resource module unconditionally adds a
copy step for every output resource in the taskrun.
However not every output resource will generate content to be
copied. Limit the copy to Storage and Git outoputs for now.

This is meant as a hotfix for bug knative#401 until an implementation
of the image output resource is available via knative#216. Until
image resource output is implemented, any consumer won't
have guarantee that the image has been actually built and pushed,
and won't know the hash of the created image.
@vbatts

This comment has been minimized.

Copy link
Contributor

vbatts commented Feb 1, 2019

Maybe this belongs in #124, but it's specific to images. I'd like to offer two design ideas based on Source-to-URL use cases:

* It's unclear to me if ImageResource's `URL` property should include tag or not. I assume it can, but my suggestion is to add an optional `Tag` property and validate that URL does not contain a `:`. Otherwise the ambiguity will probably lead to variations among Tasks, with some having tag as param instead.
  
  * Also the Tag property would be useful until we have tag-to-digest resolution, allowing Resources to set `git` `revision` and `pipeline` `tag` to the same git ref. That would let us deploy withou the [caveat](https://github.com/knative/build-pipeline/commit/54051bf06f22f15260b9d4dd87f6b6c82e7c03fe#diff-1c0f522a61adc59307209c8e0296db49R32).
  * Even with tag-to-digest such a trick can be used to support concurrent builds.

* For Tasks that consume the built image resource it can be useful to know the previous digest, maybe through an optional property `PreviousDigest`. The use case is to see, in multi-image builds like the current example, if an image has changed or not. If it hasn't maybe some resource intensive tasks like integration tests can be skipped, as can some redeployments. With build caching enabled and builds triggered on every push such checks could save lots of resources. I got the impression that for example Buildpack looks up the previous image for the URL+tag already.

Cool cool. As for "tags" (see also "labels" or "annotations" (I know annotations in k8s are even more heavily utilitized)), hopefully we can make reuse of keys like https://github.com/opencontainers/image-spec/blob/master/annotations.md#pre-defined-annotation-keys and the label-schema.org effort.
So as we could have build artifacts like an OCI image-layout these "tags" could be more shared and widely interpreted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment