From 524230c13164561c1f2b9a1a10257deb9f4fde27 Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Tue, 8 Dec 2020 17:40:34 +0100
Subject: [PATCH 01/11] feat: added possibility to setup NB configuration from
 file at VPP-Agent start

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 client/local_client.go                        |   8 +-
 clientv2/vpp/dbadapter/data_change_db.go      |   5 +-
 clientv2/vpp/dbadapter/data_resync_db.go      |   5 +-
 cmd/vpp-agent/app/vpp_agent.go                |  17 +-
 pkg/models/registry.go                        | 113 ++++++-
 pkg/models/remote_model.go                    |   2 +-
 plugins/configurator/configurator.go          |   5 +-
 .../orchestrator/contextdecorator/context.go  |  31 ++
 plugins/orchestrator/dispatcher.go            |   3 +-
 .../localregistry/nbinitfileregistry.go       | 286 ++++++++++++++++++
 plugins/orchestrator/meta_service.go          |   3 +-
 plugins/orchestrator/orchestrator.go          |  36 ++-
 plugins/orchestrator/store.go                 |  14 -
 plugins/orchestrator/watcher/aggregator.go    |  29 +-
 14 files changed, 492 insertions(+), 65 deletions(-)
 create mode 100644 plugins/orchestrator/contextdecorator/context.go
 create mode 100644 plugins/orchestrator/localregistry/nbinitfileregistry.go

diff --git a/client/local_client.go b/client/local_client.go
index e0ae30a94f..fdebce0b32 100644
--- a/client/local_client.go
+++ b/client/local_client.go
@@ -23,7 +23,7 @@ import (
 	"go.ligato.io/cn-infra/v2/db/keyval"
 
 	"go.ligato.io/vpp-agent/v3/pkg/models"
-	orch "go.ligato.io/vpp-agent/v3/plugins/orchestrator"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/contextdecorator"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/generic"
 )
 
@@ -64,7 +64,7 @@ func (c *client) ResyncConfig(items ...proto.Message) error {
 	}
 
 	ctx := context.Background()
-	ctx = orch.DataSrcContext(ctx, "localclient")
+	ctx = contextdecorator.DataSrcContext(ctx, "localclient")
 	return txn.Commit(ctx)
 }
 
@@ -121,9 +121,9 @@ func (r *changeRequest) Send(ctx context.Context) error {
 	if r.err != nil {
 		return r.err
 	}
-	_, withDataSrc := orch.DataSrcFromContext(ctx)
+	_, withDataSrc := contextdecorator.DataSrcFromContext(ctx)
 	if !withDataSrc {
-		ctx = orch.DataSrcContext(ctx, "localclient")
+		ctx = contextdecorator.DataSrcContext(ctx, "localclient")
 	}
 	return r.txn.Commit(ctx)
 }
