Review Request 44513: Added missing flag to authentication docs.

[Greg Mann]

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

Review request for mesos and Adam B.


Repository: mesos


Description
---

Added missing flag to authentication docs.


Diffs
-

  docs/authentication.md 2de0eedd75e6f34549351c0ace4fee9aba7f2fd1 

Diff: https://reviews.apache.org/r/44513/diff/


Testing
---


Thanks,

Greg Mann

Review Request 44512: Support to get weights info by /weights.

[Yongqiao Wang]

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Support to get weights info by /weights.


Diffs
-

  include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 
  src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/weights_handler.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44512/diff/


Testing
---

make && make check.

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
--authenticate_http --credentials=/opt/credentials.json  >> 
/tmp/mesos-master.log 2>&1 &)

$ curl --user framework1:secret_string1 http://localhost:5050/weights | python 
-mjson.tool
{}

$ curl --user framework1:secret_string1 --data 
"[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
 -X PUT http://127.0.0.1:5050/weights

$ curl --user framework1:secret_string1 http://localhost:5050/weights | python 
-mjson.tool
{
"infos": [
{
"role": "role3",
"weight": 3.4
},
{
"role": "role2",
"weight": 1.0
},
{
"role": "role1",
"weight": 1.8
}
]
}

(TODO) Add couple of tests in another patch.


Thanks,

Yongqiao Wang

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Qian Zhang]

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

