Junos : Add parser support for set protocols isis overload advertise-high-metrics

[pranavbj-amzn]

From juniper docs:

The advertise-high-metric setting is only valid while the routing device is in overload mode. When advertise-high-metric is configured, IS-IS does not set the overload bit. Rather, it sets the metric to 63 or 16,777,214, depending whether wide metrics are enabled. This allows the overloaded routing device to be used for transit as a last resort.

An L1-L2 router in overload mode stops leaking route information between L1 and L2 levels and clears its attached bit. This is also true when advertise-high-metrics is configured.

For now marking it as a todo to model this correctly. This fix handles the parse warning.

[batfish-bot]

This change is 

[dhalperi]

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_isis.g4 line 84 at r1 (raw file):

  (
    advertise_high_metrics
    | apply

apply is the junos-specific exception to alphabetization -- it's always first if present. Sorry.

(This is because it's a stupid meta thing that's required for flattening and apply-groups and weird hierarchical config stuff. Sorry.)

Code quote:

    advertise_high_metrics
    | apply

[dhalperi]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @pranavbj-amzn)

---

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_isis.g4 line 83 at r1 (raw file):

  OVERLOAD
  (
    advertise_high_metrics

iso_advertise_high_metrics representing the iso_ prefix for isis overload. Again, apply is a weird Junos-specific exception to this. Sorry!

Code quote:

advertise_high_metrics

---

projects/batfish/src/test/java/org/batfish/grammar/flatjuniper/FlatJuniperGrammarTest.java line 1639 at r1 (raw file):

        hasParseWarning(
            "configs/" + hostname, containsString("This feature is not currently supported")));
  }

fine as-is, but also you don't need to check for warnings with todo. Those can be assumed to just work, and code coverage will make sure the line gets hit.

Code quote:

    String hostname = "juniper-set-protocols-isis";
    Batfish batfish = getBatfishForConfigurationNames(hostname);
    ParseVendorConfigurationAnswerElement pvcae =
        batfish.loadParseVendorConfigurationAnswerElement(batfish.getSnapshot());
    assertEquals(pvcae.getWarnings().size(), 1);
    assertThat(
        pvcae,
        hasParseWarning(
            "configs/" + hostname, containsString("This feature is not currently supported")));
  }

[codecov]

Codecov Report

Merging #8888 (a54e44a) into master (4ea5d1b) will decrease coverage by 0.01%.
Report is 2 commits behind head on master.
The diff coverage is 100.00%.

❗ Current head a54e44a differs from pull request most recent head 6d604c5. Consider uploading reports for the commit 6d604c5 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8888      +/-   ##
==========================================
- Coverage   72.51%   72.50%   -0.01%     
==========================================
  Files        3319     3319              
  Lines      169437   169439       +2     
  Branches    19878    19878              
==========================================
- Hits       122865   122856       -9     
- Misses      37426    37436      +10     
- Partials     9146     9147       +1     

Files	Coverage Δ
...fish/grammar/flatjuniper/ConfigurationBuilder.java	67.65% <100.00%> (+0.01%)	⬆️

... and 2 files with indirect coverage changes

[dhalperi]

Reviewed all commit messages.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @pranavbj-amzn)

---

projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java line 5847 at r2 (raw file):

  @Override
  public void exitAdvertise_high_metrics(Advertise_high_metricsContext ctx) {

FYI needs to be changed now that you renamed the rule

[dhalperi]

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pranavbj-amzn)

[pranavbj-amzn]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dhalperi)

---

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_isis.g4 line 83 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

iso_advertise_high_metrics representing the iso_ prefix for isis overload. Again, apply is a weird Junos-specific exception to this. Sorry!

okay interesting, updated accordingly!

---

projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_isis.g4 line 84 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

apply is the junos-specific exception to alphabetization -- it's always first if present. Sorry.

(This is because it's a stupid meta thing that's required for flattening and apply-groups and weird hierarchical config stuff. Sorry.)

fixed

---

projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java line 5847 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

FYI needs to be changed now that you renamed the rule

yes , done!

---

projects/batfish/src/test/java/org/batfish/grammar/flatjuniper/FlatJuniperGrammarTest.java line 1639 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

fine as-is, but also you don't need to check for warnings with todo. Those can be assumed to just work, and code coverage will make sure the line gets hit.

makes sense, will keep in mind.

[dhalperi]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @pranavbj-amzn)

---

projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java line 5847 at r2 (raw file):

Previously, pranavbj-amzn (Pranav Bhardwaj) wrote…

yes , done!

not pushed yet tho.

[dhalperi]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @pranavbj-amzn)

[dhalperi]

projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java line 230 at r3 (raw file):

import org.batfish.grammar.flatjuniper.FlatJuniperParser.Address_specifierContext;
import org.batfish.grammar.flatjuniper.FlatJuniperParser.Address_specifier_nameContext;
import org.batfish.grammar.flatjuniper.FlatJuniperParser.Iso_advertise_high_metricsContext;

looks like IDE didn't resort imports correctly.

Code quote:

erParser.Iso_ad

[pranavbj-amzn]

Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @dhalperi)

---

projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java line 230 at r3 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

looks like IDE didn't resort imports correctly.

thanks,

is there a command to run formatting check locally ?

[dhalperi]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @pranavbj-amzn)

---

projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java line 230 at r3 (raw file):

Previously, pranavbj-amzn (Pranav Bhardwaj) wrote…

thanks,

is there a command to run formatting check locally ?

./tools/fix_java_format.sh Also, the pre-commit hooks you set up if you followed the dev instructions. Also, hopefully, the IDE does it on AutoSave using the google-java-format plugin.