| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-26 | |||
| 12:45:31 | sean-k-mooney | and i was not clear what you were proposeing droping in the mail | |
| 12:45:38 | mriedem | it's unrelated to the relationship between compute nodes and services | |
| 12:45:46 | sean-k-mooney | ok | |
| 12:45:56 | mriedem | see all of the notes/todos around ComputeNode.service_id in the code | |
| 12:46:00 | mriedem | and ComputeNode.host | |
| 12:46:22 | lyarwood | mriedem: https://review.opendev.org/#/c/457886/ - btw, would you mind taking a look at this if you have time this week. | |
| 12:46:48 | sean-k-mooney | mriedem: ok im reading them now thanks | |
| 12:47:13 | mriedem | lyarwood: sure | |
| 12:47:19 | lyarwood | thanks | |
| 13:00:40 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add missing tests for flavor extra_specs mv 2.61 https://review.opendev.org/667600 | |
| 13:08:52 | bauzas | mriedem: FWIW, I need to reload a shit ton of context from Kilo before replying to you but I saw your email | |
| 13:09:32 | bauzas | mriedem: because I wonder if we need a major version bump for the ComputeNode object | |
| 13:15:31 | mriedem | bauzas: i wondered about that as well but figured it wasn't required | |
| 13:15:53 | openstackgerrit | Martin Midolesov proposed openstack/nova master: Implementing graceful shutdown. https://review.opendev.org/666245 | |
| 13:16:22 | mriedem | i think we've only ever bumped the major version on an object and that's when dansmith did Instance v2.0 | |
| 13:16:54 | mriedem | i don't remember the details of how complicated it was but i'm pretty sure i'd screw it up if i tried to do it myself | |
| 13:24:15 | bauzas | mriedem: yeah I need to remember why I was thinking about that by Kilo time | |
| 13:28:00 | mdbooth | stephenfin or sean-k-mooney: https://review.opendev.org/#/c/663382/4/nova/compute/manager.py Not my area of expertise, but would the prior call to _deallocate_network not mean that neutron would no longer return this stuff? | |
| 13:28:27 | mriedem | sean-k-mooney: i've replied on https://review.opendev.org/#/c/667264/2 with what i think they should do in the 2.53 case, | |
| 13:28:37 | mriedem | whether or not novaclient has all of the plumbing they need i haven't checked | |
| 13:29:28 | sean-k-mooney | mriedem: thanks osc is not what i normally review but since they asked me to take a look i said i would review | |
| 13:31:18 | mriedem | sean-k-mooney: mdbooth: also commented in https://review.opendev.org/#/c/663382/4 | |
| 13:31:50 | sean-k-mooney | maybe im looking. we could proably use try_dealocate_networks there too | |
| 13:31:59 | mdbooth | mriedem: Ooh, I'd forgotten that gem. | |
| 13:33:05 | mriedem | mdbooth: what? force_refresh? | |
| 13:33:07 | dansmith | mriedem: correct, and yes, it's complicated | |
| 13:33:08 | mriedem | you don't want to use that in this case | |
| 13:33:23 | mriedem | mdbooth: because force_refresh only goes back to stein and i'm guessing you want to backport this further than that | |
| 13:33:36 | mdbooth | mriedem: Ack. | |
| 13:43:52 | kashyap | Any others seeing stable/stein failures with the 'tempest-slow-p3' job? | |
| 13:44:01 | kashyap | http://logs.openstack.org/89/667389/1/check/tempest-slow-py3/2606bcc/testr_results.html.gz | |
| 13:44:38 | mriedem | kashyap: yes known issue | |
| 13:44:47 | mriedem | https://review.opendev.org/#/c/667216 | |
| 13:45:14 | kashyap | Ah, thanks. I didn't wanted to mindlessly do 'recheck' | |
| 13:46:55 | sean-k-mooney | mdbooth: deallocate_network delete the neutron port that were auto allcoated by nova so yes we proably should move that to the end of the function since it clears the network info caceh https://opendev.org/openstack/nova/src/branch/master/nova/network/neutronv2/api.py#L1603-L1604 | |
| 13:56:27 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Add a rbd_connect_timeout configurable https://review.opendev.org/667421 | |
| 14:00:59 | mriedem | sean-k-mooney: i left some more comments/questions in that one and added some vmware and zvm driver devs | |
| 14:01:13 | sean-k-mooney | mriedem: it looks like https://review.opendev.org/#/c/660761/8 is trying to fix the same or a similar bug | |
| 14:03:53 | sean-k-mooney | mriedem: if we delete while building there is a scond race which causes us to not clean up the vif | |
| 14:04:22 | mriedem | that's amorin's fix yes | |
| 14:04:31 | mriedem | which is different from stephenfin's which is handling a failure while building | |
| 14:04:37 | sean-k-mooney | e.g. if the vm has spawned but we get teh delete before we update the instcance state in the db we raise an exception which is what cause us to not clean them up | |
| 14:04:42 | mriedem | and amorin was just in here the other day saying he had a similar issue there | |
| 14:05:17 | sean-k-mooney | mriedem: no stephens issue was a failure caused when you delete while building | |
| 14:05:52 | sean-k-mooney | spefically for the customer it was cause because one of the isntance in there heat stack failed to build and that cause all fo the instance to be deleted | |
| 14:07:35 | sean-k-mooney | mriedem: i think amorin bug is a duplicate of stephens but im not sure it would fix it in all cases as in sthepens edgecase we never call distroy | |
| 14:08:58 | sean-k-mooney | well maybe they are both bugs i did not fully review there bug in detail | |
| 14:12:17 | mriedem | as i said amorin said he still has an issue which stephenfin's patch might resolve | |
| 14:12:29 | mriedem | amorin said he was going to try and recreate and use stephen's patch to test it | |
| 14:12:51 | sean-k-mooney | ya i think on reading there bug both would be needed | |
| 14:13:58 | sean-k-mooney | mriedem: amorin is fixing the fact we might be useing an outdated network_info object form the instance and stephenfin is fixing that if we fail due to the db update we never even tried to clean up the vifs | |
| 14:14:35 | sean-k-mooney | so to fix the downsteam bug we will need to backprot both. | |
| 14:14:47 | sean-k-mooney | ok this make more sense to me now. | |
| 14:20:28 | amorin | hey all | |
| 14:22:04 | amorin | the bug I faced 2 days ago was not fixed by stephenfin patch | |
| 14:22:16 | amorin | I found that it was something else in our code | |
| 14:23:01 | amorin | cc mriedem sean-k-mooney | |
| 14:23:43 | mriedem | mnaser: i think you just hit something like this nw info cache lost thing, so you might have input here http://lists.openstack.org/pipermail/openstack-discuss/2019-June/007363.html | |
| 14:23:45 | amorin | by the way, I faced an other one, related to the patch I did: | |
| 14:23:46 | amorin | https://review.opendev.org/#/c/667294/ | |
| 14:23:48 | mriedem | maciejjozefczyk: sean-k-mooney: ^ | |
| 14:24:43 | mriedem | amorin: one step forward, two steps back :( | |
| 14:25:00 | amorin | yup | |
| 14:25:06 | mriedem | i remember a similar check was added here https://github.com/openstack/nova/blob/707deb158996d540111c23afd8c916ea1c18906a/nova/network/base_api.py#L35 | |
| 14:25:27 | amorin | exact | |
| 14:26:20 | sean-k-mooney | ok so we might need all 3 patches | |
| 14:27:04 | sean-k-mooney | amorin: stephenfin patch is a generalised fix to a very specific edgecase | |
| 14:28:12 | sean-k-mooney | amorin: what you originally tried to fix was more subtle as we were passing stale data in some cases | |
| 14:29:14 | maciejjozefczyk | ehh, instance_info_cache :) | |
| 14:29:41 | openstackgerrit | Martin Midolesov proposed openstack/nova master: Implementing graceful shutdown. https://review.opendev.org/666245 | |
| 14:30:05 | sean-k-mooney | maciejjozefczyk: yep its awsome... | |
| 14:30:41 | sean-k-mooney | mriedem: out of interest why do we store the instance info cache in the db? | |
| 14:31:14 | sean-k-mooney | i fell like we would have fewer bugs related to it if we actully just made it an in process dict cache | |
| 14:31:42 | mriedem | sean-k-mooney: i'll direct your question to the people that worked on nova back in 2011 or something | |
| 14:32:26 | sean-k-mooney | well my next question was going to be "i assume this is because of nova networks legacy choices" | |
| 14:32:33 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove no longer required "inner" methods. https://review.opendev.org/655282 | |
| 14:32:34 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove unused FP device creation and deletion methods. https://review.opendev.org/635433 | |
| 14:32:34 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Privsepify ipv4 forwarding enablement. https://review.opendev.org/635431 | |
| 14:32:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move adding vlans to interfaces to privsep. https://review.opendev.org/635436 | |
| 14:32:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Privsep the ebtables modification code. https://review.opendev.org/635435 | |
| 14:32:36 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move dnsmasq restarts to privsep. https://review.opendev.org/639280 | |
| 14:32:36 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move iptables rule fetching and setting to privsep. https://review.opendev.org/636508 | |
| 14:32:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move calls to ovs-vsctl to privsep. https://review.opendev.org/639282 | |
| 14:32:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move router advertisement daemon restarts to privsep. https://review.opendev.org/639281 | |
| 14:32:38 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move setting of device trust to privsep. https://review.opendev.org/639283 | |
| 14:32:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: We no longer need rootwrap. https://review.opendev.org/554438 | |
| 14:32:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Cleanup the _execute shim in nova/network. https://review.opendev.org/639581 | |
| 14:32:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move final bridge commands to privsep. https://review.opendev.org/639580 | |
| 14:32:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Cleanup no longer required filters and add a release note. https://review.opendev.org/639826 | |
| 14:33:00 | mriedem | sean-k-mooney: idk, you'd have to do some digging to find out when the network info cache was introduced, i don't know if it was before quantum or not | |
| 14:33:20 | mriedem | but we also store bdms in the db which are essentially the same thing - a cache of volume information for the server | |
| 14:33:28 | mriedem | which was probably before cinder existed | |
| 14:33:41 | sean-k-mooney | im seeing a pattern there | |
| 14:34:17 | sean-k-mooney | ok well lets fix the current issue first but i think i might look into if we could remove storing it to the db | |
| 14:34:31 | amorin | I would love that | |
| 14:34:35 | amorin | :p | |
| 14:34:46 | sean-k-mooney | cacheing in memory in the compute agent would likely be enough | |
| 14:35:18 | sean-k-mooney | we would have to rebuild it every time the compute agent restarts but i think that is fine | |
| 14:37:22 | sean-k-mooney | actully we could use memcache to cache it too which would mean all the services would have acess to it anway its now on my todo list | |
| 14:38:02 | sean-k-mooney | messing up the netron policy and currpting the network info cache is what cause our ci cloud production outage at the weekend | |