Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass additional volumes via flag to bpm run #74

Closed
jamesjoshuahill opened this issue Jul 20, 2018 · 9 comments
Closed

Pass additional volumes via flag to bpm run #74

jamesjoshuahill opened this issue Jul 20, 2018 · 9 comments

Comments

@jamesjoshuahill
Copy link
Contributor

The Platform Recovery team have explored using bpm run in BOSH Backup and Restore (BBR) scripts.

Part of the BBR contract is $BBR_ARTIFACT_DIRECTORY will be set, which is the location of the backup artifact for backup or restore.

This location must be mounted within the BPM container. Because it is only determined at backup or restore time, it is not possible to template it into the BOSH job at deployment time.

Currently the BBR CLI creates $BBR_ARTIFACT_DIRECTORY in the form: /var/vcap/store/bbr-backup/JOB_NAME. We would like to avoid the magic string /var/vcap/store/bbr-backup becoming encoded in bpm.yml config files across the many BOSH releases that support BBR. What do you think?

To avoid extending the BBR contract perhaps it would be possible for BPM to allow us to specify additional volumes as a flag to the bpm run command?

@cloudfoundry-incubator/bosh-backup-and-restore-team

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/159192566

The labels on this github issue will be updated when the story is started.

@jfmyers9
Copy link
Contributor

@jamesjoshuahill This seems like a good suggestion. It also makes us more consistent with some of the other container CLI's.

The only thing I think we need to sort out before making this change is how to express the volume schema through this flag. For your use case, would you want to be able to configure things like allow_executions and writable when specifying volumes on the command line?

@jamesjoshuahill
Copy link
Contributor Author

Hi @jfmyers9 that's good news.

We would like to be able to configurewritable so that BBR backup scripts can write to $BBR_ARTIFACT_DIRECTORY.

Also, we looked at mount_only and wondered how this would integrate with BBR. Currently, the BBR CLI creates $BBR_ARTIFACT_DIRECTORY as root before invoking BBR scripts. Would bpm run fail if an additional volume directory already exists? Would the command invoked by bpm run have permissions to read the additional volume directory? To avoid any change in the BBR contract we would like bpm run not to fail when the additional volume exists and chown it as required for the bpm run user.

We don't think that allow_executions would be required for $BBR_ARTIFACT_DIRECTORY. This looks like an acceptable resource limitation.

Many thanks
Josh and @aclevername

@xoebus
Copy link
Contributor

xoebus commented Jul 31, 2018

What about something like -v path:allow_executions,mount_only?

@xoebus
Copy link
Contributor

xoebus commented Jul 31, 2018

In the past we've talked about versioning the configuration so we can make backwards incompatible changes. These would be the first configuration outside configuration file and so we wouldn't be able version these.

@jamesjoshuahill
Copy link
Contributor Author

The -v flag looks fine to our uneducated eyes; we don't have a strong opinion about the format.

@jfmyers9
Copy link
Contributor

jfmyers9 commented Aug 1, 2018

I think the format you proposed looks pretty good. If there are no other concerns, I can whip up a story and hopefully we can get this in a release soon.

@jfmyers9
Copy link
Contributor

jfmyers9 commented Aug 1, 2018

https://www.pivotaltracker.com/story/show/159472729

jfmyers9 added a commit to jfmyers9/bpm-release that referenced this issue Aug 14, 2018
This change adds support for specifying volumes when using the `bpm run`
command. The format of a volume definition is `<path>[:<options>]` where
`<options>` is a comma separated list of: writable, mount_only,
allow_executions. The `--volume` or `-v` flag can be specified multiple
times to add more than one volume.

Fixes cloudfoundry#74

[finishes #159472729]
jfmyers9 added a commit to jfmyers9/bpm-release that referenced this issue Aug 14, 2018
This change adds support for specifying volumes when using the `bpm run`
command. The format of a volume definition is `<path>[:<options>]` where
`<options>` is a comma separated list of: writable, mount_only,
allow_executions. The `--volume` or `-v` flag can be specified multiple
times to add more than one volume.

Fixes cloudfoundry#74

[finishes #159472729]
jfmyers9 added a commit to jfmyers9/bpm-release that referenced this issue Aug 14, 2018
This change adds support for specifying volumes when using the `bpm run`
command. The format of a volume definition is `<path>[:<options>]` where
`<options>` is a comma separated list of: writable, mount_only,
allow_executions. The `--volume` or `-v` flag can be specified multiple
times to add more than one volume.

Fixes cloudfoundry#74

[finishes #159472729]
jfmyers9 added a commit to jfmyers9/bpm-release that referenced this issue Aug 14, 2018
This change adds support for specifying volumes when using the `bpm run`
command. The format of a volume definition is `<path>[:<options>]` where
`<options>` is a comma separated list of: writable, mount_only,
allow_executions. The `--volume` or `-v` flag can be specified multiple
times to add more than one volume.

Fixes cloudfoundry#74

[finishes #159472729]
jfmyers9 added a commit to jfmyers9/bpm-release that referenced this issue Aug 14, 2018
This change adds support for specifying volumes when using the `bpm run`
command. The format of a volume definition is `<path>[:<options>]` where
`<options>` is a comma separated list of: writable, mount_only,
allow_executions. The `--volume` or `-v` flag can be specified multiple
times to add more than one volume.

Fixes cloudfoundry#74

[finishes #159472729]
jfmyers9 added a commit to jfmyers9/bpm-release that referenced this issue Aug 14, 2018
This change adds support for specifying volumes when using the `bpm run`
command. The format of a volume definition is `<path>[:<options>]` where
`<options>` is a comma separated list of: writable, mount_only,
allow_executions. The `--volume` or `-v` flag can be specified multiple
times to add more than one volume.

Fixes cloudfoundry#74

[finishes #159472729]
@glestaris
Copy link

thanks Jim

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

No branches or pull requests

5 participants