00:02:11 <nitzmahone> #startmeeting Ansible Azure Working Group 00:02:11 <zodbot> Meeting started Thu May 3 00:02:11 2018 UTC. 00:02:11 <zodbot> This meeting is logged and archived in a public location. 00:02:11 <zodbot> The chair is nitzmahone. Information about MeetBot at http://wiki.debian.org/MeetBot. 00:02:11 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic. 00:02:11 <zodbot> The meeting name has been set to 'ansible_azure_working_group' 00:02:14 <jborean93> haha, nearly :) 00:02:20 <nitzmahone> most impressive 00:03:56 <yuwei> hhhhh 00:03:57 <yungezz> @nitzmahone, about pr regarding auth username/password with default az cli client id, https://github.com/ansible/ansible/pull/37909, we've had an internal discussion with aad team, we're planning to remove the default client id from the pr 00:04:15 <yungezz> at same time, do well customer communication 00:04:41 <nitzmahone> OK, sounds good 00:04:44 <yungezz> will update the pr later 00:05:22 <nitzmahone> I'll try to do the review on it other than that this afternoon- only other thing I remember is that we could probably come up with a more descriptive name than "authority" 00:05:46 <Kylie_> Hi all 00:05:49 <nitzmahone> Like "adfs_endpoint" or ... something 00:05:55 <yungezz> authority is from aad library, it's for consistent 00:06:14 <yungezz> or adfs_authority something like that? 00:06:22 <Kylie_> Apologize for late. Failed to connect to IRC at home. Just found a machine with VPN. 00:06:39 <Kylie_> Did somebody start meeting? 00:06:44 <yungezz> yes, i met same issue when connecting with home network 00:06:47 <nitzmahone> Yeah, that's better- something like that. Maybe adfs_authority_url or something 00:06:54 <yungezz> ok 00:07:06 <nitzmahone> Bummer that IRC is so difficult to access for you 00:07:38 <nitzmahone> Kylie_, yes, meeting's been started 00:07:40 <nitzmahone> (twice ;) ) 00:07:49 <yungezz> :) 00:08:04 <nitzmahone> @yuwei - I'm *almost* finished with the SDK pinning update. 00:08:20 <nitzmahone> I keep thinking I have everything fixed when I have a few minutes, then I find a new issue/test failure :( 00:08:42 <nitzmahone> The underlying pattern we're using there is awfully fragile (our fault) 00:09:49 <yuwei> what's the failure? 00:09:53 <nitzmahone> @yuwei https://github.com/ansible/ansible/pull/38077 is approved, just waiting on tests to pass after I added the changelog fragment and we're good to go 00:10:12 <nitzmahone> @yuwei depends on how I change things 00:10:30 <yuwei> good to see it 00:10:46 <yuwei> I have a question about the backport 00:10:51 <nitzmahone> We'd originally said we were going to move all the clients to the modules, but with the latest stuff we discussed with Laurent in Redmond and the decision to explicitly pin *all* SDK versions, that's much less important now 00:11:37 <nitzmahone> So the failures depend on how I move that stuff around. I have one last (known) issue to solve with the sku validation on azure_rm_storageaccount, then I *think* we're goood 00:11:48 <nitzmahone> @yuwei what's your question? 00:11:57 <yuwei> you mean that maybe we will use model out of the module? 00:12:27 <nitzmahone> Probably still easiest to leave models inside the module rather than shared, but the shared clients in azure_rm_common are OK for now 00:12:47 <nitzmahone> (since there are N models for versioned clients that have the models tied to the operations) 00:13:12 <yuwei> I sent a backport pr https://github.com/ansible/ansible/pull/39266 but the integration test uses an option added in another pr which should not been backport 00:13:16 <nitzmahone> Regardless, it doesn't really matter except when we want to bump version (hopefully only on 2.5->2.6->2.7 transitions going forward) 00:13:22 <yuwei> so the test is failed 00:13:51 <nitzmahone> So in that case, it's not a simple cherry pick but a "real" backport where you'll need to remove the "new features" stuff from the 2.5 backport 00:15:11 <yuwei> yes yes 00:15:48 <yuwei> what I actually want to backport is the code, but test contains dirty part 00:16:13 <jborean93> yuwei: you can add further commits to your backport to remove the new features part 00:17:31 <nitzmahone> and/or use `git cherry-pick -n` to prevent committing the cherry-pick (allowing you to make the alteration inline with a single commit). Either way is fine- the end result post-merge will be the same. 00:18:06 <nitzmahone> Since you already have a PR created, adding a commit to remove the offending test code is probably easiest, but in the future either way is fine 00:18:07 * jborean93 didn't know about the -n option, will keep it in mind 00:19:09 <nitzmahone> On another note, I did a little cleanup on the Azure WG wiki- posted the meeting time and added a link to the "Broken Azure Tests" project: https://github.com/ansible/community/wiki/Azure 00:19:54 <nitzmahone> There are currently 12 Azure modules with disabled/unstable tests, would be good if those can get stabilized. 00:20:02 <yuwei> yes i will do the -n cherrypick 00:20:20 <yuwei> Zim is working on the unstable list 00:20:30 <nitzmahone> The underlying problem in many cases appear to be races in the module, where the test tries to delete a resource before it's fully "baked" 00:20:33 <nitzmahone> OK cool 00:20:38 <yuwei> I think he has drived some pr 00:20:49 <nitzmahone> Cool, I'll keep on the lookout for those 00:20:53 <Kylie_> I have a list for PR which need Jordan or Matt's review. 00:21:19 <Kylie_> Jordan, #36222, you approved the change. So it will be merged soon. Right? See the approval happened on 3/21 but now the status is still open. 00:21:25 <nitzmahone> Sounds like we may also at some point add a thing to CI where it can run a test repeatedly when unmarking a test "unstable" to check for stability over many runs 00:21:52 <nitzmahone> Kylie_ sure that's the right PR#? 00:22:02 <jborean93> https://github.com/ansible/ansible/pull/36222 seems to be a network one 00:22:02 <nitzmahone> 36222 is a network module PR 00:23:18 <Kylie_> sorry, let me double check. 00:24:34 <Kylie_> #37333 needs Jordan's review. And how about #38279? Not all requested changes address. Right? 00:25:35 <jborean93> https://github.com/ansible/ansible/pull/38279 needs tests 00:25:51 <Kylie_> @nitzmahone, https://github.com/ansible/ansible/pull/38643 needs your review. Somebody is asking for the progress in github. 00:26:01 <jborean93> havne't looked at https://github.com/ansible/ansible/pull/37333 00:26:04 <jborean93> yet 00:26:49 <Kylie_> #action @jborean93 help review https://github.com/ansible/ansible/pull/37333 00:27:03 <jborean93> regarding 38279, I'm still a bit wary about the way we want to approach facts 00:27:31 <jborean93> I think the meeting at Redmond it was decided to do 3 different types of facts but jumping into this too quickly probably isn't the best 00:28:01 <jborean93> we need to decide on how to expose the ways to return the different facts, the default option going forward, how to document the different results 00:28:42 <nitzmahone> Yeah, I talked over the "round-trippable" facts/module_args thing with a couple other people- we're not sure if that approach will be realistically workable or not 00:28:58 <jborean93> also documenting the return values is going to be hard 00:29:20 <nitzmahone> yep 00:29:26 <Kylie_> @yungezz, could you please discuss 38729 with Zim to see whether it is the approach we would like to adopt? 00:29:29 <nitzmahone> polymorphic return values are very painful to document 00:29:34 <yungezz> yes 00:30:03 <jborean93> there's also the question (if we are going with different return values) what the default form would be 00:30:18 <nitzmahone> I think we agreed that "curated" was best 00:30:19 <jborean93> Once we merge the module we are somewhat stuck with that form 00:30:44 <jborean93> so for 38279 we would need the curated option first before doing the raw ones which are currently in that PR 00:30:46 <nitzmahone> (so a relatively flat hand-picked set of "important" values) 00:31:10 * nitzmahone is also hoping to have return value deprecation warnings figured out for 2.7 00:31:23 <nitzmahone> So we can reasonably change the existing ones without too much pain 00:31:42 <jborean93> it's not just that, if we merge it as it is, then we are stuck with that default view without breaking people's playbooks 00:31:47 <jborean93> unless we return both 00:31:50 <yungezz> i think we met some facts module issue's outoupt format hard to fit into non-facts modules input 00:32:35 <nitzmahone> @yuwei I'm almost done with review on https://github.com/ansible/ansible/pull/38643/ - might want to rename one new arg and a couple minor doc changes, but otherwise looks good so far 00:32:40 <jborean93> did we decide whether the facts would be in a child key, e.g. `curated` would be returned under `results.curated`, `raw` would be `results.raw`? 00:32:53 <Kylie_> It is https://github.com/ansible/ansible/pull/36322. Jordan approved the change. So it will be merged soon. Right? See the approval happened on 3/21 but now the status is still open. 00:32:56 <jborean93> or whether it would just be returned straight in results 00:33:46 <jborean93> I believe a backport is under a manual merge and ansibot doesn't merge them automatically 00:33:47 <nitzmahone> We can merge 36322, but there's no planned 2.4 update at the moment 00:34:32 <nitzmahone> (merged) 00:34:45 <Kylie_> O, understand. It is for 2.4. Thank you. 00:34:47 <Kylie_> Matt, #36109, #36049 are for inventory. Yuwei did review. Need your review to move forward. The external contributor is active. Could you please review PRs to move forward? Thank you. 00:34:58 <Kylie_> Matt, #35888, a new module for Azure Kubernetes Service. Tested and ready_for_review. 00:35:06 <Kylie_> @yuwei, which one is for UA of inventory? 00:35:27 <Kylie_> @yuwei, where are we for 37627? Ready for review? 00:38:25 <yuwei> ua is not sent out yet 00:38:28 <yungezz> @jborean93, about the facts module return value, let's continue discuss on it in the pr or in this channel, i'll sync with zim on it 00:38:41 <nitzmahone> Merged 36109 00:39:42 <jborean93> yungezz: no worries, I would say having tests and the whole return value format sorted needs to be solved before we can merge that in 00:39:46 <yuwei> 37627 is ready for review 00:40:16 <Kylie_> 👍 @nitzmahone 00:40:25 <yungezz> ok 00:40:39 <nitzmahone> Need to play with 36049, at first glance there's a few things that look ... odd 00:40:55 <nitzmahone> (assigned myself for review) 00:41:12 <Kylie_> Thank you Jordan and Catherine. It seems the fact module for VM is needed by many people according to latest feedback. If needed, we could call a discussion on IRC to finalize the solution at this timeframe. 00:41:43 <Kylie_> #action @nitzmahone, help review #36049, #35888 and #37627. 00:41:47 <jborean93> sounds good to me 00:41:53 <yungezz> ok 00:42:02 <nitzmahone> So, for 37627, I guess we need to decide: do we want to do a separate top-level arg for all the different places where we can refer to a resource in another group, or not? 00:42:29 <nitzmahone> I can see reasons for both, but we have different patterns in different places 00:43:16 <nitzmahone> In some places, we accept either (a sub-dict of resource-group-name + resource name) or (a URL) or (an object name in the same resource group with the parent) 00:43:17 <yungezz> would that be confusing to users? 00:43:54 <nitzmahone> If it were just my choice, I'd probably say "accept either a resource ID URL or a resource name in the same group" 00:44:13 <nitzmahone> Jordan says having a way to do the named resource group + resource is useful too though 00:44:13 <jborean93> can you easily get the resource ID though? 00:44:34 <nitzmahone> IIRC there's an SDK construct for creating one if you know the right model type to use 00:44:37 <nitzmahone> (which we would) 00:45:09 <nitzmahone> but IIRC we have to be able to include the subscription, which we don't always have, which could require another round trip to look up using the subscription client 00:45:52 <nitzmahone> I guess if we agree that it's important to support all three modes, I prefer the sub-dict approach for RG + object_name, vs a separate top-level arg. 00:46:18 <nitzmahone> (eg, virtual_network_name + virtual_network_resource_group as top-level args) 00:46:36 <jborean93> As long as we are consistent with that I don't see an issue 00:46:40 <yungezz> for me, i wold vote for sub-dict for rource group + object_name 00:47:02 <nitzmahone> Right- currently we have all three patterns in use- I'd like us to agree on the one way we should do this going forward 00:47:16 <jborean93> sub dict for me 00:47:24 <yungezz> that make sense, we've seen lots of same request 00:47:26 <nitzmahone> Any other opinions? Going once... :) 00:47:28 <jborean93> with the ability to specify the full ID manually 00:47:34 <nitzmahone> yep 00:47:53 <yungezz> @yuwei, what's your thinking 00:48:08 <nitzmahone> So the sub-dict would only be *required* if you want to specify the RG name + object name, though we should probably *accept* it with just an object name (and assume the same group) 00:48:13 <yuwei> i m ok 00:48:31 <Kylie_> Then do you need update the PR? 00:48:48 <yuwei> remove some code hhhh 00:48:49 <nitzmahone> Yeah, IIRC that PR uses the "new toplevel arg" approch 00:49:07 <jborean93> yep, accept string or dict, if dict then make rg and sub id optional (default to current sub and rg) 00:49:25 <nitzmahone> We could probably also work out some shared code to handle those in a uniform fashion 00:49:46 <jborean93> + keep track/deprecate other areas where we use the top level args 00:49:53 <nitzmahone> yep 00:50:40 <Kylie_> Great. Thank you for suggestions. #action @yuwei, update #37627 00:50:42 <nitzmahone> #agreed new code that has external object refs should use a single top-level arg that can be either a resource URL, an object name in the same RG, or a sub-dict accepting the resource name and optionally which RG name it lives in 00:51:13 <nitzmahone> We should also agree on the sub-dict keys we're going to use... Where did I just see that sub-dict pattern used? 00:51:34 <jborean93> wouldn't we just have `name` and `resource_group`? 00:51:58 <nitzmahone> Probably, but for the top-level args we call it `resource_group_name` don't we? 00:52:00 * nitzmahone looks 00:52:19 <nitzmahone> Nope, you're right 00:52:24 <nitzmahone> it's just resource_group 00:52:29 <nitzmahone> So yeah, consistency is good 00:52:43 <jborean93> I like the shorter version anyway but looks to me `resource_group` :) 00:53:19 <nitzmahone> And the for the top-level arg names, it should just be $resource_type, so `virtual_network` or `security_group` or whatever. We can go back and alias the existing ones when we apply this 00:53:59 <nitzmahone> #info 2.6 core freeze is May 17, community freeze is May 31 00:53:59 <jborean93> agreed 00:55:05 <Kylie_> This month. I thought 2.6 will be around September. What is expected release date? 00:55:09 <nitzmahone> #agreed new arg names for external resource refs should just be $resource_type (eg, `virtual_network`, `security_group`), sometimes with a clarifying prefix/suffix if necessary 00:55:27 <nitzmahone> 2.6 will ship late June or mid-July 00:55:55 <nitzmahone> https://docs.ansible.com/ansible/devel/roadmap/ 00:57:05 <Kylie_> Ok, Thank you for heads-up. 00:57:35 <nitzmahone> Pretty much everything Azure falls under the May 31 deadline 00:57:52 <yungezz> leaving to catch shuttle 00:58:06 <Kylie_> Understand. It seems the cycle between 2.5 and 2.6 is shorter than 2.4-2.5. 00:58:30 <jborean93> 2.5 was long in general but the stabilisation plans made 2.6 a bit shorter than normal 00:58:48 <Kylie_> I see. Thank you. 00:59:13 <nitzmahone> We try to do 3-4 months between releases in general 00:59:38 <Kylie_> Got. I thought it was 4-6 months;) 00:59:38 <nitzmahone> Any other items for today? 00:59:54 <Kylie_> No. I think that is all. 00:59:56 <Kylie_> Thank you all 01:00:17 <Kylie_> #endmeeting Ansible_Azure_Working_Group 01:00:25 <nitzmahone> #endmeeting