ansible_network_working_group
LOGS
16:03:07 <Qalthos> #startmeeting Ansible Network Working Group
16:03:07 <zodbot> Meeting started Wed Mar  6 16:03:07 2019 UTC.
16:03:07 <zodbot> This meeting is logged and archived in a public location.
16:03:07 <zodbot> The chair is Qalthos. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:03:07 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
16:03:07 <zodbot> The meeting name has been set to 'ansible_network_working_group'
16:03:34 <Qalthos> #chair justjais  privateip trishnag Anil_Lenovo pabelanger
16:03:34 <zodbot> Current chairs: Anil_Lenovo Qalthos justjais pabelanger privateip trishnag
16:03:54 <Qalthos> #topic Core Updates
16:04:20 <Qalthos> #info Released Network Engine 2.7.5 - Bugfix (ansible-network/network-engine#230)
16:05:01 <Qalthos> Not a lot to say this week
16:05:30 <Qalthos> #link https://github.com/ansible/community/labels/network is where the agenda lives
16:05:31 <grastogi> Please review https://github.com/ansible/ansible/pull/52661. some of our customers are hitting this issue.
16:06:48 <grastogi> We would also like to know if requests library is a safe assumption to make in network modules. We want to remove dependency of Avi SDK from the ansible modules by pushing the sdk as part of the network utils itself
16:07:00 <Qalthos> #topic cnos_l3_interface bugfix (#53267)
16:07:08 <Qalthos> Anil_Lenovo: anything you want to mention?
16:08:02 <Anil_Lenovo> I have one PR. Its actually a bug which my QA team has found out
16:08:21 <Anil_Lenovo> Well I have fixed it, tested it and raised this PR
16:08:47 <Qalthos> It looks pretty straightforward
16:09:04 <Anil_Lenovo> If the port is not configured as L3 port then the module cribs, So I have added a check on that
16:10:03 <Anil_Lenovo> Yes, very much in line with L2_interface module, there the port have to be made sure as SwitchPort
16:10:50 <Qalthos> Anil_Lenovo: My only nit is that https://github.com/ansible/ansible/pull/53267/files#diff-ad3cd39271814cd42e3bd2b5d4a37a0dL164 seems to be trying to configure the same interface twice?
16:11:38 <Qalthos> It's not a /problem/, but it is the documentation the the user sees, and doesn't really show off what aggregate can do
16:11:59 <Anil_Lenovo> Yes, True my mistake, I will correct it to Ethernet1/44
16:12:48 <Qalthos> Anil_Lenovo: +1, thanks
16:13:25 <Qalthos> #topic Fixed issues of using AviCredentials due to import errors #52661
16:17:15 <grastogi> There was an older PR to fix but it was not merge. It does not break the modules except avi_api_session.py module.
16:18:36 <Qalthos> grastogi: do you have a link to the older PR? was there a reason it was not merged?
16:18:52 <smolz> l
16:19:08 <grastogi> let me try to find it.
16:20:30 <ankitb__> Guys, can someone please review "ansible/ansible#51238"
16:20:31 <ankitb__> ansible/ansible#51238
16:21:15 <ankitb__> https://github.com/ansible/ansible/pull/51238
16:21:39 <smolz> I have one as well if possible:  https://github.com/ansible/ansible/pull/52892
16:22:27 <ankitb__> It was on the agenda the last time, but it has not been reviewed yet. Thanks in advance.
16:25:07 <Anil_Lenovo> Qalthos: I have modified based on your review comment. Have a look and merge it in due course of time
16:25:11 <grastogi> I can't find the last PR on it. will update it on the comment thread. However, we are preparing a new wider change to completely remove dependency on the external avisdk package
16:26:01 <Qalthos> grastogi: Is the issue that that is not expected to make 2.8?
16:26:18 <Qalthos> Or are you looking to backport this to 2.7?
16:26:37 <Qalthos> Anil_Lenovo: Will look when we're done here
16:27:25 <grastogi> yes, just this PR. The bigger change can go into next ansible
16:27:27 <grastogi> release
16:29:10 <Qalthos> grastogi: Understood. Seems good to go then
16:29:43 <Qalthos> #action Qalthos follow up on ansible/ansible#52661
16:29:55 <Qalthos> Let's see here...
16:30:41 <Qalthos> #topic New module itential iap_start_workflow (#51238)
16:30:55 <Qalthos> I see tests are passing now
16:33:29 <Qalthos> ankitb__: Based on what I'm seeing here, I'm guessing this meant to run as `connection: local`?
16:34:00 <ankitb__> @Qalthos, yes
16:35:50 <ankitb__> iap_start_workflow : I tested it with my platform, it takes in the token from iap_token and then kicks off a workflow
16:36:32 <ankitb__> Sorry, I was not around, I am online here, let me know if you have any other question
16:37:32 <Qalthos> I'd rather see it be able to use `connection: httpapi`... that would take a lot of implementation burden off the module to deal with the HTTP(S) things like authentication and SSL
16:38:03 <Qalthos> And should mean you don't need a separate iap_token module
16:40:10 <ankitb__> iap_token module gets the token, iap_start_workflow consumes that token for authentication and there are other inputs to the iap_start_workflow task. It kicks off the workflow
16:40:21 <ankitb__> iap_token is already merged.
16:40:36 <Qalthos> Better docs for using httpapi are unfortunately still on my work list, but https://github.com/ansible/ansible/tree/devel/lib/ansible/plugins/httpapi should give you an overview of what it can do
16:40:46 <Qalthos> ankitb__: Then why is it a new module in this PR?
16:41:18 * Qalthos looks
16:41:18 <ankitb__> iap_token and iap_start_workflow are two separate module.
16:43:13 <Qalthos> *sigh*, GitHub diff is telling lies again
16:43:55 <Qalthos> ankitb__: It isn't the end of the world, but if you want to look into it, I think httpapi could really simplify your modules down the line
16:44:25 <Qalthos> #action Qalthos follow up on ansible/ansible#51238
16:45:32 <ankitb__> ohk, I can take a look on how you guys are doing httpapi...
16:45:38 <Qalthos> I don't see anything obviously wrong, but I'll give it a response either way today
16:46:55 <Qalthos> #topic Add voice vlan option to ios_l2_interface module (#52892)
16:46:59 <ankitb__> Thanks, appreciate that.
16:47:03 <Qalthos> smolz_: If you're still with us
16:47:51 <smolz_> yup
16:49:39 <Qalthos> https://github.com/ansible/ansible/tree/devel/test/units/modules/network/ios has the other unit tests for ios modules. It should be fairly straightforward to make a new one for ios_l2_interface based on those
16:50:11 <Qalthos> Additionally https://docs.ansible.com/ansible/latest/dev_guide/testing_units_modules.html may be of some help
16:52:05 <smolz_> is that going to be a requirement to get this merged?
16:52:10 <Qalthos> It only /needs/ to cover the functionality that you're adding, but we are not accepting features without associated unit tests
16:58:25 <ankitb__> @Qalthos, I assume, once you merge my pr "https://github.com/ansible/ansible/pull/51238" I would have total of two modules merged. iap_token and iap_start_workflow?
16:59:58 <Qalthos> ankitb__: that seems to be the case
17:02:08 <ankitb__> Cool, ty for the confirmation.
17:02:26 <Qalthos> #action smolz_ Add unit tests to ansible/ansible#52892
17:02:27 <Qalthos> smolz_: If you run in to trouble, you should be able to find help in here
17:03:07 <smolz> looking at the others now
17:03:41 <Qalthos> And with that, we're past the hour, so I'm going to close the meeting
17:03:48 <Qalthos> Thanks to everyone for showing up
17:04:10 <Qalthos> Sorry that the beginning was a little sluggish
17:04:26 <Qalthos> #endmeeting