(Updated March 8, 2016, 10:42 p.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


Changes
---

Introduced a protobuf message "NetworkConfig"


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


Repository: mesos


Description
---

Added the framework of 'network/cni' isolator.


Diffs (updated)
-

  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/spec.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/spec.proto PRE-CREATION 

Diff: https://reviews.apache.org/r/44269/diff/


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 43824: Addressed comments of 41672.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43824]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 8, 2016, 12:10 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43824/
> ---
> 
> (Updated March 8, 2016, 12:10 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addressed comments of 41672.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43824/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 43920: Added a helper function to stout : os/which.hpp.

[Disha Singh]

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

(Updated March 8, 2016, 2:22 p.m.)


Review request for mesos and Joseph Wu.


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


Repository: mesos


Description
---

Added a helper function to stout : os/which.hpp.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/which.hpp PRE-CREATION 

Diff: https://reviews.apache.org/r/43920/diff/


Testing
---


Thanks,

Disha  Singh

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

> On Feb. 9, 2016, 7:13 p.m., Joerg Schad wrote:
> > src/docker/docker.cpp, line 307
> > 
> >
> > Are these additional checks which should apply in both cases (i.e. 
> > deprecated and new `addressLocation`? I.e. prior to your change it did not 
> > return an error if `HostConfig.NetworkMode` was not set, or am I mistaken?
> > Feel free to drop if this is the intended behavior!
> 
> haosdent huang wrote:
> `HostConfig.NetworkMode` exists since api 
> [1.15](https://docs.docker.com/engine/reference/api/docker_remote_api_v1.15/) 
> I think it's better we use the old way to get ip first. If we get the ip by 
> old way failed, try use `HostConfig.NetworkMode` and the new way to get ip 
> again.
> 
> Guangya Liu wrote:
> +1 to haosdent, we can first use old way to get IP address then if old 
> way failed, use the new way to get IP address.
> 
> Kapil Arya wrote:
> I think it's better to first attempt the new API, since that's forward 
> looking. If that fails, then we can go for the deprecated route.
> 
> Joerg Schad wrote:
> I wasn't so much concerned about the order of old vs new. The additional 
> check for `HostConfig.NetworkMode` just made me curios. Imagine a scenario 
> using the old way via `NetworkSettings.IPAddress` but not setting 
> `HostConfig.NetworkMode`: After this patch this will fail with `Unable to 
> detect network mode.`. Is that a scenario which might break currently working 
> setups?
> 
> haosdent huang wrote:
> >Is that a scenario which might break currently working setups
> 
> Yes, `HostConfig.NetworkMode` need make sure docker version >=1.3.x.
> 
> Travis Hegner wrote:
> I don't believe that using the old way first is the correct route to go 
> because that field still exists in the new docker API, it is just left blank 
> when using a custom network driver. I can't base it off of it's existence, 
> and I can't base it off of whether or not the field is blank, because blank 
> might be an expected scenario if the container was started without networking.
> 
> I agree with Kapil that we should check the new API first, but Haosdent 
> has a valid point that the check of HostConfig.NetworkMode could cause a 
> failure scenario with older docker versions. I will work on handling that 
> error condition in a more graceful way.

This issue is now handled by validating the docker version. If the version is 
greater than 1.9.0, the new API is used, otherwise the old one is used. In this 
case, returning an error if network mode is not detected is valid, as the 
network mode is required to determine the IP address.


- Travis


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


On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 17, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated March 8, 2016, 2:08 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs
-

  docs/upgrades.md b22e87ff42698ffafe29f2af829e7505d611a658 

Diff: https://reviews.apache.org/r/43798/diff/


Testing
---

Viewed via gist (https://gist.github.com/joerg84/5a6862bbe0809518c0c7) and 
docker website container.


File Attachments (updated)


overview
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Rendered Website
  
https://reviews.apache.org/media/uploaded/files/2016/03/08/dae6b1ea-a627-4198-b5ad-8e744120ab68__Screen_Shot_2016-03-08_at_15.06.55.png


Thanks,

Joerg Schad

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated March 8, 2016, 2:05 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs (updated)
-

  docs/upgrades.md b22e87ff42698ffafe29f2af829e7505d611a658 

Diff: https://reviews.apache.org/r/43798/diff/


Testing
---

Viewed via gist (https://gist.github.com/joerg84/5a6862bbe0809518c0c7) and 
docker website container.


File Attachments (updated)


overview
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png


Thanks,

Joerg Schad

Review Request 44511: Add registry tests for /weights endpoint.

[Yongqiao Wang]

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Add registry tests for /weights endpoint.


Diffs
-

  src/tests/dynamic_weights_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/44511/diff/


Testing
---

make && make check.


Thanks,

Yongqiao Wang

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated March 8, 2016, 1:33 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs
-

  docs/upgrades.md b22e87ff42698ffafe29f2af829e7505d611a658 

Diff: https://reviews.apache.org/r/43798/diff/


Testing
---

Viewed via gist (https://gist.github.com/joerg84/5a6862bbe0809518c0c7) and 
docker website container.


File Attachments (updated)


overview
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png
HTML Table
  
https://reviews.apache.org/media/uploaded/files/2016/03/08/72054577-c08b-46b8-8d76-4b26541f179f__Screen_Shot_2016-03-08_at_14.32.04.png


Thanks,

Joerg Schad

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated March 8, 2016, 1:24 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs (updated)
-

  docs/upgrades.md b22e87ff42698ffafe29f2af829e7505d611a658 

Diff: https://reviews.apache.org/r/43798/diff/


Testing
---

Viewed via gist (https://gist.github.com/joerg84/5a6862bbe0809518c0c7) and 
docker website container.


File Attachments


overview
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png


Thanks,

Joerg Schad

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated March 8, 2016, 1:29 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs (updated)
-

  docs/upgrades.md b22e87ff42698ffafe29f2af829e7505d611a658 

Diff: https://reviews.apache.org/r/43798/diff/


Testing
---

Viewed via gist (https://gist.github.com/joerg84/5a6862bbe0809518c0c7) and 
docker website container.


File Attachments


overview
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png


Thanks,

Joerg Schad

Re: Review Request 43093: MESOS-4370 NetworkSettings.IPAddress field is deprectaed in Docker

[Travis Hegner]

> On March 7, 2016, 5:36 p.m., Timothy Chen wrote:
> > Are you still be able to work on this? We like to get this merged, so if 
> > you can't or don't reply we will create a new patch based on this.
> 
> Travis Hegner wrote:
> Hi Timothy,
> 
> I've been stalling this for https://reviews.apache.org/r/42516, as that 
> patch will change the way this patch should be written. It only works now out 
> of pure luck in the way docker intereprets multiple "--net" parameters, since 
> mesos doesn't yet officially support user defined networks.
> 
> The linked patch will make this patch support it properly, as well as 
> make this patch easier to write, as the network name will not have to be 
> queried at all from json; it will be available as a variable.
> 
> Timothy Chen wrote:
> I don't quite understand the pure luck part, IIUC your patch is updating 
> how we parse the Docker inspect output for Network IPAddress, which shouldn't 
> matter how we actually launched the container right? How is the linked patch 
> related?
> 
> Timothy Chen wrote:
> At this moment this is actually like becomes a blocker for 0.28 as we're 
> not returning the right IP address with the latest Docker daemon, so I'd like 
> to get this in ASAP. For now I'll create a new patch based on your changes, 
> and once the dependend patch goes in will love if you can continue the work.

I think I understand the miscommunication better now. As you know, beginning 
with docker 1.9 the docker inspect output changed the location of the 
IPAddress. The original location was still populated for backwards 
compatability, but only for the common "bridge" and "host" network types. Mesos 
is written to fail with any other network type. With the new user defined 
networks feature, the old location was not populated. My patch was originally 
intended to address the fact that with user defined networks, the original ip 
location was null.

In order to utilize user defined networks in my environment, we are passing 
arbitraty docker parameters to mesos with the docker containerizer from 
marathon. This results in multiple "--net" parameters passed to docker. The 
luck comes into play because mesos interperets the first --net parameter of 
"bridge" and succeeds, and docker interperets the second --net parameter of my 
UDN, and connects to the right network. I would consider this behavior unstable 
at best.

Based on the sudden up-tick in interest in this patch, I am speculating that 
docker 1.10 is no longer populating the original ip address field (I would be 
un-aware, because I've been running my cluster with this patch), which this 
patch will successfully fix, and even be stable for the typical "host" and 
"bridge" networks.

All that said, I can see why this patch is now more important, even though it 
should be re-structured after review 42516 is implemented. I'll see if I can 
spend some time today and address the remaining issues with this patch.


- Travis


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


On Feb. 17, 2016, 10:52 p.m., Travis Hegner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43093/
> ---
> 
> (Updated Feb. 17, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Kapil Arya, and Timothy Chen.
> 
> 
> Bugs: MESOS-4370
> https://issues.apache.org/jira/browse/MESOS-4370
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes [MESOS-4370]
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp b4b8d3e 
> 
> Diff: https://reviews.apache.org/r/43093/diff/
> 
> 
> Testing
> ---
> 
> This patch will first query the docker API for the HostConfig.NetworkMode, 
> which is populated with the network name. (Essentially what was passed in 
> --net  to the docker run command). This name is then used as a key in 
> NetworkSettings.Networks..IPAddress to get the IP address that is 
> currently in use by the container.
> 
> It appears that even though the docker API has been set up to allow for 
> multiple networks, our testing has indicated that it's still only applying 
> one network to the container (the last one via the --net argument on the run 
> line). I can only speculate that the docker API will change again in the near 
> future, but I can't speculate how, so at least this fixes the problem as it 
> stands right now.
> 
> Tested and working with Docker 1.9.1, Mesos 0.27.0, on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Travis Hegner
> 
>

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated March 8, 2016, 1:19 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


Changes
---

Improved html style.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs (updated)
-

  docs/upgrades.md b22e87ff42698ffafe29f2af829e7505d611a658 

Diff: https://reviews.apache.org/r/43798/diff/


Testing
---

Viewed via gist (https://gist.github.com/joerg84/5a6862bbe0809518c0c7) and 
docker website container.


File Attachments


overview
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png


Thanks,

Joerg Schad

Re: Review Request 42719: Add doc for weights.

[Yongqiao Wang]

> On March 8, 2016, 9:19 a.m., Adam B wrote:
> > docs/weights.md, lines 22-23
> > 
> >
> > This will have to be updated when we add GET

I will update this doc for GET request after addressing #MESOS-4316.


- Yongqiao


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


On March 8, 2016, 11:27 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42719/
> ---
> 
> (Updated March 8, 2016, 11:27 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3945
> https://issues.apache.org/jira/browse/MESOS-3945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add doc for weights.
> 
> 
> Diffs
> -
> 
>   docs/home.md 4327d461f3bd4cced85ceb505414b3e1afefff17 
>   docs/weights.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42719/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 43863: Move the implementation of updateWeights out of header.

[Yongqiao Wang]

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

(Updated March 8, 2016, 1:05 p.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Addressed comments of Adam.


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


Repository: mesos


Description
---

Move the implementation of updateWeights out of header.


Diffs (updated)
-

  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/master/weights.hpp PRE-CREATION 
  src/master/weights.cpp PRE-CREATION 
  src/master/weights_handler.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/43863/diff/


Testing
---

make && make check successfully!


Thanks,

Yongqiao Wang

Re: Review Request 42719: Add doc for weights.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42719]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 8, 2016, 11:27 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42719/
> ---
> 
> (Updated March 8, 2016, 11:27 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3945
> https://issues.apache.org/jira/browse/MESOS-3945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add doc for weights.
> 
> 
> Diffs
> -
> 
>   docs/home.md 4327d461f3bd4cced85ceb505414b3e1afefff17 
>   docs/weights.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42719/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

[Yongqiao Wang]

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

(Updated March 8, 2016, 12:47 p.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Addressed comments of Adam.


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


Repository: mesos


Description
---

Introduce HTTP endpoint /weights for updating weight.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
ec6c9928c55c3096c7de634f900419abbdd00886 
  include/mesos/authorizer/authorizer.proto 
723da93fa1e968bd98612dd7594b59115050f489 
  src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
  src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
  src/authorizer/local/authorizer.hpp 96baf77709cf721caf46b6c2c096a843c1b5d9c0 
  src/authorizer/local/authorizer.cpp 4e5c2c2869823ec957735cebfc80dc850d40f9eb 
  src/master/http.cpp a3ad57a1c3f8a01aa609b28c12825670bb243387 
  src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
  src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
  src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
  src/master/weights_handler.cpp PRE-CREATION 
  src/tests/mesos.hpp 9c62833e0a64cfd62fce8cffd04f9cdd933646c8 
  src/tests/mesos.cpp 395b23d32b2d03aef446858e197cb9788644eefa 

Diff: https://reviews.apache.org/r/41681/diff/


Testing
---

Make & Make check successfully!

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
--weights="role1=4.2,role2=3.1" --authenticate_http 
--credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
$ curl http://localhost:5050/roles | python -mjson.tool
{
"roles": [
{
"frameworks": [ ], 
"name": "*", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 1
}, 
{
"frameworks": [ ], 
"name": "role1", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 4.2
}, 
{
"frameworks": [ ], 
"name": "role2", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 3.1
}
]
}

Test update:
$ curl --user framework1:secret_string1 --data 
"[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
 -X PUT http://127.0.0.1:5050/weights
$ curl http://localhost:5050/roles | python -mjson.tool
{
"roles": [
{
"frameworks": [],
"name": "*",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"name": "role1",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.8
},
{
"frameworks": [],
"name": "role2",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"name": "role3",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 3.4
}
]
}

Test recovuery:
$ ps -ef | grep mesos-master
501 56292 1   0  6:18PM ttys0010:00.31 
/Users/yqwyq/Desktop/mesos/build/src/.libs/mesos-master --ip=127.0.0.1 
--work_dir=/tmp/mesos-master --weights=role1=4.2,role2=3.1 --authenticate_http 
--credentials=/opt/credentials.json
$ kill -9 56292

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
--weights="role1=4.2,role2=3.1,role6=9.0" --authenticate_http 
--credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
$ curl http://localhost:5050/roles | python -mjson.tool
{
"roles": [
{
"frameworks": [],
"name": "*",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"name": "role1",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.8
},
{
"frameworks": [],
"name": "role2",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{

Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

[Yongqiao Wang]

> On March 8, 2016, 8:07 a.m., Adam B wrote:
> > src/master/master.hpp, lines 1040-1042
> > 
> >
> > So if I try to update N roles in one request, but I am not authorized 
> > to update 1, then I would get back a simple boolean "Forbidden" response, 
> > rather than a pointer to which role was unauthorized? I guess the best 
> > thing the client/user can do then is just try to update each role 
> > individually until one is rejected.
> > Is there some way we could give back a more helpful error message with 
> > the Forbidden response?

Current we do a batch authorization, the called functions `allows` and 
`matches` only return a `bool` value, so we can not get the detailed Forbidden 
response. Maybe two solutions can improve it:
1. Enhance `allows` and `matches` to return the Forbidden response.
2. Do authorization one by one, but it will have a worse performance than the 
corrent implementation.

Can us enhance this in another JIRA later?


- Yongqiao


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


On March 7, 2016, 12:19 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> ---
> 
> (Updated March 7, 2016, 12:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduce HTTP endpoint /weights for updating weight.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 5ee3c7afadd131802c93febbb6b4dbad069c2d81 
>   include/mesos/authorizer/authorizer.proto 
> 84d2cb3fbff3fbc7c3854d6eec5a3a55ad5760f8 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/authorizer/local/authorizer.hpp 
> c7321c276d566eca6a91f45c546468bea1b0da15 
>   src/authorizer/local/authorizer.cpp 
> a1486bd042e1d59e5ac99c2619fb3228c37b9788 
>   src/master/http.cpp 8276baa538eb4d2aaf54cc1aa516bffaadacc4dd 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
>   src/master/weights_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp d36840f6e23e5823332de53061bf6852330bdf0b 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/41681/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully!
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
> --weights="role1=4.2,role2=3.1" --authenticate_http 
> --credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
> "roles": [
> {
> "frameworks": [ ], 
> "name": "*", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 1
> }, 
> {
> "frameworks": [ ], 
> "name": "role1", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 4.2
> }, 
> {
> "frameworks": [ ], 
> "name": "role2", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 3.1
> }
> ]
> }
> 
> Test update:
> $ curl --user framework1:secret_string1 --data 
> "[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
>  -X PUT http://127.0.0.1:5050/weights
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
> "roles": [
> {
> "frameworks": [],
> "name": "*",
> "resources": {
> "cpus": 0,
> "disk": 0,
> "mem": 0
> },
> "weight": 1.0
> },
> {
> "frameworks": [],
> "name": "role1",
> "resources": {
> "cpus": 0,
> "disk": 0,
> "mem": 0
> },
> "weight": 1.8
> },
> {
> "frameworks": [],
> "name": "role2",
> "resources": {
> "cpus": 0,
> "disk": 0,
> "mem": 0
> },
>  

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated March 8, 2016, 12:29 p.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


Changes
---

Moved table to direct html.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs (updated)
-

  docs/upgrades.md b22e87ff42698ffafe29f2af829e7505d611a658 

Diff: https://reviews.apache.org/r/43798/diff/


Testing (updated)
---

Viewed via gist (https://gist.github.com/joerg84/5a6862bbe0809518c0c7) and 
docker website container.


File Attachments


overview
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png


Thanks,

Joerg Schad

Re: Review Request 43824: Addressed comments of 41672.

[Yongqiao Wang]

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

(Updated March 8, 2016, 12:10 p.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Addressed comments of Adam.


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


Repository: mesos


Description
---

Addressed comments of 41672.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 

Diff: https://reviews.apache.org/r/43824/diff/


Testing
---

make && make check successfully.


Thanks,

Yongqiao Wang

Re: Review Request 43629: Especially updated tests to use the updated MesosTest helpers.

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On March 4, 2016, 3:55 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43629/
> ---
> 
> (Updated March 4, 2016, 3:55 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Continuation of https://reviews.apache.org/r/43615/ with a slightly different 
> pattern.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_cache_tests.cpp f9c48f5d938c2601cb8f826029d6969d676ab98e 
>   src/tests/resource_offers_tests.cpp 
> 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
>   src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
> 
> Diff: https://reviews.apache.org/r/43629/diff/
> 
> 
> Testing
> ---
> 
> Tests are run at the end of this review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 43629: Especially updated tests to use the updated MesosTest helpers.

[Bernd Mathiske]

> On March 3, 2016, 5:55 a.m., Bernd Mathiske wrote:
> > src/tests/slave_recovery_tests.cpp, line 3461
> > 
> >
> > Why was this moved up here? Couldn't this be in line 3389/3402?
> 
> Joseph Wu wrote:
> In this case, it's because all injections of `Try 
> slave` must be stack-allocated above.  Even though we use `containerizer2` 
> much later, if we put it lower down, it would be deallocated before the 
> second `slave` (and then segfault!).
> 
> Bernd Mathiske wrote:
> I am not following yet. 
> 
> 1. How could it be deallocated before it has been allocated? How could it 
> even be referenced then? I assume "it" refers to sontainerizer2. 
> 2. The second slave would still be constructed later than containerizer2.
> 
> Joseph Wu wrote:
> In the updated test, the items go on the stack like this:
> ```
> containerizer1
> containerizer2
> slave
> ```
> 
> If I don't move `containerizer2` up, the stack will look like this:
> ```
> containerizer1
> slave
> containerizer2
> ```
> because re-constructing `slave` doesn't change `slave`'s order in the 
> stack.

So it's about the destruction order, not the construction order. Good point!


- Bernd


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


On March 4, 2016, 3:55 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43629/
> ---
> 
> (Updated March 4, 2016, 3:55 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Continuation of https://reviews.apache.org/r/43615/ with a slightly different 
> pattern.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_cache_tests.cpp f9c48f5d938c2601cb8f826029d6969d676ab98e 
>   src/tests/resource_offers_tests.cpp 
> 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
>   src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
> 
> Diff: https://reviews.apache.org/r/43629/diff/
> 
> 
> Testing
> ---
> 
> Tests are run at the end of this review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Bernd Mathiske]

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


Fix it, then Ship it!





src/tests/scheduler_event_call_tests.cpp (line 367)


In most other places you have the blank line before the detector.



src/tests/slave_tests.cpp (line 179)


With this line you have now introduced a 3rd way to write this stretch of 
code. Please pick one. This one is probably the least controversial. :-)


- Bernd Mathiske


On March 4, 2016, 4:14 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43615/
> ---
> 
> (Updated March 4, 2016, 4:14 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Includes the following changes:
> 
> * Added the `` header where appropriate.
> * Added the namespace `using process::Owned;` where appropriate.
> * Generally replaced `Try` with `Owned`.  And 
> `Try` with `Owned`.
> * Added the (now required) `MasterDetector` argument to all slaves.  Before, 
> this was fetched from the first master in `Cluster`.
> * Removed `Shutdown();` from all tests.
> * Replaced `Stop(...)` with the appropriate master/slave destruction calls.
> * Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
> launchers, etc).
> * Replace `CHECK` in tests with `ASSERT`.
> 
> 
> Diffs
> -
> 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/command_executor_tests.cpp 
> 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
>   src/tests/container_logger_tests.cpp 
> 00f4129e46aa9268fbb66da25b34e61004fa87b2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6aecd912fc84b72d2b64f7a842891fddcbc469ac 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e72239a55724f1aeeec5362cc370c93dbeca7164 
>   src/tests/containerizer/isolator_tests.cpp 
> 342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 03879d99c371f296f8d9904666911b34209c114d 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 983a6be160aefe5a32acb6111bb3c85230ec 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 
>   src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
>   src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
>   src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
>   src/tests/executor_http_api_tests.cpp 
> 2fc0893f5f5e80a783296fb31b30abe86d92df1b 
>   src/tests/fault_tolerance_tests.cpp 
> f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/hook_tests.cpp 88ad7806bf9f3194e434743c35199a896edea92c 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_quota_tests.cpp 4fabc1473ec3e048afe7171abbb8d6e49e863847 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
>   src/tests/master_validation_tests.cpp 
> c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
>   src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 81185a161498394020a27f1f5bf747bac5425f43 
>   src/tests/persistent_volume_tests.cpp 
> bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
>   src/tests/rate_limiting_tests.cpp 352a5e7a58b500c25c7c8a421047c4e79434e38e 
>   src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
>   src/tests/registrar_zookeeper_tests.cpp 
> 3df9779ee5d076e16f6a538326693a36f986b6d0 
>   

Re: Review Request 44269: Added the framework of 'network/cni' isolator.

[Abhishek Dasgupta]

> On March 4, 2016, 6:55 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni.cpp, line 61
> > 
> >
> > Add a `+` at the end. While not necessary, makes it more readable.
> 
> Qian Zhang wrote:
> Agree.
> 
> Qian Zhang wrote:
> There will be a compile error if adding a `+` at the end :-(

Probably, you have to add this way:

return Error(
"Failed to find location of the CNI network" +
std::string(" configuration files '") + 
flags.network_cni_config_dir.get() + "'");


- Abhishek


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


On March 6, 2016, 4:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44269/
> ---
> 
> (Updated March 6, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the framework of 'network/cni' isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/slave/containerizer/mesos/isolators/network/cni.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44269/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 42719: Add doc for weights.

[Yongqiao Wang]

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

(Updated March 8, 2016, 11:27 a.m.)


Review request for mesos and Adam B.


Changes
---

Addressed comments of Adam.


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


Repository: mesos


Description
---

Add doc for weights.


Diffs (updated)
-

  docs/home.md 4327d461f3bd4cced85ceb505414b3e1afefff17 
  docs/weights.md PRE-CREATION 

Diff: https://reviews.apache.org/r/42719/diff/


Testing
---


Thanks,

Yongqiao Wang

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Bernd Mathiske]

> On March 4, 2016, 6:45 a.m., Bernd Mathiske wrote:
> >
> 
> Joseph Wu wrote:
> I also noticed a couple of these:
> ```
>   MesosSchedulerDriver driver(
> , DEFAULT_FRAMEWORK_INFO, master.get()->pid, 
> DEFAULT_CREDENTIAL);
> ```
> Now fixed (there were two spaces rather than four).
> 
> ---
> 
> Also went through and changed a couple of these:
> ```
>   Future slaveReregisteredMessage =
> FUTURE_PROTOBUF(
> SlaveReregisteredMessage(), master.get()->pid, slave.get()->pid);
> ```
> To:
> ```
>   Future slaveReregisteredMessage =
> FUTURE_PROTOBUF(
> SlaveReregisteredMessage(), 
> master.get()->pid, 
> slave.get()->pid);
> ```

OK!


> On March 4, 2016, 6:45 a.m., Bernd Mathiske wrote:
> > src/tests/master_tests.cpp, line 1140
> > 
> >
> > It's OK to continue with the first arg on the same line in such cases.
> > 
> > Here and elsewhere.
> 
> Joseph Wu wrote:
> I believe the preferred style is:
> ```
> EXPECT_EQ(
> ...,
> ...);
> ```
> Rather than:
> ```
> EXPECT_EQ(...
>   ...);
> ```
> (This is based on how we indented the MasterMaintenanceTests.)

AFAIK, it depends on how long the args are. If they are short, either way is 
fine. If they are "too long" you are always right :-)


- Bernd


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


On March 4, 2016, 4:14 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43615/
> ---
> 
> (Updated March 4, 2016, 4:14 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Includes the following changes:
> 
> * Added the `` header where appropriate.
> * Added the namespace `using process::Owned;` where appropriate.
> * Generally replaced `Try` with `Owned`.  And 
> `Try` with `Owned`.
> * Added the (now required) `MasterDetector` argument to all slaves.  Before, 
> this was fetched from the first master in `Cluster`.
> * Removed `Shutdown();` from all tests.
> * Replaced `Stop(...)` with the appropriate master/slave destruction calls.
> * Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
> launchers, etc).
> * Replace `CHECK` in tests with `ASSERT`.
> 
> 
> Diffs
> -
> 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/command_executor_tests.cpp 
> 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
>   src/tests/container_logger_tests.cpp 
> 00f4129e46aa9268fbb66da25b34e61004fa87b2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 6aecd912fc84b72d2b64f7a842891fddcbc469ac 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e72239a55724f1aeeec5362cc370c93dbeca7164 
>   src/tests/containerizer/isolator_tests.cpp 
> 342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 03879d99c371f296f8d9904666911b34209c114d 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 983a6be160aefe5a32acb6111bb3c85230ec 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 
>   src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
>   src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
>   src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
>   src/tests/executor_http_api_tests.cpp 
> 2fc0893f5f5e80a783296fb31b30abe86d92df1b 
>   src/tests/fault_tolerance_tests.cpp 
> f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   src/tests/hook_tests.cpp 88ad7806bf9f3194e434743c35199a896edea92c 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_quota_tests.cpp 

Re: Review Request 43824: Addressed comments of 41672.

[Adam B]

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


Fix it, then Ship it!




Just a bunch a grammar fixes. Otherwise shippable.


src/tests/hierarchical_allocator_tests.cpp (line 2445)


s/all/each/
s/half of resources/half of the resources/



src/tests/hierarchical_allocator_tests.cpp (line 2446)


s/role's weight/roles' weights/



src/tests/hierarchical_allocator_tests.cpp (line 2458)


s/due to/since/



src/tests/hierarchical_allocator_tests.cpp (line 2490)


s/,/;/



src/tests/hierarchical_allocator_tests.cpp (lines 2495 - 2496)


s|the 1/3 resources|1/3 of the resources|
s|the 2/3 resources|2/3 of the resources|



src/tests/hierarchical_allocator_tests.cpp (line 2496)


s/role's weight/roles' weights/



src/tests/hierarchical_allocator_tests.cpp (line 2516)


Seems like we repeat the contents of this loop a few times. Could all/part 
of the contents be worth factoring out into a reusable helper function?



src/tests/hierarchical_allocator_tests.cpp (line 2545)


s/,/;/



src/tests/hierarchical_allocator_tests.cpp (lines 2550 - 2551)


s|the X/6 resources|X/6 of the resources|g



src/tests/hierarchical_allocator_tests.cpp (line 2552)


s/role's weight/roles' weights/g



src/tests/hierarchical_allocator_tests.cpp (line 2559)


s/due to/because/



src/tests/hierarchical_allocator_tests.cpp (line 2605)


s/,/;/


- Adam B


On March 7, 2016, 5:55 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43824/
> ---
> 
> (Updated March 7, 2016, 5:55 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addressed comments of 41672.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43824/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 43824: Addressed comments of 41672.

[Adam B]

> On March 3, 2016, 1:03 a.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 2607-2609
> > 
> >
> > I'm an ESL, but having both "per weight" and "by weight" sounds a bit 
> > strange. Maybe Adam can help wiht finding the right preposition.
> 
> Yongqiao Wang wrote:
> Thanks Alex. @Adam, I am also an ESL. What is your suggestion for this?

s/per role's weight/according to each role's weight/
s/number of resources by their role's weight/appropriate number of resources/


> On March 3, 2016, 1:03 a.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 2652-2654
> > 
> >
> > How about
> > 
> > // Framework2 registers with 'role2' which also uses the default 
> > weight. It
> > // will not get any offers because all resources are offered to 
> > `framework1`.
> 
> Yongqiao Wang wrote:
> I remember I wrote this comment like you before. The current comment is 
> changed with Adam's comments. @Adam, any further comments for this?

I'm fine with either, but Alex's is shorter.


- Adam


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


On March 7, 2016, 5:55 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43824/
> ---
> 
> (Updated March 7, 2016, 5:55 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4200
> https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Addressed comments of 41672.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 
> 
> Diff: https://reviews.apache.org/r/43824/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 43863: Move the implementation of updateWeights out of header.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/master/weights.hpp (line 35)


Kill the blank line



src/master/weights.hpp (line 60)


I like that you omit names for unused parameters, but please keep those, 
which are used (`registry`)! This serves as documentation.



src/master/weights.cpp (lines 51 - 52)


Blank line, please.


- Alexander Rukletsov


On Feb. 27, 2016, 12:43 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43863/
> ---
> 
> (Updated Feb. 27, 2016, 12:43 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Move the implementation of updateWeights out of header.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 49a5645ef7242dbaee31e7b26dbbcb1f4f1f910e 
>   src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp 7c62f2a882a1c89d73f328b2ae665422fd84d7a1 
>   src/master/weights.hpp PRE-CREATION 
>   src/master/weights.cpp PRE-CREATION 
>   src/master/weights_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43863/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully!
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 43863: Move the implementation of updateWeights out of header.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On Feb. 27, 2016, 4:43 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43863/
> ---
> 
> (Updated Feb. 27, 2016, 4:43 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Move the implementation of updateWeights out of header.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 49a5645ef7242dbaee31e7b26dbbcb1f4f1f910e 
>   src/Makefile.am 2a26261b513bb7c03437ed8e850c3b36b93d82f5 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp 7c62f2a882a1c89d73f328b2ae665422fd84d7a1 
>   src/master/weights.hpp PRE-CREATION 
>   src/master/weights.cpp PRE-CREATION 
>   src/master/weights_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/43863/diff/
> 
> 
> Testing
> ---
> 
> make && make check successfully!
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 42719: Add doc for weights.

[Adam B]

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


Fix it, then Ship it!





docs/weights.md (lines 51 - 53)


"Weights are now persisted in the registry on cluster bootstrap and after 
any updates. Once the weights are persisted in the registry, any Mesos master 
that subsequently starts with `--weights` still specified will warn and use the 
registry value instead."



docs/weights.md (lines 22 - 23)


This will have to be updated when we add GET


- Adam B


On March 7, 2016, 3:35 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42719/
> ---
> 
> (Updated March 7, 2016, 3:35 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3945
> https://issues.apache.org/jira/browse/MESOS-3945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add doc for weights.
> 
> 
> Diffs
> -
> 
>   docs/home.md 821026a12f422385e347e0037d6527efa9ffa2e1 
>   docs/weights.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42719/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 44441: Treated command as executable value and arguments in mesos-execute.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [1]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 8, 2016, 5:17 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1/
> ---
> 
> (Updated March 8, 2016, 5:17 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4882
> https://issues.apache.org/jira/browse/MESOS-4882
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Treated command as executable value and arguments in mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp ed42cb568f3d16856f48b3bbd354cb2b0fb83e8e 
> 
> Diff: https://reviews.apache.org/r/1/diff/
> 
> 
> Testing
> ---
> 
> ./src/mesos-execute --master=192.168.56.12:5050  --name=test_mesos 
> --docker_image=busybox:latest --containerizer=mesos --no-shell  
> --command="ls,/etc/passwd" 
> I0307 15:48:09.834506 27450 sched.cpp:222] Version: 0.29.0
> I0307 15:48:09.841404 27468 sched.cpp:326] New master detected at 
> master@192.168.56.12:5050
> I0307 15:48:09.843992 27468 sched.cpp:336] No credentials provided. 
> Attempting to register without authentication
> I0307 15:48:09.848901 27468 sched.cpp:703] Framework registered with 
> a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
> Framework registered with a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001
> task test_mesos submitted to slave 6fa2afa1-768b-4f2e-9c69-9f1017634e72-S2
> Received status update TASK_RUNNING for task test_mesos
> Received status update TASK_FINISHED for task test_mesos
> I0307 15:48:10.284418 27466 sched.cpp:1903] Asked to stop the driver
> I0307 15:48:10.284494 27466 sched.cpp:1143] Stopping framework 
> 'a767db64-1bc8-4d7a-9eb5-90ee1f4ab8cf-0001'
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 43798: Added overview section to upgrades.md.

[Joerg Schad]

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

(Updated March 8, 2016, 8:13 a.m.)


Review request for mesos, Michael Park, Neil Conway, and Till Toenshoff.


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


Repository: mesos


Description
---

Added overview section to upgrades.md.


Diffs
-

  docs/upgrades.md 4f30d725c6ed28c09a1c5528fd4193c3f06b2d93 

Diff: https://reviews.apache.org/r/43798/diff/


Testing
---

Viewed via gist (https://gist.github.com/joerg84/eddbc0302a5a4b291e81) and 
docker website container.


File Attachments (updated)


overview
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/39d7552f-9633-4d19-8e32-d11bbd1357f5__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/feea0b5d-7b05-4353-a898-cb83f648c206__Screen_Shot_2016-02-29_at_3.15.12_AM.png
Screen Shot 2016-02-29 at 3.15.12 AM.png
  
https://reviews.apache.org/media/uploaded/files/2016/02/29/44ca4c96-5849-45b9-b1de-0be25630fbaa__Screen_Shot_2016-02-29_at_3.15.12_AM.png


Thanks,

Joerg Schad

Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

[Adam B]

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


Fix it, then Ship it!




Would love to commit this asap, but we *need* to get the registry tests in with 
these changes. I'd like to at least do a quick pass over the registry tests 
first so a) we know we're testing the critical changes, and b) we have 
confidence that the tests will land in the same release as the endpoint.


include/mesos/authorizer/authorizer.proto (line 138)


Oops, looks like you copied my typo. Sorry.
s/updated/update/



src/master/master.cpp (lines 1549 - 1552)


How about:
"The registry contains dynamic weights, so static weights should be 
ignored. We must neutralize (set to 1.0) any weights statically set in 
`--weights` that are not explicitly overridden by the registry."
or
"Before recovering weights from the registry, the allocator was already 
initialized with `--weights`, so here we need to reset (to 1.0) weights in the 
allocator that are not overridden by the registry."



src/master/master.hpp (lines 1040 - 1042)


So if I try to update N roles in one request, but I am not authorized to 
update 1, then I would get back a simple boolean "Forbidden" response, rather 
than a pointer to which role was unauthorized? I guess the best thing the 
client/user can do then is just try to update each role individually until one 
is rejected.
Is there some way we could give back a more helpful error message with the 
Forbidden response?



src/master/master.cpp (lines 1546 - 1547)


This log message will be easier to read if you wrap the flags value in 
single-quotes, so it becomes:
```
... flag '" << flags.weights.get()
<< "', and ...
```



src/master/master.cpp (line 1566)


Why do you need a utils::copy() if `registry_weights` is not const, and 
will not be used after this?



src/master/master.cpp (lines 1571 - 1572)


Let's add a comment that "The allocator was already updated with the flag 
values on startup."



src/master/weights_handler.cpp (line 71)


How about renaming this to `validatedWeightInfos`? Both this and 
`weightInfos` are for updating, but only one of these is validated.



src/master/weights_handler.cpp (line 111)


Why not just s/updateWeightInfos/weightInfos/?


- Adam B


On March 7, 2016, 4:19 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> ---
> 
> (Updated March 7, 2016, 4:19 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduce HTTP endpoint /weights for updating weight.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 5ee3c7afadd131802c93febbb6b4dbad069c2d81 
>   include/mesos/authorizer/authorizer.proto 
> 84d2cb3fbff3fbc7c3854d6eec5a3a55ad5760f8 
>   src/CMakeLists.txt 8f57a5701073bf1eaaa223383e928cf5db8f8ae4 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/authorizer/local/authorizer.hpp 
> c7321c276d566eca6a91f45c546468bea1b0da15 
>   src/authorizer/local/authorizer.cpp 
> a1486bd042e1d59e5ac99c2619fb3228c37b9788 
>   src/master/http.cpp 8276baa538eb4d2aaf54cc1aa516bffaadacc4dd 
>   src/master/master.hpp ea26670e6c6c67314406fded510e8fdd46053dc8 
>   src/master/master.cpp 57ff4a39039f573b8586bc03f873f97826b97f6f 
>   src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
>   src/master/weights_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp d36840f6e23e5823332de53061bf6852330bdf0b 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/41681/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully!
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
> --weights="role1=4.2,role2=3.1" --authenticate_http 
> --credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
> "roles": [
> {
> "frameworks": [ ], 
> "name": "*", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
>