| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-25 | |||
| 17:50:20 | mriedem | it's not | |
| 17:50:32 | mriedem | it's for Kevin_Zheng's root bdm attach/detach series | |
| 17:50:45 | sean-k-mooney | oh ok | |
| 17:51:21 | sean-k-mooney | am well if you detach attach anoth volume and then attach the root again i guess it could change | |
| 17:51:42 | sean-k-mooney | i dont know that code that well however | |
| 17:52:15 | mriedem | https://review.openstack.org/#/c/614750/34/nova/compute/manager.py | |
| 17:52:57 | mriedem | if i boot from volume and get vda, then attach a data volume which is vdb, then detach the root volume and attach another root volume, would i expect to have that as vda or vdc? | |
| 17:53:10 | mriedem | i would expect vda because the root volume being higher than the data volume seems wrong | |
| 17:54:02 | dansmith | mriedem: yeah, expect the name to remain stable | |
| 17:54:25 | sean-k-mooney | mriedem: that might depend on the os and the udev rules. but i would expect it to stay the same. i dont know if it actully would | |
| 17:56:42 | mriedem | well, we also don't guarantee the device name the user requests is honored by the hypervisor anyway | |
| 18:01:04 | artom | mriedem, interesting, but I feel like that's specific to what you're doing with them (which is fine!) | |
| 18:02:19 | sean-k-mooney | mriedem: for the detach attach root volume spec | |
| 18:02:34 | sean-k-mooney | mriedem: is it the same volume or can it be any volume that is reattached | |
| 18:02:37 | artom | And as far as I can tell _assertEqualObjects doens't handle nested objects | |
| 18:03:15 | artom | Anyways, as I said, it's an annoyance, but a minor one, though it might be worth it to put in the time to do it properly in a single place so that we stop fixing this each in our little corners | |
| 18:04:08 | sean-k-mooney | mriedem: im wondering if we supprot reading the hw_disk_bus key form image metadata on a volume | |
| 18:04:55 | sean-k-mooney | i think the answer is no but im checking | |
| 18:10:54 | openstackgerrit | Merged openstack/os-vif master: Fix nits in brctl removal (vif_plug_linux_bridge) https://review.openstack.org/639099 | |
| 18:11:57 | sean-k-mooney | mriedem: it looks like we can get the image meta form the volume https://github.com/openstack/nova/blob/af78b13c24d4abf393d17ac57e9135204ef12b73/nova/utils.py#L928 | |
| 18:12:25 | sean-k-mooney | mriedem: so if we are allowing attaching an arbiatry volume as the new root volume the diskbus could change | |
| 18:12:37 | sean-k-mooney | if it has to be the same volume it wont | |
| 18:14:09 | sean-k-mooney | that is called form https://github.com/openstack/nova/blob/5a09c81af3b438ecbcf27fa653095ff55abb3ed4/nova/compute/api.py#L1057 | |
| 18:24:09 | sean-k-mooney | mriedem: ah never mind the propsed change stats the detach will be garded by the instance being shelve offloaded | |
| 18:24:17 | sean-k-mooney | ill update my comment on the patch | |
| 18:25:18 | sean-k-mooney | oh thats the mitaka spec... | |
| 18:27:07 | sean-k-mooney | the stein spech allow detach when the instance is powered off which may not work if the iamge changes | |
| 18:32:29 | openstackgerrit | Merged openstack/python-novaclient master: Handle unicode multi-byte characters https://review.openstack.org/632942 | |
| 18:35:39 | melwitt | o/ | |
| 18:35:57 | sean-k-mooney | melwitt: o/ | |
| 18:36:11 | openstackgerrit | Merged openstack/nova master: Pass resource provider mapping to neutronv2 api https://review.openstack.org/616240 | |
| 18:36:19 | openstackgerrit | Merged openstack/nova master: Recalculate request group - RP mapping during re-schedule https://review.openstack.org/619529 | |
| 18:36:29 | openstackgerrit | Merged openstack/nova master: Add microversion to expose virtual device tags https://review.openstack.org/631948 | |
| 18:36:42 | openstackgerrit | Merged openstack/nova master: api-ref: mark os-cells as deprecated https://review.openstack.org/636708 | |
| 18:36:52 | openstackgerrit | Merged openstack/nova master: Replace ansible --sudo with --become in live_migration/hooks scripts https://review.openstack.org/635308 | |
| 18:48:45 | openstackgerrit | Andrey Volkov proposed openstack/nova master: Check hosts have no instances for AZ rename https://review.openstack.org/509206 | |
| 18:49:55 | mriedem | sean-k-mooney: yeah it can be a different volume | |
| 18:50:50 | sean-k-mooney | mriedem: do you think my concern regarding powered off instance is vlaid | |
| 18:51:04 | sean-k-mooney | mriedem: i updated the comment on the patch | |
| 18:51:40 | mriedem | how would powered off be different from when we unshelve the instance with a new root volume? | |
| 18:52:06 | sean-k-mooney | a powered off instace is associated with a host | |
| 18:52:16 | sean-k-mooney | we read the image metadata form volumes | |
| 18:52:28 | sean-k-mooney | so if you can change the voluems you can change the requirement for the host | |
| 18:52:50 | sean-k-mooney | in unshevle we will hit the schuler | |
| 18:52:57 | sean-k-mooney | but for powered off instnace we dont | |
| 18:53:11 | sean-k-mooney | when we we start it again that is | |
| 18:55:25 | mriedem | yes i see the issue, but i don't think he's reading the new root volume image_meta on unshelve either, | |
| 18:55:36 | sean-k-mooney | a concreate example would be if the instance was pinned and the original volume was create from an image with hw:numa_nodes=1 and the new volume was created form an iamge with hw:numa_node=2 it will invalidate the pinnings | |
| 18:55:39 | mriedem | so as far as i know when we unshelve we're not using the new image meta anyway | |
| 18:56:04 | sean-k-mooney | oh well it could be broken in both cases | |
| 18:56:11 | sean-k-mooney | i didnt actully check the code | |
| 18:56:54 | sean-k-mooney | we do get the metadata from the volume when initally spawning the instace | |
| 18:57:03 | mriedem | yeah i know, and on rebuild | |
| 18:57:06 | sean-k-mooney | i assume we would do it again on unselve but maybe not | |
| 18:58:15 | mriedem | no, | |
| 18:58:33 | mriedem | because on unshelve you either boot from the shelve snapshot (if not volume-backed), otherwise you boot from the root volume, | |
| 18:58:36 | mriedem | which before this couldn't change | |
| 18:59:32 | sean-k-mooney | right but on unselve we would have hit the schuler right. i guess it uses the embeded image metadata so it does not have to go back to cinder | |
| 18:59:58 | mriedem | yes unshelve hits the scheduler | |
| 19:00:10 | mriedem | and it would use whatever is in the request spec from the original server create | |
| 19:00:42 | mriedem | which uses the volume image meta here https://github.com/openstack/nova/blob/af78b13c24d4abf393d17ac57e9135204ef12b73/nova/compute/api.py#L886 | |
| 19:01:10 | sean-k-mooney | ya. so we could enable this to work for unselve by updating the request spec or just regtiving the metadata form the volume but i dont know how to "fix" start of a powered off instance | |
| 19:01:41 | sean-k-mooney | i would expect the image to go into an error state on start if its requirements chagne | |
| 19:01:51 | sean-k-mooney | but that is not very friendly | |
| 19:02:03 | sean-k-mooney | or start with the old requirement i guess | |
| 19:02:46 | mriedem | inititally the spec never said anything about supporting detach/attach of the root volume for powered off instances | |
| 19:02:50 | mriedem | only shelved offloaded | |
| 19:02:56 | mriedem | which is how hpe wrote it up long ago | |
| 19:02:57 | sean-k-mooney | yep in mitaka | |
| 19:03:25 | mriedem | i do remember that the RequestSpec.image.id is *not* set for volume-backed servers, so some of our scheduler filters don't work on volume-backed servers | |
| 19:03:52 | mriedem | like IsolatedHostsFilter | |
| 19:04:21 | mriedem | during server create, we do calculate the numa topology requirements from the flavor and image meta, and for volume-backed we get that image meta from the root volume, like you said | |
| 19:04:34 | mriedem | so technically on unshelve we could screw up and not honor the image meta in the new root volume | |
| 19:04:42 | mriedem | b/c we don't update the request spec | |
| 19:05:57 | sean-k-mooney | right. i think that is just an oversight however. e.g. readign the spec i would have assume we woudl cater for that edgecase in the implemnation and they jsut chose not to document it in the spec | |
| 19:06:29 | mriedem | i wouldn't be so sure | |
| 19:06:41 | sean-k-mooney | anyway it looks like newton was still only allowing this for shelved instance so its only allowed for powered off instance in the stein spec | |
| 19:07:16 | mriedem | i'm pretty sure i didn't think about the root image meta changing when s10 pushed for supporting attach/detach of the root volume on stopped instances | |
| 19:07:17 | sean-k-mooney | mriedem: im giving people the benifit of the doubt but perhaps it was not taught of | |
| 19:09:40 | sean-k-mooney | so with 2 weeks to feature freeze i dont really want to reopen the design of this given how long people have been waiting for it. | |
| 19:10:37 | sean-k-mooney | i could propose a revision to the spec to reduce it to only shelved insntace or to document the behavior that will happen for poered off instace | |
| 19:11:16 | sean-k-mooney | *powered off | |
| 19:11:47 | sean-k-mooney | im going to grab dinner but ill be back in an hour or so | |
| 19:12:08 | sean-k-mooney | actully i have to push a small revision to a patch then dinner | |
| 19:13:47 | mriedem | sean-k-mooney: i commented on https://review.openstack.org/#/c/614750/ as well to recap our irc discussion. i'll talk with Kevin_Zheng about it tonight since we have a meeting anyway | |
| 19:14:55 | mriedem | i don't think we can support the stopped case - that would essentially get us back to volume-backed rebuild with a new image that we don't really honor | |
| 19:16:11 | sean-k-mooney | mriedem: ya that was my feeling too but i could see use allowing it and refusing to boot the instacne if the requirement could not be supported | |
| 19:16:20 | sean-k-mooney | that felt overly complicated however | |
| 19:16:42 | sean-k-mooney | mriedem: can we shelve a powered off instance? | |
| 19:17:24 | mriedem | yes | |
| 19:17:31 | mriedem | which i think i said in the spec review when this came up, | |
| 19:17:46 | mriedem | just stop the instance, shelve it, swap the root volume, and unshelve, | |
| 19:17:47 | sean-k-mooney | ok so you jsut need to shelve it and then unshelve after changing volumes and you can more or less achive the same use case | |
| 19:17:52 | mriedem | but people thought that was too complicated | |
| 19:18:11 | sean-k-mooney | perhaps but ill take complicated over broken any day | |
| 19:22:29 | mriedem | sean-k-mooney: i guess i did think about this https://review.openstack.org/#/c/600628/7/specs/stein/approved/detach-boot-volume.rst@127 | |
| 19:25:27 | sean-k-mooney | mriedem ya looks like you did but it never made it into the spec in the end. | |
| 19:26:12 | sean-k-mooney | mriedem: atleast a concreate proposal for what to do in this case was not added to the spec. your comment are obviously there | |
| 19:28:30 | openstackgerrit | sean mooney proposed openstack/os-vif master: modify functional base.py to allow using vscode https://review.openstack.org/638058 | |
| 19:30:22 | mriedem | yeah i just don't want to get into a case where someone swaps the root volume on a stopped instance, then they rebuild it with what they thought was the original volume and rebuild blows up saying you can't rebuild a volume-backed instance with a new image, or they pass the image_id of the new root volume and the scheduler kicks it out saying it's not valid for the current host | |