Merge pull request #1861 from flavio-fernandes/err_wrap.ds.4.13

[release-4.13] OCPBUGS-18672: Check libovsdbclient.ErrNotFound on wrapped errors
openshift · Sep 27, 2023 · d0925e2 · d0925e2
2 parents f2f537e + c76ae2c
commit d0925e2
Show file tree

Hide file tree

Showing 10 changed files with 33 additions and 26 deletions.
diff --git a/go-controller/pkg/libovsdbops/model_client.go b/go-controller/pkg/libovsdbops/model_client.go
@@ -292,8 +292,8 @@ func (m *modelClient) buildOps(ops []ovsdb.Operation, doWhenFound opModelToOpMap
 	for _, opModel := range opModels {
 		// do lookup
 		err := m.lookup(&opModel)
-		if err != nil && err != client.ErrNotFound {
-			return nil, nil, fmt.Errorf("unable to lookup model %+v: %v", opModel, err)
+		if err != nil && !errors.Is(err, client.ErrNotFound) {
+			return nil, nil, fmt.Errorf("unable to lookup model %+v: %w", opModel, err)
 		}
 
 		// do updates
@@ -357,7 +357,7 @@ func (m *modelClient) create(opModel *operationModel) ([]ovsdb.Operation, error)
 
 	ops, err := m.client.Create(opModel.Model)
 	if err != nil {
-		return nil, fmt.Errorf("unable to create model, err: %v", err)
+		return nil, fmt.Errorf("unable to create model, err: %w", err)
 	}
 
 	klog.V(5).Infof("Create operations generated as: %+v", ops)
@@ -367,7 +367,7 @@ func (m *modelClient) create(opModel *operationModel) ([]ovsdb.Operation, error)
 func (m *modelClient) update(lookUpModel interface{}, opModel *operationModel) (o []ovsdb.Operation, err error) {
 	o, err = m.client.Where(lookUpModel).Update(opModel.Model, opModel.OnModelUpdates...)
 	if err != nil {
-		return nil, fmt.Errorf("unable to update model, err: %v", err)
+		return nil, fmt.Errorf("unable to update model, err: %w", err)
 	}
 	klog.V(5).Infof("Update operations generated as: %+v", o)
 	return o, nil
@@ -383,7 +383,7 @@ func (m *modelClient) mutate(lookUpModel interface{}, opModel *operationModel, m
 	}
 	o, err = m.client.Where(lookUpModel).Mutate(opModel.Model, modelMutations...)
 	if err != nil {
-		return nil, fmt.Errorf("unable to mutate model, err: %v", err)
+		return nil, fmt.Errorf("unable to mutate model, err: %w", err)
 	}
 	klog.V(5).Infof("Mutate operations generated as: %+v", o)
 	return o, nil
@@ -392,7 +392,7 @@ func (m *modelClient) mutate(lookUpModel interface{}, opModel *operationModel, m
 func (m *modelClient) delete(lookUpModel interface{}, opModel *operationModel) (o []ovsdb.Operation, err error) {
 	o, err = m.client.Where(lookUpModel).Delete()
 	if err != nil {
-		return nil, fmt.Errorf("unable to delete model, err: %v", err)
+		return nil, fmt.Errorf("unable to delete model, err: %w", err)
 	}
 	klog.V(5).Infof("Delete operations generated as: %+v", o)
 	return o, nil
@@ -444,7 +444,7 @@ func (m *modelClient) lookup(opModel *operationModel) error {
 	// the only operation that can be performed without a lookup (it can have no db indexes and no ModelPredicate set)
 	// is Create.
 	if lookupRequired(opModel) {
-		return fmt.Errorf("missing model indixes or predicate when a lookup was required")
+		return fmt.Errorf("missing model indexes or predicate when a lookup was required")
 	}
 	return nil
 }

diff --git a/go-controller/pkg/libovsdbops/router.go b/go-controller/pkg/libovsdbops/router.go
@@ -2,6 +2,7 @@ package libovsdbops
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 
@@ -463,7 +464,7 @@ func DeleteNextHopsFromLogicalRouterPolicies(nbClient libovsdbclient.Client, rou
 	for _, lrp := range lrps {
 		nextHops := lrp.Nexthops
 		lrp, err := GetLogicalRouterPolicy(nbClient, lrp)
-		if err == libovsdbclient.ErrNotFound {
+		if errors.Is(err, libovsdbclient.ErrNotFound) {
 			continue
 		}
 		if err != nil {
@@ -982,7 +983,7 @@ func GetRouterNATs(nbClient libovsdbclient.Client, router *nbdb.LogicalRouter) (
 	nats := []*nbdb.NAT{}
 	for _, uuid := range r.Nat {
 		nat, err := GetNAT(nbClient, &nbdb.NAT{UUID: uuid})
-		if err == libovsdbclient.ErrNotFound {
+		if errors.Is(err, libovsdbclient.ErrNotFound) {
 			continue
 		}
 		if err != nil {
@@ -1052,7 +1053,7 @@ func CreateOrUpdateNATs(nbClient libovsdbclient.Client, router *nbdb.LogicalRout
 // logical router and returns the corresponding ops
 func DeleteNATsOps(nbClient libovsdbclient.Client, ops []libovsdb.Operation, router *nbdb.LogicalRouter, nats ...*nbdb.NAT) ([]libovsdb.Operation, error) {
 	routerNats, err := GetRouterNATs(nbClient, router)
-	if err == libovsdbclient.ErrNotFound {
+	if errors.Is(err, libovsdbclient.ErrNotFound) {
 		return ops, nil
 	}
 	if err != nil {

diff --git a/go-controller/pkg/libovsdbops/router_test.go b/go-controller/pkg/libovsdbops/router_test.go
@@ -147,7 +147,8 @@ func TestDeleteNATsFromRouter(t *testing.T) {
 	}{
 		{
 			desc:         "no router",
-			expectErr:    true,
+			routerName:   "doesNotExistRouter",
+			expectErr:    false,
 			nats:         []*nbdb.NAT{fakeNAT1.DeepCopy(), fakeNAT2.DeepCopy(), fakeNAT3.DeepCopy(), fakeNAT4.DeepCopy()},
 			expectedNbdb: initialNbdb,
 		},
@@ -206,6 +207,8 @@ func TestDeleteNATsFromRouter(t *testing.T) {
 			err = DeleteNATs(nbClient, &logicalRouter, tt.nats...)
 			if err != nil && !tt.expectErr {
 				t.Fatal(fmt.Errorf("DeleteNATsFromRouter() error = %v", err))
+			} else if err == nil && tt.expectErr {
+				t.Fatal(fmt.Errorf("DeleteNATsFromRouter() no error, but an error was expected"))
 			}
 
 			matcher := libovsdbtest.HaveData(tt.expectedNbdb.NBData)

diff --git a/go-controller/pkg/metrics/master.go b/go-controller/pkg/metrics/master.go
@@ -1,6 +1,7 @@
 package metrics
 
 import (
+	"errors"
 	"fmt"
 	"hash/fnv"
 	"math"
@@ -1390,7 +1391,7 @@ func getGlobalOptionsValue(client libovsdbclient.Client, field string) float64 {
 	sbGlobal := sbdb.SBGlobal{}
 
 	if dbName == "OVN_Northbound" {
-		if nbGlobal, err := libovsdbops.GetNBGlobal(client, &nbGlobal); err != nil && err != libovsdbclient.ErrNotFound {
+		if nbGlobal, err := libovsdbops.GetNBGlobal(client, &nbGlobal); err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) {
 			klog.Errorf("Failed to get NB_Global table err: %v", err)
 			return 0
 		} else {
@@ -1399,7 +1400,7 @@ func getGlobalOptionsValue(client libovsdbclient.Client, field string) float64 {
 	}
 
 	if dbName == "OVN_Southbound" {
-		if sbGlobal, err := libovsdbops.GetSBGlobal(client, &sbGlobal); err != nil && err != libovsdbclient.ErrNotFound {
+		if sbGlobal, err := libovsdbops.GetSBGlobal(client, &sbGlobal); err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) {
 			klog.Errorf("Failed to get SB_Global table err: %v", err)
 			return 0
 		} else {

diff --git a/go-controller/pkg/ovn/base_network_controller.go b/go-controller/pkg/ovn/base_network_controller.go
@@ -22,6 +22,7 @@ import (
 	ovnretry "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/retry"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
+	"github.com/pkg/errors"
 
 	kapi "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -507,10 +508,10 @@ func (bnc *BaseNetworkController) determineOVNTopoVersionFromOVN() error {
 func (bnc *BaseNetworkController) getOVNTopoVersionFromLogicalRouter(clusterRouterName string) (int, error) {
 	logicalRouter := &nbdb.LogicalRouter{Name: clusterRouterName}
 	logicalRouter, err := libovsdbops.GetLogicalRouter(bnc.nbClient, logicalRouter)
-	if err != nil && err != libovsdbclient.ErrNotFound {
+	if err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) {
 		return 0, fmt.Errorf("error getting router %s: %v", clusterRouterName, err)
 	}
-	if err == libovsdbclient.ErrNotFound {
+	if errors.Is(err, libovsdbclient.ErrNotFound) {
 		// no OVNClusterRouter exists, DB is empty, nothing to upgrade
 		return math.MaxInt32, nil
 	}
@@ -529,10 +530,10 @@ func (bnc *BaseNetworkController) getOVNTopoVersionFromLogicalRouter(clusterRout
 func (bnc *BaseNetworkController) getOVNTopoVersionFromLogicalSwitch(switchName string) (int, error) {
 	logicalSwitch := &nbdb.LogicalSwitch{Name: switchName}
 	logicalSwitch, err := libovsdbops.GetLogicalSwitch(bnc.nbClient, logicalSwitch)
-	if err != nil && err != libovsdbclient.ErrNotFound {
+	if err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) {
 		return 0, fmt.Errorf("error getting switch %s: %v", switchName, err)
 	}
-	if err == libovsdbclient.ErrNotFound {
+	if errors.Is(err, libovsdbclient.ErrNotFound) {
 		// no switch exists, DB is empty, nothing to upgrade
 		return math.MaxInt32, nil
 	}

diff --git a/go-controller/pkg/ovn/base_network_controller_pods.go b/go-controller/pkg/ovn/base_network_controller_pods.go
@@ -186,7 +186,7 @@ func (bnc *BaseNetworkController) deletePodLogicalPort(pod *kapi.Pod, portInfo *
 		// Since portInfo is not available, use ovn to locate the logical switch (named after the node name) for the logical port.
 		portUUID, switchName, err = bnc.lookupPortUUIDAndSwitchName(logicalPort)
 		if err != nil {
-			if err != libovsdbclient.ErrNotFound {
+			if !errors.Is(err, libovsdbclient.ErrNotFound) {
 				return nil, fmt.Errorf("unable to locate portUUID+switchName for %s: %w", podDesc, err)
 			}
 			// The logical port no longer exists in OVN. The caller expects this function to be idem-potent,
@@ -537,10 +537,10 @@ func (bnc *BaseNetworkController) addLogicalPortToNetwork(pod *kapi.Pod, nadName
 	// will still have the old UUID.
 	lsp = &nbdb.LogicalSwitchPort{Name: portName}
 	existingLSP, err := libovsdbops.GetLogicalSwitchPort(bnc.nbClient, lsp)
-	if err != nil && err != libovsdbclient.ErrNotFound {
+	if err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) {
 		return nil, nil, nil, false, fmt.Errorf("unable to get the lsp %s from the nbdb: %s", portName, err)
 	}
-	lspExist = err != libovsdbclient.ErrNotFound
+	lspExist = !errors.Is(err, libovsdbclient.ErrNotFound)
 
 	// Sanity check. If port exists, it should be in the logical switch obtained from the pod spec.
 	if lspExist {

diff --git a/go-controller/pkg/ovn/egressgw.go b/go-controller/pkg/ovn/egressgw.go
@@ -599,7 +599,7 @@ func deletePodSNATOps(nbClient libovsdbclient.Client, ops []ovsdb.Operation, nod
 		Name: types.GWRouterPrefix + nodeName,
 	}
 	ops, err = libovsdbops.DeleteNATsOps(nbClient, ops, &logicalRouter, nats...)
-	if err != nil {
+	if err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) {
 		return nil, fmt.Errorf("failed create operation for deleting SNAT rule for pod on gateway router %s: %v", logicalRouter.Name, err)
 	}
 	return ops, nil

diff --git a/go-controller/pkg/ovn/gateway_init.go b/go-controller/pkg/ovn/gateway_init.go
@@ -154,7 +154,7 @@ func (oc *DefaultNetworkController) gatewayInit(nodeName string, clusterIPSubnet
 	// so let's save the old value before we update the router for later use
 	var oldExtIPs []net.IP
 	oldLogicalRouter, err := libovsdbops.GetLogicalRouter(oc.nbClient, &logicalRouter)
-	if err != nil && err != libovsdbclient.ErrNotFound {
+	if err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) {
 		return fmt.Errorf("failed in retrieving %s, error: %v", gatewayRouter, err)
 	}
 

diff --git a/go-controller/pkg/ovn/master.go b/go-controller/pkg/ovn/master.go
@@ -124,7 +124,7 @@ func (oc *DefaultNetworkController) SetupMaster(existingNodeNames []string) erro
 		Name: types.ClusterPortGroupName,
 	}
 	pg, err = libovsdbops.GetPortGroup(oc.nbClient, pg)
-	if err != nil && err != libovsdbclient.ErrNotFound {
+	if err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) {
 		return err
 	}
 	if pg == nil {
@@ -142,7 +142,7 @@ func (oc *DefaultNetworkController) SetupMaster(existingNodeNames []string) erro
 		Name: types.ClusterRtrPortGroupName,
 	}
 	pg, err = libovsdbops.GetPortGroup(oc.nbClient, pg)
-	if err != nil && err != libovsdbclient.ErrNotFound {
+	if err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) {
 		return err
 	}
 	if pg == nil {

diff --git a/go-controller/pkg/util/util.go b/go-controller/pkg/util/util.go
@@ -1,6 +1,7 @@
 package util
 
 import (
+	"errors"
 	"fmt"
 	"hash/fnv"
 	"net"
@@ -304,7 +305,7 @@ func UpdateNodeSwitchExcludeIPs(nbClient libovsdbclient.Client, nodeName string,
 	haveManagementPort := true
 	managmentPort := &nbdb.LogicalSwitchPort{Name: types.K8sPrefix + nodeName}
 	_, err := libovsdbops.GetLogicalSwitchPort(nbClient, managmentPort)
-	if err == libovsdbclient.ErrNotFound {
+	if errors.Is(err, libovsdbclient.ErrNotFound) {
 		klog.V(5).Infof("Management port does not exist for node %s", nodeName)
 		haveManagementPort = false
 	} else if err != nil {
@@ -314,7 +315,7 @@ func UpdateNodeSwitchExcludeIPs(nbClient libovsdbclient.Client, nodeName string,
 	haveHybridOverlayPort := true
 	HOPort := &nbdb.LogicalSwitchPort{Name: types.HybridOverlayPrefix + nodeName}
 	_, err = libovsdbops.GetLogicalSwitchPort(nbClient, HOPort)
-	if err == libovsdbclient.ErrNotFound {
+	if errors.Is(err, libovsdbclient.ErrNotFound) {
 		klog.V(5).Infof("Hybridoverlay port does not exist for node %s", nodeName)
 		haveHybridOverlayPort = false
 	} else if err != nil {