Re: Review Request 63022: Imported resources from CSI plugins in storage local resource provider.

2017-10-18 Thread Chun-Hung Hsiao

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/
---

(Updated Oct. 19, 2017, 3:54 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Called `GetNodeID`.


Summary (updated)
-

Imported resources from CSI plugins in storage local resource provider.


Bugs: MESOS-8101
https://issues.apache.org/jira/browse/MESOS-8101


Repository: mesos


Description (updated)
---

The following lists the steps to import resources from a CSI plugin:
1. Launch the node plugin
  1.1 GetSupportedVersions
  1.2 GetPluginInfo
  1.3 ProbeNode
  1.4 GetNodeCapabilities
  1.5 GetNodeID
2. Launch the controller plugin
  2.1 GetSuportedVersions
  2.2 GetPluginInfo
  2.3 GetControllerCapabilities
3. GetCapacity
4. ListVolumes
5. Report to the resource provider through UPDATE_TOTAL_RESOURCES


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


Diff: https://reviews.apache.org/r/63022/diff/6/

Changes: https://reviews.apache.org/r/63022/diff/5-6/


Testing
---


Thanks,

Chun-Hung Hsiao



Re: Review Request 63021: Added functions to launch CSI plugin in storage local resource provider.

2017-10-18 Thread Chun-Hung Hsiao

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63021/
---

(Updated Oct. 19, 2017, 3:49 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Changed the `csi_plugins` map to repeated field.


Bugs: MESOS-8032
https://issues.apache.org/jira/browse/MESOS-8032


Repository: mesos


Description
---

During initialization, the storage local resource provider first tries
to recover its ID and CSI socket dir of the last session through reading
the actual path linked by
`work_dir/resource_providers///latest`, then subscribe to
the agent's resource provider manager. If this is a new subscription,
it will set up a new dir for CSI plugins to put their socket files.

Once the CSI socket dir is set up, the storage local resource provider
can connect to a CSI plugin through the following procedure:
1. It first tries to connect to the existing socket file if there is
   one. On success, just return the connection.
2. On failure, kill the running plugin and remove the socket file.
3. Launch a container to run the plugin, and w for the socket file to
   appear, then connect to the socket file.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


Diff: https://reviews.apache.org/r/63021/diff/5/

Changes: https://reviews.apache.org/r/63021/diff/4-5/


Testing
---


Thanks,

Chun-Hung Hsiao



Re: Review Request 63017: Added storage resource provider information in ResourceProviderInfo.

2017-10-18 Thread Chun-Hung Hsiao

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63017/
---

(Updated Oct. 19, 2017, 3:23 a.m.)


Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, Joseph Wu, 
and Jan Schlicht.


Changes
---

Removed the use of proto2 map.


Bugs: MESOS-7561
https://issues.apache.org/jira/browse/MESOS-7561


Repository: mesos


Description
---

The `CSIPlugiInfo` protobuf specifies the configurations to launch a
CSI plugin in a standalone container.

The `ResourceProviderInfo::StorageInfo` protobuf specifies which CSI
plugins are the controller and node plugins for a storage resource
provider.


Diffs (updated)
-

  include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466 
  include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282 


Diff: https://reviews.apache.org/r/63017/diff/2/

Changes: https://reviews.apache.org/r/63017/diff/1-2/


Testing
---

make


Thanks,

Chun-Hung Hsiao



Re: Review Request 63017: Added storage resource provider information in ResourceProviderInfo.

2017-10-18 Thread Chun-Hung Hsiao

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63017/#review188629
---




include/mesos/v1/mesos.proto
Lines 976 (patched)


I'll change the map so we don't depend on the proto2 map support for now.


- Chun-Hung Hsiao


On Oct. 16, 2017, 5:58 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63017/
> ---
> 
> (Updated Oct. 16, 2017, 5:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, Joseph 
> Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-7561
> https://issues.apache.org/jira/browse/MESOS-7561
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `CSIPlugiInfo` protobuf specifies the configurations to launch a
> CSI plugin in a standalone container.
> 
> The `ResourceProviderInfo::StorageInfo` protobuf specifies which CSI
> plugins are the controller and node plugins for a storage resource
> provider.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 830985a3265b7c104d8fdc50749c395d98f5f3c8 
>   include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
> 
> 
> Diff: https://reviews.apache.org/r/63017/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 63104: Added a helper to extract resources from storage operations.

2017-10-18 Thread Jie Yu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63104/#review188625
---




src/common/protobuf_utils.hpp
Lines 188 (patched)


I would call it `protobuf::getConsumedResources`


- Jie Yu


On Oct. 18, 2017, 2:36 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63104/
> ---
> 
> (Updated Oct. 18, 2017, 2:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7594
> https://issues.apache.org/jira/browse/MESOS-7594
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper to extract resources from storage operations.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp c43ab75b5492320dfe19a7c723a72ac52b8ab722 
>   src/common/protobuf_utils.cpp fd4858a64dfc136dd03cb1eef4c97d0f8d43bdae 
> 
> 
> Diff: https://reviews.apache.org/r/63104/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 63107: Added operation feedback for storage operations.

2017-10-18 Thread Jie Yu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/#review188618
---



Not a complete review yet. Will take look again once existing issues are 
addressed.


src/master/master.cpp
Line 4524 (original), 4526 (patched)


We probably needs to send `OfferOperation` too here?



src/master/master.cpp
Lines 5103-5107 (original), 5105-5109 (patched)


We shouldn't use `Resources::apply` here. `apply` to me is to apply a 
conversion. This is basically consume resources! Follow your logic, why we 
don't do anything for LAUNCH? is it consuming resources too?

The following `slave->apply` is problematic. It'll reduce slave's 
`totalResources`?



src/master/master.cpp
Line 5111 (original), 5113 (patched)


Again, I won't call it "Applying", i would call it "Processing"



src/master/master.cpp
Lines 5126 (patched)


I think putting operations in each Framework struct makes more sense. It'll 
help us to see all pending operations from a given framework in state.json or 
in UI in the future.

I'd suggest we have a `Framework::addOfferOperation(...)` method.

```
Framework::addOfferOperation(
const Offer.Operation& info,
const string& uuid)
```



src/master/master.cpp
Line 5157 (original), 5161 (patched)


This will update `checkpointedResources`, even if this operation is for a 
resource provider provided resources. Do we want to do that? I think 
`checkpointedResources` should be limited to agent default resources (no 
resource provider).



src/master/master.cpp
Lines 7080 (patched)


Do we want to set this if it's old operations like RESERVE?



src/master/master.cpp
Lines 7083 (patched)


slave->totalResources should not change (just like launching a task does 
not change slave's total).

you should do a Resource::apply here for new operations with additional 
`converted_resources`:

```
Resources::apply(
const Offer.Operation& operation,
const Option& converted_resources = None())
```



src/master/master.cpp
Lines 7085 (patched)


Let's make sure `checkpointedResources` is only for agent default 
resources. We only need that because the new master old agent case (depends on 
agent capabilities, we'll still need to send CheckpointResourcesMessage



src/master/master.cpp
Lines 7088-7089 (patched)


This shouldn't be necessary.



src/resource_provider/manager.cpp
Lines 318-324 (original), 319-325 (patched)


Hum, shouldn't we have a validation function for this? I think a single 
operation cannot operate on resources from different providers.



src/resource_provider/manager.cpp
Lines 329-332 (original), 330-333 (patched)


We probably need to maintain state for resource providers. What if a 
resource provider is temporarily disconnected?

Chatted with CHun on this yesterday. I think it makes sense to decouple RP 
regisration from RP reporting resources.

As a result, each RP will have `CONNECTED` as well as `READY` state.

CONNECTED means that the RP subscribed, but hasn't reported any resources 
yet. Operation should be dropped in that case.



src/resource_provider/manager.cpp
Line 330 (original), 331 (patched)


We need to log any operation dropping in the log.



src/resource_provider/manager.cpp
Lines 344-345 (original), 345-346 (patched)


This is the same as dropping an operation. Please log the operation.



src/resource_provider/message.hpp
Line 36 (original), 37 (patched)


Not yours, this should probably be `UpdateState`



src/resource_provider/message.hpp
Lines 46 (patched)


I'd call it `UpdateOfferOperationStatus`


- Jie Yu


On Oct. 18, 2017, 2:39 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> ---
> 
> (Updated Oct. 18, 2017, 2:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: 

Re: Review Request 63095: Added the Getting Started landing page.

2017-10-18 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63095/#review188622
---



Patch looks great!

Reviews applied: [62548, 63093, 63095]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Oct. 18, 2017, 6:46 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63095/
> ---
> 
> (Updated Oct. 18, 2017, 6:46 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After moving the build documentation to its own page, we can now have a
> real "Getting Started" page suitable for anyone to get started with
> Mesos. It is purposefully short, and therefore not overwhelming.
> 
> 
> Diffs
> -
> 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/source/downloads.html.erb 3e24100a76a99b23ebc6589ee4f7332f43ef0ac9 
>   site/source/getting-started.html.md PRE-CREATION 
>   site/source/index.html.erb b4e847fadf6682503a4e09139491b539bc59e0fd 
>   site/source/layouts/documentation.erb 
> a91f916a5fb7348b2702c272e7a2059bdf628c66 
>   site/source/layouts/getting_started_section.erb PRE-CREATION 
>   site/source/layouts/layout.erb 9213c63e6fef3f57fc225a87a7d7e0abdcc11a88 
> 
> 
> Diff: https://reviews.apache.org/r/63095/diff/2/
> 
> 
> Testing
> ---
> 
> Built the site and tested it out locally.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63094: Added resource version uuid for offer operations.

2017-10-18 Thread Jie Yu


> On Oct. 18, 2017, 9:46 a.m., Benjamin Bannier wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 56-66 (patched)
> > 
> >
> > I would prefer if we would not assume that this number changes for 
> > _every_ resource change, but just for failures.
> > * * *
> > 
> > For one we do not want users to assume that this number has certain 
> > values so we can recognize failures. Imagine a user applied an operation to 
> > the resource provider resources (`sequence_id=0`), and the agent speculated 
> > that the new `sequence_id` would be `1`. Now the operation fails in the RP 
> > and its `sequence_id` would be incremented to `1`. We cannot distinguish a 
> > case where the agent assumed the operation succeeded from a case where it 
> > failed based on the clock value alone in this case. Would we just increase 
> > this number in case of failures it would be clear that any operation 
> > assuming `sequence_id=0` would be invalid after the failure since the agent 
> > would be unlikely to assume failures on a "normal" execution path. 
> > 
> > * * *
> > 
> > Another reason would be that it would be great to be able to use 
> > similar notions of `sequence_id` for the agent's view of its RPs, and the 
> > master's view of its agents. We plan to implement speculative application 
> > of "legacy" offer operations like e.g., `RESERVE`, `CREATE` in the master, 
> > and incrementing this number even in successful cases would require too 
> > much knowledge about how the agent applies these operations in the master.
> > 
> > Imagine an agent with resource provider resources `disk(*):1` and 
> > `disk(*):1` (each `disk` from a different resource provider). The agent 
> > will expose `disk(*):2` to the master. A framework now reserves, 
> > `disk(*):2->disk(A):2`. The agent would inform the resource providers about 
> > that operation. Let's say the first of these operations succeeds, but the 
> > second on fails (the argument works as well for the other order, or even 
> > concurrent application). Having the master increment this 
> > `resource_sequence_id` for speculated operations would require it to know 
> > that the single `RESERVE` operation did map onto two state changes, i.e., 
> > it should have incremented its agent `resource_sequence_id` by two instead 
> > of just by one.
> > 
> > This seems not only leaky, but also inflexible since it becomes hard to 
> > decompose an agent's resources without affecting the master's assumptions.
> > 
> > * * *
> > 
> > Having `sequence_id` just increase in case of failures would eliminate 
> > a large class of cases where users would need to make assumption about how 
> > it does change. Instead we would more clearly move into a direction where 
> > it is only changed by pushes from lower layers (where an operation either 
> > succeeded or failed) to user layers above. This should also simplify how we 
> > would project multiple `sequence_id`s onto a single `sequence_id` (multiple 
> > resource provider `sequence_id`s onto agent `sequence_id`); in that case we 
> > could just take the maximum of all RP IDs and the agent ID (e.g., last 
> > failure of an operation on any RP is equivalent to last failure of any 
> > operation on the agent). Counting all operations makes this harder, 
> > especially when thinking of master-speculated operations.
> 
> Greg Mann wrote:
> This confused me a bit: "Having the master increment this 
> resource_sequence_id for speculated operations"
> 
> IIUC, the master will only increment the sequence ID when it receives a 
> {Register,Reregister,Update}SlaveMessage from the agent containing a new ID. 
> So, I don't think the ID is incremented routinely during the successful 
> application of offer operations.
> 
> My understanding of the comment in this patch is that the sequence ID 
> will be incremented only when a legacy operation fails, or when the RP has 
> its total resources updated (i.e., storage backend has more/less storage to 
> offer and updates the CSI plugin/RP accordingly).
> 
> Jie Yu wrote:
> I had some offline discussion with Benjamin. It looks to us that using a 
> single scalar clock value for the agent does not make sense. It's more easy 
> to reason about the correctness if we maintain per RP resource version uuid. 
> I updated this review with the new thinking. PTAL.

See this notes to see why a single scalar clock for the agnet does not work:
https://docs.google.com/document/d/1RrrLVATZUyaURpEOeGjgxA6ccshuLo94G678IbL-Yco/edit#bookmark=id.v893d0oct2wh


- Jie


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63094/#review188465
---


On Oct. 19, 2017, 12:10 a.m., Jie Yu wrote:
> 
> 

Re: Review Request 63094: Added resource version uuid for offer operations.

2017-10-18 Thread Jie Yu


> On Oct. 18, 2017, 9:46 a.m., Benjamin Bannier wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 56-66 (patched)
> > 
> >
> > I would prefer if we would not assume that this number changes for 
> > _every_ resource change, but just for failures.
> > * * *
> > 
> > For one we do not want users to assume that this number has certain 
> > values so we can recognize failures. Imagine a user applied an operation to 
> > the resource provider resources (`sequence_id=0`), and the agent speculated 
> > that the new `sequence_id` would be `1`. Now the operation fails in the RP 
> > and its `sequence_id` would be incremented to `1`. We cannot distinguish a 
> > case where the agent assumed the operation succeeded from a case where it 
> > failed based on the clock value alone in this case. Would we just increase 
> > this number in case of failures it would be clear that any operation 
> > assuming `sequence_id=0` would be invalid after the failure since the agent 
> > would be unlikely to assume failures on a "normal" execution path. 
> > 
> > * * *
> > 
> > Another reason would be that it would be great to be able to use 
> > similar notions of `sequence_id` for the agent's view of its RPs, and the 
> > master's view of its agents. We plan to implement speculative application 
> > of "legacy" offer operations like e.g., `RESERVE`, `CREATE` in the master, 
> > and incrementing this number even in successful cases would require too 
> > much knowledge about how the agent applies these operations in the master.
> > 
> > Imagine an agent with resource provider resources `disk(*):1` and 
> > `disk(*):1` (each `disk` from a different resource provider). The agent 
> > will expose `disk(*):2` to the master. A framework now reserves, 
> > `disk(*):2->disk(A):2`. The agent would inform the resource providers about 
> > that operation. Let's say the first of these operations succeeds, but the 
> > second on fails (the argument works as well for the other order, or even 
> > concurrent application). Having the master increment this 
> > `resource_sequence_id` for speculated operations would require it to know 
> > that the single `RESERVE` operation did map onto two state changes, i.e., 
> > it should have incremented its agent `resource_sequence_id` by two instead 
> > of just by one.
> > 
> > This seems not only leaky, but also inflexible since it becomes hard to 
> > decompose an agent's resources without affecting the master's assumptions.
> > 
> > * * *
> > 
> > Having `sequence_id` just increase in case of failures would eliminate 
> > a large class of cases where users would need to make assumption about how 
> > it does change. Instead we would more clearly move into a direction where 
> > it is only changed by pushes from lower layers (where an operation either 
> > succeeded or failed) to user layers above. This should also simplify how we 
> > would project multiple `sequence_id`s onto a single `sequence_id` (multiple 
> > resource provider `sequence_id`s onto agent `sequence_id`); in that case we 
> > could just take the maximum of all RP IDs and the agent ID (e.g., last 
> > failure of an operation on any RP is equivalent to last failure of any 
> > operation on the agent). Counting all operations makes this harder, 
> > especially when thinking of master-speculated operations.
> 
> Greg Mann wrote:
> This confused me a bit: "Having the master increment this 
> resource_sequence_id for speculated operations"
> 
> IIUC, the master will only increment the sequence ID when it receives a 
> {Register,Reregister,Update}SlaveMessage from the agent containing a new ID. 
> So, I don't think the ID is incremented routinely during the successful 
> application of offer operations.
> 
> My understanding of the comment in this patch is that the sequence ID 
> will be incremented only when a legacy operation fails, or when the RP has 
> its total resources updated (i.e., storage backend has more/less storage to 
> offer and updates the CSI plugin/RP accordingly).

I had some offline discussion with Benjamin. It looks to us that using a single 
scalar clock value for the agent does not make sense. It's more easy to reason 
about the correctness if we maintain per RP resource version uuid. I updated 
this review with the new thinking. PTAL.


- Jie


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63094/#review188465
---


On Oct. 19, 2017, 12:10 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> 

Re: Review Request 63094: Added resource version uuid for offer operations.

2017-10-18 Thread Jie Yu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63094/
---

(Updated Oct. 19, 2017, 12:10 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, 
Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.


Changes
---

Used UUID and a vector instead. Singular clock idea for the agent does not 
work. See reasoning in this notes:
https://docs.google.com/document/d/1RrrLVATZUyaURpEOeGjgxA6ccshuLo94G678IbL-Yco/edit#bookmark=id.v893d0oct2wh

This patch is based on https://reviews.apache.org/r/62903/


Summary (updated)
-

Added resource version uuid for offer operations.


Repository: mesos


Description (updated)
---

The resource version UUID is used to establish the relationship
between the operation and the resources that the operation is
operating on. Each resource provider will keep a resource version
UUID, and changes it when it believes that the resources from this
resource provider are out of sync from the master's view.  The master
will keep track of the last known resource version UUID for each
resource provider, and attach the resource version UUID in each
operation it sends out. The resource provider should reject operations
that have a different resource version UUID than that it maintains,
because this means the operation is operating on resources that might
have already been invalidated.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto 
e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/tests/resource_provider_manager_tests.cpp 
ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
  src/tests/resource_provider_validation_tests.cpp 
f182bff4670318e9de22c2915c5dbb423a74ad6c 


Diff: https://reviews.apache.org/r/63094/diff/2/

Changes: https://reviews.apache.org/r/63094/diff/1-2/


Testing
---

make check


Thanks,

Jie Yu



Re: Review Request 63001: Updated protobuf definitions related to offer operations.

2017-10-18 Thread Jie Yu

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
---

(Updated Oct. 19, 2017, 12:08 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
Greg Mann, Jan Schlicht, and Vinod Kone.


Changes
---

Rebased.


Repository: mesos


Description
---

Updated protobuf definitions related to offer operations.


Diffs (updated)
-

  include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466 
  include/mesos/resource_provider/resource_provider.proto 
f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282 
  include/mesos/v1/resource_provider/resource_provider.proto 
e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
  src/tests/resource_provider_validation_tests.cpp 
f182bff4670318e9de22c2915c5dbb423a74ad6c 


Diff: https://reviews.apache.org/r/63001/diff/9/

Changes: https://reviews.apache.org/r/63001/diff/8-9/


Testing
---

make check


Thanks,

Jie Yu



Re: Review Request 62911: Added a RWMutex to libprocess.

2017-10-18 Thread Benjamin Mahler


> On Oct. 17, 2017, 9:56 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/rwmutex.hpp
> > Lines 28 (patched)
> > 
> >
> > Did a scan of other libraries, interestingly these are named pretty 
> > differently:
> > 
> > c++: shared_mutex (generic terminology, assuming not necessarily used 
> > for read/write pattern)
> > 
> > go: RWMutex
> > rust: RWLock
> > java: ReadWriteLock
> > 
> > `ReadWriteLock` seems like the best choice for us.
> > 
> > Also, we should have done this for `process::Mutex` and I realize 
> > you're following the lack of documentation there, but it would be great to 
> > document this class, specifically the priority policy that is currently 
> > burned in (it looks like the same notion of fairness described here: 
> > https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html
> >  ?).
> 
> Zhitao Li wrote:
> Ack.
> 
> Do you think we should also rename `Mutex` to `Lock` for consistency.

Since "lock" doesn't specify what kind of lock, I think the consistent naming 
there would be `MutexLock` and `ReadWriteLock`. I'm ok with either `MutexLock` 
or `Mutex`, since people generally understand that `Mutex` is a lock, and this 
is the naming across go, rust, c++.


> On Oct. 17, 2017, 9:56 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/rwmutex.hpp
> > Lines 50 (patched)
> > 
> >
> > Not your fault since you're just copying, but as an aside, we probably 
> > should have designed process::Mutex::lock to return a `Locked` object that 
> > the user can release, which is safer than having someone else potentially 
> > call into unlock accidentally.
> 
> Zhitao Li wrote:
> I like this design. I'll see how much I can incorporate it in this patch.

I noticed it's similar to what rust provides for its synchronous RWLock, but I 
don't think you need to tackle this here if you don't want to since it's pretty 
subtle. For example, we can't quite use the RAII design that rust has because 
it's pretty implicit as to when a last future reference goes away. If we have a 
method on the returned locked object, then we have to crash if it's called 
twice, whereas the RAII design doesn't have this issue.


> On Oct. 17, 2017, 9:56 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/rwmutex.hpp
> > Lines 95-96 (patched)
> > 
> >
> > This would probably read a bit easier as:
> > 
> > ```
> > if (unlocked) {
> >   grab read lock
> > } else if (read locked and no waiters) {
> >   grab read lock
> > } else {
> >   queue
> > }
> > ```
> > 
> > Also it's probably better to explain the priority semantics at the top 
> > rather than here.
> 
> Zhitao Li wrote:
> Ack for moving the comment into class comment.
> 
> For the conditions, we would have duplicate on "grab read lock" which I 
> tried to avoid. Do you think that's more readable?

Well, I just wrote the conditional that way because that's how I reasoned about 
it when I wrote the suggested comment about the lock states.

When I read the condition as it is now, I feel like I have to infer from it 
(i.e. to proceed it has to be not write locked and no one is waiting, which 
means that either (1) it's unlocked or (2) a read is in progress with no one 
waiting). So the reasoning seems less direct to me when reading.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62911/#review188392
---


On Oct. 17, 2017, 7:56 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62911/
> ---
> 
> (Updated Oct. 17, 2017, 7:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, 
> and Jason Lai.
> 
> 
> Bugs: MESOS-8075
> https://issues.apache.org/jira/browse/MESOS-8075
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `RWMutex` class is similar to `Mutex`, but allows extra concurrency if
> some actions can be performed concurrently safely while certain actions
> require full mutual exclusive.
> 
> This implementation guarantees starvation free for `lock()` by queuing up
> `rlock()` when some `lock()` is already in queue.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> 94c7a722aab6c36174f117f0b6239cb988e476a9 
>   3rdparty/libprocess/include/process/rwmutex.hpp PRE-CREATION 
> 
> 
> Diff: 

Re: Review Request 63095: Added the Getting Started landing page.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63095/#review188591
---



PASS: Mesos patch 63095 was successfully built and tested.

Reviews applied: `['62548', '63093', '63095']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63095

- Mesos Reviewbot Windows


On Oct. 18, 2017, 11:46 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63095/
> ---
> 
> (Updated Oct. 18, 2017, 11:46 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After moving the build documentation to its own page, we can now have a
> real "Getting Started" page suitable for anyone to get started with
> Mesos. It is purposefully short, and therefore not overwhelming.
> 
> 
> Diffs
> -
> 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/source/downloads.html.erb 3e24100a76a99b23ebc6589ee4f7332f43ef0ac9 
>   site/source/getting-started.html.md PRE-CREATION 
>   site/source/index.html.erb b4e847fadf6682503a4e09139491b539bc59e0fd 
>   site/source/layouts/documentation.erb 
> a91f916a5fb7348b2702c272e7a2059bdf628c66 
>   site/source/layouts/getting_started_section.erb PRE-CREATION 
>   site/source/layouts/layout.erb 9213c63e6fef3f57fc225a87a7d7e0abdcc11a88 
> 
> 
> Diff: https://reviews.apache.org/r/63095/diff/2/
> 
> 
> Testing
> ---
> 
> Built the site and tested it out locally.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 62911: Added a RWMutex to libprocess.

2017-10-18 Thread Zhitao Li


> On Oct. 17, 2017, 9:56 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/rwmutex.hpp
> > Lines 28 (patched)
> > 
> >
> > Did a scan of other libraries, interestingly these are named pretty 
> > differently:
> > 
> > c++: shared_mutex (generic terminology, assuming not necessarily used 
> > for read/write pattern)
> > 
> > go: RWMutex
> > rust: RWLock
> > java: ReadWriteLock
> > 
> > `ReadWriteLock` seems like the best choice for us.
> > 
> > Also, we should have done this for `process::Mutex` and I realize 
> > you're following the lack of documentation there, but it would be great to 
> > document this class, specifically the priority policy that is currently 
> > burned in (it looks like the same notion of fairness described here: 
> > https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html
> >  ?).

Ack.

Do you think we should also rename `Mutex` to `Lock` for consistency.


> On Oct. 17, 2017, 9:56 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/rwmutex.hpp
> > Lines 33 (patched)
> > 
> >
> > How about write_lock, write_unlock, read_lock, read_unlock?

ack. The current names were borrowed from golang which I uses mostly, but your 
set of names seem better for `ReadWriteLock`.


> On Oct. 17, 2017, 9:56 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/rwmutex.hpp
> > Lines 50 (patched)
> > 
> >
> > Not your fault since you're just copying, but as an aside, we probably 
> > should have designed process::Mutex::lock to return a `Locked` object that 
> > the user can release, which is safer than having someone else potentially 
> > call into unlock accidentally.

I like this design. I'll see how much I can incorporate it in this patch.


> On Oct. 17, 2017, 9:56 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/rwmutex.hpp
> > Lines 58 (patched)
> > 
> >
> > Don't want to check both?
> > 
> > ```
> > CHECK(data->write_locked);
> > CHECK_EQ(data->read_locked, 0u);
> > ```

Ack.


> On Oct. 17, 2017, 9:56 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/rwmutex.hpp
> > Lines 75-77 (patched)
> > 
> >
> > This is a little hard to follow, maybe move the empty case to the top?

I'm going to move `data->write_locked = false;` unconditionally right after top 
level `CHECK`s. If we should reacquire write lock, reassign true to it.


> On Oct. 17, 2017, 9:56 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/include/process/rwmutex.hpp
> > Lines 95-96 (patched)
> > 
> >
> > This would probably read a bit easier as:
> > 
> > ```
> > if (unlocked) {
> >   grab read lock
> > } else if (read locked and no waiters) {
> >   grab read lock
> > } else {
> >   queue
> > }
> > ```
> > 
> > Also it's probably better to explain the priority semantics at the top 
> > rather than here.

Ack for moving the comment into class comment.

For the conditions, we would have duplicate on "grab read lock" which I tried 
to avoid. Do you think that's more readable?


- Zhitao


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62911/#review188392
---


On Oct. 17, 2017, 7:56 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62911/
> ---
> 
> (Updated Oct. 17, 2017, 7:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, Gilbert Song, 
> and Jason Lai.
> 
> 
> Bugs: MESOS-8075
> https://issues.apache.org/jira/browse/MESOS-8075
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `RWMutex` class is similar to `Mutex`, but allows extra concurrency if
> some actions can be performed concurrently safely while certain actions
> require full mutual exclusive.
> 
> This implementation guarantees starvation free for `lock()` by queuing up
> `rlock()` when some `lock()` is already in queue.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/Makefile.am 
> 94c7a722aab6c36174f117f0b6239cb988e476a9 
>   3rdparty/libprocess/include/process/rwmutex.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62911/diff/3/
> 
> 
> Testing
> ---
> 
> 
> 

Re: Review Request 62980: Added link anchors to all website headings.

2017-10-18 Thread Greg Mann

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62980/#review188573
---


Ship it!




I just verified this using the web development Docker container; looks great! 
Thanks James!!!

I think black is OK for the links; I'll merge as-is and we can always update 
later.

- Greg Mann


On Oct. 13, 2017, 4:58 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62980/
> ---
> 
> (Updated Oct. 13, 2017, 4:58 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This follows the example of the Middleman documentation and
> uses AnchorJS to inject link anchors to all headings at page
> load time. The styling is basic black to match the styling of
> the header titles.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 83596ddbd833e36b60bdbbd487ebd464b3874119 
>   site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 
> 
> 
> Diff: https://reviews.apache.org/r/62980/diff/3/
> 
> 
> Testing
> ---
> 
> Verified manually using the Docker image to run Middleman in dev mode.
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 63107: Added operation feedback for storage operations.

2017-10-18 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/#review188571
---



Bad patch!

Reviews applied: [63107, 63106, 61947, 63105, 61946, 61810, 63104, 63001, 
61183, 62158, 62655]

Failed command: python support/apply-reviews.py -n -r 63107

Error:
2017-10-18 20:27:48 URL:https://reviews.apache.org/r/63107/diff/raw/ 
[12456/12456] -> "63107.patch" [1]
error: patch failed: src/resource_provider/message.hpp:18
error: src/resource_provider/message.hpp: patch does not apply
error: patch failed: src/slave/slave.cpp:6660
error: src/slave/slave.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19839/console

- Mesos Reviewbot


On Oct. 18, 2017, 2:39 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> ---
> 
> (Updated Oct. 18, 2017, 2:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8087
> https://issues.apache.org/jira/browse/MESOS-8087
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework accepts an offer that contains resource provider
> resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
> `CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
> be guessed and will only be known after the operation has been
> successfully applied by the resource provider.
> This patch introduces the necessary handling for such operations. The
> internal bookkeeping of available resources in the master and agent has
> been updated to update resources only after operation feedback has been
> received. This ensures that converted resources can only be offered
> after the operation was executed by a resource provider.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 
> 
> 
> Diff: https://reviews.apache.org/r/63107/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 63001: Updated protobuf definitions related to offer operations.

2017-10-18 Thread Chun-Hung Hsiao


> On Oct. 17, 2017, 5:51 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Lines 2187-2195 (patched)
> > 
> >
> > We also need to include a unique UUID for each operation status update, 
> > so that the framework can include it when acknowledging updates.
> > 
> > If we follow the pattern of task status updates, we could include an 
> > `optional bytes uuid` field here.

Hmm I missed the comment here :(. Please see my comments below. How about a 
wrapper protobuf that includes an `OfferOperationStatus` and a `optional bytes 
uuid` that is only used internally in Mesos, or introduce another protobuf for 
`resource_provider.Call.UpdateOperationStatus` instead of reusing this one?


- Chun-Hung


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188340
---


On Oct. 17, 2017, 10:10 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> ---
> 
> (Updated Oct. 17, 2017, 10:10 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
> Greg Mann, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf definitions related to offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/resource_provider_validation_tests.cpp 
> f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63001/diff/8/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 63001: Updated protobuf definitions related to offer operations.

2017-10-18 Thread Chun-Hung Hsiao

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188565
---



Since we're designing the RP API, I'd prefer to only expose fields that are 
required by RP's polymorphic behaviors, and hide the things that should only be 
known by Mesos in the RP manager, even though this implies that the code of the 
RP manager would become more complicated, which I think it is an affordable 
cost in general.


include/mesos/v1/mesos.proto
Lines 2170 (patched)


This protobuf is exposed to RP API. Do we want an RP to be aware of the 
scheduler? Do we allow each RP to decide if an operation should be acked? If we 
don't, then I think we should handle this `uuid` in the RP manager. If we do, 
then we should also include the ACK mechanism in the RP API.


- Chun-Hung Hsiao


On Oct. 17, 2017, 10:10 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> ---
> 
> (Updated Oct. 17, 2017, 10:10 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, 
> Greg Mann, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated protobuf definitions related to offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/resource_provider_validation_tests.cpp 
> f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63001/diff/8/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 63116: Mesos UI: fixed use of undefined variable for the pailer title.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63116/#review188559
---



PASS: Mesos patch 63116 was successfully built and tested.

Reviews applied: `['63116']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63116

- Mesos Reviewbot Windows


On Oct. 18, 2017, 7 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63116/
> ---
> 
> (Updated Oct. 18, 2017, 7 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes the use of an undefined variable to generate the
> pailer's window title in `pailer()`.
> 
> The title used for pailers that read agent files used to be: "path
> (host:port)", it is now "path (agentId)".
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> fa0ad54cbcec7a3b0bea9e2b0dc70a4c3f41e822 
> 
> 
> Diff: https://reviews.apache.org/r/63116/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>



Re: Review Request 62548: Reorganized and updated the contribution guidelines.

2017-10-18 Thread Greg Mann

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62548/
---

(Updated Oct. 18, 2017, 7:36 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, and Vinod Kone.


Bugs: MESOS-564
https://issues.apache.org/jira/browse/MESOS-564


Repository: mesos


Description (updated)
---

This patch splits the contribution guidelines into two parts:
a beginner contribution guide, and an advanced guide. Some files
are also renamed, and links updated.


Diffs
-

  docs/beginner-contribution.md PRE-CREATION 
  docs/home.md 6a6bd736367505e00c27e5ecd37a68c8e01efe10 
  docs/newbie-guide.md e9f2aaac1bb986120f34b1d006e3a2f5eb2779ff 
  docs/reopening-reviews.md fe5046830bbdf28fcc2377ffa5792549920afbc8 
  docs/reporting-a-bug.md a7e372c1f0d3a34a06244aecb1dfeef7356b8928 
  docs/submitting-a-patch.md ffc6e561b8721b8849ef6025c15936ea712d3bfa 
  site/source/community.html.md f56131fedb935ff695206948e16252c62ae0f36a 


Diff: https://reviews.apache.org/r/62548/diff/5/


Testing (updated)
---

The updated docs were examined using GitHub's markdown viewer.


Thanks,

Greg Mann



Re: Review Request 62548: Reorganized and updated the contribution guidelines.

2017-10-18 Thread Greg Mann

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62548/
---

(Updated Oct. 18, 2017, 7:35 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, and Vinod Kone.


Bugs: MESOS-564
https://issues.apache.org/jira/browse/MESOS-564


Repository: mesos


Description
---

Reorganized and updated the contribution guidelines.


Diffs (updated)
-

  docs/beginner-contribution.md PRE-CREATION 
  docs/home.md 6a6bd736367505e00c27e5ecd37a68c8e01efe10 
  docs/newbie-guide.md e9f2aaac1bb986120f34b1d006e3a2f5eb2779ff 
  docs/reopening-reviews.md fe5046830bbdf28fcc2377ffa5792549920afbc8 
  docs/reporting-a-bug.md a7e372c1f0d3a34a06244aecb1dfeef7356b8928 
  docs/submitting-a-patch.md ffc6e561b8721b8849ef6025c15936ea712d3bfa 
  site/source/community.html.md f56131fedb935ff695206948e16252c62ae0f36a 


Diff: https://reviews.apache.org/r/62548/diff/5/

Changes: https://reviews.apache.org/r/62548/diff/4-5/


Testing
---


Thanks,

Greg Mann



Re: Review Request 62548: Reorganized and updated the contribution guidelines.

2017-10-18 Thread Greg Mann

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62548/
---

(Updated Oct. 18, 2017, 7:23 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, and Vinod Kone.


Bugs: MESOS-564
https://issues.apache.org/jira/browse/MESOS-564


Repository: mesos


Description
---

Reorganized and updated the contribution guidelines.


Diffs (updated)
-

  docs/beginner-contribution.md PRE-CREATION 
  docs/home.md 6a6bd736367505e00c27e5ecd37a68c8e01efe10 
  docs/newbie-guide.md e9f2aaac1bb986120f34b1d006e3a2f5eb2779ff 
  docs/reopening-reviews.md fe5046830bbdf28fcc2377ffa5792549920afbc8 
  docs/reporting-a-bug.md a7e372c1f0d3a34a06244aecb1dfeef7356b8928 
  docs/submitting-a-patch.md ffc6e561b8721b8849ef6025c15936ea712d3bfa 
  site/source/community.html.md f56131fedb935ff695206948e16252c62ae0f36a 


Diff: https://reviews.apache.org/r/62548/diff/4/

Changes: https://reviews.apache.org/r/62548/diff/3-4/


Testing
---


Thanks,

Greg Mann



Re: Review Request 63113: Fix flakyness in 'SlaveTest.ExecutorShutdownGracePeriod'.

2017-10-18 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63113/#review188550
---


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 18, 2017, 3:49 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63113/
> ---
> 
> (Updated Oct. 18, 2017, 3:49 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On slow systems, it could happen that an additional offer was
> delivered to the scheduler before the test case finished.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 91d97d195acd695ae9c469651596511eafb50557 
> 
> 
> Diff: https://reviews.apache.org/r/63113/diff/1/
> 
> 
> Testing
> ---
> 
> `./mesos-tests --gtest_filter="*ExecutorShutdownGracePeriod*" 
> --gtest_repeat=5000 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 62213: Fix unit tests that were broken by the additional TASK_STARTING update.

2017-10-18 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62213/#review188553
---


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 18, 2017, 4:24 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62213/
> ---
> 
> (Updated Oct. 18, 2017, 4:24 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 0e99e7bf3a2cf4243cd73a5cb857bfc4d4e55f78 
>   src/tests/check_tests.cpp fd15a47f1d67ad852d84d83aad54c9fa93c60bbb 
>   src/tests/command_executor_tests.cpp 
> f706f55b5bfab824268498d95d775b216561cd66 
>   src/tests/container_logger_tests.cpp 
> fb8e441bd3842b1840a384a025fa8e9ccbb22cf6 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 3fc93417f2d3febf2feca3ec1c8476c9edcfbf4d 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> e673d914fa9251fa585deea3a29438371c185fdb 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> 153990d8960b1e74379e37b2e2ddf23f242a3712 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 45f0d1dbc5b21b174cd9974664299d466129df01 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> ed7d43840bf73fd4dd5c21f8d65e0a538338ee26 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> cf7b9eb6fd76dbdd3650ea2ad5b9097cf490e948 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> a657a6f60995330d14861e86df59b090ab7a015e 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> 14bd7050e147fabfae5fb32fe14d78a893a892c5 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> c4d8bfc63d6c0953cc2b3524401767fdbdf0648d 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> f8b4423de8a468501336acc5ee0c67f181dc65f5 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 56e193bd84222648f47e36d6a1107046b2954977 
>   src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
> 0030cd1e1f73422bef806bbc0453134e3d7840d8 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 89fe4fcd1b123edc762835595432f24cb699fd61 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 920be77b16178a4458d72145020c015130799ec4 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> ea5d035ee25ead1966db8b9e4772180dfb20747d 
>   src/tests/containerizer/volume_host_path_isolator_tests.cpp 
> f692b87e1cc95944a626b9e7c6dfb4ea9ac2bbef 
>   src/tests/default_executor_tests.cpp 
> 68312010a45df5dbdb6d9d4c49d1faa5d8c60472 
>   src/tests/disk_quota_tests.cpp 742b6e1a6340d488f4bae8acbd09a3e575c72ad2 
>   src/tests/fault_tolerance_tests.cpp 
> c34850aaa76289d822317f30e5da0e0cb7f115c6 
>   src/tests/gc_tests.cpp 37d3eac5c8e241a423e3a2dcd9c0f7e8dfe05bc4 
>   src/tests/health_check_tests.cpp f4b50b1cb505084f64bf2dd279d9189ca65c8cdc 
>   src/tests/hook_tests.cpp c4fadbb6258aa1e87fbfa6a906d2e4f1f67880fa 
>   src/tests/master_tests.cpp 5d96457c86871b27c2fbe7f41a9444bbc2da6e06 
>   src/tests/master_validation_tests.cpp 
> f00dd9b6fcb963b995fa238766a531f073205ce9 
>   src/tests/oversubscription_tests.cpp 
> cd98b8f8eb301c41c991452ae36b0cfe767fb11f 
>   src/tests/partition_tests.cpp 0597bd2afaa60121245e0d43b81ac223257e377a 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 444737a6c1d251e23971d9a4501f3ef76fcf5ed5 
>   src/tests/persistent_volume_tests.cpp 
> 11fe43255cd8121f94c93a0437f7499fee1b6514 
>   src/tests/reconciliation_tests.cpp 64a1d3da9f03b863551555c4734aef39e835b122 
>   src/tests/reservation_endpoints_tests.cpp 
> e70dd0dd36def4fbb5b61519f8bb949c50afe36e 
>   src/tests/role_tests.cpp 568ea90427cfb870b77a3c1809d8be1715d2ca33 
>   src/tests/scheduler_tests.cpp 4eda96e50a505eb36588bbcf9c3ba76d9ede7839 
>   src/tests/slave_authorization_tests.cpp 
> 868e39ebac3b56374463b6b8278e93a49a2dc8cd 
>   src/tests/slave_recovery_tests.cpp 30d8c23de4312cc386354ef6cb44a62055c70f64 
>   src/tests/slave_tests.cpp 91d97d195acd695ae9c469651596511eafb50557 
>   src/tests/teardown_tests.cpp 5eada4f4392b7c721f5efbd537b0e817d40c7a27 
> 
> 
> Diff: https://reviews.apache.org/r/62213/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 62212: Send TASK_STARTING from the built-in executors.

2017-10-18 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62212/#review188554
---


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 12, 2017, 11:20 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62212/
> ---
> 
> (Updated Oct. 12, 2017, 11:20 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives schedulers more information about a tasks status,
> in particular it gives a better estimate of a tasks start time
> and helps differentiating between tasks stuck in TASK_STAGING
> and tasks stuck in TASK_STARTING.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 73743aba31f9d0ca827d318e2ecb4752a91b1be0 
>   src/docker/executor.cpp 3b0767ffbbe9982639d0b87fbb0573accdd48559 
>   src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 
>   src/launcher/executor.cpp 0131577d5b9018e7094815f1377b2ee59a4493d4 
> 
> 
> Diff: https://reviews.apache.org/r/62212/diff/6/
> 
> 
> Testing
> ---
> 
> (for the complete chain starting with this RR)
> - `make check`
> - Installed in local DC/OS cluster and successfully completed the tweeter 
> tutorial
> - Installed in local DC/OS cluster and successfully ran a periodic sleep task 
> using chronos.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 61855: Removed a stray trailing parenthesis from a validation error.

2017-10-18 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61855/#review188552
---


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 6, 2017, 10:29 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61855/
> ---
> 
> (Updated Oct. 6, 2017, 10:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed a stray trailing parenthesis from a validation error.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp 01bc2e0ad4de0bad570453cdaafb260c61c511eb 
> 
> 
> Diff: https://reviews.apache.org/r/61855/diff/2/
> 
> 
> Testing
> ---
> 
> None, but it passed all the commit hooks. YOLO =).
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>



Re: Review Request 63116: Mesos UI: fixed use of undefined variable for the pailer title.

2017-10-18 Thread Benjamin Mahler

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63116/#review188548
---


Ship it!




Ship It!

- Benjamin Mahler


On Oct. 18, 2017, 5 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63116/
> ---
> 
> (Updated Oct. 18, 2017, 5 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch fixes the use of an undefined variable to generate the
> pailer's window title in `pailer()`.
> 
> The title used for pailers that read agent files used to be: "path
> (host:port)", it is now "path (agentId)".
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> fa0ad54cbcec7a3b0bea9e2b0dc70a4c3f41e822 
> 
> 
> Diff: https://reviews.apache.org/r/63116/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>



Re: Review Request 63095: Added the Getting Started landing page.

2017-10-18 Thread Andrew Schwartzmeyer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63095/
---

(Updated Oct. 18, 2017, 11:46 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Remove extra JIRA link.


Repository: mesos


Description
---

After moving the build documentation to its own page, we can now have a
real "Getting Started" page suitable for anyone to get started with
Mesos. It is purposefully short, and therefore not overwhelming.


Diffs (updated)
-

  site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
  site/source/downloads.html.erb 3e24100a76a99b23ebc6589ee4f7332f43ef0ac9 
  site/source/getting-started.html.md PRE-CREATION 
  site/source/index.html.erb b4e847fadf6682503a4e09139491b539bc59e0fd 
  site/source/layouts/documentation.erb 
a91f916a5fb7348b2702c272e7a2059bdf628c66 
  site/source/layouts/getting_started_section.erb PRE-CREATION 
  site/source/layouts/layout.erb 9213c63e6fef3f57fc225a87a7d7e0abdcc11a88 


Diff: https://reviews.apache.org/r/63095/diff/2/

Changes: https://reviews.apache.org/r/63095/diff/1-2/


Testing
---

Built the site and tested it out locally.


Thanks,

Andrew Schwartzmeyer



Re: Review Request 63093: Moved building docs to `building.md`.

2017-10-18 Thread Andrew Schwartzmeyer


> On Oct. 18, 2017, 3 a.m., Vinod Kone wrote:
> > docs/getting-started.md
> > Line 241 (original), 241 (patched)
> > 
> >
> > Should we move this to a "Running Mesos" section/page? This is not 
> > technically "Building Mesos"

I thought about this, because you're right, the examples aren't technically 
"Building Mesos". But I posit that we keep them here, as the examples aren't 
really part of anything else. You pretty much use the examples to prove that 
your build worked. It even says:

> The framework binaries will only be available after running `make check`, as
described in the ***Building Mesos*** section above.

However, once we have a "Getting Familiar" page, or something similar, I think 
it would make sense to move examples into there (like as part of a tutorial 
beyond just building). What do you think?


- Andrew


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63093/#review188469
---


On Oct. 17, 2017, 7:58 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63093/
> ---
> 
> (Updated Oct. 17, 2017, 7:58 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing "Getting Started" documentation does not cover how to "get
> started" with Mesos, but instead how to build it from source on multiple
> platforms. Also added a link to `configuration.md` in the build
> documentation section too, as it was not obvious.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md da1471e6149d2f7b7313416ade0cd5b20daf8ba7 
>   docs/home.md 6a6bd736367505e00c27e5ecd37a68c8e01efe10 
> 
> 
> Diff: https://reviews.apache.org/r/63093/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63113: Fix flakyness in 'SlaveTest.ExecutorShutdownGracePeriod'.

2017-10-18 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63113/#review188545
---



Patch looks great!

Reviews applied: [63113]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Oct. 18, 2017, 3:49 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63113/
> ---
> 
> (Updated Oct. 18, 2017, 3:49 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On slow systems, it could happen that an additional offer was
> delivered to the scheduler before the test case finished.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 91d97d195acd695ae9c469651596511eafb50557 
> 
> 
> Diff: https://reviews.apache.org/r/63113/diff/1/
> 
> 
> Testing
> ---
> 
> `./mesos-tests --gtest_filter="*ExecutorShutdownGracePeriod*" 
> --gtest_repeat=5000 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 63094: Added resource sequence id for offer operations.

2017-10-18 Thread Greg Mann


> On Oct. 18, 2017, 9:46 a.m., Benjamin Bannier wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 56-66 (patched)
> > 
> >
> > I would prefer if we would not assume that this number changes for 
> > _every_ resource change, but just for failures.
> > * * *
> > 
> > For one we do not want users to assume that this number has certain 
> > values so we can recognize failures. Imagine a user applied an operation to 
> > the resource provider resources (`sequence_id=0`), and the agent speculated 
> > that the new `sequence_id` would be `1`. Now the operation fails in the RP 
> > and its `sequence_id` would be incremented to `1`. We cannot distinguish a 
> > case where the agent assumed the operation succeeded from a case where it 
> > failed based on the clock value alone in this case. Would we just increase 
> > this number in case of failures it would be clear that any operation 
> > assuming `sequence_id=0` would be invalid after the failure since the agent 
> > would be unlikely to assume failures on a "normal" execution path. 
> > 
> > * * *
> > 
> > Another reason would be that it would be great to be able to use 
> > similar notions of `sequence_id` for the agent's view of its RPs, and the 
> > master's view of its agents. We plan to implement speculative application 
> > of "legacy" offer operations like e.g., `RESERVE`, `CREATE` in the master, 
> > and incrementing this number even in successful cases would require too 
> > much knowledge about how the agent applies these operations in the master.
> > 
> > Imagine an agent with resource provider resources `disk(*):1` and 
> > `disk(*):1` (each `disk` from a different resource provider). The agent 
> > will expose `disk(*):2` to the master. A framework now reserves, 
> > `disk(*):2->disk(A):2`. The agent would inform the resource providers about 
> > that operation. Let's say the first of these operations succeeds, but the 
> > second on fails (the argument works as well for the other order, or even 
> > concurrent application). Having the master increment this 
> > `resource_sequence_id` for speculated operations would require it to know 
> > that the single `RESERVE` operation did map onto two state changes, i.e., 
> > it should have incremented its agent `resource_sequence_id` by two instead 
> > of just by one.
> > 
> > This seems not only leaky, but also inflexible since it becomes hard to 
> > decompose an agent's resources without affecting the master's assumptions.
> > 
> > * * *
> > 
> > Having `sequence_id` just increase in case of failures would eliminate 
> > a large class of cases where users would need to make assumption about how 
> > it does change. Instead we would more clearly move into a direction where 
> > it is only changed by pushes from lower layers (where an operation either 
> > succeeded or failed) to user layers above. This should also simplify how we 
> > would project multiple `sequence_id`s onto a single `sequence_id` (multiple 
> > resource provider `sequence_id`s onto agent `sequence_id`); in that case we 
> > could just take the maximum of all RP IDs and the agent ID (e.g., last 
> > failure of an operation on any RP is equivalent to last failure of any 
> > operation on the agent). Counting all operations makes this harder, 
> > especially when thinking of master-speculated operations.

This confused me a bit: "Having the master increment this resource_sequence_id 
for speculated operations"

IIUC, the master will only increment the sequence ID when it receives a 
{Register,Reregister,Update}SlaveMessage from the agent containing a new ID. 
So, I don't think the ID is incremented routinely during the successful 
application of offer operations.

My understanding of the comment in this patch is that the sequence ID will be 
incremented only when a legacy operation fails, or when the RP has its total 
resources updated (i.e., storage backend has more/less storage to offer and 
updates the CSI plugin/RP accordingly).


- Greg


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63094/#review188465
---


On Oct. 17, 2017, 11:12 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> ---
> 
> (Updated Oct. 17, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, 
> Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This sequence id is used to establish ordering between the operation
> and the 

Re: Review Request 62980: Added link anchors to all website headings.

2017-10-18 Thread James Peach


> On Oct. 18, 2017, 6:06 p.m., Andrew Schwartzmeyer wrote:
> > I tested this out locally on my Mac, rebased and with my getting started 
> > doc changes just to test it out. It's all working as expected. Though 
> > perhaps instead of black, might a medium gray look better?

I said `black` so that it would be the same as the title text color. I'm happy 
for it to be any specific color that works.


- James


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62980/#review188538
---


On Oct. 13, 2017, 4:58 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62980/
> ---
> 
> (Updated Oct. 13, 2017, 4:58 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This follows the example of the Middleman documentation and
> uses AnchorJS to inject link anchors to all headings at page
> load time. The styling is basic black to match the styling of
> the header titles.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 83596ddbd833e36b60bdbbd487ebd464b3874119 
>   site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 
> 
> 
> Diff: https://reviews.apache.org/r/62980/diff/2/
> 
> 
> Testing
> ---
> 
> Verified manually using the Docker image to run Middleman in dev mode.
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 62980: Added link anchors to all website headings.

2017-10-18 Thread Andrew Schwartzmeyer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62980/#review188538
---


Fix it, then Ship it!




I tested this out locally on my Mac, rebased and with my getting started doc 
changes just to test it out. It's all working as expected. Though perhaps 
instead of black, might a medium gray look better?


site/source/layouts/basic.erb
Lines 125 (patched)


s/do we/we do/ right?


- Andrew Schwartzmeyer


On Oct. 13, 2017, 9:58 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62980/
> ---
> 
> (Updated Oct. 13, 2017, 9:58 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This follows the example of the Middleman documentation and
> uses AnchorJS to inject link anchors to all headings at page
> load time. The styling is basic black to match the styling of
> the header titles.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 83596ddbd833e36b60bdbbd487ebd464b3874119 
>   site/source/layouts/basic.erb 3bf1f55d7d8feafd3caaa7902755404f7e45bae6 
> 
> 
> Diff: https://reviews.apache.org/r/62980/diff/2/
> 
> 
> Testing
> ---
> 
> Verified manually using the Docker image to run Middleman in dev mode.
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 63113: Fix flakyness in 'SlaveTest.ExecutorShutdownGracePeriod'.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63113/#review188540
---



PASS: Mesos patch 63113 was successfully built and tested.

Reviews applied: `['63113']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63113

- Mesos Reviewbot Windows


On Oct. 18, 2017, 3:49 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63113/
> ---
> 
> (Updated Oct. 18, 2017, 3:49 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On slow systems, it could happen that an additional offer was
> delivered to the scheduler before the test case finished.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 91d97d195acd695ae9c469651596511eafb50557 
> 
> 
> Diff: https://reviews.apache.org/r/63113/diff/1/
> 
> 
> Testing
> ---
> 
> `./mesos-tests --gtest_filter="*ExecutorShutdownGracePeriod*" 
> --gtest_repeat=5000 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 62891: Increased level of verbose logs for libprocess actor state transitions.

2017-10-18 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62891/#review188533
---


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 11, 2017, 1:57 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62891/
> ---
> 
> (Updated Oct. 11, 2017, 1:57 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-8074
> https://issues.apache.org/jira/browse/MESOS-8074
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Increases vlog level of some recurring events to improve readability.
> Specifically, we now log the following events on vlog level three
> instead of level two:
>   "Spawned process ",
>   "Resuming ",
>   "Cleaning up ",
>   "Donating thread to  while waiting".
> 
> This does not imply a general change or a holistic approach concerning
> the libprocess verbose logs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> a81ef747957db5bf388fac1a853c12fdd60d0cf7 
> 
> 
> Diff: https://reviews.apache.org/r/62891/diff/3/
> 
> 
> Testing
> ---
> 
> Run `GLOG_v=2 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 
> --port=5037 --work_dir=/tmp/test-mesos/work/` and `GLOG_v=3 ./mesos-local.sh 
> --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 
> --work_dir=/tmp/test-mesos/work/` and checked that the new output was as 
> expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62887: Increased level of some verbose logs from master and agent.

2017-10-18 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62887/#review188535
---


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 11, 2017, 1:29 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62887/
> ---
> 
> (Updated Oct. 11, 2017, 1:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-8072
> https://issues.apache.org/jira/browse/MESOS-8072
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Increases vlog level of some recurring events to improve readability.
> Specifically, we now log the following events on vlog level two
> instead of level one:
>   "Sending heartbeat",
>   "Received ping from ",
>   "Querying resource estimator for oversubscribable resources",
>   "Received oversubscribable resources".
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
>   src/slave/slave.cpp c35cf7d72c66bf7cdfd7efa322bd7d73c923fac9 
> 
> 
> Diff: https://reviews.apache.org/r/62887/diff/1/
> 
> 
> Testing
> ---
> 
> Run `GLOG_v=1 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 
> --port=5037 --work_dir=/tmp/test-mesos/work/` and `GLOG_v=2 ./mesos-local.sh 
> --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 
> --work_dir=/tmp/test-mesos/work/` and checked that the new output was as 
> expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62886: Increased level of some verbose logs from allocator.

2017-10-18 Thread Alexander Rukletsov

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62886/#review188534
---


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 11, 2017, 1:29 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62886/
> ---
> 
> (Updated Oct. 11, 2017, 1:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-8072
> https://issues.apache.org/jira/browse/MESOS-8072
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Increases vlog level of some recurring events to improve readability.
> Specifically, we now log the following events on vlog level two
> instead of level one:
>   "Skipped allocation because the allocator is paused",
>   "No allocations performed",
>   "No inverse offers to send out!".
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 445307411bc1df4b1468df0d8fb02181bc4287ed 
> 
> 
> Diff: https://reviews.apache.org/r/62886/diff/3/
> 
> 
> Testing
> ---
> 
> Run `GLOG_v=1 ./mesos-local.sh --log_dir=/tmp/test-mesos/log/ --num_slaves=3 
> --port=5037 --work_dir=/tmp/test-mesos/work/` and `GLOG_v=2 ./mesos-local.sh 
> --log_dir=/tmp/test-mesos/log/ --num_slaves=3 --port=5037 
> --work_dir=/tmp/test-mesos/work/` and checked that the new output was as 
> expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Review Request 63116: Mesos UI: fixed use of undefined variable for the pailer title.

2017-10-18 Thread Gaston Kleiman

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63116/
---

Review request for mesos, Alexander Rojas and Benjamin Mahler.


Repository: mesos


Description
---

This patch fixes the use of an undefined variable to generate the
pailer's window title in `pailer()`.

The title used for pailers that read agent files used to be: "path
(host:port)", it is now "path (agentId)".


Diffs
-

  src/webui/master/static/js/controllers.js 
fa0ad54cbcec7a3b0bea9e2b0dc70a4c3f41e822 


Diff: https://reviews.apache.org/r/63116/diff/1/


Testing
---


Thanks,

Gaston Kleiman



Re: Review Request 62213: Fix unit tests that were broken by the additional TASK_STARTING update.

2017-10-18 Thread Benno Evers

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62213/
---

(Updated Oct. 18, 2017, 4:24 p.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


Changes
---

Fix DynamicAddDelofCniConfig that was broken during rebase


Bugs: MESOS-7941
https://issues.apache.org/jira/browse/MESOS-7941


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/api_tests.cpp 0e99e7bf3a2cf4243cd73a5cb857bfc4d4e55f78 
  src/tests/check_tests.cpp fd15a47f1d67ad852d84d83aad54c9fa93c60bbb 
  src/tests/command_executor_tests.cpp f706f55b5bfab824268498d95d775b216561cd66 
  src/tests/container_logger_tests.cpp fb8e441bd3842b1840a384a025fa8e9ccbb22cf6 
  src/tests/containerizer/cgroups_isolator_tests.cpp 
3fc93417f2d3febf2feca3ec1c8476c9edcfbf4d 
  src/tests/containerizer/cni_isolator_tests.cpp 
e673d914fa9251fa585deea3a29438371c185fdb 
  src/tests/containerizer/cpu_isolator_tests.cpp 
153990d8960b1e74379e37b2e2ddf23f242a3712 
  src/tests/containerizer/docker_containerizer_tests.cpp 
45f0d1dbc5b21b174cd9974664299d466129df01 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
ed7d43840bf73fd4dd5c21f8d65e0a538338ee26 
  src/tests/containerizer/environment_secret_isolator_tests.cpp 
cf7b9eb6fd76dbdd3650ea2ad5b9097cf490e948 
  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
a657a6f60995330d14861e86df59b090ab7a015e 
  src/tests/containerizer/memory_isolator_tests.cpp 
14bd7050e147fabfae5fb32fe14d78a893a892c5 
  src/tests/containerizer/memory_pressure_tests.cpp 
c4d8bfc63d6c0953cc2b3524401767fdbdf0648d 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
f8b4423de8a468501336acc5ee0c67f181dc65f5 
  src/tests/containerizer/port_mapping_tests.cpp 
56e193bd84222648f47e36d6a1107046b2954977 
  src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
0030cd1e1f73422bef806bbc0453134e3d7840d8 
  src/tests/containerizer/provisioner_appc_tests.cpp 
89fe4fcd1b123edc762835595432f24cb699fd61 
  src/tests/containerizer/provisioner_docker_tests.cpp 
920be77b16178a4458d72145020c015130799ec4 
  src/tests/containerizer/runtime_isolator_tests.cpp 
ea5d035ee25ead1966db8b9e4772180dfb20747d 
  src/tests/containerizer/volume_host_path_isolator_tests.cpp 
f692b87e1cc95944a626b9e7c6dfb4ea9ac2bbef 
  src/tests/default_executor_tests.cpp 68312010a45df5dbdb6d9d4c49d1faa5d8c60472 
  src/tests/disk_quota_tests.cpp 742b6e1a6340d488f4bae8acbd09a3e575c72ad2 
  src/tests/fault_tolerance_tests.cpp c34850aaa76289d822317f30e5da0e0cb7f115c6 
  src/tests/gc_tests.cpp 37d3eac5c8e241a423e3a2dcd9c0f7e8dfe05bc4 
  src/tests/health_check_tests.cpp f4b50b1cb505084f64bf2dd279d9189ca65c8cdc 
  src/tests/hook_tests.cpp c4fadbb6258aa1e87fbfa6a906d2e4f1f67880fa 
  src/tests/master_tests.cpp 5d96457c86871b27c2fbe7f41a9444bbc2da6e06 
  src/tests/master_validation_tests.cpp 
f00dd9b6fcb963b995fa238766a531f073205ce9 
  src/tests/oversubscription_tests.cpp cd98b8f8eb301c41c991452ae36b0cfe767fb11f 
  src/tests/partition_tests.cpp 0597bd2afaa60121245e0d43b81ac223257e377a 
  src/tests/persistent_volume_endpoints_tests.cpp 
444737a6c1d251e23971d9a4501f3ef76fcf5ed5 
  src/tests/persistent_volume_tests.cpp 
11fe43255cd8121f94c93a0437f7499fee1b6514 
  src/tests/reconciliation_tests.cpp 64a1d3da9f03b863551555c4734aef39e835b122 
  src/tests/reservation_endpoints_tests.cpp 
e70dd0dd36def4fbb5b61519f8bb949c50afe36e 
  src/tests/role_tests.cpp 568ea90427cfb870b77a3c1809d8be1715d2ca33 
  src/tests/scheduler_tests.cpp 4eda96e50a505eb36588bbcf9c3ba76d9ede7839 
  src/tests/slave_authorization_tests.cpp 
868e39ebac3b56374463b6b8278e93a49a2dc8cd 
  src/tests/slave_recovery_tests.cpp 30d8c23de4312cc386354ef6cb44a62055c70f64 
  src/tests/slave_tests.cpp 91d97d195acd695ae9c469651596511eafb50557 
  src/tests/teardown_tests.cpp 5eada4f4392b7c721f5efbd537b0e817d40c7a27 


Diff: https://reviews.apache.org/r/62213/diff/8/

Changes: https://reviews.apache.org/r/62213/diff/7-8/


Testing
---


Thanks,

Benno Evers



Re: Review Request 63114: Updated jQuery and Angular versions used in Makefile.am.

2017-10-18 Thread Armand Grillet

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63114/
---

(Updated Oct. 18, 2017, 6:22 p.m.)


Review request for mesos and Alexander Rojas.


Changes
---

Added distcheck test which was what caused the problem on the first place.


Repository: mesos


Description
---

Following the updates of these two JavaScript libraries, the versions
in Makefile.am also need to be updated.


Diffs
-

  src/Makefile.am 085ff3b820693c9ef0115a74d72b0d87f158a887 


Diff: https://reviews.apache.org/r/63114/diff/1/


Testing (updated)
---

```sh
$ make check
$ DISTCHECK_CONFIGURE_FLAGS=" --disable-java" make distcheck
```


Thanks,

Armand Grillet



Review Request 63114: Updated jQuery and Angular versions used in Makefile.am.

2017-10-18 Thread Armand Grillet

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63114/
---

Review request for mesos and Alexander Rojas.


Repository: mesos


Description
---

Following the updates of these two JavaScript libraries, the versions
in Makefile.am also need to be updated.


Diffs
-

  src/Makefile.am 085ff3b820693c9ef0115a74d72b0d87f158a887 


Diff: https://reviews.apache.org/r/63114/diff/1/


Testing
---

```
$ make check
```


Thanks,

Armand Grillet



Re: Review Request 63107: Added operation feedback for storage operations.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/#review188506
---



FAIL: Failed to apply the dependent review: 62655.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62655`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63107

Relevant logs:

- 
[apply-review-62655-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63107/logs/apply-review-62655-stdout.log):

```
error: patch failed: src/master/master.cpp:6782
error: src/master/master.cpp: patch does not apply
error: patch failed: src/messages/messages.proto:610
error: src/messages/messages.proto: patch does not apply
error: patch failed: src/slave/slave.cpp:1264
error: src/slave/slave.cpp: patch does not apply
error: patch failed: src/tests/oversubscription_tests.cpp:323
error: src/tests/oversubscription_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 18, 2017, 5:39 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> ---
> 
> (Updated Oct. 18, 2017, 5:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8087
> https://issues.apache.org/jira/browse/MESOS-8087
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a framework accepts an offer that contains resource provider
> resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
> `CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
> be guessed and will only be known after the operation has been
> successfully applied by the resource provider.
> This patch introduces the necessary handling for such operations. The
> internal bookkeeping of available resources in the master and agent has
> been updated to update resources only after operation feedback has been
> received. This ensures that converted resources can only be offered
> after the operation was executed by a resource provider.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 
> 
> 
> Diff: https://reviews.apache.org/r/63107/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 63035: Made `getMountNamespaceTarget` more reliable by ignoring exited childs.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63035/#review188503
---



PASS: Mesos patch 63035 was successfully built and tested.

Reviews applied: `['63074', '63035']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63035

- Mesos Reviewbot Windows


On Oct. 17, 2017, 1:45 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63035/
> ---
> 
> (Updated Oct. 17, 2017, 1:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7504
> https://issues.apache.org/jira/browse/MESOS-7504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To launch a nested container it might be necessarry to enter parent's
> mnt namespace. `getMountNamespaceTarget()` is used for doing that.
> 
> Previously, `getMountNamespaceTarget` returned a failure when a
> parent's child or grandchild process exited after enumerating child
> processes and before getting mnt namespace for the process. Mesos
> containerizer launches pre-exec hooks that might exit when calling
> `getMountNamespaceTarget` during a subsequent launch of a
> nested container. This commit makes `getMountNamespaceTarget` tolerant
> to blinking child processes.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/utils.cpp 
> ec6d6c79b0f93cabe880cd697094c20e999af4d1 
> 
> 
> Diff: https://reviews.apache.org/r/63035/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> ./src/mesos-tests 
> --gtest_filter=NestedMesosContainerizerTest.ROOT_CGROUPS_DestroyDebugContainerOnRecover
>  --gtest_break_on_failure --gtest_repeat=100
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Review Request 63113: Fix flakyness in 'SlaveTest.ExecutorShutdownGracePeriod'.

2017-10-18 Thread Benno Evers

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63113/
---

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

On slow systems, it could happen that an additional offer was
delivered to the scheduler before the test case finished.


Diffs
-

  src/tests/slave_tests.cpp 91d97d195acd695ae9c469651596511eafb50557 


Diff: https://reviews.apache.org/r/63113/diff/1/


Testing
---

./mesos-tests --gtest_filter="*ExecutorShutdownGracePeriod*" 
--gtest_repeat=5000 --gtest_break_on_failure


Thanks,

Benno Evers



Re: Review Request 63035: Made `getMountNamespaceTarget` more reliable by ignoring exited childs.

2017-10-18 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63035/#review188497
---



Patch looks great!

Reviews applied: [63074, 63035]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Oct. 17, 2017, 1:45 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63035/
> ---
> 
> (Updated Oct. 17, 2017, 1:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7504
> https://issues.apache.org/jira/browse/MESOS-7504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To launch a nested container it might be necessarry to enter parent's
> mnt namespace. `getMountNamespaceTarget()` is used for doing that.
> 
> Previously, `getMountNamespaceTarget` returned a failure when a
> parent's child or grandchild process exited after enumerating child
> processes and before getting mnt namespace for the process. Mesos
> containerizer launches pre-exec hooks that might exit when calling
> `getMountNamespaceTarget` during a subsequent launch of a
> nested container. This commit makes `getMountNamespaceTarget` tolerant
> to blinking child processes.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/utils.cpp 
> ec6d6c79b0f93cabe880cd697094c20e999af4d1 
> 
> 
> Diff: https://reviews.apache.org/r/63035/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> ./src/mesos-tests 
> --gtest_filter=NestedMesosContainerizerTest.ROOT_CGROUPS_DestroyDebugContainerOnRecover
>  --gtest_break_on_failure --gtest_repeat=100
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 63106: Added a metric value for received offer operation updates.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63106/
---

(Updated Oct. 18, 2017, 4:39 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Bugs: MESOS-8087
https://issues.apache.org/jira/browse/MESOS-8087


Repository: mesos


Description
---

Added a metric value for received offer operation updates.


Diffs
-

  src/master/metrics.hpp f701efec0a82d6ba72b2414f739fcd1cd7ee2491 
  src/master/metrics.cpp 64fc829ac3b58d95fc0bd074571a46518a80bbba 


Diff: https://reviews.apache.org/r/63106/diff/1/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 63107: Added operation feedback for storage operations.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/
---

(Updated Oct. 18, 2017, 4:39 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Bugs: MESOS-8087
https://issues.apache.org/jira/browse/MESOS-8087


Repository: mesos


Description
---

When a framework accepts an offer that contains resource provider
resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
`CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
be guessed and will only be known after the operation has been
successfully applied by the resource provider.
This patch introduces the necessary handling for such operations. The
internal bookkeeping of available resources in the master and agent has
been updated to update resources only after operation feedback has been
received. This ensures that converted resources can only be offered
after the operation was executed by a resource provider.


Diffs
-

  src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
  src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 


Diff: https://reviews.apache.org/r/63107/diff/1/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 63105: Added 'apply' handlers for storage operations.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63105/
---

(Updated Oct. 18, 2017, 4:36 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Bugs: MESOS-7594
https://issues.apache.org/jira/browse/MESOS-7594


Repository: mesos


Description
---

Added 'apply' handlers for storage operations.


Diffs
-

  src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8 


Diff: https://reviews.apache.org/r/63105/diff/1/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 63104: Added a helper to extract resources from storage operations.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63104/
---

(Updated Oct. 18, 2017, 4:36 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Bugs: MESOS-7594
https://issues.apache.org/jira/browse/MESOS-7594


Repository: mesos


Description
---

Added a helper to extract resources from storage operations.


Diffs
-

  src/common/protobuf_utils.hpp c43ab75b5492320dfe19a7c723a72ac52b8ab722 
  src/common/protobuf_utils.cpp fd4858a64dfc136dd03cb1eef4c97d0f8d43bdae 


Diff: https://reviews.apache.org/r/63104/diff/1/


Testing
---

make check


Thanks,

Jan Schlicht



Review Request 63107: Added operation feedback for storage operations.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/
---

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

When a framework accepts an offer that contains resource provider
resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
`CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
be guessed and will only be known after the operation has been
successfully applied by the resource provider.
This patch introduces the necessary handling for such operations. The
internal bookkeeping of available resources in the master and agent has
been updated to update resources only after operation feedback has been
received. This ensures that converted resources can only be offered
after the operation was executed by a resource provider.


Diffs
-

  src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
  src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 


Diff: https://reviews.apache.org/r/63107/diff/1/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 63101: Upgrades jQuery used by Mesos WebUI to version 3.2.1.

2017-10-18 Thread Armand Grillet

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63101/#review188488
---


Ship it!




Ship It!

- Armand Grillet


On Oct. 18, 2017, 12:12 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63101/
> ---
> 
> (Updated Oct. 18, 2017, 12:12 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Gaston Kleiman, and Tomasz 
> Janiszewski.
> 
> 
> Bugs: MESOS-8057
> https://issues.apache.org/jira/browse/MESOS-8057
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The version of jQuery distributed with Mesos (1.7.1) was found to have
> security issues which have been addressed in latter versions.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 0b977f58fa1f348b582a0e0bcafcb7d170862cf4 
>   src/webui/master/static/js/jquery-1.7.1.js 
> 00c4e23a27b15f6158ea46a016beb0ae95c6596d 
>   src/webui/master/static/js/jquery-1.7.1.min.js 
> 198b3ff07d801dffa2c42fcf3b67eb3295eef85f 
>   src/webui/master/static/js/jquery-3.2.1.js PRE-CREATION 
>   src/webui/master/static/js/jquery-3.2.1.min.js PRE-CREATION 
>   src/webui/master/static/pailer.html 
> c8ada4b19b88c701b23926121dd6061573c615a4 
> 
> 
> Diff: https://reviews.apache.org/r/63101/diff/1/
> 
> 
> Testing
> ---
> 
> manual testing, verified with jQuery Update. Launched a cluster with two 
> agents and two frameworks. Verified that the UI behaved as before.
> 
> Not that this is the result of two updates:
> From 1.7.1 to 1.12.4 using the first jquery-upgrade. Once it was verified a 
> second upgrade from 1.12.4 to 3.2.1 was done with the help of the second 
> jquery-upgrade.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 63102: Upgrades AngularJS used by Mesos WebUI to version 1.2.32.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63102/#review188487
---



PASS: Mesos patch 63102 was successfully built and tested.

Reviews applied: `['63101', '63102']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63102

- Mesos Reviewbot Windows


On Oct. 18, 2017, 12:21 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63102/
> ---
> 
> (Updated Oct. 18, 2017, 12:21 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Gaston Kleiman, and Tomasz 
> Janiszewski.
> 
> 
> Bugs: MESOS-8057
> https://issues.apache.org/jira/browse/MESOS-8057
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The version of AngularJS distributed with Mesos (1.2.3) was found to
> have security issues which have been addressed in latter versions.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 0b977f58fa1f348b582a0e0bcafcb7d170862cf4 
>   src/webui/master/static/js/angular-1.2.3.js 
> 41afab54ba4403d4fc8c65f68fe0b2b5709800ca 
>   src/webui/master/static/js/angular-1.2.3.min.js 
> 3f93ab77219374de24146f30e17bad4c02f1a93d 
>   src/webui/master/static/js/angular-1.2.32.min.js PRE-CREATION 
>   src/webui/master/static/js/angular-route-1.2.3.js 
> 532d1e7fe8610384f8548956a3d786f8407aaff2 
>   src/webui/master/static/js/angular-route-1.2.3.min.js 
> 5870a6a82472bce06388d16f88ffa11fd2be2e81 
>   src/webui/master/static/js/angular-route-1.2.32.min.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63102/diff/1/
> 
> 
> Testing
> ---
> 
> manual testing Launched a cluster with two agents and two frameworks. 
> Verified that the UI behaved as before.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Review Request 63106: Added a metric value for received offer operation updates.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63106/
---

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Added a metric value for received offer operation updates.


Diffs
-

  src/master/metrics.hpp f701efec0a82d6ba72b2414f739fcd1cd7ee2491 
  src/master/metrics.cpp 64fc829ac3b58d95fc0bd074571a46518a80bbba 


Diff: https://reviews.apache.org/r/63106/diff/1/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 61947: Implemented handling of resource provider offer operations.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61947/
---

(Updated Oct. 18, 2017, 3:54 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Rebased on top of #63001.


Bugs: MESOS-7594
https://issues.apache.org/jira/browse/MESOS-7594


Repository: mesos


Description
---

While the resource provider manager is responsible to apply offer
operations by sending events to the respective resource providers,
the master takes care of accepting these operations. Hence, for local
resource providers the master sends an `ApplyResourceOperationMessage`
to the agent where the resource provider is running on. The agent
then relays the operation contained in the message to the resource
provider manager.


Diffs (updated)
-

  src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 


Diff: https://reviews.apache.org/r/61947/diff/3/

Changes: https://reviews.apache.org/r/61947/diff/2-3/


Testing
---

make check


Thanks,

Jan Schlicht



Review Request 63105: Added 'apply' handlers for storage operations.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63105/
---

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Added 'apply' handlers for storage operations.


Diffs
-

  src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8 


Diff: https://reviews.apache.org/r/63105/diff/1/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 61946: Added validation of resource provider operations.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61946/
---

(Updated Oct. 18, 2017, 3:51 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Renamed variables.


Bugs: MESOS-7594
https://issues.apache.org/jira/browse/MESOS-7594


Repository: mesos


Description
---

Added validation of resource provider operations.


Diffs (updated)
-

  src/master/validation.hpp f4925752f20ae8ca4de1d9b4a3d5ffc394db9585 
  src/master/validation.cpp 01bc2e0ad4de0bad570453cdaafb260c61c511eb 


Diff: https://reviews.apache.org/r/61946/diff/2/

Changes: https://reviews.apache.org/r/61946/diff/1-2/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 61810: Added a function to apply offer operations.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61810/
---

(Updated Oct. 18, 2017, 3:50 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Rebased on top of #63001.


Bugs: MESOS-7594
https://issues.apache.org/jira/browse/MESOS-7594


Repository: mesos


Description
---

The resource provider manager provides an 'apply' function for offer
operation affecting resource providers. The resource on which the
operation should be applied contains a resource provider ID. This will
be extracted and an event will be sent to the respective resource
provider.


Diffs (updated)
-

  src/resource_provider/manager.hpp 3b70e75c6b6721864ae0ee9c4a593b5035d8388f 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/tests/resource_provider_manager_tests.cpp 
ca49e1f0203494fc8b4a4507c33e5a3885a14a59 


Diff: https://reviews.apache.org/r/61810/diff/2/

Changes: https://reviews.apache.org/r/61810/diff/1-2/


Testing
---

make check


Thanks,

Jan Schlicht



Review Request 63104: Added a helper to extract resources from storage operations.

2017-10-18 Thread Jan Schlicht

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63104/
---

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Added a helper to extract resources from storage operations.


Diffs
-

  src/common/protobuf_utils.hpp c43ab75b5492320dfe19a7c723a72ac52b8ab722 
  src/common/protobuf_utils.cpp fd4858a64dfc136dd03cb1eef4c97d0f8d43bdae 


Diff: https://reviews.apache.org/r/63104/diff/1/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 63074: Changed return type of `ns::getns()` from `Try` to `Result`.

2017-10-18 Thread Andrei Budnik


> On Oct. 17, 2017, 9:11 p.m., Andrei Budnik wrote:
> > src/linux/ns.hpp
> > Lines 246 (patched)
> > 
> >
> > There might be a race when `stat` has failed, but a new process with 
> > the same PID is launched before `os::exist` is called. This race reproduced 
> > when two pre-exec hooks was started.

Fixed.


- Andrei


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63074/#review188395
---


On Oct. 17, 2017, 1:44 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63074/
> ---
> 
> (Updated Oct. 17, 2017, 1:44 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7504
> https://issues.apache.org/jira/browse/MESOS-7504
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, `getns` checked existence of the process's pid before
> trying to `stat` the namespace file. If the pid didn't exist, then
> it returned a failure. However, the process might exit before `stat`
> is called.
> 
> Now, `getns` doesn't check existence of the process's pid explicitly.
> The fact that the process is gone can be detected by checking returned
> errno of `stat`: if it returns ENOENT, then the function returns
> `None()`.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp e96116343b132ff4aae36c7b3c1b0e99c41246af 
>   src/slave/containerizer/mesos/utils.cpp 
> ec6d6c79b0f93cabe880cd697094c20e999af4d1 
>   src/tests/containerizer/isolator_tests.cpp 
> 5072bafabc0e37bc756588b88e4d9f8a8c84d337 
>   src/tests/containerizer/ns_tests.cpp 
> 2e28139d241e5e37c8fd0f8b4531c3d271727c8d 
> 
> 
> Diff: https://reviews.apache.org/r/63074/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 63095: Added the Getting Started landing page.

2017-10-18 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63095/#review188478
---



Bad patch!

Reviews applied: [63095, 63093, 62548]

Failed command: python support/apply-reviews.py -n -r 62548

Error:
2017-10-18 12:20:25 URL:https://reviews.apache.org/r/62548/diff/raw/ 
[29054/29054] -> "62548.patch" [1]
error: patch failed: docs/home.md:96
error: docs/home.md: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19831/console

- Mesos Reviewbot


On Oct. 18, 2017, 2:58 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63095/
> ---
> 
> (Updated Oct. 18, 2017, 2:58 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After moving the build documentation to its own page, we can now have a
> real "Getting Started" page suitable for anyone to get started with
> Mesos. It is purposefully short, and therefore not overwhelming.
> 
> 
> Diffs
> -
> 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/source/downloads.html.erb 3e24100a76a99b23ebc6589ee4f7332f43ef0ac9 
>   site/source/getting-started.html.md PRE-CREATION 
>   site/source/index.html.erb b4e847fadf6682503a4e09139491b539bc59e0fd 
>   site/source/layouts/documentation.erb 
> a91f916a5fb7348b2702c272e7a2059bdf628c66 
>   site/source/layouts/getting_started_section.erb PRE-CREATION 
>   site/source/layouts/layout.erb 9213c63e6fef3f57fc225a87a7d7e0abdcc11a88 
> 
> 
> Diff: https://reviews.apache.org/r/63095/diff/1/
> 
> 
> Testing
> ---
> 
> Built the site and tested it out locally.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63101: Upgrades jQuery used by Mesos WebUI to version 3.2.1.

2017-10-18 Thread Alexander Rojas

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63101/
---

(Updated Oct. 18, 2017, 2:12 p.m.)


Review request for mesos, Armand Grillet, Gaston Kleiman, and Tomasz 
Janiszewski.


Bugs: MESOS-8057
https://issues.apache.org/jira/browse/MESOS-8057


Repository: mesos


Description
---

The version of jQuery distributed with Mesos (1.7.1) was found to have
security issues which have been addressed in latter versions.


Diffs
-

  src/webui/master/static/index.html 0b977f58fa1f348b582a0e0bcafcb7d170862cf4 
  src/webui/master/static/js/jquery-1.7.1.js 
00c4e23a27b15f6158ea46a016beb0ae95c6596d 
  src/webui/master/static/js/jquery-1.7.1.min.js 
198b3ff07d801dffa2c42fcf3b67eb3295eef85f 
  src/webui/master/static/js/jquery-3.2.1.js PRE-CREATION 
  src/webui/master/static/js/jquery-3.2.1.min.js PRE-CREATION 
  src/webui/master/static/pailer.html c8ada4b19b88c701b23926121dd6061573c615a4 


Diff: https://reviews.apache.org/r/63101/diff/1/


Testing (updated)
---

manual testing, verified with jQuery Update. Launched a cluster with two agents 
and two frameworks. Verified that the UI behaved as before.

Not that this is the result of two updates:
>From 1.7.1 to 1.12.4 using the first jquery-upgrade. Once it was verified a 
>second upgrade from 1.12.4 to 3.2.1 was done with the help of the second 
>jquery-upgrade.


Thanks,

Alexander Rojas



Re: Review Request 62214: Added JavaScript linter.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62214/#review188477
---



PASS: Mesos patch 62214 was successfully built and tested.

Reviews applied: `['62214']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62214

- Mesos Reviewbot Windows


On Oct. 18, 2017, 11:23 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Oct. 18, 2017, 11:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The linter runs when changes on a JavaScript file are being committed.
> We use ESLint with a configuration based on our current JS code base.
> The linter and its dependencies (i.e. Node.js) are installed in a
> virtual environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/7/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> Following this commit, I have also tried to commit a change on a JavaScript 
> file and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62502: Added events to publish and unpublish resources.

2017-10-18 Thread Benjamin Bannier


> On Oct. 17, 2017, 10:48 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 38 (patched)
> > 
> >
> > No need for `UNPUBLISH` for now. Let's introduce it once we actually 
> > need it.
> 
> Chun-Hung Hsiao wrote:
> We may still want `UNPUBLISH`, depending on what we want to implement in 
> the following case.
> A storage local resource provider needs to call 
> `csi::Node::NodeUnpublishVolume` before `csi::Controller::DeleteVolume`.
> We can either:
> 1. Ask the storage local resource provider to checkpoint what's being 
> published, and do the proper unpublish call before deleting. This requires no 
> `UNPUBLISH`.
> 2. Ask the resource provider manager to checkpoint what's being 
> published, and it is responsible to issue an `UNPUBLISH` before sending out 
> the `DestroyVolume` offer operation.
> Which one sounds better? Also if we consider general resource providers, 
> should the manager or the provider checkpoint the published resources?
> 
> Chun-Hung Hsiao wrote:
> Now I think since the RP need to remember what is published, because we 
> don't do `UBPUBLISH` for local resources for now, I'll just make the RP do 
> it's own unpublish.
> 
> Chun-Hung Hsiao wrote:
> If we want to keep the concept of "resource usage" and that of "resource 
> publish" decoupled and maintain the same "ensure total" semantics in the 
> future, we probably need `PUBLISH_AT_LEAST` and `PUBLISH_NO_MORE_THAN` 
> instead of `PUBLISH`/`UNPUBLISH`. These operations provide flexibility for us 
> to decide how to do reaource unpublishing (eager/periodic GC/lazy) later. Are 
> there better names for these operations?

I am not sure decoupling these on this level is useful, and for simplicity I'd 
much prefer callers to only have to use a single pair of messages to signal 
that resources are needed or not needed anymore. Currently it looks like these 
messages would be used to inform the resource provider about tasks which are 
being launched and have ended, and we don't currently have or plan to add knobs 
to parameterize this behavior along the axes you outline. 

Not sure what scenarios you have in mind for `PUBLISH_NO_MORE_THAN` (better: 
use case for isolation? not sure), but as a user I would already expect 
operations on the plugin to be idempotent enough so that `PUBLISH_AT_LEAST` 
wouldn't be needed. I also not sure callers at this level would want to 
parameterize unpublishing like you hinted; this instead sounds like a use case 
for plugin configuration to me.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62502/#review188386
---


On Oct. 13, 2017, 2:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> ---
> 
> (Updated Oct. 13, 2017, 2:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
> https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 62214: Added JavaScript linter.

2017-10-18 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62214/#review188474
---



Patch looks great!

Reviews applied: [62214]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Oct. 18, 2017, 9:23 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Oct. 18, 2017, 9:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The linter runs when changes on a JavaScript file are being committed.
> We use ESLint with a configuration based on our current JS code base.
> The linter and its dependencies (i.e. Node.js) are installed in a
> virtual environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/7/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> Following this commit, I have also tried to commit a change on a JavaScript 
> file and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 63102: Upgrades AngularJS used by Mesos WebUI to version 1.2.32.

2017-10-18 Thread Armand Grillet

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63102/#review188473
---


Ship it!




Ship It!

- Armand Grillet


On Oct. 18, 2017, 10:21 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63102/
> ---
> 
> (Updated Oct. 18, 2017, 10:21 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Gaston Kleiman, and Tomasz 
> Janiszewski.
> 
> 
> Bugs: MESOS-8057
> https://issues.apache.org/jira/browse/MESOS-8057
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The version of AngularJS distributed with Mesos (1.2.3) was found to
> have security issues which have been addressed in latter versions.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 0b977f58fa1f348b582a0e0bcafcb7d170862cf4 
>   src/webui/master/static/js/angular-1.2.3.js 
> 41afab54ba4403d4fc8c65f68fe0b2b5709800ca 
>   src/webui/master/static/js/angular-1.2.3.min.js 
> 3f93ab77219374de24146f30e17bad4c02f1a93d 
>   src/webui/master/static/js/angular-1.2.32.min.js PRE-CREATION 
>   src/webui/master/static/js/angular-route-1.2.3.js 
> 532d1e7fe8610384f8548956a3d786f8407aaff2 
>   src/webui/master/static/js/angular-route-1.2.3.min.js 
> 5870a6a82472bce06388d16f88ffa11fd2be2e81 
>   src/webui/master/static/js/angular-route-1.2.32.min.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63102/diff/1/
> 
> 
> Testing
> ---
> 
> manual testing Launched a cluster with two agents and two frameworks. 
> Verified that the UI behaved as before.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 63101: Upgrades jQuery used by Mesos WebUI to version 3.2.1.

2017-10-18 Thread Tomasz Janiszewski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63101/#review188472
---


Ship it!




Ship It!

- Tomasz Janiszewski


On Oct. 18, 2017, 10:16 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63101/
> ---
> 
> (Updated Oct. 18, 2017, 10:16 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Gaston Kleiman, and Tomasz 
> Janiszewski.
> 
> 
> Bugs: MESOS-8057
> https://issues.apache.org/jira/browse/MESOS-8057
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The version of jQuery distributed with Mesos (1.7.1) was found to have
> security issues which have been addressed in latter versions.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 0b977f58fa1f348b582a0e0bcafcb7d170862cf4 
>   src/webui/master/static/js/jquery-1.7.1.js 
> 00c4e23a27b15f6158ea46a016beb0ae95c6596d 
>   src/webui/master/static/js/jquery-1.7.1.min.js 
> 198b3ff07d801dffa2c42fcf3b67eb3295eef85f 
>   src/webui/master/static/js/jquery-3.2.1.js PRE-CREATION 
>   src/webui/master/static/js/jquery-3.2.1.min.js PRE-CREATION 
>   src/webui/master/static/pailer.html 
> c8ada4b19b88c701b23926121dd6061573c615a4 
> 
> 
> Diff: https://reviews.apache.org/r/63101/diff/1/
> 
> 
> Testing
> ---
> 
> manual testing, verified with jQuery Update. Launched a cluster with two 
> agents and two frameworks. Verified that the UI behaved as before.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 63102: Upgrades AngularJS used by Mesos WebUI to version 1.2.32.

2017-10-18 Thread Tomasz Janiszewski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63102/#review188471
---


Ship it!




Ship It!

- Tomasz Janiszewski


On Oct. 18, 2017, 10:21 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63102/
> ---
> 
> (Updated Oct. 18, 2017, 10:21 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Gaston Kleiman, and Tomasz 
> Janiszewski.
> 
> 
> Bugs: MESOS-8057
> https://issues.apache.org/jira/browse/MESOS-8057
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The version of AngularJS distributed with Mesos (1.2.3) was found to
> have security issues which have been addressed in latter versions.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 0b977f58fa1f348b582a0e0bcafcb7d170862cf4 
>   src/webui/master/static/js/angular-1.2.3.js 
> 41afab54ba4403d4fc8c65f68fe0b2b5709800ca 
>   src/webui/master/static/js/angular-1.2.3.min.js 
> 3f93ab77219374de24146f30e17bad4c02f1a93d 
>   src/webui/master/static/js/angular-1.2.32.min.js PRE-CREATION 
>   src/webui/master/static/js/angular-route-1.2.3.js 
> 532d1e7fe8610384f8548956a3d786f8407aaff2 
>   src/webui/master/static/js/angular-route-1.2.3.min.js 
> 5870a6a82472bce06388d16f88ffa11fd2be2e81 
>   src/webui/master/static/js/angular-route-1.2.32.min.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63102/diff/1/
> 
> 
> Testing
> ---
> 
> manual testing Launched a cluster with two agents and two frameworks. 
> Verified that the UI behaved as before.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Review Request 63102: Upgrades AngularJS used by Mesos WebUI to version 1.2.32.

2017-10-18 Thread Alexander Rojas

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63102/
---

Review request for mesos, Armand Grillet, Gaston Kleiman, and Tomasz 
Janiszewski.


Bugs: MESOS-8057
https://issues.apache.org/jira/browse/MESOS-8057


Repository: mesos


Description
---

The version of AngularJS distributed with Mesos (1.2.3) was found to
have security issues which have been addressed in latter versions.


Diffs
-

  src/webui/master/static/index.html 0b977f58fa1f348b582a0e0bcafcb7d170862cf4 
  src/webui/master/static/js/angular-1.2.3.js 
41afab54ba4403d4fc8c65f68fe0b2b5709800ca 
  src/webui/master/static/js/angular-1.2.3.min.js 
3f93ab77219374de24146f30e17bad4c02f1a93d 
  src/webui/master/static/js/angular-1.2.32.min.js PRE-CREATION 
  src/webui/master/static/js/angular-route-1.2.3.js 
532d1e7fe8610384f8548956a3d786f8407aaff2 
  src/webui/master/static/js/angular-route-1.2.3.min.js 
5870a6a82472bce06388d16f88ffa11fd2be2e81 
  src/webui/master/static/js/angular-route-1.2.32.min.js PRE-CREATION 


Diff: https://reviews.apache.org/r/63102/diff/1/


Testing
---

manual testing Launched a cluster with two agents and two frameworks. Verified 
that the UI behaved as before.


Thanks,

Alexander Rojas



Review Request 63101: Upgrades jQuery used by Mesos WebUI to version 3.2.1.

2017-10-18 Thread Alexander Rojas

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63101/
---

Review request for mesos, Armand Grillet, Gaston Kleiman, and Tomasz 
Janiszewski.


Bugs: MESOS-8057
https://issues.apache.org/jira/browse/MESOS-8057


Repository: mesos


Description
---

The version of jQuery distributed with Mesos (1.7.1) was found to have
security issues which have been addressed in latter versions.


Diffs
-

  src/webui/master/static/index.html 0b977f58fa1f348b582a0e0bcafcb7d170862cf4 
  src/webui/master/static/js/jquery-1.7.1.js 
00c4e23a27b15f6158ea46a016beb0ae95c6596d 
  src/webui/master/static/js/jquery-1.7.1.min.js 
198b3ff07d801dffa2c42fcf3b67eb3295eef85f 
  src/webui/master/static/js/jquery-3.2.1.js PRE-CREATION 
  src/webui/master/static/js/jquery-3.2.1.min.js PRE-CREATION 
  src/webui/master/static/pailer.html c8ada4b19b88c701b23926121dd6061573c615a4 


Diff: https://reviews.apache.org/r/63101/diff/1/


Testing
---

manual testing, verified with jQuery Update. Launched a cluster with two agents 
and two frameworks. Verified that the UI behaved as before.


Thanks,

Alexander Rojas



Re: Review Request 63093: Moved building docs to `building.md`.

2017-10-18 Thread Vinod Kone

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63093/#review188469
---




docs/getting-started.md
Line 241 (original), 241 (patched)


Should we move this to a "Running Mesos" section/page? This is not 
technically "Building Mesos"


- Vinod Kone


On Oct. 18, 2017, 2:58 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63093/
> ---
> 
> (Updated Oct. 18, 2017, 2:58 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing "Getting Started" documentation does not cover how to "get
> started" with Mesos, but instead how to build it from source on multiple
> platforms. Also added a link to `configuration.md` in the build
> documentation section too, as it was not obvious.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md da1471e6149d2f7b7313416ade0cd5b20daf8ba7 
>   docs/home.md 6a6bd736367505e00c27e5ecd37a68c8e01efe10 
> 
> 
> Diff: https://reviews.apache.org/r/63093/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63095: Added the Getting Started landing page.

2017-10-18 Thread Vinod Kone

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63095/#review188468
---


Fix it, then Ship it!





site/source/getting-started.html.md
Lines 19 (patched)


i would not link JIRA here for consistency.


- Vinod Kone


On Oct. 18, 2017, 2:58 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63095/
> ---
> 
> (Updated Oct. 18, 2017, 2:58 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After moving the build documentation to its own page, we can now have a
> real "Getting Started" page suitable for anyone to get started with
> Mesos. It is purposefully short, and therefore not overwhelming.
> 
> 
> Diffs
> -
> 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/source/downloads.html.erb 3e24100a76a99b23ebc6589ee4f7332f43ef0ac9 
>   site/source/getting-started.html.md PRE-CREATION 
>   site/source/index.html.erb b4e847fadf6682503a4e09139491b539bc59e0fd 
>   site/source/layouts/documentation.erb 
> a91f916a5fb7348b2702c272e7a2059bdf628c66 
>   site/source/layouts/getting_started_section.erb PRE-CREATION 
>   site/source/layouts/layout.erb 9213c63e6fef3f57fc225a87a7d7e0abdcc11a88 
> 
> 
> Diff: https://reviews.apache.org/r/63095/diff/1/
> 
> 
> Testing
> ---
> 
> Built the site and tested it out locally.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63094: Added resource sequence id for offer operations.

2017-10-18 Thread Benjamin Bannier

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63094/#review188465
---




include/mesos/resource_provider/resource_provider.proto
Lines 56-66 (patched)


I would prefer if we would not assume that this number changes for _every_ 
resource change, but just for failures.
* * *

For one we do not want users to assume that this number has certain values 
so we can recognize failures. Imagine a user applied an operation to the 
resource provider resources (`sequence_id=0`), and the agent speculated that 
the new `sequence_id` would be `1`. Now the operation fails in the RP and its 
`sequence_id` would be incremented to `1`. We cannot distinguish a case where 
the agent assumed the operation succeeded from a case where it failed based on 
the clock value alone in this case. Would we just increase this number in case 
of failures it would be clear that any operation assuming `sequence_id=0` would 
be invalid after the failure since the agent would be unlikely to assume 
failures on a "normal" execution path. 

* * *

Another reason would be that it would be great to be able to use similar 
notions of `sequence_id` for the agent's view of its RPs, and the master's view 
of its agents. We plan to implement speculative application of "legacy" offer 
operations like e.g., `RESERVE`, `CREATE` in the master, and incrementing this 
number even in successful cases would require too much knowledge about how the 
agent applies these operations in the master.

Imagine an agent with resource provider resources `disk(*):1` and 
`disk(*):1` (each `disk` from a different resource provider). The agent will 
expose `disk(*):2` to the master. A framework now reserves, 
`disk(*):2->disk(A):2`. The agent would inform the resource providers about 
that operation. Let's say the first of these operations succeeds, but the 
second on fails (the argument works as well for the other order, or even 
concurrent application). Having the master increment this 
`resource_sequence_id` for speculated operations would require it to know that 
the single `RESERVE` operation did map onto two state changes, i.e., it should 
have incremented its agent `resource_sequence_id` by two instead of just by one.

This seems not only leaky, but also inflexible since it becomes hard to 
decompose an agent's resources without affecting the master's assumptions.

* * *

Having `sequence_id` just increase in case of failures would eliminate a 
large class of cases where users would need to make assumption about how it 
does change. Instead we would more clearly move into a direction where it is 
only changed by pushes from lower layers (where an operation either succeeded 
or failed) to user layers above. This should also simplify how we would project 
multiple `sequence_id`s onto a single `sequence_id` (multiple resource provider 
`sequence_id`s onto agent `sequence_id`); in that case we could just take the 
maximum of all RP IDs and the agent ID (e.g., last failure of an operation on 
any RP is equivalent to last failure of any operation on the agent). Counting 
all operations makes this harder, especially when thinking of master-speculated 
operations.


- Benjamin Bannier


On Oct. 18, 2017, 1:12 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> ---
> 
> (Updated Oct. 18, 2017, 1:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, 
> Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This sequence id is used to establish ordering between the operation
> and the resources that the operation is operating on.  Each agent (or
> external resource provider) will keep a sequence id, and increments it
> when the resources on the agent (or the external resource provider)
> has changed. The master will keep track of the last known agent (or
> external resource provider) sequence id, and attach this sequence id
> in each operation it sends out. The agent (or the external resource
> provider) should reject operations that have smaller sequence id than
> its own sequence id, because this means the operation is operating on
> resources that might have already been invalidated.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
> 
> 
> 

Re: Review Request 62781: Mesos UI: extract the agent URL generation to a function.

2017-10-18 Thread Alexander Rojas

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62781/#review188467
---




src/webui/master/static/js/controllers.js
Line 50 (original), 86 (patched)


This patch introduced an issue where `host` is no longer defined when used 
in this line.


- Alexander Rojas


On Oct. 6, 2017, 11:05 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62781/
> ---
> 
> (Updated Oct. 6, 2017, 11:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This path extracts the generation of URLs of agent endpoints to a single
> function.
> 
> Having this logic in just one place makes it easier to modify it, e.g.,
> to make it possible to use the UI via a reverse proxy.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> c586b9050e14697ca89fabfe636c2d18a4ecf6f5 
>   src/webui/master/static/js/controllers.js 
> cb1f093ea21864eadd0ae32f3c33a7ff88b6fc27 
>   src/webui/master/static/js/services.js 
> b99ad685e9614ef77586efb5fa52345fc173e51e 
> 
> 
> Diff: https://reviews.apache.org/r/62781/diff/5/
> 
> 
> Testing
> ---
> 
> Verified that the UI still works when running a master and an agent.
> 
> I tried:
> 
>   * Clicking all the tabs.
>   * Viewing the master/agent logs.
>   * Browsing a task sandbox and reading a file using the pailer.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>



Re: Review Request 62214: Added JavaScript linter.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62214/#review188466
---



FAIL: Failed to apply the dependent review: 62333.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62333`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62214

Relevant logs:

- 
[apply-review-62333-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62214/logs/apply-review-62333-stdout.log):

```
error: patch failed: support/mesos-style.py:153
error: support/mesos-style.py: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 18, 2017, 9:23 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Oct. 18, 2017, 9:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The linter runs when changes on a JavaScript file are being committed.
> We use ESLint with a configuration based on our current JS code base.
> The linter and its dependencies (i.e. Node.js) are installed in a
> virtual environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/7/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> Following this commit, I have also tried to commit a change on a JavaScript 
> file and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 62214: Added JavaScript linter.

2017-10-18 Thread Armand Grillet

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62214/
---

(Updated Oct. 18, 2017, 9:23 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, and Kevin Klues.


Changes
---

Rebase.


Bugs: MESOS-7924
https://issues.apache.org/jira/browse/MESOS-7924


Repository: mesos


Description (updated)
---

The linter runs when changes on a JavaScript file are being committed.
We use ESLint with a configuration based on our current JS code base.
The linter and its dependencies (i.e. Node.js) are installed in a
virtual environment using Virtualenv and then Nodeenv.


Diffs (updated)
-

  src/webui/.eslintrc.js PRE-CREATION 
  src/webui/.gitignore PRE-CREATION 
  src/webui/bootstrap PRE-CREATION 
  src/webui/pip-requirements.txt PRE-CREATION 
  support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 


Diff: https://reviews.apache.org/r/62214/diff/7/

Changes: https://reviews.apache.org/r/62214/diff/6-7/


Testing (updated)
---

```
$ make check
```

Following this commit, I have also tried to commit a change on a JavaScript 
file and checked that ESLinter was correctly running.


Thanks,

Armand Grillet



Re: Review Request 62502: Added events to publish and unpublish resources.

2017-10-18 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62502/#review188464
---



Bad patch!

Reviews applied: [62502, 61947, 61946, 61810, 58021, 58047, 58048, 62282]

Failed command: python support/apply-reviews.py -n -r 61947

Error:
2017-10-18 08:59:42 URL:https://reviews.apache.org/r/61947/diff/raw/ 
[8417/8417] -> "61947.patch" [1]
error: patch failed: src/tests/resource_provider_manager_tests.cpp:339
error: src/tests/resource_provider_manager_tests.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19829/console

- Mesos Reviewbot


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> ---
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
> https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 62003: Added `network/ports` isolator nested container tests.

2017-10-18 Thread Qian Zhang

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62003/#review188461
---


Fix it, then Ship it!





src/tests/containerizer/ports_isolator_tests.cpp
Lines 1083-1090 (patched)


I think these code can be changed to:
```
  v1::ExecutorInfo executorInfo = v1::createExecutorInfo(
  v1::DEFAULT_EXECUTOR_ID,
  None(),
  "cpus:0.1;mem:32;disk:32",
  v1::ExecutorInfo::DEFAULT,
  frameworkId);
```



src/tests/containerizer/ports_isolator_tests.cpp
Lines 1148 (patched)


Kill this blank line.



src/tests/containerizer/ports_isolator_tests.cpp
Lines 1190-1199 (patched)


It seems we only do these checks for nested container tests, we should do 
it for the normal container tests in your previous patches as well?



src/tests/containerizer/ports_isolator_tests.cpp
Lines 1225 (patched)


Why do we need the `slaveId`? Can we just call `StartSlave()` like what you 
does in the test `ROOT_NC_TaskGroup`.



src/tests/containerizer/ports_isolator_tests.cpp
Lines 1262-1269 (patched)


Ditto.



src/tests/containerizer/ports_isolator_tests.cpp
Lines 1400 (patched)


s/after and agent restart/after an agent restart/



src/tests/containerizer/ports_isolator_tests.cpp
Lines 1421 (patched)


Ditto.



src/tests/containerizer/ports_isolator_tests.cpp
Lines 1458-1465 (patched)


Ditto.


- Qian Zhang


On Oct. 18, 2017, 3:15 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62003/
> ---
> 
> (Updated Oct. 18, 2017, 3:15 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `network/ports` isolator nested container tests using the v1
> TaskGroups API. This tests that rogue port usage by a nested task is
> detected both with and without agent recovery, and that a well-behaved
> task is preserved across agent recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62003/diff/6/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 63095: Added the Getting Started landing page.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63095/#review188460
---



FAIL: Failed to apply the dependent review: 62548.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62548`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63095

Relevant logs:

- 
[apply-review-62548-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63095/logs/apply-review-62548-stdout.log):

```
error: patch failed: docs/home.md:96
error: docs/home.md: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 18, 2017, 2:58 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63095/
> ---
> 
> (Updated Oct. 18, 2017, 2:58 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After moving the build documentation to its own page, we can now have a
> real "Getting Started" page suitable for anyone to get started with
> Mesos. It is purposefully short, and therefore not overwhelming.
> 
> 
> Diffs
> -
> 
>   site/config.rb 9e6738fccc48365e16221188269c1ed40772eca7 
>   site/source/downloads.html.erb 3e24100a76a99b23ebc6589ee4f7332f43ef0ac9 
>   site/source/getting-started.html.md PRE-CREATION 
>   site/source/index.html.erb b4e847fadf6682503a4e09139491b539bc59e0fd 
>   site/source/layouts/documentation.erb 
> a91f916a5fb7348b2702c272e7a2059bdf628c66 
>   site/source/layouts/getting_started_section.erb PRE-CREATION 
>   site/source/layouts/layout.erb 9213c63e6fef3f57fc225a87a7d7e0abdcc11a88 
> 
> 
> Diff: https://reviews.apache.org/r/63095/diff/1/
> 
> 
> Testing
> ---
> 
> Built the site and tested it out locally.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 62903: Added a call to update total resources and pending operations.

2017-10-18 Thread Mesos Reviewbot Windows

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62903/#review188459
---



PASS: Mesos patch 62903 was successfully built and tested.

Reviews applied: `['63001', '62903']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62903

- Mesos Reviewbot Windows


On Oct. 17, 2017, 11:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62903/
> ---
> 
> (Updated Oct. 17, 2017, 11:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8107
> https://issues.apache.org/jira/browse/MESOS-8107
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now a resource provider first `SUBSCRIBE` to the resource provider
> manager without resources to get its ID, then use the ID to prepare the
> checkpoints for recovery and persistent work directory, and then
> update its total resources and pending operations through
> `UPDATE_STATE`.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/validation.cpp 
> d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/resource_provider_manager_tests.cpp 
> ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
>   src/tests/resource_provider_validation_tests.cpp 
> f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/62903/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>



Re: Review Request 62003: Added `network/ports` isolator nested container tests.

2017-10-18 Thread Mesos Reviewbot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62003/#review188458
---



Patch looks great!

Reviews applied: [60491, 60493, 60494, 60764, 60901, 60836, 61538, 60902, 
60492, 60495, 61536, 60496, 60592, 60591, 60766, 60903, 60765, 60593, 62003]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Oct. 17, 2017, 7:15 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62003/
> ---
> 
> (Updated Oct. 17, 2017, 7:15 p.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
> https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `network/ports` isolator nested container tests using the v1
> TaskGroups API. This tests that rogue port usage by a nested task is
> detected both with and without agent recovery, and that a well-behaved
> task is preserved across agent recovery.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/ports_isolator_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62003/diff/6/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>