Add support for storage resource #321

shashwathi · 2018-12-09T02:13:11Z

Add support for generic storage blob resource.
Add implementation for gcs as of now, resource declaration
includes bucket location(url), type params to configure credentials for private
bucket.
If storage resource is added as input resource then updated build steps to pull
the blob storage. Params of resources are set as environment variables
for private buckets.
If storage is added as output resource then build step is added to
update the blob to the bucket after execution of build steps.
Add documentation how to use declare resource
Refactor design output resources

Co-authored-by: Maria Ntalla [email protected]

Fixes #124

cc @bobcatfish @pivotal-nader-ziada

googlebot · 2018-12-09T02:13:13Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

googlebot · 2018-12-09T02:36:05Z

CLAs look good, thanks!

config/controller.yaml

docs/using.md

nader-ziada · 2018-12-10T18:23:16Z

docs/using.md

+      secretName: service_account.json
+```
+
+Note: `GOOGLE_APPLICATION_CREDENTIALS` is the environment variable to be set for the container to download/upload GCS resource.


is this field in the resource? or a different env variable user has to set?

is this field in the resource?

No.If storage resource is private buckets then secret params expects this env variable to be set. I will update the docs to make this more clear. I did not want to hard code this env variable in resource definition because in future we could update to passing token instead of service account with minimal code change.

pkg/apis/pipeline/v1alpha1/gcs_resource.go

nader-ziada · 2018-12-10T18:26:43Z

pkg/apis/pipeline/v1alpha1/gcs_resource_test.go

+		Secrets: []SecretParam{{
+			SecretName: "secretName",
+			SecretKey:  "secretKey",
+			FieldName:  "GOOGLE_APPLICATION_CREDENTIALS",


I think this should be a field in the resource and not relying on the env variable

If bucket is public then user doesn't have to set it. Then we need some boolean flag to decide whether we want this variable or not. Interface will become complex for no reason

so now we need the user to set an env variable of the bucket is private?

pkg/apis/pipeline/v1alpha1/metadata_validation_test.go

pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go

nader-ziada · 2018-12-10T19:24:27Z

pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go

@@ -65,12 +73,13 @@ func AddInputResource(
 			return nil, fmt.Errorf("task %q failed to Get Pipeline Resource: %q", taskName, boundResource)
 		}

+		// git resource sets source in build so execute this always.


we don't support the git repo moving from one task to the next?

We do. Implementation gets unnecessarily complex with git being inherently supported by build as source and rest of them not. Let me try simplify this a bit more.

bkuschel · 2018-12-11T15:12:13Z

pkg/apis/pipeline/v1alpha1/storage_resource.go

+
+	for _, param := range r.Spec.Params {
+		switch {
+		case strings.EqualFold(param.Name, "type"):


in the two scenarios I know about (gs and s3) the type can be derived from the protocol segment of the "location". Do we need type in those cases?

type here refers to "gcs" and not the type of object downloading. Clients are configured to deduce the format of object from URL

the prefix of the location s3://my_bucket/ or gs://my_bucket isn't actually the object type but the protocol specifier which is usually specific to the underlying API. I suppose there may be some storage services that doesn't have a protocol as part of location (azure?) so it may be relevant in such cases.
In any case, I don't think it's a big deal.

shashwathi · 2018-12-11T18:06:27Z

@pivotal-nader-ziada : I have addressed all your comments. Ready for another round of review.

bobcatfish

Generally I am very excited about supporting a new resource for generic binary data!! I have a few questions before I get too far into the review :D

bobcatfish · 2018-12-11T19:57:26Z

docs/using.md

+
+If resources are only declared in output then a copy of resource to be uploaded 
+or shared for next task is expected to be present under `/output/resource_name/`. If resource is declared in both input and output then resource destination path is used instead of `/output/resource_name`.
+


Woooot thanks for adding these docs! 😍

I'm feeling like the "Container Contract" section is getting big enough that we can maybe start breaking it up into subsections, maybe a section on outputs?

bobcatfish · 2018-12-11T19:58:37Z

docs/using.md

+### Storage Resource
+
+Storage resource represents a blob storage, that contains either an object or directory.
+Adding the storage resource as an input to a task will download the blob and allow the task to perform the required actions on the contents of the blob. Blob storage type "Google Cloud Storage" is supported as of now.


Maybe something like:

" Blob storage type "Google Cloud Storage" (gcs)"

Since gcs is the type they need to actually use

bobcatfish · 2018-12-11T19:59:05Z

docs/using.md

+Params that can be added are the following:
+
+1. `location`: represents the location of the blobstorage.
+1. `type`: represents the type of blob storage. Currently there is implementation


should probably be gcs

bobcatfish · 2018-12-11T19:59:18Z

docs/using.md

+1. `location`: represents the location of the blobstorage.
+1. `type`: represents the type of blob storage. Currently there is implementation
+   for only GCS.
+1. `dir`: represents whether the blob storage is a director or not.


typo, directory?

what does it mean if it isn't a directory?

or is this a path to a directory?

I will add more description to explain why we are using this flag

bobcatfish · 2018-12-11T20:00:01Z

docs/using.md

+1. `dir`: represents whether the blob storage is a director or not.
+
+To use GCS private buckets, [service accounts](https://cloud.google.com/compute/docs/access/service-accounts
+) could be configured to access GCS bucket for `read` and `write` authorities. Download the service account keys to create a secret.


i think there's an extra newline here that's interfering with service accounts being a link?

I'm not sure what "read and write authorities" means - is this a GCS term? Or is this "or read and write permissions"?

Permissions. I will correct it

bobcatfish · 2018-12-11T20:00:57Z

docs/using.md

+  secrets:
+    - fieldName: GOOGLE_APPLICATION_CREDENTIALS
+      secretKey: bucket-sa
+      secretName: service_account.json


bobcatfish · 2018-12-11T20:04:14Z

pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go

-var kubeconfigWriterImage = flag.String("kubeconfig-writer-image", "override-with-kubeconfig-writer:latest", "The container image containing our kubeconfig writer binary.")
+var (
+	kubeconfigWriterImage = flag.String("kubeconfig-writer-image", "override-with-kubeconfig-writer:latest", "The container image containing our kubeconfig writer binary.")
+	bashNoopImage         = flag.String("bash-noop-image", "override-with-bash-noop:latest", "The container image containing bash shell")


before I get too far into this PR - could you explain a bit (maybe even in some comments in the main.gos?) a bit more about what these binaries are for? it's a bit hard to tell at a glance

do we need a generic bash image? alternatively we could run any other container that has bash in it, maybe set the entrypoint to bash?

do we need a generic bash image?

I do not think so but I remember in build there was an image build-base which was custom to what build base image required (I think). I am not entirely sure why we need new custom image in this case because .ko.yaml lets you perfectly override if user wants different base image. I added comment in .ko.yaml file about the expectation of image.

bkuschel · 2018-12-11T21:08:07Z

It should be noted that the implementation for S3 is almost exactly the same except for the image and a very small difference in the command line arguments. Basically the initialize and the two Container Spec functions will be different.
I may follow this PR with an S3 implementation that may move some of the common functions to storage_resource.go.

shashwathi · 2018-12-12T14:46:53Z

/test pull-knative-build-pipeline-integration-tests
/test pull-knative-build-pipeline-go-coverage

nader-ziada · 2018-12-12T15:30:39Z

cmd/bash/main.go

+
+func main() {
+	flag.Parse()
+	logger, _ := logging.NewLogger("", "bash_command")


can we rename the bash_command to shell_command or something similar since you are using sh?

nader-ziada · 2018-12-12T15:40:14Z

docs/using.md

+1. `location`: represents the location of the blob storage.
+1. `type`: represents the type of blob storage. Currently there is implementation for only `gcs`.
+1. `dir`: represents whether the blob storage is a directory or not.
+    1. If artifact is a directory then `-r`(recursive) flag is used to copy all files under source directory to gcs bucket. Eg: `gsutil cp -r source_dir gs://some-bucket`


