VOL-2001 resolve sca errors
Change-Id: Iee4c814e721437c6f2f0d3387cac92be498ceb99
diff --git a/Makefile b/Makefile
index a51bc52..5e492bf 100644
--- a/Makefile
+++ b/Makefile
@@ -148,10 +148,15 @@
GOLANGCI_LINT_TOOL:=$(shell which golangci-lint)
sca:
+ifeq (,$(GOLANGCI_LINT_TOOL))
@echo "Please install golangci-lint tool to run sca"
+ exit 1
+endif
+ rm -rf ./sca-report
@mkdir -p ./sca-report
- golangci-lint run --out-format junit-xml ./... 2>&1 | tee ./sca-report/sca-report.xml ;\
- rm -rf sca-report
+ $(GOLANGCI_LINT_TOOL) run --out-format junit-xml ./... 2>&1 | tee ./sca-report/sca-report.xml ;\
+ RETURN=$$? ;\
+ exit $$RETURN
test:
ifeq (,$(GO_JUNIT_REPORT))
@@ -172,5 +177,6 @@
clean:
distclean: clean
+ rm -rf ./sca_report
# end file
diff --git a/cmd/arouter/arouter.go b/cmd/arouter/arouter.go
index d3838ac..cfd0282 100644
--- a/cmd/arouter/arouter.go
+++ b/cmd/arouter/arouter.go
@@ -23,7 +23,7 @@
"github.com/opencord/voltha-go/common/version"
_ "github.com/opencord/voltha-protos"
"google.golang.org/grpc/grpclog"
- slog "log"
+ "io/ioutil"
"os"
)
@@ -40,7 +40,14 @@
log.With(log.Fields{"error": err}).Fatal("Cannot setup logging")
}
- defer log.CleanUp()
+ defer func() {
+ err := log.CleanUp()
+ if err != nil {
+ // Let's not use the logger to print the error message, since the
+ // logger could be in a bad state.
+ fmt.Fprintf(os.Stderr, "Failed to cleanup logger: %v", err)
+ }
+ }()
if *conf.DisplayVersionOnly {
fmt.Println("VOLTHA API Server (afrouter)")
@@ -58,12 +65,15 @@
// Enable grpc logging
if *conf.GrpcLog {
- grpclog.SetLogger(slog.New(os.Stderr, "grpc: ", slog.LstdFlags))
- //grpclog.SetLoggerV2(lgr)
+ grpclog.SetLoggerV2(grpclog.NewLoggerV2(os.Stderr, ioutil.Discard, ioutil.Discard))
}
// Install the signal and error handlers.
- afrouter.InitExitHandler()
+ err = afrouter.InitExitHandler()
+ if err != nil {
+ log.Errorf("Failed to initialize exit handler, exiting: %v", err)
+ return
+ }
// Create the affinity router proxy...
if ap, err := afrouter.NewArouterProxy(conf); err != nil {
diff --git a/cmd/arouterd/arouterd.go b/cmd/arouterd/arouterd.go
index 1c9cd80..de469b5 100644
--- a/cmd/arouterd/arouterd.go
+++ b/cmd/arouterd/arouterd.go
@@ -48,12 +48,11 @@
)
type volthaPod struct {
- name string
- ipAddr string
- node string
- devIds map[string]struct{}
- backend string
- connection string
+ name string
+ ipAddr string
+ node string
+ devIds map[string]struct{}
+ backend string
}
type Configuration struct {
diff --git a/internal/pkg/afrouter/affinity-router.go b/internal/pkg/afrouter/affinity-router.go
index 7a94f7b..90c0e7f 100644
--- a/internal/pkg/afrouter/affinity-router.go
+++ b/internal/pkg/afrouter/affinity-router.go
@@ -17,7 +17,6 @@
package afrouter
import (
- "errors"
"fmt"
"github.com/golang/protobuf/proto"
"github.com/opencord/voltha-go/common/log"
@@ -35,7 +34,6 @@
type AffinityRouter struct {
name string
association associationType
- routingField string
grpcService string
methodMap map[string]byte
nbBindingMethodMap map[string]byte
@@ -45,7 +43,7 @@
}
func newAffinityRouter(rconf *RouterConfig, config *RouteConfig) (Router, error) {
- var err error = nil
+ var err error
var rtrn_err = false
var pkg_re = regexp.MustCompile(`^(\.[^.]+\.)(.+)$`)
// Validate the configuration
@@ -76,7 +74,6 @@
// routing_field. This needs to be added so that methods
// can have different routing fields.
var bptr *backend
- bptr = nil
dr := AffinityRouter{
name: config.Name,
grpcService: rconf.ProtoService,
@@ -157,7 +154,7 @@
}
// Create the backend cluster or link to an existing one
- ok := true
+ var ok bool
if dr.cluster, ok = clusters[config.backendCluster.Name]; !ok {
if dr.cluster, err = newBackendCluster(config.backendCluster); err != nil {
log.Errorf("Could not create a backend for router %s", config.Name)
@@ -166,7 +163,7 @@
}
if rtrn_err {
- return dr, errors.New(fmt.Sprintf("Failed to create a new router '%s'", dr.name))
+ return dr, fmt.Errorf("Failed to create a new router '%s'", dr.name)
}
return dr, nil
@@ -259,7 +256,7 @@
return "", e
}
default:
- err := errors.New(fmt.Sprintf("Only integer and string route selectors are permitted"))
+ err := fmt.Errorf("Only integer and string route selectors are permitted")
log.Error(err)
return "", err
}
@@ -298,9 +295,12 @@
log.Debugf("MUST CREATE A NEW AFFINITY MAP ENTRY!!")
var err error
if *ar.currentBackend, err = ar.cluster.nextBackend(*ar.currentBackend, BackendSequenceRoundRobin); err == nil {
- ar.setAffinity(selector, *ar.currentBackend)
- //ar.affinity[selector] = *ar.currentBackend
- //log.Debugf("New affinity set to backend %s",(*ar.currentBackend).name)
+ err := ar.setAffinity(selector, *ar.currentBackend)
+ if err != nil {
+ log.Errorf("Failed to set affinity during Route: %v", err)
+ // TODO: Should we return nil here? We do have a backend, so we can return it, but we did fail
+ // to set affinity...
+ }
return *ar.currentBackend, nil
} else {
sl.err = err
@@ -350,14 +350,14 @@
}
return nil
} else {
- err := errors.New(fmt.Sprintf("Failed to decode reply field %d for method %s", fld, sl.method))
+ err := fmt.Errorf("Failed to decode reply field %d for method %s", fld, sl.method)
log.Error(err)
return err
}
}
return nil
default:
- err := errors.New(fmt.Sprintf("Internal: invalid data type in ReplyHander call %v", sl))
+ err := fmt.Errorf("Internal: invalid data type in ReplyHander call %v", sl)
log.Error(err)
return err
}
@@ -368,8 +368,8 @@
ar.affinity[key] = be
log.Debugf("New affinity set to backend %s for key %s", be.name, key)
} else if be2 != be {
- err := errors.New(fmt.Sprintf("Attempting multiple sets of affinity for key %s to backend %s from %s on router %s",
- key, be.name, ar.affinity[key].name, ar.name))
+ err := fmt.Errorf("Attempting multiple sets of affinity for key %s to backend %s from %s on router %s",
+ key, be.name, ar.affinity[key].name, ar.name)
log.Error(err)
return err
}
diff --git a/internal/pkg/afrouter/affinity-router_test.go b/internal/pkg/afrouter/affinity-router_test.go
index e78b948..922c08a 100644
--- a/internal/pkg/afrouter/affinity-router_test.go
+++ b/internal/pkg/afrouter/affinity-router_test.go
@@ -19,7 +19,6 @@
import (
"fmt"
"github.com/golang/protobuf/proto"
- "github.com/opencord/voltha-go/common/log"
common_pb "github.com/opencord/voltha-protos/go/common"
voltha_pb "github.com/opencord/voltha-protos/go/voltha"
"github.com/stretchr/testify/assert"
@@ -27,17 +26,6 @@
"testing"
)
-const (
- AFFINITY_ROUTER_PROTOFILE = "../../../vendor/github.com/opencord/voltha-protos/voltha.pb"
-)
-
-// Unit test initialization
-func init() {
- // Logger must be configured or bad things happen
- log.SetDefaultLogger(log.JSON, log.DebugLevel, log.Fields{"instanceId": 1})
- log.AddPackage(log.JSON, log.WarnLevel, nil)
-}
-
// Build an affinity router configuration
func MakeAffinityTestConfig(numBackends int, numConnections int) (*RouteConfig, *RouterConfig) {
@@ -83,7 +71,7 @@
ProtoService: "VolthaService",
ProtoPackage: "voltha",
Routes: []RouteConfig{routeConfig},
- ProtoFile: AFFINITY_ROUTER_PROTOFILE,
+ ProtoFile: TEST_PROTOFILE,
}
return &routeConfig, &routerConfig
}
@@ -384,20 +372,24 @@
*/
s, err := aRouter.decodeProtoField(portData, 1) // field 1 is PortNo
+ assert.Nil(t, err)
assert.Equal(t, "123", s)
// Test VOL-1882, skipping of varint field. Note: May cause infinite loop if not fixed!
s, err = aRouter.decodeProtoField(portData, 2) // field 2 is Label
+ assert.Nil(t, err)
assert.Equal(t, "testlabel", s)
s, err = aRouter.decodeProtoField(portData, 3) // field 3 is PortType
+ assert.Nil(t, err)
assert.Equal(t, "3", s)
s, err = aRouter.decodeProtoField(portData, 7) // field 7 is DeviceId
+ assert.Nil(t, err)
assert.Equal(t, "5678", s)
// TODO: Seems like an int64 ought to be allowed...
- s, err = aRouter.decodeProtoField(portData, 9) // field 7 is RxPackets
+ _, err = aRouter.decodeProtoField(portData, 9) // field 7 is RxPackets
assert.EqualError(t, err, "Only integer and string route selectors are permitted")
}
diff --git a/internal/pkg/afrouter/api.go b/internal/pkg/afrouter/api.go
index 8b72897..1d2360b 100644
--- a/internal/pkg/afrouter/api.go
+++ b/internal/pkg/afrouter/api.go
@@ -50,7 +50,7 @@
if rtrn_err {
return nil, errors.New("Errors in API configuration")
} else {
- var err error = nil
+ var err error
aa := &ArouterApi{addr: config.Addr, port: int(config.Port), ar: ar}
// Create the listener for the API server
if aa.apiListener, err =
@@ -68,13 +68,14 @@
func (aa *ArouterApi) getServer(srvr string) (*server, error) {
if s, ok := aa.ar.servers[srvr]; !ok {
- err := errors.New(fmt.Sprintf("Server '%s' doesn't exist", srvr))
+ err := fmt.Errorf("Server '%s' doesn't exist", srvr)
return nil, err
} else {
return s, nil
}
}
+// nolint: unused
func (aa *ArouterApi) getRouter(s *server, clstr string) (Router, error) {
for _, pkg := range s.routers {
for _, r := range pkg {
@@ -83,7 +84,7 @@
}
}
}
- err := errors.New(fmt.Sprintf("Cluster '%s' doesn't exist", clstr))
+ err := fmt.Errorf("Cluster '%s' doesn't exist", clstr)
return nil, err
}
@@ -95,7 +96,7 @@
}
}
}
- err := errors.New(fmt.Sprintf("Cluster '%s' doesn't exist", clstr))
+ err := fmt.Errorf("Cluster '%s' doesn't exist", clstr)
return nil, err
}
@@ -105,14 +106,14 @@
return b, nil
}
}
- err := errors.New(fmt.Sprintf("Backend '%s' doesn't exist in cluster %s",
- bknd, c.name))
+ err := fmt.Errorf("Backend '%s' doesn't exist in cluster %s",
+ bknd, c.name)
return nil, err
}
func (aa *ArouterApi) getConnection(b *backend, con string) (*connection, error) {
if c, ok := b.connections[con]; !ok {
- err := errors.New(fmt.Sprintf("Connection '%s' doesn't exist", con))
+ err := fmt.Errorf("Connection '%s' doesn't exist", con)
return nil, err
} else {
return c, nil
@@ -142,7 +143,11 @@
log.Debug("Affinity router found")
b := rr.FindBackendCluster(in.Cluster).getBackend(in.Backend)
if b != nil {
- rr.setAffinity(in.Id, b)
+ err := rr.setAffinity(in.Id, b)
+ if err != nil {
+ log.Debugf("Couldn't set affinity: %s", err.Error())
+ return &pb.Result{Success: false, Error: err.Error()}, err
+ }
} else {
log.Errorf("Requested backend '%s' not found", in.Backend)
}
@@ -155,7 +160,8 @@
_ = rr
}
} else {
- log.Debugf("Couldn't get router type")
+ err = errors.New("Couldn't get router type")
+ log.Debugf("%v", err)
return &pb.Result{Success: false, Error: err.Error()}, err
}
@@ -176,7 +182,7 @@
aap := &aa
if s, err = (aap).getServer(in.Server); err != nil {
- err := errors.New(fmt.Sprintf("Server '%s' doesn't exist", in.Server))
+ err := fmt.Errorf("Server '%s' doesn't exist", in.Server)
log.Error(err)
return &pb.Result{Success: false, Error: err.Error()}, err
}
diff --git a/internal/pkg/afrouter/arproxy.go b/internal/pkg/afrouter/arproxy.go
index 72641de..4265584 100644
--- a/internal/pkg/afrouter/arproxy.go
+++ b/internal/pkg/afrouter/arproxy.go
@@ -25,7 +25,7 @@
)
// String names for display in error messages.
-var arProxy *ArouterProxy = nil
+var arProxy *ArouterProxy
type ArouterProxy struct {
servers map[string]*server // Defined in handler.go
diff --git a/internal/pkg/afrouter/backend.go b/internal/pkg/afrouter/backend.go
index c597d49..aa18b81 100644
--- a/internal/pkg/afrouter/backend.go
+++ b/internal/pkg/afrouter/backend.go
@@ -26,6 +26,7 @@
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"
+ "google.golang.org/grpc/status"
"net/url"
"strconv"
"strings"
@@ -38,7 +39,6 @@
name string
beType backendType
activeAssociation association
- connFailCallback func(string, *backend) bool
connections map[string]*connection
openConns map[*connection]*grpc.ClientConn
activeRequests map[*request]struct{}
@@ -152,7 +152,7 @@
log.Debug("Starting request stream forwarding")
if s2cErr := request.forwardRequestStream(serverStream); s2cErr != nil {
// exit with an error to the stack
- return grpc.Errorf(codes.Internal, "failed proxying s2c: %v", s2cErr)
+ return status.Errorf(codes.Internal, "failed proxying s2c: %v", s2cErr)
}
// wait for response stream to complete
return <-request.responseErrChan
@@ -287,9 +287,3 @@
go cn.connect()
}
}
-
-// Set a callback for connection failure notification
-// This is currently not used.
-func (be *backend) setConnFailCallback(cb func(string, *backend) bool) {
- be.connFailCallback = cb
-}
diff --git a/internal/pkg/afrouter/binding-router.go b/internal/pkg/afrouter/binding-router.go
index 2602137..e656398 100644
--- a/internal/pkg/afrouter/binding-router.go
+++ b/internal/pkg/afrouter/binding-router.go
@@ -64,7 +64,7 @@
}
// Determine if one of the method routing keys exists in the metadata
- if _, ok := md[br.bindingField]; ok == true {
+ if _, ok := md[br.bindingField]; ok {
rtrnV = md[br.bindingField][0]
rtrnK = br.bindingField
}
@@ -84,19 +84,19 @@
var err error
switch sl := sel.(type) {
case *requestFrame:
- if b, ok := br.bindings[sl.metaVal]; ok == true { // binding exists, just return it
+ if b, ok := br.bindings[sl.metaVal]; ok { // binding exists, just return it
return b, nil
} else { // establish a new binding or error.
if sl.metaVal != "" {
- err = errors.New(fmt.Sprintf("Attempt to route on non-existent metadata value '%s' in key '%s'",
- sl.metaVal, sl.metaKey))
+ err = fmt.Errorf("Attempt to route on non-existent metadata value '%s' in key '%s'",
+ sl.metaVal, sl.metaKey)
log.Error(err)
sl.err = err
return nil, nil
}
if sl.methodInfo.method != br.bindingMethod {
- err = errors.New(fmt.Sprintf("Binding must occur with method %s but attempted with method %s",
- br.bindingMethod, sl.methodInfo.method))
+ err = fmt.Errorf("Binding must occur with method %s but attempted with method %s",
+ br.bindingMethod, sl.methodInfo.method)
log.Error(err)
sl.err = err
return nil, nil
@@ -120,7 +120,7 @@
func newBindingRouter(rconf *RouterConfig, config *RouteConfig) (Router, error) {
var rtrn_err = false
- var err error = nil
+ var err error
log.Debugf("Creating binding router %s", config.Name)
// A name must exist
if config.Name == "" {
@@ -154,7 +154,6 @@
// so the router will route all methods based on the
// routing_field. This needs to be done.
var bptr *backend
- bptr = nil
br := BindingRouter{
name: config.Name,
grpcService: rconf.ProtoService,
@@ -193,8 +192,8 @@
}
// Create the backend cluster or link to an existing one
- ok := true
- if br.beCluster, ok = clusters[config.backendCluster.Name]; ok == false {
+ var ok bool
+ if br.beCluster, ok = clusters[config.backendCluster.Name]; !ok {
if br.beCluster, err = newBackendCluster(config.backendCluster); err != nil {
log.Errorf("Could not create a backend for router %s", config.Name)
rtrn_err = true
@@ -204,7 +203,7 @@
// HERE HERE HERE
if rtrn_err {
- return br, errors.New(fmt.Sprintf("Failed to create a new router '%s'", br.name))
+ return br, fmt.Errorf("Failed to create a new router '%s'", br.name)
}
return br, nil
diff --git a/internal/pkg/afrouter/cluster.go b/internal/pkg/afrouter/cluster.go
index 859d92f..9cf7ab2 100644
--- a/internal/pkg/afrouter/cluster.go
+++ b/internal/pkg/afrouter/cluster.go
@@ -38,7 +38,7 @@
// level. All backends should really be of the same type.
// Create a new backend cluster
func newBackendCluster(conf *BackendClusterConfig) (*cluster, error) {
- var err error = nil
+ var err error
var rtrn_err = false
var be *backend
log.Debugf("Creating a backend cluster with %v", conf)
diff --git a/internal/pkg/afrouter/codec.go b/internal/pkg/afrouter/codec.go
index 278fc0a..b11d102 100644
--- a/internal/pkg/afrouter/codec.go
+++ b/internal/pkg/afrouter/codec.go
@@ -17,22 +17,34 @@
package afrouter
import (
- "fmt"
"github.com/golang/protobuf/proto"
"github.com/opencord/voltha-go/common/log"
- "google.golang.org/grpc"
+ "google.golang.org/grpc/encoding"
)
-func Codec() grpc.Codec {
+/*
+ * encoding.Codec renamed grpc.Codec's String() method to Name()
+ *
+ * grpc.ForceCodec() expects an encoding.Codec, alternative use of grpc.Codec is deprecated
+ * grpc.CustomCodec() expects a grpc.Codec, has no mechanism to use encoding.Codec
+ *
+ * Make an interface that supports them both.
+ */
+type hybridCodec interface {
+ encoding.Codec
+ String() string
+}
+
+func Codec() hybridCodec {
return CodecWithParent(&protoCodec{})
}
-func CodecWithParent(parent grpc.Codec) grpc.Codec {
+func CodecWithParent(parent encoding.Codec) hybridCodec {
return &transparentRoutingCodec{parent}
}
type transparentRoutingCodec struct {
- parentCodec grpc.Codec
+ parentCodec encoding.Codec
}
// responseFrame is a frame being "returned" to whomever established the connection
@@ -74,7 +86,15 @@
case *responseFrame:
t.payload = data
// This is where the affinity is established on a northbound response
- t.router.ReplyHandler(v)
+
+ /*
+ * NOTE: Ignoring this error is intentional. MethodRouter returns
+ * error when a reply is not processed for northbound affinity,
+ * but we still need to unmarshal it.
+ *
+ * TODO: Investigate error-return semantics of ReplyHandler.
+ */
+ _ = t.router.ReplyHandler(v)
return nil
case *requestFrame:
t.payload = data
@@ -93,8 +113,12 @@
}
}
+func (cdc *transparentRoutingCodec) Name() string {
+ return cdc.parentCodec.Name()
+}
+
func (cdc *transparentRoutingCodec) String() string {
- return fmt.Sprintf("%s", cdc.parentCodec.String())
+ return cdc.Name()
}
// protoCodec is a Codec implementation with protobuf. It is the default Codec for gRPC.
@@ -108,6 +132,6 @@
return proto.Unmarshal(data, v.(proto.Message))
}
-func (protoCodec) String() string {
+func (protoCodec) Name() string {
return "protoCodec"
}
diff --git a/internal/pkg/afrouter/common_test.go b/internal/pkg/afrouter/common_test.go
new file mode 100644
index 0000000..322eeee
--- /dev/null
+++ b/internal/pkg/afrouter/common_test.go
@@ -0,0 +1,56 @@
+/*
+ * Portions copyright 2019-present Open Networking Foundation
+ * Original copyright 2019-present Ciena Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the"github.com/stretchr/testify/assert" "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package afrouter
+
+/*
+ * This file has common code that is imported for all test cases, but
+ * is not built into production binaries.
+ */
+
+import (
+ "github.com/opencord/voltha-go/common/log"
+)
+
+const (
+ TEST_PROTOFILE = "../../../vendor/github.com/opencord/voltha-protos/voltha.pb"
+
+ /*
+ * This sets the LogLevel of the Voltha logger. It's pinned to FatalLevel here, as we
+ * generally don't want to see logger output, even when running go test in verbose
+ * mode. Even "Error" level messages are expected to be output by some unit tests.
+ *
+ * If you are developing a unit test, and experiencing problems or wish additional
+ * debugging from Voltha, then changing this constant to log.DebugLevel may be
+ * useful.
+ */
+
+ VOLTHA_LOGLEVEL = log.FatalLevel
+)
+
+// Unit test initialization. This init() function will be run once for all unit tests in afrouter
+func init() {
+ // Logger must be configured or bad things happen
+ _, err := log.SetDefaultLogger(log.JSON, VOLTHA_LOGLEVEL, log.Fields{"instanceId": 1})
+ if err != nil {
+ panic(err)
+ }
+
+ _, err = log.AddPackage(log.JSON, VOLTHA_LOGLEVEL, nil)
+ if err != nil {
+ panic(err)
+ }
+}
diff --git a/internal/pkg/afrouter/config.go b/internal/pkg/afrouter/config.go
index 09e1816..5dccf32 100644
--- a/internal/pkg/afrouter/config.go
+++ b/internal/pkg/afrouter/config.go
@@ -227,14 +227,14 @@
log.Debugf("Reference to router '%s' found for package '%s'", rPkg.Router, rPkg.Package)
conf.Servers[k].routers[rPkg.Package] = &conf.Routers[rk]
} else {
- err := errors.New(fmt.Sprintf("Duplicate router '%s' defined for package '%s'", rPkg.Router, rPkg.Package))
+ err := fmt.Errorf("Duplicate router '%s' defined for package '%s'", rPkg.Router, rPkg.Package)
log.Error(err)
return err
}
}
}
if !found {
- err := errors.New(fmt.Sprintf("Router %s for server %s not found in config", conf.Servers[k].Name, rPkg.Router))
+ err := fmt.Errorf("Router %s for server %s not found in config", conf.Servers[k].Name, rPkg.Router)
log.Error(err)
return err
}
@@ -252,14 +252,14 @@
conf.Routers[rk].Routes[rtk].backendCluster = &conf.BackendClusters[bek]
found = true
} else if rtv.BackendCluster == bev.Name && found {
- err := errors.New(fmt.Sprintf("Duplicate backend defined, %s", conf.BackendClusters[bek].Name))
+ err := fmt.Errorf("Duplicate backend defined, %s", conf.BackendClusters[bek].Name)
log.Error(err)
return err
}
}
if !found {
- err := errors.New(fmt.Sprintf("Backend %s for router %s not found in config",
- rtv.BackendCluster, rv.Name))
+ err := fmt.Errorf("Backend %s for router %s not found in config",
+ rtv.BackendCluster, rv.Name)
log.Error(err)
return err
}
diff --git a/internal/pkg/afrouter/connection.go b/internal/pkg/afrouter/connection.go
index dcdb8d6..fc1c057 100644
--- a/internal/pkg/afrouter/connection.go
+++ b/internal/pkg/afrouter/connection.go
@@ -41,7 +41,7 @@
// Check back later to confirm and increase the connection count.
var err error
- conn, err := grpc.Dial(cn.addr+":"+cn.port, grpc.WithCodec(Codec()), grpc.WithInsecure(), grpc.WithBackoffMaxDelay(time.Second*15))
+ conn, err := grpc.Dial(cn.addr+":"+cn.port, grpc.WithDefaultCallOptions(grpc.ForceCodec(Codec())), grpc.WithInsecure(), grpc.WithBackoffMaxDelay(time.Second*15))
if err != nil {
log.Fatalf("Dialing connection %v:%v", cn, err)
}
diff --git a/internal/pkg/afrouter/method-router.go b/internal/pkg/afrouter/method-router.go
index 2916edf..41cd203 100644
--- a/internal/pkg/afrouter/method-router.go
+++ b/internal/pkg/afrouter/method-router.go
@@ -80,7 +80,7 @@
}
if len(config.Routes) == 0 {
- return nil, errors.New(fmt.Sprintf("Router %s must have at least one route", config.Name))
+ return nil, fmt.Errorf("Router %s must have at least one route", config.Name)
}
for _, rtv := range config.Routes {
//log.Debugf("Processing route: %v",rtv)
@@ -99,7 +99,7 @@
}
switch len(rtv.Methods) {
case 0:
- return nil, errors.New(fmt.Sprintf("Route for router %s must have at least one method", config.Name))
+ return nil, fmt.Errorf("Route for router %s must have at least one method", config.Name)
case 1:
if rtv.Methods[0] == "*" {
return r, nil
@@ -108,8 +108,8 @@
if _, ok := mr.methodRouter[idx1][rtv.Methods[0]]; !ok {
mr.methodRouter[idx1][rtv.Methods[0]] = r
} else {
- err := errors.New(fmt.Sprintf("Attempt to define method %s for 2 routes: %s & %s", rtv.Methods[0],
- r.Name(), mr.methodRouter[idx1][rtv.Methods[0]].Name()))
+ err := fmt.Errorf("Attempt to define method %s for 2 routes: %s & %s", rtv.Methods[0],
+ r.Name(), mr.methodRouter[idx1][rtv.Methods[0]].Name())
log.Error(err)
return mr, err
}
@@ -121,7 +121,7 @@
log.Debugf("Setting router '%s' for method '%s'", r.Name(), m)
mr.methodRouter[idx1][m] = r
} else {
- err := errors.New(fmt.Sprintf("Attempt to define method %s for 2 routes: %s & %s", m, r.Name(), mr.methodRouter[idx1][m].Name()))
+ err := fmt.Errorf("Attempt to define method %s for 2 routes: %s & %s", m, r.Name(), mr.methodRouter[idx1][m].Name())
log.Error(err)
return mr, err
}
@@ -198,7 +198,7 @@
if r, ok := mr.methodRouter[metaKey][mthd]; ok {
return r.BackendCluster(mthd, metaKey)
}
- err := errors.New(fmt.Sprintf("No backend cluster exists for method '%s' using meta key '%s'", mthd, metaKey))
+ err := fmt.Errorf("No backend cluster exists for method '%s' using meta key '%s'", mthd, metaKey)
log.Error(err)
return nil, err
}
diff --git a/internal/pkg/afrouter/method-router_test.go b/internal/pkg/afrouter/method-router_test.go
index 47204dd..fd0285a 100644
--- a/internal/pkg/afrouter/method-router_test.go
+++ b/internal/pkg/afrouter/method-router_test.go
@@ -19,7 +19,6 @@
import (
"fmt"
"github.com/golang/protobuf/proto"
- "github.com/opencord/voltha-go/common/log"
common_pb "github.com/opencord/voltha-protos/go/common"
voltha_pb "github.com/opencord/voltha-protos/go/voltha"
"github.com/stretchr/testify/assert"
@@ -27,17 +26,6 @@
"testing"
)
-const (
- METHOD_ROUTER_PROTOFILE = "../../../vendor/github.com/opencord/voltha-protos/voltha.pb"
-)
-
-// Unit test initialization
-func init() {
- // Logger must be configured or bad things happen
- log.SetDefaultLogger(log.JSON, log.DebugLevel, log.Fields{"instanceId": 1})
- log.AddPackage(log.JSON, log.WarnLevel, nil)
-}
-
// Build an method router configuration
func MakeMethodTestConfig(numBackends int, numConnections int) (*RouteConfig, *RouterConfig) {
@@ -83,7 +71,7 @@
ProtoService: "VolthaService",
ProtoPackage: "voltha",
Routes: []RouteConfig{routeConfig},
- ProtoFile: METHOD_ROUTER_PROTOFILE,
+ ProtoFile: TEST_PROTOFILE,
}
return &routeConfig, &routerConfig
}
diff --git a/internal/pkg/afrouter/request.go b/internal/pkg/afrouter/request.go
index 8fc15aa..dbd9968 100644
--- a/internal/pkg/afrouter/request.go
+++ b/internal/pkg/afrouter/request.go
@@ -66,7 +66,10 @@
}
}
if r.sendClosed {
- stream.CloseSend()
+ err := stream.CloseSend()
+ if err != nil {
+ log.Errorf("%v", err)
+ }
}
r.mutex.Unlock()
@@ -111,9 +114,9 @@
if err = r.sendResponseFrame(stream, frame); err != nil {
break
}
- } else { // !activeStream && !r.isStreamingRequest && !r.isStreamingResponse
- // just read & discard until the stream dies
}
+ // !activeStream && !r.isStreamingRequest && !r.isStreamingResponse
+ // just read & discard until the stream dies
}
}
}
@@ -264,7 +267,10 @@
log.Debug("Closing southbound streams")
r.sendClosed = true
for _, stream := range r.streams {
- stream.CloseSend()
+ err := stream.CloseSend()
+ if err != nil {
+ log.Errorf("%v", err)
+ }
}
r.mutex.Unlock()
diff --git a/internal/pkg/afrouter/round-robin-router.go b/internal/pkg/afrouter/round-robin-router.go
index b5f031e..8fe495a 100644
--- a/internal/pkg/afrouter/round-robin-router.go
+++ b/internal/pkg/afrouter/round-robin-router.go
@@ -17,7 +17,6 @@
package afrouter
import (
- "errors"
"fmt"
"github.com/opencord/voltha-go/common/log"
"google.golang.org/grpc"
@@ -31,7 +30,7 @@
}
func newRoundRobinRouter(rconf *RouterConfig, config *RouteConfig) (Router, error) {
- var err error = nil
+ var err error
var rtrn_err = false
// Validate the configuration
@@ -53,7 +52,6 @@
}
var bptr *backend
- bptr = nil
rr := RoundRobinRouter{
name: config.Name,
grpcService: rconf.ProtoService,
@@ -61,7 +59,7 @@
}
// Create the backend cluster or link to an existing one
- ok := true
+ var ok bool
if rr.cluster, ok = clusters[config.backendCluster.Name]; !ok {
if rr.cluster, err = newBackendCluster(config.backendCluster); err != nil {
log.Errorf("Could not create a backend for router %s", config.Name)
@@ -70,7 +68,7 @@
}
if rtrn_err {
- return rr, errors.New(fmt.Sprintf("Failed to create a new router '%s'", rr.name))
+ return rr, fmt.Errorf("Failed to create a new router '%s'", rr.name)
}
return rr, nil
diff --git a/internal/pkg/afrouter/round-robin-router_test.go b/internal/pkg/afrouter/round-robin-router_test.go
index 972832e..d38defc 100644
--- a/internal/pkg/afrouter/round-robin-router_test.go
+++ b/internal/pkg/afrouter/round-robin-router_test.go
@@ -19,22 +19,12 @@
import (
"fmt"
"github.com/golang/protobuf/proto"
- "github.com/opencord/voltha-go/common/log"
common_pb "github.com/opencord/voltha-protos/go/common"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc"
"testing"
)
-const (
- ROUND_ROBIN_ROUTER_PROTOFILE = "../../../vendor/github.com/opencord/voltha-protos/voltha.pb"
-)
-
-func init() {
- log.SetDefaultLogger(log.JSON, log.DebugLevel, log.Fields{"instanceId": 1})
- log.AddPackage(log.JSON, log.WarnLevel, nil)
-}
-
func MakeRoundRobinTestConfig(numBackends int, numConnections int) (*RouteConfig, *RouterConfig) {
var backends []BackendConfig
@@ -77,7 +67,7 @@
ProtoService: "VolthaService",
ProtoPackage: "voltha",
Routes: []RouteConfig{routeConfig},
- ProtoFile: ROUND_ROBIN_ROUTER_PROTOFILE,
+ ProtoFile: TEST_PROTOFILE,
}
return &routeConfig, &routerConfig
}
diff --git a/internal/pkg/afrouter/router.go b/internal/pkg/afrouter/router.go
index 7abbceb..f1ac388 100644
--- a/internal/pkg/afrouter/router.go
+++ b/internal/pkg/afrouter/router.go
@@ -17,7 +17,6 @@
package afrouter
import (
- "errors"
"fmt"
"google.golang.org/grpc"
)
@@ -75,6 +74,6 @@
}
return r, err
default:
- return nil, errors.New(fmt.Sprintf("Internal error, undefined router type: %s", config.Type))
+ return nil, fmt.Errorf("Internal error, undefined router type: %s", config.Type)
}
}
diff --git a/internal/pkg/afrouter/router_test.go b/internal/pkg/afrouter/router_test.go
index 940f6cc..c38b70e 100644
--- a/internal/pkg/afrouter/router_test.go
+++ b/internal/pkg/afrouter/router_test.go
@@ -17,16 +17,10 @@
package afrouter
import (
- "github.com/opencord/voltha-go/common/log"
"github.com/stretchr/testify/assert"
"testing"
)
-func init() {
- log.SetDefaultLogger(log.JSON, log.DebugLevel, log.Fields{"instanceId": 1})
- log.AddPackage(log.JSON, log.WarnLevel, nil)
-}
-
func TestRouter(t *testing.T) {
routeConfig, routerConfig := MakeRoundRobinTestConfig(1, 1)
router, err := newRouter(routerConfig)
diff --git a/internal/pkg/afrouter/server.go b/internal/pkg/afrouter/server.go
index 82269d2..0b4c03f 100644
--- a/internal/pkg/afrouter/server.go
+++ b/internal/pkg/afrouter/server.go
@@ -22,6 +22,7 @@
"github.com/opencord/voltha-go/common/log"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
+ "google.golang.org/grpc/status"
"net"
"net/url"
"strconv"
@@ -43,7 +44,7 @@
}
func newServer(config *ServerConfig) (*server, error) {
- var err error = nil
+ var err error
var rtrn_err = false
var s *server
// Change over to the new configuration format
@@ -107,6 +108,7 @@
}
}
}
+
// Configure the grpc handler
s.proxyServer = grpc.NewServer(
grpc.CustomCodec(Codec()),
@@ -143,7 +145,7 @@
// Determine what router is intended to handle this request
fullMethodName, ok := grpc.MethodFromServerStream(serverStream)
if !ok {
- return grpc.Errorf(codes.Internal, "lowLevelServerStream doesn't exist in context")
+ return status.Errorf(codes.Internal, "lowLevelServerStream doesn't exist in context")
}
log.Debugf("\n\nProcessing grpc request %s on server %s", fullMethodName, s.name)
methodInfo := newMethodDetails(fullMethodName)
diff --git a/internal/pkg/afrouter/server_test.go b/internal/pkg/afrouter/server_test.go
index 162937c..2c39c53 100644
--- a/internal/pkg/afrouter/server_test.go
+++ b/internal/pkg/afrouter/server_test.go
@@ -17,40 +17,12 @@
package afrouter
import (
- "fmt"
- "github.com/opencord/voltha-go/common/log"
"github.com/stretchr/testify/assert"
"testing"
)
-func init() {
- log.SetDefaultLogger(log.JSON, log.DebugLevel, log.Fields{"instanceId": 1})
- log.AddPackage(log.JSON, log.WarnLevel, nil)
-}
-
-func MakeServerTestConfig(numBackends int, numConnections int) *ServerConfig {
-
+func MakeServerTestConfig() *ServerConfig {
var routerPackage []RouterPackage
- var backends []BackendConfig
- for backendIndex := 0; backendIndex < numBackends; backendIndex++ {
- var connections []ConnectionConfig
- for connectionIndex := 0; connectionIndex < numConnections; connectionIndex++ {
- connectionConfig := ConnectionConfig{
- Name: fmt.Sprintf("ro_vcore%d%d", backendIndex, connectionIndex+1),
- Addr: "foo",
- Port: "123",
- }
- connections = append(connections, connectionConfig)
- }
-
- backendConfig := BackendConfig{
- Name: fmt.Sprintf("ro_vcore%d", backendIndex),
- Type: BackendSingleServer,
- Connections: connections,
- }
-
- backends = append(backends, backendConfig)
- }
routerPackageConfig := RouterPackage{
Router: `json:"router"`,
@@ -73,7 +45,7 @@
// Test creation of a new Server
func TestServerInit(t *testing.T) {
- serverConfig := MakeServerTestConfig(1, 1)
+ serverConfig := MakeServerTestConfig()
serv, err := newServer(serverConfig)
@@ -85,7 +57,7 @@
// Test creation of a new Server, error in Addr
func TestServerInitWrongAddr(t *testing.T) {
- serverConfig := MakeServerTestConfig(1, 1)
+ serverConfig := MakeServerTestConfig()
serverConfig.Addr = "127.300.1.1"
serv, err := newServer(serverConfig)
@@ -97,7 +69,7 @@
// Test creation of a new Server, error in Port
func TestServerInitWrongPort(t *testing.T) {
- serverConfig := MakeServerTestConfig(1, 1)
+ serverConfig := MakeServerTestConfig()
serverConfig.Port = 23
serv, err := newServer(serverConfig)
@@ -109,7 +81,7 @@
// Test creation of a new Server, error in Name
func TestServerInitNoName(t *testing.T) {
- serverConfig := MakeServerTestConfig(1, 1)
+ serverConfig := MakeServerTestConfig()
serverConfig.Name = ""
serv, err := newServer(serverConfig)
@@ -121,7 +93,7 @@
// Test creation of a new Server, error in Type
func TestServerInitWrongType(t *testing.T) {
- serverConfig := MakeServerTestConfig(1, 1)
+ serverConfig := MakeServerTestConfig()
serverConfig.Type = "xxx"
serv, err := newServer(serverConfig)
@@ -133,7 +105,7 @@
// Test creation of a new Server, error in Router
func TestServerInitNoRouter(t *testing.T) {
- serverConfig := MakeServerTestConfig(1, 1)
+ serverConfig := MakeServerTestConfig()
serverConfig.routers = nil
serv, err := newServer(serverConfig)
@@ -145,7 +117,7 @@
// Test creation of a new Server
func TestServerInitHandler(t *testing.T) {
- serverConfig := MakeServerTestConfig(1, 1)
+ serverConfig := MakeServerTestConfig()
serverConfig.Port = 55556
serv, err := newServer(serverConfig)
diff --git a/internal/pkg/afrouter/signals.go b/internal/pkg/afrouter/signals.go
index 37b154b..950537c 100644
--- a/internal/pkg/afrouter/signals.go
+++ b/internal/pkg/afrouter/signals.go
@@ -45,7 +45,6 @@
sigchan := make(chan os.Signal, 1)
signal.Notify(sigchan, os.Interrupt)
signal.Notify(sigchan, syscall.SIGTERM)
- signal.Notify(sigchan, syscall.SIGKILL)
// Block until we receive a signal on the channel
<-sigchan
diff --git a/internal/pkg/afrouter/signals_test.go b/internal/pkg/afrouter/signals_test.go
index 4033b70..213e141 100644
--- a/internal/pkg/afrouter/signals_test.go
+++ b/internal/pkg/afrouter/signals_test.go
@@ -21,16 +21,10 @@
package afrouter
import (
- "github.com/opencord/voltha-go/common/log"
"github.com/stretchr/testify/assert"
"testing"
)
-func init() {
- log.SetDefaultLogger(log.JSON, log.DebugLevel, log.Fields{"instanceId": 1})
- log.AddPackage(log.JSON, log.WarnLevel, nil)
-}
-
// Test for function signalHandler and cleanExit
func TestSignalsInitExitHandler(t *testing.T) {
diff --git a/internal/pkg/afrouter/source-router.go b/internal/pkg/afrouter/source-router.go
index 7841554..61754b3 100644
--- a/internal/pkg/afrouter/source-router.go
+++ b/internal/pkg/afrouter/source-router.go
@@ -43,16 +43,14 @@
)
type SourceRouter struct {
- name string
- //association associationType
- routingField string
- grpcService string
- methodMap map[string]byte
- cluster *cluster
+ name string
+ grpcService string
+ methodMap map[string]byte
+ cluster *cluster
}
func newSourceRouter(rconf *RouterConfig, config *RouteConfig) (Router, error) {
- var err error = nil
+ var err error
var rtrn_err = false
var pkg_re = regexp.MustCompile(`^(\.[^.]+\.)(.+)$`)
// Validate the configuration
@@ -142,7 +140,7 @@
// We need to pick a cluster, because server will call cluster.handler. The choice we make doesn't
// matter, as we can return a different cluster from Route().
- ok := true
+ var ok bool
if dr.cluster, ok = clusters[config.backendCluster.Name]; !ok {
if dr.cluster, err = newBackendCluster(config.backendCluster); err != nil {
log.Errorf("Could not create a backend for router %s", config.Name)
@@ -151,7 +149,7 @@
}
if rtrn_err {
- return dr, errors.New(fmt.Sprintf("Failed to create a new router '%s'", dr.name))
+ return dr, fmt.Errorf("Failed to create a new router '%s'", dr.name)
}
return dr, nil
@@ -229,7 +227,7 @@
return "", e
}
default:
- err := errors.New(fmt.Sprintf("Only integer and string route selectors are permitted"))
+ err := errors.New("Only integer and string route selectors are permitted")
log.Error(err)
return "", err
}
diff --git a/internal/pkg/afrouter/source-router_test.go b/internal/pkg/afrouter/source-router_test.go
index b33916c..3be2a6e 100644
--- a/internal/pkg/afrouter/source-router_test.go
+++ b/internal/pkg/afrouter/source-router_test.go
@@ -18,21 +18,11 @@
import (
"github.com/golang/protobuf/proto"
- "github.com/opencord/voltha-go/common/log"
common_pb "github.com/opencord/voltha-protos/go/common"
"github.com/stretchr/testify/assert"
"testing"
)
-const (
- SOURCE_ROUTER_PROTOFILE = "../../../vendor/github.com/opencord/voltha-protos/voltha.pb"
-)
-
-func init() {
- log.SetDefaultLogger(log.JSON, log.DebugLevel, log.Fields{"instanceId": 1})
- log.AddPackage(log.JSON, log.WarnLevel, nil)
-}
-
func MakeSourceRouterTestConfig() (*RouteConfig, *RouterConfig) {
connectionConfig := ConnectionConfig{
Name: "ro_vcore01",
@@ -65,7 +55,7 @@
ProtoService: "VolthaService",
ProtoPackage: "voltha",
Routes: []RouteConfig{routeConfig},
- ProtoFile: SOURCE_ROUTER_PROTOFILE,
+ ProtoFile: TEST_PROTOFILE,
}
return &routeConfig, &routerConfig
}
@@ -105,9 +95,11 @@
assert.Nil(t, err)
s, err := sourceRouter.decodeProtoField(loggingData, 2) // field 2 is package_name
+ assert.Nil(t, err)
assert.Equal(t, s, "default")
s, err = sourceRouter.decodeProtoField(loggingData, 3) // field 2 is component_name
+ assert.Nil(t, err)
assert.Equal(t, s, "ro_vcore0.ro_vcore01")
}