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