19:00:01 <nitzmahone> #startmeeting Ansible Core Public IRC Meeting 19:00:01 <zodbot> Meeting started Tue May 4 19:00:01 2021 UTC. 19:00:01 <zodbot> This meeting is logged and archived in a public location. 19:00:01 <zodbot> The chair is nitzmahone. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:00:01 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic. 19:00:01 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting' 19:00:05 <nitzmahone> bah 19:00:09 <felixfontein> hi :) 19:00:09 <nitzmahone> #chair jborean93 19:00:09 <zodbot> Current chairs: jborean93 nitzmahone 19:00:13 <nitzmahone> howdy! 19:00:17 <jborean93> hey 19:00:28 <nitzmahone> #info agenda https://github.com/ansible/community/issues/606 19:00:36 <tadeboro> o/ 19:00:58 <sdoran> \o 19:01:02 <nitzmahone> Just one item on the agenda today... 19:01:19 <sdoran> Feels like we've talked about this recently. ;) 19:01:26 <nitzmahone> #topic Backwards compatibility of role argspec 19:01:26 <felixfontein> maybe also https://github.com/ansible/ansible/issues/74558 ? 19:01:40 <bcoca> sdoran: i dont remember such things! 19:01:58 <nitzmahone> Sam, do you want to recap what we talked about this morning? 19:02:03 <sdoran> Sure! 19:02:27 <bcoca> it was all zodbot's fault! 19:04:16 <sdoran> We basically had two options: 1) Backport a feature/bugfix to 2.9 - 2.11 that allows `argument_specs` in `meta/main.yml` without error or 2) Move `argument_specs` out of `meta/main.yml` into `meta/argument_specs.yml`, allowing both locations to work with preference given to `meta/argument_specs.yml`. 19:04:52 <bcoca> 3) ignore it 19:05:08 <felixfontein> 3) is a very bad option. (but yes, it's an option.) 19:05:19 <sdoran> We decided on option two since it doesn't required backporting a feature-ish thing to Ansible versions that are in security/critical bugfix mode. 19:05:41 <sdoran> And it allows 2.9.x and 2.10.x to work with roles that contain arg specs without requiring an update. 19:06:36 <nitzmahone> So shrews is working on a PR to enable that for an upcoming 2.11 release 19:06:41 <sivel> So a bugf(eature)ix for 2.11.next ;) 19:06:51 <nitzmahone> (ish, depending on timing) 19:06:56 <bcoca> bugureix 19:07:37 <nitzmahone> That kinda seemed to be the best middle ground 19:07:49 <sdoran> I think that about does it for the summary and plan of action. 19:08:28 <nitzmahone> Any other discussion needed for that beyond "wait for the PR and/or the 2.11.x release that contains it:? 19:08:33 <felixfontein> is it possible to deepen the hierarchy for the new file a bit so that the options aren't directly under the entrypoint name? so it's easier to add support for other things, like doc fragments (with shared options used by multiple roles), `description`, `requirements`, ... in the future? 19:09:14 <Shrews> felixfontein: they are not now, and we don't plan to change it. everything is under `argument_specs`, irrc 19:09:17 <Shrews> iirc 19:09:42 <Shrews> or do you mean "specifically" under entrypoint? 19:10:24 <felixfontein> I mean `argument_specs -> <name of entrypoint> -> options -> <option name>` instead of `argument_specs -> <name of entrypoint> -> <option name>` 19:10:41 <Shrews> in any case, we will not be changing the format of the spec at this time 19:10:56 <tadeboro> Thanks for the recap. I am happy the 2) won because I do not have to bump the minimal required ansible version in our collections that will get argspecs. 19:11:30 <felixfontein> Shrews: in that case the format will never change 19:11:42 <nitzmahone> https://www.irccloud.com/pastebin/b88aVDaM/ 19:12:04 <nitzmahone> It looks like that's already how it is 19:12:38 <felixfontein> oh. 19:12:43 <felixfontein> I thought it was one less level 19:12:50 <bcoca> nitzmahone: that was fast! 19:12:55 <nitzmahone> (that's from one of the integration tests) 19:13:07 <felixfontein> in that case, I'm happy :) 19:13:19 * bcoca waits 30s 19:13:28 <felixfontein> looks like I wasn't able to properly read the docs... 19:13:42 <tadeboro> https://github.com/xlab-steampunk/sensu-go-ansible/blob/role-argspec/roles/backend/meta/main.yml is one "real-world" example that found the issue if anone needs one. 19:14:10 <nitzmahone> OK, so next topic then 19:14:11 <felixfontein> I even looked at that one, but missed that level... 19:14:12 <nitzmahone> #topic https://github.com/ansible/ansible/issues/74558 19:14:34 <nitzmahone> proposal: ansible-test bombs on unrecognized dirs 19:14:34 <felixfontein> sorry for the noise! 19:14:54 <nitzmahone> I'd be -1 for that, since it creates another instance of exactly the problem we just solved 19:15:15 <felixfontein> I think it shouldn't do that by default (i.e. needs to be enabled by config file), resp. there should be a way to disable it with the config file 19:15:42 <bcoca> -1 to that specific request, fine with having a test warn/default off for full collection schema 19:16:05 <jborean93> I would also be -1 on that as well 19:16:28 <nitzmahone> Or on the flipside, it's an on by default but "ignore"able error- "opt-in" sanity tests are often worthless, since the worst offenders don't opt-in ;) 19:16:54 <tadeboro> I think this is something most collection author need because we are not always 100% up-to-date what we can and what we cannot use so that we are safe in the future. But I agree that for ansible/ansible, it does not make sense. 19:17:12 <sdoran> -1 to restricting this in testing by default. 19:17:36 <sdoran> But it could be helpful to allow collection authors to make that a strict list. 19:18:03 <tadeboro> So I would say this should be enabled by default on collections. Core team should know what they are doing ;) 19:18:07 <jborean93> I just feel like it's yet another restriction on the code. The only benefit that it might bring is to avoid future plugin types from colliding with someones own folder 19:18:12 <felixfontein> this is potentially something that could go into a collection linter; one question would be whether ansible-test should take that role, or whether there should be another (separate) tool to be used in conjunction with `ansible-test sanity` 19:19:11 <felixfontein> this not being a part of ansible-core's ansible-test also has the advantage that you don't have to add ignore.txt entries for some ansible-core versions if you test with multple ansible-core versions 19:19:45 <bcoca> if we realy want this restriction, it should not be in a test, but the loader code itself 19:20:08 <tadeboro> bcoca: What loader? 19:20:13 <bcoca> collection loader 19:20:37 * nitzmahone runs 19:20:41 <felixfontein> :D 19:20:51 <felixfontein> I don't think the collection loader should do such validations 19:20:54 <tadeboro> Ah, OK, I thought you meant plugin loaded. That would be useless because not all things are loaded using plugin loaders in collections. 19:21:10 <bcoca> since i don tthink anyone wants that, i would accept a 'full collection schema check' that warns about 'unkown' dirs .. or a 'error' but a check that is not done by default 19:21:28 <bcoca> tadeboro: it is still used to 'find them' 19:21:41 <nitzmahone> bcoca: not always 19:21:50 <tadeboro> bcoca: import ... does not use loaders at all. 19:21:59 <bcoca> import IS the loader ... 19:22:00 <jborean93> I think this is more a linter problem rather than a validation 19:22:07 <nitzmahone> agreed 19:22:08 <bcoca> we just add layser on top 19:22:12 <felixfontein> jborean93: I agree 19:23:29 <tadeboro> I am not 100% sure this is a linter issue. What do we say to a collection author that uses plugins/bla for something and ansible starts using for something else? 19:23:47 <jborean93> they should have known better 19:23:48 <bcoca> his stuff will break, but that is current risk in many places 19:24:04 <bcoca> mkidr my_plugins in role/playbooks ... then we add 'my' plugins to ansible 19:24:22 <mattclay> The proposal seems to have two goals: 1) protect against typos in directory names and 2) people trying to do non-standard things -- the first seems unnecessary, unless the collection isn't being tested, as typos should cause tests to fail anyways. 19:25:08 <mattclay> As for the second, that would only appear to be an issue if there was a conflict with a future plugin type supported by ansible-core. 19:25:30 <tadeboro> I thing about the proposal more in a sense "inform user that some things in the collection are not as future proof as they could be". 19:25:41 <jborean93> I do think we should probably check that we've documented this, including `plugin_utils`/warning in the docs stating use that 19:25:52 <felixfontein> tadeboro: yes, and that's more a linting thing for me 19:25:56 <jborean93> +1 19:26:31 <bcoca> a) publish colleciotn spec 19:26:48 <bcoca> b) have note about 'random dirs and no assurances we wont break' 19:27:16 <felixfontein> plugin_utils is currently not mentioned in the ansible docs I think, only in https://github.com/ansible-collections/overview/blob/main/collection_requirements.rst#modules--plugins 19:27:27 <jborean93> our recommendation is to put custom code it in `plugin_utils` right? 19:28:03 <felixfontein> might depend on whom you mean by 'we' ;) 19:28:11 <bcoca> module_utils, but for those that wanted to ignore 'older python checks', plugin_utils 19:28:43 <felixfontein> bcoca: more importantly, for those who want to import 'forbidden' things from ansible 19:28:50 <briantist> ^ this 19:28:53 <tadeboro> Community requirements mandate that control node shared code lives there (it it imports stuff from ansible.*). 19:30:50 <tadeboro> The only documented recommendation from the core I saw was the readme created when generating new collection, but that file does not talk about things that are not listed there. 19:31:10 <bcoca> cause for core they mostly 'dont exist' 19:31:14 <jborean93> that README should be updated to link to these docs now things are more official 19:31:21 <jborean93> "official"* 19:31:35 <tadeboro> And when we reviewed ansible.utils for inclusion in Ansible 3.0.0, we discovered a lot of things in plugins that we never saw before. 19:31:56 <tadeboro> Networking people are quite good at (ab)using things ;) 19:32:01 <nitzmahone> OK, so I think we're about ready to close this one out- sounds like there's not a lot of appetite for adding ansible-test checks right now, and in the absence of someplace else to hang a structural check, there should be a little docs tweak to recommend plugin_utils. Anybody feel like submitting a PR for that? 19:34:26 <felixfontein> so many volunteers :D 19:34:46 <tadeboro> I can take a stab at it since I need to educate some internal people about this in the next few weeks. 19:35:00 <nitzmahone> #action tadeboro to submit a docs PR to describe plugin_utils 19:35:05 <nitzmahone> thanks! 19:35:21 <nitzmahone> #topic open floor 19:35:28 <felixfontein> thanks tadeboro! 19:35:57 <briantist> +1 thanks tadeboro , look forward to reading that since I had to learn about `plugin_utils` via random IRC conversations ;) 19:36:04 <felixfontein> I've reactivated https://github.com/ansible/ansible/pull/71734 (i.e. rebased and made sure it still works). whom should I bug for reviews? 19:36:16 <felixfontein> (it's validate-modules extended to plugins) 19:36:56 <bcoca> thinking of adding 2 new types to plugins config 19:37:28 <bcoca> and some properties, 'static', 'vaulted' , etc 19:38:42 <felixfontein> it would really help to have automatic validation for plugins as well. when rebasing the PR I found some problems in recently added plugins in community.general 19:41:01 <mattclay> felixfontein: I have that PR on my list to review. I just don't know how soon I'll get to it. 19:41:36 <felixfontein> mattclay: thanks! 19:41:46 <nitzmahone> any other topics for today? 19:41:50 <felixfontein> I hope it'll make it into ansible-core 2.12 19:42:48 <nitzmahone> We'll close in 2m if not... 19:43:58 <bcoca> https://github.com/ansible/ansible/pull/74403 19:44:14 <bcoca> ^ what do poeople feel 'snippets' should output 19:44:24 <bcoca> for modules it creates a 'heavily commented task' 19:44:32 <bcoca> i added lookups with 'all lparams set to default' 19:44:42 <bcoca> for other plugins i add 'ini config set to defaults with comments' 19:47:00 <Shrews> those sound reasonable, imo 19:47:20 <bcoca> its also possible to show ini for lookups and vars/env vars for all 19:47:22 <shertel> yeah, I like it 19:49:40 <mattclay> It needs tests. 19:50:11 <bcoca> mattclay: they will come, just wanted to get consensus on 'what expected output shoudl be' before i add tests for it 19:53:14 <jborean93> seems good to me 19:54:29 <jborean93> anything else we want to talk about? 19:55:29 <bcoca> weather just got nice ... 19:55:48 <jborean93> sun still isn't up here so I can't tell :) 19:58:21 <jborean93> cool will give y'all back your 2 minutes then 19:58:23 <jborean93> #endmeeting