diff --git a/clientv2/vpp/dbadapter/data_change_db.go b/clientv2/vpp/dbadapter/data_change_db.go
index fdf72eb2d1..7ad4be0944 100644
--- a/clientv2/vpp/dbadapter/data_change_db.go
+++ b/clientv2/vpp/dbadapter/data_change_db.go
@@ -18,11 +18,10 @@ import (
 	"context"
 
 	"go.ligato.io/cn-infra/v2/db/keyval"
-
 	"go.ligato.io/vpp-agent/v3/pkg/models"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/contextdecorator"
 
 	vppclient "go.ligato.io/vpp-agent/v3/clientv2/vpp"
-	orch "go.ligato.io/vpp-agent/v3/plugins/orchestrator"
 	abf "go.ligato.io/vpp-agent/v3/proto/ligato/vpp/abf"
 	acl "go.ligato.io/vpp-agent/v3/proto/ligato/vpp/acl"
 	intf "go.ligato.io/vpp-agent/v3/proto/ligato/vpp/interfaces"
@@ -73,7 +72,7 @@ func (dsl *DataChangeDSL) Delete() vppclient.DeleteDSL {
 // Send propagates requested changes to the plugins.
 func (dsl *DataChangeDSL) Send() vppclient.Reply {
 	ctx := context.Background()
-	ctx = orch.DataSrcContext(ctx, "localclient")
+	ctx = contextdecorator.DataSrcContext(ctx, "localclient")
 	err := dsl.txn.Commit(ctx)
 	return &Reply{err}
 }
diff --git a/clientv2/vpp/dbadapter/data_resync_db.go b/clientv2/vpp/dbadapter/data_resync_db.go
index bfddd08675..d1de03e269 100644
--- a/clientv2/vpp/dbadapter/data_resync_db.go
+++ b/clientv2/vpp/dbadapter/data_resync_db.go
@@ -18,11 +18,10 @@ import (
 	"context"
 
 	"go.ligato.io/cn-infra/v2/db/keyval"
-
 	"go.ligato.io/vpp-agent/v3/pkg/models"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/contextdecorator"
 
 	vppclient "go.ligato.io/vpp-agent/v3/clientv2/vpp"
-	orch "go.ligato.io/vpp-agent/v3/plugins/orchestrator"
 	abf "go.ligato.io/vpp-agent/v3/proto/ligato/vpp/abf"
 	acl "go.ligato.io/vpp-agent/v3/proto/ligato/vpp/acl"
 	intf "go.ligato.io/vpp-agent/v3/proto/ligato/vpp/interfaces"
@@ -367,7 +366,7 @@ func (dsl *DataResyncDSL) Send() vppclient.Reply {
 	}
 
 	ctx := context.Background()
-	ctx = orch.DataSrcContext(ctx, "localclient")
+	ctx = contextdecorator.DataSrcContext(ctx, "localclient")
 	err := dsl.txn.Commit(ctx)
 
 	return &Reply{err: err}
diff --git a/cmd/vpp-agent/app/vpp_agent.go b/cmd/vpp-agent/app/vpp_agent.go
index 4c64413411..93313583bc 100644
--- a/cmd/vpp-agent/app/vpp_agent.go
+++ b/cmd/vpp-agent/app/vpp_agent.go
@@ -15,11 +15,11 @@
 package app
 
 import (
+	"github.com/go-errors/errors"
 	"go.ligato.io/cn-infra/v2/datasync"
 	"go.ligato.io/cn-infra/v2/datasync/kvdbsync"
 	"go.ligato.io/cn-infra/v2/datasync/kvdbsync/local"
 	"go.ligato.io/cn-infra/v2/datasync/msgsync"
-	"go.ligato.io/cn-infra/v2/datasync/resync"
 	"go.ligato.io/cn-infra/v2/db/keyval/consul"
 	"go.ligato.io/cn-infra/v2/db/keyval/etcd"
 	"go.ligato.io/cn-infra/v2/db/keyval/redis"
@@ -28,7 +28,6 @@ import (
 	"go.ligato.io/cn-infra/v2/infra"
 	"go.ligato.io/cn-infra/v2/logging/logmanager"
 	"go.ligato.io/cn-infra/v2/messaging/kafka"
-
 	"go.ligato.io/vpp-agent/v3/plugins/configurator"
 	linux_ifplugin "go.ligato.io/vpp-agent/v3/plugins/linux/ifplugin"
 	linux_iptablesplugin "go.ligato.io/vpp-agent/v3/plugins/linux/iptablesplugin"
@@ -36,6 +35,8 @@ import (
 	linux_nsplugin "go.ligato.io/vpp-agent/v3/plugins/linux/nsplugin"
 	"go.ligato.io/vpp-agent/v3/plugins/netalloc"
 	"go.ligato.io/vpp-agent/v3/plugins/orchestrator"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/localregistry"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/watcher"
 	"go.ligato.io/vpp-agent/v3/plugins/restapi"
 	"go.ligato.io/vpp-agent/v3/plugins/telemetry"
 	"go.ligato.io/vpp-agent/v3/plugins/vpp/abfplugin"
@@ -100,12 +101,14 @@ func New() *VPPAgent {
 	)
 
 	// Set watcher for KVScheduler.
-	watchers := datasync.KVProtoWatchers{
+	initFileRegistry := localregistry.NewNBInitFileRegistryPlugin()
+	watchers := watcher.NewPlugin(watcher.UseWatchers(
 		local.DefaultRegistry,
+		initFileRegistry,
 		etcdDataSync,
 		consulDataSync,
 		redisDataSync,
-	}
+	))
 	orchestrator.DefaultPlugin.Watcher = watchers
 	orchestrator.DefaultPlugin.StatusPublisher = writers
 	orchestrator.EnabledGrpcMetrics()
@@ -157,9 +160,9 @@ func (a *VPPAgent) Init() error {
 
 // AfterInit executes resync.
 func (a *VPPAgent) AfterInit() error {
-	// manually start resync after all plugins started
-	resync.DefaultPlugin.DoResync()
-	//orchestrator.DefaultPlugin.InitialSync()
+	if err := orchestrator.DefaultPlugin.InitialSync(); err != nil {
+		return errors.Errorf("failure in initial sync: %v", err)
+	}
 	a.StatusCheck.ReportStateChange(a.PluginName, statuscheck.OK, nil)
 	return nil
 }
diff --git a/pkg/models/registry.go b/pkg/models/registry.go
index 9ae132848b..55ded31a4d 100644
--- a/pkg/models/registry.go
+++ b/pkg/models/registry.go
@@ -20,7 +20,10 @@ import (
 	"reflect"
 	"strings"
 
+	"github.com/go-errors/errors"
+	"github.com/golang/protobuf/jsonpb"
 	"github.com/golang/protobuf/proto"
+	protoV2 "google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/reflect/protoregistry"
 	"google.golang.org/protobuf/types/dynamicpb"
@@ -36,18 +39,20 @@ var (
 // LocalRegistry defines model registry for managing registered local models. Local models are locally compiled into
 // the program binary and hence some additional information in compare to remote models, i.e. go type.
 type LocalRegistry struct {
-	registeredTypes map[reflect.Type]*LocallyKnownModel
-	modelNames      map[string]*LocallyKnownModel
-	ordered         []reflect.Type
-	proxied         *RemoteRegistry
+	registeredModelsByGoType    map[reflect.Type]*LocallyKnownModel
+	registeredModelsByProtoName map[string]*LocallyKnownModel
+	modelNames                  map[string]*LocallyKnownModel
+	ordered                     []reflect.Type
+	proxied                     *RemoteRegistry
 }
 
 // NewRegistry returns initialized Registry.
 func NewRegistry() *LocalRegistry {
 	return &LocalRegistry{
-		registeredTypes: make(map[reflect.Type]*LocallyKnownModel),
-		modelNames:      make(map[string]*LocallyKnownModel),
-		proxied:         NewRemoteRegistry(),
+		registeredModelsByGoType:    make(map[reflect.Type]*LocallyKnownModel),
+		registeredModelsByProtoName: make(map[string]*LocallyKnownModel),
+		modelNames:                  make(map[string]*LocallyKnownModel),
+		proxied:                     NewRemoteRegistry(),
 	}
 }
 
@@ -66,12 +71,35 @@ func (r *LocalRegistry) GetModel(name string) (KnownModel, error) {
 
 // GetModelFor returns registered model for the given proto message.
 func (r *LocalRegistry) GetModelFor(x interface{}) (KnownModel, error) {
+	// find model by Go type
 	t := reflect.TypeOf(x)
-	model, found := r.registeredTypes[t]
+	model, found := r.registeredModelsByGoType[t]
 	if !found {
+		// check remotely retrieved models registered in local registry
 		if proxModel, err := r.proxied.GetModelFor(x); err == nil {
 			return proxModel, nil
 		}
+
+		// find model by Proto name
+		// (useful when using dynamically generated config instead of configurator.Config => go type of proto
+		// messages is in such case always dynamicpb.Message and never the go type of registered (generated)
+		// proto message)
+		if len(r.registeredModelsByProtoName) == 0 && len(r.registeredModelsByGoType) > 0 {
+			r.lazyInitRegisteredTypesByProtoName()
+		}
+		var protoName string
+		if pb, ok := x.(protoreflect.ProtoMessage); ok {
+			protoName = string(pb.ProtoReflect().Descriptor().FullName())
+		} else if v1, ok := x.(proto.Message); ok {
+			protoName = string(proto.MessageV2(v1).ProtoReflect().Descriptor().FullName())
+		}
+		if protoName != "" {
+			if model, found = r.registeredModelsByProtoName[protoName]; found {
+				return model, nil
+			}
+		}
+
+		// find model by checking proto options
 		if model = r.checkProtoOptions(x); model == nil {
 			return &LocallyKnownModel{}, fmt.Errorf("no model registered for type %v", t)
 		}
@@ -79,9 +107,23 @@ func (r *LocalRegistry) GetModelFor(x interface{}) (KnownModel, error) {
 	return model, nil
 }
 
+// lazyInitRegisteredTypesByProtoName performs lazy initialization of registeredModelsByProtoName. The reason
+// why initialization can't happen while registration (call of func Register(...)) is that some proto reflect
+// functionality is not available during this time. The registration happens as variable initialization, but
+// the reflection is initialized in init() func and that happens after variable initialization.
+//
+// Alternative solution would be to change when the models are registered (VPP-Agent have it like described
+// above and 3rd party model are probably copying the same behaviour). So to not break anything, the lazy
+// initialization seems like the best solution for now.
+func (r *LocalRegistry) lazyInitRegisteredTypesByProtoName() {
+	for _, model := range r.registeredModelsByGoType {
+		r.registeredModelsByProtoName[model.ProtoName()] = model // ProtoName() == ProtoReflect().Descriptor().FullName()
+	}
+}
+
 // GetModelForKey returns registered model for the given key or error.
 func (r *LocalRegistry) GetModelForKey(key string) (KnownModel, error) {
-	for _, model := range r.registeredTypes {
+	for _, model := range r.registeredModelsByGoType {
 		if model.IsKeyValid(key) {
 			return model, nil
 		}
@@ -96,7 +138,7 @@ func (r *LocalRegistry) GetModelForKey(key string) (KnownModel, error) {
 func (r *LocalRegistry) RegisteredModels() []KnownModel {
 	var models []KnownModel
 	for _, typ := range r.ordered {
-		models = append(models, r.registeredTypes[typ])
+		models = append(models, r.registeredModelsByGoType[typ])
 	}
 	models = append(models, r.proxied.RegisteredModels()...)
 	return models
@@ -109,7 +151,7 @@ func (r *LocalRegistry) MessageTypeRegistry() *protoregistry.Types {
 		typeRegistry.RegisterMessage(dynamicpb.NewMessageType(model.proto.ProtoReflect().Descriptor()))
 	}
 	proxiedTypes := r.proxied.MessageTypeRegistry()
-	proxiedTypes.RangeMessages(func (mt protoreflect.MessageType) bool {
+	proxiedTypes.RangeMessages(func(mt protoreflect.MessageType) bool {
 		typeRegistry.RegisterMessage(mt)
 		return true
 	})
@@ -135,7 +177,7 @@ func (r *LocalRegistry) Register(x interface{}, spec Spec, opts ...ModelOption)
 
 	goType := reflect.TypeOf(x)
 	// Check go type duplicate registration
-	if m, ok := r.registeredTypes[goType]; ok {
+	if m, ok := r.registeredModelsByGoType[goType]; ok {
 		return nil, fmt.Errorf("go type %v already registered for model %v", goType, m.Name())
 	}
 
@@ -174,6 +216,14 @@ func (r *LocalRegistry) Register(x interface{}, spec Spec, opts ...ModelOption)
 	// Use GetName as fallback for generating name
 	if _, ok := x.(named); ok {
 		model.nameFunc = func(obj interface{}) (s string, e error) {
+			// handling dynamic messages (they don't implement named interface)
+			if dynMessage, ok := obj.(*dynamicpb.Message); ok {
+				obj, e = dynamicMessageToRegisteredGoType(dynMessage, model.goType)
+				if e != nil {
+					return "", e
+				}
+			}
+			// handling other proto message
 			return obj.(named).GetName(), nil
 		}
 		model.nameTemplate = namedTemplate
@@ -184,7 +234,7 @@ func (r *LocalRegistry) Register(x interface{}, spec Spec, opts ...ModelOption)
 		opt(&model.modelOptions)
 	}
 
-	r.registeredTypes[goType] = model
+	r.registeredModelsByGoType[goType] = model
 	r.modelNames[model.Name()] = model
 	r.ordered = append(r.ordered, goType)
 
@@ -193,3 +243,40 @@ func (r *LocalRegistry) Register(x interface{}, spec Spec, opts ...ModelOption)
 	}
 	return model, nil
 }
+
+// dynamicMessageToRegisteredGoType converts proto dynamic message to proto message that was used at
+// the registration of the proto model corresponding to the proto dynamic message.
+// The registration proto message is usually the protoc-generated proto message. This conversion method
+// should help handling dynamic proto messages in mostly protoc-generated proto message oriented codebase
+// (i.e. help for type conversions to named, help handle missing data fields as seen in generated proto messages,...)
+func dynamicMessageToRegisteredGoType(dynamicMessage *dynamicpb.Message, goType reflect.Type) (proto.Message, error) {
+	// create empty proto message of the same type as it was used for registration
+	var registeredGoType interface{}
+	if goType.Kind() == reflect.Ptr {
+		registeredGoType = reflect.New(goType.Elem()).Interface()
+	} else {
+		registeredGoType = reflect.Zero(goType).Interface()
+	}
+	message, isProtoV1 := registeredGoType.(proto.Message)
+	if !isProtoV1 {
+		messageV2, isProtoV2 := registeredGoType.(protoV2.Message)
+		if !isProtoV2 {
+			return nil, errors.Errorf("registered go type(%T) is not proto.Message", registeredGoType)
+		}
+		message = proto.MessageV1(messageV2)
+	}
+
+	// fill empty proto message with data from its dynamic proto message counterpart
+	// (dynamic proto message -> json -> registered-type proto message)
+	marshaller := jsonpb.Marshaler{EmitDefaults: true} // using jsonbp to generate json with json name field in proto tag
+	jsonData, err := marshaller.MarshalToString(dynamicMessage)
+	if err != nil {
+		return nil, errors.Errorf("can't marshall dynamic proto message "+
+			"to json due to: %v (message: %+v)", err, dynamicMessage)
+	}
+	if err := jsonpb.Unmarshal(strings.NewReader(jsonData), message); err != nil {
+		return nil, errors.Errorf("can't load json to registered-type "+
+			"proto message due to: %v (json=%v)", err, jsonData)
+	}
+	return message, nil
+}
diff --git a/pkg/models/remote_model.go b/pkg/models/remote_model.go
index cd5c116e40..44b6425629 100644
--- a/pkg/models/remote_model.go
+++ b/pkg/models/remote_model.go
@@ -29,7 +29,7 @@ import (
 	"google.golang.org/protobuf/types/dynamicpb"
 )
 
-// RemotelyKnownModel represents a registered remnote model (remote model has only information about model
+// RemotelyKnownModel represents a registered remote model (remote model has only information about model
 // from remote source, i.e. missing go type because VPP-Agent meta service doesn't provide it)
 type RemotelyKnownModel struct {
 	model *ModelInfo
diff --git a/plugins/configurator/configurator.go b/plugins/configurator/configurator.go
index d0612c920e..3568502a2f 100644
--- a/plugins/configurator/configurator.go
+++ b/plugins/configurator/configurator.go
@@ -33,6 +33,7 @@ import (
 	"go.ligato.io/vpp-agent/v3/pkg/util"
 	kvs "go.ligato.io/vpp-agent/v3/plugins/kvscheduler/api"
 	"go.ligato.io/vpp-agent/v3/plugins/orchestrator"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/contextdecorator"
 	pb "go.ligato.io/vpp-agent/v3/proto/ligato/configurator"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/kvscheduler"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/linux"
@@ -110,7 +111,7 @@ func (svc *configuratorServer) Update(ctx context.Context, req *pb.UpdateRequest
 		ctx = kvs.WithResync(ctx, kvs.FullResync, true)
 	}
 
-	ctx = orchestrator.DataSrcContext(ctx, "grpc")
+	ctx = contextdecorator.DataSrcContext(ctx, "grpc")
 	results, err := svc.dispatch.PushData(ctx, kvPairs)
 
 	header := map[string]string{}
@@ -181,7 +182,7 @@ func (svc *configuratorServer) Delete(ctx context.Context, req *pb.DeleteRequest
 		})
 	}
 
-	ctx = orchestrator.DataSrcContext(ctx, "grpc")
+	ctx = contextdecorator.DataSrcContext(ctx, "grpc")
 	results, err := svc.dispatch.PushData(ctx, kvPairs)
 
 	header := map[string]string{}
diff --git a/plugins/orchestrator/contextdecorator/context.go b/plugins/orchestrator/contextdecorator/context.go
new file mode 100644
index 0000000000..2843b6fd3a
--- /dev/null
+++ b/plugins/orchestrator/contextdecorator/context.go
@@ -0,0 +1,31 @@
+// Copyright (c) 2020 Pantheon.tech
+//
+// 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 contextdecorator handles insertions and extractions of orchestrator related data from context.
+package contextdecorator
+
+import "context"
+
+type dataSrcKeyT string
+
+var dataSrcKey = dataSrcKeyT("dataSrc")
+
+func DataSrcContext(ctx context.Context, dataSrc string) context.Context {
+	return context.WithValue(ctx, dataSrcKey, dataSrc)
+}
+
+func DataSrcFromContext(ctx context.Context) (dataSrc string, ok bool) {
+	dataSrc, ok = ctx.Value(dataSrcKey).(string)
+	return
+}
diff --git a/plugins/orchestrator/dispatcher.go b/plugins/orchestrator/dispatcher.go
index c3228a4bfc..2e6b088918 100644
--- a/plugins/orchestrator/dispatcher.go
+++ b/plugins/orchestrator/dispatcher.go
@@ -24,6 +24,7 @@ import (
 	"github.com/golang/protobuf/proto"
 	"github.com/pkg/errors"
 	"go.ligato.io/cn-infra/v2/logging"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/contextdecorator"
 
 	"go.ligato.io/vpp-agent/v3/pkg/models"
 	kvs "go.ligato.io/vpp-agent/v3/plugins/kvscheduler/api"
@@ -102,7 +103,7 @@ func (p *dispatcher) PushData(ctx context.Context, kvPairs []KeyVal) (results []
 
 	pr := trace.StartRegion(ctx, "prepare kv data")
 
-	dataSrc, ok := DataSrcFromContext(ctx)
+	dataSrc, ok := contextdecorator.DataSrcFromContext(ctx)
 	if !ok {
 		dataSrc = "global"
 	}
diff --git a/plugins/orchestrator/localregistry/nbinitfileregistry.go b/plugins/orchestrator/localregistry/nbinitfileregistry.go
new file mode 100644
index 0000000000..9fe1c27bcf
--- /dev/null
+++ b/plugins/orchestrator/localregistry/nbinitfileregistry.go
@@ -0,0 +1,286 @@
+// Copyright (c) 2020 Pantheon.tech
+//
+// 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 localregistry
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+
+	yaml2 "github.com/ghodss/yaml"
+	"github.com/go-errors/errors"
+	"github.com/golang/protobuf/proto"
+	"go.ligato.io/cn-infra/v2/config"
+	"go.ligato.io/cn-infra/v2/datasync"
+	"go.ligato.io/cn-infra/v2/datasync/kvdbsync/local"
+	"go.ligato.io/cn-infra/v2/datasync/resync"
+	"go.ligato.io/cn-infra/v2/datasync/syncbase"
+	"go.ligato.io/cn-infra/v2/db/keyval"
+	"go.ligato.io/cn-infra/v2/infra"
+	"go.ligato.io/vpp-agent/v3/client"
+	"google.golang.org/protobuf/encoding/protojson"
+	protoV2 "google.golang.org/protobuf/proto"
+)
+
+const (
+	registryName          = "nb-init-file-registry"
+	defaultNBInitFilePath = "nb-initial-config.yaml"
+)
+
+type Option func(*NBInitFileRegistry)
+
+// NBInitFileRegistry is local read-only NB configuration provider with exclusive data source from a file
+// given by a file path (InitConfigFilePath). Its purpose is to seamlessly integrated NB configuration
+// from file as another NB configuration provider (to existing providers: etcd, consul, redis) and integrate
+// it's configuration into agent in the same standard way(datasync.KeyValProtoWatcher). The content of this
+// registry is meant to be only the initial NB configuration for the agent and will not reflect any changes
+// inside given file after initial content loading.
+//
+// The NB configuration provisioning process and how this registry fits into it:
+// 	1. NB data sources register to default resync plugin (NBInitFileRegistry registers too in watchNBResync(),
+//	   but only when there are some NB config data from file, otherwise it makes no sense to register because
+//	   there is nothing to forward. This also means that before register to resync plugin, the NB config from
+//	   file will be preloaded)
+// 	2. Call to resync plugin's DoResync triggers resync to NB configuration sources (NBInitFileRegistry takes
+//	   its preloaded NB config and stores it into another inner local registry)
+// 	3. NB configuration sources are also watchable (datasync.KeyValProtoWatcher) and the resync data is
+//	   collected by the watcher.Aggregator (NBInitFileRegistry is also watchable/forwards data to watcher.Aggregator,
+//	   it relies on the watcher capabilities of its inner local registry. This is the cause why to preloaded
+//	   the NB config from file([]proto.Message storage) and push it to another inner local storage later
+//	   (syncbase.Registry). If we used only one storage (syncbase.Registry for its watch capabilities), we
+//	   couldn't answer some questins about the storage soon enough (watcher.Aggregator in Watch(...) needs to
+//	   know whether this storage will send some data or not, otherwise the retrieval can hang on waiting for
+//	   data that never come))
+// 	4. watcher.Aggregator merges all collected resync data and forwards them its watch clients (it also implements
+//	   datasync.KeyValProtoWatcher just like the NB data sources).
+//  5. Clients of Aggregator (currently orchestrator and ifplugin) handle the NB changes/resync properly.
+type NBInitFileRegistry struct {
+	infra.PluginDeps
+
+	initialized             bool
+	config                  *Config
+	watchedRegistry         *syncbase.Registry
+	pushedToWatchedRegistry bool
+	preloadedNBConfigs      []proto.Message
+}
+
+// Config holds the NBInitFileRegistry configuration.
+type Config struct {
+	DisableNBInitialConfiguration  bool   `json:"disable-nb-initial-configuration"`
+	NBInitialConfigurationFilePath string `json:"nb-initial-configuration-file-path"`
+}
+
+// NewNBInitFileRegistryPlugin creates a new Plugin with the provides Options
+func NewNBInitFileRegistryPlugin(opts ...Option) *NBInitFileRegistry {
+	p := &NBInitFileRegistry{}
+
+	p.PluginName = "nbinitfileregistry"
+	p.watchedRegistry = syncbase.NewRegistry()
+
+	for _, o := range opts {
+		o(p)
+	}
+	if p.Cfg == nil {
+		p.Cfg = config.ForPlugin(p.String(),
+			config.WithCustomizedFlag(config.FlagName(p.String()), "nbinitfileregistryplugin.conf"),
+		)
+	}
+	p.PluginDeps.SetupLog()
+
+	return p
+}
+
+// Init initialize registry
+func (r *NBInitFileRegistry) Init() error {
+	if !r.initialized {
+		return r.initialize()
+	}
+	return nil
+}
+
+// Empty checks whether this registry holds data or not. As result of the properties of this registry
+// (readonly, will be filled only once from initial file import), this method directly indicates whether
+// the watchers of this registry will receive any data (Empty() == false, receive initial resync) or
+// won't receive anything at all (Empty() == true)
+func (r *NBInitFileRegistry) Empty() bool {
+	if !r.initialized { // could be called from init of other plugins -> possibly before this plugin init
+		if err := r.initialize(); err != nil {
+			r.Log.Errorf("can't initialize NBInitFileRegistry due to: %v", err)
+		}
+	}
+	return len(r.preloadedNBConfigs) == 0
+}
+
+// Watch functionality is forwarded to inner syncbase.Registry. For some watchers might be relevant
+// whether any data will be pushed to them at all (i.e. watcher.Aggregator). They should use the
+// Empty() method to find out whether there are (=ever will be do to nature of this registry) any
+// data for pushing to watchers.
+func (r *NBInitFileRegistry) Watch(resyncName string, changeChan chan datasync.ChangeEvent,
+	resyncChan chan datasync.ResyncEvent, keyPrefixes ...string) (datasync.WatchRegistration, error) {
+	return r.watchedRegistry.Watch(resyncName, changeChan, resyncChan, keyPrefixes...)
+}
+
+// initialize will try to pre-load the NB initial data
+// (watchers of this registry will receive it only after call to resync)
+func (r *NBInitFileRegistry) initialize() error {
+	defer func() {
+		r.initialized = true
+	}()
+
+	// parse configuration file
+	var err error
+	r.config, err = r.retrieveConfig()
+	if err != nil {
+		return err
+	}
+
+	// Initial NB configuration loaded from file
+	if !r.config.DisableNBInitialConfiguration {
+		// preload NB config data from file
+		if err := r.preloadNBConfigs(r.config.NBInitialConfigurationFilePath); err != nil {
+			return errors.Errorf("can't preload initial NB configuration from file due to: %v", err)
+		}
+		if len(r.preloadedNBConfigs) != 0 {
+			// watch for resync.DefaultPlugin.DoResync() that will trigger pushing of preloaded
+			// NB config data from file into NB aggregator watcher
+			// (see NBInitFileRegistry struct docs for detailed explanation)
+			r.watchNBResync()
+		}
+	}
+	return nil
+}
+
+// retrieveConfig loads NBInitFileRegistry plugin configuration file.
+func (r *NBInitFileRegistry) retrieveConfig() (*Config, error) {
+	config := &Config{
+		// default configuration
+		DisableNBInitialConfiguration:  false,
+		NBInitialConfigurationFilePath: defaultNBInitFilePath,
+	}
+	found, err := r.Cfg.LoadValue(config)
+	if !found {
+		r.Log.Debug("NBInitFileRegistry plugin config not found")
+		return config, nil
+	}
+	if err != nil {
+		return nil, err
+	}
+	return config, err
+}
+
+// watchNBResync will watch to default resync plugin's resync call(resync.DefaultPlugin.DoResync()) and will
+// load NB initial config from file (already preloaded from initialize()) when the first resync will be fired.
+func (r *NBInitFileRegistry) watchNBResync() {
+	registration := resync.DefaultPlugin.Register(registryName)
+	go r.watchResync(registration)
+}
+
+// watchResync will listen to resync plugin resync signals and at first resync will push the preloaded
+// NB initial config into internal local register (p.registry)
+func (r *NBInitFileRegistry) watchResync(resyncReg resync.Registration) {
+	for resyncStatus := range resyncReg.StatusChan() {
+		// resyncReg.StatusChan == Started => resync
+		if resyncStatus.ResyncStatus() == resync.Started && !r.pushedToWatchedRegistry {
+			if !r.Empty() { // put preloaded NB init file data into watched p.registry
+				c := client.NewClient(&txnFactory{r.watchedRegistry})
+				if err := c.ResyncConfig(r.preloadedNBConfigs...); err != nil {
+					r.Log.Errorf("resyncing preloaded NB init file data "+
+						"into watched registry failed: %v", err)
+				}
+			}
+			r.pushedToWatchedRegistry = true
+			resyncStatus.Ack()
+			// TODO some done channel to not continue as NOP goroutine
+			continue // can't unregister anymore -> need to listen to further resync signals, but it will be just NO-OPs
+		}
+		resyncStatus.Ack()
+	}
+}
+
+// preloadNBConfigs imports NB configuration from file(filepath) into preloadedNBConfigs. If file is not found,
+// it is not considered as error, but as a sign that the NB-configuration-loading-from-file feature should be
+// not used (inner registry remains empty and watchers of this registry get no data).
+func (r *NBInitFileRegistry) preloadNBConfigs(filePath string) error {
+	// check existence of NB init file
+	if _, err := os.Stat(filePath); os.IsNotExist(err) {
+		filePath := filePath
+		if absFilePath, err := filepath.Abs(filePath); err == nil {
+			filePath = absFilePath
+		}
+		r.Log.Debugf("Initialization NB configuration file(%v) not found. "+
+			"Skipping its preloading.", filePath)
+		return nil
+	}
+
+	// read data from file
+	b, err := ioutil.ReadFile(filePath)
+	if err != nil {
+		return errors.Errorf("problem reading file %s: %w", filePath, err)
+	}
+
+	// create dynamic config (using it instead of configurator.Config because it can hold also models defined
+	// outside the VPP-Agent repo, i.e. if this code is using 3rd party code based on VPP-Agent and having its
+	// additional properly registered configuration models)
+	knownModels, err := client.LocalClient.KnownModels("config") // locally registered models
+	if err != nil {
+		return errors.Errorf("can't get registered models: %w", err)
+	}
+	config, err := client.NewDynamicConfig(knownModels)
+	if err != nil {
+		return errors.Errorf("can't create dynamic config due to: %w", err)
+	}
+
+	// filling dynamically created config with data from NB init file
+	bj, err := yaml2.YAMLToJSON(b)
+	if err != nil {
+		return errors.Errorf("can't converting to JSON: %w", err)
+	}
+	err = protojson.Unmarshal(bj, config)
+	if err != nil {
+		return errors.Errorf("can't unmarshall NB init file data into dynamic config due to: %v", err)
+	}
+
+	// extracting proto messages from dynamic config structure
+	// (generic client wants single proto messages and not one big hierarchical config)
+	configMessages, err := client.DynamicConfigExport(config)
+	if err != nil {
+		return errors.Errorf("can't extract single NB init configuration proto messages "+
+			"from one big configuration proto message due to: %v", err)
+	}
+
+	// remember extracted data for later push to watched registry
+	r.preloadedNBConfigs = convertToProtoV1(configMessages)
+
+	return nil
+}
+
+type txnFactory struct {
+	registry *syncbase.Registry
+}
+
+func (p *txnFactory) NewTxn(resync bool) keyval.ProtoTxn {
+	if resync {
+		return local.NewProtoTxn(p.registry.PropagateResync)
+	}
+	return local.NewProtoTxn(p.registry.PropagateChanges)
+}
+
+func convertToProtoV1(messages []protoV2.Message) []proto.Message {
+	result := make([]proto.Message, 0, len(messages))
+	for _, message := range messages {
+		result = append(result, proto.MessageV1(message.ProtoReflect().Interface()))
+	}
+	return result
+}
diff --git a/plugins/orchestrator/meta_service.go b/plugins/orchestrator/meta_service.go
index b31ae2d572..1309dc180b 100644
--- a/plugins/orchestrator/meta_service.go
+++ b/plugins/orchestrator/meta_service.go
@@ -23,6 +23,7 @@ import (
 	"go.ligato.io/cn-infra/v2/logging"
 	"go.ligato.io/vpp-agent/v3/pkg/models"
 	kvs "go.ligato.io/vpp-agent/v3/plugins/kvscheduler/api"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/contextdecorator"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/generic"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc/codes"
@@ -118,7 +119,7 @@ func (s *genericService) SetConfig(ctx context.Context, req *generic.SetConfigRe
 		})
 	}
 
-	ctx = DataSrcContext(ctx, "grpc")
+	ctx = contextdecorator.DataSrcContext(ctx, "grpc")
 	if req.OverwriteAll {
 		ctx = kvs.WithResync(ctx, kvs.FullResync, true)
 	}
diff --git a/plugins/orchestrator/orchestrator.go b/plugins/orchestrator/orchestrator.go
index cfd8e7dc1a..5dd97eb96e 100644
--- a/plugins/orchestrator/orchestrator.go
+++ b/plugins/orchestrator/orchestrator.go
@@ -19,18 +19,20 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/go-errors/errors"
 	"github.com/golang/protobuf/proto"
 	"go.ligato.io/cn-infra/v2/datasync"
+	"go.ligato.io/cn-infra/v2/datasync/resync"
 	"go.ligato.io/cn-infra/v2/infra"
 	"go.ligato.io/cn-infra/v2/logging"
 	"go.ligato.io/cn-infra/v2/rpc/grpc"
-	"golang.org/x/net/context"
-	"google.golang.org/grpc/reflection"
-
 	"go.ligato.io/vpp-agent/v3/pkg/models"
 	kvs "go.ligato.io/vpp-agent/v3/plugins/kvscheduler/api"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/contextdecorator"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/generic"
 	"go.ligato.io/vpp-agent/v3/proto/ligato/kvscheduler"
+	"golang.org/x/net/context"
+	"google.golang.org/grpc/reflection"
 )
 
 var (
@@ -149,17 +151,23 @@ func (p *Plugin) Close() (err error) {
 	return nil
 }
 
-// InitialSync will start initial synchronization with downstream.
-func (p *Plugin) InitialSync() {
-	// FIXME: KVScheduler needs to have some type of sync that only refreshes state from SB
-	p.Log.Debugf("starting initial sync")
+// InitialSync will start initial synchronization.
+func (p *Plugin) InitialSync() error {
+	// SB resync
+	p.Log.Debugf("starting initial SB sync")
 	txn := p.KVScheduler.StartNBTransaction()
 	ctx := kvs.WithResync(context.Background(), kvs.DownstreamResync, true)
 	if _, err := txn.Commit(ctx); err != nil {
-		p.Log.Warnf("initial sync failed: %v", err)
-	} else {
-		p.Log.Infof("initial sync complete")
+		return errors.Errorf("initial SB sync failed: %v", err)
 	}
+	p.Log.Infof("initial SB sync complete")
+
+	// NB resync
+	p.Log.Debugf("starting initial NB sync")
+	resync.DefaultPlugin.DoResync() // NB init file data is also resynced here
+	p.Log.Infof("initial NB sync complete")
+
+	return nil
 }
 
 func (p *Plugin) watchEvents() {
@@ -202,9 +210,9 @@ func (p *Plugin) watchEvents() {
 			if ctx == nil {
 				ctx = context.Background()
 			}
-			_, withDataSrc := DataSrcFromContext(ctx)
+			_, withDataSrc := contextdecorator.DataSrcFromContext(ctx)
 			if !withDataSrc {
-				ctx = DataSrcContext(ctx, "datasync")
+				ctx = contextdecorator.DataSrcContext(ctx, "datasync")
 			}
 			ctx = kvs.WithRetryDefault(ctx)
 
@@ -248,9 +256,9 @@ func (p *Plugin) watchEvents() {
 			if ctx == nil {
 				ctx = context.Background()
 			}
-			_, withDataSrc := DataSrcFromContext(ctx)
+			_, withDataSrc := contextdecorator.DataSrcFromContext(ctx)
 			if !withDataSrc {
-				ctx = DataSrcContext(ctx, "datasync")
+				ctx = contextdecorator.DataSrcContext(ctx, "datasync")
 			}
 			ctx = kvs.WithResync(ctx, kvs.FullResync, true)
 			ctx = kvs.WithRetryDefault(ctx)
diff --git a/plugins/orchestrator/store.go b/plugins/orchestrator/store.go
index 5ce5c1e41c..f509d8a381 100644
--- a/plugins/orchestrator/store.go
+++ b/plugins/orchestrator/store.go
@@ -15,7 +15,6 @@
 package orchestrator
 
 import (
-	"context"
 	"sort"
 
 	"github.com/golang/protobuf/proto"
@@ -83,16 +82,3 @@ func (s *memStore) Delete(dataSrc, key string) {
 func (s *memStore) Reset(dataSrc string) {
 	delete(s.db, dataSrc)
 }
-
-type dataSrcKeyT string
-
-var dataSrcKey = dataSrcKeyT("dataSrc")
-
-func DataSrcContext(ctx context.Context, dataSrc string) context.Context {
-	return context.WithValue(ctx, dataSrcKey, dataSrc)
-}
-
-func DataSrcFromContext(ctx context.Context) (dataSrc string, ok bool) {
-	dataSrc, ok = ctx.Value(dataSrcKey).(string)
-	return
-}
diff --git a/plugins/orchestrator/watcher/aggregator.go b/plugins/orchestrator/watcher/aggregator.go
index 0c10860bfa..43d2cb701d 100644
--- a/plugins/orchestrator/watcher/aggregator.go
+++ b/plugins/orchestrator/watcher/aggregator.go
@@ -20,12 +20,14 @@ import (
 	"strings"
 
 	"go.ligato.io/cn-infra/v2/datasync"
+	"go.ligato.io/cn-infra/v2/datasync/kvdbsync"
 	"go.ligato.io/cn-infra/v2/datasync/kvdbsync/local"
 	"go.ligato.io/cn-infra/v2/datasync/resync"
 	"go.ligato.io/cn-infra/v2/datasync/syncbase"
 	"go.ligato.io/cn-infra/v2/infra"
 	"go.ligato.io/cn-infra/v2/logging"
 	"go.ligato.io/cn-infra/v2/utils/safeclose"
+	"go.ligato.io/vpp-agent/v3/plugins/orchestrator/localregistry"
 )
 
 // Option is a function that acts on a Plugin to inject Dependencies or configuration
@@ -92,6 +94,29 @@ func (p *Aggregator) Watch(
 			p.Log.Warn("found local registry (localclient) in watchers, ignoring it..")
 			continue
 		}
+		// ignoring watchers that have data sources that will be never used and
+		// therefore never send configuration data to this aggregator
+		if syncer, ok := w.(*kvdbsync.Plugin); ok {
+			if syncer.KvPlugin != nil && syncer.KvPlugin.Disabled() {
+				continue
+			}
+		}
+		// TODO Handle kvdbsync.Plugin watchers that are not disabled, but won't transmit any resync data.
+		//  This aggregator collects resyncs from all watchers, but if one or more resync from watcher don't happen,
+		//  the agregator resyncing won't be triggered. This might happen when one of watchers is not registered
+		//  to resync plugin and that is usually due to not connecting to data source as the registration to
+		//  resync plugin happen in OnConnect callback function.
+		//  To properly handle the situation, OnConnect callback must be used to distinguish what watched is
+		//  reached by resync trigger. That might lead to delayed readiness of all watchers and hence the trigger
+		//  for initial call of DoResync() to do initial Agent resync must be delayed as well SOMEHOW (can't use
+		//  init or after init of plugins).
+
+		// localregistry.NBInitFileRegistry can also be watcher that never sends anything (i.e. if misconfigured
+		// or no default NB init file is present or loading of data fails or ... -> check for Empty())
+		if initRegistry, ok := w.(*localregistry.NBInitFileRegistry); ok && initRegistry.Empty() {
+			continue
+		}
+
 		watchers = append(watchers, w)
 	}
 	p.Watchers = watchers
@@ -109,7 +134,7 @@ func (p *Aggregator) Watch(
 		partResync := make(chan datasync.ResyncEvent)
 
 		name := fmt.Sprint(adapter) + "/" + resyncName
-		watcherReg, err := adapter.Watch(name, changeChan, partResync, keyPrefixes...)
+		watcherReg, err := adapter.Watch(name, partChange, partResync, keyPrefixes...)
 		if err != nil {
 			return nil, err
 		}
@@ -286,7 +311,7 @@ func (p *Aggregator) watchLocalEvents(partChange, changeChan chan datasync.Chang
 			e.Done(nil)
 
 			p.Log.Debug("LOCAL watcher calling RESYNC")
-			p.Resync.DoResync()
+			p.Resync.DoResync() // execution will appear in p.watchAggrResync go routine where p.localKVs will handled
 		}
 	}
 }

From 3b2f0aee96bedfcea1b68ac14d4f2a157b51edff Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Tue, 8 Dec 2020 17:42:43 +0100
Subject: [PATCH 02/11] test: added tests for setting up NB configuration from
 file at VPP-Agent start

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 tests/e2e/000_initial_test.go  | 184 +++++++++++++++++++++++++++++++
 tests/e2e/100_agentctl_test.go |   4 +-
 tests/e2e/e2e.go               | 113 +++++++++++++++++++-
 tests/e2e/etcd.go              | 190 ++++++++++++++++++++++++++-------
 tests/e2e/run_e2e.sh           |   9 ++
 5 files changed, 458 insertions(+), 42 deletions(-)

diff --git a/tests/e2e/000_initial_test.go b/tests/e2e/000_initial_test.go
index 93ec1dc403..a58ad21583 100644
--- a/tests/e2e/000_initial_test.go
+++ b/tests/e2e/000_initial_test.go
@@ -15,6 +15,7 @@
 package e2e
 
 import (
+	"fmt"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -63,3 +64,186 @@ func TestStartStopAgent(t *testing.T) {
 	ctx.StopAgent(agent1)
 	Eventually(msState).Should(Equal(kvscheduler.ValueState_NONEXISTENT))
 }
+
+// TestNBInitFromFile test configuring initial state of NB from file
+func TestNBInitFromFile(t *testing.T) {
+	ctx := Setup(t, WithoutVPPAgent())
+	defer ctx.Teardown() // will teardown also VPP-Agent created later
+
+	// create NB init file content
+	initialNBConfig := `
+netallocConfig: {}
+linuxConfig: {}
+vppConfig:
+  interfaces:
+    - name: loop-test-from-init-file
+      type: SOFTWARE_LOOPBACK
+      enabled: true
+      ipAddresses:
+        - 10.10.1.1/24
+      mtu: 1500
+`
+	initialNBConfigFileName := CreateFile(t, "nb-init-file.yaml", initialNBConfig)
+
+	// create config content for NB init file usage
+	nbInitFileRegistryConfig := `
+disable-nb-initial-configuration: false
+nb-initial-configuration-file-path: %v
+`
+	nbInitFileRegistryConfig = fmt.Sprintf(nbInitFileRegistryConfig, initialNBConfigFileName)
+
+	// create VPP-Agent
+	SetupVPPAgent(t, ctx,
+		WithAdditionalAgentProcessParams(WithPluginConfigArg(t, "nbinitfileregistry", nbInitFileRegistryConfig)),
+		WithoutManualInitialAgentResync(),
+	)
+
+	// check whether NB initial configuration inside file is correctly loaded in running VPP-Agent
+	initInterfaceConfigState := func() kvscheduler.ValueState {
+		return ctx.GetValueStateByKey("vpp/interface/loop-test-from-init-file/address/static/10.10.1.1/24")
+	}
+	Eventually(initInterfaceConfigState).Should(Equal(kvscheduler.ValueState_CONFIGURED),
+		"loopback from init file was not properly created")
+}
+
+// TestNBInitFromEtcd test configuring initial state of NB from Etcd
+func TestNBInitFromEtcd(t *testing.T) {
+	ctx := Setup(t,
+		WithEtcd(),
+		WithoutVPPAgent(),
+	)
+	defer ctx.Teardown() // will teardown also VPP-Agent created later
+
+	// put NB config into Etcd
+	Expect(ctx.Etcd.Put(
+		"/vnf-agent/vpp1/config/vpp/v2/interfaces/loop-test-from-etcd",
+		`{"name":"loop-test-from-etcd","type":"SOFTWARE_LOOPBACK","enabled":true,"ip_addresses":["10.10.1.2/24"], "mtu":1500}`)).
+		To(Succeed(), "can't insert data into ETCD")
+
+	// prepare Etcd config for VPP-Agent
+	etcdConfig := `insecure-transport: true
+dial-timeout: 1s
+endpoints:
+    - "%v:2379"
+`
+	etcdConfig = fmt.Sprintf(etcdConfig, ctx.Etcd.Inspect().NetworkSettings.IPAddress)
+
+	// create VPP-Agent
+	SetupVPPAgent(t, ctx,
+		WithAdditionalAgentProcessParams(WithPluginConfigArg(t, "etcd", etcdConfig)),
+		WithoutManualInitialAgentResync(),
+	)
+
+	// check whether NB initial configuration is correctly loaded from Etcd in running VPP-Agent
+	initInterfaceConfigState := func() kvscheduler.ValueState {
+		return ctx.GetValueStateByKey("vpp/interface/loop-test-from-etcd/address/static/10.10.1.2/24")
+	}
+	Eventually(initInterfaceConfigState).Should(Equal(kvscheduler.ValueState_CONFIGURED),
+		"loopback from etcd was not properly created")
+}
+
+// TestNBInitFromFileAndEtcd test configuring initial state of NB from Etcd and from file
+func TestNBInitFromFileAndEtcd(t *testing.T) {
+	ctx := Setup(t,
+		WithEtcd(),
+		WithoutVPPAgent(),
+	)
+	defer ctx.Teardown() // will teardown also VPP-Agent created later
+
+	// put NB config into Etcd
+	Expect(ctx.Etcd.Put(
+		"/vnf-agent/vpp1/config/vpp/v2/interfaces/memif-from-etcd",
+		`{
+"name":"memif-from-etcd",
+"type":"MEMIF",
+"enabled":true,
+"ip_addresses":["10.10.1.1/32"], 
+"mtu":1500,
+"memif": {
+		"master": false,
+		"id": 1,
+		"socket_filename": "/run/vpp/default.sock"
+	}
+}`)).To(Succeed(), "can't insert data1 into ETCD")
+	Expect(ctx.Etcd.Put(
+		"/vnf-agent/vpp1/config/vpp/v2/interfaces/memif-from-both-sources",
+		`{
+"name":"memif-from-both-sources",
+"type":"MEMIF",
+"enabled":true,
+"ip_addresses":["10.10.1.3/32"], 
+"mtu":1500,
+"memif": {
+		"master": false,
+		"id": 3,
+		"socket_filename": "/run/vpp/default.sock"
+	}
+}`)).To(Succeed(), "can't insert data2 into ETCD")
+
+	// create NB init file content
+	initialNBConfig := `
+netallocConfig: {}
+linuxConfig: {}
+vppConfig:
+ interfaces:
+   - name: memif-from-init-file
+     type: MEMIF
+     enabled: true
+     ipAddresses:
+       - 10.10.1.2/32
+     mtu: 1500
+     memif:
+         master: false
+         id: 2
+         socketFilename: /run/vpp/default.sock
+   - name: memif-from-both-sources
+     type: MEMIF
+     enabled: true
+     ipAddresses:
+       - 10.10.1.4/32
+     mtu: 1500
+     memif:
+         master: false
+         id: 4
+         socketFilename: /run/vpp/default.sock
+`
+	initialNBConfigFileName := CreateFile(t, "nb-init-file.yaml", initialNBConfig)
+
+	// create config content for NB init file usage
+	nbInitFileRegistryConfig := `
+disable-nb-initial-configuration: false
+nb-initial-configuration-file-path: %v
+`
+	nbInitFileRegistryConfig = fmt.Sprintf(nbInitFileRegistryConfig, initialNBConfigFileName)
+
+	// create config content for etcd connection
+	etcdConfig := `insecure-transport: true
+dial-timeout: 1s
+endpoints:
+    - "%v:2379"
+`
+	etcdConfig = fmt.Sprintf(etcdConfig, ctx.Etcd.Inspect().NetworkSettings.IPAddress)
+
+	// create VPP-Agent
+	SetupVPPAgent(t, ctx,
+		WithAdditionalAgentProcessParams(WithPluginConfigArg(t, "etcd", etcdConfig),
+			WithPluginConfigArg(t, "nbinitfileregistry", nbInitFileRegistryConfig)),
+		WithoutManualInitialAgentResync(),
+	)
+
+	// check whether NB initial configuration is correctly loaded from Etcd in running VPP-Agent
+	initInterfaceConfigState := func(interfaceName string, ipAddress string) kvscheduler.ValueState {
+		return ctx.GetValueStateByKey(
+			fmt.Sprintf("vpp/interface/%v/address/static/%v/32", interfaceName, ipAddress))
+	}
+	Eventually(initInterfaceConfigState("memif-from-etcd", "10.10.1.1")).
+		Should(Equal(kvscheduler.ValueState_CONFIGURED),
+			"unique memif from etcd was not properly created")
+	Eventually(initInterfaceConfigState("memif-from-init-file", "10.10.1.2")).
+		Should(Equal(kvscheduler.ValueState_CONFIGURED),
+			"unique memif from init file was not properly created")
+	Eventually(initInterfaceConfigState("memif-from-both-sources", "10.10.1.3")).
+		Should(Equal(kvscheduler.ValueState_CONFIGURED),
+			"conflicting memif (defined in init file and etcd) was either not correctly "+
+				"merged (etcd data should have priority) or other things prevented its proper creation")
+}
diff --git a/tests/e2e/100_agentctl_test.go b/tests/e2e/100_agentctl_test.go
index f3f5238b06..20c158ebcf 100644
--- a/tests/e2e/100_agentctl_test.go
+++ b/tests/e2e/100_agentctl_test.go
@@ -365,10 +365,8 @@ func TestAgentCtlSecureGrpc(t *testing.T) {
 }
 
 func TestAgentCtlSecureETCD(t *testing.T) {
-	ctx := Setup(t)
+	ctx := Setup(t, WithEtcd(), WithEtcdHTTPsConnection(), WithEtcdVPPAgentContainerNetworking())
 	defer ctx.Teardown()
-	etcdID := ctx.StartEtcd()
-	defer ctx.StopEtcd(etcdID)
 
 	// test without any TLS
 	t.Run("no TLS", func(t *testing.T) {
diff --git a/tests/e2e/e2e.go b/tests/e2e/e2e.go
index 161f184631..f50308ba93 100644
--- a/tests/e2e/e2e.go
+++ b/tests/e2e/e2e.go
@@ -19,8 +19,12 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"io"
+	"io/ioutil"
 	"log"
 	"os"
+	"os/exec"
+	"path/filepath"
 	"regexp"
 	"strings"
 	"testing"
@@ -66,7 +70,17 @@ const (
 	//memifInputNode    = "memif-input"
 )
 
+// Setup options constants
+const (
+	NoManualInitialAgentResync   = "NoManualInitialAgentResync"
+	AdditionalAgentProcessParams = "AdditionalAgentProcessParams"
+	DontSetupVPPAgent            = "DontSetupVPPAgent"
+	SetupEtcdContainer           = "SetupEtcdContainer"
+)
+
 type TestCtx struct {
+	Etcd *EtcdContainer // TODO change?
+
 	t      *testing.T
 	ctx    context.Context
 	cancel context.CancelFunc
@@ -85,7 +99,13 @@ type TestCtx struct {
 	logger    *log.Logger
 }
 
-func NewTest(t *testing.T) *TestCtx {
+// Option is key-value pair for customizing setup of tests
+type Option struct {
+	key   string
+	value interface{}
+}
+// TODO recheck options due to rebasing mess
+func NewTest(t *testing.T, options ...*Option) *TestCtx {
 	RegisterTestingT(t)
 	// TODO: Do not use global test registration.
 	//  It is now deprecated and you should use NewWithT() instead.
@@ -93,6 +113,8 @@ func NewTest(t *testing.T) *TestCtx {
 
 	logrus.Debugf("Environ:\n%v", strings.Join(os.Environ(), "\n"))
 
+	optionsMap := optionsMap(options)
+
 	SetDefaultEventuallyPollingInterval(checkPollingInterval)
 	SetDefaultEventuallyTimeout(checkTimeout)
 
@@ -112,6 +134,19 @@ func NewTest(t *testing.T) *TestCtx {
 	return te
 }
 
+// TODO resolver vppagent mess
+//if _, found := optionsMap[DontSetupVPPAgent]; !found {
+//  SetupVPPAgent(t, testCtx, options...)
+//}
+//
+//optionsMap := optionsMap(options)
+//
+//var agentArgs []string
+//if additionalParams, found := optionsMap[AdditionalAgentProcessParams]; found {
+//agentArgs = additionalParams.([]string)
+//}
+
+
 func Setup(t *testing.T) *TestCtx {
 	var err error
 
@@ -159,7 +194,9 @@ func Setup(t *testing.T) *TestCtx {
 	Eventually(testCtx.checkAgentReady, agentInitTimeout, checkPollingInterval).Should(Succeed())
 
 	// run initial resync
-	testCtx.syncAgent()
+	if _, found := optionsMap[NoManualInitialAgentResync]; !found {
+		testCtx.syncAgent()
+	}
 
 	if version, err := testCtx.ExecVppctl("show version"); err != nil {
 		t.Fatalf("Retrieving VPP version via vppctl failed: %v", err)
@@ -173,9 +210,23 @@ func Setup(t *testing.T) *TestCtx {
 		}
 	}
 
+	// setup Etcd
+	if _, found := optionsMap[SetupEtcdContainer]; found {
+		testCtx.Etcd = NewEtcdContainer(testCtx, options...)
+	}
+
 	return testCtx
 }
 
+func optionsMap(options []*Option) map[string]interface{} {
+	// convert options to map
+	optionsMap := make(map[string]interface{})
+	for _, option := range options {
+		optionsMap[option.key] = option.value
+	}
+	return optionsMap
+}
+
 func (test *TestCtx) Teardown() {
 	if test.t.Failed() || *debug {
 		defer test.dumpLog()
@@ -196,9 +247,15 @@ func (test *TestCtx) Teardown() {
 		test.t.Logf("closing the client failed: %v", err)
 	}
 
+	// TODO check fon non-nil?
 	if err := test.agent.stop(); err != nil {
 		test.t.Logf("failed to stop vpp-agent: %v", err)
 	}
+
+	// terminate etcd
+	if test.Etcd != nil {
+		test.Etcd.Terminate(test)
+	}
 }
 
 func (test *TestCtx) dumpLog() {
@@ -215,6 +272,58 @@ func (test *TestCtx) VppRelease() string {
 	return test.vppVersion
 }
 
+// WithoutManualInitialAgentResync is test setup option disabling manual agent resync just after agent setup
+func WithoutManualInitialAgentResync() *Option {
+	return &Option{
+		key:   NoManualInitialAgentResync,
+		value: struct{}{}, // only presence is needed
+	}
+}
+
+// WithAdditionalAgentProcessParams is test setup option adding additional parameters to executing vpp-agent process
+func WithAdditionalAgentProcessParams(params ...string) *Option {
+	return &Option{
+		key:   AdditionalAgentProcessParams,
+		value: params,
+	}
+}
+
+// WithPluginConfigArg persists configContent for give VPP-Agent plugin (expecting generic plugin config name)
+// and returns argument for VPP-Agent executable to use this plugin configuration file.
+func WithPluginConfigArg(t *testing.T, pluginName string, configContent string) string {
+	configFilePath := CreateFile(t, fmt.Sprintf("%v.config", pluginName), configContent)
+	return fmt.Sprintf("-%v-config=%v", pluginName, configFilePath)
+}
+
+// CreateFile persists fileContent to file in OS temp directory. It returns the absolute path to the newly
+// created file.
+func CreateFile(t *testing.T, simpleFileName string, fileContent string) string {
+	filePath, err := filepath.Abs(filepath.Join(os.TempDir(), fmt.Sprintf("e2e-test-%v-%v", t.Name(), simpleFileName)))
+	Expect(err).To(Not(HaveOccurred()))
+	Expect(ioutil.WriteFile(filePath, []byte(fileContent), 0777)).To(Succeed())
+
+	// TODO register in context and delete in teardown? this doesn't matter
+	//  that much because file names contain unique test names so no file collision can happen
+	return filePath
+}
+
+// WithoutVPPAgent is test setup option disabling vpp-agent setup
+func WithoutVPPAgent() *Option {
+	return &Option{
+		key:   DontSetupVPPAgent,
+		value: struct{}{}, // only presence is needed
+	}
+}
+
+// WithEtcd is test setup option enabling vpp-agent setup
+func WithEtcd() *Option {
+	return &Option{
+		key:   SetupEtcdContainer,
+		value: struct{}{}, // only presence is needed
+	}
+}
+
+
 func (test *TestCtx) GenericClient() client.GenericClient {
 	c, err := test.agentClient.GenericClient()
 	if err != nil {
diff --git a/tests/e2e/etcd.go b/tests/e2e/etcd.go
index a63a7ce7f9..ea2884b806 100644
--- a/tests/e2e/etcd.go
+++ b/tests/e2e/etcd.go
@@ -15,73 +15,189 @@
 package e2e
 
 import (
-	"path/filepath"
+	"bytes"
+	"os"
 
 	docker "github.com/fsouza/go-dockerclient"
 )
 
-func (test *TestCtx) pullEtcd() {
-	err := test.dockerClient.PullImage(docker.PullImageOptions{
+// Setup options constants
+const (
+	HTTPsConnection             = "HTTPsConnection"
+	VPPAgentContainerNetworking = "VPPAgentContainerNetworking"
+)
+
+// EtcdContainer is represents running ETCD container
+type EtcdContainer struct {
+	ctx         *TestCtx
+	containerID string
+}
+
+// NewEtcdContainer creates and starts new ETCD container
+func NewEtcdContainer(ctx *TestCtx, options ...*Option) *EtcdContainer {
+	ec := &EtcdContainer{
+		ctx: ctx,
+	}
+	container := ec.create(ctx, options...)
+	ec.start(ctx, container)
+	ec.containerID = container.ID
+	return ec
+}
+
+// Put inserts key-value pair into the ETCD inside its running docker container
+func (ec *EtcdContainer) Put(key string, value string) error {
+	_, err := ec.exec("etcdctl", "put", key, value)
+	return err
+}
+
+// Get retrieves value for the key from the ETCD that is running in its docker container
+func (ec *EtcdContainer) Get(key string) (string, error) {
+	return ec.exec("etcdctl", "get", key)
+}
+
+// Get retrieves all key-value pairs from the ETCD that is running in its docker container
+func (ec *EtcdContainer) GetAll() (string, error) {
+	return ec.exec("etcdctl", "get", "", "--prefix=true")
+}
+
+// Inspect provides docker.Container of running ETCD container that can be
+// used to inspect various things about ETCD container
+func (ec *EtcdContainer) Inspect() *docker.Container {
+	container, err := ec.ctx.dockerClient.InspectContainer(ec.containerID)
+	if err != nil {
+		ec.ctx.t.Fatalf("failed to inspect container with ID %v due to: %v", ec.containerID, err)
+	}
+	return container
+}
+
+func (ec *EtcdContainer) create(ctx *TestCtx, options ...*Option) *docker.Container {
+	optionsMap := optionsMap(options)
+
+	// pull image
+	err := ctx.dockerClient.PullImage(docker.PullImageOptions{
 		Repository: etcdImage,
 		Tag:        "latest",
 	}, docker.AuthConfiguration{})
 	if err != nil {
-		test.t.Fatalf("failed to pull ETCD image: %v", err)
+		ctx.t.Fatalf("failed to pull ETCD image: %v", err)
+	}
+
+	// construct command string and container host config
+	cmd := []string{
+		"/usr/local/bin/etcd",
+	}
+	hostConfig := &docker.HostConfig{}
+	if _, found := optionsMap[HTTPsConnection]; found {
+		cmd = append(cmd,
+			"--client-cert-auth",
+			"--trusted-ca-file=/etc/certs/ca.pem",
+			"--cert-file=/etc/certs/cert1.pem",
+			"--key-file=/etc/certs/cert1-key.pem",
+			"--advertise-client-urls=https://127.0.0.1:2379",
+			"--listen-client-urls=https://127.0.0.1:2379",
+		)
+		hostConfig.Binds = []string{os.Getenv("CERTS_PATH") + ":/etc/certs"}
+	} else { // HTTP connection
+		cmd = append(cmd,
+			"--advertise-client-urls=http://0.0.0.0:2379",
+			"--listen-client-urls=http://0.0.0.0:2379",
+		)
+	}
+	if _, found := optionsMap[VPPAgentContainerNetworking]; found {
+		hostConfig.NetworkMode = "container:vpp-agent-e2e-test"
+	} else { // separate container networking (default)
+		hostConfig.PortBindings = map[docker.Port][]docker.PortBinding{
+			"2379/tcp": {{HostIP: "0.0.0.0", HostPort: "2379"}},
+		}
 	}
-}
 
-func (test *TestCtx) StartEtcd() string {
-	container, err := test.dockerClient.CreateContainer(docker.CreateContainerOptions{
+	// create container
+	container, err := ctx.dockerClient.CreateContainer(docker.CreateContainerOptions{
 		Name: "e2e-test-etcd",
 		Config: &docker.Config{
-			Image: etcdImage,
 			Env:   []string{"ETCDCTL_API=3"},
-			Cmd: []string{
-				"/usr/local/bin/etcd",
-				"--client-cert-auth",
-				"--trusted-ca-file=/etc/certs/ca.pem",
-				"--cert-file=/etc/certs/cert1.pem",
-				"--key-file=/etc/certs/cert1-key.pem",
-				"--advertise-client-urls=https://127.0.0.1:2379",
-				"--listen-client-urls=https://127.0.0.1:2379",
-			},
-		},
-		HostConfig: &docker.HostConfig{
-			NetworkMode: "container:vpp-agent-e2e-test",
-			Binds: []string{
-				filepath.Join(test.testDataDir, "certs") + ":/etc/certs:ro",
-			},
+			Image: etcdImage,
+			Cmd:   cmd,
 		},
+		HostConfig: hostConfig,
 	})
 	if err != nil {
-		test.t.Fatalf("failed to create ETCD container: %v", err)
+		ctx.t.Fatalf("failed to create ETCD container: %v", err)
+	}
+	return container
+}
+
+// WithEtcdHTTPsConnection is ETCD test setup option that will use HTTPS connection to ETCD (by default it is used
+// unsecure HTTP connection)
+func WithEtcdHTTPsConnection() *Option {
+	return &Option{
+		key:   HTTPsConnection,
+		value: struct{}{}, // only presence is needed
+	}
+}
+
+// WithEtcdVPPAgentContainerNetworking is ETCD test setup option that will use VPP-Agent test container for
+// networking (by default the ETCD has separate networking)
+func WithEtcdVPPAgentContainerNetworking() *Option {
+	return &Option{
+		key:   VPPAgentContainerNetworking,
+		value: struct{}{}, // only presence is needed
 	}
-	err = test.dockerClient.StartContainer(container.ID, nil)
+}
+
+func (ec *EtcdContainer) start(ctx *TestCtx, container *docker.Container) {
+	err := ctx.dockerClient.StartContainer(container.ID, nil)
 	if err != nil {
-		err = test.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
+		err = ctx.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
 			ID:    container.ID,
 			Force: true,
 		})
 		if err != nil {
-			test.t.Errorf("failed to remove ETCD container: %v", err)
+			ctx.t.Errorf("failed to remove ETCD container: %v", err)
 		}
-		test.t.Fatalf("failed to start ETCD container: %v", err)
+		ctx.t.Fatalf("failed to start ETCD container: %v", err)
 	}
-	test.t.Logf("started ETCD container %v", container.ID)
-	return container.ID
+	ctx.t.Logf("started ETCD container %v", container.ID)
 }
 
-func (test *TestCtx) StopEtcd(id string) {
-	err := test.dockerClient.StopContainer(id, msStopTimeout)
+// Terminate stops and removes the ETCD container
+func (ec *EtcdContainer) Terminate(ctx *TestCtx) {
+	ec.stop(ctx)
+	ec.remove(ctx)
+}
+
+func (ec *EtcdContainer) stop(ctx *TestCtx) {
+	err := ctx.dockerClient.StopContainer(ec.containerID, msStopTimeout)
 	if err != nil {
-		test.t.Logf("failed to stop ETCD container: %v", err)
+		ctx.t.Logf("failed to stop ETCD container: %v", err)
 	}
-	err = test.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
-		ID:    id,
+}
+
+func (ec *EtcdContainer) remove(ctx *TestCtx) {
+	err := ctx.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
+		ID:    ec.containerID,
 		Force: true,
 	})
 	if err != nil {
-		test.t.Fatalf("failed to remove ETCD container: %v", err)
+		ctx.t.Fatalf("failed to remove ETCD container: %v", err)
 	}
-	test.t.Logf("removed ETCD container %v", id)
+	ctx.t.Logf("removed ETCD container %v", ec.containerID)
+}
+
+// exec executes command inside Etcd container
+func (ec *EtcdContainer) exec(cmdName string, args ...string) (output string, err error) {
+	execCtx, err := ec.ctx.dockerClient.CreateExec(docker.CreateExecOptions{
+		AttachStdout: true,
+		Cmd:          append([]string{cmdName}, args...),
+		Container:    ec.containerID,
+	})
+	if err != nil {
+		ec.ctx.t.Fatalf("failed to create docker exec instance for exec in etcd container: %v", err)
+	}
+
+	var stdout bytes.Buffer
+	err = ec.ctx.dockerClient.StartExec(execCtx.ID, docker.StartExecOptions{
+		OutputStream: &stdout,
+	})
+	return stdout.String(), err
 }
diff --git a/tests/e2e/run_e2e.sh b/tests/e2e/run_e2e.sh
index b10e825a26..c44216c822 100755
--- a/tests/e2e/run_e2e.sh
+++ b/tests/e2e/run_e2e.sh
@@ -14,6 +14,7 @@ export DOCKER_BUILDKIT=1
 
 testname="vpp-agent-e2e-test"
 imgname="vpp-agent-e2e-tests"
+etcdcontainername="e2e-test-etcd"
 
 # Compile agentctl for testing
 go build -o ./tests/e2e/agentctl.test \
@@ -46,6 +47,14 @@ cleanup() {
 	set -x
 	docker stop -t 1 "${testname}" 2>/dev/null
 	docker rm -v "${testname}" 2>/dev/null
+	set +x
+
+  echo "Stopping etcd container if running"
+  if [ "$(docker ps -a | grep "${etcdcontainername}")" ]; then
+    set -x
+    docker stop -t 1 "${etcdcontainername}" 2>/dev/null
+    docker rm -v "${etcdcontainername}" 2>/dev/null
+  fi
 }
 
 trap 'cleanup' EXIT

From f411708eb86ecbd32aa59a8c6e79cd34ac4ee5fd Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Tue, 8 Dec 2020 19:33:22 +0100
Subject: [PATCH 03/11] refactor: performance enhancement by using proto.Mege
 for conversion of dynamic proto message to generated proto message

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 pkg/models/registry.go | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/pkg/models/registry.go b/pkg/models/registry.go
index 55ded31a4d..da2b81f40b 100644
--- a/pkg/models/registry.go
+++ b/pkg/models/registry.go
@@ -21,7 +21,6 @@ import (
 	"strings"
 
 	"github.com/go-errors/errors"
-	"github.com/golang/protobuf/jsonpb"
 	"github.com/golang/protobuf/proto"
 	protoV2 "google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
@@ -267,16 +266,8 @@ func dynamicMessageToRegisteredGoType(dynamicMessage *dynamicpb.Message, goType
 	}
 
 	// fill empty proto message with data from its dynamic proto message counterpart
-	// (dynamic proto message -> json -> registered-type proto message)
-	marshaller := jsonpb.Marshaler{EmitDefaults: true} // using jsonbp to generate json with json name field in proto tag
-	jsonData, err := marshaller.MarshalToString(dynamicMessage)
-	if err != nil {
-		return nil, errors.Errorf("can't marshall dynamic proto message "+
-			"to json due to: %v (message: %+v)", err, dynamicMessage)
-	}
-	if err := jsonpb.Unmarshal(strings.NewReader(jsonData), message); err != nil {
-		return nil, errors.Errorf("can't load json to registered-type "+
-			"proto message due to: %v (json=%v)", err, jsonData)
-	}
+	// (alternative approach to this is marshalling dynamicMessage to json and unmarshalling it back to message)
+	proto.Merge(message, dynamicMessage)
+
 	return message, nil
 }

From 40c0b53f2146120a49051eaa40bb6f59a21d63b8 Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Wed, 9 Dec 2020 18:29:30 +0100
Subject: [PATCH 04/11] fix: enhanced the logging and error passing for init
 file registry config loading

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 plugins/orchestrator/localregistry/nbinitfileregistry.go | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/plugins/orchestrator/localregistry/nbinitfileregistry.go b/plugins/orchestrator/localregistry/nbinitfileregistry.go
index 9fe1c27bcf..381d9e4a0e 100644
--- a/plugins/orchestrator/localregistry/nbinitfileregistry.go
+++ b/plugins/orchestrator/localregistry/nbinitfileregistry.go
@@ -171,8 +171,12 @@ func (r *NBInitFileRegistry) retrieveConfig() (*Config, error) {
 	}
 	found, err := r.Cfg.LoadValue(config)
 	if !found {
-		r.Log.Debug("NBInitFileRegistry plugin config not found")
-		return config, nil
+		if err == nil {
+			r.Log.Debug("NBInitFileRegistry plugin config not found")
+		} else {
+			r.Log.Debugf("NBInitFileRegistry plugin config can't be loaded due to: %v", err)
+		}
+		return config, err
 	}
 	if err != nil {
 		return nil, err

From 5afb768524c6f650e701fa9fdad564eb7480fe63 Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Wed, 9 Dec 2020 18:30:56 +0100
Subject: [PATCH 05/11] fix: fixed model instance name retrieval for dynamic
 proto messages of locally registered models

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 pkg/models/model.go        |  2 +-
 pkg/models/models.go       | 34 ++++++++++++++++++++++++++++++++++
 pkg/models/options.go      | 20 ++++++++++++++++++--
 pkg/models/registry.go     | 35 ++---------------------------------
 pkg/models/remote_model.go |  2 +-
 5 files changed, 56 insertions(+), 37 deletions(-)

diff --git a/pkg/models/model.go b/pkg/models/model.go
index 1d0497ac7e..12cbb582ae 100644
--- a/pkg/models/model.go
+++ b/pkg/models/model.go
@@ -150,5 +150,5 @@ func (m *LocallyKnownModel) InstanceName(x interface{}) (string, error) {
 	if m.nameFunc == nil {
 		return "", nil
 	}
-	return m.nameFunc(x)
+	return m.nameFunc(x, m.goType)
 }
diff --git a/pkg/models/models.go b/pkg/models/models.go
index ee2ace18f4..81c993c20f 100644
--- a/pkg/models/models.go
+++ b/pkg/models/models.go
@@ -16,10 +16,13 @@ package models
 
 import (
 	"path"
+	"reflect"
 	"strings"
 
 	"github.com/go-errors/errors"
 	"github.com/golang/protobuf/proto"
+	protoV2 "google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/dynamicpb"
 )
 
 // Register registers model in DefaultRegistry.
@@ -136,3 +139,34 @@ func keyPrefix(modelSpec Spec, hasTemplateName bool) string {
 	}
 	return keyPrefix
 }
+
+// dynamicMessageToGeneratedMessage converts proto dynamic message to corresponding generated proto message
+// (identified by go type).
+// This conversion method should help handling dynamic proto messages in mostly protoc-generated proto message
+// oriented codebase (i.e. help for type conversions to named, help handle missing data fields as seen
+// in generated proto messages,...)
+func dynamicMessageToGeneratedMessage(dynamicMessage *dynamicpb.Message,
+	goTypeOfGeneratedMessage reflect.Type) (proto.Message, error) {
+
+	// create empty proto message of the same type as it was used for registration
+	var registeredGoType interface{}
+	if goTypeOfGeneratedMessage.Kind() == reflect.Ptr {
+		registeredGoType = reflect.New(goTypeOfGeneratedMessage.Elem()).Interface()
+	} else {
+		registeredGoType = reflect.Zero(goTypeOfGeneratedMessage).Interface()
+	}
+	message, isProtoV1 := registeredGoType.(proto.Message)
+	if !isProtoV1 {
+		messageV2, isProtoV2 := registeredGoType.(protoV2.Message)
+		if !isProtoV2 {
+			return nil, errors.Errorf("registered go type(%T) is not proto.Message", registeredGoType)
+		}
+		message = proto.MessageV1(messageV2)
+	}
+
+	// fill empty proto message with data from its dynamic proto message counterpart
+	// (alternative approach to this is marshalling dynamicMessage to json and unmarshalling it back to message)
+	proto.Merge(message, dynamicMessage)
+
+	return message, nil
+}
diff --git a/pkg/models/options.go b/pkg/models/options.go
index f7477af4a8..8a3615ffea 100644
--- a/pkg/models/options.go
+++ b/pkg/models/options.go
@@ -2,8 +2,11 @@ package models
 
 import (
 	"net"
+	"reflect"
 	"strings"
 	"text/template"
+
+	"google.golang.org/protobuf/types/dynamicpb"
 )
 
 type modelOptions struct {
@@ -15,7 +18,10 @@ type modelOptions struct {
 type ModelOption func(*modelOptions)
 
 // NameFunc represents function which can name model instance.
-type NameFunc func(obj interface{}) (string, error)
+// To properly handle also dynamic Messages (dynamicpb.Message)
+// as model instances, the go type of corresponding generated
+// proto message must be given.
+type NameFunc func(obj interface{}, messageGoType reflect.Type) (string, error)
 
 // WithNameTemplate returns option for models which sets function
 // for generating name of instances using custom template.
@@ -36,7 +42,17 @@ func NameTemplate(t string) NameFunc {
 	tmpl := template.Must(
 		template.New("name").Funcs(funcMap).Option("missingkey=error").Parse(t),
 	)
-	return func(obj interface{}) (string, error) {
+	return func(obj interface{}, messageGoType reflect.Type) (string, error) {
+		// handling dynamic messages (they don't have data fields as generated proto messages)
+		if dynMessage, ok := obj.(*dynamicpb.Message); ok {
+			var err error
+			obj, err = dynamicMessageToGeneratedMessage(dynMessage, messageGoType)
+			if err != nil {
+				return "", err
+			}
+		}
+
+		// execute name template on generated proto message
 		var s strings.Builder
 		if err := tmpl.Execute(&s, obj); err != nil {
 			return "", err
diff --git a/pkg/models/registry.go b/pkg/models/registry.go
index da2b81f40b..e5e14820f5 100644
--- a/pkg/models/registry.go
+++ b/pkg/models/registry.go
@@ -20,9 +20,7 @@ import (
 	"reflect"
 	"strings"
 
-	"github.com/go-errors/errors"
 	"github.com/golang/protobuf/proto"
-	protoV2 "google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/reflect/protoregistry"
 	"google.golang.org/protobuf/types/dynamicpb"
@@ -214,10 +212,10 @@ func (r *LocalRegistry) Register(x interface{}, spec Spec, opts ...ModelOption)
 
 	// Use GetName as fallback for generating name
 	if _, ok := x.(named); ok {
-		model.nameFunc = func(obj interface{}) (s string, e error) {
+		model.nameFunc = func(obj interface{}, messageGoType reflect.Type) (s string, e error) {
 			// handling dynamic messages (they don't implement named interface)
 			if dynMessage, ok := obj.(*dynamicpb.Message); ok {
-				obj, e = dynamicMessageToRegisteredGoType(dynMessage, model.goType)
+				obj, e = dynamicMessageToGeneratedMessage(dynMessage, messageGoType)
 				if e != nil {
 					return "", e
 				}
@@ -242,32 +240,3 @@ func (r *LocalRegistry) Register(x interface{}, spec Spec, opts ...ModelOption)
 	}
 	return model, nil
 }
-
-// dynamicMessageToRegisteredGoType converts proto dynamic message to proto message that was used at
-// the registration of the proto model corresponding to the proto dynamic message.
-// The registration proto message is usually the protoc-generated proto message. This conversion method
-// should help handling dynamic proto messages in mostly protoc-generated proto message oriented codebase
-// (i.e. help for type conversions to named, help handle missing data fields as seen in generated proto messages,...)
-func dynamicMessageToRegisteredGoType(dynamicMessage *dynamicpb.Message, goType reflect.Type) (proto.Message, error) {
-	// create empty proto message of the same type as it was used for registration
-	var registeredGoType interface{}
-	if goType.Kind() == reflect.Ptr {
-		registeredGoType = reflect.New(goType.Elem()).Interface()
-	} else {
-		registeredGoType = reflect.Zero(goType).Interface()
-	}
-	message, isProtoV1 := registeredGoType.(proto.Message)
-	if !isProtoV1 {
-		messageV2, isProtoV2 := registeredGoType.(protoV2.Message)
-		if !isProtoV2 {
-			return nil, errors.Errorf("registered go type(%T) is not proto.Message", registeredGoType)
-		}
-		message = proto.MessageV1(messageV2)
-	}
-
-	// fill empty proto message with data from its dynamic proto message counterpart
-	// (alternative approach to this is marshalling dynamicMessage to json and unmarshalling it back to message)
-	proto.Merge(message, dynamicMessage)
-
-	return message, nil
-}
diff --git a/pkg/models/remote_model.go b/pkg/models/remote_model.go
index 44b6425629..ccadaa3b30 100644
--- a/pkg/models/remote_model.go
+++ b/pkg/models/remote_model.go
@@ -155,7 +155,7 @@ func (m *RemotelyKnownModel) InstanceName(x interface{}) (string, error) {
 		return "", errors.Errorf("can't load json of marshalled "+
 			"message to generic map due to: %v (json=%v)", err, jsonData)
 	}
-	name, err := NameTemplate(nameTemplate)(mapData)
+	name, err := NameTemplate(nameTemplate)(mapData, nil)
 	if err != nil {
 		return "", errors.Errorf("can't compute name from name template by applying generic map "+
 			"due to: %v (name template=%v, generic map=%v)", err, nameTemplate, mapData)

From b605636bdc0f88d16e28bf34bf3c5ef3f3ef898a Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Thu, 10 Dec 2020 11:35:17 +0100
Subject: [PATCH 06/11] refactoring: removing NB prefix from config files and
 structs

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 cmd/vpp-agent/app/vpp_agent.go                |  2 +-
 ...nitfileregistry.go => initfileregistry.go} | 72 +++++++++----------
 plugins/orchestrator/watcher/aggregator.go    |  6 +-
 tests/e2e/000_initial_test.go                 | 50 ++++++-------
 4 files changed, 65 insertions(+), 65 deletions(-)
 rename plugins/orchestrator/localregistry/{nbinitfileregistry.go => initfileregistry.go} (79%)

diff --git a/cmd/vpp-agent/app/vpp_agent.go b/cmd/vpp-agent/app/vpp_agent.go
index 93313583bc..21b0a7ecde 100644
--- a/cmd/vpp-agent/app/vpp_agent.go
+++ b/cmd/vpp-agent/app/vpp_agent.go
@@ -101,7 +101,7 @@ func New() *VPPAgent {
 	)
 
 	// Set watcher for KVScheduler.
-	initFileRegistry := localregistry.NewNBInitFileRegistryPlugin()
+	initFileRegistry := localregistry.NewInitFileRegistryPlugin()
 	watchers := watcher.NewPlugin(watcher.UseWatchers(
 		local.DefaultRegistry,
 		initFileRegistry,
diff --git a/plugins/orchestrator/localregistry/nbinitfileregistry.go b/plugins/orchestrator/localregistry/initfileregistry.go
similarity index 79%
rename from plugins/orchestrator/localregistry/nbinitfileregistry.go
rename to plugins/orchestrator/localregistry/initfileregistry.go
index 381d9e4a0e..d7724ca6a8 100644
--- a/plugins/orchestrator/localregistry/nbinitfileregistry.go
+++ b/plugins/orchestrator/localregistry/initfileregistry.go
@@ -35,13 +35,13 @@ import (
 )
 
 const (
-	registryName          = "nb-init-file-registry"
-	defaultNBInitFilePath = "nb-initial-config.yaml"
+	registryName        = "init-file-registry"
+	defaultInitFilePath = "initial-config.yaml"
 )
 
-type Option func(*NBInitFileRegistry)
+type Option func(*InitFileRegistry)
 
-// NBInitFileRegistry is local read-only NB configuration provider with exclusive data source from a file
+// InitFileRegistry is local read-only NB configuration provider with exclusive data source from a file
 // given by a file path (InitConfigFilePath). Its purpose is to seamlessly integrated NB configuration
 // from file as another NB configuration provider (to existing providers: etcd, consul, redis) and integrate
 // it's configuration into agent in the same standard way(datasync.KeyValProtoWatcher). The content of this
@@ -49,14 +49,14 @@ type Option func(*NBInitFileRegistry)
 // inside given file after initial content loading.
 //
 // The NB configuration provisioning process and how this registry fits into it:
-// 	1. NB data sources register to default resync plugin (NBInitFileRegistry registers too in watchNBResync(),
+// 	1. NB data sources register to default resync plugin (InitFileRegistry registers too in watchNBResync(),
 //	   but only when there are some NB config data from file, otherwise it makes no sense to register because
 //	   there is nothing to forward. This also means that before register to resync plugin, the NB config from
 //	   file will be preloaded)
-// 	2. Call to resync plugin's DoResync triggers resync to NB configuration sources (NBInitFileRegistry takes
+// 	2. Call to resync plugin's DoResync triggers resync to NB configuration sources (InitFileRegistry takes
 //	   its preloaded NB config and stores it into another inner local registry)
 // 	3. NB configuration sources are also watchable (datasync.KeyValProtoWatcher) and the resync data is
-//	   collected by the watcher.Aggregator (NBInitFileRegistry is also watchable/forwards data to watcher.Aggregator,
+//	   collected by the watcher.Aggregator (InitFileRegistry is also watchable/forwards data to watcher.Aggregator,
 //	   it relies on the watcher capabilities of its inner local registry. This is the cause why to preloaded
 //	   the NB config from file([]proto.Message storage) and push it to another inner local storage later
 //	   (syncbase.Registry). If we used only one storage (syncbase.Registry for its watch capabilities), we
@@ -66,7 +66,7 @@ type Option func(*NBInitFileRegistry)
 // 	4. watcher.Aggregator merges all collected resync data and forwards them its watch clients (it also implements
 //	   datasync.KeyValProtoWatcher just like the NB data sources).
 //  5. Clients of Aggregator (currently orchestrator and ifplugin) handle the NB changes/resync properly.
-type NBInitFileRegistry struct {
+type InitFileRegistry struct {
 	infra.PluginDeps
 
 	initialized             bool
@@ -76,17 +76,17 @@ type NBInitFileRegistry struct {
 	preloadedNBConfigs      []proto.Message
 }
 
-// Config holds the NBInitFileRegistry configuration.
+// Config holds the InitFileRegistry configuration.
 type Config struct {
-	DisableNBInitialConfiguration  bool   `json:"disable-nb-initial-configuration"`
-	NBInitialConfigurationFilePath string `json:"nb-initial-configuration-file-path"`
+	DisableInitialConfiguration  bool   `json:"disable-initial-configuration"`
+	InitialConfigurationFilePath string `json:"initial-configuration-file-path"`
 }
 
-// NewNBInitFileRegistryPlugin creates a new Plugin with the provides Options
-func NewNBInitFileRegistryPlugin(opts ...Option) *NBInitFileRegistry {
-	p := &NBInitFileRegistry{}
+// NewInitFileRegistryPlugin creates a new InitFileRegistry Plugin with the provided Options
+func NewInitFileRegistryPlugin(opts ...Option) *InitFileRegistry {
+	p := &InitFileRegistry{}
 
-	p.PluginName = "nbinitfileregistry"
+	p.PluginName = "initfileregistry"
 	p.watchedRegistry = syncbase.NewRegistry()
 
 	for _, o := range opts {
@@ -94,7 +94,7 @@ func NewNBInitFileRegistryPlugin(opts ...Option) *NBInitFileRegistry {
 	}
 	if p.Cfg == nil {
 		p.Cfg = config.ForPlugin(p.String(),
-			config.WithCustomizedFlag(config.FlagName(p.String()), "nbinitfileregistryplugin.conf"),
+			config.WithCustomizedFlag(config.FlagName(p.String()), "initfileregistry.conf"),
 		)
 	}
 	p.PluginDeps.SetupLog()
@@ -103,7 +103,7 @@ func NewNBInitFileRegistryPlugin(opts ...Option) *NBInitFileRegistry {
 }
 
 // Init initialize registry
-func (r *NBInitFileRegistry) Init() error {
+func (r *InitFileRegistry) Init() error {
 	if !r.initialized {
 		return r.initialize()
 	}
@@ -114,10 +114,10 @@ func (r *NBInitFileRegistry) Init() error {
 // (readonly, will be filled only once from initial file import), this method directly indicates whether
 // the watchers of this registry will receive any data (Empty() == false, receive initial resync) or
 // won't receive anything at all (Empty() == true)
-func (r *NBInitFileRegistry) Empty() bool {
+func (r *InitFileRegistry) Empty() bool {
 	if !r.initialized { // could be called from init of other plugins -> possibly before this plugin init
 		if err := r.initialize(); err != nil {
-			r.Log.Errorf("can't initialize NBInitFileRegistry due to: %v", err)
+			r.Log.Errorf("can't initialize InitFileRegistry due to: %v", err)
 		}
 	}
 	return len(r.preloadedNBConfigs) == 0
@@ -127,14 +127,14 @@ func (r *NBInitFileRegistry) Empty() bool {
 // whether any data will be pushed to them at all (i.e. watcher.Aggregator). They should use the
 // Empty() method to find out whether there are (=ever will be do to nature of this registry) any
 // data for pushing to watchers.
-func (r *NBInitFileRegistry) Watch(resyncName string, changeChan chan datasync.ChangeEvent,
+func (r *InitFileRegistry) Watch(resyncName string, changeChan chan datasync.ChangeEvent,
 	resyncChan chan datasync.ResyncEvent, keyPrefixes ...string) (datasync.WatchRegistration, error) {
 	return r.watchedRegistry.Watch(resyncName, changeChan, resyncChan, keyPrefixes...)
 }
 
 // initialize will try to pre-load the NB initial data
 // (watchers of this registry will receive it only after call to resync)
-func (r *NBInitFileRegistry) initialize() error {
+func (r *InitFileRegistry) initialize() error {
 	defer func() {
 		r.initialized = true
 	}()
@@ -147,34 +147,34 @@ func (r *NBInitFileRegistry) initialize() error {
 	}
 
 	// Initial NB configuration loaded from file
-	if !r.config.DisableNBInitialConfiguration {
+	if !r.config.DisableInitialConfiguration {
 		// preload NB config data from file
-		if err := r.preloadNBConfigs(r.config.NBInitialConfigurationFilePath); err != nil {
+		if err := r.preloadNBConfigs(r.config.InitialConfigurationFilePath); err != nil {
 			return errors.Errorf("can't preload initial NB configuration from file due to: %v", err)
 		}
 		if len(r.preloadedNBConfigs) != 0 {
 			// watch for resync.DefaultPlugin.DoResync() that will trigger pushing of preloaded
 			// NB config data from file into NB aggregator watcher
-			// (see NBInitFileRegistry struct docs for detailed explanation)
+			// (see InitFileRegistry struct docs for detailed explanation)
 			r.watchNBResync()
 		}
 	}
 	return nil
 }
 
-// retrieveConfig loads NBInitFileRegistry plugin configuration file.
-func (r *NBInitFileRegistry) retrieveConfig() (*Config, error) {
+// retrieveConfig loads InitFileRegistry plugin configuration file.
+func (r *InitFileRegistry) retrieveConfig() (*Config, error) {
 	config := &Config{
 		// default configuration
-		DisableNBInitialConfiguration:  false,
-		NBInitialConfigurationFilePath: defaultNBInitFilePath,
+		DisableInitialConfiguration:  false,
+		InitialConfigurationFilePath: defaultInitFilePath,
 	}
 	found, err := r.Cfg.LoadValue(config)
 	if !found {
 		if err == nil {
-			r.Log.Debug("NBInitFileRegistry plugin config not found")
+			r.Log.Debug("InitFileRegistry plugin config not found")
 		} else {
-			r.Log.Debugf("NBInitFileRegistry plugin config can't be loaded due to: %v", err)
+			r.Log.Debugf("InitFileRegistry plugin config can't be loaded due to: %v", err)
 		}
 		return config, err
 	}
@@ -186,14 +186,14 @@ func (r *NBInitFileRegistry) retrieveConfig() (*Config, error) {
 
 // watchNBResync will watch to default resync plugin's resync call(resync.DefaultPlugin.DoResync()) and will
 // load NB initial config from file (already preloaded from initialize()) when the first resync will be fired.
-func (r *NBInitFileRegistry) watchNBResync() {
+func (r *InitFileRegistry) watchNBResync() {
 	registration := resync.DefaultPlugin.Register(registryName)
 	go r.watchResync(registration)
 }
 
 // watchResync will listen to resync plugin resync signals and at first resync will push the preloaded
 // NB initial config into internal local register (p.registry)
-func (r *NBInitFileRegistry) watchResync(resyncReg resync.Registration) {
+func (r *InitFileRegistry) watchResync(resyncReg resync.Registration) {
 	for resyncStatus := range resyncReg.StatusChan() {
 		// resyncReg.StatusChan == Started => resync
 		if resyncStatus.ResyncStatus() == resync.Started && !r.pushedToWatchedRegistry {
@@ -216,14 +216,14 @@ func (r *NBInitFileRegistry) watchResync(resyncReg resync.Registration) {
 // preloadNBConfigs imports NB configuration from file(filepath) into preloadedNBConfigs. If file is not found,
 // it is not considered as error, but as a sign that the NB-configuration-loading-from-file feature should be
 // not used (inner registry remains empty and watchers of this registry get no data).
-func (r *NBInitFileRegistry) preloadNBConfigs(filePath string) error {
+func (r *InitFileRegistry) preloadNBConfigs(filePath string) error {
 	// check existence of NB init file
 	if _, err := os.Stat(filePath); os.IsNotExist(err) {
 		filePath := filePath
 		if absFilePath, err := filepath.Abs(filePath); err == nil {
 			filePath = absFilePath
 		}
-		r.Log.Debugf("Initialization NB configuration file(%v) not found. "+
+		r.Log.Debugf("Initialization configuration file(%v) not found. "+
 			"Skipping its preloading.", filePath)
 		return nil
 	}
@@ -253,14 +253,14 @@ func (r *NBInitFileRegistry) preloadNBConfigs(filePath string) error {
 	}
 	err = protojson.Unmarshal(bj, config)
 	if err != nil {
-		return errors.Errorf("can't unmarshall NB init file data into dynamic config due to: %v", err)
+		return errors.Errorf("can't unmarshall init file data into dynamic config due to: %v", err)
 	}
 
 	// extracting proto messages from dynamic config structure
 	// (generic client wants single proto messages and not one big hierarchical config)
 	configMessages, err := client.DynamicConfigExport(config)
 	if err != nil {
-		return errors.Errorf("can't extract single NB init configuration proto messages "+
+		return errors.Errorf("can't extract single init configuration proto messages "+
 			"from one big configuration proto message due to: %v", err)
 	}
 
diff --git a/plugins/orchestrator/watcher/aggregator.go b/plugins/orchestrator/watcher/aggregator.go
index 43d2cb701d..878ed6e237 100644
--- a/plugins/orchestrator/watcher/aggregator.go
+++ b/plugins/orchestrator/watcher/aggregator.go
@@ -111,9 +111,9 @@ func (p *Aggregator) Watch(
 		//  for initial call of DoResync() to do initial Agent resync must be delayed as well SOMEHOW (can't use
 		//  init or after init of plugins).
 
-		// localregistry.NBInitFileRegistry can also be watcher that never sends anything (i.e. if misconfigured
-		// or no default NB init file is present or loading of data fails or ... -> check for Empty())
-		if initRegistry, ok := w.(*localregistry.NBInitFileRegistry); ok && initRegistry.Empty() {
+		// localregistry.InitFileRegistry can also be watcher that never sends anything (i.e. if misconfigured
+		// or no default init file is present or loading of data fails or ... -> check for Empty())
+		if initRegistry, ok := w.(*localregistry.InitFileRegistry); ok && initRegistry.Empty() {
 			continue
 		}
 
diff --git a/tests/e2e/000_initial_test.go b/tests/e2e/000_initial_test.go
index a58ad21583..784313c1a3 100644
--- a/tests/e2e/000_initial_test.go
+++ b/tests/e2e/000_initial_test.go
@@ -65,13 +65,13 @@ func TestStartStopAgent(t *testing.T) {
 	Eventually(msState).Should(Equal(kvscheduler.ValueState_NONEXISTENT))
 }
 
-// TestNBInitFromFile test configuring initial state of NB from file
-func TestNBInitFromFile(t *testing.T) {
+// TestInitFromFile tests configuring initial state of NB from file
+func TestInitFromFile(t *testing.T) {
 	ctx := Setup(t, WithoutVPPAgent())
 	defer ctx.Teardown() // will teardown also VPP-Agent created later
 
-	// create NB init file content
-	initialNBConfig := `
+	// create init file content
+	initialConfig := `
 netallocConfig: {}
 linuxConfig: {}
 vppConfig:
@@ -83,22 +83,22 @@ vppConfig:
         - 10.10.1.1/24
       mtu: 1500
 `
-	initialNBConfigFileName := CreateFile(t, "nb-init-file.yaml", initialNBConfig)
+	initialConfigFileName := CreateFile(t, "initial-config.yaml", initialConfig)
 
-	// create config content for NB init file usage
-	nbInitFileRegistryConfig := `
-disable-nb-initial-configuration: false
-nb-initial-configuration-file-path: %v
+	// create config content for init file usage
+	initFileRegistryConfig := `
+disable-initial-configuration: false
+initial-configuration-file-path: %v
 `
-	nbInitFileRegistryConfig = fmt.Sprintf(nbInitFileRegistryConfig, initialNBConfigFileName)
+	initFileRegistryConfig = fmt.Sprintf(initFileRegistryConfig, initialConfigFileName)
 
 	// create VPP-Agent
 	SetupVPPAgent(t, ctx,
-		WithAdditionalAgentProcessParams(WithPluginConfigArg(t, "nbinitfileregistry", nbInitFileRegistryConfig)),
+		WithAdditionalAgentProcessParams(WithPluginConfigArg(t, "initfileregistry", initFileRegistryConfig)),
 		WithoutManualInitialAgentResync(),
 	)
 
-	// check whether NB initial configuration inside file is correctly loaded in running VPP-Agent
+	// check whether initial configuration inside file is correctly loaded in running VPP-Agent
 	initInterfaceConfigState := func() kvscheduler.ValueState {
 		return ctx.GetValueStateByKey("vpp/interface/loop-test-from-init-file/address/static/10.10.1.1/24")
 	}
@@ -106,8 +106,8 @@ nb-initial-configuration-file-path: %v
 		"loopback from init file was not properly created")
 }
 
-// TestNBInitFromEtcd test configuring initial state of NB from Etcd
-func TestNBInitFromEtcd(t *testing.T) {
+// TestInitFromEtcd tests configuring initial state of NB from Etcd
+func TestInitFromEtcd(t *testing.T) {
 	ctx := Setup(t,
 		WithEtcd(),
 		WithoutVPPAgent(),
@@ -142,8 +142,8 @@ endpoints:
 		"loopback from etcd was not properly created")
 }
 
-// TestNBInitFromFileAndEtcd test configuring initial state of NB from Etcd and from file
-func TestNBInitFromFileAndEtcd(t *testing.T) {
+// TestInitFromFileAndEtcd tests configuring initial state of NB from Etcd and from file
+func TestInitFromFileAndEtcd(t *testing.T) {
 	ctx := Setup(t,
 		WithEtcd(),
 		WithoutVPPAgent(),
@@ -180,8 +180,8 @@ func TestNBInitFromFileAndEtcd(t *testing.T) {
 	}
 }`)).To(Succeed(), "can't insert data2 into ETCD")
 
-	// create NB init file content
-	initialNBConfig := `
+	// create init file content
+	initialConfig := `
 netallocConfig: {}
 linuxConfig: {}
 vppConfig:
@@ -207,14 +207,14 @@ vppConfig:
          id: 4
          socketFilename: /run/vpp/default.sock
 `
-	initialNBConfigFileName := CreateFile(t, "nb-init-file.yaml", initialNBConfig)
+	initialConfigFileName := CreateFile(t, "initial-config.yaml", initialConfig)
 
 	// create config content for NB init file usage
-	nbInitFileRegistryConfig := `
-disable-nb-initial-configuration: false
-nb-initial-configuration-file-path: %v
+	initFileRegistryConfig := `
+disable-initial-configuration: false
+initial-configuration-file-path: %v
 `
-	nbInitFileRegistryConfig = fmt.Sprintf(nbInitFileRegistryConfig, initialNBConfigFileName)
+	initFileRegistryConfig = fmt.Sprintf(initFileRegistryConfig, initialConfigFileName)
 
 	// create config content for etcd connection
 	etcdConfig := `insecure-transport: true
@@ -227,11 +227,11 @@ endpoints:
 	// create VPP-Agent
 	SetupVPPAgent(t, ctx,
 		WithAdditionalAgentProcessParams(WithPluginConfigArg(t, "etcd", etcdConfig),
-			WithPluginConfigArg(t, "nbinitfileregistry", nbInitFileRegistryConfig)),
+			WithPluginConfigArg(t, "initfileregistry", initFileRegistryConfig)),
 		WithoutManualInitialAgentResync(),
 	)
 
-	// check whether NB initial configuration is correctly loaded from Etcd in running VPP-Agent
+	// check whether initial configuration is correctly loaded from Etcd and file in running VPP-Agent
 	initInterfaceConfigState := func(interfaceName string, ipAddress string) kvscheduler.ValueState {
 		return ctx.GetValueStateByKey(
 			fmt.Sprintf("vpp/interface/%v/address/static/%v/32", interfaceName, ipAddress))

From 3c70b7a73dfb0f4f5dc63a8b003b7ad1fd24b133 Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Thu, 17 Dec 2020 17:49:45 +0100
Subject: [PATCH 07/11] fix: fixed init file tests for changes from master
 (created sharing volume for passing config files, etc.)

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 tests/e2e/000_initial_test.go |  28 +++----
 tests/e2e/e2e.go              | 144 ++++++++++++++++++++--------------
 tests/e2e/etcd.go             |  61 ++++++++------
 tests/e2e/run_e2e.sh          |  21 +++++
 tests/e2e/vppagent.go         |  17 ++--
 5 files changed, 167 insertions(+), 104 deletions(-)

diff --git a/tests/e2e/000_initial_test.go b/tests/e2e/000_initial_test.go
index 784313c1a3..a9d5ca0dbc 100644
--- a/tests/e2e/000_initial_test.go
+++ b/tests/e2e/000_initial_test.go
@@ -59,10 +59,10 @@ func TestStartStopAgent(t *testing.T) {
 	}
 
 	ctx.StartAgent(agent1)
-	Eventually(msState).Should(Equal(kvscheduler.ValueState_OBTAINED))
+	Eventually(msState).Should(Equal(kvscheduler.ValueState_OBTAINED)) // FIXME testing agent0 and not agent1
 
 	ctx.StopAgent(agent1)
-	Eventually(msState).Should(Equal(kvscheduler.ValueState_NONEXISTENT))
+	Eventually(msState).Should(Equal(kvscheduler.ValueState_NONEXISTENT)) // FIXME testing agent0 and not agent1 , did this test even passed with previous e2e changes?
 }
 
 // TestInitFromFile tests configuring initial state of NB from file
@@ -83,7 +83,7 @@ vppConfig:
         - 10.10.1.1/24
       mtu: 1500
 `
-	initialConfigFileName := CreateFile(t, "initial-config.yaml", initialConfig)
+	initialConfigFileName := CreateFileOnSharedVolume(ctx, "initial-config.yaml", initialConfig)
 
 	// create config content for init file usage
 	initFileRegistryConfig := `
@@ -93,8 +93,8 @@ initial-configuration-file-path: %v
 	initFileRegistryConfig = fmt.Sprintf(initFileRegistryConfig, initialConfigFileName)
 
 	// create VPP-Agent
-	SetupVPPAgent(t, ctx,
-		WithAdditionalAgentProcessParams(WithPluginConfigArg(t, "initfileregistry", initFileRegistryConfig)),
+	SetupVPPAgent(ctx,
+		WithAdditionalAgentCmdParams(WithPluginConfigArg(ctx, "initfileregistry", initFileRegistryConfig)),
 		WithoutManualInitialAgentResync(),
 	)
 
@@ -116,7 +116,7 @@ func TestInitFromEtcd(t *testing.T) {
 
 	// put NB config into Etcd
 	Expect(ctx.Etcd.Put(
-		"/vnf-agent/vpp1/config/vpp/v2/interfaces/loop-test-from-etcd",
+		fmt.Sprintf("/vnf-agent/%v/config/vpp/v2/interfaces/loop-test-from-etcd", AgentInstanceName(ctx)),
 		`{"name":"loop-test-from-etcd","type":"SOFTWARE_LOOPBACK","enabled":true,"ip_addresses":["10.10.1.2/24"], "mtu":1500}`)).
 		To(Succeed(), "can't insert data into ETCD")
 
@@ -129,8 +129,8 @@ endpoints:
 	etcdConfig = fmt.Sprintf(etcdConfig, ctx.Etcd.Inspect().NetworkSettings.IPAddress)
 
 	// create VPP-Agent
-	SetupVPPAgent(t, ctx,
-		WithAdditionalAgentProcessParams(WithPluginConfigArg(t, "etcd", etcdConfig)),
+	SetupVPPAgent(ctx,
+		WithAdditionalAgentCmdParams(WithPluginConfigArg(ctx, "etcd", etcdConfig)),
 		WithoutManualInitialAgentResync(),
 	)
 
@@ -152,7 +152,7 @@ func TestInitFromFileAndEtcd(t *testing.T) {
 
 	// put NB config into Etcd
 	Expect(ctx.Etcd.Put(
-		"/vnf-agent/vpp1/config/vpp/v2/interfaces/memif-from-etcd",
+		fmt.Sprintf("/vnf-agent/%v/config/vpp/v2/interfaces/memif-from-etcd", AgentInstanceName(ctx)),
 		`{
 "name":"memif-from-etcd",
 "type":"MEMIF",
@@ -166,7 +166,7 @@ func TestInitFromFileAndEtcd(t *testing.T) {
 	}
 }`)).To(Succeed(), "can't insert data1 into ETCD")
 	Expect(ctx.Etcd.Put(
-		"/vnf-agent/vpp1/config/vpp/v2/interfaces/memif-from-both-sources",
+		fmt.Sprintf("/vnf-agent/%v/config/vpp/v2/interfaces/memif-from-both-sources", AgentInstanceName(ctx)),
 		`{
 "name":"memif-from-both-sources",
 "type":"MEMIF",
@@ -207,7 +207,7 @@ vppConfig:
          id: 4
          socketFilename: /run/vpp/default.sock
 `
-	initialConfigFileName := CreateFile(t, "initial-config.yaml", initialConfig)
+	initialConfigFileName := CreateFileOnSharedVolume(ctx, "initial-config.yaml", initialConfig)
 
 	// create config content for NB init file usage
 	initFileRegistryConfig := `
@@ -225,9 +225,9 @@ endpoints:
 	etcdConfig = fmt.Sprintf(etcdConfig, ctx.Etcd.Inspect().NetworkSettings.IPAddress)
 
 	// create VPP-Agent
-	SetupVPPAgent(t, ctx,
-		WithAdditionalAgentProcessParams(WithPluginConfigArg(t, "etcd", etcdConfig),
-			WithPluginConfigArg(t, "initfileregistry", initFileRegistryConfig)),
+	SetupVPPAgent(ctx,
+		WithAdditionalAgentCmdParams(WithPluginConfigArg(ctx, "etcd", etcdConfig),
+			WithPluginConfigArg(ctx, "initfileregistry", initFileRegistryConfig)),
 		WithoutManualInitialAgentResync(),
 	)
 
diff --git a/tests/e2e/e2e.go b/tests/e2e/e2e.go
index f50308ba93..71fe7ed213 100644
--- a/tests/e2e/e2e.go
+++ b/tests/e2e/e2e.go
@@ -19,11 +19,9 @@ import (
 	"context"
 	"flag"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"log"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -55,13 +53,14 @@ var (
 )
 
 const (
-	etcdImage = "gcr.io/etcd-development/etcd"
-
 	agentInitTimeout     = time.Second * 15
 	processExitTimeout   = time.Second * 3
 	checkPollingInterval = time.Millisecond * 100
 	checkTimeout         = time.Second * 6
 	execTimeout          = 10 * time.Second
+	defaultTestShareDir  = "/test-share"
+	shareVolumeName      = "share-for-vpp-agent-e2e-tests"
+	nameOfDefaultAgent   = "agent0"
 
 	// VPP input nodes for packet tracing (uncomment when needed)
 	tapv2InputNode = "virtio-input"
@@ -85,7 +84,8 @@ type TestCtx struct {
 	ctx    context.Context
 	cancel context.CancelFunc
 
-	testDataDir string
+	testDataDir  string
+	testShareDir string
 
 	agent         *agent
 	agents        map[string]*agent
@@ -99,13 +99,7 @@ type TestCtx struct {
 	logger    *log.Logger
 }
 
-// Option is key-value pair for customizing setup of tests
-type Option struct {
-	key   string
-	value interface{}
-}
-// TODO recheck options due to rebasing mess
-func NewTest(t *testing.T, options ...*Option) *TestCtx {
+func NewTest(t *testing.T) *TestCtx {
 	RegisterTestingT(t)
 	// TODO: Do not use global test registration.
 	//  It is now deprecated and you should use NewWithT() instead.
@@ -113,8 +107,6 @@ func NewTest(t *testing.T, options ...*Option) *TestCtx {
 
 	logrus.Debugf("Environ:\n%v", strings.Join(os.Environ(), "\n"))
 
-	optionsMap := optionsMap(options)
-
 	SetDefaultEventuallyPollingInterval(checkPollingInterval)
 	SetDefaultEventuallyTimeout(checkTimeout)
 
@@ -124,6 +116,7 @@ func NewTest(t *testing.T, options ...*Option) *TestCtx {
 	te := &TestCtx{
 		t:             t,
 		testDataDir:   os.Getenv("TESTDATA_DIR"),
+		testShareDir:  defaultTestShareDir,
 		agents:        make(map[string]*agent),
 		microservices: make(map[string]*microservice),
 		nsCalls:       nslinuxcalls.NewSystemHandler(),
@@ -134,21 +127,15 @@ func NewTest(t *testing.T, options ...*Option) *TestCtx {
 	return te
 }
 
-// TODO resolver vppagent mess
-//if _, found := optionsMap[DontSetupVPPAgent]; !found {
-//  SetupVPPAgent(t, testCtx, options...)
-//}
-//
-//optionsMap := optionsMap(options)
-//
-//var agentArgs []string
-//if additionalParams, found := optionsMap[AdditionalAgentProcessParams]; found {
-//agentArgs = additionalParams.([]string)
-//}
-
+// Option is key-value pair for customizing setup of tests
+type Option struct {
+	key   string
+	value interface{}
+}
 
-func Setup(t *testing.T) *TestCtx {
+func Setup(t *testing.T, options ...*Option) *TestCtx {
 	var err error
+	optionsMap := optionsMap(options)
 
 	testCtx := NewTest(t)
 
@@ -165,7 +152,7 @@ func Setup(t *testing.T) *TestCtx {
 	removeDanglingAgents(t, testCtx.dockerClient)
 	removeDanglingMicroservices(t, testCtx.dockerClient)
 
-	// if setupE2E fails we need to stop started processes
+	// if setupE2E fails we need to stop started containers
 	defer func() {
 		if testCtx.t.Failed() || *debug {
 			testCtx.dumpLog()
@@ -176,46 +163,79 @@ func Setup(t *testing.T) *TestCtx {
 					t.Logf("failed to stop vpp-agent: %v", err)
 				}
 			}
+			if testCtx.Etcd != nil {
+				if err := testCtx.Etcd.Terminate(); err != nil {
+					t.Logf("failed to terminate etcd due to: %v", err)
+				}
+			}
 		}
 	}()
 
-	testCtx.agent = testCtx.StartAgent("agent0")
-	agentAddr := testCtx.agent.IPAddress()
+	// setup Etcd
+	if _, found := optionsMap[SetupEtcdContainer]; found {
+		testCtx.Etcd = NewEtcdContainer(testCtx, options...)
+	}
+
+	if _, found := optionsMap[DontSetupVPPAgent]; !found {
+		SetupVPPAgent(testCtx)
+	}
+
+	return testCtx
+}
+
+func SetupVPPAgent(testCtx *TestCtx, opts ...AgentOptModifier) {
+	// prepare options
+	options := DefaultAgentOpt()
+	for _, optionsModifier := range opts {
+		optionsModifier(options)
+	}
+
+	// start agent container
+	testCtx.agent = testCtx.StartAgent(nameOfDefaultAgent, opts...) // not passing prepared options due to public visibility
 
 	// interact with the agent using the client from agentctl
+	agentAddr := testCtx.agent.IPAddress()
+	var err error
 	testCtx.agentClient, err = agentctl.NewClientWithOpts(
 		agentctl.WithHost(agentAddr),
 		agentctl.WithGrpcPort(*agentGrpcPort),
 		agentctl.WithHTTPPort(*agentHTTPPort))
 	if err != nil {
-		t.Fatalf("Failed to create VPP-agent client: %v", err)
+		testCtx.t.Fatalf("Failed to create VPP-agent client: %v", err)
 	}
 
+	// wait to agent to start properly
 	Eventually(testCtx.checkAgentReady, agentInitTimeout, checkPollingInterval).Should(Succeed())
 
 	// run initial resync
-	if _, found := optionsMap[NoManualInitialAgentResync]; !found {
+	if !options.NoManualInitialResync {
 		testCtx.syncAgent()
 	}
 
+	// fill VPP version (this depends on agentctl and that depends on agent to be set up)
 	if version, err := testCtx.ExecVppctl("show version"); err != nil {
-		t.Fatalf("Retrieving VPP version via vppctl failed: %v", err)
+		testCtx.t.Fatalf("Retrieving VPP version via vppctl failed: %v", err)
 	} else {
 		versionParts := strings.SplitN(version, " ", 3)
 		if len(versionParts) > 1 {
 			testCtx.vppVersion = version
-			t.Logf("VPP version: %v", testCtx.vppVersion)
+			testCtx.t.Logf("VPP version: %v", testCtx.vppVersion)
 		} else {
-			t.Logf("invalid VPP version: %q", version)
+			testCtx.t.Logf("invalid VPP version: %q", version)
 		}
 	}
+}
 
-	// setup Etcd
-	if _, found := optionsMap[SetupEtcdContainer]; found {
-		testCtx.Etcd = NewEtcdContainer(testCtx, options...)
+// AgentInstanceName provides instance name of VPP-Agent that is created by setup by default. This name is
+// used i.e. in ETCD key prefix.
+func AgentInstanceName(testCtx *TestCtx) string {
+	//TODO API boundaries becomes blurry as tests and support structures are in the same package and there
+	// is strong temptation to misuse it and create an unmaintainable dependency mesh -> create different
+	// package for test supporting files (setup/teardown/util stuff) and define clear boundaries
+	if testCtx.agent != nil {
+		return testCtx.agent.name
 	}
-
-	return testCtx
+	return nameOfDefaultAgent
 }
 
 func optionsMap(options []*Option) map[string]interface{} {
@@ -254,7 +274,7 @@ func (test *TestCtx) Teardown() {
 
 	// terminate etcd
 	if test.Etcd != nil {
-		test.Etcd.Terminate(test)
+		test.Etcd.Terminate()
 	}
 }
 
@@ -273,32 +293,35 @@ func (test *TestCtx) VppRelease() string {
 }
 
 // WithoutManualInitialAgentResync is test setup option disabling manual agent resync just after agent setup
-func WithoutManualInitialAgentResync() *Option {
-	return &Option{
-		key:   NoManualInitialAgentResync,
-		value: struct{}{}, // only presence is needed
+func WithoutManualInitialAgentResync() func(o *AgentOpt) {
+	return func(o *AgentOpt) {
+		o.NoManualInitialResync = true
 	}
 }
 
-// WithAdditionalAgentProcessParams is test setup option adding additional parameters to executing vpp-agent process
-func WithAdditionalAgentProcessParams(params ...string) *Option {
-	return &Option{
-		key:   AdditionalAgentProcessParams,
-		value: params,
+// WithAdditionalAgentCmdParams is test setup option adding additional command line parameters to executing vpp-agent
+func WithAdditionalAgentCmdParams(params ...string) func(o *AgentOpt) {
+	return func(o *AgentOpt) {
+		o.Env = append(o.Env, params...)
 	}
 }
 
 // WithPluginConfigArg persists configContent for give VPP-Agent plugin (expecting generic plugin config name)
 // and returns argument for VPP-Agent executable to use this plugin configuration file.
-func WithPluginConfigArg(t *testing.T, pluginName string, configContent string) string {
-	configFilePath := CreateFile(t, fmt.Sprintf("%v.config", pluginName), configContent)
-	return fmt.Sprintf("-%v-config=%v", pluginName, configFilePath)
+func WithPluginConfigArg(ctx *TestCtx, pluginName string, configContent string) string {
+	configFilePath := CreateFileOnSharedVolume(ctx, fmt.Sprintf("%v.config", pluginName), configContent)
+	return fmt.Sprintf("%v_CONFIG=%v", strings.ToUpper(pluginName), configFilePath)
 }
 
-// CreateFile persists fileContent to file in OS temp directory. It returns the absolute path to the newly
-// created file.
-func CreateFile(t *testing.T, simpleFileName string, fileContent string) string {
-	filePath, err := filepath.Abs(filepath.Join(os.TempDir(), fmt.Sprintf("e2e-test-%v-%v", t.Name(), simpleFileName)))
+// FIXME container that will use it can have it mounted in different location as seen by the container where
+//  it is created (this works now due to the same mountpoint of shared volume in every container)
+
+// CreateFileOnSharedVolume persists fileContent to file in mounted shared volume used for sharing file
+// between containers. It returns the absolute path to the newly created file as seen by the container
+// that creates it.
+func CreateFileOnSharedVolume(ctx *TestCtx, simpleFileName string, fileContent string) string {
+	filePath, err := filepath.Abs(filepath.Join(ctx.testShareDir,
+		fmt.Sprintf("e2e-test-%v-%v", ctx.t.Name(), simpleFileName)))
 	Expect(err).To(Not(HaveOccurred()))
 	Expect(ioutil.WriteFile(filePath, []byte(fileContent), 0777)).To(Succeed())
 
@@ -323,7 +346,6 @@ func WithEtcd() *Option {
 	}
 }
 
-
 func (test *TestCtx) GenericClient() client.GenericClient {
 	c, err := test.agentClient.GenericClient()
 	if err != nil {
@@ -427,7 +449,7 @@ func (test *TestCtx) StopMicroservice(name string) {
 	delete(test.microservices, name)
 }
 
-func (test *TestCtx) StartAgent(name string, o ...*AgentOpt) *agent {
+func (test *TestCtx) StartAgent(name string, opts ...AgentOptModifier) *agent {
 	test.t.Helper()
 
 	if _, ok := test.agents[name]; ok {
@@ -436,8 +458,8 @@ func (test *TestCtx) StartAgent(name string, o ...*AgentOpt) *agent {
 
 	// prepare agent options
 	opt := DefaultAgentOpt()
-	if len(o) > 0 {
-		opt = o[0]
+	for _, optModifier := range opts {
+		optModifier(opt)
 	}
 	opt.Env = append(opt.Env, "MICROSERVICE_LABEL="+name)
 
diff --git a/tests/e2e/etcd.go b/tests/e2e/etcd.go
index ea2884b806..0dff831cde 100644
--- a/tests/e2e/etcd.go
+++ b/tests/e2e/etcd.go
@@ -19,6 +19,12 @@ import (
 	"os"
 
 	docker "github.com/fsouza/go-dockerclient"
+	"github.com/go-errors/errors"
+)
+
+const (
+	etcdImage       = "gcr.io/etcd-development/etcd"
+	etcdStopTimeout = 1 // seconds
 )
 
 // Setup options constants
@@ -27,6 +33,8 @@ const (
 	VPPAgentContainerNetworking = "VPPAgentContainerNetworking"
 )
 
+// TODO unify whether return errors or use test context's log fatal
+
 // EtcdContainer is represents running ETCD container
 type EtcdContainer struct {
 	ctx         *TestCtx
@@ -38,8 +46,8 @@ func NewEtcdContainer(ctx *TestCtx, options ...*Option) *EtcdContainer {
 	ec := &EtcdContainer{
 		ctx: ctx,
 	}
-	container := ec.create(ctx, options...)
-	ec.start(ctx, container)
+	container := ec.create(options...)
+	ec.start(container)
 	ec.containerID = container.ID
 	return ec
 }
@@ -70,16 +78,16 @@ func (ec *EtcdContainer) Inspect() *docker.Container {
 	return container
 }
 
-func (ec *EtcdContainer) create(ctx *TestCtx, options ...*Option) *docker.Container {
+func (ec *EtcdContainer) create(options ...*Option) *docker.Container {
 	optionsMap := optionsMap(options)
 
 	// pull image
-	err := ctx.dockerClient.PullImage(docker.PullImageOptions{
+	err := ec.ctx.dockerClient.PullImage(docker.PullImageOptions{
 		Repository: etcdImage,
 		Tag:        "latest",
 	}, docker.AuthConfiguration{})
 	if err != nil {
-		ctx.t.Fatalf("failed to pull ETCD image: %v", err)
+		ec.ctx.t.Fatalf("failed to pull ETCD image: %v", err)
 	}
 
 	// construct command string and container host config
@@ -112,7 +120,7 @@ func (ec *EtcdContainer) create(ctx *TestCtx, options ...*Option) *docker.Contai
 	}
 
 	// create container
-	container, err := ctx.dockerClient.CreateContainer(docker.CreateContainerOptions{
+	container, err := ec.ctx.dockerClient.CreateContainer(docker.CreateContainerOptions{
 		Name: "e2e-test-etcd",
 		Config: &docker.Config{
 			Env:   []string{"ETCDCTL_API=3"},
@@ -122,7 +130,7 @@ func (ec *EtcdContainer) create(ctx *TestCtx, options ...*Option) *docker.Contai
 		HostConfig: hostConfig,
 	})
 	if err != nil {
-		ctx.t.Fatalf("failed to create ETCD container: %v", err)
+		ec.ctx.t.Fatalf("failed to create ETCD container: %v", err)
 	}
 	return container
 }
@@ -145,43 +153,50 @@ func WithEtcdVPPAgentContainerNetworking() *Option {
 	}
 }
 
-func (ec *EtcdContainer) start(ctx *TestCtx, container *docker.Container) {
-	err := ctx.dockerClient.StartContainer(container.ID, nil)
+func (ec *EtcdContainer) start(container *docker.Container) {
+	err := ec.ctx.dockerClient.StartContainer(container.ID, nil)
 	if err != nil {
-		err = ctx.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
+		err = ec.ctx.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
 			ID:    container.ID,
 			Force: true,
 		})
 		if err != nil {
-			ctx.t.Errorf("failed to remove ETCD container: %v", err)
+			ec.ctx.t.Errorf("failed to remove ETCD container: %v", err)
 		}
-		ctx.t.Fatalf("failed to start ETCD container: %v", err)
+		ec.ctx.t.Fatalf("failed to start ETCD container: %v", err)
 	}
-	ctx.t.Logf("started ETCD container %v", container.ID)
+	ec.ctx.t.Logf("started ETCD container %v", container.ID)
 }
 
 // Terminate stops and removes the ETCD container
-func (ec *EtcdContainer) Terminate(ctx *TestCtx) {
-	ec.stop(ctx)
-	ec.remove(ctx)
+func (ec *EtcdContainer) Terminate() error {
+	if err := ec.stop(); err != nil {
+		return err
+	}
+	if err := ec.remove(); err != nil {
+		return err
+	}
+	return nil
 }
 
-func (ec *EtcdContainer) stop(ctx *TestCtx) {
-	err := ctx.dockerClient.StopContainer(ec.containerID, msStopTimeout)
+func (ec *EtcdContainer) stop() error {
+	err := ec.ctx.dockerClient.StopContainer(ec.containerID, etcdStopTimeout)
 	if err != nil {
-		ctx.t.Logf("failed to stop ETCD container: %v", err)
+		return errors.Errorf("failed to stop ETCD container: %v", err)
 	}
+	return nil
 }
 
-func (ec *EtcdContainer) remove(ctx *TestCtx) {
-	err := ctx.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
+func (ec *EtcdContainer) remove() error {
+	err := ec.ctx.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
 		ID:    ec.containerID,
 		Force: true,
 	})
 	if err != nil {
-		ctx.t.Fatalf("failed to remove ETCD container: %v", err)
+		return errors.Errorf("failed to remove ETCD container: %v", err)
 	}
-	ctx.t.Logf("removed ETCD container %v", ec.containerID)
+	ec.ctx.t.Logf("removed ETCD container %v", ec.containerID)
+	return nil
 }
 
 // exec executes command inside Etcd container
diff --git a/tests/e2e/run_e2e.sh b/tests/e2e/run_e2e.sh
index c44216c822..b7aa167e0b 100755
--- a/tests/e2e/run_e2e.sh
+++ b/tests/e2e/run_e2e.sh
@@ -15,6 +15,7 @@ export DOCKER_BUILDKIT=1
 testname="vpp-agent-e2e-test"
 imgname="vpp-agent-e2e-tests"
 etcdcontainername="e2e-test-etcd"
+sharevolumename="share-for-vpp-agent-e2e-tests"
 
 # Compile agentctl for testing
 go build -o ./tests/e2e/agentctl.test \
@@ -54,11 +55,30 @@ cleanup() {
     set -x
     docker stop -t 1 "${etcdcontainername}" 2>/dev/null
     docker rm -v "${etcdcontainername}" 2>/dev/null
+    set +x
   fi
+
+  echo "Removing volume for sharing files between containers"
+  if [ "$(docker volume ls | grep "${sharevolumename}")" ]; then
+    set -x
+    docker volume rm -f "${sharevolumename}"
+    set +x
+  fi
+
 }
 
 trap 'cleanup' EXIT
 
+echo "Creating volume for sharing files between containers.."
+if docker volume create "${sharevolumename}"
+then
+	echo >&2 -e " \e[32m...created\e[0m"
+else
+	res=$?
+	echo >&2 -e " \e[31m...volume creation failed!\e[0m (exit code: $res)"
+	exit $res
+fi
+
 vppver=$(docker run --rm -i "$VPP_AGENT" dpkg-query -f '${Version}' -W vpp)
 
 echo "=========================================================================="
@@ -80,6 +100,7 @@ if docker run -it \
 	--label io.ligato.vpp-agent.testname="${testname}" \
 	--volume "${TESTDATA_DIR}":/testdata:ro \
 	--volume /var/run/docker.sock:/var/run/docker.sock \
+	--volume "${sharevolumename}":/test-share \
 	--env TESTDATA_DIR \
 	--env INITIAL_LOGLVL \
 	--env VPP_AGENT \
diff --git a/tests/e2e/vppagent.go b/tests/e2e/vppagent.go
index 10e58fc455..8a2bb4180a 100644
--- a/tests/e2e/vppagent.go
+++ b/tests/e2e/vppagent.go
@@ -38,12 +38,15 @@ const (
 )
 
 type AgentOpt struct {
-	Image             string
-	Env               []string
-	UseEtcd           bool
-	ContainerOptsHook func(*docker.CreateContainerOptions)
+	Image                 string
+	Env                   []string
+	UseEtcd               bool
+	NoManualInitialResync bool
+	ContainerOptsHook     func(*docker.CreateContainerOptions)
 }
 
+type AgentOptModifier func(*AgentOpt)
+
 func DefaultAgentOpt() *AgentOpt {
 	agentImg := vppAgentDefaultImg
 	if img := os.Getenv("VPP_AGENT"); img != "" {
@@ -58,8 +61,9 @@ func DefaultAgentOpt() *AgentOpt {
 		etcdConfig = val
 	}
 	opt := &AgentOpt{
-		Image:   agentImg,
-		UseEtcd: false,
+		Image:                 agentImg,
+		UseEtcd:               false,
+		NoManualInitialResync: false,
 		Env: []string{
 			"INITIAL_LOGLVL=" + logging.DefaultLogger.GetLevel().String(),
 			"ETCD_CONFIG=" + etcdConfig,
@@ -102,6 +106,7 @@ func startAgent(ctx *TestCtx, name string, opt *AgentOpt) (*agent, error) {
 				"/var/run/docker.sock:/var/run/docker.sock",
 				ctx.testDataDir + ":/testdata:ro",
 				filepath.Join(ctx.testDataDir, "certs") + ":/etc/certs:ro",
+				shareVolumeName + ":" + ctx.testShareDir,
 			},
 		},
 	}

From c915d1b6dbc57ef13656d0b43c83996e8bdfa875 Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Thu, 17 Dec 2020 19:39:10 +0100
Subject: [PATCH 08/11] refactor: unified test setup option usage

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 tests/e2e/100_agentctl_test.go |   2 +-
 tests/e2e/e2e.go               |  97 ++--------------
 tests/e2e/etcd.go              |  40 ++-----
 tests/e2e/options.go           | 202 +++++++++++++++++++++++++++++++++
 tests/e2e/run_e2e.sh           |   4 +-
 tests/e2e/vppagent.go          |  38 -------
 6 files changed, 226 insertions(+), 157 deletions(-)
 create mode 100644 tests/e2e/options.go

diff --git a/tests/e2e/100_agentctl_test.go b/tests/e2e/100_agentctl_test.go
index 20c158ebcf..01ab95a8bd 100644
--- a/tests/e2e/100_agentctl_test.go
+++ b/tests/e2e/100_agentctl_test.go
@@ -365,7 +365,7 @@ func TestAgentCtlSecureGrpc(t *testing.T) {
 }
 
 func TestAgentCtlSecureETCD(t *testing.T) {
-	ctx := Setup(t, WithEtcd(), WithEtcdHTTPsConnection(), WithEtcdVPPAgentContainerNetworking())
+	ctx := Setup(t, WithEtcd(WithEtcdHTTPsConnection(), WithEtcdVPPAgentContainerNetworking()))
 	defer ctx.Teardown()
 
 	// test without any TLS
diff --git a/tests/e2e/e2e.go b/tests/e2e/e2e.go
index 71fe7ed213..87ed7e3c4d 100644
--- a/tests/e2e/e2e.go
+++ b/tests/e2e/e2e.go
@@ -19,10 +19,8 @@ import (
 	"context"
 	"flag"
 	"fmt"
-	"io/ioutil"
 	"log"
 	"os"
-	"path/filepath"
 	"regexp"
 	"strings"
 	"testing"
@@ -69,14 +67,6 @@ const (
 	//memifInputNode    = "memif-input"
 )
 
-// Setup options constants
-const (
-	NoManualInitialAgentResync   = "NoManualInitialAgentResync"
-	AdditionalAgentProcessParams = "AdditionalAgentProcessParams"
-	DontSetupVPPAgent            = "DontSetupVPPAgent"
-	SetupEtcdContainer           = "SetupEtcdContainer"
-)
-
 type TestCtx struct {
 	Etcd *EtcdContainer // TODO change?
 
@@ -127,19 +117,17 @@ func NewTest(t *testing.T) *TestCtx {
 	return te
 }
 
-// Option is key-value pair for customizing setup of tests
-type Option struct {
-	key   string
-	value interface{}
-}
-
-func Setup(t *testing.T, options ...*Option) *TestCtx {
-	var err error
-	optionsMap := optionsMap(options)
+func Setup(t *testing.T, options ...SetupOptModifier) *TestCtx {
+	// prepare setup options
+	opt := DefaultSetupOpt()
+	for _, optModifier := range options {
+		optModifier(opt)
+	}
 
 	testCtx := NewTest(t)
 
 	// connect to the docker daemon
+	var err error
 	testCtx.dockerClient, err = docker.NewClientFromEnv()
 	if err != nil {
 		t.Fatalf("failed to get docker client instance from the environment variables: %v", err)
@@ -172,12 +160,12 @@ func Setup(t *testing.T, options ...*Option) *TestCtx {
 	}()
 
 	// setup Etcd
-	if _, found := optionsMap[SetupEtcdContainer]; found {
-		testCtx.Etcd = NewEtcdContainer(testCtx, options...)
+	if opt.SetupEtcd {
+		testCtx.Etcd = NewEtcdContainer(testCtx, extractEtcdOptions(opt))
 	}
 
-	if _, found := optionsMap[DontSetupVPPAgent]; !found {
-		SetupVPPAgent(testCtx)
+	if opt.SetupAgent {
+		SetupVPPAgent(testCtx, extractAgentOptions(opt))
 	}
 
 	return testCtx
@@ -238,15 +226,6 @@ func AgentInstanceName(testCtx *TestCtx) string {
 	return nameOfDefaultAgent
 }
 
-func optionsMap(options []*Option) map[string]interface{} {
-	// convert options to map
-	optionsMap := make(map[string]interface{})
-	for _, option := range options {
-		optionsMap[option.key] = option.value
-	}
-	return optionsMap
-}
-
 func (test *TestCtx) Teardown() {
 	if test.t.Failed() || *debug {
 		defer test.dumpLog()
@@ -292,60 +271,6 @@ func (test *TestCtx) VppRelease() string {
 	return test.vppVersion
 }
 
-// WithoutManualInitialAgentResync is test setup option disabling manual agent resync just after agent setup
-func WithoutManualInitialAgentResync() func(o *AgentOpt) {
-	return func(o *AgentOpt) {
-		o.NoManualInitialResync = true
-	}
-}
-
-// WithAdditionalAgentCmdParams is test setup option adding additional command line parameters to executing vpp-agent
-func WithAdditionalAgentCmdParams(params ...string) func(o *AgentOpt) {
-	return func(o *AgentOpt) {
-		o.Env = append(o.Env, params...)
-	}
-}
-
-// WithPluginConfigArg persists configContent for give VPP-Agent plugin (expecting generic plugin config name)
-// and returns argument for VPP-Agent executable to use this plugin configuration file.
-func WithPluginConfigArg(ctx *TestCtx, pluginName string, configContent string) string {
-	configFilePath := CreateFileOnSharedVolume(ctx, fmt.Sprintf("%v.config", pluginName), configContent)
-	return fmt.Sprintf("%v_CONFIG=%v", strings.ToUpper(pluginName), configFilePath)
-}
-
-// FIXME container that will use it can have it mounted in different location as seen by the container where
-//  it is created (this works now due to the same mountpoint of shared volume in every container)
-
-// CreateFileOnSharedVolume persists fileContent to file in mounted shared volume used for sharing file
-// between containers. It returns the absolute path to the newly created file as seen by the container
-// that creates it.
-func CreateFileOnSharedVolume(ctx *TestCtx, simpleFileName string, fileContent string) string {
-	filePath, err := filepath.Abs(filepath.Join(ctx.testShareDir,
-		fmt.Sprintf("e2e-test-%v-%v", ctx.t.Name(), simpleFileName)))
-	Expect(err).To(Not(HaveOccurred()))
-	Expect(ioutil.WriteFile(filePath, []byte(fileContent), 0777)).To(Succeed())
-
-	// TODO register in context and delete in teardown? this doesn't matter
-	//  that much because file names contain unique test names so no file collision can happen
-	return filePath
-}
-
-// WithoutVPPAgent is test setup option disabling vpp-agent setup
-func WithoutVPPAgent() *Option {
-	return &Option{
-		key:   DontSetupVPPAgent,
-		value: struct{}{}, // only presence is needed
-	}
-}
-
-// WithEtcd is test setup option enabling vpp-agent setup
-func WithEtcd() *Option {
-	return &Option{
-		key:   SetupEtcdContainer,
-		value: struct{}{}, // only presence is needed
-	}
-}
-
 func (test *TestCtx) GenericClient() client.GenericClient {
 	c, err := test.agentClient.GenericClient()
 	if err != nil {
diff --git a/tests/e2e/etcd.go b/tests/e2e/etcd.go
index 0dff831cde..c1aee9297d 100644
--- a/tests/e2e/etcd.go
+++ b/tests/e2e/etcd.go
@@ -16,6 +16,7 @@ package e2e
 
 import (
 	"bytes"
+	"fmt"
 	"os"
 
 	docker "github.com/fsouza/go-dockerclient"
@@ -27,12 +28,6 @@ const (
 	etcdStopTimeout = 1 // seconds
 )
 
-// Setup options constants
-const (
-	HTTPsConnection             = "HTTPsConnection"
-	VPPAgentContainerNetworking = "VPPAgentContainerNetworking"
-)
-
 // TODO unify whether return errors or use test context's log fatal
 
 // EtcdContainer is represents running ETCD container
@@ -42,7 +37,7 @@ type EtcdContainer struct {
 }
 
 // NewEtcdContainer creates and starts new ETCD container
-func NewEtcdContainer(ctx *TestCtx, options ...*Option) *EtcdContainer {
+func NewEtcdContainer(ctx *TestCtx, options ...EtcdOptModifier) *EtcdContainer {
 	ec := &EtcdContainer{
 		ctx: ctx,
 	}
@@ -78,8 +73,11 @@ func (ec *EtcdContainer) Inspect() *docker.Container {
 	return container
 }
 
-func (ec *EtcdContainer) create(options ...*Option) *docker.Container {
-	optionsMap := optionsMap(options)
+func (ec *EtcdContainer) create(options ...EtcdOptModifier) *docker.Container {
+	opts := DefaultEtcdOpt()
+	for _, optionModifier := range options {
+		optionModifier(opts)
+	}
 
 	// pull image
 	err := ec.ctx.dockerClient.PullImage(docker.PullImageOptions{
@@ -95,7 +93,7 @@ func (ec *EtcdContainer) create(options ...*Option) *docker.Container {
 		"/usr/local/bin/etcd",
 	}
 	hostConfig := &docker.HostConfig{}
-	if _, found := optionsMap[HTTPsConnection]; found {
+	if opts.UseHTTPS {
 		cmd = append(cmd,
 			"--client-cert-auth",
 			"--trusted-ca-file=/etc/certs/ca.pem",
@@ -111,8 +109,8 @@ func (ec *EtcdContainer) create(options ...*Option) *docker.Container {
 			"--listen-client-urls=http://0.0.0.0:2379",
 		)
 	}
-	if _, found := optionsMap[VPPAgentContainerNetworking]; found {
-		hostConfig.NetworkMode = "container:vpp-agent-e2e-test"
+	if opts.UseAgentContainerForNetworking {
+		hostConfig.NetworkMode = fmt.Sprintf("container:e2e-test-vppagent-%v", AgentInstanceName(ec.ctx))
 	} else { // separate container networking (default)
 		hostConfig.PortBindings = map[docker.Port][]docker.PortBinding{
 			"2379/tcp": {{HostIP: "0.0.0.0", HostPort: "2379"}},
@@ -135,24 +133,6 @@ func (ec *EtcdContainer) create(options ...*Option) *docker.Container {
 	return container
 }
 
-// WithEtcdHTTPsConnection is ETCD test setup option that will use HTTPS connection to ETCD (by default it is used
-// unsecure HTTP connection)
-func WithEtcdHTTPsConnection() *Option {
-	return &Option{
-		key:   HTTPsConnection,
-		value: struct{}{}, // only presence is needed
-	}
-}
-
-// WithEtcdVPPAgentContainerNetworking is ETCD test setup option that will use VPP-Agent test container for
-// networking (by default the ETCD has separate networking)
-func WithEtcdVPPAgentContainerNetworking() *Option {
-	return &Option{
-		key:   VPPAgentContainerNetworking,
-		value: struct{}{}, // only presence is needed
-	}
-}
-
 func (ec *EtcdContainer) start(container *docker.Container) {
 	err := ec.ctx.dockerClient.StartContainer(container.ID, nil)
 	if err != nil {
diff --git a/tests/e2e/options.go b/tests/e2e/options.go
new file mode 100644
index 0000000000..b02349aa10
--- /dev/null
+++ b/tests/e2e/options.go
@@ -0,0 +1,202 @@
+// Copyright (c) 2020 Pantheon.tech
+//
+// 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 e2e
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"reflect"
+	"strings"
+
+	docker "github.com/fsouza/go-dockerclient"
+	. "github.com/onsi/gomega"
+	"go.ligato.io/cn-infra/v2/logging"
+)
+
+const vppAgentDefaultImg = "ligato/vpp-agent:latest"
+
+// SetupOpt is options data holder for customizing setup of tests
+type SetupOpt struct {
+	*AgentOpt
+	*EtcdOpt
+	SetupAgent bool
+	SetupEtcd  bool
+}
+
+// AgentOpt is options data holder for customizing setup of agent
+type AgentOpt struct {
+	Image                 string
+	Env                   []string
+	UseEtcd               bool
+	NoManualInitialResync bool
+	ContainerOptsHook     func(*docker.CreateContainerOptions)
+}
+
+// EtcdOpt is options data holder for customizing setup of ETCD
+type EtcdOpt struct {
+	UseHTTPS                       bool
+	UseAgentContainerForNetworking bool
+}
+
+// SetupOptModifier is function customizing general setup options
+type SetupOptModifier func(*SetupOpt)
+
+// AgentOptModifier is function customizing Agent setup options
+type AgentOptModifier func(*AgentOpt)
+
+// EtcdOptModifier is function customizing ETCD seup options
+type EtcdOptModifier func(*EtcdOpt)
+
+// DefaultSetupOpt creates default values for SetupOpt
+func DefaultSetupOpt() *SetupOpt {
+	opt := &SetupOpt{
+		AgentOpt:   DefaultAgentOpt(),
+		EtcdOpt:    DefaultEtcdOpt(),
+		SetupAgent: true,
+		SetupEtcd:  false,
+	}
+	return opt
+}
+
+// DefaultEtcdOpt creates default values for EtcdOpt
+func DefaultEtcdOpt() *EtcdOpt {
+	return &EtcdOpt{
+		UseHTTPS:                       false,
+		UseAgentContainerForNetworking: false,
+	}
+}
+
+// DefaultAgentOpt creates default values for AgentOpt
+func DefaultAgentOpt() *AgentOpt {
+	agentImg := vppAgentDefaultImg
+	if img := os.Getenv("VPP_AGENT"); img != "" {
+		agentImg = img
+	}
+	grpcConfig := "grpc.conf"
+	if val := os.Getenv("GRPC_CONFIG"); val != "" {
+		grpcConfig = val
+	}
+	etcdConfig := "DISABLED"
+	if val := os.Getenv("ETCD_CONFIG"); val != "" {
+		etcdConfig = val
+	}
+	opt := &AgentOpt{
+		Image:                 agentImg,
+		UseEtcd:               false,
+		NoManualInitialResync: false,
+		Env: []string{
+			"INITIAL_LOGLVL=" + logging.DefaultLogger.GetLevel().String(),
+			"ETCD_CONFIG=" + etcdConfig,
+			"GRPC_CONFIG=" + grpcConfig,
+		},
+	}
+	return opt
+}
+
+// WithoutVPPAgent is test setup option disabling vpp-agent setup
+func WithoutVPPAgent() SetupOptModifier {
+	return func(o *SetupOpt) {
+		o.SetupAgent = false
+	}
+}
+
+// WithEtcd is test setup option enabling vpp-agent setup
+func WithEtcd(etcdOpts ...EtcdOptModifier) SetupOptModifier {
+	return func(o *SetupOpt) {
+		o.SetupEtcd = true
+		if o.EtcdOpt == nil {
+			o.EtcdOpt = DefaultEtcdOpt()
+		}
+		for _, etcdOptModifier := range etcdOpts {
+			etcdOptModifier(o.EtcdOpt)
+		}
+	}
+}
+
+// WithoutManualInitialAgentResync is test setup option disabling manual agent resync just after agent setup
+func WithoutManualInitialAgentResync() AgentOptModifier {
+	return func(o *AgentOpt) {
+		o.NoManualInitialResync = true
+	}
+}
+
+// WithAdditionalAgentCmdParams is test setup option adding additional command line parameters to executing vpp-agent
+func WithAdditionalAgentCmdParams(params ...string) AgentOptModifier {
+	return func(o *AgentOpt) {
+		o.Env = append(o.Env, params...)
+	}
+}
+
+// WithPluginConfigArg persists configContent for give VPP-Agent plugin (expecting generic plugin config name)
+// and returns argument for VPP-Agent executable to use this plugin configuration file.
+func WithPluginConfigArg(ctx *TestCtx, pluginName string, configContent string) string {
+	configFilePath := CreateFileOnSharedVolume(ctx, fmt.Sprintf("%v.config", pluginName), configContent)
+	return fmt.Sprintf("%v_CONFIG=%v", strings.ToUpper(pluginName), configFilePath)
+}
+
+// FIXME container that will use it can have it mounted in different location as seen by the container where
+//  it is created (this works now due to the same mountpoint of shared volume in every container)
+
+// CreateFileOnSharedVolume persists fileContent to file in mounted shared volume used for sharing file
+// between containers. It returns the absolute path to the newly created file as seen by the container
+// that creates it.
+func CreateFileOnSharedVolume(ctx *TestCtx, simpleFileName string, fileContent string) string {
+	filePath, err := filepath.Abs(filepath.Join(ctx.testShareDir,
+		fmt.Sprintf("e2e-test-%v-%v", ctx.t.Name(), simpleFileName)))
+	Expect(err).To(Not(HaveOccurred()))
+	Expect(ioutil.WriteFile(filePath, []byte(fileContent), 0777)).To(Succeed())
+
+	// TODO register in context and delete in teardown? this doesn't matter
+	//  that much because file names contain unique test names so no file collision can happen
+	return filePath
+}
+
+// WithEtcdHTTPsConnection is ETCD test setup option that will use HTTPS connection to ETCD (by default it is used
+// unsecure HTTP connection)
+func WithEtcdHTTPsConnection() EtcdOptModifier {
+	return func(o *EtcdOpt) {
+		o.UseHTTPS = true
+	}
+}
+
+// WithEtcdVPPAgentContainerNetworking is ETCD test setup option that will use VPP-Agent test container for
+// networking (by default the ETCD has separate networking)
+func WithEtcdVPPAgentContainerNetworking() EtcdOptModifier {
+	return func(o *EtcdOpt) {
+		o.UseAgentContainerForNetworking = true
+	}
+}
+
+func extractEtcdOptions(opt *SetupOpt) EtcdOptModifier {
+	return func(etcdOpt *EtcdOpt) {
+		copyOptions(etcdOpt, opt.EtcdOpt)
+	}
+}
+
+func extractAgentOptions(opt *SetupOpt) AgentOptModifier {
+	return func(agentOpt *AgentOpt) {
+		copyOptions(agentOpt, opt.AgentOpt)
+	}
+}
+
+func copyOptions(to interface{}, from interface{}) {
+	fromVal := reflect.ValueOf(from).Elem()
+	toVal := reflect.ValueOf(to).Elem()
+	for i := 0; i < fromVal.NumField(); i++ {
+		toVal.Field(i).Set(fromVal.Field(i))
+	}
+}
diff --git a/tests/e2e/run_e2e.sh b/tests/e2e/run_e2e.sh
index b7aa167e0b..93e3711045 100755
--- a/tests/e2e/run_e2e.sh
+++ b/tests/e2e/run_e2e.sh
@@ -72,10 +72,10 @@ trap 'cleanup' EXIT
 echo "Creating volume for sharing files between containers.."
 if docker volume create "${sharevolumename}"
 then
-	echo >&2 -e " \e[32m...created\e[0m"
+	echo >&2 -e "\e[32m...created\e[0m"
 else
 	res=$?
-	echo >&2 -e " \e[31m...volume creation failed!\e[0m (exit code: $res)"
+	echo >&2 -e "\e[31m...volume creation failed!\e[0m (exit code: $res)"
 	exit $res
 fi
 
diff --git a/tests/e2e/vppagent.go b/tests/e2e/vppagent.go
index 8a2bb4180a..f55119da84 100644
--- a/tests/e2e/vppagent.go
+++ b/tests/e2e/vppagent.go
@@ -19,7 +19,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 	"path/filepath"
 	"testing"
 
@@ -32,47 +31,10 @@ import (
 )
 
 const (
-	vppAgentDefaultImg = "ligato/vpp-agent:latest"
 	vppAgentLabelKey   = "e2e.test.vppagent"
 	vppAgentNamePrefix = "e2e-test-vppagent-"
 )
 
-type AgentOpt struct {
-	Image                 string
-	Env                   []string
-	UseEtcd               bool
-	NoManualInitialResync bool
-	ContainerOptsHook     func(*docker.CreateContainerOptions)
-}
-
-type AgentOptModifier func(*AgentOpt)
-
-func DefaultAgentOpt() *AgentOpt {
-	agentImg := vppAgentDefaultImg
-	if img := os.Getenv("VPP_AGENT"); img != "" {
-		agentImg = img
-	}
-	grpcConfig := "grpc.conf"
-	if val := os.Getenv("GRPC_CONFIG"); val != "" {
-		grpcConfig = val
-	}
-	etcdConfig := "DISABLED"
-	if val := os.Getenv("ETCD_CONFIG"); val != "" {
-		etcdConfig = val
-	}
-	opt := &AgentOpt{
-		Image:                 agentImg,
-		UseEtcd:               false,
-		NoManualInitialResync: false,
-		Env: []string{
-			"INITIAL_LOGLVL=" + logging.DefaultLogger.GetLevel().String(),
-			"ETCD_CONFIG=" + etcdConfig,
-			"GRPC_CONFIG=" + grpcConfig,
-		},
-	}
-	return opt
-}
-
 type agent struct {
 	*AgentOpt
 	ctx         *TestCtx

From 26d9f981c8941c2cdb8a8c9d10e5a1b0feaa20d0 Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Thu, 17 Dec 2020 19:59:02 +0100
Subject: [PATCH 09/11] refactor: moved error handling from etcd to higher
 level

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 tests/e2e/000_initial_test.go |  8 +++++--
 tests/e2e/e2e.go              |  7 +++++--
 tests/e2e/etcd.go             | 39 ++++++++++++++++++++---------------
 3 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/tests/e2e/000_initial_test.go b/tests/e2e/000_initial_test.go
index a9d5ca0dbc..9fbec585e9 100644
--- a/tests/e2e/000_initial_test.go
+++ b/tests/e2e/000_initial_test.go
@@ -126,7 +126,9 @@ dial-timeout: 1s
 endpoints:
     - "%v:2379"
 `
-	etcdConfig = fmt.Sprintf(etcdConfig, ctx.Etcd.Inspect().NetworkSettings.IPAddress)
+	etcdContainer, err := ctx.Etcd.Inspect()
+	Expect(err).ShouldNot(HaveOccurred())
+	etcdConfig = fmt.Sprintf(etcdConfig, etcdContainer.NetworkSettings.IPAddress)
 
 	// create VPP-Agent
 	SetupVPPAgent(ctx,
@@ -222,7 +224,9 @@ dial-timeout: 1s
 endpoints:
     - "%v:2379"
 `
-	etcdConfig = fmt.Sprintf(etcdConfig, ctx.Etcd.Inspect().NetworkSettings.IPAddress)
+	etcdContainer, err := ctx.Etcd.Inspect()
+	Expect(err).ShouldNot(HaveOccurred())
+	etcdConfig = fmt.Sprintf(etcdConfig, etcdContainer.NetworkSettings.IPAddress)
 
 	// create VPP-Agent
 	SetupVPPAgent(ctx,
diff --git a/tests/e2e/e2e.go b/tests/e2e/e2e.go
index 87ed7e3c4d..ab5b39f600 100644
--- a/tests/e2e/e2e.go
+++ b/tests/e2e/e2e.go
@@ -161,7 +161,8 @@ func Setup(t *testing.T, options ...SetupOptModifier) *TestCtx {
 
 	// setup Etcd
 	if opt.SetupEtcd {
-		testCtx.Etcd = NewEtcdContainer(testCtx, extractEtcdOptions(opt))
+		testCtx.Etcd, err = NewEtcdContainer(testCtx, extractEtcdOptions(opt))
+		Expect(err).ShouldNot(HaveOccurred())
 	}
 
 	if opt.SetupAgent {
@@ -253,7 +254,9 @@ func (test *TestCtx) Teardown() {
 
 	// terminate etcd
 	if test.Etcd != nil {
-		test.Etcd.Terminate()
+		if err := test.Etcd.Terminate(); err != nil {
+			test.t.Logf("failed to terminate ETCD: %v", err)
+		}
 	}
 }
 
diff --git a/tests/e2e/etcd.go b/tests/e2e/etcd.go
index c1aee9297d..bd10c5085b 100644
--- a/tests/e2e/etcd.go
+++ b/tests/e2e/etcd.go
@@ -28,8 +28,6 @@ const (
 	etcdStopTimeout = 1 // seconds
 )
 
-// TODO unify whether return errors or use test context's log fatal
-
 // EtcdContainer is represents running ETCD container
 type EtcdContainer struct {
 	ctx         *TestCtx
@@ -37,14 +35,19 @@ type EtcdContainer struct {
 }
 
 // NewEtcdContainer creates and starts new ETCD container
-func NewEtcdContainer(ctx *TestCtx, options ...EtcdOptModifier) *EtcdContainer {
+func NewEtcdContainer(ctx *TestCtx, options ...EtcdOptModifier) (*EtcdContainer, error) {
 	ec := &EtcdContainer{
 		ctx: ctx,
 	}
-	container := ec.create(options...)
-	ec.start(container)
+	container, err := ec.create(options...)
+	if err != nil {
+		return nil, errors.Errorf("can't create ETCD container due to: %v", err)
+	}
+	if err := ec.start(container); err != nil {
+		return nil, errors.Errorf("can't start ETCD container due to: %v", err)
+	}
 	ec.containerID = container.ID
-	return ec
+	return ec, nil
 }
 
 // Put inserts key-value pair into the ETCD inside its running docker container
@@ -65,15 +68,15 @@ func (ec *EtcdContainer) GetAll() (string, error) {
 
 // Inspect provides docker.Container of running ETCD container that can be
 // used to inspect various things about ETCD container
-func (ec *EtcdContainer) Inspect() *docker.Container {
+func (ec *EtcdContainer) Inspect() (*docker.Container, error) {
 	container, err := ec.ctx.dockerClient.InspectContainer(ec.containerID)
 	if err != nil {
-		ec.ctx.t.Fatalf("failed to inspect container with ID %v due to: %v", ec.containerID, err)
+		return nil, errors.Errorf("failed to inspect container with ID %v due to: %v", ec.containerID, err)
 	}
-	return container
+	return container, nil
 }
 
-func (ec *EtcdContainer) create(options ...EtcdOptModifier) *docker.Container {
+func (ec *EtcdContainer) create(options ...EtcdOptModifier) (*docker.Container, error) {
 	opts := DefaultEtcdOpt()
 	for _, optionModifier := range options {
 		optionModifier(opts)
@@ -85,7 +88,7 @@ func (ec *EtcdContainer) create(options ...EtcdOptModifier) *docker.Container {
 		Tag:        "latest",
 	}, docker.AuthConfiguration{})
 	if err != nil {
-		ec.ctx.t.Fatalf("failed to pull ETCD image: %v", err)
+		return nil, errors.Errorf("failed to pull ETCD image: %v", err)
 	}
 
 	// construct command string and container host config
@@ -128,12 +131,12 @@ func (ec *EtcdContainer) create(options ...EtcdOptModifier) *docker.Container {
 		HostConfig: hostConfig,
 	})
 	if err != nil {
-		ec.ctx.t.Fatalf("failed to create ETCD container: %v", err)
+		return nil, errors.Errorf("failed to create ETCD container: %v", err)
 	}
-	return container
+	return container, nil
 }
 
-func (ec *EtcdContainer) start(container *docker.Container) {
+func (ec *EtcdContainer) start(container *docker.Container) error {
 	err := ec.ctx.dockerClient.StartContainer(container.ID, nil)
 	if err != nil {
 		err = ec.ctx.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
@@ -141,11 +144,12 @@ func (ec *EtcdContainer) start(container *docker.Container) {
 			Force: true,
 		})
 		if err != nil {
-			ec.ctx.t.Errorf("failed to remove ETCD container: %v", err)
+			return errors.Errorf("failed to remove ETCD container: %v", err)
 		}
-		ec.ctx.t.Fatalf("failed to start ETCD container: %v", err)
+		return errors.Errorf("failed to start ETCD container: %v", err)
 	}
 	ec.ctx.t.Logf("started ETCD container %v", container.ID)
+	return nil
 }
 
 // Terminate stops and removes the ETCD container
@@ -187,7 +191,8 @@ func (ec *EtcdContainer) exec(cmdName string, args ...string) (output string, er
 		Container:    ec.containerID,
 	})
 	if err != nil {
-		ec.ctx.t.Fatalf("failed to create docker exec instance for exec in etcd container: %v", err)
+		return "", errors.Errorf(
+			"failed to create docker exec instance for exec in etcd container: %v", err)
 	}
 
 	var stdout bytes.Buffer

From a7fe339b92deeddddb768b4c00ae953373f4262d Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Thu, 17 Dec 2020 20:14:12 +0100
Subject: [PATCH 10/11] refactor: just removed unwanted TODO

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 tests/e2e/000_initial_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/e2e/000_initial_test.go b/tests/e2e/000_initial_test.go
index 9fbec585e9..c0671b2978 100644
--- a/tests/e2e/000_initial_test.go
+++ b/tests/e2e/000_initial_test.go
@@ -59,10 +59,10 @@ func TestStartStopAgent(t *testing.T) {
 	}
 
 	ctx.StartAgent(agent1)
-	Eventually(msState).Should(Equal(kvscheduler.ValueState_OBTAINED)) // FIXME testing agent0 and not agent1
+	Eventually(msState).Should(Equal(kvscheduler.ValueState_OBTAINED))
 
 	ctx.StopAgent(agent1)
-	Eventually(msState).Should(Equal(kvscheduler.ValueState_NONEXISTENT)) // FIXME testing agent0 and not agent1 , did this test even passed with previous e2e changes?
+	Eventually(msState).Should(Equal(kvscheduler.ValueState_NONEXISTENT))
 }
 
 // TestInitFromFile tests configuring initial state of NB from file

From f0e46c0dfbef99ef6975ff519c5f600e5d9dd7ac Mon Sep 17 00:00:00 2001
From: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Date: Thu, 17 Dec 2020 20:33:36 +0100
Subject: [PATCH 11/11] fix: fixed failing e2e test

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
---
 tests/e2e/100_agentctl_test.go |  2 +-
 tests/e2e/etcd.go              | 16 ++++++++--------
 tests/e2e/options.go           | 14 +++++++-------
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/tests/e2e/100_agentctl_test.go b/tests/e2e/100_agentctl_test.go
index 01ab95a8bd..1708cadd26 100644
--- a/tests/e2e/100_agentctl_test.go
+++ b/tests/e2e/100_agentctl_test.go
@@ -365,7 +365,7 @@ func TestAgentCtlSecureGrpc(t *testing.T) {
 }
 
 func TestAgentCtlSecureETCD(t *testing.T) {
-	ctx := Setup(t, WithEtcd(WithEtcdHTTPsConnection(), WithEtcdVPPAgentContainerNetworking()))
+	ctx := Setup(t, WithEtcd(WithEtcdHTTPsConnection(), WithEtcdTestContainerNetworking()))
 	defer ctx.Teardown()
 
 	// test without any TLS
diff --git a/tests/e2e/etcd.go b/tests/e2e/etcd.go
index bd10c5085b..14eb399f99 100644
--- a/tests/e2e/etcd.go
+++ b/tests/e2e/etcd.go
@@ -16,8 +16,7 @@ package e2e
 
 import (
 	"bytes"
-	"fmt"
-	"os"
+	"path/filepath"
 
 	docker "github.com/fsouza/go-dockerclient"
 	"github.com/go-errors/errors"
@@ -105,15 +104,15 @@ func (ec *EtcdContainer) create(options ...EtcdOptModifier) (*docker.Container,
 			"--advertise-client-urls=https://127.0.0.1:2379",
 			"--listen-client-urls=https://127.0.0.1:2379",
 		)
-		hostConfig.Binds = []string{os.Getenv("CERTS_PATH") + ":/etc/certs"}
+		hostConfig.Binds = []string{filepath.Join(ec.ctx.testDataDir, "certs") + ":/etc/certs:ro"}
 	} else { // HTTP connection
 		cmd = append(cmd,
 			"--advertise-client-urls=http://0.0.0.0:2379",
 			"--listen-client-urls=http://0.0.0.0:2379",
 		)
 	}
-	if opts.UseAgentContainerForNetworking {
-		hostConfig.NetworkMode = fmt.Sprintf("container:e2e-test-vppagent-%v", AgentInstanceName(ec.ctx))
+	if opts.UseTestContainerForNetworking {
+		hostConfig.NetworkMode = "container:vpp-agent-e2e-test"
 	} else { // separate container networking (default)
 		hostConfig.PortBindings = map[docker.Port][]docker.PortBinding{
 			"2379/tcp": {{HostIP: "0.0.0.0", HostPort: "2379"}},
@@ -139,12 +138,13 @@ func (ec *EtcdContainer) create(options ...EtcdOptModifier) (*docker.Container,
 func (ec *EtcdContainer) start(container *docker.Container) error {
 	err := ec.ctx.dockerClient.StartContainer(container.ID, nil)
 	if err != nil {
-		err = ec.ctx.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
+		errRemove := ec.ctx.dockerClient.RemoveContainer(docker.RemoveContainerOptions{
 			ID:    container.ID,
 			Force: true,
 		})
-		if err != nil {
-			return errors.Errorf("failed to remove ETCD container: %v", err)
+		if errRemove != nil {
+			return errors.Errorf("failed to remove ETCD container: %v "+
+				"(after failed start due to: %v)", errRemove, err)
 		}
 		return errors.Errorf("failed to start ETCD container: %v", err)
 	}
diff --git a/tests/e2e/options.go b/tests/e2e/options.go
index b02349aa10..1146663c0b 100644
--- a/tests/e2e/options.go
+++ b/tests/e2e/options.go
@@ -48,8 +48,8 @@ type AgentOpt struct {
 
 // EtcdOpt is options data holder for customizing setup of ETCD
 type EtcdOpt struct {
-	UseHTTPS                       bool
-	UseAgentContainerForNetworking bool
+	UseHTTPS                      bool
+	UseTestContainerForNetworking bool
 }
 
 // SetupOptModifier is function customizing general setup options
@@ -75,8 +75,8 @@ func DefaultSetupOpt() *SetupOpt {
 // DefaultEtcdOpt creates default values for EtcdOpt
 func DefaultEtcdOpt() *EtcdOpt {
 	return &EtcdOpt{
-		UseHTTPS:                       false,
-		UseAgentContainerForNetworking: false,
+		UseHTTPS:                      false,
+		UseTestContainerForNetworking: false,
 	}
 }
 
@@ -173,11 +173,11 @@ func WithEtcdHTTPsConnection() EtcdOptModifier {
 	}
 }
 
-// WithEtcdVPPAgentContainerNetworking is ETCD test setup option that will use VPP-Agent test container for
+// WithEtcdTestContainerNetworking is ETCD test setup option that will use main Test container for
 // networking (by default the ETCD has separate networking)
-func WithEtcdVPPAgentContainerNetworking() EtcdOptModifier {
+func WithEtcdTestContainerNetworking() EtcdOptModifier {
 	return func(o *EtcdOpt) {
-		o.UseAgentContainerForNetworking = true
+		o.UseTestContainerForNetworking = true
 	}
 }