Merge pull request #27 from colinmcintosh/cm/issue-24

Added check for list of cache errors in addition to single cache errors.

.github/workflows/go.yml

@@ -12,7 +12,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go: [ '1.13', '1.14', '1.15' ]
+        go: [ '1.14', '1.15', '1.16' ]
     steps:
       - name: Set up timezone
         uses: zcong1993/[email protected]

.github/workflows/release.yml

@@ -26,7 +26,7 @@ jobs:
       - name: Set up Golang
         uses: actions/setup-go@v2
         with:
-          go-version: 1.15
+          go-version: 1.16
         id: go
 
       - name: Checkout code

README.md

@@ -101,9 +101,8 @@ See the [godoc pages][8] for documentation and usage examples.
 
 
 ## Pre-requisites
-- Golang 1.13 or newer
-- A target that supports gNMI Subscribe. This is usually a network router or
-  switch.
+- Golang 1.14 or newer
+- A target that supports gNMI Subscribe. This is usually a network router or switch.
 - A running instance of [Apache Zookeeper][3]. If you only want to run
   a single instance of gnmi-gateway (i.e. without failover)
   you don't need Zookeeper. See the development instructions below for how

gateway/connections/state.go

@@ -33,6 +33,7 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
+	"github.com/openconfig/gnmi/errlist"
 	"sync"
 	"time"
 
@@ -70,12 +71,12 @@ type ConnectionState struct {
 	// lock is the distributed lock that must be acquired before a connection is made if .connectWithLock() is called
 	lock locking.DistributedLocker
 	// The unique name of the target that is being connected to
-	name        string
+	name string
 	// noTLSWarning indicates if the warning about the NoTLS flag deprecation
 	// has been displayed yet.
 	noTLSWarning bool
-	queryTarget string
-	request     *gnmipb.SubscribeRequest
+	queryTarget  string
+	request      *gnmipb.SubscribeRequest
 	// seen is the list of targets that have been seen on this connection
 	seen      map[string]bool
 	seenMutex sync.Mutex
@@ -164,7 +165,7 @@ func (t *ConnectionState) doConnect() {
 
 	// TODO (cmcintosh): make PR for targetpb to include TLS config and remove this
 	_, NoTLS := t.target.Meta["NoTLS"]
-	if NoTLS  && !t.noTLSWarning {
+	if NoTLS && !t.noTLSWarning {
 		t.noTLSWarning = true
 		t.config.Log.Warn().Msg("DEPRECATED: The 'NoTLS' target flag has been deprecated and will be removed in a future release. Please use 'NoTLSVerify' instead.")
 	}
@@ -382,22 +383,40 @@ func (t *ConnectionState) sync() {
 }
 
 func (t *ConnectionState) updateTargetCache(cache *cache.Target, update *gnmipb.Notification) error {
+	var hasError bool
 	err := cache.GnmiUpdate(update)
 	if err != nil {
 		// Some errors won't corrupt the cache so no need to return an error to the ProtoHandler caller. For these
 		// errors we just log them and move on.
-		switch err.Error() {
-		case "suppressed duplicate value":
-		case "update is stale":
-			t.counterStale.Increment()
-			//t.config.Log.Warn().Msgf("Target %s: %s: %s", t.name, err, utils.GNMINotificationPrettyString(update))
-		default:
+		errList, isList := err.(errlist.Error)
+		if isList {
+			for _, err := range errList.Errors() {
+				hasError = t.handleCacheError(err)
+				if hasError {
+					break
+				}
+			}
+		} else {
+			hasError = t.handleCacheError(err)
+		}
+		if hasError {
 			return fmt.Errorf("target '%s' cache update error: %v: %+v", t.name, err, utils.GNMINotificationPrettyString(update))
 		}
 	}
 	return nil
 }
 
+func (t *ConnectionState) handleCacheError(err error) bool {
+	switch err.Error() {
+	case "suppressed duplicate value":
+	case "update is stale":
+		t.counterStale.Increment()
+		//t.config.Log.Warn().Msgf("Target %s: %s: %s", t.name, err, utils.GNMINotificationPrettyString(update))
+		return false
+	}
+	return true
+}
+
 // rejectUpdate returns true if the gNMI notification is unwanted based on the RejectUpdates
 // configuration in GatewayConfig.
 func (t *ConnectionState) rejectUpdate(notification *gnmipb.Notification) bool {

gateway/connections/state_test.go

@@ -0,0 +1,82 @@
+// Copyright 2020 Netflix Inc
+// Author: Colin McIntosh ([email protected])
+//
+// Licensed under the Apache License, Version 2.0 (the "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 connections
+
+import (
+	"github.com/openconfig/gnmi/cache"
+	gnmipb "github.com/openconfig/gnmi/proto/gnmi"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestConnectionState_updateTargetCache(t *testing.T) {
+	state := &ConnectionState{
+		//clusterMember: clusterMember,
+		//config:        c.config,
+		//connManager:   c,
+		name:        "test_state",
+		targetCache: cache.New(nil).Add("a"),
+		//target:        newConfig,
+		//request:       msg.Insert.Request[newConfig.Request],
+		seen: make(map[string]bool),
+		//useLock:       c.zkConn != nil && !noLock,
+	}
+	state.InitializeMetrics()
+
+	update := []*gnmipb.Update{
+		{
+			Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "x"}}},
+			Val:  &gnmipb.TypedValue{Value: &gnmipb.TypedValue_IntVal{IntVal: 1}},
+		},
+		{
+			Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "y"}}},
+			Val:  &gnmipb.TypedValue{Value: &gnmipb.TypedValue_IntVal{IntVal: 2}},
+		},
+		{
+			Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "z"}}},
+			Val:  &gnmipb.TypedValue{Value: &gnmipb.TypedValue_IntVal{IntVal: 3}},
+		},
+	}
+	notifications := []struct {
+		notification *gnmipb.Notification
+		expected     error
+	}{
+		{
+			&gnmipb.Notification{Timestamp: 1, Prefix: &gnmipb.Path{Target: "a"}, Update: update},
+			nil,
+		},
+		{
+			&gnmipb.Notification{Timestamp: 2, Prefix: &gnmipb.Path{Target: "a"}, Update: update},
+			nil,
+		},
+		{
+			&gnmipb.Notification{Timestamp: 3, Prefix: &gnmipb.Path{Target: "a"}, Update: update},
+			nil,
+		},
+		{
+			&gnmipb.Notification{Timestamp: 1, Prefix: &gnmipb.Path{Target: "a"}, Update: update},
+			nil,
+		},
+	}
+	for i, notificationCase := range notifications {
+		notificationCase.notification.Update[0].Val.Value.(*gnmipb.TypedValue_IntVal).IntVal += int64(i)
+		notificationCase.notification.Update[1].Val.Value.(*gnmipb.TypedValue_IntVal).IntVal += int64(i)
+		notificationCase.notification.Update[2].Val.Value.(*gnmipb.TypedValue_IntVal).IntVal += int64(i)
+		err := state.updateTargetCache(state.targetCache, notificationCase.notification)
+		assert.Equal(t, notificationCase.expected, err)
+	}
+	assert.Equal(t, float64(3), state.counterStale.Count())
+}