Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(evpn-bridge): fix for frr to handle vrf setup and teardown #401

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(evpn-bridge): fix for frr to handle vrf setup and teardown #401

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 23 additions & 22 deletions pkg/frr/frr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,6 @@ func handlevrf(objectData *eventbus.ObjectData) {
return
}

if len(vrf.Status.Components) != 0 {
for i := 0; i < len(vrf.Status.Components); i++ {
if vrf.Status.Components[i].Name == frrComp {
comp = vrf.Status.Components[i]
}
}
}
if objectData.ResourceVersion != vrf.ResourceVersion {
log.Printf("FRR: Mismatch in resoruce version %+v\n and vrf resource version %+v\n", objectData.ResourceVersion, vrf.ResourceVersion)
comp.Name = frrComp
Expand Down Expand Up @@ -366,9 +359,9 @@ func setUpVrf(vrf *infradb.Vrf) (string, bool) {
// Configure the vrf in FRR and set up BGP EVPN for it
vrfName := fmt.Sprintf("vrf %s", path.Base(vrf.Name))
vniID := fmt.Sprintf("vni %s", strconv.Itoa(int(*vrf.Spec.Vni)))
_, err := Frr.FrrZebraCmd(ctx, fmt.Sprintf("configure terminal\n %s\n %s\n exit-vrf\n exit", vrfName, vniID))
// fmt.Printf("FrrZebraCmd: %v:%v", data, err)
if err != nil {
data, err := Frr.FrrZebraCmd(ctx, fmt.Sprintf("configure terminal\n %s\n %s\n exit-vrf\n exit", vrfName, vniID))
if err != nil || checkFrrResult(data, false) {
log.Printf("FRR: Error Executing frr config t %s %s exit-vrf exit data %v err is %v data is %v\n", vrfName, vniID, data, err, data)
return "", false
}
log.Printf("FRR: Executed frr config t %s %s exit-vrf exit\n", vrfName, vniID)
Expand All @@ -379,24 +372,24 @@ func setUpVrf(vrf *infradb.Vrf) (string, bool) {
} else {
LbiP = fmt.Sprintf("%+v", vrf.Spec.LoopbackIP.IP)
}
_, err = Frr.FrrBgpCmd(ctx, fmt.Sprintf("configure terminal\n router bgp %+v vrf %s\n bgp router-id %s\n no bgp ebgp-requires-policy\n no bgp hard-administrative-reset\n no bgp graceful-restart notification\n address-family ipv4 unicast\n redistribute connected\n redistribute static\n exit-address-family\n address-family l2vpn evpn\n advertise ipv4 unicast\n exit-address-family\n exit", localas, path.Base(vrf.Name), LbiP))
if err != nil {
data, err = Frr.FrrBgpCmd(ctx, fmt.Sprintf("configure terminal\n router bgp %+v vrf %s\n bgp router-id %s\n no bgp ebgp-requires-policy\n no bgp hard-administrative-reset\n no bgp graceful-restart notification\n address-family ipv4 unicast\n redistribute connected\n redistribute static\n exit-address-family\n address-family l2vpn evpn\n advertise ipv4 unicast\n exit-address-family\n exit", localas, path.Base(vrf.Name), LbiP))
if err != nil || checkFrrResult(data, false) {
log.Printf("FRR: Error Executing config t bgpVrfName router bgp %+v vrf %s bgp_route_id %s no bgp ebgp-requires-policy exit-vrf exit data %v \n", localas, vrf.Name, LbiP, data)
return "", false
}

log.Printf("FRR: Executed config t bgpVrfName router bgp %+v vrf %s bgp_route_id %s no bgp ebgp-requires-policy exit-vrf exit\n", localas, vrf.Name, LbiP)
// Update the vrf with attributes from FRR
cmd := fmt.Sprintf("show bgp l2vpn evpn vni %d json", *vrf.Spec.Vni)
cp, err := Frr.FrrBgpCmd(ctx, cmd)
if err != nil {
log.Printf("error-%v", err)
if err != nil || checkFrrResult(cp, true) {
log.Printf("FRR Error-show bgp l2vpn evpn vni %v cp %v", err, cp)
}
hname, _ := os.Hostname()
L2vpnCmd := strings.Split(cp, "json")
L2vpnCmd = strings.Split(L2vpnCmd[1], hname)
cp = L2vpnCmd[0]
// fmt.Printf("FRR_L2vpn[0]: %s\n",cp)
if len(cp) != 7 {
if len(cp) != 7 { // Checking CMD o/p
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

magic number. Rely on string length is very unreliable

cp = cp[3 : len(cp)-3]
} else {
log.Printf("FRR: unable to get the command %s\n", cmd)
Expand All @@ -409,7 +402,7 @@ func setUpVrf(vrf *infradb.Vrf) (string, bool) {
}
cmd = fmt.Sprintf("show bgp vrf %s json", path.Base(vrf.Name))
cp, err = Frr.FrrBgpCmd(ctx, cmd)
if err != nil {
if err != nil || checkFrrResult(cp, true) {
log.Printf("error-%v", err)
}
BgpCmd := strings.Split(cp, "json")
Expand Down Expand Up @@ -437,7 +430,7 @@ func setUpVrf(vrf *infradb.Vrf) (string, bool) {

// checkFrrResult checks the vrf result
func checkFrrResult(cp string, show bool) bool {
return ((show && reflect.ValueOf(cp).IsZero()) || strings.Contains(cp, "warning") || strings.Contains(cp, "unknown") || strings.Contains(cp, "Unknown") || strings.Contains(cp, "Warning") || strings.Contains(cp, "Ambiguous") || strings.Contains(cp, "specified does not exist"))
return ((show && reflect.ValueOf(cp).IsZero()) || strings.Contains(cp, "warning") || strings.Contains(cp, "unknown") || strings.Contains(cp, "Unknown") || strings.Contains(cp, "Warning") || strings.Contains(cp, "Ambiguous") || strings.Contains(cp, "specified does not exist") || strings.Contains(cp, "Error"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reflect.ValueOf(cp).IsZero()) -> cp == ""

what if cp contains waRning? Will you add a new comparison then? Will it be better to lower case the string and only then check what itcontains?

}

// setUpSvi sets up the svi
Expand Down Expand Up @@ -484,6 +477,9 @@ func tearDownSvi(svi *infradb.Svi) bool {
bgpVrfName := fmt.Sprintf("router bgp %+v vrf %s", localas, path.Base(svi.Spec.Vrf))
noNeigh := fmt.Sprintf("no neighbor %s peer-group", linkSvi)
data, err := Frr.FrrBgpCmd(ctx, fmt.Sprintf("configure terminal\n %s\n %s\n exit", bgpVrfName, noNeigh))
if strings.Contains(data, "Create the peer-group first") { // Trying to delete non exist peer-group return true
return true
}
if err != nil || checkFrrResult(data, false) {
log.Printf("FRR: Error in conf Delete vrf/VNI command %s\n", data)
return false
Expand Down Expand Up @@ -514,12 +510,17 @@ func tearDownVrf(vrf *infradb.Vrf) bool {
log.Printf("FRR Deleted event")
delCmd1 := fmt.Sprintf("no router bgp %+v vrf %s", localas, path.Base(vrf.Name))
delCmd2 := fmt.Sprintf("no vrf %s", path.Base(vrf.Name))
_, err = Frr.FrrBgpCmd(ctx, fmt.Sprintf("configure terminal\n %s\n exit\n", delCmd1))
if err != nil {
data, err = Frr.FrrBgpCmd(ctx, fmt.Sprintf("configure terminal\n %s\n exit\n", delCmd1))
if strings.Contains(data, "Can't find BGP instance") { // Trying to delete non exist VRF return true
return true
}
if err != nil || checkFrrResult(data, false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we need to check frr for each frr cmd, why not move that check into FrrBgpCmd/FrrZebraCmd and other frr functions?

as well as strings.Contains(?

log.Printf("FRR: Error %s\n", data)
return false
}
_, err = Frr.FrrZebraCmd(ctx, fmt.Sprintf("configure terminal\n %s\n exit\n", delCmd2))
if err != nil {
data, err = Frr.FrrZebraCmd(ctx, fmt.Sprintf("configure terminal\n %s\n exit\n", delCmd2))
if err != nil || checkFrrResult(data, false) {
log.Printf("FRR: Error %s\n", data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does data contain error description?

return false
}
log.Printf("FRR: Executed vtysh -c conf t -c %s -c %s -c exit\n", delCmd1, delCmd2)
Expand Down
Loading