| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-11 | |||
| 16:46:56 | mriedem | efried: i don't personally want it in config since i make the decision each time | |
| 16:48:51 | mriedem | melwitt: i want to say it wasn't in https://specs.openstack.org/openstack/nova-specs/specs/newton/implemented/deprecate-api-proxies.html because it's an operation on the server which is not deprecated | |
| 16:48:54 | mriedem | even if it's a proxy to neutron | |
| 16:49:10 | mriedem | but we're not very consistent about that b/c the floating IP things in https://specs.openstack.org/openstack/nova-specs/specs/pike/implemented/deprecate-multinic-proxy-api.html are similar | |
| 16:49:49 | melwitt | mriedem: yeah, was thinking similar. at first I was comparing to attach interface but that's different bc you can't do that without nova. whereas it looks like you could do the add/remove secgroup to port without nova | |
| 16:49:51 | mriedem | addFloatingIP and removeFloatingIP would be in the same group as add/removeSecurityGroup on a server | |
| 16:51:23 | melwitt | ok, yeah. just wanted to ask bc the patch was abandoned on that basis (the potential for deprecation). otherwise, someone said they had tested the patch and it worked to fix the concurrent update problem | |
| 18:04:54 | mriedem | efried: i'm +2 on gibi's big heal_allocations change now https://review.opendev.org/#/c/637955/35 | |
| 18:05:03 | efried | oh goodie | |
| 18:05:13 | mriedem | i'm sure you can just rubber stamp it with no effort | |
| 18:05:27 | efried | I'm sure | |
| 18:05:37 | efried | been waiting for you to quit hammering at it before looking again | |
| 18:05:42 | efried | I'm sure it's nigh unrecognizable now. | |
| 18:05:49 | mriedem | not since you last looked on | |
| 18:05:50 | mriedem | *no | |
| 18:06:01 | mriedem | more tests, | |
| 18:06:09 | mriedem | and the rollback stuff was changed | |
| 18:06:33 | mriedem | he first tries to update ports before allocations and if the port updates fail or allocation update fails the port updates are rolled back | |
| 18:07:42 | efried | okay, all of that sounds familiar. | |
| 18:07:48 | efried | I'll hit it now. | |
| 18:18:21 | mriedem | i'm working on a fup patch for my nits | |
| 18:21:42 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Follow up for "nova-manage: heal port allocations" https://review.opendev.org/670361 | |
| 18:35:17 | openstackgerrit | Merged openstack/nova master: Add host and hypervisor_hostname flag to create server https://review.opendev.org/645520 | |
| 19:50:45 | efried | mriedem: gonna need your help understanding a thing here... | |
| 19:51:22 | efried | is this a patch-style update or a replace? | |
| 19:51:22 | efried | neutron.update_port(uuid, body=body) | |
| 19:51:53 | efried | i.e. does it only change the fields that are present in the body? | |
| 20:01:38 | mriedem | artom: sean-k-mooney: dansmith: https://review.opendev.org/#/c/667177/10 | |
| 20:01:50 | mriedem | efried: replace | |
| 20:02:00 | artom | zomgies | |
| 20:02:15 | mriedem | efried: are you worried about overwriting something in the binding profile during the rollback from the time we originally got the port? | |
| 20:02:47 | efried | mriedem: Actually I'm worried about blowing away the whole content of the port *except* the binding:profile['allocation'] when we're trying to set that guy. | |
| 20:03:04 | mriedem | efried: wait, | |
| 20:03:28 | mriedem | efried: link me to the line | |
| 20:03:36 | efried | mriedem: yes, sec. | |
| 20:03:41 | mriedem | but we do port binding updates like this in the neutronv2/api code as well | |
| 20:04:50 | efried | mriedem: https://review.opendev.org/#/c/637955/35/nova/cmd/manage.py@1896 and continuation https://review.opendev.org/#/c/637955/35/nova/cmd/manage.py@1926 | |
| 20:05:19 | melwitt | I didn't think it's like placement where anything not in the payload will get "erased" like a replace | |
| 20:05:55 | efried | well, there's a bug somewhere then. Because either we blow away everything but the allocation on the initial pass, or we do nothing on the rollback. | |
| 20:06:25 | efried | but mriedem yes indeed, I see port updates in neutronv2/api.py which are similar in spirit. | |
| 20:07:49 | melwitt | https://developer.openstack.org/api-ref/network/v2/?expanded=update-port-detail#update-port | |
| 20:08:17 | efried | melwitt: I read that; are you seeing something in there that implies "partial update"? | |
| 20:08:26 | efried | (I admit I didn't scrutinize it) | |
| 20:08:40 | mriedem | efried: agree that https://review.opendev.org/#/c/637955/35/nova/cmd/manage.py@1896 looks like trouble | |
| 20:08:50 | mriedem | https://developer.openstack.org/api-ref/network/v2/index.html?expanded=update-port-detail#revisions | |
| 20:08:56 | mriedem | neutron does have revisions and an If-Match header | |
| 20:09:03 | mriedem | like generations in placement and etags | |
| 20:09:18 | mriedem | anyway, that's more about what i thought you were going to bring up | |
| 20:10:10 | efried | I guess I would sort of hope you're not effing out of band with whatever ports you're trying to heal | |
| 20:10:53 | efried | but yeah, it would be nice to handle generations too. | |
| 20:11:03 | mriedem | looking at where this is used https://github.com/openstack/nova/blob/ff0feed25d56c8ccd2298d5b5b82e636880fa986/nova/network/neutronv2/api.py#L92 | |
| 20:11:12 | mriedem | it looks like we only ever modify the full binding:profile and then send back our updates | |
| 20:11:15 | mriedem | not just pieces of it | |
| 20:12:04 | efried | so like, update_port knows to replace the thing for the top-level key you're sending, but doesn't go deeper? | |
| 20:12:51 | mriedem | i would assume that the neutron API takes the value for whatever key in the PUT /ports/{port_id} request and replaces the value, not parts of the value | |
| 20:12:52 | efried | in which case the second one (rollback) is fine, but the first one is broken? | |
| 20:12:58 | mriedem | it would assume so | |
| 20:13:25 | efried | "whatever key in the PUT request" -- the key that's in there is 'port'. | |
| 20:13:35 | efried | you mean the one under 'port'? | |
| 20:13:38 | mriedem | yes | |
| 20:13:40 | efried | okay | |
| 20:13:42 | mriedem | keys in the port resource body | |
| 20:14:00 | efried | okay, so the first one is borked. I shall -1 accordingly. Thanks. | |
| 20:21:27 | cdent | all this fiddling suggests some kind of heal integration test would be a nice thing | |
| 20:21:38 | cdent | if one could be conceived | |
| 20:22:19 | efried | I think mriedem wrote one recently. Though I sort of doubt it went anywhere near this deep. | |
| 20:22:27 | mriedem | gibi has a patch for that eventually | |
| 20:22:34 | mriedem | fiddle me timbers | |
| 20:22:43 | efried | cdent: https://review.opendev.org/#/c/667994/ | |
| 20:22:50 | mriedem | https://review.opendev.org/#/c/669879/ | |
| 20:22:52 | mriedem | drats | |
| 20:23:26 | efried | different patches | |
| 20:23:42 | cdent | yeah, the depth is apparently not quite depthy? | |
| 20:23:42 | mriedem | oh right, gibi's builds on mine | |
| 20:24:00 | efried | cdent: look at gibi's followon, it does a little better | |
| 20:24:03 | mriedem | i didn't dig into his actual test changes | |
| 20:24:39 | mriedem | if i can get https://review.opendev.org/#/c/670196/ then eventually we could break apart the post_test_hook mega script into a smaller series of exercises | |
| 20:24:43 | mriedem | so it's more manageable | |
| 20:25:25 | cdent | that would be nice | |
| 20:25:45 | cdent | gibi's follow does look like an improvement. | |
| 20:26:17 | efried | good call cdent. I've commented accordingly in gibi's patch. | |
| 20:27:14 | cdent | i've been on the hook this week for a bunch of internal bug handling, and omg, i can't even | |
| 20:27:21 | cdent | everything is horrible, everywhere | |
| 20:27:33 | cdent | i had a nice afternoon fishing, though | |
| 20:27:49 | efried | like, fishing in the water for fish? | |
| 20:28:04 | efried | or something job-related that's virtually like fishing? | |
| 20:28:15 | efried | (i.e. excruciatingly boring and unrewarding) | |
| 20:30:20 | cdent | https://tank-binaries.s3.amazonaws.com/08e47f8678ed462d95e5a273d927c77a.jpe | |
| 20:30:45 | cdent | sister's sister's family down for the week, invited for some deep sea fishing | |
| 20:30:57 | cdent | that was my dinner | |
| 20:34:15 | melwitt | efried: sorry, I missed your earlier message bc I guess my znc server disconnected.. I was referring to the wording like, "replaces the fixed_ip attribute when you specify it in the request body" which I took to mean it will replace fixed_ip if you specify it in the body but will not erase the fixed_ip if the field is not specified in the request body | |
| 20:56:19 | openstackgerrit | Eric Fried proposed openstack/os-resource-classes master: Propose FPGA and PGPU resource classes https://review.opendev.org/657464 | |
| 21:01:14 | efried | melwitt: okay, cool, thanks for pointing that out. | |
| 21:02:23 | efried | ...though that makes it by no means obvious that similar behavior applies to the whole API. | |
| 21:03:15 | melwitt | yeah, I find the doc not comprehensive. was just guessing based on those bits | |
| 21:03:41 | melwitt | also, I'm not advocating guessing. was just thinking aloud | |
| 21:04:31 | efried | Ima open a doc bug for that | |
| 21:05:45 | efried | I just skimmed the front matter too, and there doesn't appear to be anything there. | |
| 21:07:37 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Use SDK instead of ironicclient for node.list https://review.opendev.org/656027 | |
| 21:07:37 | openstackgerrit | Dustin Cowles proposed openstack/nova master: Introduces SDK to IronicDriver and uses for node.get https://review.opendev.org/642899 | |