| KSR-DK | Hi jm1[m] | 14:06 |
|---|---|---|
| KSR-DK | jm1[m] - I'm trying to read the https://opendev.org/openstack/ansible-collections-openstack/src/branch/master/plugins/modules/identity_domain.py module.. But I'm not smart enough it seems.. Anyway, it is used by https://opendev.org/openstack/openstack-ansible-os_keystone/src/branch/stable/2023.1/tasks/keystone_federation_sp_idp_setup.yml and it seems like it's breaking my default domain. My default domain if defined with id defa | 14:10 |
| KSR-DK | On top my user_variables contain federated_identities - domain: default, and whenever the task - name: Ensure domain which remote IDP users are mapped onto exists - in playbook keystone_federation_sp_idp_setup.yml - is called, it changes the name of my default domain to lower case "default" | 14:13 |
| KSR-DK | Is this intended ? jm1[m] | 14:15 |
| gtema | ksr-dk: you refer to openstack-ansible and I now think it is responsible for changing data, not the module itself. The module is ensuring resource is having attributes as passed by the caller (and here it does not have any own defaults) | 14:17 |
| KSR-DK | gtema - yeah I know, and you might be correct - I'm just wondering if it is intended functionality, that if the module finds a domain with the name if it, should correct the casing, or if it should just update the description and wether it's enabled like it did in the previous versions. | 14:21 |
| gtema | openstack-ansible is not what we are here responsible for - we take care of the module itself, not the playbooks that use it | 14:22 |
| KSR-DK | the functionality I'm refering to is in - https://opendev.org/openstack/ansible-collections-openstack/src/branch/master/plugins/modules/identity_domain.py | 14:23 |
| gtema | noonedeadpunk can help you further | 14:23 |
| gtema | ksr-dk: what you refer to would ensure resource has properties set as passed by the caller. And if the caller ignores case the module will apply this | 14:25 |
| * noonedeadpunk looking | 14:25 | |
| gtema | the module is not doing any case operations (at least not on purpose) | 14:25 |
| jrosser_ | KSR-DK: it would be helpful if you could illustrate what you think is happening by providing the ansible output with -vvv showing the name having been changed | 14:26 |
| noonedeadpunk | huh, so my guess would be that module finds name by ID and then changes name to the id | 14:27 |
| KSR-DK | Yes | 14:27 |
| KSR-DK | That's my guess as well. | 14:28 |
| noonedeadpunk | that's fun kinda | 14:28 |
| KSR-DK | Not when deploying in production ;-) | 14:28 |
| KSR-DK | or rather upgrading | 14:28 |
| noonedeadpunk | gtema: I kind of tend to agree, that domain `name` should not be updated, as I don't see any way on how to do that intentionally | 14:32 |
| noonedeadpunk | or whell | 14:33 |
| KSR-DK | Actually I think the module uses the "name" aka the Domain name input, to find the domain by id, and then corrects the name of the Domain with the same variable | 14:33 |
| noonedeadpunk | now i do | 14:33 |
| noonedeadpunk | yeah, but then you need to provide it as ID | 14:33 |
| gtema | the only current potential explanation I have is that Keystone is doing case insensitive search. In that case module may decide to update the value | 14:34 |
| noonedeadpunk | but I guess module allows names to be resolved to id | 14:34 |
| gtema | I do not have access currently to anywhere with admin access so can't verify assumption | 14:34 |
| noonedeadpunk | yeah, so I wonder if it would be reasonable to drop name from attributed if id is not provided? | 14:34 |
| noonedeadpunk | or like populate `non_updateable_attributes` maybe... | 14:35 |
| KSR-DK | the module seems only to take "name" as input, not id | 14:36 |
| noonedeadpunk | it can take ID from what I see | 14:36 |
| noonedeadpunk | oh, wait | 14:36 |
| noonedeadpunk | it's return, lol | 14:36 |
| noonedeadpunk | ok | 14:36 |
| KSR-DK | ;-) | 14:36 |
| noonedeadpunk | yeah, ok, so back to my statement that I don't see how you can intentionally update domain name with the module at all | 14:37 |
| gtema | making name unchangeable by default is definitely not what you want to do | 14:37 |
| noonedeadpunk | but how you can change it? | 14:38 |
| gtema | and I checked at one cloud - keystone is doing case insensitive search (at least for projects) | 14:38 |
| noonedeadpunk | like as a user | 14:38 |
| noonedeadpunk | in order to change name you should be able to suppluy uuid, right? | 14:38 |
| noonedeadpunk | and there's no argument for that | 14:38 |
| noonedeadpunk | and when you provide "name" - you shouldn't attempt to change it, imo | 14:39 |
| noonedeadpunk | like it's the only identifier | 14:39 |
| gtema | I was for a very long time stating that interface of modules evolved in the wrong direction, nobody was listening to me | 14:39 |
| gtema | now we have more and more of the tricky cases, and all in the sake of "backwardscompatibility" | 14:40 |
| noonedeadpunk | I'm not sure if it's tricky - more consequence of refactoring I assume | 14:41 |
| noonedeadpunk | but dunno | 14:42 |
| noonedeadpunk | looking at 1.0.0 it's also.... wierd in a different way | 14:42 |
| gtema | it's tricky by default - looking at all openstack apis there is an ocean of corner cases | 14:42 |
| noonedeadpunk | but there we fetched list of domains and supplied uuid to the update request and name... | 14:43 |
| noonedeadpunk | so it should have behaved pretty much the same | 14:43 |
| noonedeadpunk | yeah, I'm not arguing that | 14:43 |
| noonedeadpunk | https://opendev.org/openstack/ansible-collections-openstack/src/branch/stable/1.0.0/plugins/modules/identity_domain.py#L145-L147 | 14:43 |
| gtema | I fear that if you pass uuid here the domain name will become this uuid | 14:43 |
| noonedeadpunk | actually | 14:44 |
| noonedeadpunk | it would give more funny consequence I guess | 14:44 |
| noonedeadpunk | as if you supply uuid as name, your name will become uuid? | 14:44 |
| noonedeadpunk | yeah | 14:44 |
| noonedeadpunk | exactly | 14:44 |
| noonedeadpunk | but Domain/domain will work :D | 14:44 |
| noonedeadpunk | crap | 14:44 |
| gtema | that all means that assumption to have interface like "name": "Name or ID of the resource" is completely wrong | 14:45 |
| gtema | I mean such interface should not exist | 14:45 |
| noonedeadpunk | unless we say that name can not be changed through the module | 14:46 |
| noonedeadpunk | but yes | 14:46 |
| noonedeadpunk | now it's non-functional in this way | 14:47 |
| noonedeadpunk | however, I totally get that such interface should exist | 14:47 |
| gtema | in this interface from the code we never know what user actually passed: name or id and whether the server side applied regex like search | 14:48 |
| noonedeadpunk | as you can decide what you have only during runtime, so it might vary from execution to execution | 14:48 |
| noonedeadpunk | So, I assume that name should be palced as `non_updateable_attributes` atm | 14:49 |
| gtema | so the best way is to get rid of all "assumptions" and make modules like the new CLI: pure api wrap. This will let consumer to be very explicit in what he does and have less of such cases | 14:49 |
| noonedeadpunk | and then mark as todo fixing way of updating name, as that should involve new attribute introduction | 14:49 |
| gtema | if you place name into non_updatable_attributes you have absolutely no way in controlling the name | 14:49 |
| gtema | this is surely not what you want | 14:50 |
| noonedeadpunk | so you mean even for resource creation? | 14:50 |
| gtema | for creation it would, but no update would be ever possible | 14:50 |
| gtema | I mean with this module | 14:50 |
| noonedeadpunk | as I guess otherwise we agreed that it's not possible anyway? | 14:50 |
| noonedeadpunk | other then Default -> default | 14:51 |
| noonedeadpunk | lol | 14:51 |
| gtema | option 1) add everywhere explicit "id" param as readOnly. This will allow preventing name->uuid renaming and control over the name | 14:52 |
| gtema | option 2) I forgot already. option 1) is the only proper solution | 14:53 |
| KSR-DK | I guess It's not possible to update the uuid for a domain anyway. | 14:54 |
| gtema | uuid is not updatable ever | 14:54 |
| KSR-DK | Precise.. Maybe I'm not getting you option 1 then. | 14:55 |
| noonedeadpunk | I guess my point still is that without adding another input attribute - it's no way to update name either | 14:55 |
| noonedeadpunk | Like current `name` is the only thing that does search basically | 14:55 |
| gtema | module input must include both "id" and "name" to give user possibility to say explictly: what I am passing is ID | 14:55 |
| noonedeadpunk | yes | 14:56 |
| gtema | in this case param name would be enforced on the found resource | 14:56 |
| gtema | if name is passed - treat it as name and set it as name in the found entity | 14:56 |
| noonedeadpunk | well, I personally think that one of these could be name_or_id... | 14:57 |
| noonedeadpunk | but then you don't update name | 14:57 |
| noonedeadpunk | but it's another wierd solution I guess... | 14:57 |
| gtema | then you have id+name+name_or_id - which is again unnecessary complexity | 14:57 |
| gtema | I would rather leave the _info modules to behave like name_or_id and in the regular modules have explicitly id+name | 14:58 |
| noonedeadpunk | well, as I said - you don't always know what you have. And as in ansible it's impossible to "undefine" variable - it becomes even more tricky when passing arguments, as you'd need to make complex logic in role to detect if you have id or name to pass accrodingly | 14:58 |
| jrosser_ | part of this stems from there being a bunch of missing _info modules | 14:59 |
| gtema | for that you use _info to actually locate resource first | 14:59 |
| jrosser_ | which makes it super tricky to always ensure you have the id | 14:59 |
| gtema | and it's response is both name and id | 14:59 |
| noonedeadpunk | in perfect world... which we are not in yet | 15:00 |
| KSR-DK | Now that we are brainstorming.. How about adding a parameter to the module called - action: (values - create,readm update,delete) - If it's create it will only create and not change anything existing.. | 15:00 |
| gtema | if only I would have employer paying me for resolving this I would be glad to work from openapi side on generation of all avaiable modules | 15:01 |
| KSR-DK | :-) | 15:01 |
| gtema | ksr-dk: I am not going to explain in detail, but we abandoned such ideas in past since it makes things even more complex and unmaintainable | 15:01 |
| KSR-DK | Ok | 15:02 |
| noonedeadpunk | yeah.......... | 15:02 |
| jrosser_ | that is also not really matching the pattern in core ansible modules for idempotency | 15:02 |
| KSR-DK | I'm a newcommer with no history | 15:02 |
| KSR-DK | Ok, well I can the add to the when clause of os_keystone/src/branch/stable/2023.1/tasks/keystone_federation_sp_idp_setup.yml that it should only execute if domain NOT default | 15:03 |
| KSR-DK | But that of cause does not solve the core issue | 15:04 |
| gtema | my current strong stand opinion: get rid of any "assumptions" and provide stable and complete pure API wrap. If any additional "usability" modules are desired they can be provided on top by combining individual modules, but we SHOULD STOP coding this into standard API wraps | 15:04 |
| gtema | as a workaround (I think now that was supposed to be option 2) you invoke identity_domain_info module first to find the domain and further pass it's name into the identity_domain module | 15:05 |
| gtema | I know how "bad" it is | 15:05 |
| gtema | but that is what "declarative" approach in reality is about | 15:05 |
| noonedeadpunk | well, frankly, it's easier to re-write into separate module. at least in terms of execution efficiency.... | 15:06 |
| gtema | I am strongly opposed to just giving a birth to another modules that nobody really maintains. Unless maintenance issue is solved there should be no new modules | 15:07 |
| jm1[m] | agree 100% with gtema. Basically he is the only maintainer left for openstack.cloud | 15:12 |
| jm1[m] | name vs id vs name_or_id caused issues in the past but introducing a clear distinction between id and name met too much resistance | 15:14 |
| gtema | right, and now we see why it is just unavoidable (to have a clear interface) | 15:15 |
| jm1[m] | openstacksdk correctly does this distinction in id and name, but openstack.cloud has always mixed up both. no way to change that. except... | 15:15 |
| noonedeadpunk | well, imo, we can really leave name as name_or_id, while passing ID as ID only | 15:16 |
| noonedeadpunk | then, when user passes id and name - name is updated | 15:16 |
| noonedeadpunk | when it's only name - name is added to non_updateable_attributes | 15:16 |
| gtema | and how are you going to treat cinder which does a prefix based search? | 15:17 |
| gtema | and in the domain case it will still lowcase the name | 15:17 |
| jm1[m] | if your openstack api of interest allows it, you can use the openstack.cloud.resource module as a workaround. basically it offers you a passthrough to openstacksdk resources | 15:17 |
| gtema | I am stongly against of "fixing" inteface and leaving inconsistent naming (with "name" working as something else) | 15:18 |
| jm1[m] | agree with gtema again. its unmaintainable and confusing. | 15:18 |
| noonedeadpunk | it will locase only for search? which is again api issue | 15:18 |
| noonedeadpunk | well, it's also question of how usable it would be when name can be only name, and uuid only uuid | 15:19 |
| gtema | noonedeadpunk - there are no api issues. API is working as designed (and declared, hopefully). Problem is when you try to stretch one standard way of handling across all openstack services. It just doesn't work | 15:20 |
| noonedeadpunk | and ansible is really too slow to doublecheck everything through _info | 15:20 |
| noonedeadpunk | ok, I was quite focused on the current issue frankly speaking... Like I got that it affects everything.... | 15:20 |
| gtema | api-sig failed and my learning of last 6 years in openstack apis: stop trying to fix them. Find the way of dealing with that by letting user tell explicitly what he wants | 15:21 |
| noonedeadpunk | Though, openstackclient also almost always treat name as uuid. And I gues what everyone expects, to be able to use ansible modules as cli more or less | 15:21 |
| noonedeadpunk | but not sure | 15:21 |
| noonedeadpunk | it can be only my wish ofc | 15:22 |
| gtema | osc is invoking the same search. But since this is an explicit input parameter (and not the final expected value) you don't have this particular issue | 15:23 |
| gtema | input/search | 15:23 |
| gtema | and honestly - I hate this behavior in OSC also - there are just way too many assumptions that do not work across all apis and require very dirty hacks | 15:24 |
| noonedeadpunk | it's all about tradeoff of usability/maintenance cost | 15:24 |
| noonedeadpunk | so there should be some kind of tolerated middle-ground | 15:24 |
| gtema | disagree, the usability is not affected. It's a "used to" effect more | 15:25 |
| gtema | all middle-ground requires manual maintenance with no possibility to automate things. What we really need to have is 1-to-1 API wrappers. If API is bad - cli/ansible/etc usability is bad. And API developer is to blame. It should not be a maintenance burden of the ecosystem tooling | 15:26 |
| gtema | nobody, literally nobody now is willing to spend on maintenance. | 15:27 |
| gtema | all CTOs are only chasing features | 15:27 |
| noonedeadpunk | yeah | 15:28 |
| noonedeadpunk | true | 15:28 |
| noonedeadpunk | I know | 15:28 |
| noonedeadpunk | Though I'm not sure who are "we" that needs to have 1-to-1. As openstack users unlikely need that :D | 15:29 |
| gtema | if you would use other cli (gh, kubectl, etc) they offer you precisely what API is providing | 15:30 |
| noonedeadpunk | neither of them are as shattered | 15:31 |
| gtema | there are some additions, but majority of the day2day things are just what API provides | 15:31 |
| noonedeadpunk | these are single-repo/project things | 15:31 |
| noonedeadpunk | openstack is jsut very different | 15:31 |
| gtema | I mean here is that from the usability pov they offer you 1-to-1 API wrappings | 15:31 |
| noonedeadpunk | kubectl doesn't deal with all ecosystem around k8s | 15:31 |
| gtema | and users are happy with that | 15:32 |
| noonedeadpunk | and openstacksdk is | 15:32 |
| noonedeadpunk | and openstack _is_ ecosystem | 15:32 |
| gtema | which is part of a problem. It should not be all treated as "core" openstack, but similarly as "openstack-affiliated" things. This will allow making it similar to kubectl plugins or gh extensions | 15:33 |
| gtema | where service developer is responsible for the usability, not the core cli maintainer | 15:33 |
| noonedeadpunk | yeah, and I guess that's where service clis become pluggable? | 15:34 |
| noonedeadpunk | and I totally do agree that service developers probably should pull their weight in ansible/terraform as well... | 15:34 |
| gtema | right. But here you will again say: usability. Every thing becomes like a thing on its own with own standards | 15:34 |
| noonedeadpunk | yup | 15:35 |
| gtema | actually no, service developers should not take care of ansible/tf/etc. They need to be aware of those and challenges coming from the different mgmt concepts | 15:35 |
| noonedeadpunk | And it's not like I have solution rather then put more ppl on the problem, as like you said - nobody wants to pay for that | 15:35 |
| gtema | but what they absolutely SHOULD do is provide openapi spec what make it possible to generate 1-to-1 api bindings trying to have some sort of standartization | 15:36 |
| jm1[m] | what do we do short-term though? due to lack of maintainers for openstack.cloud and no improvement in sight, shouldn't we mark openstack.cloud as unmaintained? | 15:39 |
| jm1[m] | communicating the maintenance problems openly would be more honest and useful to users, wouldnt it? | 15:41 |
| gtema | I would rather state: "nearly unmaintained" I am afraid that declaring it "unmaintained" will make more governance problems | 15:42 |
| gtema | or "very close to be unmaintained" | 15:42 |
| jm1[m] | governance problems? | 15:42 |
| noonedeadpunk | eventually, I kind of wonder how SIG is even extended | 15:43 |
| noonedeadpunk | as it's very unlcear, for instance | 15:44 |
| gtema | yeah. TC may decide to kick it off from opendev.org/openstack or similar stuff | 15:44 |
| jm1[m] | openstack.cloud is not an official openstack project, at least not in the same sense as neutron or openstacksdk. | 15:44 |
| noonedeadpunk | yeah, ^ that is very confusing part | 15:44 |
| noonedeadpunk | as it's not clear how to step in with some maintenance help | 15:44 |
| gtema | looking at all the discussions around active/unactive/unmaintained/etc discussions in TC (and I know collections is not an official project) I just fear there might be additional long discussions jsut sucking energy | 15:45 |
| jm1[m] | ack, you have better insights here so leave it up to you :D | 15:48 |
| gtema | thks jm1 | 15:49 |
| opendevreview | Tom Clark proposed openstack/ansible-collections-openstack master: Add missing network quota options https://review.opendev.org/c/openstack/ansible-collections-openstack/+/916805 | 15:54 |
| noonedeadpunk | as frankly, until breaking changes will arrive, I think we still should keep current modules at least basically reviewed/maintained | 15:55 |
| noonedeadpunk | and actually sigs activity is really another topic.... | 15:56 |
| opendevreview | Tom Clark proposed openstack/ansible-collections-openstack master: Add missing network quota options https://review.opendev.org/c/openstack/ansible-collections-openstack/+/916805 | 15:58 |
| opendevreview | Tom Clark proposed openstack/ansible-collections-openstack master: Add missing network quota options https://review.opendev.org/c/openstack/ansible-collections-openstack/+/916805 | 15:58 |
| opendevreview | Tom Clark proposed openstack/ansible-collections-openstack master: Add missing network quota options https://review.opendev.org/c/openstack/ansible-collections-openstack/+/916805 | 16:10 |
| opendevreview | Tom Clark proposed openstack/ansible-collections-openstack master: Add missing network quota options https://review.opendev.org/c/openstack/ansible-collections-openstack/+/916805 | 16:13 |
| opendevreview | Tom Clark proposed openstack/ansible-collections-openstack master: Add missing network quota options https://review.opendev.org/c/openstack/ansible-collections-openstack/+/916805 | 16:13 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!