Re: Review Request 35981: Added persistent volume user guide.
On June 30, 2015, 12:14 a.m., Adam B wrote:
docs/persistent-volume.md, line 56
https://reviews.apache.org/r/35981/diff/1/?file=994064#file994064line56
Can a volume/reservation created by one framework principal only be
destroyed/unreserved by a framework scheduler using the same principal? If
the principal is different, then the operation would fail, even if the role
or even frameworkId is identical?
We only look at the principal of an framework or an operator. The role does not
matter because an operator does not have a role. The frameworkId is also
irrelavent because framework B can unreserve/destroy a reservation/volume
created by framework B as long as ACL allows it.
On June 30, 2015, 12:14 a.m., Adam B wrote:
docs/persistent-volume.md, lines 67-69
https://reviews.apache.org/r/35981/diff/1/?file=994064#file994064line67
You might need a blank line preceding this, and/or some indentation for
it to show up as a numbered list. Doesn't render properly as is.
Unique per role per slave? So I could use the same volumeId for related
volume instances (e.g. my 'logging' or 'data' volumes), up to a max of one
per slave? But if I happen to launch another executor/task on a slave where
one of those volumes is already created/mounted, then I have to rename it
('data2')?
Yes, this is in the design doc. Persistent volumes can be shared within a role,
so it has to be unique per role.
On June 30, 2015, 12:14 a.m., Adam B wrote:
docs/persistent-volume.md, lines 120-122
https://reviews.apache.org/r/35981/diff/1/?file=994064#file994064line120
Any chance it could return the host_path too? Or do I have to go
digging through slave logs to figure out where my volume was created?
Why the framework wants to know that? This is similar to sandbox. Do we have a
way to let the framework know about the location of the sandbox?
- Jie
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35981/#review89827
---
On July 7, 2015, 12:34 p.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35981/
---
(Updated July 7, 2015, 12:34 p.m.)
Review request for mesos, Adam B, Jie Yu, and Timothy Chen.
Bugs: MESOS-2405
https://issues.apache.org/jira/browse/MESOS-2405
Repository: mesos
Description
---
The Github rendered version is available [here](
https://github.com/mesosphere/mesos/blob/user-docs/docs/persistent-volume.md)
Diffs
-
docs/persistent-volume.md PRE-CREATION
Diff: https://reviews.apache.org/r/35981/diff/
Testing
---
Documentation.
Thanks,
Michael Park
Re: Review Request 35981: Added persistent volume user guide.
On June 29, 2015, 5:14 p.m., Adam B wrote:
docs/persistent-volume.md, line 169
https://reviews.apache.org/r/35981/diff/1/?file=994064#file994064line169
There is no `volumes` field. Just a `resources` field, where each
resource in the list must contain a `disk.volume` to be destroyed.
Michael Park wrote:
I was actually referring to:
```
message Create {
repeated Resource volumes = 1;
}
```
I see, so the inconsistency was in the example json, not in the comment.
Dropping this issue in favor of new ones.
- Adam
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35981/#review89827
---
On June 29, 2015, 10:59 p.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35981/
---
(Updated June 29, 2015, 10:59 p.m.)
Review request for mesos, Adam B, Jie Yu, and Timothy Chen.
Bugs: MESOS-2405
https://issues.apache.org/jira/browse/MESOS-2405
Repository: mesos
Description
---
The Github rendered version is available [here](
https://github.com/mesosphere/mesos/blob/user-docs/docs/persistent-volume.md)
Diffs
-
docs/persistent-volume.md PRE-CREATION
Diff: https://reviews.apache.org/r/35981/diff/
Testing
---
Documentation.
Thanks,
Michael Park
Re: Review Request 35981: Added persistent volume user guide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35981/#review89773 --- Ship it! LGTM overall! Thanks a lot, mpark! docs/persistent-volume.md (lines 23 - 24) https://reviews.apache.org/r/35981/#comment142569 We don't store 'principal' in DiskInfo yet. Maybe add a coming soon somewhere as well? docs/persistent-volume.md (line 29) https://reviews.apache.org/r/35981/#comment142571 via the `acceptOffers` API (or the `ACCEPT` call in the new API). docs/persistent-volume.md (lines 63 - 64) https://reviews.apache.org/r/35981/#comment142574 by sending the following `Offer::Operation` message via the `acceptOffers` API. docs/persistent-volume.md (lines 67 - 69) https://reviews.apache.org/r/35981/#comment142576 The numbering is not correct:) docs/persistent-volume.md (line 68) https://reviews.apache.org/r/35981/#comment142577 We only support non nested relative container path right now. docs/persistent-volume.md (line 69) https://reviews.apache.org/r/35981/#comment142579 Please call out that 'RO' is not supported yet. docs/persistent-volume.md (line 168) https://reviews.apache.org/r/35981/#comment142580 sending the `Offer::Operation` message via the `acceptOffers` API. - Jie Yu On June 28, 2015, 3:35 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35981/ --- (Updated June 28, 2015, 3:35 a.m.) Review request for mesos, Adam B, Jie Yu, and Timothy Chen. Bugs: MESOS-2405 https://issues.apache.org/jira/browse/MESOS-2405 Repository: mesos Description --- The Github rendered version is available [here]( https://github.com/mesosphere/mesos/blob/user-docs/docs/persistent-volume.md) Diffs - docs/persistent-volume.md PRE-CREATION Diff: https://reviews.apache.org/r/35981/diff/ Testing --- Documentation. Thanks, Michael Park
Re: Review Request 35981: Added persistent volume user guide.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35981/#review89827
---
Nice work! Just a few cleanups and questions/clarifications.
docs/persistent-volume.md (line 16)
https://reviews.apache.org/r/35981/#comment142630
s/containig/containing/
docs/persistent-volume.md (line 20)
https://reviews.apache.org/r/35981/#comment142631
s/Reservation/the Reservation doc/
docs/persistent-volume.md (line 56)
https://reviews.apache.org/r/35981/#comment142632
Can a volume/reservation created by one framework principal only be
destroyed/unreserved by a framework scheduler using the same principal? If the
principal is different, then the operation would fail, even if the role or even
frameworkId is identical?
docs/persistent-volume.md (lines 67 - 69)
https://reviews.apache.org/r/35981/#comment142633
You might need a blank line preceding this, and/or some indentation for it
to show up as a numbered list. Doesn't render properly as is.
Unique per role per slave? So I could use the same volumeId for related
volume instances (e.g. my 'logging' or 'data' volumes), up to a max of one per
slave? But if I happen to launch another executor/task on a slave where one of
those volumes is already created/mounted, then I have to rename it ('data2')?
docs/persistent-volume.md (lines 120 - 122)
https://reviews.apache.org/r/35981/#comment142635
Any chance it could return the host_path too? Or do I have to go digging
through slave logs to figure out where my volume was created?
docs/persistent-volume.md (lines 135 - 136)
https://reviews.apache.org/r/35981/#comment142637
Even after you destroy the volume, it still won't be garbage-collected in
0.23. Coming Soon: https://issues.apache.org/jira/browse/MESOS-2408
docs/persistent-volume.md (line 169)
https://reviews.apache.org/r/35981/#comment142641
There is no `volumes` field. Just a `resources` field, where each resource
in the list must contain a `disk.volume` to be destroyed.
- Adam B
On June 27, 2015, 8:35 p.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35981/
---
(Updated June 27, 2015, 8:35 p.m.)
Review request for mesos, Adam B, Jie Yu, and Timothy Chen.
Bugs: MESOS-2405
https://issues.apache.org/jira/browse/MESOS-2405
Repository: mesos
Description
---
The Github rendered version is available [here](
https://github.com/mesosphere/mesos/blob/user-docs/docs/persistent-volume.md)
Diffs
-
docs/persistent-volume.md PRE-CREATION
Diff: https://reviews.apache.org/r/35981/diff/
Testing
---
Documentation.
Thanks,
Michael Park
Re: Review Request 35981: Added persistent volume user guide.
On June 29, 2015, 7:09 p.m., Jie Yu wrote: docs/persistent-volume.md, lines 23-24 https://reviews.apache.org/r/35981/diff/1/?file=994064#file994064line23 We don't store 'principal' in DiskInfo yet. Maybe add a coming soon somewhere as well? I put it at the end actually. Do you think it would read better if we maybe put it at the beginning? Persistent volumes can be created by __operators__ and authorized __frameworks__. We require a `principal` from the operator or framework in order to authenticate/authorize the operations. [Authorization](authorization.md) is specified via the existing ACL mechanism. (___Coming Soon___) Persistent volumes can be created by __operators__ and authorized __frameworks__. ___Coming Soon___: We require a `principal` from the operator or framework in order to authenticate/authorize the operations. [Authorization](authorization.md) is specified via the existing ACL mechanism. On June 29, 2015, 7:09 p.m., Jie Yu wrote: docs/persistent-volume.md, lines 67-69 https://reviews.apache.org/r/35981/diff/1/?file=994064#file994064line67 The numbering is not correct:) This is actually a markdown feature I learned about: ``` It’s important to note that the actual numbers you use to mark the list have no effect on the HTML output Markdown produces. The HTML Markdown produces from the above list is: ol liBird/li liMcHale/li liParish/li /ol If you instead wrote the list in Markdown like this: 1. Bird 1. McHale 1. Parish or even: 3. Bird 1. McHale 8. Parish you’d get the exact same HTML output. ``` It allows us to not have to update all of the other numbers if we were to add more items, remove an item or even rearrange them. It's already used in `docs/release-guide.md` at the end: `Update external tooling`. - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35981/#review89773 --- On June 28, 2015, 3:35 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35981/ --- (Updated June 28, 2015, 3:35 a.m.) Review request for mesos, Adam B, Jie Yu, and Timothy Chen. Bugs: MESOS-2405 https://issues.apache.org/jira/browse/MESOS-2405 Repository: mesos Description --- The Github rendered version is available [here]( https://github.com/mesosphere/mesos/blob/user-docs/docs/persistent-volume.md) Diffs - docs/persistent-volume.md PRE-CREATION Diff: https://reviews.apache.org/r/35981/diff/ Testing --- Documentation. Thanks, Michael Park
Re: Review Request 35981: Added persistent volume user guide.
On June 30, 2015, 12:14 a.m., Adam B wrote:
docs/persistent-volume.md, line 169
https://reviews.apache.org/r/35981/diff/1/?file=994064#file994064line169
There is no `volumes` field. Just a `resources` field, where each
resource in the list must contain a `disk.volume` to be destroyed.
I was actually referring to:
```
message Create {
repeated Resource volumes = 1;
}
```
- Michael
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35981/#review89827
---
On June 30, 2015, 5:50 a.m., Michael Park wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35981/
---
(Updated June 30, 2015, 5:50 a.m.)
Review request for mesos, Adam B, Jie Yu, and Timothy Chen.
Bugs: MESOS-2405
https://issues.apache.org/jira/browse/MESOS-2405
Repository: mesos
Description
---
The Github rendered version is available [here](
https://github.com/mesosphere/mesos/blob/user-docs/docs/persistent-volume.md)
Diffs
-
docs/persistent-volume.md PRE-CREATION
Diff: https://reviews.apache.org/r/35981/diff/
Testing
---
Documentation.
Thanks,
Michael Park
Review Request 35981: Added persistent volume user guide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35981/ --- Review request for mesos, Adam B, Jie Yu, and Timothy Chen. Bugs: MESOS-2405 https://issues.apache.org/jira/browse/MESOS-2405 Repository: mesos Description --- The Github rendered version is available [here]( https://github.com/mesosphere/mesos/blob/user-docs/docs/persistent-volume.md) Diffs - docs/persistent-volume.md PRE-CREATION Diff: https://reviews.apache.org/r/35981/diff/ Testing --- Documentation. Thanks, Michael Park
Re: Review Request 35981: Added persistent volume user guide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35981/#review89655 --- Patch looks great! Reviews applied: [32982, 35981] All tests passed. - Mesos ReviewBot On June 28, 2015, 3:35 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35981/ --- (Updated June 28, 2015, 3:35 a.m.) Review request for mesos, Adam B, Jie Yu, and Timothy Chen. Bugs: MESOS-2405 https://issues.apache.org/jira/browse/MESOS-2405 Repository: mesos Description --- The Github rendered version is available [here]( https://github.com/mesosphere/mesos/blob/user-docs/docs/persistent-volume.md) Diffs - docs/persistent-volume.md PRE-CREATION Diff: https://reviews.apache.org/r/35981/diff/ Testing --- Documentation. Thanks, Michael Park
