Added support for Arista vlan to vni mapping new syntax

[drosarius]

fix #8164

[batfish-bot]

This change is 

[codecov]

Codecov Report

Merging #8197 (192b0f1) into master (3dca4a8) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #8197      +/-   ##
============================================
+ Coverage     74.50%   74.51%   +0.01%     
- Complexity    43496    43584      +88     
============================================
  Files          3385     3394       +9     
  Lines        168700   168953     +253     
  Branches      20174    20204      +30     
============================================
+ Hits         125686   125902     +216     
- Misses        33469    33498      +29     
- Partials       9545     9553       +8     

Impacted Files	Coverage Δ
...sh/grammar/arista/AristaControlPlaneExtractor.java	46.98% <100.00%> (+0.39%)	⬆️
.../datamodel/routing_policy/statement/Statement.java	72.72% <0.00%> (-9.10%)	⬇️
...src/main/java/org/batfish/coordinator/PoolMgr.java	54.76% <0.00%> (-5.96%)	⬇️
...ndor/sonic/grammar/SonicControlPlaneExtractor.java	89.21% <0.00%> (-5.23%)	⬇️
.../batfish/grammar/flatjuniper/GroupTreeBuilder.java	88.37% <0.00%> (-1.88%)	⬇️
...tamodel/routing_policy/expr/ExplicitPrefixSet.java	60.00% <0.00%> (-1.54%)	⬇️
...a/org/batfish/common/util/BatfishObjectMapper.java	72.72% <0.00%> (-1.19%)	⬇️
...a/org/batfish/job/ParseVendorConfigurationJob.java	82.64% <0.00%> (-1.02%)	⬇️
...ain/java/org/batfish/coordinator/WorkQueueMgr.java	70.93% <0.00%> (-0.59%)	⬇️
...src/main/java/org/batfish/coordinator/WorkMgr.java	75.66% <0.00%> (-0.32%)	⬇️
... and 35 more

[arifogel]

Reviewed 4 of 5 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @drosarius)

---

projects/batfish/src/main/java/org/batfish/grammar/arista/AristaControlPlaneExtractor.java, line 1100 at r2 (raw file):

  @Nonnull
  private static IntegerSpace toIntegerSpace(Vlan_rangeContext ctx) {

For this and the toIntegerSpace function below, please add a comment: // TODO: validate values.

---

projects/batfish/src/main/java/org/batfish/grammar/arista/AristaControlPlaneExtractor.java, line 3565 at r2 (raw file):

  @Override
  public void enterEos_vxif_vxlan_vlan_vni_range(Eos_vxif_vxlan_vlan_vni_rangeContext ctx) {
    List<Integer> vnis = new ArrayList<>();

I'm not sure this extraction strategy works.
What is the semantics of this line?
Do you enumerate vlans and vnis one by one in order declared and map them accordingly? If so, your iteration method will mess up the order, since you end up enumerating each space in sorted order (not to mention contiguous ranges are coalesced in IntegerSpace enumeration of ranges).
If the semantics is map each vlan subrange in order to each vxlan range in order, then you shouldn't use IntegerSpace at all.
Either way, you should check that the mappings are 1:1 and abort with a warning (warn(ctx, "some message")) if not.

---

projects/batfish/src/main/java/org/batfish/grammar/arista/AristaControlPlaneExtractor.java, line 3568 at r2 (raw file):

    List<Integer> vlans = new ArrayList<>();

    if (ctx.vlans != null) {

Remove. This is always true.

---

projects/batfish/src/main/java/org/batfish/grammar/arista/AristaControlPlaneExtractor.java, line 3575 at r2 (raw file):

        }
      }
      if (ctx.vnis != null) {

Remove. This is always true.

---

projects/batfish/src/main/java/org/batfish/grammar/arista/AristaControlPlaneExtractor.java, line 3584 at r2 (raw file):

        for (int i = 0; i < vnis.size(); i++) {
          int idx = i;
          _eosVxlan.getVlanVnis().computeIfAbsent(vlans.get(idx), n -> vnis.get(idx));

FYI If it's typical to use large ranges here, the current use of individual mappings in the vendor model is a bad (inefficient) representation. But fixing that is out of scope for this PR.

---

projects/batfish/src/test/resources/org/batfish/grammar/arista/testconfigs/arista_vxlan_new_syntax, line 3 at r2 (raw file):

!RANCID-CONTENT-TYPE: arista
!
hostname arista_vxlan_new_syntax

Which lines are new syntax? Please add a ! comment above each line that is new syntax, and identify (any) version using this new syntax.

[arifogel]

Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @drosarius)

---

projects/batfish/src/main/java/org/batfish/grammar/arista/AristaControlPlaneExtractor.java, line 1142 at r3 (raw file):

  }

  private static SubRange toSubRange(Vlan_subrangeContext ctx) {

Please add the comment: // TODO: validate values to each of the toSubRange functions you added..

---

projects/batfish/src/main/java/org/batfish/grammar/arista/AristaControlPlaneExtractor.java, line 3578 at r3 (raw file):

    for (int i = 0; i < vnis.size(); i++) {
      int idx = i;
      _eosVxlan.getVlanVnis().computeIfAbsent(vlans.get(idx), n -> vnis.get(idx));

Regarding use of computeIfAbsent:
So you cannot overwrite a mapping with this line? Are you sure you shouldn't just put instead?

---

projects/batfish/src/test/java/org/batfish/grammar/arista/AristaGrammarTest.java, line 377 at r3 (raw file):

  public void testVxlanVniNewSyntax() {
    AristaConfiguration config = parseVendorConfig("arista_vxlan_new_syntax");
    assertThat(config.getEosVxlan().getVlanVnis(), hasEntry(1, 222));

Please test every mapping.
I'd suggest a pattern that is slightly more verbose but still complete and gives good assertion failure messages:

// (with static import of Maps.immutableEntry)

assertThat(config.getEosVxlan().getVlanVnis().entrySet(), containsInAnyOrder(
  immutableEntry(k1, v1),
  immutableEntry(k2,v2),
  // ...
  immutableEntry(kn,vn)));

---

projects/batfish/src/test/resources/org/batfish/grammar/arista/testconfigs/arista_vxlan_new_syntax, line 9 at r3 (raw file):

   vxlan udp-port 4789
!New Syntax
   vxlan vlan 1,2,3,5-10,11-20 vni 222,111,2,555-560,11-20

Can you split this up into qualitatively different cases so each is easy to read and reason about?
E.g.

! 1:1
vxlan vlan 1,2 vni 1,2
! matching ranges
vxlan vlan 3-5 vni 3-5
! range on right
vxlan vlan 8,7,6 vni 6-8
! range on left
vxlan vlan 9-11 vni 11,10,9
! invalid size not equal (test absence of key `12`)
vxlan vlan 12 vni 13,14

If the semantics of the vxlan vlan line is to clear and replace all mappings, then you'll have to split each interesting case into a separate file and junit test.
Else if the semantics of the vxlan vlan line is to append (and replace existing?) mappings, can just split into separate lines as in example code given above.

[arifogel]

Reviewed 7 of 7 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @drosarius)

---

projects/batfish/src/test/java/org/batfish/grammar/arista/AristaGrammarTest.java, line 413 at r4 (raw file):

    {
      AristaConfiguration config = parseVendorConfig("arista_vxlan_new_syntax_invalid");
      assertThat(

Please also test that the mapping was not saved.

[arifogel]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @drosarius)

[arifogel]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @drosarius)