19:15:00 <sdoran> #startmeeting Ansible Core IRC meeting https://github.com/ansible/community/issues/486
19:15:00 <zodbot> Meeting started Tue Oct  1 19:15:00 2019 UTC.
19:15:00 <zodbot> This meeting is logged and archived in a public location.
19:15:00 <zodbot> The chair is sdoran. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:15:00 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:15:00 <zodbot> The meeting name has been set to 'ansible_core_irc_meeting_https://github.com/ansible/community/issues/486'
19:15:11 <sdoran> We need to open a new issue for October, too.
19:15:26 <sdoran> Fest last week consumed pretty much all of our collective attention.
19:15:54 <sdoran> #topic https://github.com/ansible/ansible/pull/28552
19:17:05 <felixfontein> is nre-ableton around?
19:17:18 <sdoran> I'm not sure
19:17:30 <cyberpear> sounds like a good conceptual change... just would like to have someone verify that it reports for all configured interfaces at the VMware level, not just what was called in the vmware_guest task
19:17:31 <sdoran> Goneri: Do you have any thoughts on this PR?
19:17:50 <jtanner> hello
19:18:04 <cyberpear> (I'm not familiar enough w/ the innards of vmware module_utils to know)
19:18:10 <nitzmahone> I think this one is better for the VMware wg
19:18:37 <sdoran> Yeah, looks like that was Goneri's intent.
19:18:44 <sdoran> I"ll add it to that agenda instead.
19:18:51 <sivel> Yeah, we are not the appropriate venue for this discussion
19:18:54 <Goneri> I agree with @nre-ableton, but it's a disruptive change, so it should only land in 2.10 without any backport
19:19:02 <felixfontein> my main worries would be whether that breaks backwards compatibility
19:19:10 <felixfontein> Goneri++
19:20:16 <Goneri> do we have a consensus?
19:20:32 <nitzmahone> Punt to WG
19:20:46 <felixfontein> and probably don't backport this
19:20:54 <nitzmahone> Yep
19:21:04 <sdoran> I moved it to the VMware WG agenda.
19:21:06 <sdoran> #topic https://github.com/ansible/ansible/pull/62719
19:21:12 <sdoran> felixfontein: You're up.
19:21:48 <felixfontein> this is about module default groups. I think they are great, and should be used more, so I created a PR to add the acme_* modules as a group :)
19:21:58 <felixfontein> there are two things to discuss, I think:
19:22:11 <felixfontein> a) (what sdoran mentioned in the PR) module default groups currently don't scale well,
19:22:15 <sdoran> My comment was mainly to restate that the current module defaults system doesn't work and needs to be revamped, not to say we should not merge that change.
19:22:44 <jtanner> who is acme maintainer?
19:22:49 <felixfontein> b) what to do with exceptions? (like one acme_* module which shouldn't use the default args because most make no sense) - should that be allowed? documented? how?
19:22:55 <felixfontein> jtanner: me :)
19:23:13 <jtanner> then i'd say we just let you decide and we merge it if you want
19:23:19 <jtanner> but no backport
19:23:24 <felixfontein> sdoran: it's still a good point, and I also noticed in the past that stuff got forgotten in that YAML file
19:23:32 <sdoran> `team_crypto: felixfontein MarkusTeufelberger puiterwijk resmo Spredzy Shaps Xyon`
19:23:48 <jtanner> ah, should cc them in the PR
19:24:01 <felixfontein> oh. I didn't notice there was no auto-CC...
19:24:03 <sdoran> We'll always be playing cat and mouse until we revamp this. Plus, defaults don't work/make sense in the collections world.
19:24:07 <felixfontein> will do
19:24:10 <jtanner> i assume component didn't map to something logical for the bot
19:24:18 <shertel> Yeah, I agree with sdoran that it would be good to move away from the static file. It would be nice if it groups were configurable.
19:24:43 <felixfontein> shertel: definitely, then also no modules will get forgotten accidentally
19:24:57 <sdoran> I'd love for it to be something that can be defined in the module itself, but that's likely a lot of work.
19:25:18 <sdoran> Anyway, any objections to merging this?
19:25:19 <felixfontein> sdoran: the not working with collections is bothering me a bit. I'm not sure it is a good idea to add new groups when it is not sure that the feature will still work when stuff is moved to collections
19:25:29 <agaffney> is there any existing case of metadata pulled from a module *before* the module is executed?
19:25:48 <felixfontein> agaffney: I think not, but I can think of some other use-cases where that would be nice as well :)
19:25:55 <sdoran> agaffney: Probably not. Hence why I think it would be a lot of work.
19:26:25 <nitzmahone> That's a big meatball
19:26:45 <sdoran> Yup. And probably very spicy.
19:26:48 <cyberpear> might be interesting to tie the defaults group into a module_utils?
19:27:05 <nitzmahone> Also falls over with collections
19:27:18 <agaffney> just making the module defaults groups user configurable would solve most/all of these problems, even if it's not completely ideal
19:27:27 <agaffney> but that's a completely separate issue
19:27:37 <sdoran> #topic https://github.com/ansible/proposals/issues/171
19:27:54 <sdoran> nkakouros: Are you around?
19:28:04 <tterranigma> yes
19:28:11 <tterranigma> thats me
19:28:14 <cyberpear> I don't like the proposal as-is, but would be nice to have some clean solution
19:28:23 <shertel> felixfontein: regarding b), I'd just not add those to module_defaults. ec2_metadata_facts is like that.
19:28:32 <cyberpear> my "works today" workaround: https://github.com/jamescassell/multi-os-role-example/blob/master/defaults/main.yml
19:28:37 <cyberpear> doesn't scale well, though
19:28:41 <felixfontein> shertel: would it make sense to mention them in the documentation?
19:28:51 <agaffney> same here...I don't like using meta info for this, but *some* solution would be nice
19:29:04 <cyberpear> but maybe a `first_defined` lookup could help make it cleaner
19:29:14 <agaffney> it's possible to do this now with a bit of extra work on the part of the module author, but it would be nice to have a simpler method
19:29:25 <agaffney> err, role author
19:29:42 <tterranigma> @agaffney what else could the role author do?
19:29:55 <cyberpear> maybe a map | select defined | first
19:29:55 <tterranigma> without introducing side effects
19:30:33 <agaffney> tterranigma: use include_vars to include an OS-specific vars file that defines a secondary var, and then use that secondary var in the definition for the primary var, with a fallback to a default value
19:30:34 <cyberpear> can you map a lookup? (rather than a filter?)
19:30:43 <cyberpear> agaffney: include_vars mucks the precedence
19:30:45 <agaffney> cyberpear: no
19:31:03 <agaffney> cyberpear: which is why you use a secondary var, so that the include_vars doesn't directly overwrite the var defined in defaults/
19:31:19 <agaffney> it's a bit cumbersome, but very possible
19:31:30 <cyberpear> right. the extra intermediate vars are noisy, and as you say cumbersome
19:31:48 <agaffney> I never claimed it was pretty :)
19:31:49 <cyberpear> In some roles I have myvar and myvar_default to work around it similarly
19:32:27 <tterranigma> would an extra special file under the defaults/ folder that contains some kind of resolution rules be a bad idea?
19:32:28 <nitzmahone> I actually don't hate it as-proposed, what do people dislike about that?
19:32:33 <cyberpear> `q('vars', precedence_list) | select('defined') | first`?
19:32:58 <agaffney> nitzmahone: defining it in meta/ is implicit functionality, and explicit is "better" for maintainability
19:33:15 <cyberpear> yeah, it gets messy to throw too much in `meta/`
19:33:33 <agaffney> my ideal solution is an `include_defaults` action, but there be dragons
19:33:46 <felixfontein> also meta/main.yml used to be free of jinja2 so far, I think
19:33:48 <nitzmahone> Yeah, that's a lot nastier
19:34:06 <agaffney> I just really don't like any implicit solution
19:34:13 <cyberpear> include_vars, precedence=defaults? (adn inventory_host, inventory_group, etc)
19:34:27 <nitzmahone> It's not, it should be templateable like everything else. The current solution is already implicit
19:34:47 <agaffney> cyberpear: same dragons as `include_defaults`, plus it's a bit uglier
19:35:05 <agaffney> nitzmahone: defaults/main.yml is implicit, but it's a known quantity
19:35:09 <bcoca> which is why defaults/main.yml should die and people just have a way to assign defaults in play
19:35:40 <bcoca> just cause bad magic is preexisging does not mean we make more of it
19:35:53 <cyberpear> -1 to killing defaults/main.yml
19:35:54 <nitzmahone> Role argspec would handle static cases nicely, but not dynamic
19:35:54 <agaffney> imo, meta/main.yml should not be consumed by ansible itself. currently, only 'dependencies' is consumed by ansible, and that can be replaced with import_role/include_role
19:35:59 <jtanner> "Intelligent" sounds like it's suggesting do_magic
19:36:18 <cyberpear> agaffney: precedence=* would be more flexible, even if ugly
19:36:40 <agaffney> cyberpear: there are ~30 levels of precedence. should they all be supported? dragons!
19:36:47 <tterranigma> @jtanner I wanted to sound cool
19:36:53 <jtanner> heh
19:37:00 <bcoca> precedence is misleading, not all precedence steps are maintained, some do overwrite, others are cached and overlayed on consumption
19:37:07 <jtanner> i'm not sure i fully understand the proposed "problem"
19:37:19 <agaffney> anything involving VarsManager is chock full of dragons...angry dragons
19:37:27 <nitzmahone> Basically dynamic includes for roles
19:37:27 <bcoca> problem is the data organization does not work with current ansible variables
19:37:39 <nitzmahone> Sorry, role defaults
19:37:45 <agaffney> jtanner: OS-specific defaults in a role that can be overridden by the role caller (which include_vars cannot easily be)
19:37:48 <jtanner> dynamic var*/defaults* includes for roles?
19:37:52 <bcoca> agaffney: epileptic sociopatic dragons with breath issues
19:38:09 <nitzmahone> Just the ability to override the source
19:38:12 <cyberpear> jtanner: the request is to have an easy way to specify os-specific role defaults that don't stomp on precedence like `include_vars: "{{ os}}"` would, and without having to duplicate all vars to have the real var an its default
19:38:14 <bcoca> agaffney: first found
19:38:24 <jtanner> eh, with the spottiness i still see on os detection in fact bug reports , i'm leery of trusting those
19:38:43 <cyberpear> bcoca: does first_found work with vars? I thought it was files only
19:38:44 <bcoca> ^ also, these would have to rely on ansible_distro/os facts
19:38:52 <bcoca> works with include_vars
19:39:11 <cyberpear> right, and os facts aren't available at parsing time like defaults are
19:39:22 <agaffney> jtanner: there's potential for weirdness there, but there already is with `include_vars: '{{ os_family }}.yml'`
19:39:40 <jtanner> true
19:40:24 <nitzmahone> Ah yes, forgot about that part
19:41:20 <bcoca> agaffney: but there doesnt need to be since |default ...
19:41:29 <sdoran> So the main issue is we want something between role defaults and role vars since role vars clobber host/group vars (and pretty much everything besides extra_vars).
19:41:44 <cyberpear> (whatever the solution, I wouldn't want it to be specific to OS facts)
19:41:52 <agaffney> yes, with the ability to dynamically include them
19:41:52 <sdoran> I use `first_found` with `include_vars` to do this.
19:41:53 <bcoca> you can define other vars in role
19:42:02 <bcoca> but its task/block level
19:42:16 <sdoran> Though that still isn't _quite_ the same.
19:42:18 <agaffney> sdoran: but include_vars clobbers most other vars at other precendence levels
19:42:21 <agaffney> which is the problem
19:42:24 <sdoran> Yeah.
19:42:24 <jtanner> isn't precedence configurable now?
19:42:25 <bcoca> or you can use import_role/include_role with defaults_from:
19:42:32 <bcoca> jtanner: somewhat, not totally
19:43:05 <jtanner> i feel like this is adding extra complexity and would be better served by having private role vars, tbh
19:43:12 <agaffney> terrible idea...a role importing/including itself with a different 'defaults_from' value
19:43:19 <bcoca> jtanner: we have those already
19:43:26 <jtanner> no vars are private though
19:43:33 <bcoca> agaffney: not itself, by caller
19:43:43 <bcoca> jtanner: there is swtich, to make them private
19:43:49 <jtanner> hrm
19:43:51 <bcoca> which is default in include_role
19:43:52 * jtanner is out of touch
19:43:53 <cyberpear> include_vars/with_first_found might be good enough w/ an added capability of pre-defining any undefined vars w/in a role namespace, such as foo_* resolves to foo_*_default, and the _default version lives in the os-specific vars file
19:44:52 <agaffney> var name rewriting? more dragons!
19:45:20 <agaffney> there's already 'name' for 'include_vars' for namespacing, but that's not much different than the current workaround solution
19:45:27 <cyberpear> not rewriting, just defaults for an undefined variable... I'll think on it before proposing it more concretely
19:45:47 <sdoran> So I use a "last resort" file in the `include_vars`.
19:45:59 <nitzmahone> Role argspec should cover that part
19:46:30 <cyberpear> there's also the issue of dependent roles having a higher-precedence with_first_found file, and the depending roles missing some required vars (or vice versa)
19:46:42 <cyberpear> (with role dependencies) -- been bitten by that before
19:47:03 <tterranigma> I am clueless about the actual implications or if this is a horrible idea, but sth that popped to my mind, could there be files under `defaults/` that have names like `ansible_os_family` and be picked according to the variable's value?
19:47:11 <cyberpear> in an included role, the with_first_found will find vars files from the including role
19:47:45 <bcoca> tterranigma: yes, with defaults_from: '{{ansible_os ...}}.yml
19:48:16 <sdoran> If the main use case is "defaults per OS", I think there are ways to do that already.
19:48:23 <bcoca> cyberpear:  just avoid role dependencies, they dont work as most expect, they never will, using include/import_role with specific params is much more flexible
19:48:36 <sdoran> `defaults_from` or using `include_vars`.
19:48:41 <bcoca> sdoran: yes, with include_vars +f irst_found
19:48:46 <sdoran> Exactly.
19:48:54 <bcoca> ^ we have examples in docs
19:48:57 <cyberpear> bcoca: there are certain cases where role deps they are quite elegant
19:49:18 <tterranigma> @bcoca I meant without resorting to `defaults_from`, for instance having files such as `defaults/ansible_os_family.yml` and `defaults/other_var_name.yml`
19:49:24 <cyberpear> but yes, I'll agree that in general they can be problematic
19:49:34 <bcoca> cyberpear: the exception and not worth the rule
19:49:51 <sdoran> While I have often felt the need for something in between role defaults and role vars, I haven't found a real world case that wasn't solved by the combination of role defaults + include_vars.
19:49:58 <bcoca> tterranigma: and if you didnt gather facts?
19:50:16 <tterranigma> @bcoca you pet your dragon.. :-/
19:50:35 <bcoca> tterranigma: which one? have to be very careful, they get jealous
19:50:44 <cyberpear> tterranigma: a trivial implementation of your solution would only work with include_role, not import_role or roles: because of when facts are gathered
19:51:24 <bcoca> nor roles: <= also get imported before facts
19:51:31 <bcoca> just tasks dont get executed
19:51:44 <cyberpear> yep
19:52:29 <sdoran> We need to move on from his topic. Please continue the discussion in the proposal.
19:52:37 <sdoran> #topic https://github.com/ansible/ansible/pull/59958
19:52:40 <cyberpear> the proposal as-is has no legs... can we table and re-visit if a better solution is proposed? I've got an idea about undefined defaults, but it needs fleshing out, so I'll bring it up later
19:52:45 <bcoca> which one? i got like 3 that are related
19:52:58 <cyberpear> (maybe we can link the 3 proposals)
19:53:14 <cyberpear> the #topic is that debug in a loop loses its changed status
19:53:23 <cyberpear> this is because of when the clean_results is called
19:53:34 <bcoca> that sounds like bug
19:53:44 <cyberpear> the PR referenced "updates position of _clean_results() in v2_runner_item_on_ok to match the position in v2_runner_on_ok"
19:53:44 * bcoca deletes debug and starts new with 'display action'
19:54:04 <cyberpear> basically, there's a callback for non-loops and loop-items
19:54:04 <bcoca> yeah, but not that simple, that move was done to fix another bug
19:54:08 <cyberpear> they're basically identical
19:54:18 <bcoca> not really, they are very close, but not the same
19:54:19 <cyberpear> bcoca: please show me
19:54:31 <bcoca> cannot lookup on phone
19:54:35 <agaffney> in what case should a 'debug' show as "changed"?
19:54:47 <bcoca> yes, that was an explicit 'ask'
19:54:50 <agaffney> I thought it always showed as "ok", but maybe that is just this bug
19:54:53 <cyberpear> agaffney: when the role author wants to call the attention to the role user
19:55:08 <bcoca> s/role//
19:55:14 <cyberpear> this PR makes them closer and calls the clean_results in the _item case the same as the base case
19:55:33 <cyberpear> bug was introduced here: https://github.com/ansible/ansible/commit/5ffb40fcdbefc03fee4994e5bbb8bf5cf5f76638
19:56:21 <miouge362> I hate to jump in like that but, I have 2 PRs stuck in review since May 2019 (55965 & 55989), CI is green, they got positive feedback from a couple of people (cloudnull and bcoca). Anything I can do to get things moving?
19:56:59 <agaffney> miouge362: please wait until the open floor, or until we get to your agenda items (if you added them to the agenda)
19:59:48 <sdoran> Anyone else have input on this? I will need to read through it more than I have time to right now.
20:00:52 <agaffney> bcoca: what was the original bug that change was fixing? it doesn't seem like there's a test for it, since the tests pass in this PR
20:01:02 <sdoran> Seems like it is working as intended currently. And it's pretty long standing behavior.
20:01:12 <bcoca> disclosure issue
20:01:22 <cyberpear> sdoran: it was broken w/ 2.4, worked in 2.3
20:01:46 <cyberpear> I've been bitten multiple times by it and wasted hours... the PR has links to 2 bug reports on it
20:01:49 <sdoran> cyberpear: Right. Longstanding "incorrect" behavior is still longstanding behavior. :)
20:02:02 <bcoca> dont know if the pr breaks it, just need to check it ... when i get some time
20:02:07 <sdoran> It's been there long enough that some may be relying on it, so it needs careful evaluation.
20:02:11 <cyberpear> if I specify `changed_when: yes`, I expect the thing to show as yellow changed
20:02:26 <cyberpear> where would someone specify changed_when: yes and want it to show green?
20:03:23 <sdoran> That seems reasonable. I just need time to evaluate it more.
20:03:47 <sdoran> I want to see what that change was fixing originally to fully understand it.
20:04:13 <sdoran> I'll try to look it before Thursday's meeting.
20:04:22 <sdoran> We are out of time for today.
20:04:34 <sdoran> Thank you everyone for participating.
20:04:36 <sdoran> #endmeeting