is there a default value for the dir field? since its not the example here, its not clear what the value would be

nader-ziada · 2018-12-12T15:50:58Z

docs/using.md

+        - fieldName: GOOGLE_APPLICATION_CREDENTIALS
+          secretKey: bucket-sa
+          secretName: service_account.json
+    ```


The use of fieldName here is confusing because this is not actually a field, I would prefer if it was a field on the gcs resource, doesn't have to be part of the generic interface

nader-ziada · 2018-12-12T16:26:53Z

pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go

+		// to the desired destination directory
+		var copyStepsFromPrevTasks []corev1.Container
+
+		for i, path := range boundResource.Paths {


The name Paths does not make it clear at a glance that it means you have to copy from a previous task, maybe something like

passedPath

fromOtherTaskPath

onVolumePath

cachedPath

fetchedPath

Yeah I agree field name is not intuitive and it should be updated but refactoring this would add more changes to this PR. This is already growing too much in scope. I want to follow up with another PR for that change

yeah, makes sense since this was already there before this PR

nader-ziada · 2018-12-12T16:30:35Z

pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go

+
+// AddOutputResources reads the output resources and adds the corresponding container steps
+// This function also reads the inputs to check if resources are redeclared in inputs and has any custom
+// target directory.


why can't we require that the output path will always be in output/resource_name even if its different in the input, this way it at least will be consistent and easier for users to know what to expect, it might require copying it to the output folder if its an output

You mean if user declares output resources then expect the resource to be present in output/resource_name folder?

Then user has to add a step to copy the resource to the folder path /output/resource_name. I think it might be too much work. I agree its more predictable but I was trying to infer the path from already given information which means less yaml. I do not have strong opinion about this as both methods as have pros and cons.

You mean if user declares output resources then expect the resource to be present in output/resource_name folder?

yes exactly

Then user has to add a step to copy the resource to the folder path /output/resource_name. I think it might be too much work. I agree its more predictable but I was trying to infer the path from already given information which means less yaml. I do not have strong opinion about this as both methods as have pros and cons.

not the user has to add a step, the code automatically does that

If resource is defined in input then in code we can figure out where is the resource placed. I do not see value in copying from input resource destination -> /output/resource_name -> external bucket. As of current implementation we have input resource destination -> external bucket

This would introduce additional copy step.

code complexity is not solved but just moved to input resource. Now input resource has to check if resource is defined in output and add some additional step to ensure copy has happened before resolving outputs. I am not sure of this change

vdemeester

Nice work 👍 , few comments 👼

vdemeester · 2018-12-13T10:26:10Z

cmd/bash/main.go

+*/
+
+/*
+Base image expects shell in $PATH


expects sh in …

vdemeester · 2018-12-13T10:26:34Z

cmd/controller/main.go

@@ -88,6 +88,14 @@ func main() {
 		logger.Fatalf("Error building pipeline clientset: %v", err)
 	}

+<<<<<<< HEAD


conflicts 😓

I just resolved them.My bad. I forgot to push local changes after resolving conflicts.

vdemeester · 2018-12-13T10:28:06Z

docs/using.md

+steps:
+  - image: objectuser/run-java-jar #https://hub.docker.com/r/objectuser/run-java-jar/
+    command: [jar]
+    args: ["-cvf", "-o", "/workspace/outputs/storage-gcs/", "projectname.war", "*"]


the path should be /workspace/output/storage-gcs

ah. Good catch

vdemeester · 2018-12-13T10:29:40Z

docs/using.md

+Params that can be added are the following:
+
+1. `location`: represents the location of the blob storage.
+1. `type`: represents the type of blob storage. Currently there is implementation for only `gcs`.


vdemeester · 2018-12-13T10:29:47Z

docs/using.md

+
+1. `location`: represents the location of the blob storage.
+1. `type`: represents the type of blob storage. Currently there is implementation for only `gcs`.
+1. `dir`: represents whether the blob storage is a directory or not. By default storage artifact is considered not a directory.


vdemeester · 2018-12-13T10:30:39Z

docs/using.md

+1. `type`: represents the type of blob storage. Currently there is implementation for only `gcs`.
+1. `dir`: represents whether the blob storage is a directory or not. By default storage artifact is considered not a directory.
+    1. If artifact is a directory then `-r`(recursive) flag is used to copy all files under source directory to gcs bucket. Eg: `gsutil cp -r source_dir gs://some-bucket`
+    1. If is a single file like zip, tar files then copy will be only 1 level deep.  Eg: `gsutil cp source.tar gs://some-bucket.tar`. This will not trigger copy of sub directories in source directory.


