Closed kmala closed 8 years ago
@kmala since slugrunner, slugbuilder and dockerbuilder are gonna need this storage layer, wouldn't it be better to put it into a separate repository and also build a CLI for it (for the shell components to use)?
@arschles does some such abstraction already exist? It seems to me there are several. s3cmd comes to mind as one example. Do they all fall short?
Azure is not supported by s3cmd, for example. I'd imagine this being like a generic key/value store library for the three components.
@krancour I think we should we use the native client libraries instead of using some common abstraction because first they may not support everything and second it will be hard to support future requirements when we might need to extend to some other storage backends.
:+1: to what @bacongobbler said. I'd love to be able to interact with azure, gcs, s3 and others all from the same CLI. See below for a rough outline of what this mythical CLI might be able to do.
# interact with Azure
deis-obj-storage register azure https://someurl.com mycreds
deis-obj-storage upload azure/mybucket/myobj
# interact with GCS
deis-obj-storage register standard_gcs https://storage.googleapis.com mycreds
deis-obj-storage upload standard_gcs/mybucket/myobj
# interact with S3
deis-obj-storage register standard_s3 https://s3.amazonaws.com mycreds
deis-obj-storage upload standard_s3/mybucket/myobj
# interact with Minio
deis-obj-storage register minio http://$DEIS_MINIO_SERVICE_HOST:$DEIS_MINIO_SERVICE_PORT mycreds
deis-obj-storage upload minio/mybucket/myobj
# also, support more obj storage systems in the future?
Just a reminder that right now, we need such a CLI because we have bash-based apps (to my chagrin) that need to interact with object storage. If that wasn't the case, I wouldn't be advocating for a CLI.
However, since many of our apps are written in Go, I am advocating for pulling this storage logic into a separate package in a new repository. Refer to https://github.com/docker/distribution/issues/1537, which @mboersma submitted for moving that package out of docker/distribution.
fixes #224
fixes #266
@kmala I just merged https://github.com/deis/builder/pull/248, which is gonna cause you some rebasing pain. Let me know when you hit that and I'll help out
doesn't this require https://github.com/deis/slugbuilder/pull/41 and https://github.com/deis/dockerbuilder/pull/41?
Yes. all the PR's in the first point https://github.com/deis/deis/issues/4966 except the Postgres should go together.
@kmala code LGTM.
Has this been manually tested with https://github.com/deis/slugbuilder/pull/41, https://github.com/deis/slugrunner/pull/24 and https://github.com/deis/dockerbuilder/pull/41? It seems that all of those will need to be merged at the same time.
Yes i had manually tested and ran the e2e tests locally...i will run them again before merging.
@kmala another comment that I just thought of - there is code in builder to create a bucket for storing code (and later, slugs), but I see that particular code block got removed here. Is that functionality replaced anywhere else?
@arschles yes its moved to the storage layer https://github.com/deis/distribution/blob/master/registry/storage/driver/s3-aws/s3.go#L308
This PR is to create a separate storage layer for the deis builder so that it can support multiple storage backends.