Refactor(eos_designs): Deprecating port_channel.short_esi under connected_endpoints

[Shivani-chourasiya]

Change Summary

Deprecating port_channel.short_esi under connected_endpoints

Related Issue(s)

Fixes #86

Component(s) name

arista.avd.eos_designs

Checklist

User Checklist

• N/A

Repository Checklist

• My code has been rebased from devel before I start
• I have read the CONTRIBUTING document.
• My change requires a change to the documentation and documentation have been updated accordingly.
• I have updated molecule CI testing accordingly. (check the box if not applicable)

[gmuloc]

@Shivani-chourasiya / @carlbuchmann
So the issue states that the key would be deprecated in 4.0 (which has not been done yet since there is no deprecation)

And then it should be removed in 5.0.0

However the PR right now targets devel which will be used for 4.2.0,...
So in my opinion you cannot set removed: true now

You should only deprecate it and output a deprecation warning and when we are working on releasing 5.0 we will need a new PR to remove the key (and implement the tests you have here)..

So for now the PR should only focus on outputing a deprecation warning when the key is used and when we are closer to release 5.0 we should remove it only.

[Shivani-chourasiya]

@Shivani-chourasiya / @carlbuchmann So the issue states that the key would be deprecated in 4.0 (which has not been done yet since there is no deprecation)

And then it should be removed in 5.0.0

However the PR right now targets devel which will be used for 4.2.0,... So in my opinion you cannot set removed: true now

You should only deprecate it and output a deprecation warning and when we are working on releasing 5.0 we will need a new PR to remove the key (and implement the tests you have here)..

So for now the PR should only focus on outputing a deprecation warning when the key is used and when we are closer to release 5.0 we should remove it only.

Sure @gmuloc, will update it

[Shivani-chourasiya]

@Shivani-chourasiya / @carlbuchmann So the issue states that the key would be deprecated in 4.0 (which has not been done yet since there is no deprecation)

And then it should be removed in 5.0.0

However the PR right now targets devel which will be used for 4.2.0,... So in my opinion you cannot set removed: true now

You should only deprecate it and output a deprecation warning and when we are working on releasing 5.0 we will need a new PR to remove the key (and implement the tests you have here)..

So for now the PR should only focus on outputing a deprecation warning when the key is used and when we are closer to release 5.0 we should remove it only.

@gmuloc if I remove removed: true from schema, it won't be raising error on using port_channel.short_esi. Shall I remove the test-case from eos_designs_negative_unit_tests for now?

[gmuloc]

@Shivani-chourasiya / @carlbuchmann So the issue states that the key would be deprecated in 4.0 (which has not been done yet since there is no deprecation)
And then it should be removed in 5.0.0
However the PR right now targets devel which will be used for 4.2.0,... So in my opinion you cannot set removed: true now
You should only deprecate it and output a deprecation warning and when we are working on releasing 5.0 we will need a new PR to remove the key (and implement the tests you have here)..
So for now the PR should only focus on outputing a deprecation warning when the key is used and when we are closer to release 5.0 we should remove it only.

@gmuloc if I remove removed: true from schema, it won't be raising error on using port_channel.short_esi. Shall I remove the test-case from eos_designs_negative_unit_tests for now?

@Shivani-chourasiya so what you should do is the following:

1/ Deprecate the key without removing it
2/ Update all the molecule tests in the eos_designs repo to use the new syntax (maybe they already do)
3/ Add a molecule scenario eos_designs_deprecated_vars like the one we have for eos_cli_config_gen https://github.com/aristanetworks/ansible-avd/tree/devel/ansible_collections/arista/avd/molecule/eos_cli_config_gen_deprecated_vars where you can test for deprecated vars so we make sure we don't break support until we remove them

Let me know if you need help

[Shivani-chourasiya]

@Shivani-chourasiya / @carlbuchmann So the issue states that the key would be deprecated in 4.0 (which has not been done yet since there is no deprecation)
And then it should be removed in 5.0.0
However the PR right now targets devel which will be used for 4.2.0,... So in my opinion you cannot set removed: true now
You should only deprecate it and output a deprecation warning and when we are working on releasing 5.0 we will need a new PR to remove the key (and implement the tests you have here)..
So for now the PR should only focus on outputing a deprecation warning when the key is used and when we are closer to release 5.0 we should remove it only.

@gmuloc if I remove removed: true from schema, it won't be raising error on using port_channel.short_esi. Shall I remove the test-case from eos_designs_negative_unit_tests for now?

@Shivani-chourasiya so what you should do is the following:

1/ Deprecate the key without removing it 2/ Update all the molecule tests in the eos_designs repo to use the new syntax (maybe they already do) 3/ Add a molecule scenario eos_designs_deprecated_vars like the one we have for eos_cli_config_gen https://github.com/aristanetworks/ansible-avd/tree/devel/ansible_collections/arista/avd/molecule/eos_cli_config_gen_deprecated_vars where you can test for deprecated vars so we make sure we don't break support until we remove them

Let me know if you need help

Hi @gmuloc, I have added the molecule scenario with a test-case for port_channel.short_esi.
Once this is reviewed I will add it for cvp_instance_ip PR as well

[gmuloc]

so a couple of things to fix still in there.

The issue does state to deprecate port_channel.short_esi and this is taking the right direction. I am now wondering if anything should be done for the port_channel.subinterface.short_esi. Technically we still want to be able to set different ethernet_segment.identifier per port-channel subinterfaces so I think this should stay but would like to get the feedback of other reviewers.

[gmuloc]

LGTM - thanks

[ClausHolbechArista]

Connected endpoints ESI should only matter for EVPN VTEPs, so please change the test to l3leaf instead.
But this also reveals that our code for l2leaf is configuring ESI even though we should not. (Opening seperate issue on that.)