| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2020-07-31 | |||
| 17:46:46 | artom | In the context of fixing server rebuild parameters and bring them in line with the actual Nova API (https://review.opendev.org/#/c/741688/) - what's the difference between rebuild_server() in openstack/cloud/_compute.py and openstack/compute/v2/_proxy.py? | |
| 17:59:51 | openstackgerrit | Merged openstack/keystoneauth master: Correct major version discovery for non-keystone plugins https://review.opendev.org/743282 | |
| 18:19:20 | mordred | artom: openstack/cloud/_compute.py is the layer that used to be shade. it's the layer where we do a bunch of extra business logic for people. If you have a Connection called conn - you'd call that one like conn.rebuild_server - amongst the things it does is hide even which service a particular action comes from (a design that was more important back in the day of nova-image and nova-network alongside glance and neutron) | |
| 18:20:06 | mordred | artom: openstack/compute/v2/_proxy.py is a layer down - we have an eventual goal to have the shade layer use the resource layer instead of making direct rest calls | |
| 18:20:26 | mordred | artom: so - you know - all sorts of good history there :) | |
| 18:21:16 | artom | mordred, so no one is expected to call stuff in openstack/compute/v2/_proxy.py directly? | |
| 18:22:24 | mordred | artom: nah - either are actually fine - that both exist is a historical oddity at this point - with a few things we've gotten all the way to the shade layer being just a backwards compat shim over the other layer | |
| 18:22:45 | mordred | I personally just use the shade layer because it's less typing | |
| 18:23:54 | artom | mordred, so specifically in my rebuild case, shade doesn't actually call the compute proxy, it does the REST request directly | |
| 18:24:18 | mordred | at one point I was thiking that the proxy/resource layer should be the thing that more closely tracks the rest api and the cloud layer is where we do extra things - but it turns out we have to deviate from the rest api at the proxy/resource layer anyway in many cases, so the distinction is one that I think ultimately is not useful | |
| 18:24:32 | artom | Which means if I want to "fix" the rebuild_server args, I'd have to add **kwargs there as well, because there's a whole bunch of stuff that's in the API that's currently impossible with shade | |
| 18:25:36 | mordred | yeah - also, wow that's a terrible signature in that rebuild_server call in the sahde layer :( | |
| 18:25:46 | artom | Tell me about it :( | |
| 18:26:52 | mordred | I almost think we might be better off making the shade layer use your updated resource layer and just figure out how to make the backwards compat not totally suck. I hate that we called the image parameter image_id though :( | |
| 18:28:31 | artom | Ah, I see | |
| 18:29:10 | artom | Yeah, that's a really bizarre method signature | |
| 18:29:20 | artom | Some args are for the rebuild itself, others for when you show the server after | |
| 18:29:35 | artom | Can we burn and start over? :) | |
| 18:30:27 | mordred | at least the shade layer requires image as a parameter! ;) | |
| 18:31:06 | artom | Hehe, true | |
| 18:31:21 | artom | Ugh, I wasn't planning on thinking this much on a Friday afternoon | |
| 18:31:57 | mordred | so - I'm ok with breaking compat in the resource layer for this - like you said the current thing is pretty broken | |
| 18:32:29 | mordred | I think we can do the "update shade layer to use resource layer" as a followon - and we should be able to do it without breaking the shade layer's contract | |
| 18:32:41 | mordred | since we'll just be adding a **kwargs or something similar | |
| 18:32:50 | artom | Shovel down the line, future me can deal with it. I like how you think :) | |
| 18:33:00 | mordred | it'll just be lame that the parameter name is image_id - but if that's the worst thing that happens | |
| 18:33:31 | mordred | (I think we can improve that paramter to take image objects too and just document it - if people don't like the name they can pass it positionally :) ) | |
| 18:33:58 | mordred | future me is always smarter than today me anyway | |
| 18:35:08 | artom | Words of wisdom right there. | |
| 18:51:21 | mordred | artom: so - I'm +2 on that once there's a release note - probably with someting in the upgrade section | |
| 18:51:45 | artom | mordred, cool, I have that locally, need to fix the test on top of it, then push | |
| 18:52:00 | mordred | cool. | |
| 18:52:09 | mordred | I might take a stab at the shade patch once you do | |
| 18:52:12 | mordred | or I might leave that for tomorrow us | |
| 18:55:32 | artom | Hehe | |
| 19:43:34 | openstackgerrit | Artom Lifshitz proposed openstack/openstacksdk master: Func test for compute microversion 2.9 https://review.opendev.org/741689 | |
| 19:43:34 | openstackgerrit | Artom Lifshitz proposed openstack/openstacksdk master: Fix rebuild_server named argument https://review.opendev.org/741688 | |
| 20:37:29 | mnaser | mordred: so i think https://github.com/openstack/openstacksdk/commit/75ae5bf4aadedf47e26264f278c19adb64871735 might actually break every single OSA deployment out there :( | |
| 20:38:02 | mnaser | because by default the 'supports image import' is always enabled, even when we were deploying via uwsgi | |
| 20:38:06 | mordred | mnaser: well that's bad | |
| 20:38:20 | mnaser | dansmith pushed a patch to 'autodisable' that flag which was sadly -2d | |
| 20:38:30 | mordred | mnaser: well ... we default to false | |
| 20:38:34 | mordred | you have to opt-in to using import | |
| 20:38:47 | mnaser | mordred: https://review.opendev.org/#/c/741497/ | |
| 20:38:54 | mnaser | oh? well | |
| 20:38:59 | mnaser | i'm a little confused then because | |
| 20:39:12 | mnaser | https://de5fd82ca097fe66eeac-be64ab2c1211c8399b7ac2efe4751a63.ssl.cf5.rackcdn.com/744212/2/check/openstack-operator:functional/4959c4e/docker/k8s_glance_glance-zsvzl_openstack_71c819a2-2401-44f6-bbd7-6f222940b4c0_0.txt clearly makes /import calls | |
| 20:39:22 | mnaser | https://de5fd82ca097fe66eeac-be64ab2c1211c8399b7ac2efe4751a63.ssl.cf5.rackcdn.com/744212/2/check/openstack-operator:functional/4959c4e/job-output.txt but devstack is just doing openstack image create | |
| 20:39:43 | mordred | I mean - it shouldn't use import unless you explicitly request import - because even if it is there based on our discussion it's a less advantageous upload mechanism due to need for local things | |
| 20:39:45 | mordred | hrm | |
| 20:39:59 | mnaser | and --import is not actually being used | |
| 20:40:08 | mnaser | in the cli calls | |
| 20:40:17 | mnaser | i.e. `openstack --os-cloud=devstack-admin --os-region-name=RegionOne image create cirros-0.5.1-x86_64-disk --public --container-format bare --disk-format qcow2 --property hw_rng_model=virtio` | |
| 20:41:09 | mordred | yeah. did we get the osc patch wrong? | |
| 20:41:37 | mnaser | this is like... weirdly recent | |
| 20:41:40 | mnaser | i dont know what changed.. | |
| 20:41:57 | mordred | yeah - I don't see any code that should make that happen | |
| 20:42:05 | mnaser | gmann: ^ any ideas? this just recently surfaced | |
| 20:42:22 | mnaser | and there isnt much merged into osc lately | |
| 20:42:28 | mnaser | or openstacksdk | |
| 20:42:39 | mnaser | and i don't see anything tagged recently? | |
| 20:43:21 | mordred | nope. | |
| 20:43:40 | mordred | and I don't think we have any support to declaring that a cloud wants to default to import in clouds.yaml | |
| 20:45:01 | mordred | mnaser: | |
| 20:45:02 | mordred | use_import = True | |
| 20:45:02 | mordred | if stores or all_stores or all_stores_must_succeed: | |
| 20:45:14 | gmann | mnaser: it is enabled with GLANCE_USE_IMPORT_WORKFLOW flag in devstack case and that is false by default | |
| 20:45:28 | mnaser | gmann: yeah and in devstack, i don't see it using --import | |
| 20:45:42 | mnaser | but i think mordred is onto something, we're enabling import if we're specifying a store? | |
| 20:46:02 | mordred | we have to - apparently you can't specify stores without import | |
| 20:46:19 | mordred | I mean ... I don't see any stores listed in that command | |
| 20:46:31 | mordred | but maybe osc is passing al_stores by default? | |
| 20:46:32 | mnaser | yeah this is it: openstack --os-cloud=devstack-admin --os-region-name=RegionOne image create cirros-0.5.1-x86_64-disk --public --container-format bare --disk-format qcow2 --property hw_rng_model=virtio | |
| 20:47:14 | mordred | nope - osc doesn't have any support for stores | |
| 20:47:16 | mnaser | yeah | |
| 20:47:16 | mnaser | https://github.com/openstack/python-openstackclient/blob/8aed5feab8a5ae65658ea03fe1f2c238f6ffd908/openstackclient/image/v2/image.py#L399-L400 | |
| 20:47:38 | mnaser | https://github.com/openstack/python-openstackclient/blob/8aed5feab8a5ae65658ea03fe1f2c238f6ffd908/openstackclient/image/v2/image.py#L327-L334 | |
| 20:47:40 | mordred | mnaser: dude. that's weird | |
| 20:47:42 | mnaser | do we have to put a default=false or something? | |
| 20:48:41 | mnaser | and i dont think this has to do with my operator stuff cause the cli invocations dont change | |
| 20:49:41 | mordred | mnaser: zomg, having to put default=False would be super lame | |
| 20:49:57 | mordred | but - would be the best explanation | |
| 20:49:58 | mnaser | but normal devstack would have broken then no? | |
| 20:50:02 | mnaser | i think devstack uses uwsgi too | |
| 20:50:12 | gmann | mnaser: is it tempest-ful-py3 based job? becasue i enabled the import in that | |
| 20:50:47 | mnaser | gmann: i don't think its based off that BUT searching devstack logs shows `GLANCE_USE_IMPORT_WORKFLOW=False` so | |
| 20:51:30 | gmann | yeah it is parent with devstack-tempest | |
| 20:52:28 | mnaser | wth, a more recent change that ran -- https://review.opendev.org/#/c/668668/ -- did not have the same issue | |
| 20:52:42 | mnaser | that is 25 hours ago and i've noticed this surfaced very recently | |
| 20:52:45 | mnaser | like a cople hours ago | |
| 20:53:26 | mordred | mnaser: o_O | |
| 20:56:46 | mnaser | this change 5 hours ago was ok https://review.opendev.org/#/c/743943/ | |
| 20:56:57 | gmann | mnaser: devstack is default to standalone glance (not under uwsgi) and in your jobs also | |
| 20:57:14 | mnaser | ah well i run under uwsgi (the devstack portion is largely noop for glance at that point) | |
| 20:57:25 | mnaser | but even then looking at logs | |
| 20:57:32 | mnaser | its doing post /file | |
| 20:57:45 | mnaser | ohhhh | |
| 20:57:52 | mnaser | i wonder if https://review.opendev.org/#/c/743943/3/devstack/lib/glance is the reason | |
| 20:58:20 | mnaser | this further pushes the theory of enabled_import_methods mattering to the CLI | |
| 20:59:36 | mordred | mnaser: but how? | |