| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-23 | |||
| 13:35:48 | sdague | I honestly remember this exact issue when we hit 2.10 | |
| 13:36:03 | efried | lifeless Yes, https://github.com/testing-cabal/mock/issues/398 is me. | |
| 13:36:47 | efried | mriedem I've got a doc appt during the nova meeting today. I left my update in the agenda. | |
| 13:38:11 | cdent | sdague: sure. the other two issues were: didn't want to reuse nova code because I thought we'd be moving soon; and when I first did this I couldn't seem to untangle the way in which nova was doing version_handler from other parts of the wsgi stack. | |
| 13:38:30 | cdent | In other words I found the code too hard to read, so wrote some more. Which is sad but true. | |
| 13:39:47 | cdent | (it's the stuff in Controller that bit me) | |
| 13:44:25 | mriedem | efried: ok | |
| 13:46:16 | sdague | cdent: ok, but you've effectively replicated the APIVersionRequest object as a tuple now. It feels like that's going to be a pretty direct transition | |
| 13:46:37 | openstackgerrit | Eric Fried proposed openstack/nova master: PowerVM Driver: spawn/delete #1: no-ops https://review.openstack.org/438119 | |
| 13:46:47 | openstackgerrit | Hironori Shiina proposed openstack/nova-specs master: Ironic: Cold migration support https://review.openstack.org/449155 | |
| 13:47:31 | cdent | sdague; that part is not the interesting/useful/verbose part of the handling though. It is the users of the Version concept, and those parts between placement and nova are much different (simply because the framework is too) | |
| 13:47:44 | cdent | I agree with you that there's some duplication, and that's unfortunate, but it's where we are right now | |
| 13:48:12 | cdent | and I'm not too het up about repeating bug fixes, it is good for learning, for refactoring, and refreshing, especially when they are caught before they leak | |
| 13:48:42 | cdent | we had the review process for all this work, and it missed some things, that's okay, that's life. | |
| 14:00:11 | mriedem | nova meeting time | |
| 14:11:27 | moshele | sean-k-mooney: hi, can you review the https://review.openstack.org/#/c/398277/ | |
| 14:18:15 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.volume_attach notification https://review.openstack.org/401992 | |
| 14:18:16 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.volume_detach notification https://review.openstack.org/408676 | |
| 14:19:26 | jgriffith | lyarwood sorry, I thought you were indicating that your patches that replaced mine had merged; still waiting before refactoring everything | |
| 14:20:12 | jgriffith | lyarwood looks like there's still some dialogue to hash out | |
| 14:21:00 | johnthetubaguy | I think lyarwood is out for a week, not 100% sure | |
| 14:22:10 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.volume_attach notification https://review.openstack.org/401992 | |
| 14:55:19 | sdague | where are servers assigned to server_groups? | |
| 14:56:02 | dansmith | sdague: hint group=foo | |
| 14:56:54 | sdague | oh, it's buried inside hints? | |
| 14:57:32 | gibi | mriedem: during the nova meeting I got the info from sjmc7 about the needed transformations | |
| 14:57:47 | sdague | which aren't documented... that's the awesome :) | |
| 14:58:30 | gibi | mriedem: < sjmc7> gibi: the only notification we rely on that's not implemented is compute.instance.volume.attach/detach | |
| 14:58:54 | gibi | mriedem: so we are on a good track as those notifications already in focus | |
| 14:59:47 | openstack | Launchpad bug 1675276 in OpenStack Compute (nova) "Volumes attached to shelved instance may contain incorrect device_name" [Undecided,In progress] - Assigned to Zhenyu Zheng (zhengzhenyu) | |
| 14:59:47 | Kevin_Zheng | https://bugs.launchpad.net/nova/+bug/1675276 | |
| 15:00:25 | mriedem | Kevin_Zheng: the device_name is never guaranteed | |
| 15:00:39 | dansmith | sdague: yep, create a group foo, then boot an instance with hint group=foo | |
| 15:00:39 | bauzas | sdague: yup | |
| 15:00:43 | mriedem | Kevin_Zheng: we've talked about just removing the device_name from the os-volume_attachments API in the past | |
| 15:00:46 | bauzas | sdague: you create a group thru the API | |
| 15:00:53 | bauzas | and what dansmith said | |
| 15:01:15 | bauzas | no way to remove an instance from a group, long-lived story | |
| 15:01:43 | mriedem | there have been specs for that though, adding and removing members | |
| 15:01:43 | Kevin_Zheng | mriedem: Hmm OK，what I dicovered is | |
| 15:01:46 | mriedem | i remember those from kilo | |
| 15:01:56 | bauzas | yup | |
| 15:02:02 | bauzas | and I can name the person :) | |
| 15:02:08 | bauzas | who is now a Watcher core :) | |
| 15:02:26 | bauzas | anyway, my point is | |
| 15:02:27 | Kevin_Zheng | cinder can save wrong device name for shelved instance | |
| 15:02:53 | Kevin_Zheng | could you check for detail？ on my bug report？ | |
| 15:02:59 | bauzas | I'm not sure the servergroup API is the good abstract model for describing colocation rules for placement between instances | |
| 15:03:01 | sdague | yeh, it's just without reading nova code you can't use any of this, there is nothing in the API ref that connects all this together (slight asside, but a little relevant for figuring out a path forward) | |
| 15:03:15 | mriedem | Kevin_Zheng: yeah i just read it | |
| 15:03:26 | mriedem | Kevin_Zheng: i think we have to pass a mountpoint to the cinder os-attach API | |
| 15:04:05 | Kevin_Zheng | device name didnt got checked for shelved instance | |
| 15:04:20 | Kevin_Zheng | so I can | |
| 15:04:35 | Kevin_Zheng | pass vda and cinder will save vda | |
| 15:05:02 | mriedem | Kevin_Zheng: right we don't check the device name for the instance during attach to a shelved offloaded instance because the instance does not have a host, so we can't call the compute to reserve the device name, like during normal volume attach | |
| 15:05:29 | Kevin_Zheng | yeah，I was thinking do | |
| 15:05:45 | mriedem | Kevin_Zheng: that's why we create the BDM with device_name=None here http://git.openstack.org/cgit/openstack/nova/tree/nova/compute/api.py#n3678 | |
| 15:05:48 | Kevin_Zheng | do something once we got the right name | |
| 15:05:55 | mriedem | otherwise we do http://git.openstack.org/cgit/openstack/nova/tree/nova/compute/api.py#n3687 | |
| 15:06:13 | mriedem | Kevin_Zheng: are you proposing that we set the device_name in the BDM after unshelve? | |
| 15:06:20 | madhaviy | @mriedem, one question on nova/network/neutronv2/api's _check_external_network_attach() method. This is checking whether user/context can attach a network which is marked as external. so when this code is executed during deploy/spawn, policy checking is been done on compute host. Do we need to validate this even before | |
| 15:06:24 | Kevin_Zheng | yeah I did some learning this afternoon | |
| 15:06:30 | gibi | bauzas: there was a proposal from jaypipes to revisit the server group concept here https://review.openstack.org/#/c/183837/4/specs/liberty/approved/generic-scheduling-policies.rst | |
| 15:06:37 | mriedem | Kevin_Zheng: i think that might still be a lie though, | |
| 15:06:52 | mriedem | because just because the user asked for /dev/vda doesn't mean that's where it will show up on the guest | |
| 15:06:58 | mriedem | even though we stored that in cinder | |
| 15:07:03 | Kevin_Zheng | yeah | |
| 15:07:16 | mriedem | this is really why we need to just have a microversion to take device_name out of the volume attach API :) | |
| 15:07:21 | mriedem | because we can't honor it | |
| 15:07:27 | mriedem | we don't even try with the libvirt driver | |
| 15:07:29 | mriedem | we ignore it | |
| 15:07:32 | Kevin_Zheng | yeah | |
| 15:07:45 | Kevin_Zheng | I will try to put up one | |
| 15:07:56 | Kevin_Zheng | if nobody does | |
| 15:08:03 | mriedem | thanks, this is something we've talked about in previous summits and meetups and just never had someone write a spec | |
| 15:08:16 | mriedem | but it comes up as a problem about every 3 months :) | |
| 15:08:23 | Kevin_Zheng | ：） | |
| 15:08:37 | Kevin_Zheng | but still | |
| 15:08:58 | Kevin_Zheng | we will have none in cinder if not provide one | |
| 15:09:01 | bauzas | gibi: yup, I reviewed it even :) | |
| 15:09:17 | Kevin_Zheng | anyway to sync it？ | |
| 15:09:23 | gibi | bauzas: I think that spec contains a lot of nice idea and discussion on this matter | |
| 15:11:45 | mriedem | Kevin_Zheng: let me check the cinder api | |
| 15:11:58 | mriedem | if mountpoint is required then we'd have an issue | |
| 15:12:52 | Kevin_Zheng | mriedem: Im using phone so I can't send you the code，but there is a if condation in | |
| 15:13:14 | Kevin_Zheng | nova's attach function | |
| 15:13:59 | Kevin_Zheng | it checked volume.attach_status =detached | |
| 15:14:36 | Kevin_Zheng | maybe we can also check device name there，if none，send again the | |
| 15:14:42 | Kevin_Zheng | right one | |
| 15:15:48 | openstackgerrit | Maciej Szankin proposed openstack/nova-specs master: Add deprecate-nova-cert spec https://review.openstack.org/448882 | |
| 15:16:49 | openstackgerrit | Maciej Szankin proposed openstack/nova-specs master: Add deprecate-nova-cert spec https://review.openstack.org/448882 | |
| 15:19:02 | mriedem | Kevin_Zheng: looks like the mountpoint is required in the os-attach API in cinder | |
| 15:19:23 | mriedem | in fact, it looks like i can trigger a 500 error if i don't include it in the request body | |
| 15:26:13 | mriedem | melwitt: dansmith: I think Kevin_Zheng found another fun local delete cell0 issue :) | |
| 15:26:44 | mriedem | not totally sure yet, but we don't call http://git.openstack.org/cgit/openstack/nova/tree/nova/compute/api.py#n2144 when doing the local delete case where the instance isn't in a cell | |
| 15:26:56 | mriedem | so the instance is in error state but has volumes attached, and we don't cleanup the volumes | |
| 15:27:19 | mriedem | i think this would be (1) create server, (2) shelve it, (3) attach volume (4) unshelve - fails, puts instance in error state (5) delete | |
| 15:27:40 | mriedem | the question is, does the instance got to cell0 on the unshelve failure in step 4? | |
| 15:28:24 | dansmith | shouldn't, | |
| 15:28:51 | dansmith | because you can't move the instance, so it wouldn't reschedule to cell0 if it's already in cell1, which it has to be because it was shelved in the first place | |