| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-06 | |||
| 18:06:44 | melwitt | you're going to opendev? so am I | |
| 18:08:15 | cburgess | dansmith Wait what did I do? | |
| 18:08:34 | dansmith | cburgess: you encouraged him by laughing at his jokes | |
| 18:08:58 | cburgess | dansmith Oh that... | |
| 18:14:41 | openstackgerrit | Merged openstack/nova master: doc: Remove deprecated call to sphinx.util.compat https://review.openstack.org/498824 | |
| 18:26:55 | mnaser | dansmith / mriedem: https://bugs.launchpad.net/nova/+bug/1715462 | |
| 18:26:57 | openstack | Launchpad bug 1715462 in OpenStack Compute (nova) "Instances failing quota recheck end up with no assigned cell" [Undecided,New] | |
| 18:28:00 | sdague | cburgess: what's your biggest concern on privsep? | |
| 18:28:13 | sdague | I apparently didn't see that in the patch (or glossed past it) | |
| 18:28:14 | mnaser | i guess a test to make sure that a cell is assigned if the quota recheck fails would be #1 and then the fix to show that its working would be the correct path? | |
| 18:28:35 | dansmith | mnaser: that'd be ideal yeah | |
| 18:28:44 | mnaser | ok, ill try to work on a test first | |
| 18:28:46 | cburgess | sdague I haven't reviewed the patch this was a review mikal and I did verbally in my dinning room this morning. | |
| 18:29:37 | sdague | cburgess: were mimosas involved? | |
| 18:29:58 | cburgess | sdague I have a few concerns but the big one is around how the privsep daemons get started. I would prefer that if a nova component detects that there is no daemon it just started/restarted it. Having to restart the entire compute/conductor/whatever process is faily heavy weight just to get the privsep daemon running again. | |
| 18:30:06 | cburgess | sdague Oh I wish.. that would have made it better | |
| 18:30:29 | sdague | cburgess: yeah, the watchdogging seems reasonable | |
| 18:30:30 | mnaser | dansmith mind if i bother you a bit as i figure out the best way to go about this with small questions? https://github.com/openstack/nova/blob/master/nova/tests/unit/conductor/test_conductor.py#L1722-L1760 -- would it be beneficial to just add an assert there instead of writing a whole new test? | |
| 18:30:54 | openstackgerrit | Merged openstack/nova master: trivial: Remove "vif" script https://review.openstack.org/491443 | |
| 18:31:19 | dansmith | mnaser: since that test is specifically aimed at checking that late quota check I think that's probably fine | |
| 18:31:33 | mnaser | alright cool, i'll give it a shot, lets hope it fails :> | |
| 18:31:36 | openstackgerrit | Merged openstack/nova master: trivial: Remove files from 'tools' https://review.openstack.org/491444 | |
| 18:31:39 | cburgess | sdague Cool so we agree and mikal did in theory when I went to the gym this morning but now he claims there might be issues and promises performance art in Denver. | |
| 18:36:59 | mnaser | woo, i have a failing test case | |
| 18:39:08 | mnaser | can i get some advice on how would be the ideal way of where to move the cell_mapping saving part? should all of it be moved earlier or should i refactor it into a private fucntion such as _update_instance_mapping(instance, cell) and then call that in the exception handling part of the quota recheck? | |
| 18:39:28 | openstackgerrit | Erik Berg proposed openstack/nova master: Fix binary name. https://review.openstack.org/501359 | |
| 18:39:40 | mnaser | that way we don't risk introducing other weird problems that might come up from shuffling the order of things | |
| 18:42:12 | dansmith | mnaser: this is a sticky place we have to be super careful | |
| 18:42:14 | dansmith | give me a few to read | |
| 18:43:02 | mnaser | dansmith: no problem! that's why i thought by refactoring it into a function and calling it in exception handling, we touch the *least* amount of codepath possible (but i'm sure folks know the codebase far more than me :-) | |
| 18:46:29 | dansmith | mnaser: so I think the right thing to do is really to map the instance right after we create it above | |
| 18:46:51 | dansmith | mnaser: so that if we end up with any instance created (which will show up in a list) it'll have a corresponding map show that show will work | |
| 18:46:58 | dansmith | s/show that/so that/ | |
| 18:47:50 | mnaser | dansmith so right after the with(..) block spanning 988-990? | |
| 18:48:03 | dansmith | mnaser: year | |
| 18:48:10 | dansmith | heh, that was either yeah or yar | |
| 18:48:43 | mnaser | dansmith in my research of the code, i found the _populate_instance_mapping function which seemed pretty robust at setting the instance mapping | |
| 18:48:51 | mnaser | would it beneficial to use that instead? | |
| 18:49:25 | mnaser | we have the host in there so we can pass it (but i'll take what you think is best overall) | |
| 18:50:25 | dansmith | mnaser: yeah if that works should be okay | |
| 18:51:06 | mnaser | dansmith cool, i'll get on this and see if it affects any other tests as well | |
| 18:51:45 | dansmith | cool | |
| 18:55:08 | mnaser | yay, that specific test is passing now, i'll just rerun all the conductor tests because that code shuffle might have affected other tests | |
| 18:55:50 | dansmith | yeah, entirely possible because that extra thing isn't mocked out now | |
| 18:59:38 | mnaser | only one test failing after that.. a bit less painless than i expected :> | |
| 18:59:56 | dansmith | run functional tests too? | |
| 19:05:21 | mnaser | nope, didnt do that yet, just the uni test of conductor | |
| 19:05:57 | dansmith | might scare up another failure or two in there depending | |
| 19:06:27 | dansmith | mnaser: I have to head to the airport in a few but will be back online from there | |
| 19:06:36 | mnaser | dansmith np, thank you for your help so far | |
| 19:06:51 | dansmith | np | |
| 19:07:44 | openstackgerrit | Merged openstack/nova master: doc: Add user index page https://review.openstack.org/498817 | |
| 19:08:31 | openstackgerrit | Merged openstack/nova master: doc: Add configuration index page https://review.openstack.org/498818 | |
| 19:15:02 | sdague | hmmmm.... https://bugs.launchpad.net/nova/+bug/1715463 doesn't seem good | |
| 19:15:03 | openstack | Launchpad bug 1715463 in OpenStack Compute (nova) "binary name gets confused under upgrades of osapi_compute and metadata" [High,Incomplete] - Assigned to Ebbex (eb4x) | |
| 19:21:45 | cdent | sdague: the fix didn’t get linked to the bug yet (because the bug came after the code): https://review.openstack.org/#/c/501359/ | |
| 19:22:01 | cdent | it’s effectively a logic problem in the code | |
| 19:22:12 | cdent | name change in the wrong place | |
| 19:22:57 | sdague | yeh, so this the deployment change after the upgrade, so they come up wsgi for the first time? | |
| 19:23:29 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass migration from API to conductor for evacuate https://review.openstack.org/500176 | |
| 19:23:29 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove dest node allocation if evacuate MoveClaim fails https://review.openstack.org/499878 | |
| 19:23:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add recreate test for evacuate claim failure https://review.openstack.org/499874 | |
| 19:23:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add a test to make sure failed evacuate cleans up dest allocation https://review.openstack.org/499877 | |
| 19:23:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Refactor out claim_resources_on_destination into a utility https://review.openstack.org/499718 | |
| 19:23:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Create allocations against forced dest host during evacuate https://review.openstack.org/499399 | |
| 19:23:32 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Modernize set_vm_state_and_notify https://review.openstack.org/499799 | |
| 19:26:49 | cdent | sdague: dunno | |
| 19:28:57 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 19:29:53 | ildikov | mriedem: hi | |
| 19:30:19 | mriedem | o/ | |
| 19:30:46 | ildikov | mriedem: just wanted to ask how to move forward with the Cinder-Nova open reviews? | |
| 19:31:06 | mriedem | looks like that dependent cinderclient change is released and in g-r | |
| 19:31:30 | mriedem | so i can start looking at https://review.openstack.org/#/c/493323/ later | |
| 19:32:43 | ildikov | mriedem: yep, the cinderclient is all set and the test runs looked clean so far | |
| 19:33:09 | ildikov | mriedem: I also uploaded the specs and left the multi-attach in WIP for now, but happy to get feedback on both | |
| 19:34:01 | openstackgerrit | Andreas Jaeger proposed openstack/nova master: Fix broken link https://review.openstack.org/501391 | |
| 19:46:41 | mnaser | conductor tests all passing with that change, functional are all passing so far so hopefully i can push this up soon :> | |
| 19:55:44 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix test_rpc_consumer_isolation for oslo.messaging 5.31.0 https://review.openstack.org/501400 | |
| 20:00:45 | openstackgerrit | Dan Smith proposed openstack/nova master: Add nova-manage db command for ironic flavor migrations https://review.openstack.org/501025 | |
| 20:00:46 | openstackgerrit | Dan Smith proposed openstack/nova master: Add ComputeNodeList.get_by_hypervisor_type() https://review.openstack.org/501343 | |
| 20:02:16 | openstackgerrit | Andreas Jaeger proposed openstack/nova master: Fix broken URLs https://review.openstack.org/501402 | |
| 20:05:27 | openstackgerrit | Andreas Jaeger proposed openstack/nova stable/pike: Fix broken link https://review.openstack.org/501403 | |
| 20:10:54 | openstackgerrit | Merged openstack/nova master: HyperV: Perform proper cleanup after failed instance spawns https://review.openstack.org/499690 | |
| 20:12:38 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Ensure instance mapping is updated in case of quota recheck fails https://review.openstack.org/501408 | |
| 20:12:47 | mnaser | dansmith ^ | |
| 20:12:53 | dansmith | wooo | |
| 20:13:05 | dansmith | melwitt: ^ | |
| 20:13:31 | melwitt | sweet | |
| 20:15:19 | melwitt | thanks for running with that mnaser | |
| 20:15:49 | mnaser | melwitt np! :) | |
| 20:24:31 | sdague | looking at old patches, is this still a thing - https://review.openstack.org/#/c/375400/ ? | |
| 20:24:57 | openstackgerrit | Nicolas Simonds proposed openstack/nova master: libvirt: add support for virtio-net rx/tx queue sizes https://review.openstack.org/484997 | |
| 20:27:31 | mriedem | sdague: yeah that doesn't seem worth it right now, plus yeah we don't want to touch older release notes if we can help it | |
| 20:27:43 | openstackgerrit | Nicolas Simonds proposed openstack/nova master: libvirt: add support for virtio-net rx/tx queue sizes https://review.openstack.org/484997 | |
| 20:33:51 | mriedem | mnaser: melwitt: dansmith: one concern in that change related to mapping the instance before we create the bdms/tags in the cell | |
| 20:34:00 | mriedem | this is why this gets all really gorpy | |
| 20:34:31 | dansmith | mriedem: yeah I was thinking about that, but if we just have the buildrequest we have less info visible right | |
| 20:34:43 | melwitt | yeah, I was similarly concerned but not sure if it's a problem yet | |
| 20:36:49 | dansmith | won't we keep using the buildrequest if present? | |
| 20:36:50 | dansmith | like, that's the lock we use to say "okay now you can look at the instance" | |