2. ? (and same below, number ain't right 🙃 )

shashwathi · 2018-12-13T14:35:27Z

@vdemeester I have addressed your comments. Let me know 👍

- Add implementation for only gcs for now, resource declaration includes bucket location(url), type params to configure credentials for bucket. - If storage resource is added as input resource then updated build steps to pull the blob storage. Params of resources are set as environment variables for private buckets. - If storage is added as output resource then step is added to update the blob to the bucket after execution of build steps. - Add documentation how to use gcs resource - Refactor design for output - Add e2e test with a gcs resource has secret param set. Test asserts that volume is mounted for secrets, lists the fetched file.

- This will allow users to provide another base image if they do not want to use "busybox" for open source compliance reasons.

- Simplify logic in taskrun to copy from previous task output or not. current implementation: taskrun input resouce checks whether there is dependency path set, then it executes copy from pvc instead of fetching fresh copy - update gcs resource to do 1 level copy if `--dir` param is not set else `-r`(recursive) copy flag is set. - update messed up indentation in docs - add docs for fetching objects from private buckets - add .ko.yaml file to override base image with bash shell

shashwathi · 2018-12-18T14:43:54Z

/test pull-knative-build-pipeline-integration-tests

vdemeester

sgtm 👼

knative-prow-robot · 2018-12-18T15:16:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: shashwathi, 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:

~~OWNERS~~ [shashwathi]

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

bobcatfish

This is looking great @shashwathi ! 😍

I've swung in with a few last minute comments, main things:

I still don't understand why we need to be building and releasing the bash image - but this might just be me not reading carefully or some missing docs!
I think we need to add the new image to our release scripts? (or maybe just update this comment if this happens automatically...)
Can you start a doc that explains the internals of how we this all works? i.e. something that talks about how we pass outputs + inptus - not directed at users of build-pipeline, but instead at maintainers (no worries if you want to do this in a separate PR, this one has been open a while! also feel free to put in the bare minimum, I just want to get something started that we can build on as this gets more complex, and we've got the init container changes coming too...)

I'm happy to LGTM this asap so we can get it merged, dont want to leave it open for too long :D

p.s. I am especially stoked about the awesome user docs in this PR 💃

bobcatfish · 2018-12-18T18:40:29Z

docs/using.md

+
+**Note**: If task is relying on output resource functionality then they cannot mount anything in file path `/workspace/output`.
+
+If resource is declared in both input and output then input resource, then destination path of input resource is used instead of `/workspace/output/resource_name`. 


when skimming i though this was saying the path would still have output - could we include an example of the actual path, or link to the section in the docs on it?

bobcatfish · 2018-12-18T18:40:57Z

docs/using.md

+      value: "world"
+```
+
+**Note**: If task is relying on output resource functionality then they cannot mount anything in file path `/workspace/output`.


why do we change the path when a resource is only an output? i'm wondering why we decided not to use the same path in both cases?

We could use the same path in both cases, @pivotal-nader-ziada had similar suggestion too. If we take that route then it becomes more burden for operator to have a copy of resource present in path /workspace/output. If resource is already declared in input then we know where the resource is already placed, then we are re-using that information.

If we take that route then it becomes more burden for operator to have a copy of resource present in path /workspace/output.

Is the idea that inputs are always in /workspace/ and outputs are always in /workspace/outputs?

An alternative could be to have unique names for all inputs + outputs, and put all of them in /workspace/

True. We could do this.

bobcatfish · 2018-12-18T18:42:04Z

docs/using.md

+
+If resource is declared in both input and output then input resource, then destination path of input resource is used instead of `/workspace/output/resource_name`. 
+
+In the following example Task `tar-artifact` resource is used both as input and output so input resource is downloaded into directory `customworkspace`(as specified in `targetPath`). Step `untar` extracts tar file into `tar-scratch-space` directory , `edit-tar` adds a new file and last step `tar-it-up` creates new tar file and places in `/workspace/customworkspace/` directory. After execution of task steps, (new) tar file in directory `customworkspace` will be uploaded to the bucket defined in `tar-artifact` resource definition.


can the docs show the complete path to customworkspace, i.e. is it /customworkspace or is it /workspace/customworkspace or /workspace/output/customworkspace?

docs/using.md

bobcatfish · 2018-12-18T18:43:49Z

docs/using.md

+2. `type`: represents the type of blob storage. Currently there is implementation for only `gcs`.
+3. `dir`: represents whether the blob storage is a directory or not. By default storage artifact is considered not a directory.
+    * If artifact is a directory then `-r`(recursive) flag is used to copy all files under source directory to GCS bucket. Eg: `gsutil cp -r source_dir gs://some-bucket`
+    * If artifact is a single file like zip, tar files then copy will be only 1 level deep(no recursive). It will not trigger copy of sub directories in source directory. Eg: `gsutil cp source.tar gs://some-bucket.tar`.


bobcatfish · 2018-12-18T18:50:56Z

pkg/reconciler/v1alpha1/taskrun/validate.go

@@ -65,7 +65,6 @@ func ValidateResolvedTaskResources(params []v1alpha1.Param, rtr *resources.Resol
 				}
 			}
 		}
