| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-26 | |||
| 12:36:31 | mriedem | lyarwood: oh ok | |
| 12:36:49 | mriedem | alex_xu: ack, i still need to read all of the replies... | |
| 12:37:20 | alex_xu | mriedem: hah, i see, a lot | |
| 12:39:45 | alex_xu | mriedem: also, there is the code for reference https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/virtual-persistent-memory, althought it is merge conflict, but it is still good to see what we probably are going to change in the code | |
| 12:42:59 | sean-k-mooney | mriedem: can you take a look at https://review.opendev.org/#/c/667264/ its a osc change for force down. you sent a mail to the list about droping computenode host/service id compat code and im wondering if that is related or not | |
| 12:43:45 | mriedem | i think my biggest hangups were on the (1) flavor extra spec definition which was a bit hard to parse from a user perspective in my opinion and (2) the questions about the new data model and versioned object which were very similar to a BDM but i realize we don't want to re-use BDMs for this | |
| 12:44:16 | mriedem | sean-k-mooney: different issue | |
| 12:44:31 | mriedem | sean-k-mooney: before 2.53 you had to call a force-down route, with 2.53 you just call the normal PUT route | |
| 12:44:39 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add mising tests for flavor extra_specs mv 2.61 https://review.opendev.org/667600 | |
| 12:44:48 | mriedem | https://developer.openstack.org/api-ref/compute/#update-forced-down for <2.53 | |
| 12:44:49 | sean-k-mooney | right i saw that | |
| 12:44:56 | mriedem | https://developer.openstack.org/api-ref/compute/#update-compute-service >=2.53 | |
| 12:45:05 | sean-k-mooney | what i was concerned about is the new form uses service id | |
| 12:45:06 | mriedem | with 2.53 the service_id in the API is a uuid | |
| 12:45:06 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add missing tests for flavor extra_specs mv 2.61 https://review.opendev.org/667600 | |
| 12:45:14 | sean-k-mooney | the old used host and binary name | |
| 12:45:29 | mriedem | service_id is the uuid of the service, it's fine | |
| 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: Privsepify ipv4 forwarding enablement. https://review.opendev.org/635431 | |
| 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:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Privsep the ebtables modification code. https://review.opendev.org/635435 | |
| 14:32:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move adding vlans to interfaces to privsep. https://review.opendev.org/635436 | |
| 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:36 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move dnsmasq restarts to privsep. https://review.opendev.org/639280 | |
| 14:32:37 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Move router advertisement daemon restarts to privsep. https://review.opendev.org/639281 | |