| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-02-05 | |||
| 14:49:17 | bauzas | again, L62 is very confusing | |
| 14:49:28 | efried | The etherpad is only talking about what we do for NUMA-aware hosts | |
| 14:49:46 | efried | and yeah, the etherpad is confusing, so we should really write it more cleanly, say, in a spec :P | |
| 14:49:47 | bauzas | there are gaps then that I need to provide | |
| 14:50:03 | bauzas | ok, then we need to trigger exactly when asking for this trait | |
| 14:50:33 | stephenfin | dansmith: Not sure I get you, so apologies if I repeat stuff you already know | |
| 14:50:40 | efried | bauzas: All of the stuff in the etherpad is triggered under the same condition: if the flavor asks for a numa topo. | |
| 14:50:45 | stephenfin | if the extension is present, we'd expect a response like this http://paste.openstack.org/show/789164/ | |
| 14:51:06 | stephenfin | If that's not enabled, the port_details field isn't present so we add it manually | |
| 14:51:23 | dansmith | stephenfin: yes, understand, it's the "adding it manually" part I'm talking about | |
| 14:51:27 | dansmith | stephenfin: but let me just stop you for a sec | |
| 14:51:40 | efried | bauzas: there are things other than PAGE_SIZE_* that make sure you always & only land on a NUMA host. For instance, the NUMA_ROOT trait. | |
| 14:51:54 | dansmith | stephenfin: I think the problem is in a comment by you on the gerrit review, not the code as it is now, if i'm reading the api-ref correctly | |
| 14:52:00 | bauzas | efried: correct, but I need to think about this | |
| 14:52:34 | dansmith | stephenfin: are we able to test this with an old or duly configured neutron (i.e. without that extension) to prove it works? other than you manually with a devstack? | |
| 14:52:35 | efried | okay. Happy to help. (I have a call at the top of the hour FYI.) | |
| 14:52:54 | bauzas | cool | |
| 14:53:14 | stephenfin | dansmith: Yeah, let me whip up a functional test real quick | |
| 14:53:37 | stephenfin | this is what I expect our "fake" response to look like with this change, btw http://paste.openstack.org/show/789165/ | |
| 14:53:50 | stephenfin | we only return the list, stripping the outer container, of course | |
| 14:54:18 | dansmith | stephenfin: yeah understand that's the desired output of your function | |
| 14:54:33 | dansmith | stephenfin: a functional test isn't going to tell us anything if you're faking the response from neutron I think | |
| 14:55:34 | stephenfin | It should, assuming our mock of the 'list_ports' API is correct | |
| 14:55:41 | stephenfin | which I really hope it is since we use it everywhere | |
| 14:55:43 | stephenfin | Could we do a depends-on from the affected neutron project(s)? | |
| 14:55:51 | dansmith | oh, maybe I misunderstood you.. stephenfin when you said "Oh, yes, that exactly. This should return something like:" did you mean "this whole function should return..." or did you mean "list_ports should return" | |
| 14:56:59 | stephenfin | Doh /o\ "This" = this function, not "list_ports" | |
| 14:57:19 | dansmith | okay, I had quoted and was asking about list_ports originally, so I thought you were giving me the output of list_ports | |
| 14:57:29 | stephenfin | Yup, my mistake. Sorry | |
| 14:57:38 | dansmith | this is why I was saying I think the comment is the problem, because api-ref makes it look like your code is correct | |
| 14:58:28 | stephenfin | Yeah, per api-ref 'list_ports' will return something like https://docs.openstack.org/api-ref/network/v2/?expanded=list-ports-detail#list-ports of course | |
| 14:58:55 | dansmith | yeah when I first asked, I was expecting a detail=True to get the detailed listing, which I think is how ours works | |
| 15:00:02 | stephenfin | Yup, I don't think we need that since we don't need the detailed view. 'device_id' is the only thing we do care about and that's always there | |
| 15:00:32 | dansmith | you mean for our current usage I guess | |
| 15:00:36 | stephenfin | yeah | |
| 15:00:40 | stephenfin | if the extension is enabled and 'port_details' _is_ present, we'd only be getting a summary view anyway | |
| 15:00:59 | dansmith | if we only put device_id in there, then someday someone could expect port_details to be full and not understand why it's not | |
| 15:01:24 | stephenfin | we could, but given these are only used by the deprecated network API proxies, I figured the risk was low | |
| 15:01:26 | dansmith | I'm assuming this is only a neutron-goes-first upgrade scenario and we'll never hit this in the future because someone turned off this extension | |
| 15:01:37 | dansmith | right | |
| 15:02:20 | stephenfin | I'm under the impression that the networking backends choose their extensions or something | |
| 15:02:30 | stephenfin | because ralonsoh referred to OVN specifically | |
| 15:02:31 | dansmith | really? | |
| 15:02:48 | stephenfin | and we didn't see this blow up in our gate, which I guess is using ml2-ovs or ml2-lb | |
| 15:02:52 | ralonsoh | stephenfin, yes, this is failing in OVN | |
| 15:03:27 | ralonsoh | but this could happen in other situations, luckily we found it in the CI | |
| 15:03:37 | stephenfin | ralonsoh: am I correct in thinking that's because networking-ovn needs to add support for this extension? | |
| 15:03:52 | ralonsoh | stephenfin, it can... but this is not a requirement | |
| 15:03:53 | stephenfin | well, what was networking-ovn and is now in the core | |
| 15:04:12 | stephenfin | right, and ml2-ovs _does_ have it? | |
| 15:04:15 | ralonsoh | yes | |
| 15:04:19 | stephenfin | dansmith: ^ | |
| 15:04:33 | dansmith | yikes :/ | |
| 15:05:09 | stephenfin | so I should probably bug lucasagomes or someone to go add that for us at some point | |
| 15:05:20 | stephenfin | can I just say neutron's extension model is weird :) | |
| 15:05:20 | ralonsoh | stephenfin, I can do it | |
| 15:05:25 | stephenfin | not bad. just weird | |
| 15:05:37 | ralonsoh | but as commented, this is not mandatory | |
| 15:05:50 | ralonsoh | https://github.com/openstack/openstacksdk/blob/master/openstack/network/v2/floating_ip.py#L60-L64 | |
| 15:06:17 | stephenfin | ralonsoh: No huge panic. Again, this is only used by deprecated networking proxy APIs in nova, which we'd be hoping no one would be using any more | |
| 15:06:24 | stephenfin | but then again, OVN is the future so... | |
| 15:25:32 | sean-k-mooney | we could run a ovn job on nova and have it trigger on any cnages to nova/network subtree | |
| 15:25:52 | sean-k-mooney | we can just grab the neutorn one and set a filter on ther files | |
| 15:26:28 | sean-k-mooney | want me to submit a patch for that? | |
| 15:27:36 | sean-k-mooney | by the way im not sure we need the port detail form the floating ip. we need the port detail from the port that has the floating ip but that is different | |
| 15:28:18 | sean-k-mooney | stephenfin: ralonsoh ^ i have been looking at other things so im not fully in sync with what the patch/code is doing and your conversation | |
| 15:28:55 | sean-k-mooney | but the only thing nova uses is the port detail form the neutron port itself | |
| 15:29:24 | ralonsoh | sean-k-mooney, I've submitted a patch to add this extension in OVN | |
| 15:29:25 | sean-k-mooney | that will always be populated if the port is bound | |
| 15:29:34 | ralonsoh | https://review.opendev.org/#/c/705982/ | |
| 15:29:57 | sean-k-mooney | ralonsoh: sure but nova never need that info and enduser should not be relying on it | |
| 15:30:26 | ralonsoh | yes, that's why I insisted saying that this extension is not mandatory | |
| 15:30:33 | sean-k-mooney | im pretty sure that optional extention was added after we deprecated the proxy apis in nova | |
| 15:31:45 | sean-k-mooney | ralonsoh: i assume there is more to supporting the exteion then just adding that 1 line | |
| 15:32:03 | ralonsoh | this is the list of supported OVN extensions | |
| 15:32:04 | sean-k-mooney | unless this is entrily implement in the ml2 core plugin above the drivers? | |
| 15:32:14 | ralonsoh | this dict is used to create the config in the CI too | |
| 15:32:38 | sean-k-mooney | right but if networking-ovn does not have code support for it and its not implemented at teh plugin level then that is incorrect to add | |
| 15:32:53 | ralonsoh | sean-k-mooney, http://codesearch.openstack.org/?q=ML2_SUPPORTED_API_EXTENSIONS&i=nope&files=&repos= | |
| 15:32:55 | sean-k-mooney | so im asking does the networking-ovn ml2 driver need to be extended to supprot it | |
| 15:33:13 | ralonsoh | https://opendev.org/openstack/networking-ovn/src/branch/master/networking_ovn/l3/l3_ovn.py | |
| 15:33:38 | mriosfer | is recomended enable watchdog in openstack instances? | |
| 15:34:14 | sean-k-mooney | ok so this still seams wrong to me you should not need to enable it in the neutorn tree the driver networking-ovn repo should be provideing the support exteion list | |
| 15:35:00 | sean-k-mooney | mriosfer: am i dont know of any guidence either way. if yo need it then you can use it but its just an optional feature some operators wanted | |
| 15:35:41 | ralonsoh | sean-k-mooney, this is something still under discussion | |
| 15:35:58 | sean-k-mooney | ralonsoh: is the neutron/common/ovn/extensions directory added as part of try ing to merge networking-ovn back in tree | |
| 15:36:19 | mriosfer | sean: im going to test your notes in instances right now :) | |
| 15:36:46 | ralonsoh | sean-k-mooney, yes, thats in the neutron repo now | |
| 15:37:07 | sean-k-mooney | ralonsoh: so networking-ovn is nolonger required at all | |
| 15:37:29 | ralonsoh | nope | |
| 15:37:39 | sean-k-mooney | ralonsoh: actully thats off topic we can talk about it someother time | |
| 15:37:46 | ralonsoh | sean-k-mooney, sure! | |
| 15:42:27 | stephenfin | sean-k-mooney: I changed the behaviour to rely on that 'port_details' field in a recent patch because I didn't know it was an optional extension | |
| 15:43:07 | stephenfin | sean-k-mooney: we need that info purely so we can get the 'device_id' field, which is the instance UUID, for the deprecated floating IP proxy APIs | |
| 15:43:12 | stephenfin | deprecated by not removed | |
| 15:46:36 | sean-k-mooney | we should not need that however. | |
| 15:47:14 | sean-k-mooney | we can list the ports assocaiated with an insnatce and then we should eb able to list the floating ips assinged ot each port | |
| 15:47:35 | mriosfer | sean : :The requested amount of video memory 128 is higher than the maximum allowed by flavor 0 :( something i changed wrong https://gyazo.com/302f96f1f0e2363da2b9dd10ad741e3e?token=b6e41e022fa6260b90802950e02137bf | |
| 15:48:04 | stephenfin | sean-k-mooney: That sounds like a lot more rework though :) | |
| 15:48:20 | stephenfin | Possible, yes. Worth it? | |