Address review comments

Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

client/dynamic_config.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/go-errors/errors"
 	"github.com/goccy/go-yaml"
 	"go.ligato.io/cn-infra/v2/logging/logrus"
 	"google.golang.org/protobuf/encoding/protojson"
@@ -93,19 +92,19 @@ func NewDynamicConfig(knownModels []*models.ModelInfo) (*dynamicpb.Message, erro
 	// create dependency registry
 	dependencyRegistry, err := createFileDescRegistry(knownModels)
 	if err != nil {
-		return nil, errors.Errorf("can't create dependency file descriptor registry due to: %v", err)
+		return nil, fmt.Errorf("cannot create dependency file descriptor registry due to: %w", err)
 	}
 
 	// get file descriptor proto for give known models
 	fileDP, rootMsgName, err := createDynamicConfigDescriptorProto(knownModels, dependencyRegistry)
 	if err != nil {
-		return nil, errors.Errorf("can't create descriptor proto for dynamic config due to: %v", err)
+		return nil, fmt.Errorf("cannot create descriptor proto for dynamic config due to: %v", err)
 	}
 
 	// convert file descriptor proto to file descriptor
 	fd, err := protodesc.NewFile(fileDP, dependencyRegistry)
 	if err != nil {
-		return nil, errors.Errorf("can't convert file descriptor proto to file descriptor due to: %v", err)
+		return nil, fmt.Errorf("cannot convert file descriptor proto to file descriptor due to: %v", err)
 	}
 
 	// get descriptor for config root message
@@ -204,7 +203,7 @@ func createDynamicConfigDescriptorProto(knownModels []*ModelInfo, dependencyRegi
 		// add proto file dependency for this known model (+ check that it is in dependency file descriptor registry)
 		protoFile, err := ModelOptionFor("protoFile", modelDetail.Options)
 		if err != nil {
-			error = errors.Errorf("can't retrieve protoFile from model options "+
+			error = fmt.Errorf("cannot retrieve protoFile from model options "+
 				"from model %v due to: %v", modelDetail.ProtoName, err)
 			return
 		}
@@ -217,12 +216,12 @@ func createDynamicConfigDescriptorProto(knownModels []*ModelInfo, dependencyRegi
 			// checking dependency registry that should already contain the linked dependency
 			if _, err := dependencyRegistry.FindFileByPath(protoFile); err != nil {
 				if err == protoregistry.NotFound {
-					error = errors.Errorf("proto file %v need to be referenced in dynamic config, but it "+
+					error = fmt.Errorf("proto file %v need to be referenced in dynamic config, but it "+
 						"is not in dependency registry that was created from file descriptor proto input "+
 						"(missing in input? check debug output from creating dependency registry) ", protoFile)
 					return
 				}
-				error = errors.Errorf("can't verify that proto file %v is in "+
+				error = fmt.Errorf("cannot verify that proto file %v is in "+
 					"dependency registry, it is due to: %v", protoFile, err)
 				return
 			}
@@ -266,7 +265,7 @@ func DynamicConfigKnownModelFieldNaming(modelDetail *models.ModelInfo) (protoNam
 // dynamic config (i.e. after json/yaml loading into dynamic config).
 func DynamicConfigExport(dynamicConfig *dynamicpb.Message) ([]proto.Message, error) {
 	if dynamicConfig == nil {
-		return nil, errors.Errorf("dynamic config can't be nil")
+		return nil, fmt.Errorf("dynamic config cannot be nil")
 	}
 
 	// iterate over config group messages and extract proto message from them
@@ -301,17 +300,17 @@ func ExportDynamicConfigStructure(dynamicConfig proto.Message) (string, error) {
 	}
 	b, err := m.Marshal(dynamicConfig)
 	if err != nil {
-		return "", errors.Errorf("can't marshal dynamic config to json due to: %v", err)
+		return "", fmt.Errorf("cannot marshal dynamic config to json due to: %v", err)
 	}
 	var jsonObj interface{}
 	err = yaml.UnmarshalWithOptions(b, &jsonObj, yaml.UseOrderedMap())
 	if err != nil {
-		return "", errors.Errorf("can't convert dynamic config's json bytes to "+
+		return "", fmt.Errorf("cannot convert dynamic config's json bytes to "+
 			"json struct for yaml marshalling due to: %v", err)
 	}
 	bb, err := yaml.Marshal(jsonObj)
 	if err != nil {
-		return "", errors.Errorf("can't marshal dynamic config from json to yaml due to: %v", err)
+		return "", fmt.Errorf("cannot marshal dynamic config from json to yaml due to: %v", err)
 	}
 	return string(bb), nil
 }
@@ -349,16 +348,16 @@ func ModelOptionFor(key string, options []*generic.ModelDetail_Option) (string,
 	for _, option := range options {
 		if option.Key == key {
 			if len(option.Values) == 0 {
-				return "", errors.Errorf("there is no value for key %v in model options", key)
+				return "", fmt.Errorf("there is no value for key %v in model options", key)
 			}
 			if strings.TrimSpace(option.Values[0]) == "" {
-				return "", errors.Errorf("there is no value(only empty string "+
+				return "", fmt.Errorf("there is no value(only empty string "+
 					"after trimming) for key %v in model options", key)
 			}
 			return option.Values[0], nil
 		}
 	}
-	return "", errors.Errorf("there is no model option with key %v (model options=%+v))", key, options)
+	return "", fmt.Errorf("there is no model option with key %v (model options=%+v))", key, options)
 }
 
 func existsModelOptionFor(key string, options []*generic.ModelDetail_Option) bool {

client/local_client.go

@@ -17,6 +17,7 @@ package client
 import (
 	"context"
 
+	"github.com/sirupsen/logrus"
 	"go.ligato.io/cn-infra/v2/datasync/kvdbsync/local"
 	"go.ligato.io/cn-infra/v2/datasync/syncbase"
 	"go.ligato.io/cn-infra/v2/db/keyval"
@@ -84,7 +85,7 @@ func (c *client) GetConfig(dsts ...interface{}) error {
 		// TODO the clearIgnoreLayerCount function argument should be a option of generic.Client
 		//  (the value 1 generates from dynamic config the same json/yaml output as the hardcoded
 		//  configurator.Config and therefore serves for backward compatibility)
-		util.PlaceProtosIntoProtos(convertToProtoV2(protos), 1, protoDsts...)
+		util.PlaceProtosIntoProtos(protoMapToList(protos), 1, protoDsts...)
 	} else {
 		util.PlaceProtos(protos, dsts...)
 	}
@@ -163,24 +164,20 @@ func (p *txnFactory) NewTxn(resync bool) keyval.ProtoTxn {
 }
 
 func extractProtoMessages(dsts []interface{}) []proto.Message {
-	protoDsts := make([]proto.Message, 0)
+	msgs := make([]proto.Message, 0)
 	for _, dst := range dsts {
-		protoV1Dst, isProtoV1 := dst.(proto.Message)
-		if isProtoV1 {
-			protoDsts = append(protoDsts, protoV1Dst)
+		msg, ok := dst.(proto.Message)
+		if ok {
+			msgs = append(msgs, msg)
 		} else {
-			protoV2Dst, isProtoV2 := dst.(proto.Message)
-			if isProtoV2 {
-				protoDsts = append(protoDsts, protoV2Dst)
-			} else {
-				break
-			}
+			logrus.Debugf("at least one of the %d items is not proto message, but: %#v", len(dsts), dst)
+			break
 		}
 	}
-	return protoDsts
+	return msgs
 }
 
-func convertToProtoV2(protoMap map[string]proto.Message) []proto.Message {
+func protoMapToList(protoMap map[string]proto.Message) []proto.Message {
 	result := make([]proto.Message, 0, len(protoMap))
 	for _, msg := range protoMap {
 		result = append(result, msg)

client/remoteclient/grpc_client.go

@@ -2,9 +2,9 @@ package remoteclient
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
-	"github.com/go-errors/errors"
 	"google.golang.org/grpc"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protodesc"
@@ -37,7 +37,7 @@ func NewClientGRPC(conn grpc.ClientConnInterface, options ...NewClientOption) (c
 	}
 	for _, option := range options {
 		if err := option(c); err != nil {
-			return nil, errors.Errorf("can't apply option to newly created GRPC client due to: %v", err)
+			return nil, fmt.Errorf("cannot apply option to newly created GRPC client due to: %w", err)
 		}
 	}
 	return c, nil
@@ -60,29 +60,29 @@ func (c *grpcClient) KnownModels(class string) ([]*client.ModelInfo, error) {
 	for _, modelDetail := range knownModels {
 		protoFilePath, err := client.ModelOptionFor("protoFile", modelDetail.Options)
 		if err != nil {
-			return nil, errors.Errorf("can't get protoFile from model options of "+
-				"known model %v due to: %v", modelDetail.ProtoName, err)
+			return nil, fmt.Errorf("cannot get protoFile from model options of "+
+				"known model %v due to: %w", modelDetail.ProtoName, err)
 		}
 		protoFilePaths[protoFilePath] = struct{}{}
 	}
 
 	// query meta service for extracted proto files to get their file descriptor protos
 	fileDescProtos := make(map[string]*descriptorpb.FileDescriptorProto) // deduplicaton + data container
-	for protoFilePath, _ := range protoFilePaths {
+	for protoFilePath := range protoFilePaths {
 		ctx := context.Background()
 		resp, err := c.meta.ProtoFileDescriptor(ctx, &generic.ProtoFileDescriptorRequest{
 			FullProtoFileName: protoFilePath,
 		})
 		if err != nil {
-			return nil, errors.Errorf("can't retrieve ProtoFileDescriptor "+
-				"for proto file %v due to: %v", protoFilePath, err)
+			return nil, fmt.Errorf("cannot retrieve ProtoFileDescriptor "+
+				"for proto file %v due to: %w", protoFilePath, err)
 		}
 		if resp.FileDescriptor == nil {
-			return nil, errors.Errorf("returned file descriptor proto "+
+			return nil, fmt.Errorf("returned file descriptor proto "+
 				"for proto file %v from meta service can't be nil", protoFilePath)
 		}
 		if resp.FileImportDescriptors == nil {
-			return nil, errors.Errorf("returned import file descriptors proto "+
+			return nil, fmt.Errorf("returned import file descriptors proto "+
 				"for proto file %v from meta service can't be nil", protoFilePath)
 		}
 
@@ -101,8 +101,8 @@ func (c *grpcClient) KnownModels(class string) ([]*client.ModelInfo, error) {
 	}
 	fileDescriptors, err := toFileDescriptors(fileDescProtosSlice)
 	if err != nil {
-		return nil, errors.Errorf("can't convert file descriptor protos to file descriptors "+
-			"(for dependency registry creation) due to: %v", err)
+		return nil, fmt.Errorf("cannot convert file descriptor protos to file descriptors "+
+			"(for dependency registry creation) due to: %w", err)
 	}
 
 	// extract all messages from file descriptors
@@ -183,7 +183,7 @@ func (c *grpcClient) GetConfig(dsts ...interface{}) error {
 		// TODO the clearIgnoreLayerCount function argument should be a option of generic.Client
 		//  (the value 1 generates from dynamic config the same json/yaml output as the hardcoded
 		//  configurator.Config and therefore serves for backward compatibility)
-		util.PlaceProtosIntoProtos(convertToProtoV2(protos), 1, protoDsts...)
+		util.PlaceProtosIntoProtos(protoMapToList(protos), 1, protoDsts...)
 	} else {
 		util.PlaceProtos(protos, dsts...)
 	}
@@ -260,14 +260,14 @@ func UseRemoteRegistry(modelClass string) NewClientOption {
 			// get all remote models
 			knownModels, err := grpcClient.KnownModels(modelClass)
 			if err != nil {
-				return errors.Errorf("can't retrieve remote models (in UseRemoteRegistry) due to: %w", err)
+				return fmt.Errorf("cannot retrieve remote models (in UseRemoteRegistry) due to: %w", err)
 			}
 
 			// fill them into new remote registry and use that registry instead of default local model registry
 			grpcClient.modelRegistry = models.NewRemoteRegistry()
 			for _, knowModel := range knownModels {
 				if _, err := grpcClient.modelRegistry.Register(knowModel, models.ToSpec(knowModel.Spec)); err != nil {
-					return errors.Errorf("can't register remote known model "+
+					return fmt.Errorf("cannot register remote known model "+
 						"for remote generic client usage due to: %w", err)
 				}
 			}
@@ -306,14 +306,14 @@ func toFileDescriptors(fileDescProtos []*descriptorpb.FileDescriptorProto) ([]pr
 					break
 				}
 				if err := reg.RegisterFile(resolvedDep); err != nil {
-					return nil, errors.Errorf("can't put resolved dependency %v "+
+					return nil, fmt.Errorf("cannot put resolved dependency %v "+
 						"into descriptor registry due to: %v", resolvedDep.Name(), err)
 				}
 			}
 			if allDepsFound {
 				fd, err := protodesc.NewFile(fdp, reg)
 				if err != nil {
-					return nil, errors.Errorf("can't create file descriptor "+
+					return nil, fmt.Errorf("cannot create file descriptor "+
 						"(from file descriptor proto named %v) due to: %v", *fdp.Name, err)
 				}
 				resolved[fdpName] = fd
@@ -323,7 +323,7 @@ func toFileDescriptors(fileDescProtos []*descriptorpb.FileDescriptorProto) ([]pr
 		}
 	}
 	if len(unresolvedFDProtos) > 0 {
-		return nil, errors.Errorf("can't resolve some FileDescriptorProtos due to missing of "+
+		return nil, fmt.Errorf("cannot resolve some FileDescriptorProtos due to missing of "+
 			"some protos of their imports (FileDescriptorProtos with unresolvable imports: %v)",
 			fileDescriptorProtoMapToString(unresolvedFDProtos))
 	}
@@ -337,7 +337,7 @@ func toFileDescriptors(fileDescProtos []*descriptorpb.FileDescriptorProto) ([]pr
 
 func fileDescriptorProtoMapToString(fdps map[string]*descriptorpb.FileDescriptorProto) string {
 	keys := make([]string, 0, len(fdps))
-	for key, _ := range fdps {
+	for key := range fdps {
 		keys = append(keys, key)
 	}
 	return strings.Join(keys, ",")
@@ -346,25 +346,20 @@ func fileDescriptorProtoMapToString(fdps map[string]*descriptorpb.FileDescriptor
 func extractProtoMessages(dsts []interface{}) []proto.Message {
 	protoDsts := make([]proto.Message, 0)
 	for _, dst := range dsts {
-		protoV1Dst, isProtoV1 := dst.(proto.Message)
-		if isProtoV1 {
-			protoDsts = append(protoDsts, proto.Message(protoV1Dst))
+		msg, ok := dst.(proto.Message)
+		if ok {
+			protoDsts = append(protoDsts, msg)
 		} else {
-			protoV2Dst, isProtoV2 := dst.(proto.Message)
-			if isProtoV2 {
-				protoDsts = append(protoDsts, protoV2Dst)
-			} else {
-				break
-			}
+			break
 		}
 	}
 	return protoDsts
 }
 
-func convertToProtoV2(protoMap map[string]proto.Message) []proto.Message {
+func protoMapToList(protoMap map[string]proto.Message) []proto.Message {
 	result := make([]proto.Message, 0, len(protoMap))
 	for _, msg := range protoMap {
-		result = append(result, proto.Message(msg))
+		result = append(result, msg)
 	}
 	return result
 }

client/txn.go

@@ -55,7 +55,7 @@ func (t *Txn) FindItem(id string) (model proto.Message, found bool) {
 	return item, ok
 }
 
-// Items returns map of items defined for the request,
+// ListItems returns map of items defined for the request,
 // where key represents model ID and nil value represents delete.
 // NOTE: Do not alter the returned map directly.
 func (t *Txn) ListItems() map[string]proto.Message {

cmd/agentctl/cli/cli.go

@@ -21,7 +21,6 @@ import (
 
 	"github.com/docker/cli/cli/streams"
 	"github.com/docker/docker/pkg/term"
-
 	"go.ligato.io/cn-infra/v2/logging"
 
 	"go.ligato.io/vpp-agent/v3/cmd/agentctl/api"

cmd/agentctl/cli/flags.go

@@ -21,7 +21,6 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
-
 	"go.ligato.io/cn-infra/v2/logging"
 
 	"go.ligato.io/vpp-agent/v3/cmd/agentctl/client"

cmd/agentctl/cli/viper.go

@@ -20,7 +20,6 @@ import (
 
 	"github.com/mitchellh/mapstructure"
 	"github.com/spf13/viper"
-
 	"go.ligato.io/cn-infra/v2/logging"
 )
 

cmd/agentctl/client/api.go

@@ -7,13 +7,14 @@ import (
 	govppapi "git.fd.io/govpp.git/api"
 	"go.ligato.io/cn-infra/v2/db/keyval"
 	"go.ligato.io/cn-infra/v2/health/probe"
+	"google.golang.org/grpc"
+
 	"go.ligato.io/vpp-agent/v3/client"
 	"go.ligato.io/vpp-agent/v3/cmd/agentctl/api/types"
 	"go.ligato.io/vpp-agent/v3/plugins/kvscheduler/api"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/configurator"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/generic"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/kvscheduler"
-	"google.golang.org/grpc"
 )
 
 // APIClient is an interface that clients that talk with a agent server must implement.

cmd/agentctl/client/errors.go

@@ -1,9 +1,8 @@
 package client
 
 import (
+	"errors"
 	"fmt"
-
-	"github.com/pkg/errors"
 )
 
 // errConnectionFailed implements an error returned when connection failed.
@@ -21,8 +20,8 @@ func (err errConnectionFailed) Error() string {
 
 // IsErrConnectionFailed returns true if the error is caused by connection failed.
 func IsErrConnectionFailed(err error) bool {
-	_, ok := errors.Cause(err).(errConnectionFailed)
-	return ok
+	var connErr *errConnectionFailed
+	return errors.As(err, &connErr)
 }
 
 // ErrorConnectionFailed returns an error with host in the error message when connection to agent failed.