-		// TODO: make sure there aren't missing???


Should I leave this in? There was no name or context. I was cleaning all my TODO so I might have accidentally did too much 😆

hahaha no feel free to remove it, I'm not sure what it is either though I suspect I wrote it 😅

bobcatfish · 2018-12-18T18:52:47Z

cmd/gsutil/main.go

+Base image expects gsutil binary in $PATH
+gsutil command uses `-m` flag(default) for faster download and uploads.
+This file executes cp sub-command like `cp source destination`
+*/


i think it would be helpful for all of our main.go's to have some info about how to use the resulting binaries, e.g. something like:

This tool is for _______ To use it, run ____ For help, run ______ For example: ______

@bobcatfish : I have updated comments to include new style. Let me know if thats good.

bobcatfish · 2018-12-18T18:56:27Z

pkg/apis/pipeline/v1alpha1/gcs_resource.go

+}
+
+// GetParams get params
+func (s *GCSResource) GetParams() []Param { return []Param{} }


should this be returning Location and Dir?

I looked at git, image, cluster resource as example and all of them return empty params but I can update this to return Location. Dir is like runtime param I am not sure it fits the case here.

bobcatfish · 2018-12-18T18:56:38Z

pkg/apis/pipeline/v1alpha1/gcs_resource.go

+		"name":     s.Name,
+		"type":     string(s.Type),
+		"location": s.Location,
+	}


