x
/
openstack-resource-agents
Code Issues Proposed changes

Update patch set 3 

Patch Set 3: Code-Review-1

(15 comments)

This is great!!  Thank you so much :-)  It's *really* close to what I imagined.  Just some minor details to sort out.

Andrew, yes there is a plan, and you helped design it ;-)  The plan is:

- A primitive for this new nova-host-alerter OCF RA would be introduced into existing RH/SUSE clusters alongside the existing NovaEvacuate, with target-role=Stopped.

- masakari would be installed on the cloud, and tested with a spare compute node to make sure it works OK when notifications are manually triggered.

- Then migration is super easy: just stop NovaEvacuate and start nova-host-alerter :-)

- Later, at your leisure, remove the NovaEvacuate primitive.

Hopefully https://aspiers.github.io/openstack-day-israel-2017-compute-ha/#/nova-host-alerter will help refresh your memory.

Patch-set: 3
Label: Code-Review=-1
This commit is contained in:
Gerrit User 2394 2018-03-29 14:35:57 +00:00 committed by Gerrit Code Review
parent 8bd108e0e6
commit d6dd4f9940
1 changed files with 289 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

289
c0d921672339e85231ecbc749149c72858f0c4bb Normal file
View File

@ -0,0 +1,289 @@
{
"comments": [
{
"key": {
"uuid": "bf659307_14621aaf",
"filename": "/COMMIT_MSG",
"patchSetId": 3
},
"lineNbr": 16,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "Well, it\u0027s partially implemented ;-)",
"range": {
"startLine": 16,
"startChar": 0,
"endLine": 16,
"endChar": 49
},
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_cff08f00",
"filename": "/COMMIT_MSG",
"patchSetId": 3
},
"lineNbr": 27,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "Also worth mentioning the architecture diagram:\n\nhttps://aspiers.github.io/openstack-day-israel-2017-compute-ha/#/nova-host-alerter",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_546bb283",
"filename": "/COMMIT_MSG",
"patchSetId": 3
},
"lineNbr": 29,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "Please mention this one too, it\u0027s more relevant:\n\nhttps://github.com/openstack/openstack-resource-agents-specs/blob/master/specs/newton/approved/newton-instance-ha-host-monitoring-spec.rst",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_ef86b313",
"filename": "ocf/masakari_driver.py",
"patchSetId": 3
},
"lineNbr": 168,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "As noted in nova-host-alerter, I think we need this driver to handle multiple hosts at once. So maybe it could do something like:\n\n hosts \u003d sys.stdin.read().splitlines()\n masakari_driver \u003d MasakariDriver()\n for host in hosts:\n masakari_driver.send_notification(host)",
"range": {
"startLine": 167,
"startChar": 0,
"endLine": 168,
"endChar": 39
},
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_74fd96af",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 5,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "I think it should be possible to get rid of this from the review, based on the suggestions I make below.",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_7416f6f3",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 29,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "Just write \"Currently only masakari is supported\".",
"range": {
"startLine": 29,
"startChar": 32,
"endLine": 29,
"endChar": 79
},
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_f4168655",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 58,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "This should not call the monitor code, because then \u0027start\u0027 would actually invoke the driver code even before it\u0027s officially started. Better to copy NovaEvacuate\u0027s approach here:\n\nhttps://git.openstack.org/cgit/openstack/openstack-resource-agents/tree/ocf/NovaEvacuate?id\u003d42bb0c53e3c21550bc37bc63e0f0b1f59a2be000#n163",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_4f1a3f6f",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 70,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "This should not call the monitor code, because then \u0027stop\u0027 would actually invoke the driver code as part of shutdown, and we need \u0027stop\u0027 to be quick to avoid it exceeding the stop timeout which would cause fencing. Better to copy NovaEvacuate\u0027s approach here:\n\nhttps://git.openstack.org/cgit/openstack/openstack-resource-agents/tree/ocf/NovaEvacuate?id\u003d42bb0c53e3c21550bc37bc63e0f0b1f59a2be000#n158",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_74a6f655",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 86,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "Let\u0027s call this nova_host_alerter_monitor, because the \u0027status\u0027 action is deprecated.",
"range": {
"startLine": 86,
"startChar": 0,
"endLine": 86,
"endChar": 24
},
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_34bf1eb1",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 93,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "This needs to check the attributes in Pacemaker\u0027s attrd, via attrd_updater. Here is how we do it in the old architecture:\n\nhttps://git.openstack.org/cgit/openstack/openstack-resource-agents/tree/ocf/NovaEvacuate?id\u003d42bb0c53e3c21550bc37bc63e0f0b1f59a2be000#n254\n\nFrom this you can get a list of *all* nodes for which we need to notify masakari.",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_0f0767cc",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 102,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "As per above, I think we should send all hosts to a single invocation of $MASAKARI_DRIVER. Maybe it would be cleaner to send them via stdin rather than as arguments?",
"range": {
"startLine": 101,
"startChar": 0,
"endLine": 102,
"endChar": 52
},
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_94e86ae2",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 111,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "I think you can remove this one until we support it.",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_74c4b65f",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 115,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "This one should result in an error, e.g.\n\n ocf_log err \"Driver type \u0027$OCF_RESKEY_driver\u0027 is not supported.\"\n return $OCF_ERR_CONFIGURED",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_747b56bd",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 131,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "\u0027status\u0027 is deprecated so we don\u0027t actually need it.",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
},
{
"key": {
"uuid": "bf659307_b4f80e7b",
"filename": "ocf/nova-host-alerter",
"patchSetId": 3
},
"lineNbr": 136,
"author": {
"id": 2394
},
"writtenOn": "2018-03-29T14:35:57Z",
"side": 1,
"message": "Let\u0027s call this nova_host_alerter_monitor, because the \u0027status\u0027 action is deprecated.",
"revId": "c0d921672339e85231ecbc749149c72858f0c4bb",
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
"unresolved": false
}
]
}