-
Notifications
You must be signed in to change notification settings - Fork 108
feat: Added aci_bulk_static_bindings_to_epgs module (DCNE-622) #822
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat: Added aci_bulk_static_bindings_to_epgs module (DCNE-622) #822
Conversation
|
Hi @andreasgraber, Thank you for creation of the PR, we will have an internal discussion regarding your proposal. A few questions already from my end:
|
|
Hi @akinross, Thanks for the quick reply!
In the end it is an performance Issue. We have an IaC Stack in Front of the Playbook where we configure Servers in ACI. If we have a Server with a few Interfaces and each Interface have 300-400 Bindings the playbook takes over 30min with looping over the module aci_static_binding_to_epg. We also tried some performance improvements described in https://galaxy.ansible.com/ui/repo/published/cisco/aci/docs/optimizing/ but we could not get it faster.
I haven't tried it with parallel Execution. But I think than we need to handle Rate limiting Issues from the APIC as there are still hundreds of API Calls to configure one interface.
That's a good Point! My Experience with the APIC is that they can deal with very big payloads, but if somebody is pushing a big number of interfaces at once it could get problematic. To address this we could also limit the amount of interfaces under the attribute interface_configs or remove the list completely and allow just one interface at a time to be configured?
The current Bulk Module is addressing the usecase to push an EPG to many Interfaces. We need the use case to configure many EPGs to an interface at once, like a "switchport trunk allowed vlan" command on the catalyst switches. I think it could get hard to implement this logic in the current module? But maybe the naming of our module is confusing with the existing once?
I just saw that the /uni object gets the ansible annotation after running our bulk change module. This feels wrong to me as this object is managed by aci itself and we just want to manage the RsPathAtt Object. But we also could remove this part? Thanks for checking it internally and just let me know if You have any further question. Once we know if we can proceed with the PR I'm going to fix the the pipeline issues. Br, |
|
Hi @andreasgraber, Regarding your point 1, why would you refer to aci_static_binding_to_epg module and not the bulk equivalent, this would reduce the amount of tasks. In this case you would either loop per EPG or execute per EPG in parallel but having an inventory per EPG. What amount of EPGs are you configuring? Regarding your point 2, this would depend on the amount of EPG as asked in point 1. Regarding your point 4, the module is configuring a N amount of fvRsPathAtt MO to an EPG. I think this module could also take in a list of EPG to which you want to configure this same fvRsPathAtt MOs. As far as I am aware this module was made for performance optimisation of the single aci_static_binding_to_epg module. I personally think this module could be further adjusted, also to limit the amount of different modules which are sort of doing similar configuration. This would be a discussion with internal team. Regarding your point 5, this is wrong and happens automatically in the payload function because the assumption previously is made that there are no modules that are directly manipulation polUni. So likely in your situation each object that gets created should have this attribute added individually in my opinion. If you want to have some additional discussion on point 1 parallel execution we could set up a call to discuss your options there. Feel free to email me directly to schedule something. |
|
Hi @akinross We would be open to do some work on the current bulk_ module but I see an issue with the datastructure that would need some change. We would need:
|
|
Hi @dariomi, thanks for the clarifications. Will discuss with the team and get back to you on this. |
|
Hi @andreasgraber and @dariomi, I had an internal discussion and we have agreed that we will be willing to add this functionality. I will support you initially with writing the module, but after would need to follow the regular review process we have in our team. I would need some more time to do a proper review of the code and look at various options we have / choices we would need to consider. Are there any deadlines on your side for this? |
|
Hi @akinross, That are great news! Thx for the Information and Your support! You can also ping us on webex to proceed with the PR. Br, |
This PR enables configuration of trunk ports in one run. It is based on the already existing module aci_bulk_static_binding_to_epg.py. But in addition to that, the Goal is to push a list of EPGs to a list of interfaces.
Some additional notes:
Thanks for a review!