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