| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-06 | |||
| 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 | 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:44:58 | dansmith | yeah, but, then we're kinda checking the quota without everything created, which is a little odd | |
| 20:45:13 | dansmith | I mean, I guess it's just checking instances | |
| 20:45:18 | melwitt | yeah | |
| 20:45:21 | mriedem | if we did just quota check at the end, then we also have to cleanup the bdms and tags | |
| 20:45:28 | mriedem | in addition to the stuff that _cleanup_build_artifacts is already removing | |
| 20:45:48 | dansmith | do we? | |
| 20:46:01 | dansmith | we delete the instance.. are those things cleaned up by compute normally or something? | |
| 20:46:12 | mriedem | it wouldn't be mapped to a host | |
| 20:46:15 | mriedem | so no compute involved | |
| 20:46:21 | dansmith | no, | |
| 20:46:35 | dansmith | this would be like a local delete | |
| 20:46:56 | dansmith | because we obviously need not call to compute in this case, so I'm asking in a normal local delete where compute is down, | |
| 20:47:04 | dansmith | do we have to clean up bdms and tags separately? | |
| 20:47:34 | mriedem | no, don't think so, those get removed via the db api | |
| 20:47:36 | mriedem | when you delete the instance | |
| 20:47:54 | dansmith | instance delete will delete BDMs | |
| 20:48:01 | mriedem | https://github.com/openstack/nova/blob/b79492f70257754f960eaf38ad6a3f56f647cb3d/nova/db/sqlalchemy/api.py#L1886 | |
| 20:48:05 | mriedem | https://github.com/openstack/nova/blob/b79492f70257754f960eaf38ad6a3f56f647cb3d/nova/db/sqlalchemy/api.py#L1894 | |
| 20:48:05 | dansmith | and tags | |
| 20:48:06 | dansmith | yeah | |
| 20:48:52 | dansmith | so when you delete the instance after the quota check those will get cleaned up if we have created them right? | |
| 20:49:45 | mriedem | well we don't delete the instance here, | |
| 20:49:54 | mriedem | we just set it to ERROR state | |
| 20:49:59 | mriedem | and then the user does the (local) delete | |
| 20:50:08 | dansmith | right | |
| 20:50:09 | mriedem | which should cleanup the bdms/tags | |
| 20:50:21 | openstackgerrit | Merged openstack/nova master: Fix broken link https://review.openstack.org/501391 | |
| 20:50:31 | mriedem | well, the bdms and tags wouldn't even exist in the cell db | |
| 20:50:37 | mriedem | if the quota check fails | |
| 20:50:47 | mriedem | oh but if we moved it to the end.. | |
| 20:51:03 | mriedem | yeah i think that would work | |
| 20:52:11 | dansmith | we can also do what mnaser wanted to do in the first place, which is just do this quick map fixup if we fail this quota check, | |
| 20:52:12 | melwitt | cells meeting in 8 minutes? | |
| 20:52:26 | dansmith | but then we will have an instance without those things in error state, and we'll be doing it from yet another place | |
| 20:52:41 | dansmith | I kinda want to keep it less spaghetti-like and do it in one place, personally | |
| 20:52:58 | dansmith | melwitt: I'm in an airport so we said this morning we'd cancel | |
| 20:53:08 | melwitt | oh, cool | |
| 20:53:11 | dansmith | if you have things to discuss we can, because I'm clearly connected, but, I didn't see the point | |
| 20:53:26 | melwitt | no, just forgot to check the log | |
| 20:53:36 | dansmith | ack | |
| 20:54:12 | mriedem | another alternative is just don't delete the build request in _cleanup_build_artifacts, or is that crazy? | |