| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-06 | |||
| 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: Remove dest node allocation if evacuate MoveClaim fails https://review.openstack.org/499878 | |
| 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: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:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add recreate test for evacuate claim failure https://review.openstack.org/499874 | |
| 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:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Refactor out claim_resources_on_destination into a utility https://review.openstack.org/499718 | |
| 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" | |
| 20:36:54 | dansmith | like, when we remove the buildrequest, I mean | |
| 20:38:16 | mriedem | no we look for the instance mapping first | |
| 20:38:51 | mriedem | https://github.com/openstack/nova/blob/b79492f70257754f960eaf38ad6a3f56f647cb3d/nova/compute/api.py#L2240 | |
| 20:39:22 | dansmith | ah, and use the cell_mapping I guess | |
| 20:40:19 | mriedem | so in the before times, you couldn't apply tags to a server until it was active, | |
| 20:40:23 | mriedem | but in pike you can create a server with tags now | |
| 20:40:55 | dansmith | mriedem: do we need to wait until later to do that? couldn't we do those two steps after the create before this save? and then check the quota? | |
| 20:41:15 | mriedem | idk | |
| 20:41:21 | dansmith | alternately we could just do this if we fail the quota check, it just seemed better to me to do it in fewer places | |
| 20:41:22 | mriedem | this is all wonky why it's all done in two steps | |
| 20:41:26 | dansmith | yeah | |
| 20:41:40 | mnaser | added a docstring while we figure out the rest :> | |
| 20:41:43 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Ensure instance mapping is updated in case of quota recheck fails https://review.openstack.org/501408 | |
| 20:42:03 | mriedem | although, | |
| 20:42:12 | mriedem | it was split into two parts due to melwitt's quota check change | |
| 20:42:38 | melwitt | I didn't change the 2-partness for the quota check | |
| 20:42:39 | mriedem | https://review.openstack.org/#/c/416521/ | |
| 20:43:07 | mriedem | melwitt: i mean the 2 loops over the tuple | |
| 20:43:28 | mriedem | that was a single for loop before | |
| 20:43:31 | melwitt | oh, I see (looking at the patch now) | |
| 20:43:47 | dansmith | hm why is that? | |
| 20:44:09 | mriedem | have to create them all to count | |
| 20:44:16 | mriedem | and then continue creating stuff | |
| 20:44:24 | melwitt | at the time I was thinking, just check all of the quota first before creating a bunch of other resources if it's just gonna possibly fail in the middle | |
| 20:44:30 | dansmith | why not do that at the end I mean | |
| 20:44:58 | dansmith | yeah, but, then we're kinda checking the quota without everything created, which is a little odd | |
| 20:44:58 | melwitt | we could. I was probably just thinking it would be less wasteful, why create BDMs and all of that if it's gonna fail the recheck anyway | |
| 20:45:13 | dansmith | I mean, I guess it's just checking instances | |