what about dir?

Dir is decided by controller and not replacement template. So did not add that key

hm kk - it looks in the docs like ppl can specify dir explicitly:

dir: represents whether the blob storage is a directory or not. By default storage artifact is considered not a directory.

so i feel like it would make sense to provide here? but i cant imagine why anyone would want to template with it so maybe it's fine!

bobcatfish · 2018-12-18T18:57:05Z

pkg/apis/pipeline/v1alpha1/gcs_resource.go

+}
+
+// GetDownloadContainerSpec returns an array of container specs to download gcs storage object
+func (s *GCSResource) GetDownloadContainerSpec() ([]corev1.Container, error) {


oooo i like this UploadContainer + DownloadContainer pattern! do you see us doing this for other resource types as well? (e.g. git)

Yes I did not want to do git container spec in this PR but it fits the case well. s3 resource would be a great fit too.

kk sounds great!

shashwathi · 2018-12-18T19:51:01Z

I think we need to add the new image to our release scripts? (or maybe just update this comment if this happens automatically...)

There is nothing to add to release script.

In the case of creds-init and git-init separate base image is used so comment is about building base image for that.
.ko.yaml declares busybox and google/cloud-sdk:alpine as base image(no custom image to be built) so this should just work (from my understanding)

I will update .ko.yaml.release script to include these new images

- In .ko.yaml gsutil base could be replaced with any other image with gsutil binaries for license compliance reasons - tests are updated to verify override-gsutil-image is used instead of "google/cloud-sdk" image directly. - gsutil main.go file logs command output to diagnose failures/success - Add documentation in main.go files on how they are being used in pipeline crd - Update documentation to include examples about configuring private GCS buckets - Fix merge conflicts

shashwathi · 2018-12-18T20:51:37Z

/test pull-knative-build-pipeline-integration-tests

Helm deploy is flaky. Retesting as new changes was doc related.

shashwathi · 2018-12-18T22:36:00Z

@bobcatfish : I have added developer docs. At this point I am dumping my thoughts about inputs/outputs contract. It is obviously not well framed. We can iterate on docs in further PR if you don't mind.

bobcatfish · 2018-12-18T22:42:40Z

Helm deploy is flaky

/me shakes fist

bobcatfish · 2018-12-18T22:43:03Z

. At this point I am dumping my thoughts about inputs/outputs contract. It is obviously not well framed. We can iterate on docs in further PR if you don't mind.

That's great!! Thanks so much for getting these started :D :D :D

bobcatfish · 2018-12-18T22:43:50Z

docs/developers/README.md

@@ -0,0 +1,54 @@
+## Developer docs


my very last request is that we have a link to these docs somewhere - maybe from the DEVELOPMENT.md and/or from the main README 🙏

i promise to stop the scope creep after this ❤️

bobcatfish · 2018-12-18T22:52:58Z

I added another question about the bash image and another doc request 😇 🙏 😇 thanks for your patience @shashwathi !!

- Adding docs explaining how different parts are working in conjuction. In this initial attemp I am adding inputs/outputs contract is controllers. - Link developer docs in README - Add comment to main.go file about the file to update for base image override.

knative-metrics-robot · 2018-12-18T23:09:24Z

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

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/gcs_resource.go	Do not exist	86.7%
pkg/apis/pipeline/v1alpha1/pipelineresource_validation.go	68.2%	80.6%	12.4
pkg/apis/pipeline/v1alpha1/storage_resource.go	Do not exist	100.0%
pkg/apis/pipeline/v1alpha1/taskrun_types.go	55.6%	63.6%	8.1
pkg/reconciler/v1alpha1/taskrun/resources/input_resources.go	87.9%	91.5%	3.6
pkg/reconciler/v1alpha1/taskrun/resources/output_resource.go	Do not exist	93.1%
pkg/reconciler/v1alpha1/taskrun/resources/volume.go	Do not exist	100.0%

bobcatfish · 2018-12-19T02:19:43Z

README.md

@@ -36,6 +36,7 @@ We are so excited to have you!

 - See [CONTRIBUTING.md](CONTRIBUTING.md) for an overview of our processes
 - See [DEVELOPMENT.md](DEVELOPMENT.md) for how to get started
+- [Deep dive](./docs/developers/README.md) into demystifying the inner workings (advanced reading material)


bobcatfish

/lgtm
/meow space

knative-prow-robot · 2018-12-19T02:23:14Z

@bobcatfish:

In response to this:

/lgtm
/meow space

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.

because it adds two more containers of which the latter, source-copy, fails with {"level":"error","ts":1545214117.0201285,"logger":"fallback-logger","caller":"bash/main.go:62","msg":"Error executing command \"cp -r /workspace/output/builtImage/. /pvc/export-npmlib/builtImage\" with arguments cp: can't stat '/workspace/output/builtImage/.': No such file or directory\n","stacktrace":"main.main\n\t/go/src/github.com/knative/build-pipeline/cmd/bash/main.go:62\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:201"} and I'd rather not figure out why just now.

knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 9, 2018

knative-prow-robot requested review from aaron-prindle and jessiezcc December 9, 2018 02:13

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 9, 2018

shashwathi force-pushed the gcs-support branch from 685ccd7 to 4472e16 Compare December 9, 2018 02:36

shashwathi force-pushed the gcs-support branch from 4472e16 to c5c8dfe Compare December 9, 2018 15:38

nader-ziada reviewed Dec 10, 2018

View reviewed changes

shashwathi force-pushed the gcs-support branch from f8231e2 to 4755eb0 Compare December 10, 2018 23:48

bkuschel mentioned this pull request Dec 11, 2018

PipelineResource for S3 buckets (for IBM COS, AWS S3, Minio ...) #323

Closed

shashwathi force-pushed the gcs-support branch from 4755eb0 to c6e1c66 Compare December 11, 2018 14:06

bkuschel reviewed Dec 11, 2018

View reviewed changes

bobcatfish reviewed Dec 11, 2018

View reviewed changes

shashwathi force-pushed the gcs-support branch from 0dd3620 to 013ce29 Compare December 11, 2018 21:57

nader-ziada reviewed Dec 12, 2018

View reviewed changes

shashwathi force-pushed the gcs-support branch from 013ce29 to 180f1ba Compare December 12, 2018 22:11

vdemeester reviewed Dec 13, 2018

View reviewed changes

shashwathi force-pushed the gcs-support branch 2 times, most recently from 13029ef to f7b74f7 Compare December 13, 2018 14:34

shashwathi added 2 commits December 17, 2018 15:50

Add the ability to override base image busybox through flag.

b42a3bf

- This will allow users to provide another base image if they do not want to use "busybox" for open source compliance reasons.

shashwathi force-pushed the gcs-support branch from f7b74f7 to a4f1ddb Compare December 17, 2018 21:03

vdemeester approved these changes Dec 18, 2018

View reviewed changes

bobcatfish reviewed Dec 18, 2018

View reviewed changes

shashwathi force-pushed the gcs-support branch from a4f1ddb to 8a935e8 Compare December 18, 2018 20:13

bobcatfish reviewed Dec 18, 2018

View reviewed changes

shashwathi force-pushed the gcs-support branch from 347b5e6 to e8e851a Compare December 18, 2018 22:59

Add developer docs

d69afcd

- Adding docs explaining how different parts are working in conjuction. In this initial attemp I am adding inputs/outputs contract is controllers. - Link developer docs in README - Add comment to main.go file about the file to update for base image override.

shashwathi force-pushed the gcs-support branch from e8e851a to d69afcd Compare December 18, 2018 23:04

bobcatfish reviewed Dec 19, 2018

View reviewed changes

knative-prow-robot assigned bobcatfish Dec 19, 2018

bobcatfish reviewed Dec 19, 2018

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 19, 2018

knative-prow-robot merged commit 9d7e6d3 into tektoncd:master Dec 19, 2018

imjasonh mentioned this pull request Oct 31, 2019

Remove cmd/gsutil image #1504

Merged


		If resources are only declared in output then a copy of resource to be uploaded
		or shared for next task is expected to be present under `/output/resource_name/`. If resource is declared in both input and output then resource destination path is used instead of `/output/resource_name`.


		Note: If task is relying on output resource functionality then they cannot mount anything in file path `/workspace/output`.

		If resource is declared in both input and output then input resource, then destination path of input resource is used instead of `/workspace/output/resource_name`.


		If resource is declared in both input and output then input resource, then destination path of input resource is used instead of `/workspace/output/resource_name`.

		In the following example Task `tar-artifact` resource is used both as input and output so input resource is downloaded into directory `customworkspace`(as specified in `targetPath`). Step `untar` extracts tar file into `tar-scratch-space` directory , `edit-tar` adds a new file and last step `tar-it-up` creates new tar file and places in `/workspace/customworkspace/` directory. After execution of task steps, (new) tar file in directory `customworkspace` will be uploaded to the bucket defined in `tar-artifact` resource definition.

Add support for storage resource #321

Add support for storage resource #321

Conversation

shashwathi commented Dec 9, 2018 • edited Loading

googlebot commented Dec 9, 2018

googlebot commented Dec 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashwathi Dec 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashwathi commented Dec 11, 2018

bobcatfish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashwathi Dec 11, 2018 • edited Loading

Choose a reason for hiding this comment

bkuschel commented Dec 11, 2018

shashwathi commented Dec 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdemeester left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashwathi commented Dec 13, 2018

shashwathi commented Dec 18, 2018

vdemeester left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Dec 18, 2018

bobcatfish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shashwathi commented Dec 9, 2018 •

edited

Loading

shashwathi Dec 10, 2018 •

edited

Loading

shashwathi Dec 11, 2018 •

edited

Loading

shashwathi commented Dec 18, 2018 •

edited

Loading