Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

doc(configuring-object-storage.md): add section on storing credentials & GCS #68

Merged
merged 14 commits into from
Mar 8, 2016

Conversation

arschles
Copy link
Member

@arschles arschles commented Mar 5, 2016

also adds the database component to the list of things that needs S3

Fixes deis/builder#197

cc/ @smothiki

…s & GCS

also adds the database component to the list of things that needs S3

As you may know Google Cloud Storage (GCS) can [interoperate with the S3 API](https://cloud.google.com/storage/docs/interoperability), and, if you choose to use Google Cloud Storage for object storage, you'll have to turn on this interoperability mode.

If you choose to use Google Cloud Storage, set your `DEIS_OUTSIDE_STORAGE_HOST` environment variable to `storage.googleapis.com`, and follow [these instructions](https://cloud.google.com/storage/docs/migrating?hl=en_US#keys) to generate an S3 compatible access key ID and access key secret. Store these credentials just as you would if they were AWS S3 or Minio credentials (see the "Storing Credentials" section above).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smothiki if the URL doesn't have a scheme, then that stripScheme func is a no-op, so it is ok to leave off the scheme.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1a0ba70

@smothiki
Copy link
Contributor

smothiki commented Mar 7, 2016

Builder needs a separate config settings for any endpoint same instructions goes for aws S3 as well.
Also BUCKET is the env variable that needs to be set, bucket names are Global and defaults to "git" which will fail

@smothiki
Copy link
Contributor

smothiki commented Mar 7, 2016

Please don't merge this until documentation is complete.

@bacongobbler
Copy link
Member

Also BUCKET is the env variable that needs to be set, bucket names are Global and defaults to "git" which will fail

I think that can be specified elsewhere in another PR since this just addresses S3 credentials.

@smothiki
Copy link
Contributor

smothiki commented Mar 7, 2016

Builder needs a separate configuration apart from other components in the platform. Also there is an error in the docs, need to set URL scheme as well

@arschles
Copy link
Member Author

arschles commented Mar 7, 2016

@smothiki:

Builder needs a separate config settings for any endpoint same instructions goes for aws S3 as well.

I don't understand what you mean here. What is the separate config setting besides DEIS_OUTSIDE_STORAGE_HOST?

Also BUCKET is the env variable that needs to be set, bucket names are Global and defaults to "git" which will fail

Why will git fail?

Builder needs a separate configuration apart from other components in the platform.

Similar to my above comment, can you give more details on what the separate configuration is? IIRC, DEIS_OUTSIDE_STORAGE_HOST and the credentials setup mentioned in these docs is enough.

I've removed the LGTM2 label from this PR until these questions are sorted out.

@smothiki
Copy link
Contributor

smothiki commented Mar 7, 2016

if there is a git bucket already exists in GCS storage we cannot PUT or GET from the bucket if "git " bucket is not registered under user credentials

@smothiki
Copy link
Contributor

smothiki commented Mar 7, 2016

Also the code defaults to MInio USER host and PORT . So if user has to set back-end to AWS . He has to do the similar config changes as GCS but it is not documented in this DOC

Aaron Schlesinger added 2 commits March 7, 2016 15:05
…UTSIDE_STORAGE_PORT

in favor of DEIS_OUTSIDE_STORAGE. also, include examples for outside
storage URLs
@arschles
Copy link
Member Author

arschles commented Mar 7, 2016

@smothiki I've added docs for the BUCKET env var in 6cbee9b

@arschles
Copy link
Member Author

arschles commented Mar 7, 2016

@smothiki according to https://github.com/deis/builder/blob/master/pkg/gitreceive/storage/endpoint.go#L53-L62, the builder defaults to outside storage. Do my recent changes in arschles@1a0ba70 clearly specify that the DEIS_OUTSIDE_STORAGE env var applies to either S3 or GCS?

@smothiki
Copy link
Contributor

smothiki commented Mar 7, 2016

I meant deis-builder-rc in deis-dev charts by default doesn't have DEIS_OUTSIDE_STORAGE variable set. So code defaults to Minio host and port.
Also s3.amazonaws.com works only for us-east-1 . If user wants to specify for some other region he has to prefix the uRL with s3-region-name .

@smothiki
Copy link
Contributor

smothiki commented Mar 7, 2016

I think you have generalized this for every deis component. The setting are only valid for builder and its components not for other services. ping @bacongobbler for database , @kmala for registry.

@@ -15,10 +16,42 @@ Additionally, Deis ships with a [Minio](http://minio.io) [component](https://git

# Telling Deis What to Use

The Deis components determine what object storage system to use via environment variables that you set up:
The Deis components determine what object storage system to use via environment variables that you set up. The below list is the lookup order for all Deis components.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arschles Im good with the PR if you can some how add a special column for builder storage settings. This heading says the below settings are for deis which is not true.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, added sections in arschles@238a71f

@arschles
Copy link
Member Author

arschles commented Mar 8, 2016

@smothiki I've separated each component into its own section in arschles@238a71f, and left TODOs under deis/database and deis/registry (cc/ @kmala and @bacongobbler)

@arschles
Copy link
Member Author

arschles commented Mar 8, 2016

@smothiki also, I've clarified that s3.amazonaws.com is specifically only for us-east-1a in arschles@3b1eb4a

@arschles
Copy link
Member Author

arschles commented Mar 8, 2016

@kmala @bacongobbler @smothiki I've removed both LGTMs, since after they were both on, there were a significant amount of comments changes. Can you all do another review?

@bacongobbler
Copy link
Member

database documentation is a wee bit generic, but it's great that we have this in here so we can point users where to look. I can follow up with more in-depth documentation in the future. 👍

@arschles
Copy link
Member Author

arschles commented Mar 8, 2016

@bacongobbler thanks for the honesty (I suspected as much!) - think this is ok to merge, along with an issue to improve these docs?

@bacongobbler
Copy link
Member

absolutely. This is a great starting point that we can build on top of.

@arschles
Copy link
Member Author

arschles commented Mar 8, 2016

k, cool. thanks!

@bacongobbler
Copy link
Member

closes #29

@bacongobbler
Copy link
Member

also closes #26

The only currently known limitation is that [the Deis registry component](https://github.com/deis/registry) will not automatically look up the minio service, nor will it look for other storage env vars. That fix is being tracked in a [GitHub issue](https://github.com/deis/registry/issues/7) and is planned for our beta release.
Below is a list of known limitations of our components' ability to interact with object storage systems.

- [The Deis registry component](https://github.com/deis/registry) will not automatically look up the Kubernetes Minio service, nor will it look for other storage env vars. That fix is being tracked in a [GitHub issue](https://github.com/deis/registry/issues/7) and is planned for our beta release.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no more a limitation.fixed in deis/registry#27

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @kmala. removed in arschles@7851b48


### Credentials

The registry reads the credential information from a `/var/run/secrets/deis/registry/creds/objectstorage-keyfile` file. See https://github.com/deis/charts/blob/master/deis-dev/tpl/deis-objectstorage-secret.yaml for an example of what that file should look like.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we write this way: The registry reads the credential information from a /var/run/secrets/deis/registry/creds/objectstorage-keyfile file. This is generated automatically during helm generate based on the configuration options given in the https://github.com/deis/charts/blob/master/deis-dev/tpl/objectstorage.toml.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kmala ok, changed it in d106a7e. Also changed the DB credentials section to have similar language in fb6131d

Aaron Schlesinger added 2 commits March 8, 2016 11:47
…rate language to the DB credentials section
…rate logic to the registry credentials section

### Environment Variables

The builder looks for the below environment variables to determine where the object storage system is. The builder looks in-order for these variables. If it finds two, the one higher in the list will be used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be easier to just explicitly state the preferred variable in the case of all of them being present. E.g.,

If it finds two, DEIS_OUTSIDE_STORAGE will take precedence and external object storage configuration will be applied.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Fixed in f575341

Aaron Schlesinger added 4 commits March 8, 2016 14:36
@jackfrancis
Copy link
Member

All amendments look good!

@arschles
Copy link
Member Author

arschles commented Mar 8, 2016

thanks @kmala @smothiki @jackfrancis @bacongobbler for your reviews

arschles added a commit that referenced this pull request Mar 8, 2016
doc(configuring-object-storage.md): add section on storing credentials & GCS
@arschles arschles merged commit 442a9a9 into deis:master Mar 8, 2016
@arschles arschles deleted the s3-docs branch March 8, 2016 22:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants