Skip to content

Commit

Permalink
Arista: improve route-map parsing on older EOS (#6384)
Browse files Browse the repository at this point in the history
As reported by @dannypetrov, EOS 4.20.7 does not include action/line number
in output (in at least some cases). The new tests are based on behavior during
that discussion.
  • Loading branch information
dhalperi committed Nov 3, 2020
1 parent db213a6 commit 27c6266
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ rm_stanza

route_map_stanza
:
ROUTE_MAP name = variable rmt = access_list_action num = DEC NEWLINE
// Both action and number are optional but number must come with action
ROUTE_MAP name = variable (rmt = access_list_action (num = DEC)?)? NEWLINE
rm_stanza*
;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3717,8 +3717,8 @@ public void enterRoute_map_stanza(Route_map_stanzaContext ctx) {
String name = ctx.name.getText();
RouteMap routeMap = _configuration.getRouteMaps().computeIfAbsent(name, RouteMap::new);
_currentRouteMap = routeMap;
int num = toInteger(ctx.num);
LineAction action = toLineAction(ctx.rmt);
int num = ctx.num != null ? toInteger(ctx.num) : 10;
LineAction action = ctx.rmt != null ? toLineAction(ctx.rmt) : LineAction.PERMIT;
RouteMapClause clause = _currentRouteMap.getClauses().get(num);
if (clause == null) {
clause = new RouteMapClause(action, name, num);
Expand All @@ -3730,6 +3730,8 @@ public void enterRoute_map_stanza(Route_map_stanzaContext ctx) {
"Route map '%s' already contains clause numbered '%d'. Duplicate clause will be"
+ " merged with original clause.",
_currentRouteMap.getName(), num));
// Yes, action can change if the line is reconfigured.
clause.setAction(action);
}
_currentRouteMapClause = clause;
_configuration.defineStructure(ROUTE_MAP, name, ctx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nonnull;
import org.batfish.datamodel.LineAction;

public class RouteMapClause implements Serializable {

private LineAction _action;
private @Nonnull LineAction _action;

private RouteMapContinue _continueLine;

Expand All @@ -21,7 +22,7 @@ public class RouteMapClause implements Serializable {

private List<RouteMapSetLine> _setList;

public RouteMapClause(LineAction action, String name, int num) {
public RouteMapClause(@Nonnull LineAction action, String name, int num) {
_action = action;
_mapName = name;
_seqNum = num;
Expand All @@ -37,7 +38,7 @@ public void addSetLine(RouteMapSetLine line) {
_setList.add(line);
}

public LineAction getAction() {
public @Nonnull LineAction getAction() {
return _action;
}

Expand Down Expand Up @@ -65,6 +66,10 @@ public List<RouteMapSetLine> getSetList() {
return _setList;
}

public void setAction(@Nonnull LineAction action) {
_action = action;
}

public void setContinueLine(RouteMapContinue continueLine) {
_continueLine = continueLine;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
import org.batfish.datamodel.Interface.DependencyType;
import org.batfish.datamodel.Ip;
import org.batfish.datamodel.IpWildcard;
import org.batfish.datamodel.LineAction;
import org.batfish.datamodel.LongSpace;
import org.batfish.datamodel.MultipathEquivalentAsPathMatchMode;
import org.batfish.datamodel.OriginType;
Expand Down Expand Up @@ -159,6 +160,8 @@
import org.batfish.main.TestrigText;
import org.batfish.representation.arista.AristaConfiguration;
import org.batfish.representation.arista.MlagConfiguration;
import org.batfish.representation.arista.RouteMap;
import org.batfish.representation.arista.RouteMapClause;
import org.batfish.representation.arista.VrrpInterface;
import org.batfish.representation.arista.eos.AristaBgpAggregateNetwork;
import org.batfish.representation.arista.eos.AristaBgpBestpathTieBreaker;
Expand Down Expand Up @@ -2362,6 +2365,30 @@ public void testIpv6RouteParsing() {
parseConfig("ipv6_route");
}

@Test
public void testRouteMapExtraction() {
AristaConfiguration c = parseVendorConfig("route_map");
assertThat(c.getRouteMaps(), hasKeys("map1", "DANAIL_PETROV_20201103", "ACTION_CHANGES"));
{
RouteMap rm = c.getRouteMaps().get("DANAIL_PETROV_20201103");
assertThat(rm.getClauses(), hasKeys(10));
RouteMapClause clause = rm.getClauses().get(10);
assertThat(clause.getAction(), equalTo(LineAction.PERMIT));
assertThat(clause.getMatchList(), hasSize(1));
assertThat(clause.getSetList(), hasSize(1));
}
{
RouteMap rm = c.getRouteMaps().get("ACTION_CHANGES");
assertThat(rm.getClauses(), hasKeys(10));
RouteMapClause clause = rm.getClauses().get(10);
// Action changed from permit to deny
assertThat(clause.getAction(), equalTo(LineAction.DENY));
// Clauses were merged
assertThat(clause.getMatchList(), hasSize(1));
assertThat(clause.getSetList(), hasSize(1));
}
}

@Test
public void testRouteMapParsing() {
// Don't crash
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,15 @@ hostname route_map
!
route-map map1 permit 1
set local-preference 4294967295
!
! The below was reported on Batfish Slack. It's for an older version of EOS: 4.20.7
route-map DANAIL_PETROV_20201103
match ip address prefix-list PL_DANAIL_PETROV_20201103
set community 12345:54321
!
route-map ACTION_CHANGES permit 10
match ip address prefix-list SOME_PL
!
route-map ACTION_CHANGES deny
set community 12345:54321
!

0 comments on commit 27c6266

Please sign in to comment.