| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2022-10-28 | |||
| 08:06:40 | ard_ | jm1: yes, this does answer it. I didn't put it in myself, wanted to discuss first as there might been a reason for this. Especially with the kernel parameter present. Also, min_disk has a default value of 0, so you can't just do like you did. It's default value should be moved to _build_params function | |
| 08:07:32 | ard_ | jm1: here https://opendev.org/openstack/ansible-collections-openstack/src/branch/master/plugins/modules/image.py#L460 | |
| 08:12:55 | jm1 | ard_: why does min_disk not work? | |
| 08:14:51 | ard_ | if you run update image without this value it will be defaulted to 0 and if image had a limit, it will be reset to 0 | |
| 08:23:42 | jm1 | ard_: sure which is to be expected with default values. but i agree we should think about removing the default values for min_disk and min_ram | |
| 08:24:45 | jm1 | ard_: i would not mix create and update parameters though, since they are not necessarily the same | |
| 08:25:58 | jm1 | ard_: actually i see more stuff which could be refactored... lets focus on the important parts here and i will refactor this one later | |
| 08:41:34 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Existing images update name, visibility etc https://review.opendev.org/c/openstack/ansible-collections-openstack/+/862565 | |
| 08:44:12 | jm1 | ard_: does this ^ work for you? are we missing anything? | |
| 08:44:16 | ard_ | jm1: I guess having min_disk and min_ram set to 0 if they were not defined is a good idea during image creation, but it is not a good idea during image change. You might try to change only one attribute of the image. | |
| 08:44:58 | ard_ | jm1: and it might not be min_disk or min_ram. This way you'll get this value reset to 0 | |
| 08:45:16 | jm1 | ard_: glance uses 0 as the default for both min_disk and min_ram, so i dropped the default values | |
| 08:45:26 | ard_ | cool | |
| 08:46:15 | jm1 | ard_: those default values werent documented anyway :/ | |
| 08:47:21 | jm1 | ard_: have you been able to reproduce this image update issue when passing image instead of image.id? | |
| 08:47:49 | ard_ | jm1: I can on my sandbox | |
| 08:48:02 | ard_ | jm1: just in python shell | |
| 08:49:05 | jm1 | ard_: would you mind sharing the code and sdk's debug output with us (without secrets/tokens/..)? | |
| 08:49:17 | jm1 | so that gtema can have a look | |
| 08:49:46 | ard_ | jm1: and as I said before, I see the difference in formed body. I used pdb to trace what is tried to be sent. I was on the meeting and delayed a bit, but soon I will give you example of the script. | |
| 08:51:48 | jm1 | ard_: ack, fyi sdk has a enable_logging() function which gives all important output, no need to use pdb | |
| 08:55:07 | ard_ | jm1: yeah, but sometimes you want to run commands and check how the vars will work in a functions by your hand. I will check enable_logging() I am new to this, so using only tools I used before... | |
| 09:12:04 | jm1 | ard_: just mentioning it because you can use this with ansible modules too which are otherwise hard to debug: https://review.opendev.org/c/openstack/bifrost/+/859430/11/playbooks/roles/bifrost-configdrives-dynamic/tasks/main.yml#38 | |
| 09:14:31 | ard_ | jm1: thanks for this as well. I used epdb to connect to the process on remote systems running ansible tasks. This way I debugged module work. Just had small dependency in venv for the ansible. | |
| 09:21:43 | jm1 | ard_: like this? https://docs.ansible.com/ansible/latest/dev_guide/debugging.html#simple-debugging | |
| 09:22:14 | ard_ | jm1: The script to reproduce https://paste.opendev.org/show/817369/ | |
| 09:22:59 | ard_ | jm1: still looking how to sanitize debug sdk output. Even if it is for the sandbox in private network... | |
| 09:23:28 | jm1 | gtema: ^ | |
| 09:24:28 | ard_ | jm1: exactly and the same way, just always got tracebacks on during debugging and now I know to use raise instead of print :) | |
| 10:00:44 | ard_ | jm1: gtema: here is a log output https://paste.opendev.org/show/bpAMU2si0ezvN8Ckf3KP/ I tried to sanitize it... but it is on the sandbox in a private network, so should not have any sensitive data. | |
| 10:06:27 | jm1 | ard_: oh wow, what is this owner_specified.* stuff? gtema handing over to you :D | |
| 10:06:55 | gtema | owner_specified are the special props of sdk (checksum verification) | |
| 10:08:04 | ard_ | gtema: but it should not request them to be removed? | |
| 10:08:15 | gtema | nope, should not | |
| 10:08:46 | gtema | this means to me (on the first sight) that those props were not returned in the get and therefore it tries to drop them | |
| 10:09:15 | gtema | btw, what is the update_payload? | |
| 10:09:15 | gtema | ah, sorry | |
| 10:19:53 | gtema | ard_ I can say that your script just worked for me | |
| 10:20:12 | gtema | it is definitely wrong that it tries to unset owner_specified*, but it definitely worked | |
| 10:20:35 | ard_ | it change the image, but drop 409 | |
| 10:21:09 | gtema | 200 for me, I assume you fall into cloud specifics | |
| 10:21:14 | jm1 | gtema: where does this 'stores' come from and why is it passed to glance api? | |
| 10:21:14 | ard_ | gtema: you mean it is something with my sandbox | |
| 10:21:14 | gtema | which cloud you use? | |
| 10:22:17 | gtema | ard_ or is it real upstream deployed as private cloud? | |
| 10:22:46 | ard_ | it is aio deployment for testing purposes | |
| 10:23:08 | gtema | ok, never tried that | |
| 10:28:00 | gtema | I can reproduce it now once I try to set another property | |
| 10:47:30 | gtema | one awesome finding here: glance responds `Invalid operation: `move`. It must be one of the following: add, remove, replace.` and patch is generated by jsonpatch lib | |
| 10:47:39 | gtema | this looks like fun | |
| 10:51:08 | jm1 | gtema: incompatibility between jsonpatch lib used in glance and ard_'s venv or bug in glance? | |
| 10:52:14 | gtema | no clue. It doesn't mean every user will face it, since you need to have situation where jsonpatch desides attribute is "renamed" keeping the content | |
| 12:04:47 | gtema | jm1 ard_ - I am able to reproduce it now in the unittest, still digging where is it coming from. It is definitely a victim of magic properties handling | |
| 12:25:33 | jm1 | ard_: look at that, you can actually update the ramdisk and kernel in devstack https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_e3c/862864/1/check/ansible-collections-openstack-functional-devstack/e3c2883/job-output.txt | |
| 12:29:55 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Existing images update name, visibility etc https://review.opendev.org/c/openstack/ansible-collections-openstack/+/862565 | |
| 14:03:28 | arddennis | jm1: yes you can probably update kernel and ramdisk, but at some point of time you could loose this ability. And the question was more to highlight my misunderstanding of the situation. Where the property which is supposed to be changeable is not and vise versa. | |
| 14:49:19 | gtema | ard_ jm1: instead of invoking update_image a call to update_image_properties can be used (https://paste.opendev.org/show/beYgrcpmaEw1pIM6cFtU/). This is what is currently used in the cloud layer and is working. I am still trying to find out why update_image itself is corrupting extra params | |
| 16:05:08 | arddennis | gtema jm1: Do you think it is reasonable to move update for the properties out of the image update and make two api requests instead of one if properties are defined? It isn't hard to implement. | |
| 16:06:15 | gtema | point is that for historical reasons all "custom" properties of images are stored in OSC and SDK under "properties" attribute | |
| 16:06:37 | gtema | as customs all attrs not currently defined under Image resource are meant | |
| 16:07:08 | gtema | so when you want to access such prop - you need to look under image.properties["foo"] | |
| 16:07:34 | gtema | and when you want to update such props you need to use coinn.image.update_image_properties(foo="bar") | |
| 16:08:16 | arddennis | yes, first when I saw it I was a little bit confused, then accepted it. | |
| 16:08:28 | gtema | if you try to pass conn.image.update_image(foo="bar") foo is identified as custom property and then SDK does not know - what do you want to do with other custom properties - only change single one or reset them all | |
| 16:08:47 | gtema | problem right now is merging or the nested dicts | |
| 16:10:31 | gtema | but I agree, this is now pretty confusing and not self-explaining from docs | |
| 16:28:20 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Revert openstack.cloud.server parameter user_data to userdata https://review.opendev.org/c/openstack/ansible-collections-openstack/+/862935 | |
| 16:38:03 | servagem | Hi, could someone review this patch? It is missing one approval. Thank you. https://review.opendev.org/c/openstack/python-openstackclient/+/862513 | |
| 17:20:42 | jm1 | arddennis: would you please review the latest patchset? https://review.opendev.org/c/openstack/ansible-collections-openstack/+/862565/ | |
| #openstack-sdks - 2022-10-29 | |||
| 00:58:51 | opendevreview | Daniel Wilson proposed openstack/openstacksdk master: Accept queries when listing migrations https://review.opendev.org/c/openstack/openstacksdk/+/862664 | |
| 01:16:34 | opendevreview | Daniel Wilson proposed openstack/python-openstackclient master: Use the compute SDK in server list https://review.opendev.org/c/openstack/python-openstackclient/+/861546 | |
| 08:57:48 | opendevreview | Merged openstack/ansible-collections-openstack master: Revert openstack.cloud.server parameter user_data to userdata https://review.opendev.org/c/openstack/ansible-collections-openstack/+/862935 | |
| #openstack-sdks - 2022-10-30 | |||
| 22:28:48 | opendevreview | Daniel Wilson proposed openstack/python-openstackclient master: Use the SDK in server migration list https://review.opendev.org/c/openstack/python-openstackclient/+/862985 | |
| #openstack-sdks - 2022-10-31 | |||
| 11:33:21 | opendevreview | Denys Mishchenko proposed openstack/ansible-collections-openstack master: Existing images update name, visibility etc https://review.opendev.org/c/openstack/ansible-collections-openstack/+/862565 | |
| #openstack-sdks - 2022-11-01 | |||
| 16:50:24 | opendevreview | Rafael Castillo proposed openstack/openstacksdk master: Add all_projects parameter to find_server https://review.opendev.org/c/openstack/openstacksdk/+/857936 | |
| 16:59:32 | opendevreview | Rafael Castillo proposed openstack/openstacksdk master: Add all_projects parameter to find_server https://review.opendev.org/c/openstack/openstacksdk/+/857936 | |
| 17:01:14 | opendevreview | Rafael Castillo proposed openstack/openstacksdk master: Adds shelf offload action to server proxy https://review.opendev.org/c/openstack/openstacksdk/+/857947 | |
| 17:25:41 | opendevreview | Rafael Castillo proposed openstack/openstacksdk master: Adds shelf offload action to server proxy https://review.opendev.org/c/openstack/openstacksdk/+/857947 | |
| 18:24:15 | opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Updates server action for 2.0.0 https://review.opendev.org/c/openstack/ansible-collections-openstack/+/857976 | |
| 18:24:31 | opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Updates server action for 2.0.0 https://review.opendev.org/c/openstack/ansible-collections-openstack/+/857976 | |
| 18:28:47 | opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Updates server action for 2.0.0 https://review.opendev.org/c/openstack/ansible-collections-openstack/+/857976 | |
| 20:20:02 | opendevreview | Rafael Castillo proposed openstack/ansible-collections-openstack master: Updates server_metadata for 2.0.0 https://review.opendev.org/c/openstack/ansible-collections-openstack/+/863178 | |
| 22:19:22 | opendevreview | Rafael Castillo proposed openstack/openstacksdk master: Add all_projects parameter to find_server https://review.opendev.org/c/openstack/openstacksdk/+/857936 | |
| 22:20:59 | opendevreview | Rafael Castillo proposed openstack/openstacksdk master: Add all_projects parameter to find_server https://review.opendev.org/c/openstack/openstacksdk/+/857936 | |
| 22:21:23 | opendevreview | Rafael Castillo proposed openstack/openstacksdk master: Adds shelf offload action to server proxy https://review.opendev.org/c/openstack/openstacksdk/+/857947 | |
| 22:51:15 | opendevreview | Rafael Castillo proposed openstack/openstacksdk master: Add all_projects parameter to find_server https://review.opendev.org/c/openstack/openstacksdk/+/857936 | |
| 22:51:39 | opendevreview | Rafael Castillo proposed openstack/openstacksdk master: Adds shelf offload action to server proxy https://review.opendev.org/c/openstack/openstacksdk/+/857947 | |
| #openstack-sdks - 2022-11-02 | |||
| 10:26:39 | opendevreview | PB proposed openstack/ansible-collections-openstack master: updated argument spec based on documentation example https://review.opendev.org/c/openstack/ansible-collections-openstack/+/863208 | |
| 10:58:45 | opendevreview | PB proposed openstack/ansible-collections-openstack master: updated argument spec based on documentation example https://review.opendev.org/c/openstack/ansible-collections-openstack/+/863208 | |
| 11:17:57 | opendevreview | PB proposed openstack/ansible-collections-openstack master: updated argument spec based on documentation example https://review.opendev.org/c/openstack/ansible-collections-openstack/+/863208 | |
| 11:39:47 | opendevreview | PB proposed openstack/ansible-collections-openstack master: updated argument spec based on documentation example https://review.opendev.org/c/openstack/ansible-collections-openstack/+/863208 | |
| 11:40:09 | opendevreview | PB proposed openstack/ansible-collections-openstack master: updated argument spec based on documentation example https://review.opendev.org/c/openstack/ansible-collections-openstack/+/863208 | |
| 12:10:39 | opendevreview | PB proposed openstack/ansible-collections-openstack master: updated argument spec based on documentation example https://review.opendev.org/c/openstack/ansible-collections-openstack/+/863208 | |
| 12:27:26 | opendevreview | PB proposed openstack/ansible-collections-openstack master: updated argument spec based on documentation example https://review.opendev.org/c/openstack/ansible-collections-openstack/+/863208 | |
| 13:08:27 | opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Existing images update name, visibility etc https://review.opendev.org/c/openstack/ansible-collections-openstack/+/862565 | |
| 13:20:43 | arddennis | jm1: regarding https://review.opendev.org/c/openstack/ansible-collections-openstack/+/862565 what is required from my side to do? Should I change script logic to use additional method to update properties instead of image.id usage? It seems that next steps are not clean for me. | |
| 13:21:15 | jm1 | arddennis: hey :) i just submitted an update for that patch :D | |
| 13:21:42 | jm1 | arddennis: you dont have to do anything. once ci has passed and you are happy with my changes, we just need another core to vote on that patch and then we can merge it | |
| 13:22:43 | arddennis | jm1: sounds good :) | |
| 13:23:08 | jm1 | arddennis: so please vote on the changes :D | |