20:00:56 <jborean93> #startmeeting Ansible Windows Working Group
20:00:56 <zodbot> Meeting started Tue Feb  5 20:00:56 2019 UTC.
20:00:56 <zodbot> This meeting is logged and archived in a public location.
20:00:56 <zodbot> The chair is jborean93. Information about MeetBot at http://wiki.debian.org/MeetBot.
20:00:56 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
20:00:56 <zodbot> The meeting name has been set to 'ansible_windows_working_group'
20:00:59 <it-praktyk[m]> hey
20:01:26 <jborean93> hey
20:01:35 <jborean93> #chair it-praktyk[m] nitzmahone
20:01:35 <zodbot> Current chairs: it-praktyk[m] jborean93 nitzmahone
20:02:07 <jhawkesworth_> hey
20:02:24 <jhawkesworth_> did i miss WWG?
20:02:36 <jborean93> nope just started
20:02:42 <jborean93> #chair jhawkesworth_
20:02:42 <zodbot> Current chairs: it-praktyk[m] jborean93 jhawkesworth_ nitzmahone
20:02:49 <jhawkesworth_> hey all
20:03:58 <chopraaa> hey
20:04:09 <jborean93> #chair chopraaa
20:04:09 <zodbot> Current chairs: chopraaa it-praktyk[m] jborean93 jhawkesworth_ nitzmahone
20:04:11 <jborean93> hey
20:05:46 * jborean93 get's up agenda
20:06:33 <jborean93> #info https://github.com/ansible/community/issues/420
20:06:50 <jhawkesworth_> are we starting here: https://github.com/ansible/community/issues/420#issuecomment-459931778 ?
20:07:01 <jborean93> looks like dag is pre-occupied in the ongoing Ansible meeting
20:07:26 <jborean93> seems like a standard review cycle for various PRs
20:08:12 <jhawkesworth_> shall we circle back later, give dag a chance to finish in other meeting?
20:08:31 <chopraaa> win_product_facts LGTM
20:09:07 <jborean93> yea just about to rerun the tests and will merge if they pass
20:09:18 <jborean93> Firefox logged me out of everything so things are a bit slow for me today
20:10:39 <jhawkesworth_> iirc CI can't test client windows versions still (ie. windows 10)
20:10:55 <jhawkesworth_> I'll try and run a manual test against win 10 now for https://github.com/ansible/ansible/pull/50079
20:11:22 <jborean93> not client but I'm about to add Server 2019 which is based on the latest Windows 10 build
20:11:41 <jhawkesworth_> that's good to know.
20:11:47 <jborean93> that would be great though, please share your results in that PR if you can
20:12:03 <jborean93> #info https://github.com/ansible/ansible/pull/50033 backup module options
20:12:27 <jhawkesworth_> I will
20:12:35 <dag> sorry guys, I was distracted
20:12:41 <jborean93> that's ok
20:12:48 * jhawkesworth_ trying to remember pip pull from gh trick
20:13:39 <chopraaa> This is a welcome change
20:13:42 <jborean93> I just clone the ansible/ansible repo
20:13:55 <jborean93> `git fetch <remove users ansible repo> <remote branch>
20:14:00 <nitzmahone> jhawkesworth: `pip install git+https://github.com/ansible/ansible.git`
20:14:02 <jborean93> `git checkout FETCH_HEAD`
20:14:39 <jhawkesworth_> nice thanks .  I usually make a branch but nice to have a one-liner
20:14:52 <nitzmahone> jhawkesworth: oh, you're trying to install a PR to test it? http://blog.rolpdog.com/2018/08/testing-and-modifying-github-prs.html
20:15:15 * jborean93 have used that blog multiple times
20:15:32 <jhawkesworth_> great thank you
20:17:19 <jborean93> For the backup work, I'm not 100% sure on the work involved yet. This is a simple PR but I haven't really sat down and thought about the best way to implement it
20:19:23 <chopraaa> Would also have to be added to ansible.basic
20:19:51 <jborean93> I don't think the backup work needs to be part of Ansible.Basic.cs specifically
20:20:02 <jhawkesworth_> I think its nice having the backup functions in a separate .psm1
20:20:14 <jhawkesworth_> not every module will need them.
20:20:19 <nitzmahone> agreed- it's code that a small handful of modules need
20:20:21 <jborean93> but having `Fail-Json` in a module_util isn't the best thing to do https://github.com/ansible/ansible/pull/50033/files#diff-b746d2fe1ff27733e0d27e698d33f70aR18. We currently do it in some places but that's not right
20:20:25 <chopraaa> agreed
20:20:36 <nitzmahone> That's how we got to basic.py being a gigantic kitchen sink of stuff
20:21:25 <jborean93> we could make it that it takes in an AnsibleModule object and derives the backup params from there
20:22:14 <jborean93> that way we have a common module option implementation that is shared between modules. The trouble is that the module needs to support AnsibleModule first
20:23:09 <nitzmahone> Seems like having the modules pass the proper options to it directly would be much simpler and more flexible
20:23:37 <jhawkesworth_> it would be less tightly coupled
20:24:00 <nitzmahone> (and not make it depend on AnsibleModule, though if it needs Fail/Exit behavior, that might be a foregone conclusion?)
20:24:02 <jhawkesworth_> not that I mind ansible modules being coupled to AnsibleModule
20:24:27 <jborean93> fair enough
20:24:51 <jborean93> anyway I'll add some review notes to it
20:25:09 <jhawkesworth_> the 'interface' (i.e. module parameters seem good and consistent)  so implementation could be elaborated if necessary
20:25:40 <jborean93> anything else to add to that?
20:26:07 <jhawkesworth_> its a nice feature.  I'll try it
20:26:12 <jborean93> #info https://github.com/ansible/ansible/pull/49701 winrm read/connection timeout
20:26:34 <jborean93> So this one can't be done until the components in pywinrm have been added and I don't see changes happening there for a while (if at all)
20:27:41 <jborean93> the psrp one is in place but winrm would have to wait
20:28:08 <jhawkesworth_> wot no pywinrm maintainer ?
20:28:33 <jborean93> nitzmahone has rights but there isn't much appetite to continue adding features to it
20:29:12 <jhawkesworth_> ah so the future is elsewhere?
20:29:48 <nitzmahone> psrp, young padawan
20:29:49 <jborean93> potentially psrp but really pywinrm fits the majority of our needs apart from some edge cases
20:30:05 <nitzmahone> and places we want to go in the future (persistent interpreter)
20:30:19 <nitzmahone> Though supporting that in pywinrm might be easier than I originally thought
20:30:57 <jhawkesworth_> I can understand not wanting to do a release just to support this.
20:31:25 <jborean93> the discussion about execute_module() in ConnectionBase gives me hope of adopting some more features that psrp provides but will have to see how that pans out
20:31:42 <dag> jborean93: ok, good to know
20:32:04 <jborean93> anywho moving on
20:32:05 <jhawkesworth_> should we mothball pywinrm officially then.
20:32:15 <nitzmahone> maybe eventually
20:32:17 <jborean93> I don't think so yet
20:32:17 <dag> jborean93: it sucks that winrm can't move faster than it does
20:32:19 <jhawkesworth_> can discuss later if hot potato
20:32:43 <jhawkesworth_> ok well worth thinking on I guess.
20:32:53 <jborean93> #info https://github.com/ansible/ansible/pull/45870 setup processor info
20:32:55 <jhawkesworth_> I am onboard with not maintain 2 transports
20:34:05 <jborean93> I remember looking over this late at night but wasn't in the mindset to actually verify at the time
20:35:28 <jhawkesworth_> It looks good.  Would like to see if its been tested on bare metal and under virtualisation
20:35:58 <jborean93> yea I need to try it out on a few combinations with and without hyper threading to test it out
20:36:00 <jhawkesworth_> I'll try it
20:36:13 <jborean93> I'm just curious it hasn't been picked up before but who knows how people actually use this value
20:36:24 <jhawkesworth_> I defintely don't
20:36:56 <jhawkesworth_> I can see how it might be useful though
20:36:57 <chopraaa> a lot of issues get caught up in the mailing list or on reddit
20:37:19 <jborean93> but for that if anyone can test and validate the behaviour that would be fantastic. Having someone else confirm it's an issue and is fixed by that PR would be nice to know
20:37:59 <jborean93> #info https://github.com/ansible/ansible/issues/25532 connection refused
20:38:07 <jborean93> So not sure what's needed to be done here
20:38:23 <jborean93> we added retry support to psrp but just like the read timeout stuff this probably won't make it into pywinrm anytime soon
20:38:33 <jborean93> Also connection refused could mean lots of other things
20:38:49 <jborean93> There's also ongoing work with adding ssh support
20:39:07 <jborean93> dag: can you share if there's anything you are waiting for with https://github.com/ansible/ansible/issues/25532
20:41:12 <jhawkesworth_> seems like same applies as it seems to need pywinrm to change and that's unlikely now
20:41:37 <jhawkesworth_> dag are you all switched over to psrp now?
20:42:16 <jborean93> I know he uses it in some places but not sure by how much. I would still classify it as experimental as I'm not sure how much it may change in the future when it comes to connection persistence
20:42:18 <jhawkesworth_> ssh is 3rd transport isn't it.
20:42:31 <jborean93> for Windows it will be the 3rd option to be added yes
20:42:49 <dag> jhawkesworth_: yes
20:43:05 <jborean93> ok moving on
20:43:09 <dag> I am doing PSRP for everything, only use WinRM in case I have an issue to verify
20:43:10 <jhawkesworth_> so that's probably had some signifcant testing
20:43:26 <jborean93> #info https://github.com/ansible/ansible/pull/50759 win_psrepository nuget upgrade
20:43:43 <jborean93> it-praktyk[m]: what's the purpose of upgrading the nuget provider here?
20:43:46 <jhawkesworth_> I'm going to try switching to psrp in dev environments then
20:44:39 <it-praktyk[m]> to run integration tests on all supported PowerShell versions
20:45:06 <jborean93> so what "fails" on the older versions
20:45:20 <jborean93> I'm just concerned about having this auto update of dependency behaviour if it's not needed
20:45:55 <it-praktyk[m]> sorry, it's not that PR
20:48:24 <jborean93> ahh I see it fails with `"Problems adding LocalRepository repository: Exception calling \"ShouldContinue\" with \"2\" argument(s): \"A command that prompts the user failed because the host program or the command type does not support user interaction. The host was attempting to request confirmation with the following message: PowerShellGet requires NuGet provider version '2.8.5.201' or newer to interact with NuGet-based repositories.
20:48:24 <jborean93> The NuGet provider must be available in 'C:\\Program Files\\PackageManagement\\ProviderAssemblies' or 'C:\\Users\\vagrant\\AppData\\Local\\PackageManagement\\ProviderAssemblies'. You can also install the NuGet provider by running 'Install-PackageProvider -Name NuGet -MinimumVersion 2.8.5.201 -Force'. Do you want PowerShellGet to install and import the NuGet provider now?\"` if Nuget hasn't been bootstrapped beforehand
20:48:41 <it-praktyk[m]> lets move I'll explain that better under PR and we will return for that next week
20:49:07 <jborean93> ok, if that's the case I think just adding a warning saying this action was done and I'll be happy with the changes
20:49:31 <jborean93> #info https://github.com/ansible/ansible/pull/51044 win_disk_facts test fix
20:50:10 <it-praktyk[m]> currently integration tests are not performed :)
20:50:20 <jborean93> they are being skipped?
20:50:28 * jborean93 checks for them being unstable
20:50:43 <it-praktyk[m]> yes
20:50:57 <jhawkesworth_> thanks for changing `raw` to `win_shell`
20:51:07 <jborean93> they aren't marked as unstable at https://github.com/ansible/ansible/blob/devel/test/integration/targets/win_disk_facts/aliases
20:51:32 <jborean93> or they are always being skipped due to the buggy when condition?
20:51:56 <it-praktyk[m]> they are always skipped
20:52:08 <jborean93> ah ok, yep I see that in https://app.shippable.com/github/ansible/ansible/runs/105703/5/console
20:52:33 <jborean93> ok rerunning for the stale_ci check and will merge once that passes
20:52:35 <jborean93> thanks for that
20:53:18 <jborean93> #info https://github.com/ansible/ansible/pull/50808 win_psrepository test on all Win versions
20:54:25 <jhawkesworth_> nice
20:54:36 <jborean93> thanks for updating the URLs to the S3 buckets
20:55:29 <jhawkesworth_> FWIW https://github.com/ansible/ansible/pull/50079 tested out ok on windows 10
20:55:45 <jborean93> will have to review 50808 and get back to that one. Anyone else is welcome to review that
20:55:52 <jborean93> thanks jahwkesworth_ for the confirmation
20:56:13 <jborean93> sorry jhawkesworth_*
20:56:38 <jhawkesworth_> no probs, stupid untypeable name!
20:56:46 <jborean93> heh
20:56:50 <jborean93> ok last item on the list
20:56:55 <jborean93> #info win_format idempotency
20:57:15 <chopraaa> Just added another if you refresh #420
20:57:16 <jborean93> chopraaa this one is yours
20:57:40 <jhawkesworth_> yeah, what are you thinking?
20:57:43 <chopraaa> so need to discuss how we deal with idempotency when formatting partitions and volumes
20:58:05 <chopraaa> can either force format everything and make it non-idempotent
20:58:39 <chopraaa> but it's possible to detect if a partition is unformatted
20:58:47 <chopraaa> so idempotency is possible in that case
20:59:05 <jhawkesworth_> sounds like a safe thing to do, too
20:59:16 <chopraaa> but we'll have to add an additional parameter to force format
20:59:45 <chopraaa> since this doesn't work when we're dealing with volumes not made from single partitions
20:59:46 <jborean93> I personally think, format always if not already in that format type. Only format the same type if some override was set
21:00:24 <chopraaa> there's a bunch of other stuff to consider along with file system type though
21:00:38 <chopraaa> but yeah thats the general idea
21:01:15 <jborean93> I know but I think a warning should suffice saying the file system type was the same but things like block size was different so set the override
21:01:20 <chopraaa> could try to do a best fit, and format accordingly, and if an override is provided, we can always format regardless
21:01:50 <chopraaa> and improvements are always welcome to that best fit analysis
21:02:09 <jhawkesworth_> not sure what you mean by 'best fit'
21:02:39 <chopraaa> do my utter best to check if its ok to format it
21:02:42 <chopraaa> :-D
21:02:54 <jhawkesworth_> the module parameters should describe what's desired.  if the state it is in is not what's desired then it gets changed
21:02:57 <chopraaa> if not, look for an override
21:03:25 <jhawkesworth_> sorry I don't often wind up formatting disks, I'm probably not helping
21:03:47 <jborean93> the trouble with this is a format is quite destructive
21:03:56 <chopraaa> depending on the block size, its tough to judge if the volume actually needs a format
21:04:19 <chopraaa> since they report different sizes
21:04:25 <jhawkesworth_> ok so its hard to detect
21:05:49 <chopraaa> yeah
21:06:45 * chopraaa if only whatif reported would-be values as well...
21:06:53 <jhawkesworth_> I guess jborean93's proposal is best then.  warn if it looks like it might have changed  and let user set `force: true` if they really are sure they want to format
21:07:13 <chopraaa> okay
21:07:45 <jhawkesworth_> it sounds like a reasonable plan.  I guess have to make a PR and try it out to see if it will work in practice
21:08:35 <jborean93> yea ultimately you probably know more than all of us when trying it out so I would try things out and see how it goes
21:08:46 <chopraaa> makes sense
21:09:34 <jborean93> anything else before going onto the last topic?
21:09:47 <chopraaa> just the win_disk_facts stuff
21:10:15 <jborean93> I'm not against having a filter option, just really depends on the implementation
21:10:51 <chopraaa> in it's current state, the module makes it really difficult to sift through it
21:10:54 <jhawkesworth_> just wondering whether to call it `gather_subset` like facts?
21:11:22 <jborean93> I don't think we need to call it gather_subset
21:11:41 <jborean93> there are a few "facts" modules in AWS and Azure that provide their generic arguments like resource_group and so on
21:11:58 <jborean93> I think just having things like `volume`, `disk`, etc is enough to filter it down
21:12:29 <chopraaa> Im not sure how it'll work without a rewrite though
21:12:31 <jhawkesworth_> ok, it was just a thought
21:13:08 <chopraaa> i'll create an issue for this to get some opinions
21:13:18 <jborean93> sounds good
21:13:26 <jborean93> I haven't really used it so not the best person to ask
21:13:48 <jhawkesworth_> yeah not used it either
21:14:06 <chopraaa> well, im done
21:14:22 <jborean93> cool
21:14:48 <jborean93> so looks like we are passed our time, will bring it to a close for the next meeting
21:14:55 <jhawkesworth_> thanks everyone
21:15:04 <it-praktyk[m]> thanks
21:15:06 <jborean93> #endmeeting