Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-05-13
13:18:47 openstackgerrit Takashi Natsume proposed openstack/nova master: Remove six.iteritems/itervalues/iterkeys https://review.opendev.org/727757
13:21:07 openstackgerrit Lee Yarwood proposed openstack/nova master: Add functional test for bug 1550919 https://review.opendev.org/631294
13:21:07 openstack bug 1550919 in OpenStack Compute (nova) "[Libvirt]Evacuate fail may cause disk image be deleted" [Medium,In progress] https://launchpad.net/bugs/1550919 - Assigned to Lee Yarwood (lyarwood)
13:21:07 openstackgerrit Lee Yarwood proposed openstack/nova master: libvirt: Don't delete disks on shared storage during evacuate https://review.opendev.org/578846
13:39:14 jkulik functional tests don't support testing the "uuid as list" case either https://github.com/openstack/nova/blob/master/nova/tests/functional/api/client.py#L248-L250
13:39:46 jkulik the lack of tests for 'uuid' in search_opts isn't good, though :D
13:45:45 sean-k-mooney jkulik: yep again i think that is becasue we did not orginally plan to expose this via the api and retroactivly had to try and fix it
13:46:15 openstackgerrit Jiri Suchomel proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.opendev.org/574301
14:24:36 openstackgerrit Takashi Natsume proposed openstack/nova master: Remove six.byte2int/int2byte https://review.opendev.org/727777
14:34:02 gmann jkulik: sean-k-mooney yeah, multi filters things are not supported. for this case where you want to list multiple servers you can use some other query filter which matches multiple servers. like ?name=test so it will return all servers matching with 'test*'
14:35:58 gmann remember, multiple filters are with AND condition
14:38:34 gmann in current behaviour only last present item is being used even you are asking for multi dict
14:41:03 gmann jkulik: it should not be hard to support that, we just need to change the way we fetch filter from GET and query DB with OR condition.
14:41:35 gmann jkulik: anyways, all API change need spec process, please feel free to add BP and spec for the same
14:43:30 gmann but remember any other filter present with mutli dict filter will be with AND condition. so you would not be able to do 'server-uuid1 OR (server-uuid2 AND vm_state=active)' it will be '(server-uuid1 OR server-uuid2) AND vm_state=active)'
15:32:43 lyarwood we should start a club :)
15:33:37 lyarwood gibi: I've not had a chance to look at your caching change btw, I'll try to get to it tomorrow
15:34:09 gibi lyarwood: dont rush it is a very incomplete messy pile of boo
15:36:02 gibi lyarwood: I just figured out that the current imagebacked code my temporarily use double of the image size disk space. For example if an image is a qcow in glance but nova configured to force_raw_images then after image download we copy out the raw data from qcow and the delete the qcow file we downloaded :/
15:36:17 gibi s/my/might/
15:36:47 gibi who thought that we will have disk space for that operation?!
15:38:03 gibi I get to feel that I have no power to patch this code in a way I imagined
15:38:26 dansmith lyarwood: is it a club if everyone is a member?
15:38:34 dansmith I think that's called "a population"
15:39:10 dansmith gibi: did you catch the scrollback of our convo yesterday?
15:39:20 lyarwood dansmith: ^_^
15:39:58 lyarwood gibi: yeah I don't think we can convert in-place tbh
15:40:01 gibi dansmith: yes. but honestly I have to go back to it as I already forget what was the pre-filter idea
15:40:43 dansmith gibi: so, I think it's not unreasonable to say "to boot an instance of $root from an image of $imgsize, the host needs to have $root+$imgsize free space, as a rule
15:41:21 dansmith although you know what..
15:41:32 dansmith I think we were totally missing something yesterday, now that I re-state that with a fresh mind
15:42:29 dansmith for that to even help, we have to account for the base in the inventory or allocations somehow, which is what we were trying to avoid there
15:42:36 dansmith so, hrmph
15:43:07 dansmith gibi: I will say that although I understand the desire to disable the cache as a workaround, it's really not a useful solution for anything other than a very small subset of cases
15:43:29 dansmith i.e. where you expect only one of each image type to be booted in a disk-constrained place anyway
15:43:43 gibi and I think it is pretty impossible to do properly due to what assumption the code currently makes
15:43:49 dansmith so I wouldn't want to spend a bunch of effort or cause a bunch of destabilization in the image backend to do so
15:43:57 dansmith gibi: right, hence my concern yesterday :)
15:44:18 gibi I needed to feel the pain to understand :)
15:44:25 dansmith :D
15:47:00 dansmith gibi: so, one thing I was wondering
15:47:03 gibi even if we start making allocation according to the images in the cache, the current code sometimes copy things over for conversions and that doubles the disk usage temporarly, or fail if no disk space for such operation
15:47:53 dansmith gibi: did you find somewhere that the DiskFilter considered the size of the image? Because even considering disk free space, you don't know if the target host will need room for the image (if it's not cached) PLUS the root, etc disks as well
15:48:22 dansmith because I don't think it really took that into account.. i.e. adding the flavor disks and the possible space required for a separate image
15:49:06 gibi DiskFilter used the disk_available_least value from the DB. There we count current available free space in the $instances_path (among other things)
15:49:21 dansmith gibi: yeah, and like I said yesterday if we start doing allocations for images, we have a lot of cases we need to cover, a lot of new potential needs for healing that data, etc.. it concerns me to make a decision to start doing that so quickly
15:49:37 gibi dansmith: ^^ agree
15:50:10 dansmith gibi: right, but the scheduler only looks to see if root+swap+ephemeral will fit in that space, but it may still fail because the compute node doesn't have the image cached already and thus need root+swap+ephemeral+image space to do the work
15:50:41 dansmith it's less of a problem, but it's similar
15:50:49 gibi correct (shit, another edge case)
15:52:08 dansmith so here's a couple of less impactful options:
15:52:56 dansmith 1. Each time we cache an image, or run the periodic, we generate a disk allocation for the compute node uuid which consumes inventory according to how much space the cache is using
15:53:04 dansmith 2. Same as above, but adjust the reserved amount
15:53:50 dansmith both cases need to consider the case where the _base is not on the same filesystem as the instances, but there's less synchronization involved, and we're not spraying tons of new allocations into place
15:54:34 dansmith we could also make it a workaround that you opt into in the short term to see how it goes, because cleanup from it would be much easier (just nuke one $cnuuid allocation)
15:55:18 dansmith and we could make compute node startup nuke that allocation if present and the workaround is disabled (or the self-correcting reserved amount)
15:55:54 gibi right, I would use allocation instead of reserved as allocation has a consumer attached
15:56:16 gibi reserved would be a sum of configured + detected
15:56:30 gibi which is math, that I dont like :)
15:56:31 dansmith hopefully glance gives us enough information to be able to increase that allocation before we start the download, so we know "oh sorry, inventory says we don't have room for this base image, so fail()"
15:56:52 dansmith I know it would, but I think reserved would be more obvious to an operator
15:57:00 dansmith even though it's a composite value
15:57:20 dansmith just because listing allocations are a bunch of meaningless-to-the-human UUIDs
15:57:46 dansmith I'm not arguing for that, I'm just saying there're benefits both ways
15:57:57 gibi I see. yes it is a tradeoff
15:58:37 dansmith I gotta get on a call
15:58:41 dansmith food for thought
15:59:42 gibi dansmith: thanks! I appreciate your help
16:00:40 gibi I've checked. glance give use the physical size of the image on the API. Which is good for ensuring we have still disk for the download
16:02:14 gibi also I think even if nova is converting a qcow2 glace image to raw locally (due to force_raw_images config) the resulting raw file is sparse
16:02:34 gibi but it might dependent on the host OS + file system support
16:26:39 gibi dansmith: if we start allocating / reserving the cache disk usage in placement then do we still need a pre-filter as well? For me it is OK to simply let the boot fail on the compute side if the extra DISK_GB resource for the cache cannot be allocated?
16:27:02 gibi s/?//
16:27:21 gibi anyhow documented your idea in the bug report
16:27:55 dansmith gibi: weighing aside, any compute with enough space for an instance but not enough for the instance+image will become a magnet for new builds, which will all fail
16:28:42 dansmith so, yeah, we can just pretend that's not worth solving, but it also sucks because avoiding that is what we're trying to solve with placement
16:31:22 gibi OK, I can imagine this as a two step solution then. First the allocation / reservation management in the compute then second the pre-filter that uses instance + image DISK_GB request but allocate only the instance disk for the instance_uuid in placement
16:32:52 dansmith perhaps
16:40:04 openstackgerrit melanie witt proposed openstack/nova master: DNM Try out running sphinx-build in parallel for releasenotes https://review.opendev.org/727429
16:48:40 openstackgerrit Balazs Gibizer proposed openstack/nova master: WIP: allow disabling image cache for raw images https://review.opendev.org/727261
17:23:58 openstackgerrit melanie witt proposed openstack/nova master: DNM Try out running sphinx-build in parallel for releasenotes https://review.opendev.org/727429
19:05:50 openstackgerrit Jiri Suchomel proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.opendev.org/574301
20:20:37 openstackgerrit melanie witt proposed openstack/nova master: DNM Try out running sphinx-build in parallel for releasenotes https://review.opendev.org/727429
20:54:18 openstackgerrit Sean McGinnis proposed openstack/nova master: DNM: Test making EM branch release notes static https://review.opendev.org/727875
23:15:54 melwitt gmann: do you understand why even after version bump we can get pep8 errors in gate? https://review.opendev.org/727589
23:18:10 gmann melwitt: it would be an error now as hacking min version bump will stop the new checks added in flake8 3.8.0 version.
23:18:28 gmann this one- https://review.opendev.org/#/c/727347/1
23:19:15 melwitt gmann: yeah but... (sorry) I thought bumping the version _stops_ the new checks from getting pulled in>
23:19:26 melwitt s/>/?/
23:19:45 gmann sorry *would not be an error
23:19:53 gmann missing *not*
23:20:31 melwitt ok, makes sense. so why second patch needed? is it nice to have for future flake8 or something>
23:20:36 melwitt gah I keep hitting >
23:21:10 gmann yeah for future but we will not be able to verify it or all error till we have new hacking pulling new checks.
23:21:47 melwitt yeah, ok.
23:21:51 gmann I am going to release the new hacking version 4.0.0 which will pull new checks and then in 727589 patch we will bump hacking version to so that we can verify the fix
23:22:38 gmann I mean fix + new hacking version bump in a single patch.
23:31:57 melwitt gmann: yeah makes sense. +2 on the 3.0.1 patch though I wondered if it's needed to avoid gate failures, would have thought we'd have a gate-failure bug around it. that is, it wasn't clear to me if the patch is needed to fix gate failure
23:34:24 gmann melwitt: I did nova patch earlier before i described the situation in other patches cmt msg like this- https://review.opendev.org/#/c/727576/
23:35:29 gmann melwitt: basically it will 1. fix local run where the latest fixed hacking 3.0.1 is not pulled automatically. in case of gate it is pulled as fresh installation. 2. it will protect if future flake8 3.9.0 version pull other new checks.

Earlier   Later