| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-05-12 | |||
| 15:49:10 | sean-k-mooney | ya i was about to ask the same https://opendev.org/openstack/nova/src/branch/master/nova/conf/glance.py#L64 | |
| 15:49:32 | dansmith | lyarwood: not sure what group, but yes, hang on | |
| 15:49:53 | sean-k-mooney | its in the glance group | |
| 15:49:59 | jsuchome | yep | |
| 15:50:27 | dansmith | if 'rbd' in CONF.glance.allowed_direct_url_schemes: | |
| 15:50:27 | sean-k-mooney | do we plan to turn this on by default | |
| 15:50:36 | dansmith | no | |
| 15:50:49 | jsuchome | it should be documented | |
| 15:50:56 | sean-k-mooney | ok we proably should remove the deprecation of that option in jsuchome patch | |
| 15:51:01 | sean-k-mooney | i assume it already does that | |
| 15:51:22 | dansmith | the spec says we will undeprecate it yes | |
| 15:51:28 | sean-k-mooney | k | |
| 15:51:39 | jsuchome | does it need a release note? | |
| 15:51:46 | dansmith | yes | |
| 15:51:56 | sean-k-mooney | the feature would even without the undeprecation | |
| 15:52:01 | sean-k-mooney | the same one can cover both | |
| 15:52:39 | jsuchome | ok, than it's another change for 574301 | |
| 15:53:18 | dansmith | jsuchome: I probably wouldn't pile that in there personally | |
| 15:56:48 | jsuchome | OK, another patch, no problem | |
| 15:57:03 | dansmith | jsuchome: at least for the moment, we can always squash | |
| 15:57:24 | dansmith | I think that by the time you get all the test stuff in this patch it will be plenty meaty | |
| 15:57:54 | dansmith | one could argue that it could go in my first patch to remove the plug point, but that just means it has two semi-related atomic changes | |
| 15:57:57 | dansmith | patches are cheap | |
| 15:58:30 | jsuchome | yeah, and for tommorrow I got to work on the tests, I can see they are not enough | |
| 15:58:38 | dansmith | cool | |
| 16:09:26 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: WIP: allow disabling image cache for raw images https://review.opendev.org/727261 | |
| 16:09:30 | gibi | dansmith, sean-k-mooney: I looked at how easy is to disable the image cache and this is my first stab on it (seem to work in devstack) ^^ | |
| 16:10:19 | gibi | I will have to disappear now but feedback is appreciated | |
| 16:11:12 | dansmith | gibi: I don't understand that | |
| 16:13:52 | sean-k-mooney | dansmith: i assumed it was just me :) | |
| 16:14:19 | sean-k-mooney | also i expected to not underdstand that when looking at that code | |
| 16:18:44 | dansmith | yeah | |
| 16:19:05 | dansmith | I'll have to get all dug-in to that code again to really be able to speak intelligently about it, | |
| 16:19:15 | dansmith | but that surely seems to be basically doing the same thing as above at first glance | |
| 16:22:01 | dansmith | sean-k-mooney: ah, maybe because this is the Flat implementation | |
| 16:22:30 | dansmith | but I really thought that it still cached even though it flattened the image before giving it to the instance, which this doesn't seem to change | |
| 16:23:32 | sean-k-mooney | dansmith: it is cached | |
| 16:23:37 | sean-k-mooney | we copy it | |
| 16:23:48 | sean-k-mooney | on line 598 | |
| 16:24:00 | sean-k-mooney | https://review.opendev.org/#/c/727261/1/nova/virt/libvirt/imagebackend.py@598 | |
| 16:24:08 | dansmith | yeah | |
| 16:24:15 | dansmith | I wonder if gibi tested this and we're missing something, or he's assuming something else | |
| 16:24:37 | sean-k-mooney | well this would also have to be done for qcow right | |
| 16:24:38 | dansmith | sean-k-mooney: we only copy if it doesn't exist, and he's passing self.path as the target | |
| 16:24:54 | dansmith | but, so does the "if generating" case above | |
| 16:25:32 | sean-k-mooney | right so if self.path whcih shoudl be the imnstance disk path does not exist | |
| 16:25:33 | dansmith | so I wonder if we're normally in Flat to grab the base image, and this makes for another case where we just download the image to our target when not caching and that somehow bypasses, | |
| 16:25:44 | sean-k-mooney | before we would take the else path and create a copy form the base path | |
| 16:25:46 | dansmith | but I think the call path to the caching is too loopy to tell that | |
| 16:26:10 | sean-k-mooney | i think this will actully work but only for the flat backend | |
| 16:26:10 | dansmith | disabling the image cache entirely is also somewhat of a way-too-big hammer to solve this problem | |
| 16:26:17 | dansmith | it's a workaround maybe, but it's really a terrible one | |
| 16:26:26 | sean-k-mooney | ya | |
| 16:26:37 | sean-k-mooney | did you see my converstaion with gibi this morining | |
| 16:26:43 | dansmith | I did | |
| 16:26:50 | dansmith | well, I skimmed it | |
| 16:27:05 | sean-k-mooney | cool what do you think of the idea of creating allocation for the cached images inplacment | |
| 16:27:12 | dansmith | sean-k-mooney: I suggested that yesterday | |
| 16:27:13 | sean-k-mooney | so we can track how much is being used | |
| 16:27:17 | sean-k-mooney | ok cool | |
| 16:27:27 | dansmith | we have to be able to do it separately if the cache is on a different filesystem though, | |
| 16:27:35 | dansmith | which will get messy, especially if you move it later | |
| 16:27:46 | sean-k-mooney | ya | |
| 16:27:48 | dansmith | and we'll need new healing stuff to make sure we can recover from getting out of sync | |
| 16:28:03 | sean-k-mooney | and if people put the cache on nfs or somehting it will be even worse | |
| 16:28:18 | dansmith | yes | |
| 16:28:35 | dansmith | so yeah, it's a thing, but it's not trivial for sure | |
| 16:28:36 | sean-k-mooney | when you say self heling are you thinking nova audit or a periodic taks in the agent | |
| 16:29:03 | dansmith | yeah, just anything that causes us to leak some allocations for images that are no longer cached, | |
| 16:29:09 | dansmith | and also notice that new images are on disk that aren't allocated | |
| 16:29:29 | dansmith | because we already tell customers they can sideload images into the disk cache and nova will (rightly) use them propery | |
| 16:29:33 | dansmith | but wouldn't have alocations for them | |
| 16:29:34 | sean-k-mooney | dansmith: ideally i was think as well that it would make the caching best efffort. e.g. it would check if it was in the cache. if not check if it can create an allcotion for the image and cache it if it can and dont cache if it cant | |
| 16:30:01 | dansmith | yes, but if it can't then it has to delete that allocation of course, | |
| 16:30:14 | dansmith | and if everything goes nuts during that, we have to be able to heal away those stale ones when we reboot | |
| 16:30:14 | sean-k-mooney | dansmith: do we actully support the sideloading or just it should work | |
| 16:30:25 | dansmith | and if images get sideloaded, we have to notice, and if images get locally deleted, we have to notice | |
| 16:30:36 | dansmith | sean-k-mooney: it does work and we (redhat) have prescribed it in a few cases :) | |
| 16:31:02 | sean-k-mooney | .... im glad you have added the image cacahing feature in teh api to have an alternitive noew | |
| 16:31:03 | dansmith | and I know people have in the past manually purged images from that cache before the timer fires | |
| 16:31:35 | dansmith | sean-k-mooney: yep, but that only works (by design) per aggregate, and doesn't let you purge, | |
| 16:31:59 | sean-k-mooney | ya | |
| 16:32:06 | sean-k-mooney | sitll an improvement | |
| 16:32:08 | dansmith | so I think it would be foolish and fragile to not be able to reconcile the state of the disk with the other system | |
| 16:32:19 | dansmith | for sure | |
| 16:34:31 | sean-k-mooney | would you be ok with the compute agent calling placment to make those allocaitons and clean them up. i think it should be fine since its already updating placment in the update_avaiable_resouces funtion just not sure if i missed anything | |
| 16:35:01 | sean-k-mooney | the compute node already needs to be able to reach the placmenet api so it really should not be much of a change in that regard | |
| 16:37:01 | dansmith | the compute node the _only_ thing that _could_ do it... | |
| 16:39:49 | sean-k-mooney | well normally we do the allocation candiate request in the conductor right and we claim the allocation before we get to the compute | |
| 16:40:09 | sean-k-mooney | so the second allcotion for the cache image would have to be done on the ocmpute node and the claim | |
| 16:40:33 | sean-k-mooney | but ya the compute node is really the only thing that could keep them in sync | |
| 16:41:03 | dansmith | but nothing outside the compute node knows about the state of the cache | |
| 16:41:17 | dansmith | so nothing else would have any idea if an image is cached or could be cached | |
| 16:41:24 | sean-k-mooney | yep | |
| 16:41:34 | sean-k-mooney | and even if they did it would be racey | |
| 16:41:53 | dansmith | you mean "even if they tried to guess" :) | |
| 16:41:54 | sean-k-mooney | so that means the healing task could not be part of nova audit | |
| 16:42:00 | dansmith | no | |
| 16:42:28 | sean-k-mooney | it would have to be in the compute manager which is fine | |
| 16:43:12 | dansmith | image cache management is kinda weirdly split between the compute manager and virt driver | |