From a8da21fc4f4c78f47325f3a964771a31c8de3b3b Mon Sep 17 00:00:00 2001 From: atulpatel261194 Date: Tue, 1 Oct 2024 10:02:57 -0700 Subject: [PATCH] fix(evpn): report error in comp details review fixes Signed-off-by: atulpatel261194 --- pkg/frr/frr.go | 50 ++++++++++++++++++++---------------------------- pkg/utils/frr.go | 44 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 59 insertions(+), 35 deletions(-) diff --git a/pkg/frr/frr.go b/pkg/frr/frr.go index d80fa269..8875152c 100644 --- a/pkg/frr/frr.go +++ b/pkg/frr/frr.go @@ -368,8 +368,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))) - 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) { + + data, err := Frr.FrrZebraCmd(ctx, fmt.Sprintf("configure terminal\n %s\n %s\n exit-vrf\n exit", vrfName, vniID), false) + if err != nil { log.Printf("FRR: Error Executing frr config t %s %s exit-vrf exit \n Error: is %v data is %v\n", vrfName, vniID, err.Error(), data) return fmt.Sprintf("FRR: Error Executing frr config t %s %s exit-vrf exit data %v \n Error: %v Data: %v\n", vrfName, vniID, data, err, data), false } @@ -381,16 +382,16 @@ func setUpVrf(vrf *infradb.Vrf) (string, bool) { } else { LbiP = fmt.Sprintf("%+v", vrf.Spec.LoopbackIP.IP) } - 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) { + 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), false) + if err != nil { 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 fmt.Sprintf("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), 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 || checkFrrResult(cp, true) { + cp, err := Frr.FrrBgpCmd(ctx, cmd, true) + if err != nil { log.Printf("FRR Error-show bgp l2vpn evpn vni %v cp %v", err, cp) } hname, _ := os.Hostname() @@ -409,8 +410,8 @@ func setUpVrf(vrf *infradb.Vrf) (string, bool) { log.Printf("error-%v", err) } cmd = fmt.Sprintf("show bgp vrf %s json", path.Base(vrf.Name)) - cp, err = Frr.FrrBgpCmd(ctx, cmd) - if err != nil || checkFrrResult(cp, true) { + cp, err = Frr.FrrBgpCmd(ctx, cmd, true) + if err != nil { log.Printf("error-%v", err) } BgpCmd := strings.Split(cp, "json") @@ -436,11 +437,6 @@ func setUpVrf(vrf *infradb.Vrf) (string, bool) { return "", true } -// 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") || strings.Contains(cp, "Error")) -} - // setUpSvi sets up the svi func setUpSvi(svi *infradb.Svi) (string, bool) { BrObj, err := infradb.GetLB(svi.Spec.LogicalBridge) @@ -461,9 +457,9 @@ func setUpSvi(svi *infradb.Svi) (string, bool) { neighlinkSr := fmt.Sprintf("neighbor %s soft-reconfiguration inbound\n", linkSvi) bgpListen := fmt.Sprintf(" bgp listen range %s peer-group %s\n", svi.Spec.GatewayIPs[0], linkSvi) - data, err := Frr.FrrBgpCmd(ctx, fmt.Sprintf("configure terminal\n %s bgp disable-ebgp-connected-route-check\n %s %s %s %s %s %s exit", bgpVrfName, neighlink, neighlinkRe, neighlinkGw, neighlinkOv, neighlinkSr, bgpListen)) + data, err := Frr.FrrBgpCmd(ctx, fmt.Sprintf("configure terminal\n %s bgp disable-ebgp-connected-route-check\n %s %s %s %s %s %s exit", bgpVrfName, neighlink, neighlinkRe, neighlinkGw, neighlinkOv, neighlinkSr, bgpListen), false) - if err != nil || checkFrrResult(data, false) { + if err != nil { log.Printf("FRR: Error in conf svi %s %s command %s\n", svi.Name, path.Base(svi.Spec.Vrf), data) return fmt.Sprintf("FRR: Error in conf svi %s %s command %s\n", svi.Name, path.Base(svi.Spec.Vrf), data), false } @@ -484,11 +480,10 @@ func tearDownSvi(svi *infradb.Svi) (string, bool) { if svi.Spec.EnableBgp && !reflect.ValueOf(svi.Spec.GatewayIPs).IsZero() { 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) { + + data, err := Frr.FrrBgpCmd(ctx, fmt.Sprintf("configure terminal\n %s\n %s\n exit", bgpVrfName, noNeigh), false) + + if err != nil { log.Printf("FRR: Error in conf Delete vrf/VNI command %s\n", data) return fmt.Sprintf("FRR: Error in conf Delete vrf/VNI command %s\n", data), false } @@ -505,8 +500,8 @@ func tearDownVrf(vrf *infradb.Vrf) (string, bool) { return "", true } - data, err := Frr.FrrZebraCmd(ctx, fmt.Sprintf("show vrf %s vni\n", path.Base(vrf.Name))) - if err != nil || checkFrrResult(data, true) { + data, err := Frr.FrrZebraCmd(ctx, fmt.Sprintf("show vrf %s vni\n", path.Base(vrf.Name)), true) + if err != nil { log.Printf("FRR: Error %s\n", data) return "", true } @@ -515,16 +510,13 @@ func tearDownVrf(vrf *infradb.Vrf) (string, 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)) - 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) { + data, err = Frr.FrrBgpCmd(ctx, fmt.Sprintf("configure terminal\n %s\n exit\n", delCmd1), false) + if err != nil { log.Printf("FRR: Error %s\n", data) return fmt.Sprintf("FRR: Error %s\n", data), false } - data, err = Frr.FrrZebraCmd(ctx, fmt.Sprintf("configure terminal\n %s\n exit\n", delCmd2)) - if err != nil || checkFrrResult(data, false) { + data, err = Frr.FrrZebraCmd(ctx, fmt.Sprintf("configure terminal\n %s\n exit\n", delCmd2), false) + if err != nil { log.Printf("FRR: Error %s\n", data) return fmt.Sprintf("FRR: Error %s\n", data), false } diff --git a/pkg/utils/frr.go b/pkg/utils/frr.go index 0bf49027..eb69d6b2 100644 --- a/pkg/utils/frr.go +++ b/pkg/utils/frr.go @@ -7,6 +7,7 @@ package utils import ( "bufio" "context" + "errors" "fmt" "strings" "time" @@ -50,8 +51,8 @@ const ( // Frr represents limited subset of functions from Frr package type Frr interface { TelnetDialAndCommunicate(ctx context.Context, command string, port int) (string, error) - FrrZebraCmd(ctx context.Context, command string) (string, error) - FrrBgpCmd(ctx context.Context, command string) (string, error) + FrrZebraCmd(ctx context.Context, command string, cmdTypeShow bool) (string, error) + FrrBgpCmd(ctx context.Context, command string, cmdTypeShow bool) (string, error) Password(conn *telnet.Conn, delim string) error EnterPrivileged(conn *telnet.Conn) error ExitPrivileged(conn *telnet.Conn) error @@ -113,16 +114,47 @@ func (n *FrrWrapper) ExitPrivileged(conn *telnet.Conn) error { return conn.SkipUntil(">") } +// checkFrrResult checks the vrf result +func checkFrrResult(data string, show bool) bool { + if show && data == "" { + return true + } + // // Trying to delete non exist VRF || Trying to delete non exist peer-group + if strings.Contains(data, "Can't find BGP instance") || strings.Contains(data, "Create the peer-group first") { + return false + } + patterns := []string{"error", "warning", "unknown", "ambiguous", "specified does not exist"} + lowerStr := strings.ToLower(data) + for _, pattern := range patterns { + if strings.Contains(lowerStr, pattern) { + return true + } + } + return false +} + // FrrZebraCmd connects to Zebra telnet with password and runs command -func (n *FrrWrapper) FrrZebraCmd(ctx context.Context, command string) (string, error) { +func (n *FrrWrapper) FrrZebraCmd(ctx context.Context, command string, cmdTypeShow bool) (string, error) { // ports defined here https://docs.frrouting.org/en/latest/setup.html#services - return n.TelnetDialAndCommunicate(ctx, command, zebra) + cmdOutput, cmdError := n.TelnetDialAndCommunicate(ctx, command, zebra) + if cmdError != nil { + return cmdOutput, cmdError + } else if checkFrrResult(cmdOutput, cmdTypeShow) { + return cmdOutput, errors.New("error while running FrrZebraCmd") + } + return cmdOutput, cmdError } // FrrBgpCmd connects to Bgp telnet with password and runs command -func (n *FrrWrapper) FrrBgpCmd(ctx context.Context, command string) (string, error) { +func (n *FrrWrapper) FrrBgpCmd(ctx context.Context, command string, cmdTypeShow bool) (string, error) { // ports defined here https://docs.frrouting.org/en/latest/setup.html#services - return n.TelnetDialAndCommunicate(ctx, command, bgpd) + cmdOutput, cmdError := n.TelnetDialAndCommunicate(ctx, command, bgpd) + if cmdError != nil { + return cmdOutput, cmdError + } else if checkFrrResult(cmdOutput, cmdTypeShow) { + return cmdOutput, errors.New("error while running FrrBgpCmd") + } + return cmdOutput, cmdError } // MultiLineCmd breaks command by lines, sends each and waits for output and returns combined output