Re: Review Request 58689: MESOS-7323: Made `addSlave` not activate any frameworks.

[Michael Park]

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

(Updated April 27, 2017, 8:13 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.


Changes
---

Moved the test from `upgrade_tests.cpp` to `master_tests.cpp`.


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


Repository: mesos


Description
---

MESOS-7323: Made `addSlave` not activate any frameworks.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
  src/tests/master_tests.cpp d1828eb42e0aedc9330c3786bbd9bb63aa42a64e 
  src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 


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

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


Testing
---

`make check` and added a test.


Thanks,

Michael Park

Re: Review Request 58689: MESOS-7323: Made `addSlave` not activate any frameworks.

[Michael Park]

> On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote:
> > src/tests/upgrade_tests.cpp
> > Lines 537-539 (patched)
> > 
> >
> > This comment seems to be inconsistent with what the test is doing?
> > 
> > Also, this doesn't look like an upgrade test, should we make it a 
> > master test?
> 
> Michael Park wrote:
> Updated the comment to more accurately reflect the test.
> 
> I was considering this to be a kind of an upgrade test, in the sense that
> we're upgrading the framework from one config to another.
> 
> I'm not sure if we have some clear criteria for what qualifies as an 
> upgrade test.
> 
> But I'm also fine with this living in master test.
> 
> Benjamin Mahler wrote:
> Well, my intention was described at the top of the file:
> 
> ```
> // This file contains upgrade integration tests. Note that tests
> // in this file can only "spoof" an old version of a component,
> // since these run against the current version of the repository.
> // The long term plan for integration tests is to checkout
> // different releases of the repository and run components from
> // different releases against each other.
> ```

Ah, sorry about that. Moved to `master_tests.cpp`


- Michael


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


On April 26, 2017, 7:54 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58689/
> ---
> 
> (Updated April 26, 2017, 7:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-7323: Made `addSlave` not activate any frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
>   src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 
> 
> 
> Diff: https://reviews.apache.org/r/58689/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` and added a test.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58689: MESOS-7323: Made `addSlave` not activate any frameworks.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On April 27, 2017, 2:54 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58689/
> ---
> 
> (Updated April 27, 2017, 2:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-7323: Made `addSlave` not activate any frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
>   src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 
> 
> 
> Diff: https://reviews.apache.org/r/58689/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` and added a test.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58689: MESOS-7323: Made `addSlave` not activate any frameworks.

[Benjamin Mahler]

> On April 26, 2017, 12:59 a.m., Benjamin Mahler wrote:
> > src/tests/upgrade_tests.cpp
> > Lines 537-539 (patched)
> > 
> >
> > This comment seems to be inconsistent with what the test is doing?
> > 
> > Also, this doesn't look like an upgrade test, should we make it a 
> > master test?
> 
> Michael Park wrote:
> Updated the comment to more accurately reflect the test.
> 
> I was considering this to be a kind of an upgrade test, in the sense that
> we're upgrading the framework from one config to another.
> 
> I'm not sure if we have some clear criteria for what qualifies as an 
> upgrade test.
> 
> But I'm also fine with this living in master test.

Well, my intention was described at the top of the file:

```
// This file contains upgrade integration tests. Note that tests
// in this file can only "spoof" an old version of a component,
// since these run against the current version of the repository.
// The long term plan for integration tests is to checkout
// different releases of the repository and run components from
// different releases against each other.
```


- Benjamin


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


On April 27, 2017, 2:54 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58689/
> ---
> 
> (Updated April 27, 2017, 2:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-7323: Made `addSlave` not activate any frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
>   src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 
> 
> 
> Diff: https://reviews.apache.org/r/58689/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` and added a test.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58689: MESOS-7323: Made `addSlave` not activate any frameworks.

[Michael Park]

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

(Updated April 26, 2017, 7:54 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.


Changes
---

Addressed the rest of bmahler's comments regarding the test.


Summary (updated)
-

MESOS-7323: Made `addSlave` not activate any frameworks.


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


Repository: mesos


Description (updated)
---

MESOS-7323: Made `addSlave` not activate any frameworks.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
  src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 


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

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


Testing
---

`make check` and added a test.


Thanks,

Michael Park