Unnumbered BGP in session questions

[corinaminer]

• Adds remote interface column to answers
• Local and remote interface columns are only populated for rows representing BGP unnumbered peers; local and remote IP columns are only populated for rows representing standard BGP peers (active or dynamic)
• New session types EBGP_UNNUMBERED and IBGP_UNNUMBERED to help tell BGP unnumbered rows apart from misconfigured standard peers

[batfish-bot]

This change is 

[codecov]

Codecov Report

Merging #3797 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #3797   +/-   ##
=========================================
  Coverage     73.99%   73.99%           
  Complexity    24181    24181           
=========================================
  Files          2038     2038           
  Lines         98284    98284           
  Branches      11803    11803           
=========================================
  Hits          72723    72723           
  Misses        20273    20273           
  Partials       5288     5288

[progwriter]

Reviewed 11 of 12 files at r1, 2 of 2 files at r2.
Reviewable status: 13 of 14 files reviewed, 2 unresolved discussions (waiting on @corinaminer and @progwriter)

---

projects/question/src/main/java/org/batfish/question/bgpsessionstatus/BgpSessionCompatibilityAnswerer.java, line 90 at r2 (raw file):

      Set<String> remoteNodes,
      Map<String, ColumnMetadata> metadataMap,
      @Nullable Layer2Topology layer2Topology) {

Be mindful of #3795, there is no reason for this not to use calls to topologyProvider for pre/post DP topologies (applies to session status as well, obviously)

---

projects/question/src/main/java/org/batfish/question/bgpsessionstatus/BgpSessionStatusAnswerer.java, line 107 at r2 (raw file):

  }

  public static List<Row> getRows(

why is this public and what is the benefit of the function wrapping above?

[corinaminer]

Reviewable status: 13 of 14 files reviewed, 2 unresolved discussions (waiting on @progwriter)

---

projects/question/src/main/java/org/batfish/question/bgpsessionstatus/BgpSessionCompatibilityAnswerer.java, line 90 at r2 (raw file):

Previously, progwriter (Victor Heorhiadi) wrote…

Be mindful of #3795, there is no reason for this not to use calls to topologyProvider for pre/post DP topologies (applies to session status as well, obviously)

The layer2Topology being passed in here is retrieved by the topologyProvider in the non-static getRows() method a few lines up. This static getRows() method is separate in order to make testing easier; same for the new static getRows() method in session status (which doesn't actually take a layer2Topology parameter, instead takes the BGP topologies directly to avoid needing a traceroute engine parameter).

Marked both answerers' static getRows() methods @VisibleForTesting and reduced their visibilities to package-private.

---

projects/question/src/main/java/org/batfish/question/bgpsessionstatus/BgpSessionStatusAnswerer.java, line 107 at r2 (raw file):

Previously, progwriter (Victor Heorhiadi) wrote…

why is this public and what is the benefit of the function wrapping above?

addressed in other comment -- separation is useful for testing.

[progwriter]

---

projects/question/src/main/java/org/batfish/question/bgpsessionstatus/BgpSessionCompatibilityAnswerer.java, line 90 at r2 (raw file):
I was referring to the calls to

BgpTopologyUtils.initBgpTopology

seemingly all over the place. If you don't init the bgp topology, you don't need the l2 topology.

[corinaminer]

Reviewable status: 9 of 14 files reviewed, 2 unresolved discussions (waiting on @progwriter)

---

projects/question/src/main/java/org/batfish/question/bgpsessionstatus/BgpSessionCompatibilityAnswerer.java, line 90 at r2 (raw file):

Previously, progwriter (Victor Heorhiadi) wrote…

I was referring to the calls to

BgpTopologyUtils.initBgpTopology

seemingly all over the place. If you don't init the bgp topology, you don't need the l2 topology.

The topology provider doesn't have the version of the topology that's needed here: i.e., including invalid peers and ignoring reachability. Replaced one of the initBgpTopology calls in BgpSessionStatusAnswerer with the topology provider's version.

[progwriter]

Reviewed 1 of 12 files at r1, 2 of 2 files at r3, 6 of 6 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved