Bug in example for build-pipeline: cp -r /workspace/output/builtImage/ fails

[a-roberts]

Expected Behavior

All steps for the tutorial complete OK

Actual Behavior

{"level":"info","ts":1547725576.5358577,"logger":"fallback-logger","caller":"bash/main.go:65","msg":"Successfully executed command \"mkdir -p /pvc/build-skaffold-web/builtImage\""}
{"level":"error","ts":1547725577.9350305,"logger":"fallback-logger","caller":"bash/main.go:62","msg":"Error executing command \"cp -r /workspace/output/builtImage/. /pvc/build-skaffold-web/builtImage\" with arguments cp: can't stat '/workspace/output/builtImage/.': No such file or directory\n","stacktrace":"main.main\n\t/Users/jessicamccreery/gopath/src/github.com/knative/build-pipeline/cmd/bash/main.go:62\nruntime.main\n\t/usr/local/Cellar/go/1.10.3/libexec/src/runtime/proc.go:198"}
2019/01/17 11:46:17 exit status 1

this is taken from my colleague @jessm12's laptop, we're both working on the tutorial together and are hitting this problem. This error is from the final container (build-step-source-copy-skaffold-image-leeroy-web):

\"cp -r /workspace/output/builtImage/. /pvc/build-skaffold-web/builtImage\" with arguments cp: can't stat '/workspace/output/builtImage/.': No such file or directory being the problem.

May need an additional task? If I look at what's at my PV's location on disk, this directory is indeed empty.

We think there's a bug here whereby the image isn't copied successfully, perhaps due to a missing task where we should make the directory?

Steps to Reproduce the Problem

1. Install Docker for Desktop (edge version) and then enable Kubernetes (8 GB memory, 4 CPUs is good)
2. Follow the simple pipeline example at https://github.com/knative/build-pipeline/tree/master/examples, changing it so that the built image is on your public Dockerhub account (this requires adding a Docker basic auth secret and patching the service account to use it). We changed resources.yaml removing the results entry and we would like to make the example even easier to run on one's laptop
3. Try to run the pipeline with a kubectl apply on the run/pipeline-run.yaml: observe the error when the demo pipeline run pod fails (the last init container should error).

Update: I've since added more notes here that may be of use for running this locally for ease of a reproduce.

Additional Info

My environment is Docker for Desktop on a Mac running the edge Docker driver.

I'm pushing the built image up to my public Docker registry (this works fine, I've configured it to use a secret and patched the service account).

Perhaps it's to do with permissions and a Docker for Desktop specific problem? Under "File Sharing" I have /Users, /Volumes, /private, and /tmp as directories/subdirectories that can be bind mounded.

For some context and what's been discussed already please see https://knative.slack.com/archives/CDCQ72D0R/p1547731362165400.

[afrittoli]

/assign

[knative-prow-robot]

@afrittoli: GitHub didn't allow me to assign the following users: afrittoli.

Note that only knative members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign

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.

[afrittoli]

The problem I see here is that output_resource schedules a mkdir and copy steps for every output resource defined, regardless of the output type.

The image resource output is not implemented yet (see #216) and thus the copy fails.
One possible fix is to schedule a copy only for the resources that are implemented. This is a quick fix for this issue, but eventually we should be able to discover from the resource itself whether an output is expected and what its location is.

An alternative fix would be to fail the pipeline run if an image is used as an output resource, until #216 is implemented.

[a-roberts]

Thanks @afrittoli for taking a look!

I think we should implement option 2, so until #216 is done there's little point in anyone thinking this tutorial will completely succeed - we should add comments to that effect in the readme and mention that the functionality here isn't complete (and that could be used to foster attention to implementing what's not quite finished).

Reminded of #386 here and how important it is.

How could we fail the pipeline? Do we have an existing not yet implemented mechanism that we could leverage?

[afrittoli]

@a-roberts Fair point. I prepared #414 nonetheless, because there's not official release yet. If we can aim to implement #216 before the next release it should be fine.

[shashwathi]

/assign @afrittoli

[knative-prow-robot]

@shashwathi: GitHub didn't allow me to assign the following users: afrittoli.

Note that only knative members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign @afrittoli

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.

[afrittoli]

thanks @shashwathi - I thought only collaborators had the "assign" power but anyone could be assigned... I should have read the bot reply more carefully

[afrittoli]

/assign @afrittoli

[knative-prow-robot]

@afrittoli: GitHub didn't allow me to assign the following users: afrittoli.

Note that only knative members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign @afrittoli

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.

[afrittoli]

uhm, collaborators is not the same as "Contributor" obviously

[bobcatfish]

p.s. just a quick note: excellent write up @a-roberts, thanks for all the detail ❤️ !

[afrittoli]

@a-roberts are you satisfied that this is fixed now that #414 is merged?
Image output resources won't really be fully functional until #216 is implemented, but at least the pipeline will build the image to latest and you can deploy that then.

[a-roberts]

Hey @afrittoli, gonna give it a try now, thanks! I assume I need to build from source as there's no release yet, I'll slack you if I run into any problems.

[afrittoli]

If there's a task that uses the image as input and it takes it from a previous task, then we hit the copy step again: https://github.com/knative/build-pipeline/blob/master/pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go#L89

Does that mean that copy actually happens twice for things like git?
Another things that is not clear to me yet is boundResource.Paths - for both input and output resources the code loops over defined Paths to schedule mkdir and cp containers. If image did not define any Paths it should just work, so I'm trying to find out where is Paths defined and why.

[a-roberts]

I've spent all day trying to get the PipelineRun tutorial to work although @mnuttall has wrote a simpler example that exercises your fix and that works perfect.

He says all is well so I'm happy to close this off, cheers again 👍