00:03:28 #startmeeting Ansible Azure Working Group 00:03:28 Meeting started Thu Dec 21 00:03:28 2017 UTC. The chair is nitzmahone. Information about MeetBot at http://wiki.debian.org/MeetBot. 00:03:28 Useful Commands: #action #agreed #halp #info #idea #link #topic. 00:03:28 The meeting name has been set to 'ansible_azure_working_group' 00:04:15 Hey 00:05:22 * nitzmahone lurks 00:06:15 hello 00:06:34 Hey 00:07:20 i have a few hanging prs :-) 00:07:44 Yep, I've got a list of those SQL ones I wanted to review today 00:07:53 Just trying to tidy up a few things this morning before getting onto those 00:08:08 ok, great! 00:08:18 I know nitzmahone had put an agenda item to talk about but not sure if anyone else is joining 00:08:51 I think Zim was the one that was talking about it anyway, so probably the right guy ;) 00:09:12 True 00:09:23 #topic https://github.com/ansible/community/issues/281#issuecomment-352886856 00:09:25 and still one older 00:09:27 https://github.com/ansible/ansible/pull/32025 00:09:35 #info agenda https://github.com/ansible/community/issues/281 00:10:25 @zikalino I think you mentioned working on a bunch of SQL modules- was it SQL Server-specific stuff, or just the various Azure DB modules you've been doing? 00:10:57 right now sql, postgresql and mysql 00:11:08 Are you looking at adding in modules that do DDL operations 00:11:18 i have prs now for server and database 00:11:19 Or just modules to manage Azure instances like the server and database itself? 00:11:54 no, not yet, now just manage instances 00:12:00 OK cool, just wanted to make sure 00:12:27 There's been some talk about doing SQL Server management modules- wanted to make sure we weren't going to duplicate effort 00:12:38 next i have authorisation and application gateway 00:14:11 sounds good 00:14:15 well, i have all of the sql modules already, at different stage of development 00:14:29 most of them are already here: https://galaxy.ansible.com/Azure/azure_preview_modules/ 00:16:19 Cool, we also wanted to talk about some improvements we wanted to introduce to stabilise how we use the Python SDK 00:16:27 i also have some questions about _facts module 00:16:34 Sure go ahead 00:16:59 yes, my idea was to fix referred sdk versions, and update them when necessary. 00:17:07 ok, but let's start with _facts 00:18:00 (1) _facts testing, should we have separate tests for _facts module, or keep them together with core module? 00:19:01 I believe they should be separate 00:19:26 While I've seen people add facts modules at the same time as the other modules each PR should really only touch 1 of them 00:19:27 (2) i was wodnering if there are any rules how the return value should be structured, i am looking at exsisting modules, and it's pretty deep, like: ansible_facts.[fact type].[some fact data] 00:20:12 That's a good question that I'm not too sure on myself, when you return an object under `ansible_facts` it get's loaded as a fact on the host which I think isn't right for that this does 00:20:38 nitzmahone: do you know the stance on this for things like facts that are not about the host itself? 00:21:32 Yeah, let's not use the ansible_facts nesting. That was a choice someone else made that was fairly controversial (and that I disagreed with) 00:22:01 We're generally making new object `_facts` modules not actually return ansible_facts. 00:22:20 yep thought so but wanted to make sure 00:22:44 ok, so i will remove this from facts 00:23:30 So 00:23:55 just wonder about older azure modules, it would be nice to have it consistent, but changing this is not a good idea at this point, right? 00:24:43 What we'll probably do is return a new "slimmer" structure outside ansible_facts, and deprecate that (leave both for a few releases). 00:25:00 ok 00:25:05 It's unfortunately difficult to deprecate return values, since we can't give in-product warnings easily 00:25:11 (on access, say) 00:25:21 Much easier to deprecate args/modules 00:26:08 #chair jborean93 zikalino yuwei Kylie_ 00:26:08 Current chairs: Kylie_ jborean93 nitzmahone yuwei zikalino 00:28:28 Any more questions for the facts modules zikalino? 00:28:41 Sorry for late. Did we discuss python SDK version issue? 00:28:51 no, that's all about _facts, thank you! 00:29:08 Cool, Kylie_ we haven't yet as we wanted to wait :) 00:29:42 #topic Azure SDK stability https://github.com/ansible/community/issues/281#issuecomment-352886577 00:31:09 We need to have a long-term solution. In general, we should not expose latest python SDK version to end user of Ansible until we tested them internally. 00:32:09 yes, i was wondering if there's any problem with fixing versions, and if anybody needs newer version , then it can be moved manually after retesting all the modules 00:32:33 I think the issue we have is that the API backend changes according to the SDK 00:33:02 A proposed solution was to set the backend version in each module for the mgmt object used so that it stays consistent until we have to update it 00:33:54 I see the matt's comment, actually we specified the API version now, not haven't been done is to fix the sdk version installed on host 00:36:19 I'll leave nitzmahone to answer that he knows more about the internals than I do 00:36:22 The difference there would be moving to creating the clients in the modules using the shared method, instead of shared clients. That way each module can specify whatever REST API version it needs. The SDK versioning thing becomes a separate issue at that point, although if we had that in place now, we wouldn't have been broken by the recent changes 00:37:20 We should *theoretically* be able to let the SDK versions float again, or at least just specify a minimum. 00:37:23 So our modules use the latest sdk but call an old version API, cause this error https://github.com/ansible/ansible/pull/33897 00:38:39 Right, but once we're not using positional args anywhere, that problem goes away 00:39:23 Sounds like it's just not safe to use positional args on these objects anyway, so we should update everything to use kwargs 00:39:39 (according to @lmazuel) 00:39:41 For the network issue we can avoid by using name arts 00:39:45 Args 00:40:56 So once we're not using positional args anywhere and each module is specifying the REST API version it wants, I'm not sure there's a benefit to pinning to a particular SDK version, only specifying the minimum that has the newest REST API version for a given module. 00:40:57 Yep which was the 3rd point to move away from positional args to kwargs 00:41:58 Hi Matt you can see there is another pull request:https://github.com/ansible/ansible/pull/33897 00:42:49 This is caused by using the latest sdk but call the old specified version API 00:43:13 Right, but it's the same issue with the positional args, right? 00:43:30 Of course anyway we should update the args to kwargs 00:43:50 Oh, maybe it's not? 00:44:17 Not about the kwargs 00:44:29 That looks like a bug we should poke @lmazuel about 00:45:53 Consider this situation, we should avoid such gap time when sdk updated, that's why we want to fix the sdk version 00:46:29 Yeah, it's just a maintenance hassle I was hoping to avoid; we're basically rolling our own metapackage. 00:47:19 requirements-azure.txt was *supposed* to just be a stopgap measure until they figured out a better way to handle the Azure metapackage, but I'm not getting any answers on what's going to happen there. 00:48:17 But for now, I guess fine; we'll just pin all the SDK deps we need to an exact version. :( 00:49:10 Could we merge 33897 and 33624 as short-term solution? Any concern? 00:50:08 Yes, here we want to fix the sdk version and update it manually after we confirm the new version is not broken us 00:50:32 Yeah, that's probably the best thing to do... I'm checking with @mattclay WRT triggering full CI on changes to that, so hopefully we can know ahead of time if an SDK dep upgrade in there is going to break something else. 00:52:17 Great. So we make the agreement - fix the SDK version in Ansible for Azure and update the version once we get that test pass. 00:52:36 @Yuwei, will you update all versions to fixed one? 00:52:58 yes 00:53:05 Yep- assuming that a full CI pass on Azure modules is triggered by changes to the requirements file, that's what I'm waiting for an answer on. 00:53:10 I think 33897 and 33624 are only for networking and storage. 00:53:29 Yeah, gotta make sure we don't end up with a merge conflict there 00:54:41 @Yuwei, could we do it today? We need to get that fixed as soon as possible. 00:55:14 2.4.3 doesn't go to RC until Jan 10... 00:55:51 Then we need to fix that on current devel branch and request cherry pick to 2.4. 00:57:25 There is a question about to the CI, I sent a pull request after the SDK updated, assuming that the test should fail for vm but it passed. So the question is does our integration tests use some specified version of SDK in the image? Or not updated frequency? 00:58:21 It uses the test/runner/requirements/integration.cloud.azure.txt from inside the PR 00:58:41 https://github.com/ansible/ansible/blob/devel/test/runner/requirements/integration.cloud.azure.txt 00:58:42 ... hmm, but maybe it's cached in the container... 00:58:57 I think that is the requirements when the container is created 00:58:57 I wonder if we're using `--upgrade` - I bet not. 00:59:22 I think he's preinstalled most of the deps in the container image though, so it might not be force-upgrading. 00:59:27 yea 00:59:31 That's what I assumed 00:59:44 Won't be an issue once everything is version-pinned... 00:59:58 But yeah, probably should have him add `--upgrade` 01:00:31 great. Should we add action item here? 01:01:21 Sure 01:02:50 Yes seems that there is a cache thing, because of that the test file is the same as azure requirement one. So the upgrade flag is used when creating image not running integration test? 01:03:10 #action item Matt, ensure CI will add --upgrade for Azure SDK test. 01:04:11 I'm guessing that's the case... 01:04:32 Well, not even when creating the container, but the container images only get re-created occasionally... 01:04:44 (and they start from empty, so we'll always get the latest on a new container build) 01:06:41 @jborean93, could you please review https://github.com/ansible/ansible/pull/33223#pullrequestreview-81016172? We would like to build VMSS e2e scenario but no ip available due to this issue. 01:07:12 sure thing 01:07:19 Thank you:) 01:07:54 @nizmahone, @jborean93, we published the first version of Ansible extension in VS Code. VSCode extension: https://marketplace.visualstudio.com/items?itemName=vscoss.vscode-ansible 01:08:11 If you could have a try and let us what we should improve, it will be great. 01:08:48 This extension is not only for Azure or MS. 😃 01:09:04 I'll see if I can play around with it at some point 01:09:39 * nitzmahone has to run 01:10:06 Ok. Thank you all. Yes, it is a generic one, you could play in local, docker or cloud. 01:10:28 #action @Yuwei, open the PR to fix the python SDK version (ensure no conflict) and cherry pick to 2.4 to catch 2.4.3 RC. 01:10:38 Happy holiday:) 01:10:43 you too 01:10:55 #endmeeting