You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2015/08/24 21:09:33 UTC

Re: Review Request 37364: Maintenance Primitives: Adds an endpoint for retrieving the maintenance status for machines.

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

(Updated Aug. 24, 2015, 12:09 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Rebased and updated diff.  No other changes.


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


Repository: mesos


Description (updated)
-------

Endpoint: /maintenance.status

Returns a JSON object like:
```
{
  "draining": [
    { "hostname": "foo", "ip": "0.0.0.1" },
    { "hostname": "bar", "ip": "0.0.0.2" },
  ],
  "deactivated": [
    { "hostname": "baz", "ip": "0.0.0.3" },
  ]
}
```


Diffs (updated)
-----

  src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 
  src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/master_maintenance_tests.cpp PRE-CREATION 

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


Testing
-------

`make check`

New Tests:
  MasterMaintenanceTest.FailToUnscheduleDeactivatedMachines
    Extra test case for the /maintenance.schedule endpoint, which requires all three endpoints to work.
  MasterMaintenanceTest.MachineStatus
    Schedules, starts, and stops maintenance.  Checks machine statuses after each step.


Thanks,

Joseph Wu


Re: Review Request 37364: Maintenance Primitives: Adds an endpoint for retrieving the maintenance status for machines.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37364/#review96315
-----------------------------------------------------------

Ship it!


Ship It!

- Guangya Liu


On Aug. 24, 2015, 7:09 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37364/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 7:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2067
>     https://issues.apache.org/jira/browse/MESOS-2067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Endpoint: /maintenance.status
> 
> Returns a JSON object like:
> ```
> {
>   "draining": [
>     { "hostname": "foo", "ip": "0.0.0.1" },
>     { "hostname": "bar", "ip": "0.0.0.2" },
>   ],
>   "deactivated": [
>     { "hostname": "baz", "ip": "0.0.0.3" },
>   ]
> }
> ```
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 
>   src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/master_maintenance_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37364/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> New Tests:
>   MasterMaintenanceTest.FailToUnscheduleDeactivatedMachines
>     Extra test case for the /maintenance.schedule endpoint, which requires all three endpoints to work.
>   MasterMaintenanceTest.MachineStatus
>     Schedules, starts, and stops maintenance.  Checks machine statuses after each step.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 37364: Maintenance Primitives: Adds an endpoint for retrieving the maintenance status for machines.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37364/#review96835
-----------------------------------------------------------



src/master/http.cpp (lines 1658 - 1679)
<https://reviews.apache.org/r/37364/#comment152519>

    Do we have a protobuf to describe this so we can just jsonify that?



src/master/http.cpp (line 1664)
<https://reviews.apache.org/r/37364/#comment152518>

    Space between 'switch' and '(' please.



src/tests/master_maintenance_tests.cpp (line 449)
<https://reviews.apache.org/r/37364/#comment152520>

    Wrap the first argument for all of these as well, otherwise please line up the second argument with the first argument, thanks!
    
    Also, an alternative suggestion here, type out the JSON yourself into a string that you parse and then compare to the parsed 'response.get().body' (so you get semantic equality) and that way you can check the entire response rather than just som e subparts. Make sense?


- Benjamin Hindman


On Aug. 26, 2015, 10:44 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37364/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 10:44 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2067
>     https://issues.apache.org/jira/browse/MESOS-2067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Endpoint: /maintenance.status
> 
> Returns a JSON object like:
> ```
> {
>   "draining": [
>     { "hostname": "foo", "ip": "0.0.0.1" },
>     { "hostname": "bar", "ip": "0.0.0.2" },
>   ],
>   "deactivated": [
>     { "hostname": "baz", "ip": "0.0.0.3" },
>   ]
> }
> ```
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 
>   src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/tests/master_maintenance_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37364/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> New Tests:
>   MasterMaintenanceTest.FailToUnscheduleDeactivatedMachines
>     Extra test case for the /maintenance.schedule endpoint, which requires all three endpoints to work.
>   MasterMaintenanceTest.MachineStatus
>     Schedules, starts, and stops maintenance.  Checks machine statuses after each step.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 37364: Maintenance Primitives: Adds an endpoint for retrieving the maintenance status for machines.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37364/
-----------------------------------------------------------

(Updated Aug. 28, 2015, 5:04 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Rebased and updated to match changes in previous reviews.


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


Repository: mesos


Description (updated)
-------

Endpoint: /maintenance/status

Also adds a protobuf for the purpose of serialization.


Diffs (updated)
-----

  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 
  src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/master_maintenance_tests.cpp PRE-CREATION 

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


Testing
-------

`make check`

New Tests:
  MasterMaintenanceTest.FailToUnscheduleDeactivatedMachines
    Extra test case for the /maintenance.schedule endpoint, which requires all three endpoints to work.
  MasterMaintenanceTest.MachineStatus
    Schedules, starts, and stops maintenance.  Checks machine statuses after each step.


Thanks,

Joseph Wu


Re: Review Request 37364: Maintenance Primitives: Adds an endpoint for retrieving the maintenance status for machines.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37364/
-----------------------------------------------------------

(Updated Aug. 26, 2015, 3:44 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Updated to match comments and changes from previous reviews.


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


Repository: mesos


Description
-------

Endpoint: /maintenance.status

Returns a JSON object like:
```
{
  "draining": [
    { "hostname": "foo", "ip": "0.0.0.1" },
    { "hostname": "bar", "ip": "0.0.0.2" },
  ],
  "deactivated": [
    { "hostname": "baz", "ip": "0.0.0.3" },
  ]
}
```


Diffs (updated)
-----

  src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 
  src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/master_maintenance_tests.cpp PRE-CREATION 

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


Testing
-------

`make check`

New Tests:
  MasterMaintenanceTest.FailToUnscheduleDeactivatedMachines
    Extra test case for the /maintenance.schedule endpoint, which requires all three endpoints to work.
  MasterMaintenanceTest.MachineStatus
    Schedules, starts, and stops maintenance.  Checks machine statuses after each step.


Thanks,

Joseph Wu


Re: Review Request 37364: Maintenance Primitives: Adds an endpoint for retrieving the maintenance status for machines.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37364/
-----------------------------------------------------------

(Updated Aug. 25, 2015, 10:07 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Adjust some spacing in the tests (correcting the style).


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


Repository: mesos


Description
-------

Endpoint: /maintenance.status

Returns a JSON object like:
```
{
  "draining": [
    { "hostname": "foo", "ip": "0.0.0.1" },
    { "hostname": "bar", "ip": "0.0.0.2" },
  ],
  "deactivated": [
    { "hostname": "baz", "ip": "0.0.0.3" },
  ]
}
```


Diffs (updated)
-----

  src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 
  src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
  src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
  src/tests/master_maintenance_tests.cpp PRE-CREATION 

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


Testing
-------

`make check`

New Tests:
  MasterMaintenanceTest.FailToUnscheduleDeactivatedMachines
    Extra test case for the /maintenance.schedule endpoint, which requires all three endpoints to work.
  MasterMaintenanceTest.MachineStatus
    Schedules, starts, and stops maintenance.  Checks machine statuses after each step.


Thanks,

Joseph Wu