Re: Review Request 57763: Fixed environment overwrite warning to not leak possibly sensitive data.

2017-03-19 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [57763]

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

- Mesos Reviewbot


On March 20, 2017, 2:51 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57763/
> ---
> 
> (Updated March 20, 2017, 2:51 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Bannier, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-7264
> https://issues.apache.org/jira/browse/MESOS-7264
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 8658525b00e78bed9227d6d400eae2cf25dd 
> 
> 
> Diff: https://reviews.apache.org/r/57763/diff/1/
> 
> 
> Testing
> ---
> 
> make check & functional testing...
> 
> ```
> ./src/mesos-execute --name="test" --env='{"key1":"value1"}' --command='sleep 
> 1000' --master=127.0.0.1:5050
> ```
> 
> Within the contents of `stdout` before applying this:
> ```
> Overwriting environment variable 'key1', original: 'value1', new: 'value1'
> ```
> 
> After applying this there is no mention of the actually duplicate but equally 
> valued variable anymore. Note, this is before applying 
> https://reviews.apache.org/r/57762.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 57534: Added and implemented RegisterAgent ACL.

2017-03-19 Thread Jiang Yan Xu 

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

(Updated March 19, 2017, 9:09 p.m.)


Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, and Greg Mann.


Changes
---

Comments.


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


Repository: mesos


Description
---

Added and implemented RegisterAgent ACL.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto e75e1879435f1c2bce47a86e9feebf9d051e969b 
  include/mesos/authorizer/authorizer.proto 
736f76d552956f2351ffd40fc51d088dff83f8c8 
  src/authorizer/local/authorizer.cpp be8037299601427e5d5e79f58f77eea3f89579d0 
  src/tests/authorization_tests.cpp cd15add7d7b01c2b316ac946e017a4d0b502237f 


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

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


Testing
---

make check.


Thanks,

Jiang Yan Xu

Re: Review Request 57534: Added and implemented RegisterAgent ACL.

2017-03-19 Thread Jiang Yan Xu 


> On March 17, 2017, 2:32 p.m., Greg Mann wrote:
> > src/tests/authorization_tests.cpp
> > Lines 4270 (patched)
> > 
> >
> > Could you also add a test case with the ANY subject?

This test already covers the subject being ANY in the ACL:

```
  {
// Nobody else can register as an agent.
mesos::ACL::RegisterAgent* acl = acls.add_register_agents();
acl->mutable_principals()->set_type(mesos::ACL::Entity::ANY);
acl->mutable_agent()->set_type(mesos::ACL::Entity::NONE);
  }
```

I assume you meant subject being ANY in the request. Sadly, it appears that 
this has never worked (see MESOS-7244), we should fix it but I don't want to 
get entangled with it in this patch. (No other tests were written to uncovere 
this)


- Jiang Yan


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


On March 14, 2017, 5:40 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57534/
> ---
> 
> (Updated March 14, 2017, 5:40 p.m.)
> 
> 
> Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, and Greg 
> Mann.
> 
> 
> Bugs: MESOS-7097
> https://issues.apache.org/jira/browse/MESOS-7097
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added and implemented RegisterAgent ACL.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> e75e1879435f1c2bce47a86e9feebf9d051e969b 
>   include/mesos/authorizer/authorizer.proto 
> 736f76d552956f2351ffd40fc51d088dff83f8c8 
>   src/authorizer/local/authorizer.cpp 
> be8037299601427e5d5e79f58f77eea3f89579d0 
>   src/tests/authorization_tests.cpp cd15add7d7b01c2b316ac946e017a4d0b502237f 
> 
> 
> Diff: https://reviews.apache.org/r/57534/diff/3/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 57762: Fixed environment duplication in default executor.

2017-03-19 Thread Till Toenshoff 

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

(Updated March 20, 2017, 4:03 a.m.)


Review request for mesos, Adam B, Benjamin Bannier, Greg Mann, Jie Yu, and 
Joseph Wu.


Changes
---

fixed gcc compilage.


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


Repository: mesos


Description
---

see summary.


Diffs (updated)
-

  src/launcher/executor.cpp bd3c0cf 


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

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


Testing
---

make check + functional testing..

Before applying the patch;

Running 
```
$ ./src/mesos-execute --name="test" --env='{"key1":"value1"}' --command='sleep 
1000' --master=127.0.0.1:5050
```
will result in the following `stdout` sandbox file:
```
Received SUBSCRIBED event
Subscribed executor on lobomacpro2.fritz.box
Received LAUNCH event
Starting task test
/Users/till/Development/mesos-private/build/src/mesos-containerizer launch 
--help="false" 
--launch_info="{"command":{"environment":{"variables":[{"name":"key1","type":"VALUE","value":"value1"}]},"shell":true,"value":"sleep
 
1000"},"environment":{"variables":[{"name":"BIN_SH","type":"VALUE","value":"xpg4"},{"name":"DUALCASE","type":"VALUE","value":"1"},{"name":"DYLD_LIBRARY_PATH","type":"VALUE","value":"\/Users\/till\/Development\/mesos-private\/build\/src\/.libs"},{"name":"LIBPROCESS_PORT","type":"VALUE","value":"0"},{"name":"MESOS_AGENT_ENDPOINT","type":"VALUE","value":"192.168.178.20:5051"},{"name":"MESOS_CHECKPOINT","type":"VALUE","value":"0"},{"name":"MESOS_DIRECTORY","type":"VALUE","value":"\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-\/executors\/test\/runs\/025c1d1a-6886-4b42-9313-0b1a411d5e39"},{"name":"MESOS_EXECUTOR_ID","type":"VALUE","value":"test"},{"name":"MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD","type":"
 
VALUE","value":"5secs"},{"name":"MESOS_FRAMEWORK_ID","type":"VALUE","value":"c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-"},{"name":"MESOS_HTTP_COMMAND_EXECUTOR","type":"VALUE","value":"0"},{"name":"MESOS_SANDBOX","type":"VALUE","value":"\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-\/executors\/test\/runs\/025c1d1a-6886-4b42-9313-0b1a411d5e39"},{"name":"MESOS_SLAVE_ID","type":"VALUE","value":"816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0"},{"name":"MESOS_SLAVE_PID","type":"VALUE","value":"slave(1)@192.168.178.20:5051"},{"name":"PATH","type":"VALUE","value":"\/usr\/local\/sbin:\/usr\/local\/bin:\/usr\/sbin:\/usr\/bin:\/sbin:\/bin"},{"name":"PWD","type":"VALUE","value":"\/private\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-\/executors\/test\/runs\/025c1d1a-6886-4b42-9313-0b1a411d5e39"},{"name":"SHLVL","type":"VALUE","value":"0"},{"name":"__CF_USER_TEXT_E
 
NCODING","type":"VALUE","value":"0x1F5:0x0:0x0"},{"name":"key1","type":"VALUE","value":"value1"},{"name":"key1","type":"VALUE","value":"value1"}]}}"
Forked command at 2376
Overwriting environment variable 'key1', original: 'value1', new: 'value1'
```

After applying the patch;

The resulting `stdout` sandbox file is as clean as it should be:
```
Received SUBSCRIBED event
Subscribed executor on lobomacpro2.fritz.box
Received LAUNCH event
Starting task test
/Users/till/Development/mesos-private/build/src/mesos-containerizer launch 
--help="false" 
--launch_info="{"command":{"environment":{"variables":[{"name":"key1","type":"VALUE","value":"value1"}]},"shell":true,"value":"sleep
 
1000"},"environment":{"variables":[{"name":"PATH","type":"VALUE","value":"\/usr\/local\/sbin:\/usr\/local\/bin:\/usr\/sbin:\/usr\/bin:\/sbin:\/bin"},{"name":"PWD","type":"VALUE","value":"\/private\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-\/executors\/test\/runs\/46f219a3-2772-4d21-89b8-85f1c1ba362e"},{"name":"MESOS_SLAVE_PID","type":"VALUE","value":"slave(1)@192.168.178.20:5051"},{"name":"MESOS_FRAMEWORK_ID","type":"VALUE","value":"816619b6-f5ce-42d6-ad6b-2ef2001adc0a-"},{"name":"MESOS_EXECUTOR_ID","type":"VALUE","value":"test"},{"name":"MESOS_CHECKPOINT","type":"VALUE","value":"0"},{"name":"MESOS_SLAVE_ID","type":"VALUE","value":"816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0"},{"name":"MESOS_S

Review Request 57764: Removed containerizer flag logging to prevent leak of sensitive data.

2017-03-19 Thread Till Toenshoff 

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

Review request for mesos, Adam B, Benjamin Bannier, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/launcher/posix/executor.cpp 59e7c0c7737e96059f117899eb4d9e69f2c8369d 


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


Testing
---

make check & functional testing...

```
$ ./src/mesos-execute --name="test" --env='{"key1":"value1"}' --command='sleep 
1000' --master=127.0.0.1:5050
```

`stdout` before applying this:
```
Received SUBSCRIBED event
Subscribed executor on lobomacpro2.fritz.box
Received LAUNCH event
Starting task test
/Users/till/Development/mesos-private/build/src/mesos-containerizer launch 
--help="false" 
--launch_info="{"command":{"environment":{"variables":[{"name":"key1","type":"VALUE","value":"value1"}]},"shell":true,"value":"sleep
 
1000"},"environment":{"variables":[{"name":"BIN_SH","type":"VALUE","value":"xpg4"},{"name":"DUALCASE","type":"VALUE","value":"1"},{"name":"DYLD_LIBRARY_PATH","type":"VALUE","value":"\/Users\/till\/Development\/mesos-private\/build\/src\/.libs"},{"name":"LIBPROCESS_PORT","type":"VALUE","value":"0"},{"name":"MESOS_AGENT_ENDPOINT","type":"VALUE","value":"192.168.178.20:5051"},{"name":"MESOS_CHECKPOINT","type":"VALUE","value":"0"},{"name":"MESOS_DIRECTORY","type":"VALUE","value":"\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-\/executors\/test\/runs\/025c1d1a-6886-4b42-9313-0b1a411d5e39"},{"name":"MESOS_EXECUTOR_ID","type":"VALUE","value":"test"},{"name":"MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD","type":"
 
VALUE","value":"5secs"},{"name":"MESOS_FRAMEWORK_ID","type":"VALUE","value":"c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-"},{"name":"MESOS_HTTP_COMMAND_EXECUTOR","type":"VALUE","value":"0"},{"name":"MESOS_SANDBOX","type":"VALUE","value":"\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-\/executors\/test\/runs\/025c1d1a-6886-4b42-9313-0b1a411d5e39"},{"name":"MESOS_SLAVE_ID","type":"VALUE","value":"816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0"},{"name":"MESOS_SLAVE_PID","type":"VALUE","value":"slave(1)@192.168.178.20:5051"},{"name":"PATH","type":"VALUE","value":"\/usr\/local\/sbin:\/usr\/local\/bin:\/usr\/sbin:\/usr\/bin:\/sbin:\/bin"},{"name":"PWD","type":"VALUE","value":"\/private\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-\/executors\/test\/runs\/025c1d1a-6886-4b42-9313-0b1a411d5e39"},{"name":"SHLVL","type":"VALUE","value":"0"},{"name":"__CF_USER_TEXT_E
 
NCODING","type":"VALUE","value":"0x1F5:0x0:0x0"},{"name":"key1","type":"VALUE","value":"value1"},{"name":"key1","type":"VALUE","value":"value1"}]}}"
Forked command at 2376
Overwriting environment variable 'key1', original: 'value1', new: 'value1'
```

after applying this:
```
Received SUBSCRIBED event
Subscribed executor on lobomacpro2.fritz.box
Received LAUNCH event
Starting task test
Running /Users/till/Development/mesos-private/build/src/mesos-containerizer 
launch [***]
Forked command at 21513
Overwriting environment variable 'key1', original: 'value1', new: 'value1'
```


Thanks,

Till Toenshoff

Review Request 57763: Fixed environment overwrite warning to not leak possibly sensitive data.

2017-03-19 Thread Till Toenshoff 

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

Review request for mesos, Adam B, Benjamin Bannier, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/containerizer/mesos/launch.cpp 
8658525b00e78bed9227d6d400eae2cf25dd 


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


Testing
---

make check & functional testing...

```
./src/mesos-execute --name="test" --env='{"key1":"value1"}' --command='sleep 
1000' --master=127.0.0.1:5050
```

Within the contents of `stdout` before applying this:
```
Overwriting environment variable 'key1', original: 'value1', new: 'value1'
```

After applying this there is no mention of the actually duplicate but equally 
valued variable anymore. Note, this is before applying 
https://reviews.apache.org/r/57762.


Thanks,

Till Toenshoff

Review Request 57762: Fixed environment duplication in default executor.

2017-03-19 Thread Till Toenshoff 

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

Review request for mesos, Adam B, Benjamin Bannier, Greg Mann, Jie Yu, and 
Joseph Wu.


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


Repository: mesos


Description
---

see summary.


Diffs
-

  src/launcher/executor.cpp bd3c0cf6ae2329ed30d6ab4b2ab3e7a90a46 


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


Testing
---

make check + functional testing..

Before applying the patch;

Running 
```
$ ./src/mesos-execute --name="test" --env='{"key1":"value1"}' --command='sleep 
1000' --master=127.0.0.1:5050
```
will result in the following `stdout` sandbox file:
```
Received SUBSCRIBED event
Subscribed executor on lobomacpro2.fritz.box
Received LAUNCH event
Starting task test
/Users/till/Development/mesos-private/build/src/mesos-containerizer launch 
--help="false" 
--launch_info="{"command":{"environment":{"variables":[{"name":"key1","type":"VALUE","value":"value1"}]},"shell":true,"value":"sleep
 
1000"},"environment":{"variables":[{"name":"BIN_SH","type":"VALUE","value":"xpg4"},{"name":"DUALCASE","type":"VALUE","value":"1"},{"name":"DYLD_LIBRARY_PATH","type":"VALUE","value":"\/Users\/till\/Development\/mesos-private\/build\/src\/.libs"},{"name":"LIBPROCESS_PORT","type":"VALUE","value":"0"},{"name":"MESOS_AGENT_ENDPOINT","type":"VALUE","value":"192.168.178.20:5051"},{"name":"MESOS_CHECKPOINT","type":"VALUE","value":"0"},{"name":"MESOS_DIRECTORY","type":"VALUE","value":"\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-\/executors\/test\/runs\/025c1d1a-6886-4b42-9313-0b1a411d5e39"},{"name":"MESOS_EXECUTOR_ID","type":"VALUE","value":"test"},{"name":"MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD","type":"
 
VALUE","value":"5secs"},{"name":"MESOS_FRAMEWORK_ID","type":"VALUE","value":"c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-"},{"name":"MESOS_HTTP_COMMAND_EXECUTOR","type":"VALUE","value":"0"},{"name":"MESOS_SANDBOX","type":"VALUE","value":"\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-\/executors\/test\/runs\/025c1d1a-6886-4b42-9313-0b1a411d5e39"},{"name":"MESOS_SLAVE_ID","type":"VALUE","value":"816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0"},{"name":"MESOS_SLAVE_PID","type":"VALUE","value":"slave(1)@192.168.178.20:5051"},{"name":"PATH","type":"VALUE","value":"\/usr\/local\/sbin:\/usr\/local\/bin:\/usr\/sbin:\/usr\/bin:\/sbin:\/bin"},{"name":"PWD","type":"VALUE","value":"\/private\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/c84f9ee5-8a6b-4f6c-9fc0-154bceb3c615-\/executors\/test\/runs\/025c1d1a-6886-4b42-9313-0b1a411d5e39"},{"name":"SHLVL","type":"VALUE","value":"0"},{"name":"__CF_USER_TEXT_E
 
NCODING","type":"VALUE","value":"0x1F5:0x0:0x0"},{"name":"key1","type":"VALUE","value":"value1"},{"name":"key1","type":"VALUE","value":"value1"}]}}"
Forked command at 2376
Overwriting environment variable 'key1', original: 'value1', new: 'value1'
```

After applying the patch;

The resulting `stdout` sandbox file is as clean as it should be:
```
Received SUBSCRIBED event
Subscribed executor on lobomacpro2.fritz.box
Received LAUNCH event
Starting task test
/Users/till/Development/mesos-private/build/src/mesos-containerizer launch 
--help="false" 
--launch_info="{"command":{"environment":{"variables":[{"name":"key1","type":"VALUE","value":"value1"}]},"shell":true,"value":"sleep
 
1000"},"environment":{"variables":[{"name":"PATH","type":"VALUE","value":"\/usr\/local\/sbin:\/usr\/local\/bin:\/usr\/sbin:\/usr\/bin:\/sbin:\/bin"},{"name":"PWD","type":"VALUE","value":"\/private\/tmp\/mesos\/slaves\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0\/frameworks\/816619b6-f5ce-42d6-ad6b-2ef2001adc0a-\/executors\/test\/runs\/46f219a3-2772-4d21-89b8-85f1c1ba362e"},{"name":"MESOS_SLAVE_PID","type":"VALUE","value":"slave(1)@192.168.178.20:5051"},{"name":"MESOS_FRAMEWORK_ID","type":"VALUE","value":"816619b6-f5ce-42d6-ad6b-2ef2001adc0a-"},{"name":"MESOS_EXECUTOR_ID","type":"VALUE","value":"test"},{"name":"MESOS_CHECKPOINT","type":"VALUE","value":"0"},{"name":"MESOS_SLAVE_ID","type":"VALUE","value":"816619b6-f5ce-42d6-ad6b-2ef2001adc0a-S0"},{"name":"MESOS_S

Re: Review Request 57628: Added a utility function to get back non-loopback address on the host.

2017-03-19 Thread Jie Yu 

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




src/tests/utils.cpp
Lines 193 (patched)


Indentation?


- Jie Yu


On March 17, 2017, 5:12 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57628/
> ---
> 
> (Updated March 17, 2017, 5:12 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6022
> https://issues.apache.org/jira/browse/MESOS-6022
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a utility function to get back non-loopback address on the host.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.hpp cdbbc1c611bbcf2d6548f6b65fb08c6a4831b7a8 
>   src/tests/utils.cpp bc5c8abd6d0a70a092e1a54ea402463c599c15a7 
> 
> 
> Diff: https://reviews.apache.org/r/57628/diff/2/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter=CniIsolator*.*
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 57719: Fixed some coding style issues.

2017-03-19 Thread Jie Yu 

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


Ship it!




Ship It!

- Jie Yu


On March 17, 2017, 5:38 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57719/
> ---
> 
> (Updated March 17, 2017, 5:38 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6022
> https://issues.apache.org/jira/browse/MESOS-6022
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed some coding style issues.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.cpp bc5c8abd6d0a70a092e1a54ea402463c599c15a7 
> 
> 
> Diff: https://reviews.apache.org/r/57719/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 57739: Completed frameworks should be tracked based on source.

2017-03-19 Thread Jiang Yan Xu 

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



Can we add a test?


src/master/master.hpp
Lines 802-804 (patched)


This method can possbily be called with a combination of these args but in 
reality we only need one of the three to be set. The abstraction suggested 
below which allows you to overload the method with each type is better.



src/master/master.hpp
Lines 1763-1764 (original), 1770-1772 (patched)


I think it's cleaner if we can model it after `slaves.registered`:

```
struct Frameworks
{
  ...
  
  struct
  {
bool contains(const FrameworkID& slaveId) const;
bool contains(const process::UPID& slaveId) const;
bool contains(const HttpConnection& slaveId) const;
void put(Framework*);
const_iterator begin() const;
const_iterator end()   const;
...
  private:
BoundedHashMap streamIds;
  } completed;
} frameworks;
```



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


Keep the error message consistent: "Framework has been removed"


- Jiang Yan Xu


On March 17, 2017, 4:36 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57739/
> ---
> 
> (Updated March 17, 2017, 4:36 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7181
> https://issues.apache.org/jira/browse/MESOS-7181
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In addition to tracking completed frameworks based on framework id, we
> also keep track of the source (`streamId` for http connections, or
> `upid` for scheduler driver). This information is used to ensure that
> retransmitted SUBSCRIBEs from the same source are dropped irrespective
> of those frameworks are connected or not.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp d92c8adef79d997f255cf26ebd10ab0e87da8413 
>   src/master/master.cpp 43e9d26167c1f405329ea05224c22e7b8c65315f 
> 
> 
> Diff: https://reviews.apache.org/r/57739/diff/1/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>