Re: Review Request 68307: Added a test for master's handling of stale authentication requests.

2018-08-15 Thread Alexander Rojas 

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


Ship it!




I would prefer another wording of the description but the code itself looks 
good to me, how about:

> This test ensures that when a master receives a new authentication 
> request for an scheduler which already has initiated authentication,
> the master discards the the old one and proceeds with the new one.
>
> Since the master does not distinguish between agents or schedulers
> during authentication, this tests is sufficient to cover both cases.

- Alexander Rojas


On Aug. 14, 2018, 12:24 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68307/
> ---
> 
> (Updated Aug. 14, 2018, 12:24 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9144
> https://issues.apache.org/jira/browse/MESOS-9144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that when the master sees a new authentication
> request for a particular agent or scheduler (we just test the
> scheduler case is tested here since the master does not distinguish),
> the master will discard the old one and proceed with the new one.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 8007f420b3a912d7eff1fa5faa7d8502eb3e9115 
>   src/internal/evolve.hpp e792ff591eff537e2a4661afe08795f00eb35843 
>   src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
>   src/master/metrics.hpp df28a486ead68421970723060850de3ac32e68a7 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68307/diff/2/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68307: Added a test for master's handling of stale authentication requests.

2018-08-15 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [68325, 68350, 68326, 68327, 68305, 68306, 68307]

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 Aug. 13, 2018, 10:24 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68307/
> ---
> 
> (Updated Aug. 13, 2018, 10:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9144
> https://issues.apache.org/jira/browse/MESOS-9144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that when the master sees a new authentication
> request for a particular agent or scheduler (we just test the
> scheduler case is tested here since the master does not distinguish),
> the master will discard the old one and proceed with the new one.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 8007f420b3a912d7eff1fa5faa7d8502eb3e9115 
>   src/internal/evolve.hpp e792ff591eff537e2a4661afe08795f00eb35843 
>   src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
>   src/master/metrics.hpp df28a486ead68421970723060850de3ac32e68a7 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68307/diff/2/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68307: Added a test for master's handling of stale authentication requests.

2018-08-14 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 68307 was successfully built and tested.

Reviews applied: `['68325', '68350', '68326', '68327', '68305', '68306', 
'68307']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2152/mesos-review-68307

- Mesos Reviewbot Windows


On Aug. 13, 2018, 10:24 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68307/
> ---
> 
> (Updated Aug. 13, 2018, 10:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9144
> https://issues.apache.org/jira/browse/MESOS-9144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that when the master sees a new authentication
> request for a particular agent or scheduler (we just test the
> scheduler case is tested here since the master does not distinguish),
> the master will discard the old one and proceed with the new one.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 8007f420b3a912d7eff1fa5faa7d8502eb3e9115 
>   src/internal/evolve.hpp e792ff591eff537e2a4661afe08795f00eb35843 
>   src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
>   src/master/metrics.hpp df28a486ead68421970723060850de3ac32e68a7 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68307/diff/2/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68307: Added a test for master's handling of stale authentication requests.

2018-08-14 Thread Benjamin Mahler 


> On Aug. 14, 2018, 11:03 p.m., Gastón Kleiman wrote:
> > src/tests/authentication_tests.cpp
> > Lines 424 (patched)
> > 
> >
> > Nit: it would be more readable to use: 
> > `DROP_PROTOBUF(AuthenticationStepMessage(), _, _)`?

That's indeed what I originally used, but unfortunately I cannot use it if I 
want to capture the message's to and from pids.


- Benjamin


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


On Aug. 13, 2018, 10:24 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68307/
> ---
> 
> (Updated Aug. 13, 2018, 10:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9144
> https://issues.apache.org/jira/browse/MESOS-9144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that when the master sees a new authentication
> request for a particular agent or scheduler (we just test the
> scheduler case is tested here since the master does not distinguish),
> the master will discard the old one and proceed with the new one.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 8007f420b3a912d7eff1fa5faa7d8502eb3e9115 
>   src/internal/evolve.hpp e792ff591eff537e2a4661afe08795f00eb35843 
>   src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
>   src/master/metrics.hpp df28a486ead68421970723060850de3ac32e68a7 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68307/diff/1/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68307: Added a test for master's handling of stale authentication requests.

2018-08-14 Thread Gastón Kleiman 

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


Fix it, then Ship it!





src/tests/authentication_tests.cpp
Lines 424 (patched)


Nit: it would be more readable to use: 
`DROP_PROTOBUF(AuthenticationStepMessage(), _, _)`?


- Gastón Kleiman


On Aug. 13, 2018, 3:24 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68307/
> ---
> 
> (Updated Aug. 13, 2018, 3:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9144
> https://issues.apache.org/jira/browse/MESOS-9144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that when the master sees a new authentication
> request for a particular agent or scheduler (we just test the
> scheduler case is tested here since the master does not distinguish),
> the master will discard the old one and proceed with the new one.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 8007f420b3a912d7eff1fa5faa7d8502eb3e9115 
>   src/internal/evolve.hpp e792ff591eff537e2a4661afe08795f00eb35843 
>   src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
>   src/master/metrics.hpp df28a486ead68421970723060850de3ac32e68a7 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68307/diff/1/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68307: Added a test for master's handling of stale authentication requests.

2018-08-13 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 68307 was successfully built and tested.

Reviews applied: `['68325', '68326', '68327', '68305', '68306', '68307']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2144/mesos-review-68307

- Mesos Reviewbot Windows


On Aug. 13, 2018, 10:24 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68307/
> ---
> 
> (Updated Aug. 13, 2018, 10:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gastón Kleiman, Meng Zhu, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-9144
> https://issues.apache.org/jira/browse/MESOS-9144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that when the master sees a new authentication
> request for a particular agent or scheduler (we just test the
> scheduler case is tested here since the master does not distinguish),
> the master will discard the old one and proceed with the new one.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 8007f420b3a912d7eff1fa5faa7d8502eb3e9115 
>   src/internal/evolve.hpp e792ff591eff537e2a4661afe08795f00eb35843 
>   src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
>   src/master/metrics.hpp df28a486ead68421970723060850de3ac32e68a7 
>   src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 
> 
> 
> Diff: https://reviews.apache.org/r/68307/diff/1/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 68307: Added a test for master's handling of stale authentication requests.

2018-08-13 Thread Benjamin Mahler 

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

Review request for mesos.


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


Repository: mesos


Description
---

This test ensures that when the master sees a new authentication
request for a particular agent or scheduler (we just test the
scheduler case is tested here since the master does not distinguish),
the master will discard the old one and proceed with the new one.


Diffs
-

  src/internal/devolve.hpp 8007f420b3a912d7eff1fa5faa7d8502eb3e9115 
  src/internal/evolve.hpp e792ff591eff537e2a4661afe08795f00eb35843 
  src/master/master.hpp 209b998db8d2bad7a3812df44f0939458f48eb11 
  src/master/metrics.hpp df28a486ead68421970723060850de3ac32e68a7 
  src/tests/authentication_tests.cpp c9a8f85951a50e278ae509f4efa7105755015ce9 


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


Testing
---

Ran in repetition.


Thanks,

Benjamin Mahler