| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-06 | |||
| 15:56:01 | mriedem | it was fubar'ed FAST | |
| 15:56:04 | dansmith | stephenfin: I was thinking it would be nice to merge another good chunk of those this cycle, but... | |
| 15:56:43 | sean-k-mooney | mriedem for what its work im hoping to get the os-vif gates down to that by droping the keystone and cinder apis test. | |
| 15:57:11 | mtreinish | keystone tests take like zero time | |
| 15:57:18 | mtreinish | dropping cinder will save a lot of time | |
| 15:57:45 | mtreinish | sean-k-mooney: if there is no integration between os-vif and cinder I was just disable cinder all together in those jobs | |
| 15:57:51 | sean-k-mooney | mtreinish: ya but os-vif is a lib that never talk to any other openstack service | |
| 15:57:53 | mtreinish | s/was/would/g | |
| 15:57:53 | mriedem | that's why i dropped the non-compute api tests from the nova-next job | |
| 15:58:08 | sean-k-mooney | mtreinish: yes im going ot | |
| 15:59:28 | sean-k-mooney | i think in the os-vif jobs i might keep the compute and networking api tests + the senario test for now and see what the job times look like | |
| 16:00:05 | sean-k-mooney | if they are under an hour ill be happy if i can get down to 45 mins or so even more so | |
| 16:02:11 | mtreinish | well if you disable cinder (not just blacklist the cinder api tests) it won't run a good chunk of the scenarios | |
| 16:02:37 | mtreinish | and those scenarios are individually among the slowest tests in a job | |
| 16:11:31 | stephenfin | dansmith: Done-zo https://review.openstack.org/#/c/624592/8 | |
| 16:17:03 | melwitt | mriedem: ack, will fix. I think it's bc that patch changed things from counting instances => instance mappings, that test is probably only creating instance records and not mapping records | |
| 16:23:03 | dansmith | mriedem: probably need someone to make a call on breaking potentially n-net-only tests with this: https://review.openstack.org/#/c/624592 | |
| 16:24:27 | mriedem | i know some of the converted privsep stuff is going untested because testing the privsep stuff is hard... | |
| 16:24:36 | mriedem | i remember wresting with trying to mock around that stuff at one point | |
| 16:24:38 | mriedem | i don't remember the details | |
| 16:24:49 | mriedem | *wrestling even | |
| 16:25:10 | mriedem | i'd rather not make that call today... | |
| 16:26:02 | dansmith | just seemed like some feature-y patches we could make independent progress on, and they were just refreshed, but.. I got stopped at the first one | |
| 16:27:00 | sean-k-mooney | dansmith: even in the nova-net case some of that code is likely dead code as os-vif will be used in most cases | |
| 16:28:05 | dansmith | sean-k-mooney: understand, but it's just hard to get on board with orphaning code with tests.. if it's dead code, then maybe we should remove it | |
| 16:28:10 | mriedem | in other fun news, when post live migration fails we don't set the instance to error status or reset the task_state to None apparently http://logs.openstack.org/43/635343/4/check/tempest-slow-py3/a2497ae/job-output.txt.gz#_2019-03-06_05_29_44_910801 | |
| 16:28:18 | sean-k-mooney | the gateway code is likely still used | |
| 16:30:12 | sean-k-mooney | mriedem: ya i have seen that happen a few times | |
| 16:31:34 | sean-k-mooney | dansmith: by the way we were waiting mainly for cern to move to cellsv2 only before droping nova-networks | |
| 16:31:46 | sean-k-mooney | so can we drop it entirly in train? | |
| 16:31:59 | dansmith | sean-k-mooney: they're not fully off it, AFAIK | |
| 16:32:09 | sean-k-mooney | we are not going to froward port it to python3 im assuming | |
| 16:32:35 | sean-k-mooney | dansmith: ok we should check with them and see. | |
| 16:33:28 | stephenfin | There was a mail about this on openstack-discuss a few weeks back | |
| 16:33:52 | stephenfin | I recall they said they were planning to migrate *everything* to neutron, but I can't recall the timeline | |
| 16:34:13 | dansmith | yeah, that's been the plan for a while, I just haven't heard that they have | |
| 16:34:25 | dansmith | either way, removal is non-trivial so cern being off it isn't the only barrier :) | |
| 16:34:50 | stephenfin | Sounds like a challenge | |
| 16:35:24 | stephenfin | Does anyone know if config drives rely on the metadata service being up? I'm assuming 'nova-compute' handles stuff by itself but haven't dived too deep yet | |
| 16:35:35 | melwitt | mriedem: I have a osc-placement change up for a batch command for setting allocation ratios per aggregate in placement, if you're interested in reviewing https://review.openstack.org/640898 | |
| 16:35:36 | sean-k-mooney | well i think we might be able to delegate some of the logic to os-vif in the interim | |
| 16:36:01 | sean-k-mooney | for example i want to pull the trusted vf stuff into a sriov plugin in os-vif | |
| 16:36:12 | dansmith | sean-k-mooney: if you think those gateway things are still used can you comment? | |
| 16:36:42 | dansmith | sean-k-mooney: I think there's also some default firewalling of nova services that still use a lot of the linux_net ip.* commands, right? like punching holes for nova-api, metadata, etc | |
| 16:42:50 | sean-k-mooney | dansmith: ya there is a bunch of firwalling stuff. i would prefer not to migrate that code to os-vif but we migth be able to clean it up abit | |
| 16:43:20 | dansmith | sean-k-mooney: yeah, not saying we should, I'm saying we shouldn't just be randomly dropping tests for stuff we convert to privsep because "it's hard" and "it might be only for n-net" | |
| 16:44:05 | openstackgerrit | melanie witt proposed openstack/nova master: Use instance mappings to count server group members https://review.openstack.org/638324 | |
| 16:44:05 | sean-k-mooney | ill take a look and see if its used and ya. | |
| 16:44:57 | sean-k-mooney | i can take a look at some to the privisep changes and see if i can convert them or help if needed too | |
| 16:45:18 | sean-k-mooney | but that will be after ff | |
| 16:45:45 | sean-k-mooney | i am happy to work with stephen on paying donw that technical debt | |
| 16:46:50 | dansmith | I was just thinking that these feature-y patches might be a few easy wins to get in before FF, several were +2 already and just refreshed | |
| 16:46:55 | dansmith | but clearly I was wrong :) | |
| 16:47:49 | sean-k-mooney | actully they are more or less indepenate so maybe we can change the odering of some of the patches that dont drop tests/nova-net stuff | |
| 16:48:15 | sean-k-mooney | e.g. merge the stuff that is a driect port/obviously correct and deffer the rest | |
| 16:49:04 | dansmith | yep, I +W'd one above that first one | |
| 16:50:25 | artom | stephenfin, the config drive is independant of the metadata API | |
| 16:50:46 | stephenfin | artom: So put configuration in nova-compute? | |
| 16:50:59 | stephenfin | I'm thinking on the vendordata stuff, specifically | |
| 16:51:15 | dansmith | stephenfin: what does that mean? | |
| 16:51:29 | artom | Yeah, what do you want to put in nova-compute? | |
| 16:51:30 | dansmith | stephenfin: config for how to reach the vendordata server/ | |
| 16:51:57 | stephenfin | dansmith: If I was using DynamicJSON, yeah | |
| 16:52:00 | dansmith | stephenfin: config_drive literally just imports the python module that generates the metadata response and uses it to dump things into a disk image | |
| 16:52:08 | dansmith | stephenfin: then yeah, nova-compute would need that config | |
| 16:52:16 | sean-k-mooney | artom: you can add force_config drive or something but you can also just enable config drive in the flavor/image | |
| 16:52:22 | stephenfin | dansmith++ ta | |
| 16:52:54 | stephenfin | I ended up down a rabbithole yesterday and ended up doing this https://review.openstack.org/#/c/640730/ and now I'm more confused than ever. Back to reviews | |
| 16:56:13 | mriedem | https://bugs.launchpad.net/nova/+bug/1818873 | |
| 16:56:14 | openstack | Launchpad bug 1818873 in OpenStack Compute (nova) "When post_live_migration_at_destination fails the instance is not put into ERROR/None vm_state/task_state" [Medium,Triaged] | |
| 16:56:26 | mriedem | so if post_live_migration_at_destination fails early, the guest is on the dest but the instance db state is all f'ed up | |
| 16:56:46 | mriedem | mdbooth: don't you have a similar bug to this ^? | |
| 16:57:41 | sean-k-mooney | mriedem: yes | |
| 16:58:07 | mdbooth | The one I recall was the other way round. The issue is when post_live_migration fails on the *source* | |
| 16:58:18 | artom | mriedem, maybe https://bugs.launchpad.net/nova/+bug/1628606? | |
| 16:58:20 | openstack | Launchpad bug 1628606 in OpenStack Compute (nova) "live migration does not clean up at target node if a failure occurs during post migration" [Low,In progress] - Assigned to Artom Lifshitz (notartom) | |
| 16:58:25 | sean-k-mooney | i brought that up in https://review.openstack.org/#/c/603844/ | |
| 16:59:00 | artom | mriedem, I've been meaning to refactor the flow to make sure we save the instance state correctly in all cases | |
| 16:59:16 | artom | I addressed the Cinder API fail case, there are more (like Neutron) | |
| 16:59:29 | sean-k-mooney | artom: didnt you partial fix this already. we just 1 addional try finally block effectily to make sure we always set the instance host | |
| 16:59:36 | openstackgerrit | Merged openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360 | |
| 16:59:41 | sean-k-mooney | artom: you didnt address all the cinder issues | |
| 16:59:44 | openstackgerrit | Merged openstack/nova master: objects: Store InstancePCIRequest.numa_policy in DB https://review.openstack.org/624444 | |
| 16:59:48 | artom | sean-k-mooney, yeah https://review.openstack.org/#/c/611093/ | |
| 17:00:28 | sean-k-mooney | you missed on place where we call cinder that can raise an exception | |
| 17:00:50 | artom | sean-k-mooney, yeah, it's entirely possible | |
| 17:01:15 | artom | sean-k-mooney, it was meant as a bandaid until a more correct fix is done | |
| 17:02:29 | mriedem | this is clearly not...enterprise ready | |
| 17:02:38 | mriedem | one could spend days cleaning up this mess | |
| 17:03:10 | sean-k-mooney | artom: ya i know | |
| 17:03:12 | sean-k-mooney | https://github.com/openstack/nova/blob/38c96993fd7163afe1a7c5f380af0d57a1bd2903/nova/compute/manager.py#L6736 | |
| 17:03:16 | mdbooth | It seems more intuitive to me to update instance.host to the destination *immediately* we're actually running there, i.e. from the source at the top of _post_live_migration, but IIRC there are weird gotchas with that. | |
| 17:03:22 | sean-k-mooney | that is that call im thinking of | |
| 17:03:24 | mriedem | another clunker is we pass the Migration object around via the MigrateData which is pretty brittle, | |
| 17:03:29 | artom | mriedem, what mdbooth said | |
| 17:03:30 | mriedem | i'd like to just partial that like what mdbooth did with source_bdms | |
| 17:04:01 | sean-k-mooney | it will hit the cinder attachments api and can raise an exception which means the vm is running on the dest but we never update the db | |
| 17:04:06 | artom | I think just making sure we always update instance state the moment it's irreversibly on the destination is a good first step | |
| 17:04:49 | sean-k-mooney | ya i wrote it down on my whiteboard after reviewing the other chage earlier in the week | |
| 17:06:41 | mdbooth | You know how sometimes you don't understand a thing, but you remember understanding a thing while somebody was explaining it? I vaguely recall dansmith explaining why instance.host needs to be updated later. | |