00:02:11 #startmeeting Ansible Azure Working Group 00:02:11 Meeting started Thu May 3 00:02:11 2018 UTC. 00:02:11 This meeting is logged and archived in a public location. 00:02:11 The chair is nitzmahone. Information about MeetBot at http://wiki.debian.org/MeetBot. 00:02:11 Useful Commands: #action #agreed #halp #info #idea #link #topic. 00:02:11 The meeting name has been set to 'ansible_azure_working_group' 00:02:14 haha, nearly :) 00:02:20 most impressive 00:03:56 hhhhh 00:03:57 @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 at same time, do well customer communication 00:04:41 OK, sounds good 00:04:44 will update the pr later 00:05:22 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 Hi all 00:05:49 Like "adfs_endpoint" or ... something 00:05:55 authority is from aad library, it's for consistent 00:06:14 or adfs_authority something like that? 00:06:22 Apologize for late. Failed to connect to IRC at home. Just found a machine with VPN. 00:06:39 Did somebody start meeting? 00:06:44 yes, i met same issue when connecting with home network 00:06:47 Yeah, that's better- something like that. Maybe adfs_authority_url or something 00:06:54 ok 00:07:06 Bummer that IRC is so difficult to access for you 00:07:38 Kylie_, yes, meeting's been started 00:07:40 (twice ;) ) 00:07:49 :) 00:08:04 @yuwei - I'm *almost* finished with the SDK pinning update. 00:08:20 I keep thinking I have everything fixed when I have a few minutes, then I find a new issue/test failure :( 00:08:42 The underlying pattern we're using there is awfully fragile (our fault) 00:09:49 what's the failure? 00:09:53 @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 @yuwei depends on how I change things 00:10:30 good to see it 00:10:46 I have a question about the backport 00:10:51 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 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 @yuwei what's your question? 00:11:57 you mean that maybe we will use model out of the module? 00:12:27 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 (since there are N models for versioned clients that have the models tied to the operations) 00:13:12 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 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 so the test is failed 00:13:51 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 yes yes 00:15:48 what I actually want to backport is the code, but test contains dirty part 00:16:13 yuwei: you can add further commits to your backport to remove the new features part 00:17:31 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 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 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 There are currently 12 Azure modules with disabled/unstable tests, would be good if those can get stabilized. 00:20:02 yes i will do the -n cherrypick 00:20:20 Zim is working on the unstable list 00:20:30 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 OK cool 00:20:38 I think he has drived some pr 00:20:49 Cool, I'll keep on the lookout for those 00:20:53 I have a list for PR which need Jordan or Matt's review. 00:21:19 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 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 Kylie_ sure that's the right PR#? 00:22:02 https://github.com/ansible/ansible/pull/36222 seems to be a network one 00:22:02 36222 is a network module PR 00:23:18 sorry, let me double check. 00:24:34 #37333 needs Jordan's review. And how about #38279? Not all requested changes address. Right? 00:25:35 https://github.com/ansible/ansible/pull/38279 needs tests 00:25:51 @nitzmahone, https://github.com/ansible/ansible/pull/38643 needs your review. Somebody is asking for the progress in github. 00:26:01 havne't looked at https://github.com/ansible/ansible/pull/37333 00:26:04 yet 00:26:49 #action @jborean93 help review https://github.com/ansible/ansible/pull/37333 00:27:03 regarding 38279, I'm still a bit wary about the way we want to approach facts 00:27:31 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 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 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 also documenting the return values is going to be hard 00:29:20 yep 00:29:26 @yungezz, could you please discuss 38729 with Zim to see whether it is the approach we would like to adopt? 00:29:29 polymorphic return values are very painful to document 00:29:34 yes 00:30:03 there's also the question (if we are going with different return values) what the default form would be 00:30:18 I think we agreed that "curated" was best 00:30:19 Once we merge the module we are somewhat stuck with that form 00:30:44 so for 38279 we would need the curated option first before doing the raw ones which are currently in that PR 00:30:46 (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 So we can reasonably change the existing ones without too much pain 00:31:42 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 unless we return both 00:31:50 i think we met some facts module issue's outoupt format hard to fit into non-facts modules input 00:32:35 @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 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 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 or whether it would just be returned straight in results 00:33:46 I believe a backport is under a manual merge and ansibot doesn't merge them automatically 00:33:47 We can merge 36322, but there's no planned 2.4 update at the moment 00:34:32 (merged) 00:34:45 O, understand. It is for 2.4. Thank you. 00:34:47 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 Matt, #35888, a new module for Azure Kubernetes Service. Tested and ready_for_review. 00:35:06 @yuwei, which one is for UA of inventory? 00:35:27 @yuwei, where are we for 37627? Ready for review? 00:38:25 ua is not sent out yet 00:38:28 @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 Merged 36109 00:39:42 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 37627 is ready for review 00:40:16 👍 @nitzmahone 00:40:25 ok 00:40:39 Need to play with 36049, at first glance there's a few things that look ... odd 00:40:55 (assigned myself for review) 00:41:12 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 #action @nitzmahone, help review #36049, #35888 and #37627. 00:41:47 sounds good to me 00:41:53 ok 00:42:02 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 I can see reasons for both, but we have different patterns in different places 00:43:16 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 would that be confusing to users? 00:43:54 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 Jordan says having a way to do the named resource group + resource is useful too though 00:44:13 can you easily get the resource ID though? 00:44:34 IIRC there's an SDK construct for creating one if you know the right model type to use 00:44:37 (which we would) 00:45:09 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 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 (eg, virtual_network_name + virtual_network_resource_group as top-level args) 00:46:36 As long as we are consistent with that I don't see an issue 00:46:40 for me, i wold vote for sub-dict for rource group + object_name 00:47:02 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 sub dict for me 00:47:24 that make sense, we've seen lots of same request 00:47:26 Any other opinions? Going once... :) 00:47:28 with the ability to specify the full ID manually 00:47:34 yep 00:47:53 @yuwei, what's your thinking 00:48:08 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 i m ok 00:48:31 Then do you need update the PR? 00:48:48 remove some code hhhh 00:48:49 Yeah, IIRC that PR uses the "new toplevel arg" approch 00:49:07 yep, accept string or dict, if dict then make rg and sub id optional (default to current sub and rg) 00:49:25 We could probably also work out some shared code to handle those in a uniform fashion 00:49:46 + keep track/deprecate other areas where we use the top level args 00:49:53 yep 00:50:40 Great. Thank you for suggestions. #action @yuwei, update #37627 00:50:42 #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 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 wouldn't we just have `name` and `resource_group`? 00:51:58 Probably, but for the top-level args we call it `resource_group_name` don't we? 00:52:00 * nitzmahone looks 00:52:19 Nope, you're right 00:52:24 it's just resource_group 00:52:29 So yeah, consistency is good 00:52:43 I like the shorter version anyway but looks to me `resource_group` :) 00:53:19 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 #info 2.6 core freeze is May 17, community freeze is May 31 00:53:59 agreed 00:55:05 This month. I thought 2.6 will be around September. What is expected release date? 00:55:09 #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 2.6 will ship late June or mid-July 00:55:55 https://docs.ansible.com/ansible/devel/roadmap/ 00:57:05 Ok, Thank you for heads-up. 00:57:35 Pretty much everything Azure falls under the May 31 deadline 00:57:52 leaving to catch shuttle 00:58:06 Understand. It seems the cycle between 2.5 and 2.6 is shorter than 2.4-2.5. 00:58:30 2.5 was long in general but the stabilisation plans made 2.6 a bit shorter than normal 00:58:48 I see. Thank you. 00:59:13 We try to do 3-4 months between releases in general 00:59:38 Got. I thought it was 4-6 months;) 00:59:38 Any other items for today? 00:59:54 No. I think that is all. 00:59:56 Thank you all 01:00:17 #endmeeting Ansible_Azure_Working_Group 01:00:25 #endmeeting