00:03:28 <nitzmahone> #startmeeting Ansible Azure Working Group
00:03:28 <zodbot> 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 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
00:03:28 <zodbot> The meeting name has been set to 'ansible_azure_working_group'
00:04:15 <jborean93> Hey
00:05:22 * nitzmahone lurks
00:06:15 <zikalino> hello
00:06:34 <jborean93> Hey
00:07:20 <zikalino> i have a few hanging prs :-)
00:07:44 <jborean93> Yep, I've got a list of those SQL ones I wanted to review today
00:07:53 <jborean93> Just trying to tidy up a few things this morning before getting onto those
00:08:08 <zikalino> ok, great!
00:08:18 <jborean93> I know nitzmahone had put an agenda item to talk about but not sure if anyone else is joining
00:08:51 <nitzmahone> I think Zim was the one that was talking about it anyway, so probably the right guy ;)
00:09:12 <jborean93> True
00:09:23 <jborean93> #topic https://github.com/ansible/community/issues/281#issuecomment-352886856
00:09:25 <zikalino> and still one older
00:09:27 <zikalino> https://github.com/ansible/ansible/pull/32025
00:09:35 <nitzmahone> #info agenda https://github.com/ansible/community/issues/281
00:10:25 <nitzmahone> @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 <zikalino> right now sql, postgresql and mysql
00:11:08 <jborean93> Are you looking at adding in modules that do DDL operations
00:11:18 <zikalino> i have prs now for server and database
00:11:19 <jborean93> Or just modules to manage Azure instances like the server and database itself?
00:11:54 <zikalino> no, not yet, now just manage instances
00:12:00 <nitzmahone> OK cool, just wanted to make sure
00:12:27 <nitzmahone> 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 <zikalino> next i have authorisation and application gateway
00:14:11 <jborean93> sounds good
00:14:15 <zikalino> well, i have all of the sql modules already, at different stage of development
00:14:29 <zikalino> most of them are already here: https://galaxy.ansible.com/Azure/azure_preview_modules/
00:16:19 <jborean93> Cool, we also wanted to talk about some improvements we wanted to introduce to stabilise how we use the Python SDK
00:16:27 <zikalino> i also have some questions about _facts module
00:16:34 <jborean93> Sure go ahead
00:16:59 <zikalino> yes, my idea was to fix referred sdk versions, and update them when necessary.
00:17:07 <zikalino> ok, but let's start with _facts
00:18:00 <zikalino> (1) _facts testing, should we have separate tests for _facts module, or keep them together with core module?
00:19:01 <jborean93> I believe they should be separate
00:19:26 <jborean93> 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 <zikalino> (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 <jborean93> 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 <jborean93> nitzmahone: do you know the stance on this for things like facts that are not about the host itself?
00:21:32 <nitzmahone> 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 <nitzmahone> We're generally making new object `_facts` modules not actually return ansible_facts.
00:22:20 <jborean93> yep thought so but wanted to make sure
00:22:44 <zikalino> ok, so i will remove this from facts
00:23:30 <yuwei> So
00:23:55 <zikalino> 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 <nitzmahone> 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 <zikalino> ok
00:25:05 <nitzmahone> It's unfortunately difficult to deprecate return values, since we can't give in-product warnings easily
00:25:11 <nitzmahone> (on access, say)
00:25:21 <nitzmahone> Much easier to deprecate args/modules
00:26:08 <nitzmahone> #chair jborean93 zikalino yuwei Kylie_
00:26:08 <zodbot> Current chairs: Kylie_ jborean93 nitzmahone yuwei zikalino
00:28:28 <jborean93> Any more questions for the facts modules zikalino?
00:28:41 <Kylie_> Sorry for late. Did we discuss python SDK version issue?
00:28:51 <zikalino> no, that's all about _facts, thank you!
00:29:08 <jborean93> Cool, Kylie_ we haven't yet as we wanted to wait :)
00:29:42 <jborean93> #topic Azure SDK stability https://github.com/ansible/community/issues/281#issuecomment-352886577
00:31:09 <Kylie_> 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 <zikalino> 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 <jborean93> I think the issue we have is that the API backend changes according to the SDK
00:33:02 <jborean93> 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 <yuwei> 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 <jborean93> I'll leave nitzmahone to answer that he knows more about the internals than I do
00:36:22 <nitzmahone> 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 <nitzmahone> We should *theoretically* be able to let the SDK versions float again, or at least just specify a minimum.
00:37:23 <yuwei> 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 <nitzmahone> Right, but once we're not using positional args anywhere, that problem goes away
00:39:23 <nitzmahone> 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 <nitzmahone> (according to @lmazuel)
00:39:41 <yuwei> For the network issue we can avoid by using name arts
00:39:45 <yuwei> Args
00:40:56 <nitzmahone> 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 <jborean93> Yep which was the 3rd point to move away from positional args to kwargs
00:41:58 <yuwei> Hi Matt you can see there is another pull request:https://github.com/ansible/ansible/pull/33897
00:42:49 <yuwei> This is caused by using the latest sdk but call the old specified version API
00:43:13 <nitzmahone> Right, but it's the same issue with the positional args, right?
00:43:30 <yuwei> Of course anyway we should update the args to kwargs
00:43:50 <nitzmahone> Oh, maybe it's not?
00:44:17 <yuwei> Not about the kwargs
00:44:29 <nitzmahone> That looks like a bug we should poke @lmazuel about
00:45:53 <yuwei> 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 <nitzmahone> Yeah, it's just a maintenance hassle I was hoping to avoid; we're basically rolling our own metapackage.
00:47:19 <nitzmahone> 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 <nitzmahone> But for now, I guess fine; we'll just pin all the SDK deps we need to an exact version. :(
00:49:10 <Kylie_> Could we merge 33897 and 33624 as short-term solution? Any concern?
00:50:08 <yuwei> 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 <nitzmahone> 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 <Kylie_> 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 <Kylie_> @Yuwei, will you update all versions to fixed one?
00:52:58 <yuwei> yes
00:53:05 <nitzmahone> 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 <Kylie_> I think 33897 and 33624 are only for networking and storage.
00:53:29 <nitzmahone> Yeah, gotta make sure we don't end up with a merge conflict there
00:54:41 <Kylie_> @Yuwei, could we do it today? We need to get that fixed as soon as possible.
00:55:14 <nitzmahone> 2.4.3 doesn't go to RC until Jan 10...
00:55:51 <Kylie_> Then we need to fix that on current devel branch and request cherry pick to 2.4.
00:57:25 <yuwei> 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 <nitzmahone> It uses the test/runner/requirements/integration.cloud.azure.txt from inside the PR
00:58:41 <jborean93> https://github.com/ansible/ansible/blob/devel/test/runner/requirements/integration.cloud.azure.txt
00:58:42 <nitzmahone> ... hmm, but maybe it's cached in the container...
00:58:57 <jborean93> I think that is the requirements when the container is created
00:58:57 <nitzmahone> I wonder if we're using `--upgrade` - I bet not.
00:59:22 <nitzmahone> I think he's preinstalled most of the deps in the container image though, so it might not be force-upgrading.
00:59:27 <jborean93> yea
00:59:31 <jborean93> That's what I assumed
00:59:44 <nitzmahone> Won't be an issue once everything is version-pinned...
00:59:58 <nitzmahone> But yeah, probably should have him add `--upgrade`
01:00:31 <Kylie_> great. Should we add action item here?
01:01:21 <nitzmahone> Sure
01:02:50 <yuwei> 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 <Kylie_> #action item Matt, ensure CI will add --upgrade for Azure SDK test.
01:04:11 <nitzmahone> I'm guessing that's the case...
01:04:32 <nitzmahone> Well, not even when creating the container, but the container images only get re-created occasionally...
01:04:44 <nitzmahone> (and they start from empty, so we'll always get the latest on a new container build)
01:06:41 <Kylie_> @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 <jborean93> sure thing
01:07:19 <Kylie_> Thank you:)
01:07:54 <Kylie_> @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 <Kylie_> If you could have a try and let us what we should improve, it will be great.
01:08:48 <yuwei> This extension is not only for Azure or MS. 😃
01:09:04 <jborean93> I'll see if I can play around with it at some point
01:09:39 * nitzmahone has to run
01:10:06 <Kylie_> Ok. Thank you all. Yes, it is a generic one, you could play in local, docker or cloud.
01:10:28 <Kylie_> #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 <Kylie_> Happy holiday:)
01:10:43 <jborean93> you too
01:10:55 <Kylie_> #endmeeting