Shared docker volumes with EFS #822

Merged
merged 12 commits into from Jul 14, 2016

Conversation

Projects
None yet
5 participants
@ddollar
Member

ddollar commented Jun 29, 2016

This pull request builds on #810 and uses EFS for the backing store for volumes. To use add volumes to the volumes: section of your process entry.

$ cat docker-compose.yml | grep volumes: -A2
  volumes:
    - /foo
    - /bar:/tmp/bar

$ convox ps
ID            NAME      RELEASE      STARTED        COMMAND
831925acee0e  database  RLMZNZSKUNA  3 minutes ago  /docker-entrypoint.sh postgres
73bf22eed6bc  web       RLMZNZSKUNA  1 minute ago   bin/web
fe66c945567d  web       RLMZNZSKUNA  3 minutes ago  bin/web

$ convox exec 73bf22eed6bc cat /foo/bar
cat: /foo/bar: No such file or directory

$ convox exec fe66c945567d 'echo qux > /foo/bar'

$ convox exec 73bf22eed6bc cat /foo/bar
qux

Release Playbook

api/models/manifest.go
@@ -468,12 +469,41 @@ func (me ManifestEntry) EnvMap() map[string]string {
return envs
}
+// TODO: remove when default
+func (me ManifestEntry) MountVolumes() bool {

This comment has been minimized.

@houndci-bot

houndci-bot Jun 29, 2016

receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"

@houndci-bot

houndci-bot Jun 29, 2016

receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"

api/models/manifest.go
@@ -468,12 +469,41 @@ func (me ManifestEntry) EnvMap() map[string]string {
return envs
}
+// TODO: remove when default

This comment has been minimized.

@houndci-bot

houndci-bot Jun 29, 2016

comment on exported method ManifestEntry.MountVolumes should be of the form "MountVolumes ..."

@houndci-bot

houndci-bot Jun 29, 2016

comment on exported method ManifestEntry.MountVolumes should be of the form "MountVolumes ..."

@ddollar ddollar changed the title from Shared docker volumes with EFS to WIP: Shared docker volumes with EFS Jun 29, 2016

@ddollar ddollar referenced this pull request Jun 29, 2016

Closed

Mountable volumes #810

0 of 11 tasks complete
@ddollar

This comment has been minimized.

Show comment
Hide comment
@ddollar

ddollar Jun 29, 2016

Member

20160629152014-efs

Member

ddollar commented Jun 29, 2016

20160629152014-efs

api/manifest/manifest.go
-func (me ManifestEntry) labelsByPrefix(prefix string) map[string]string {
+// LabelsByPrefix filters Docker Compose label names by prefixes and returns
+// a map of label names to values that match.
+func (me ManifestEntry) LabelsByPrefix(prefix string) map[string]string {

This comment has been minimized.

@houndci-bot

houndci-bot Jun 29, 2016

receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"

@houndci-bot

houndci-bot Jun 29, 2016

receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"

@MiguelMoll

This comment has been minimized.

Show comment
Hide comment
@MiguelMoll

MiguelMoll Jun 29, 2016

Contributor

Always mount volumes

Would love for that to be the default. Seems the expected behavior.

Contributor

MiguelMoll commented Jun 29, 2016

Always mount volumes

Would love for that to be the default. Seems the expected behavior.

@ddollar

This comment has been minimized.

Show comment
Hide comment
Member

ddollar commented Jun 29, 2016

@ddollar ddollar changed the title from WIP: Shared docker volumes with EFS to Shared docker volumes with EFS Jun 29, 2016

api/models/manifest.go
+ Container string
+}
+
+func (me ManifestEntry) MountableVolumes() []MountableVolume {

This comment has been minimized.

@houndci-bot

houndci-bot Jun 30, 2016

exported method ManifestEntry.MountableVolumes should have comment or be unexported
receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"

@houndci-bot

houndci-bot Jun 30, 2016

exported method ManifestEntry.MountableVolumes should have comment or be unexported
receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"

api/models/manifest.go
@@ -468,13 +469,36 @@ func (me ManifestEntry) EnvMap() map[string]string {
return envs
}
-func (me ManifestEntry) MountableVolumes() []string {
- volumes := []string{}
+type MountableVolume struct {

This comment has been minimized.

@houndci-bot

houndci-bot Jun 30, 2016

exported type MountableVolume should have comment or be unexported

@houndci-bot

houndci-bot Jun 30, 2016

exported type MountableVolume should have comment or be unexported

api/models/manifest.go
+ Container string
+}
+
+func (me ManifestEntry) MountableVolumes() []mountableVolume {

This comment has been minimized.

@houndci-bot

houndci-bot Jun 30, 2016

exported method ManifestEntry.MountableVolumes should have comment or be unexported
receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"
exported method MountableVolumes returns unexported type []models.mountableVolume, which can be annoying to use

@houndci-bot

houndci-bot Jun 30, 2016

exported method ManifestEntry.MountableVolumes should have comment or be unexported
receiver name should be a reflection of its identity; don't use generic names such as "me", "this", or "self"
exported method MountableVolumes returns unexported type []models.mountableVolume, which can be annoying to use

api/models/manifest.go
@@ -468,13 +469,37 @@ func (me ManifestEntry) EnvMap() map[string]string {
return envs
}
-func (me ManifestEntry) MountableVolumes() []string {
- volumes := []string{}
+// mountable volumes from a given manifest entry

This comment has been minimized.

@houndci-bot

houndci-bot Jun 30, 2016

comment on exported type MountableVolume should be of the form "MountableVolume ..." (with optional leading article)

@houndci-bot

houndci-bot Jun 30, 2016

comment on exported type MountableVolume should be of the form "MountableVolume ..." (with optional leading article)

api/models/manifest.go
@@ -163,9 +164,9 @@ func (m Manifest) AppName() string {
return m[0].app.Name
}
-func (me ManifestEntry) LabelsByPrefix(prefix string) map[string]string {
+func (e ManifestEntry) LabelsByPrefix(prefix string) map[string]string {

This comment has been minimized.

@houndci-bot

houndci-bot Jun 30, 2016

exported method ManifestEntry.LabelsByPrefix should have comment or be unexported

@houndci-bot

houndci-bot Jun 30, 2016

exported method ManifestEntry.LabelsByPrefix should have comment or be unexported

@@ -633,15 +634,18 @@
},
"Volumes": [
{{ range .MountableVolumes }}
- "{{ . }}",
+ {{ if eq .Host "/var/run/docker.sock" }}

This comment has been minimized.

@nzoschke

nzoschke Jul 5, 2016

Contributor

is this really the only non /volumes/ mount we expect going forward? I can't think of anything else we explicitly support, just asking out loud. And points to some documentation improvements obviously. Can't find anything about volumes right now.

@nzoschke

nzoschke Jul 5, 2016

Contributor

is this really the only non /volumes/ mount we expect going forward? I can't think of anything else we explicitly support, just asking out loud. And points to some documentation improvements obviously. Can't find anything about volumes right now.

This comment has been minimized.

@ddollar

ddollar Jul 5, 2016

Member

Yes, this is the only special volume we support. Matt is working on the docs for this stuff.

@ddollar

ddollar Jul 5, 2016

Member

Yes, this is the only special volume we support. Matt is working on the docs for this stuff.

@nzoschke

This comment has been minimized.

Show comment
Hide comment
@nzoschke

nzoschke Jul 6, 2016

Contributor

In testing I encountered one issue. After the first update, 2 out of 3 instances came up without correctly mounting EFS due to DNS problems:

mount.nfs: Failed to resolve server us-east-1b.fs-f223e6bb.efs.us-east-1.amazonaws.com: Name or service not known

Waiting a few minutes and restarting the servers with convox instances keyroll fixed it.

Contributor

nzoschke commented Jul 6, 2016

In testing I encountered one issue. After the first update, 2 out of 3 instances came up without correctly mounting EFS due to DNS problems:

mount.nfs: Failed to resolve server us-east-1b.fs-f223e6bb.efs.us-east-1.amazonaws.com: Name or service not known

Waiting a few minutes and restarting the servers with convox instances keyroll fixed it.

@ddollar ddollar added this to the 20160713 milestone Jul 13, 2016

ddollar added a commit that referenced this pull request Jul 13, 2016

Merge pull request #822 from convox/rack
---

This pull request builds on #810 and uses EFS for the backing store for volumes. To use add volumes to the `volumes:` section of your process entry.

```
$ cat docker-compose.yml | grep volumes: -A2
  volumes:
    - /foo
    - /bar:/tmp/bar

$ convox ps
ID            NAME      RELEASE      STARTED        COMMAND
831925acee0e  database  RLMZNZSKUNA  3 minutes ago  /docker-entrypoint.sh postgres
73bf22eed6bc  web       RLMZNZSKUNA  1 minute ago   bin/web
fe66c945567d  web       RLMZNZSKUNA  3 minutes ago  bin/web

$ convox exec 73bf22eed6bc cat /foo/bar
cat: /foo/bar: No such file or directory

$ convox exec fe66c945567d echo qux > /foo/bar

$ convox exec 73bf22eed6bc cat /foo/bar
qux
```

## Release Playbook
- [x] Rebase against master
- [x] Release branch (20160629185452-efs)
- [x] Pass CI (https://circleci.com/gh/convox/rack/880#build-parameters)
- [ ] Code review
- [ ] Merge into master
- [ ] Release master ()
- [ ] Pass CI ()
- [ ] Update staging rack
- [ ] Edit [Rack release record](https://github.com/convox/rack/releases) and/or update [docs](https://github.com/convox/convox.github.io)
- [ ] Publish release
- [ ] Release CLI

@ddollar ddollar referenced this pull request Jul 13, 2016

Merged

[RELEASE] 20160713 #866

10 of 14 tasks complete

@ddollar ddollar merged commit 1657d67 into master Jul 14, 2016

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
hound No violations found. Woof!

@ddollar ddollar deleted the efs branch Jul 19, 2016

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