16:00:14 <Qalthos> #startmeeting Ansible Network Working Group
16:00:14 <zodbot> Meeting started Wed Jul 17 16:00:14 2019 UTC.
16:00:14 <zodbot> This meeting is logged and archived in a public location.
16:00:14 <zodbot> The chair is Qalthos. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:00:14 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
16:00:14 <zodbot> The meeting name has been set to 'ansible_network_working_group'
16:00:36 <Qalthos> #chair justjais pabelanger privateip trishnag
16:00:36 <zodbot> Current chairs: Qalthos justjais pabelanger privateip trishnag
16:00:46 <Qalthos> #topic Core Updates
16:01:10 <Qalthos> A couple of housekeeping items today
16:01:39 <pabelanger> o/
16:01:48 <pabelanger> was just grabbing drink
16:02:12 <Qalthos> #info AnsibleFest is coming up, Sept 24-26 in Atlanta, GA
16:02:22 <Qalthos> #link https://www.ansible.com/ansiblefest
16:03:01 <pabelanger> #info IBM and Red Hat now a thing, for those who missed
16:03:06 <pabelanger> #link https://www.ibm.com/cloud/redhat
16:03:25 <pabelanger> that closed last week and part of the reason for skipping our meeting on short notice last time
16:03:43 <sumkincpp> I have a question about paramiko_ssh.py(https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/connection/paramiko_ssh.py) which is mostly used for CLI network connections.
16:03:43 <sumkincpp> Shouldn't there be a connection retry in this plugin?
16:04:08 <pabelanger> sumkincpp: we should be able to reply once we get to open discussion
16:04:08 <Qalthos> We will, as usual, be having a Contributor Summit around the conference
16:05:02 <sushma-alethea> hello all
16:05:13 <sushma-alethea> I am a first timer to this meeting
16:05:27 <Qalthos> That will be the day before, on the 23rd
16:05:29 <pabelanger> I know we plan on doing some network related things for the contributor summit, but still working on plan
16:05:38 <sushma-alethea> is this a chat meeting? Not voice meeting?
16:05:58 <pabelanger> sushma-alethea: if you'd like to wait until open discussion, we can likely help then
16:06:23 <sushma-alethea> ok
16:07:35 <Qalthos> If you have any topics you would like to see covered at the contributor's summit, the organization is happening in https://etherpad.openstack.org/p/ansible-summit-atlanta-2019
16:08:41 <Qalthos> #link https://github.com/ansible/community/issues/449 for the holding issue on ansible/community
16:10:27 <Qalthos> On another topic, I'd like to know how the time of this meeting works for everyone
16:11:03 <mrproper> Qalthos: It works pretty well for me. But it does vary since my day job takes priority.
16:11:54 <pabelanger> yah, I raised this topic with the team recently. To maybe try to get more involved for discussion
16:12:17 <pabelanger> however, the majority of the team is APAC / EU
16:12:45 <pabelanger> my initial thought was rotating time zone for weekly meeting, to make it more friend to EU / APAC, but that might be complicated
16:13:02 <pabelanger> another idea, was a 2nd meeting, with more friendly APAC / EU
16:14:04 <Qalthos> But really, it depends on when is best for the people coming to this meeting, as we're really not in here to talk to ourselves, today's performance so far notwithstanding
16:14:33 <pabelanger> ++
16:15:03 <mrproper> I'm in the US and find this meeting critical to being able to get my PRs merged. So I'm happy with the time, but flexible depending on the new proposed time.
16:15:40 <pabelanger> right, with that though, having more ansible network member could make that better
16:15:45 <pabelanger> but, unknown at this point
16:16:06 <pabelanger> given there would be more eyes on PRs raised in the meeting
16:16:12 <bcoca> ^ why core has 2 meetings at diff times in the week
16:16:19 <Qalthos> #action everyone make noise if you would be like to see this meeting happen likely several hours earlier in the day
16:16:53 <bcoca> *crick*
16:17:10 <Qalthos> bcoca: We might end up going down that route. I don't think we would end up killing this time completely regardless
16:17:41 <Qalthos> But we also don't know if there's any desire for the kinds of times we can also cover
16:17:48 <Qalthos> Right, we can move on then
16:17:54 <bcoca> 12pm is good for usa/europ, 9am west coast, endof day in most european countries
16:17:58 <bcoca> 12pm est
16:17:59 <grastogi> couple of hours < 2.5hrs earlier is ok. anything earlier would be hard
16:19:03 <Qalthos> It would likely have to be >3 hours earlier
16:19:25 <mrproper> >3 earlier would be a little harder for me.
16:19:28 <Qalthos> pabelanger: Want to take over the agenda?
16:19:36 <pabelanger> sure
16:20:07 <pabelanger> lets go back to the things we missed last week
16:20:20 <pabelanger> #topic Meraki PRs
16:20:32 <pabelanger> #link https://github.com/ansible/ansible/pull/52889
16:20:37 <pabelanger> #link https://github.com/ansible/ansible/pull/57289
16:20:43 <pabelanger> #link https://github.com/ansible/ansible/pull/57855
16:20:49 <pabelanger> #link https://github.com/ansible/ansible/pull/55485
16:21:10 <pabelanger> So, being the bad human I am, I haven't found the time to review these
16:21:52 <pabelanger> given they have been requested a few time now, I'm going to force myself to do that today
16:22:12 <Qalthos> mrproper: I see integration tests, some unit tests would be nice as we can actually run those.
16:22:33 <mrproper> Qalthos: I have a separate PR to build out a unit test framework within my modules.
16:22:49 <mrproper> But I've struggled on the mocking/patching of fetch_url() so it hasn't gone very ffar.
16:23:00 <mrproper> https://github.com/ansible/ansible/pull/55251
16:23:12 <pabelanger> Is there an example platform where we do mock integration testing?
16:23:18 <pabelanger> I haven't looked to much myself
16:23:29 <pabelanger> #link https://github.com/ansible/ansible/pull/55251
16:23:31 <mrproper> pabelanger: There is, but it still isn't working properly. I've found it quite frustrating :(
16:24:00 <pabelanger> yah, I can see that.
16:24:08 <mrproper> I'm sure it's user error. I just don't know where.
16:24:41 <mrproper> So to answer Qalthos' suggestion, unit tests are on my roadmap to get a framework in and start using tests for the acutal modules themselves.
16:24:48 <pabelanger> given that there is no way to run an appliance of this, it won't be 100%, but nice to have something. To at least confirm there is on python syntax issues
16:25:18 <mrproper> Admittedly, I don't break out tasks into functions in the modules very frequently, so unit tests may be a little more challenging (at least simply so). But it's still doable.
16:26:19 <Qalthos> While I know that I in particular am horrible about remembering this, we do require unit tests on all new modules, and have since... 2.4, maybe?
16:27:12 <mrproper> ...oh. I didn't know that.
16:27:47 <pabelanger> Yah, unit / functional testing is extremly helpful as a reviewer too.
16:28:05 <mrproper> Is there a document which says
16:28:11 <mrproper> "Here's what we like to see in unit tests for new modules?"
16:28:20 <Qalthos> Yeah, like I said, I forget it myself
16:28:27 <pabelanger> 55485 does have some integration testing, but without hardware impossible to see results
16:28:51 <mrproper> pabelanger: I'm happy to work with you to show results.
16:28:55 <pabelanger> #action Find documentation for all new modules need unit tests
16:29:13 <pabelanger> mrproper: if possible, sharing them on the PR would be helpful
16:29:31 <mrproper> I'll update my PRs to show the integration test output.
16:29:38 <mrproper> Are you open to still reviewing them in advance of that?
16:29:39 <pabelanger> great
16:29:48 <pabelanger> yah, I'll start in 1.5 hours
16:29:56 <pabelanger> after I get some food
16:30:09 <mrproper> That's all I have for today. Thank you!
16:30:16 <pabelanger> moving on
16:30:18 <Qalthos> mrproper: the only document I'm aware of is https://docs.ansible.com/ansible/latest/dev_guide/testing_units_modules.html#api-definition-with-unit-test-cases
16:30:27 <pabelanger> #topic AVI PRs
16:30:34 <pabelanger> #link https://github.com/ansible/ansible/pull/57116
16:30:40 <Qalthos> er, well, the whole page, not necessarily that heading
16:30:41 <pabelanger> #link https://github.com/ansible/ansible/pull/58382
16:30:49 <pabelanger> #link https://github.com/ansible/ansible/pull/58667
16:30:56 <pabelanger> #link https://github.com/ansible/ansible/pull/57116
16:31:04 <pabelanger> #link https://github.com/ansible/ansible/pull/58382
16:31:12 <pabelanger> #link https://github.com/ansible/ansible/pull/58667
16:31:18 <pabelanger> that is all that was listed
16:31:29 <chaitanyaavi> Yes
16:31:30 <grastogi> yes, there is really three of them
16:31:31 <pabelanger> I believe we talked a little about this today in our bug triage meeting
16:31:46 <Qalthos> Oh, wow, I had pending comments on some of those that never got submitted
16:33:31 <pabelanger> ah, yes. IPs for testing
16:33:38 <Qalthos> chaitanyaavi: Can you talk about what https://github.com/ansible/ansible/pull/58382/files#diff-3fee4fdd73f2655ac24dd299af9c1264R43 is doing?
16:33:47 <pabelanger> I think we mentioned that in past, but doesn't look like we raised it
16:35:55 <Qalthos> In general, that PR isn't really testing much. You're not doing anything with the function's result, you're just checking that it returned... /something/
16:36:34 <chaitanyaavi> Yes I will update the part of the comment
16:38:24 <grastogi> PR mostly tests different use uses of obj_cmp. It is core to all the Avi modules. Each test is testing a use case. Just wondering if the comment was for one single test or all the tests..
16:39:52 <pabelanger> personally, I'd keep them all the same
16:40:00 <pabelanger> to simplify the code for testing
16:40:25 <Qalthos> That comment is specifically for that test, but generally your tests aren't testing anything useful about that function
16:40:51 <Qalthos> It could return `"bananas"` for all inputs and your tests would still pass
16:41:05 <grastogi> that is not true. they are testing comparison. they compare a sample output with what typically passed in playbook
16:41:15 <grastogi> the results are expected to be true or false
16:41:36 <grastogi> no that is not true.. you can try adding bananas anywhere and you will see it will fail
16:42:03 <grastogi> we will fix that test
16:42:06 <grastogi> for sure
16:42:07 <Qalthos> So the result is not a diff, but is_diff instead"
16:42:32 <Qalthos> I would probably then `assert diff is True` or `assert diff is False` instead then
16:42:52 <grastogi> that is not pythonic
16:43:09 <grastogi> I shouldn't need to unless I am required
16:43:20 <grastogi> for type checking
16:43:41 <Qalthos> Is it going to return things other than True or False?
16:43:46 <grastogi> no
16:45:24 <pabelanger> okay, we still have a few more items to get to, lets dig deeper after the meeting if needed.
16:45:35 <pabelanger> so we don't run out of tim
16:45:39 <pabelanger> time*
16:45:39 <grastogi> ok
16:45:42 <Qalthos> Then I'm really not seeing the problem. Still, they appear to do the correct thing, so I withdraw most of my comments
16:45:56 <pabelanger> #topic ios_config issues
16:46:03 <grastogi> there were more PRs
16:46:21 <pabelanger> grastogi: did I miss some?
16:46:36 <grastogi> https://github.com/ansible/ansible/pull/58667
16:46:45 <pabelanger> #undo
16:46:45 <zodbot> Removing item from minutes: <MeetBot.items.Link object at 0x7f6d2cbcfd50>
16:46:51 <pabelanger> err
16:46:57 <grastogi> https://github.com/ansible/ansible/pull/57116
16:47:24 <pabelanger> #topic AVI PRs
16:47:32 <pabelanger> don't know how to roll back the topic
16:47:46 <Qalthos> pabelanger: it's fine
16:48:26 <pabelanger> grastogi: so, 58667 is something I'm going to dig more into. There is some concern if that is something the lookup plugin should do
16:48:39 <pabelanger> I just need to ask reach out to maybe core to get some feedback
16:48:46 <pabelanger> should be able to update PR for next meeting
16:49:29 <pabelanger> 57116 looks to have existing discussion, I'll need to get up to speed on that also
16:49:46 <Qalthos> grastogi: For 57116 I still don't understand what the issue is with not deconstructing the paths into parameters that get built into paths in the module
16:49:54 <pabelanger> but looks to be some concern about style of code for it
16:50:40 <grastogi> They are references. And, we cannot always put that paths. I have tried to show other examples like URI, URL, or URNs
16:52:22 <pabelanger> okay, lets also discuss this more after the meeting. I suspect it will take a little time to solve
16:52:29 <grastogi> sure
16:52:34 <pabelanger> #topic AVI PRs
16:52:40 <pabelanger> #link https://github.com/ansible/ansible/issues/43996
16:52:48 <pabelanger> err
16:52:54 <pabelanger> #undo
16:52:54 <zodbot> Removing item from minutes: <MeetBot.items.Link object at 0x7f6d2cc9b810>
16:53:02 <pabelanger> #topic ios_config issues
16:53:08 <pabelanger> #link https://github.com/ansible/ansible/issues/43996
16:53:16 <pabelanger> #link https://github.com/ansible/ansible/issues/43861
16:53:46 <pabelanger> lets look to be old issues, but didn't get triaged properly
16:54:18 <Qalthos> I've been attched to them for a while
16:54:43 <pabelanger> we likey can also ask justjais feedback too
16:55:23 <pabelanger> okay, I've replied to them
16:55:27 <pabelanger> will keep an eye out
16:55:41 <pabelanger> #topic open discussion
16:55:47 <pabelanger> that is all we had on the agenda
16:56:01 <pabelanger> sumkincpp: if had question about paramiko
16:56:14 <pabelanger> sushma-alethea: feel free to ask too
16:56:21 <justjais> @pabelanger sure, I'll check and update respective github issues asap
16:56:31 <pabelanger> justjais: thanks!
16:56:37 <justjais> np!
16:57:27 <pabelanger> sumkincpp: interestingly enough, we are also having the discussion around network_cli retries. For testing, we are seeing failures (2% of the time)
16:57:44 <pabelanger> I know for ssh connection we do retries, and I've often thought that would be nice to have for network_cli
16:58:00 <pabelanger> sumkincpp: is there a specific issue you are having?
16:58:05 <pabelanger> on a platform
16:58:18 <sushma-alethea> thanks pabelanger
16:58:26 <sushma-alethea> I have raised a PR for ICX
16:58:32 <sumkincpp> Yep, NXOS 9.2.1, but I've also checked ssh.py connection plugin and wondering about retry logic
16:58:40 <sushma-alethea> We have developed 15 modules for ICX switches
16:59:03 <sushma-alethea> need to get these into Ansible main branch
16:59:19 <sumkincpp> So the problem is definitely in Cisco, but ansible currently doesn't have a possibility to retry if credentials were rejected once
16:59:20 <pabelanger> sumkincpp: we are mostly seeing the issue on iosxr (for testing), so would be interested to hear more about it. Especially if you have debug logs
16:59:31 <pabelanger> sumkincpp: for example, are you seeing 'Socket is closed' errors
16:59:39 <sumkincpp> EOF Transport
16:59:41 <sushma-alethea> my request is to please get quick reviews
17:00:08 <pabelanger> sushma-alethea: okay, thanks. I _think_ we did some triage today for ICX but will keep an eye out
17:00:34 <sumkincpp> At first I thought about paramiko, but Ansible not retrying auth errors in any case https://github.com/paramiko/paramiko/issues/878#issuecomment-484468883
17:00:55 <sushma-alethea> mainly for 1st PR: https://github.com/ansible/ansible/pull/58969
17:01:16 <sushma-alethea> once that is merged I believe we can raise 5 PRs
17:01:30 <sumkincpp> Also I have interesting sence that most Connection plugins should have additional layer of retry, i.e
17:01:37 <sushma-alethea> for 5 PRs should we create 5 forks?
17:01:43 <pabelanger> sumkincpp: if you want to raise an issue in ansible/ansible, we can debug together more. I know I've see auth issues before on junos
17:01:44 <sushma-alethea> or 5 branches on existing forks
17:02:03 <sushma-alethea> sorry if its a rookie question.. but I am using github process for first time
17:02:13 <pabelanger> sumkincpp: agree about retries, at one point I think it was supported but removed. I need to dig more why
17:02:38 <pabelanger> sushma-alethea: 1 fork, with 5 branches should be okay
17:02:50 <sushma-alethea> thanks pabelanger, much appreciated
17:02:58 <sumkincpp> pabelanger: application based retries -- for example there is no wy to retry TACACS unauthed attempts
17:03:07 <sushma-alethea> when is next ansible release planned?
17:03:25 <sushma-alethea> Need ICX modules to be part of that
17:03:32 <pabelanger> okay, we are about 3min over for today. Lets wrap up and continue discussion
17:03:35 <pabelanger> thanks all!
17:03:38 <pabelanger> #endmeeting