| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-05-12 | |||
| 13:48:02 | dansmith | jsuchome: hey, I +2d that spec and then realized you forgot to address one critical piece of feedback from the last version | |
| 13:48:16 | dansmith | jsuchome: *other* than that, I was going to be happy with it :) | |
| 13:48:48 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/queens: stable-only: skip volume backup tests in cellsv1 job https://review.opendev.org/727150 | |
| 13:59:06 | jsuchome | dansmith: I see, I messed up the test part. And the other part I did not address intentionally, I'm not sure it deserves detailed explanation in the spec | |
| 13:59:48 | jsuchome | dansmith: the 'significant changes' in glance.py are really only about executing extra download method under given conditions. And I think this is already covered | |
| 14:01:58 | dansmith | jsuchome: you're changing the base existing glance code more than just calling the rbd function and returning the way the plug point used to work, so I think it's pretty relevant | |
| 14:02:13 | dansmith | jsuchome: there's also no documentation about it in the code change, so I don't even know what the goal of the change is | |
| 14:03:05 | dansmith | jsuchome: it does't need to be detailed, but it should be something, IMHO. | |
| 14:04:10 | jsuchome | I tried to explain it already but now I see I forgot to post my changes ... | |
| 14:05:17 | jsuchome | dansmith: I actually added some comment to that part you are (probably?) mentioning, I assume you mean "Load chunks from the downloaded image file... " bit | |
| 14:05:26 | jsuchome | I've added some comments to the spec | |
| 14:05:57 | dansmith | yeah, so re-reading the code this morning, it looks like we used to not do the image signature verification for things downloaded with the per-scheme handler, | |
| 14:06:03 | jsuchome | (I mean: I've added some comments 1. to the code and 2. now I've also commented the spec) | |
| 14:06:08 | dansmith | and this change is lining it up so we do right? | |
| 14:06:55 | jsuchome | yeah, in previous version, this signature verification was just skipped if there was anything downloaded by the download handler | |
| 14:07:20 | jsuchome | So you think it still should be mentioned in the specs? | |
| 14:08:14 | jsuchome | Maybe I could just mention it in the commit message of the code change | |
| 14:08:29 | dansmith | yeah, so just a line in the spec under proposed change like this is fine: "The glance module also never used to perform image signature verification when the per-scheme module was used. Since we are moving this into core code, we will also fix this so that per-scheme images are verified like all the rest." | |
| 14:08:55 | dansmith | jsuchome: please just add that one line (or something like it) to the spec when you fix the test thing and we can move on | |
| 14:09:17 | dansmith | it should also go into the code change commit message, btw | |
| 14:09:44 | jsuchome | OK, both places then. In a minute | |
| 14:10:15 | dansmith | honestly, if I was doing it, I would break the code into two pieces, one for that fix and one for the rbd module being added | |
| 14:10:45 | dansmith | but, it's already done, so. | |
| 14:12:37 | dansmith | (biab) | |
| 14:12:38 | jsuchome | yeah, it's not exactly part of the feature. Seems like the original author realized it during testing, it appeared in some later PS | |
| 14:18:16 | openstackgerrit | Jiri Suchomel proposed openstack/nova-specs master: Add spec for downloading images via RBD https://review.opendev.org/572805 | |
| 14:18:17 | 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:32:06 | dansmith | jsuchome: +2 on the spec, thanks | |
| 14:32:47 | jsuchome | cool | |
| 14:48:22 | jsuchome | dansmith: two questions: 1. should I add that admin guide change into the same PS as the one with code or rather a new one? 2. where is that ceph CI job I should try to copy& adapt? (I've never done this part before) | |
| 14:48:50 | dansmith | jsuchome: no, do it in a separate patch please | |
| 14:49:32 | dansmith | jsuchome: I'm not super up on the state of the job (i.e. whether it's a legacy or converted job), nor where those bits live depending | |
| 14:49:36 | dansmith | but I bet sean-k-mooney knows | |
| 14:50:10 | sean-k-mooney | which job | |
| 14:50:12 | sean-k-mooney | ceph | |
| 14:50:32 | sean-k-mooney | i think its converted but ill check | |
| 14:51:53 | openstackgerrit | Ghanshyam Mann proposed openstack/python-novaclient master: Bump hacking min version to 3.0.1 https://review.opendev.org/727214 | |
| 14:52:18 | sean-k-mooney | devstack-plugin-ceph-tempest-py3 i think is a zullv3 job. we are not defining any legacy playbooks for ti but it comes form devstack not zuul config | |
| 14:52:19 | dansmith | sean-k-mooney: jsuchome needs to take that job, tweak the config on the compute node slightly, and get a one-off run of it at least | |
| 14:52:54 | sean-k-mooney | dansmith: ah ok ill jsut triple check that its zuulv3 if so that is simpel to do | |
| 14:54:15 | gmann | that is zuulv3, derived from tempest-full-py3. | |
| 14:54:16 | lyarwood | https://review.opendev.org/#/c/708038/ is an example of me messing around with the ceph job recently | |
| 14:54:26 | sean-k-mooney | yep its zuul v3 https://github.com/openstack/devstack-plugin-ceph/blob/master/.zuul.yaml#L57-L130 | |
| 14:55:04 | sean-k-mooney | jsuchome: what sepcfically do you need to add | |
| 14:55:49 | dansmith | lyarwood: ah nice | |
| 14:56:15 | dansmith | might need a DNM change against devstack to hack the half behavior into place and then control it with something like lyarwood's example | |
| 14:56:48 | sean-k-mooney | dansmith: you should not need too you can create a job that uses it as a parrent and then add your changes | |
| 14:56:51 | dansmith | sean-k-mooney: he needs to set up all the ceph stuff, but configure the compute to *not* use rbd backend, and have a new conf option set to enable direct-from-ceph download | |
| 14:57:44 | sean-k-mooney | so jsuchome just need to override the imagebackend in the nova.conf to be qcow2 | |
| 14:57:45 | dansmith | sean-k-mooney: depends on how the ceph bit works in devstack right? in lyarwood's example above, he had https://review.opendev.org/#/c/708035/ for that reason I think | |
| 14:58:14 | sean-k-mooney | e.g. lev devstack and the plugin do its thing but just tell nova not to use it | |
| 14:58:21 | dansmith | sean-k-mooney: probably enough, as long as that sticks and devstack or something downstream doesn't override | |
| 14:58:36 | sean-k-mooney | ceph is set up by https://github.com/openstack/devstack-plugin-ceph | |
| 14:58:49 | sean-k-mooney | but if jsuchome does something like https://review.opendev.org/#/c/724387/3/.zuul.yaml | |
| 14:59:30 | sean-k-mooney | which is using the local.conf [[post-config:/etc/nova/nova.conf]] mechanism to set config options that will run after the plugin | |
| 14:59:36 | dansmith | sean-k-mooney: will that override what the plugin does? | |
| 15:00:13 | sean-k-mooney | ya i belive the order is intree modules then plugins then post-config form local.conf | |
| 15:00:17 | lyarwood | wait, to disable ceph on Nova that plugin has a few variables you can set in the job | |
| 15:00:24 | lyarwood | ENABLE_CEPH_NOVA=false etc iirc | |
| 15:00:39 | sean-k-mooney | that would deploy it for cinder only then? | |
| 15:00:46 | lyarwood | glance etc | |
| 15:00:46 | sean-k-mooney | or glance i guess | |
| 15:00:53 | lyarwood | I think that's the point of this test | |
| 15:01:06 | dansmith | needs to be set for glance, yeah | |
| 15:01:06 | lyarwood | but that might be missing the required creds to download over RBD that I assume is the point here | |
| 15:01:08 | sean-k-mooney | ya so that sound like it would work | |
| 15:01:10 | dansmith | right | |
| 15:01:21 | dansmith | he still needs all the regular rbd config, just not the imagebackend part | |
| 15:02:07 | sean-k-mooney | ya so basicaly what lyarwood example is doing is exactly what needs to be done. | |
| 15:02:12 | dansmith | yup | |
| 15:02:19 | sean-k-mooney | is jsuchome about | |
| 15:02:37 | sean-k-mooney | if not i can submit a ptach that does that but i dont know what to put it on top | |
| 15:02:54 | dansmith | hang on | |
| 15:03:04 | jsuchome | sean-k-mooney: tweak few options, mostly for nova and one for glance | |
| 15:03:26 | dansmith | sean-k-mooney: https://review.opendev.org/#/c/574301/ | |
| 15:03:29 | jsuchome | (sorry I wasn't following for a while) | |
| 15:04:14 | sean-k-mooney | clip notes the devstack-ceph plugin has an env varible that you can set to disable just the nova change it does | |
| 15:04:25 | sean-k-mooney | so you can have it deploy cpeh for just glance and cinder | |
| 15:05:02 | sean-k-mooney | so that should be easy to do im not sure what else you need to set but there is a simple way to set config options we can use | |
| 15:05:35 | sean-k-mooney | if you need to do something more involved then you need to use a pre playbook to configure the jobs properly or a local.sh script | |
| 15:06:35 | jsuchome | I think I need to test if nova correctly downloads image and spawns VM, and I need ceph as glance backend, and few options for glance and nova set for that | |
| 15:07:21 | dansmith | jsuchome: what I'm asking for is to just configure fordirect download, make sure tempest runs normally, and ten we can look at the logs to convince ourselves that it's working | |
| 15:07:23 | sean-k-mooney | ok ill put up a DNM patch that disablel the unnned jobs and just disables using ceph for nova | |
| 15:07:28 | sean-k-mooney | then we can tweak that | |
| 15:07:35 | dansmith | jsuchome: not a direct test that confirms it was somehow downloaded direct -- that would be very hard | |
| 15:08:18 | jsuchome | true, tempest run should indeed be enough | |
| 15:08:37 | dansmith | (and logs) | |
| 15:08:57 | dansmith | jsuchome: if you think that you need to add some logs to be able to validate it from the outside, you should go ahead and do that | |
| 15:09:24 | dansmith | jsuchome: perhaps a LOG.debug('Downloading %(image)s from per-scheme handler %(handler)s') that we can look for | |
| 15:11:10 | jsuchome | There's already "Successfully transferred using rbd" log line | |
| 15:11:21 | dansmith | okay cool | |
| 15:11:26 | jsuchome | LOG.info I beleive | |
| 15:12:08 | dansmith | jsuchome: ah you mean the "attempting to export" one | |
| 15:12:17 | dansmith | oh, no, | |
| 15:12:20 | dansmith | I see that one too | |
| 15:12:26 | dansmith | okay cool, should be covered and easy to validate | |
| 15:17:25 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.moves https://review.opendev.org/727224 | |
| 15:18:12 | lyarwood | jsuchome / sean-k-mooney ; https://review.opendev.org/727225 might work | |
| 15:18:16 | sean-k-mooney | dansmith: so before i push this | |
| 15:18:31 | lyarwood | ah sorry you're also working on it | |