| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-07 | |||
| 17:47:00 | karimull | Looking for a review on https://review.openstack.org/#/c/565620/ from a core member | |
| 17:48:29 | mriedem | jroll: ^? | |
| 17:50:10 | jroll | mriedem: looks fine | |
| 17:50:29 | jroll | pretty straightforward :) | |
| 17:57:33 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Add spec for downloading images via RBD https://review.openstack.org/572805 | |
| 17:58:50 | mriedem | dansmith: +2 on this pretty simple spec about using rbd for getting images from local storage even when you're not running the rbd image backend https://review.openstack.org/#/c/572805/ | |
| 17:59:06 | mriedem | related to that thread in the ML about the image handler download extension point deprecation | |
| 17:59:23 | mriedem | i have a feeling CERN has also said they wanted something like this for windows images | |
| 18:05:19 | dansmith | hmm | |
| 18:05:23 | dansmith | I didn't realize we still had that | |
| 18:05:25 | dansmith | it's hidden in __init__ | |
| 18:05:58 | melwitt | mriedem: thanks for following that btw, I had meant to ask mnaser to chime in about it too because I thought maybe it was something he also needs | |
| 18:08:14 | mriedem | i felt compelled since i deprecated it and sent out the call for input in the ML | |
| 18:08:22 | melwitt | :) | |
| 18:08:22 | mriedem | and actually caught a bite | |
| 18:10:44 | dansmith | we don't have to undeprecate the modules thing for this, | |
| 18:11:05 | dansmith | we can just convert it to a be_smart=True sort of flag I think | |
| 18:11:50 | dansmith | approving this makes me worried that someone that understands the image cache is going to show up and cry foul, | |
| 18:12:09 | dansmith | and it's not very helpful to look at a havana implementation | |
| 18:13:04 | dansmith | I also feel like there are things in that spec you would call out as needing more detail if it wasn't spec deadline day and you weren't flustered by getting a bite on the mailing list | |
| 18:14:27 | dansmith | obviously the idea seems sane | |
| 18:17:26 | mriedem | there are things i thought about like, how would one disable this if they wanted, but i expect that's just (1) if you're using rbd and you want it for both ephemeral and persistent disks, you use images_type=rbd, and (2) if you want ceph for volumes and qcow2 for local, then you don't configure the libvirt ceph stuff for local | |
| 18:17:34 | mriedem | so that doesn't seem like a problem | |
| 18:17:57 | mriedem | otherwise as i said in the review, devil is in the details when the patch shows up, but otherwise yeah spec deadline and such | |
| 18:18:13 | dansmith | I just don't have all that image cache plumbing in my head to know if it's legit to do it like this or not | |
| 18:18:20 | mriedem | i could very obviously be missing stuff and sort of kind of feel bad about this, so if there are issues you see then please don't hold back | |
| 18:18:26 | dansmith | just would feel better if someone else that kinda understands that had even looked at it I guess | |
| 18:18:31 | mriedem | mdbooth | |
| 18:18:33 | mriedem | to the rescue | |
| 18:19:38 | mriedem | also, my house is unfortunately now overrun with children since the school system decided it won't hold school year round, and i must evacuate to a coffee shop, so bbiab | |
| 18:20:21 | dansmith | melwitt: do you know more about this process and/or feel like due diligence has been done on this elsewhere? | |
| 18:21:09 | dansmith | once it is approved, it's candidate for a runway and subsequent auto-approval next cycle. the spec has been up for <24 hours and the ML post didn't get much discussion at all | |
| 18:21:21 | melwitt | dansmith: no, I don't have that much knowledge about what the extension point is providing | |
| 18:21:37 | dansmith | as long as the opt stays deprecated and not removed we're not screwing this guy over (yet) | |
| 18:22:03 | melwitt | that is, I don't want us to lose the functionality but I don't know what the right way to provide it is | |
| 18:22:04 | dansmith | and/or we could just give it a spec exception until next week in hopes that we could get a quick read from anyone who knows anything about this to trivially thumb it up or something | |
| 18:22:19 | dansmith | the spec is about adding a feature, | |
| 18:22:24 | dansmith | it's not about the extension point itself | |
| 18:22:40 | dansmith | the guy is offering to upstream his thing so that we could remove the extension point basically | |
| 18:22:48 | dansmith | but I dunno if his thing is a total hack or not | |
| 18:22:53 | melwitt | but it's about adding a feature that provides the same functionality that the deprecated extension point is currently taking care of? | |
| 18:22:57 | dansmith | no | |
| 18:23:06 | dansmith | it uses the extension point to add the feature out of tree currently | |
| 18:23:27 | dansmith | it's coming up because if the extension point goes away, he can't sideload his extra thing | |
| 18:23:48 | dansmith | the confusion may be that he asserted that bringing it into tree means we keep the extension point, but that's not really a thing | |
| 18:25:27 | melwitt | hm, okay. yeah, I would be okay with a freeze exception to review this since it showed up only yesterday and if it's an important operator issue | |
| 18:26:06 | dansmith | I'm not sure it is an important operator issue, because the thread on the ML was a week ago and nobody has really jumped on it | |
| 18:26:12 | melwitt | I just haven't connected the dots between it and the ML post, I had assumed it was proposed in reaction to the deprecation of the extension point and as a way to keep the fast download behavior | |
| 18:26:24 | dansmith | we don't have the fast download behavior now | |
| 18:26:30 | dansmith | this guy hacks it in with an out of tree extension, | |
| 18:26:34 | dansmith | which uses this extension point | |
| 18:26:43 | dansmith | he wants to upstream it so he can keep the functionality after we remove the extension point | |
| 18:26:47 | melwitt | oh, I see | |
| 18:26:53 | dansmith | which is awesome and stuff, | |
| 18:27:22 | dansmith | I just don't know if what he's describing that he's doing is a gross hack or legit, because Idon't know the image cache maze of caves very wekk | |
| 18:27:23 | dansmith | *well | |
| 18:27:33 | melwitt | yeah, gotcha | |
| 18:29:33 | melwitt | yeah, I dunno. that's a tough one, bad timing. mdbooth might be able to look at it tomorrow | |
| 18:29:51 | dansmith | the spec also kinda describes this as just bringing his extension into tree, which will still use the extension point, which is not what I think we should do | |
| 18:30:01 | dansmith | which is also why I think this needs another go-round at least :) | |
| 18:30:03 | dansmith | yeah | |
| 18:37:24 | melwitt | the concept makes a lot of sense, similar to the ceph fast clone stuff we have, I just don't yet understand why the extension point is needed to grab the image if we didn't need it for fast clone | |
| 18:39:49 | melwitt | I think the feature is definitely legit but the implementation details are the potential snag, but I think everyone said that already | |
| 18:44:37 | mriedem | dansmith: melwitt: i'm happy to sit on that until mdbooth can look it over, i'll drop the +2 | |
| 18:46:08 | mriedem | sounds like, if image isn't in cache, and images_type!=rbd but CONF.libvirt.rbd_user is configured, then look for the image in ceph, else download | |
| 18:46:38 | mriedem | which is presumably how this works today with the extension download handler | |
| 18:47:06 | melwitt | yeah, agreed sounds like it would be something like that | |
| 18:48:59 | melwitt | the only catch might be that you have to get it from the glance ceph pool. I don't think that's the case for the fast clone snapshot | |
| 18:49:41 | melwitt | but I guess it must be ... thinking about the glance direct urls | |
| 18:49:43 | dansmith | agreed that it sounds like the right thing to do, I just am unsure on the mechanics | |
| 18:49:50 | dansmith | if we can wait for mdbooth to look, I'd feel a lot better | |
| 18:50:11 | melwitt | yeah, that's fair enough | |
| 18:55:01 | mnaser | melwitt, mriedem: that spec is nice but it doesn't figure out our issue because we have 2 ceph clusters | |
| 18:55:29 | mnaser | for me the value would be if the image cache would use rbd instead of local storage | |
| 18:55:45 | mnaser | so we download the image once and then it remains cached inside the cluster | |
| 18:56:20 | mnaser | which is a problem i'm about to run into next week that i haven't fully thought out what/how i go about this :( | |
| 18:56:22 | mriedem | so maybe that's an alternative for the spec | |
| 18:56:26 | mriedem | make image cache ceph aware | |
| 18:56:53 | mriedem | mnaser: there are nova guidance counselors here to help :) | |
| 18:57:08 | mnaser | image cache and backend are decoupled.. or not? | |
| 18:58:13 | mnaser | mriedem: https://wiki.openstack.org/wiki/Technical_Committee_Tracker#Project_Teams i'm signed up ha | |
| 18:58:30 | mnaser | so if they are decoupled, that would solve that problem for that use case in that spec | |
| 18:58:36 | mriedem | ha | |
| 18:58:39 | mnaser | it would involve an extra import/export | |
| 18:58:58 | mnaser | because you'd be exporting it from glance once slowly and importing it to ceph, storing it potentially twice if its the same cluster | |
| 18:59:33 | mnaser | in my case i'd be storing it twice but in different clusters | |
| 19:01:13 | openstackgerrit | Jay Pipes proposed openstack/nova master: libvirt: Don't report DISK_GB if sharing https://review.openstack.org/560459 | |
| 19:01:59 | melwitt | cloned from it) | |
| 19:01:59 | melwitt | yeah, I dunno if I'm missing something but it sounds like we could do this by adding an extra step in the process, like instead of cow cloning from the glance pool, we take an extra step first to copy it and flatten to the nova pool (if does not exist) and then cow clone off of that instead. it would also decouple instances from the images in glance (problem of being unable to delete a glance image if any instance is in existing cow | |
| 19:02:32 | melwitt | *instance is existing that was cow cloned from it | |
| 19:02:38 | mnaser | melwitt: thats a pretty good way of going about things and it does remove that weird | |
| 19:02:45 | mnaser | "i cant delete my image!!!" complaints with rbd | |
| 19:02:48 | melwitt | yeah | |
| 19:03:22 | mnaser | the only thing is that does potentially slow down first boots | |
| 19:03:37 | mnaser | and i'm not sure how that would be handled if someone uploads an image and boots 10 VMs of it right away | |
| 19:03:38 | melwitt | yeah | |
| 19:04:44 | melwitt | heh, maybe this ties into that spec, rbd export, is that faster than clone + flatten or the same I would think? | |
| 19:05:07 | mnaser | i think flatten is faster because flatten doesnt involve the host that does it | |
| 19:05:42 | mnaser | export means you download as fast as your compute node, and upload as fast as your compute node in import | |
| 19:05:58 | melwitt | oh, okay | |