diff --git a/controller/interfaces/change/change.go b/controller/interfaces/change/change.go index bde850fe7969561982e27dc2c98579885ef6ce11..489b42303323759316675953d934b259cd5f7760 100644 --- a/controller/interfaces/change/change.go +++ b/controller/interfaces/change/change.go @@ -6,6 +6,7 @@ import ( mnepb "code.fbi.h-da.de/danet/gosdn/api/go/gosdn/networkelement" "github.com/google/uuid" + "github.com/openconfig/gnmi/proto/gnmi" ) // Change is an intended change to an MNE. It is unique and immutable. @@ -20,6 +21,7 @@ type Change interface { Confirm() error PreviousState() []byte IntendedState() []byte + Diff() *gnmi.Notification AssociatedDeviceID() uuid.UUID } @@ -28,4 +30,5 @@ type Change interface { type Payload struct { Original []byte Modified []byte + Diff *gnmi.Notification } diff --git a/controller/interfaces/transport/transport.go b/controller/interfaces/transport/transport.go index c9392787341fae1354b8bf1f6e45b1255519c837..586115cda3ff97831832ea16e5e7810f262cbd10 100644 --- a/controller/interfaces/transport/transport.go +++ b/controller/interfaces/transport/transport.go @@ -4,7 +4,6 @@ import ( "context" "code.fbi.h-da.de/danet/gosdn/controller/interfaces/change" - "code.fbi.h-da.de/danet/gosdn/controller/interfaces/plugin" gpb "github.com/openconfig/gnmi/proto/gnmi" ) @@ -13,7 +12,7 @@ import ( // like RESTCONF or gnmi. type Transport interface { Get(ctx context.Context, params ...string) (any, error) - Set(ctx context.Context, payload change.Payload, path string, plugin plugin.Plugin) error + Set(ctx context.Context, payload change.Payload) error CustomSet(ctx context.Context, req *gpb.SetRequest) (*gpb.SetResponse, error) Subscribe(ctx context.Context, params ...string) error ControlPlaneSubscribe(ctx context.Context, subscribeCallbackFunc HandleSubscribeResponse, diff --git a/controller/mocks/Change.go b/controller/mocks/Change.go index a9b23b912b03147475fb8cca6938eb9b81e48203..5faf6ff7bd010cd28d6789a87f61aba07efa387a 100644 --- a/controller/mocks/Change.go +++ b/controller/mocks/Change.go @@ -3,10 +3,12 @@ package mocks import ( - time "time" + gnmi "github.com/openconfig/gnmi/proto/gnmi" + mock "github.com/stretchr/testify/mock" networkelement "code.fbi.h-da.de/danet/gosdn/api/go/gosdn/networkelement" - mock "github.com/stretchr/testify/mock" + + time "time" uuid "github.com/google/uuid" ) @@ -74,6 +76,22 @@ func (_m *Change) Confirm() error { return r0 } +// Diff provides a mock function with given fields: +func (_m *Change) Diff() *gnmi.Notification { + ret := _m.Called() + + var r0 *gnmi.Notification + if rf, ok := ret.Get(0).(func() *gnmi.Notification); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*gnmi.Notification) + } + } + + return r0 +} + // ID provides a mock function with given fields: func (_m *Change) ID() uuid.UUID { ret := _m.Called() diff --git a/controller/mocks/Transport.go b/controller/mocks/Transport.go index 5e1615014668c6eb5d6a7f4cf3615b4e646e0b7b..b3a01d0af93be7841901f15c2da8a267554bf529 100644 --- a/controller/mocks/Transport.go +++ b/controller/mocks/Transport.go @@ -11,8 +11,6 @@ import ( mock "github.com/stretchr/testify/mock" - plugin "code.fbi.h-da.de/danet/gosdn/controller/interfaces/plugin" - transport "code.fbi.h-da.de/danet/gosdn/controller/interfaces/transport" ) @@ -122,13 +120,13 @@ func (_m *Transport) ProcessResponse(resp interface{}) error { return r0 } -// Set provides a mock function with given fields: ctx, payload, path, _a3 -func (_m *Transport) Set(ctx context.Context, payload change.Payload, path string, _a3 plugin.Plugin) error { - ret := _m.Called(ctx, payload, path, _a3) +// Set provides a mock function with given fields: ctx, payload +func (_m *Transport) Set(ctx context.Context, payload change.Payload) error { + ret := _m.Called(ctx, payload) var r0 error - if rf, ok := ret.Get(0).(func(context.Context, change.Payload, string, plugin.Plugin) error); ok { - r0 = rf(ctx, payload, path, _a3) + if rf, ok := ret.Get(0).(func(context.Context, change.Payload) error); ok { + r0 = rf(ctx, payload) } else { r0 = ret.Error(0) } diff --git a/controller/northbound/server/networkElement.go b/controller/northbound/server/networkElement.go index 6a433b7dea0888bf04f1be9c8da200e161bfe1e6..4792916171612d108fec655cd4a6e2476b5362a2 100644 --- a/controller/northbound/server/networkElement.go +++ b/controller/northbound/server/networkElement.go @@ -18,6 +18,7 @@ import ( "code.fbi.h-da.de/danet/gosdn/controller/metrics" "code.fbi.h-da.de/danet/gosdn/controller/nucleus" "code.fbi.h-da.de/danet/gosdn/controller/nucleus/types" + util "code.fbi.h-da.de/danet/gosdn/controller/nucleus/util/gnmi" "code.fbi.h-da.de/danet/gosdn/controller/store" aGNMI "code.fbi.h-da.de/danet/gosdn/forks/goarista/gnmi" "github.com/google/uuid" @@ -854,22 +855,27 @@ func (n *NetworkElementServer) ChangeMNE(duid uuid.UUID, operation mnepb.ApiOper } } - callback := func(original, modified []byte) error { - ctx := context.WithValue(context.Background(), types.CtxKeyOperation, operation) // nolint - payload := change.Payload{Original: original, Modified: modified} - pathToSet := path - if err := mne.Transport().Set(ctx, payload, pathToSet, plugin); err != nil { - return err - } - return n.mneService.Update(mne) + currentModel, err := mne.GetModelAsFilteredCopy() + if err != nil { + return uuid.Nil, err } - currentModel, err := mne.GetModelAsFilteredCopy() + diff, err := plugin.Diff(currentModel, filteredMarshaledModel) if err != nil { return uuid.Nil, err } - ch := nucleus.NewChange(duid, currentModel, filteredMarshaledModel, callback) + if util.IsGNMINotificationEmpty(diff) { + return uuid.Nil, customerrs.NoNewChangesError{Original: string(currentModel), Modified: string(filteredMarshaledModel)} + } + + callback := func(original, modified []byte) error { + ctx := context.WithValue(context.Background(), types.CtxKeyOperation, operation) // nolint + payload := change.Payload{Original: original, Modified: modified, Diff: diff} + return mne.Transport().Set(ctx, payload) + } + + ch := nucleus.NewChange(duid, currentModel, filteredMarshaledModel, diff, callback) if err := n.changeStore.Add(ch); err != nil { return uuid.Nil, err diff --git a/controller/nucleus/change.go b/controller/nucleus/change.go index b1bc268a5e2428390609f9a431a64317d54e5f28..0cdecdcc291ccaae1447fd7a1aeb47546a76db53 100644 --- a/controller/nucleus/change.go +++ b/controller/nucleus/change.go @@ -10,6 +10,7 @@ import ( mnepb "code.fbi.h-da.de/danet/gosdn/api/go/gosdn/networkelement" "github.com/google/uuid" + "github.com/openconfig/gnmi/proto/gnmi" log "github.com/sirupsen/logrus" ) @@ -33,13 +34,14 @@ func init() { // a callback function, and returns a *Change. // The callback function is used by the Commit() and Confirm() functions. It // must define how the change is carried out. -func NewChange(mne uuid.UUID, currentState, change []byte, callback func([]byte, []byte) error) *Change { +func NewChange(mne uuid.UUID, currentState, change []byte, diff *gnmi.Notification, callback func([]byte, []byte) error) *Change { c := &Change{ cuid: uuid.New(), duid: mne, state: mnepb.ChangeState_CHANGE_STATE_PENDING, timestamp: time.Now(), previousState: currentState, + diff: diff, intendedState: change, callback: callback, } @@ -63,6 +65,7 @@ type Change struct { timestamp time.Time previousState []byte intendedState []byte + diff *gnmi.Notification callback func([]byte, []byte) error stateMu sync.RWMutex errChan <-chan error @@ -132,6 +135,12 @@ func (c *Change) IntendedState() []byte { return c.intendedState } +// Diff returns the differences between the previous and the intended state as +// gnmi.Notification. +func (c *Change) Diff() *gnmi.Notification { + return c.diff +} + func stateManager(ctx context.Context, ch *Change, timeout time.Duration) (chan<- mnepb.ChangeState, <-chan mnepb.ChangeState, <-chan error) { stateIn := make(chan mnepb.ChangeState) stateOut := make(chan mnepb.ChangeState) diff --git a/controller/nucleus/gnmi_transport.go b/controller/nucleus/gnmi_transport.go index 20176b1f1027addcecaa42b6c3348a5b6a0d4d46..6494520e3a6eedbb48533ab24daf350f2c5bae2f 100644 --- a/controller/nucleus/gnmi_transport.go +++ b/controller/nucleus/gnmi_transport.go @@ -5,7 +5,6 @@ import ( "fmt" "code.fbi.h-da.de/danet/gosdn/controller/interfaces/change" - "code.fbi.h-da.de/danet/gosdn/controller/interfaces/plugin" tpInterface "code.fbi.h-da.de/danet/gosdn/controller/interfaces/transport" "code.fbi.h-da.de/danet/gosdn/controller/plugin/shared" @@ -13,7 +12,6 @@ import ( "code.fbi.h-da.de/danet/gosdn/controller/nucleus/types" "code.fbi.h-da.de/danet/gosdn/forks/goarista/gnmi" gpb "github.com/openconfig/gnmi/proto/gnmi" - "github.com/openconfig/ygot/ygot" log "github.com/sirupsen/logrus" tpb "code.fbi.h-da.de/danet/gosdn/api/go/gosdn/transport" @@ -87,39 +85,17 @@ func (g *Gnmi) Get(ctx context.Context, params ...string) (interface{}, error) { } // Set takes a change.Payload struct. -func (g *Gnmi) Set(ctx context.Context, payload change.Payload, path string, plugin plugin.Plugin) error { - p, err := ygot.StringToStructuredPath(path) - if err != nil { - return err - } +func (g *Gnmi) Set(ctx context.Context, payload change.Payload) error { if g.client == nil { return &customerrs.NilClientError{} } ctx = gnmi.NewContext(ctx, g.config) - return g.applyDiff(ctx, payload, p, plugin) -} - -// isGNMINotificationEmpty checks if the given gnmi.Notification does not -// contain any updates or deletes. -func isGNMINotificationEmpty(n *gpb.Notification) bool { - if n.Update == nil || len(n.Update) == 0 { - if n.Delete == nil || len(n.Delete) == 0 { - return true - } - } - return false + return g.applyDiff(ctx, payload) } -func (g *Gnmi) applyDiff(ctx context.Context, payload change.Payload, path *gpb.Path, plugin plugin.Plugin) error { - diff, err := plugin.Diff(payload.Original, payload.Modified) - if err != nil { - return err - } - - if isGNMINotificationEmpty(diff) { - return customerrs.NoNewChangesError{Original: string(payload.Original), Modified: string(payload.Modified)} - } +func (g *Gnmi) applyDiff(ctx context.Context, payload change.Payload) error { + diff := payload.Diff updates := diff.GetUpdate() deletes := diff.GetDelete() @@ -136,20 +112,6 @@ func (g *Gnmi) applyDiff(ctx context.Context, payload change.Payload, path *gpb. } log.Info(resp) - // apply diffs to the plugin of the managed network element. - for _, update := range updates { - err := plugin.SetNode(update.GetPath(), update.GetVal()) - if err != nil { - return err - } - } - for _, del := range deletes { - err := plugin.DeleteNode(del) - if err != nil { - return err - } - } - return nil } diff --git a/controller/nucleus/gnmi_transport_test.go b/controller/nucleus/gnmi_transport_test.go index 901275d633f0cb653c25c83a908e91c454b497a5..e7230b1af6932ce4d135e5ce2ac34f98fad69374 100644 --- a/controller/nucleus/gnmi_transport_test.go +++ b/controller/nucleus/gnmi_transport_test.go @@ -3,7 +3,6 @@ package nucleus import ( "context" "errors" - "fmt" "reflect" "testing" @@ -265,33 +264,10 @@ func TestGnmi_ProcessResponse(t *testing.T) { } func TestGnmi_Set(t *testing.T) { - mockPlugin := mockPlugin(t) - mockCall := mockPlugin.(*mocks.Plugin).On("Diff", mock.IsType([]byte{}), mock.IsType([]byte{})) - mockCall.RunFn = func(args mock.Arguments) { - path, err := ygot.StringToStructuredPath("/system/config/hostname") - if err != nil { - mockCall.ReturnArguments = mock.Arguments{nil, fmt.Errorf("StringToStructuredPath failed in ReturnArguments")} - } - if string(args.Get(0).([]byte)) == "" { - mockCall.ReturnArguments = mock.Arguments{&gpb.Notification{}, nil} - } else if string(args.Get(0).([]byte)) == "update" { - mockCall.ReturnArguments = mock.Arguments{&gpb.Notification{ - Update: []*gpb.Update{{Path: path, Val: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "newName"}}}}, - }, - nil, - } - } else if string(args.Get(0).([]byte)) == "delete" { - mockCall.ReturnArguments = mock.Arguments{&gpb.Notification{ - Delete: []*gpb.Path{path}, - }, - nil, - } - } + hostnamePath, err := ygot.StringToStructuredPath("system/config/hostname") + if err != nil { + t.Fatal(err) } - mockPlugin.(*mocks.Plugin).On("Unmarshal", mock.Anything, mock.Anything).Return(nil) - mockPlugin.(*mocks.Plugin).On("SetNode", mock.Anything, mock.Anything).Return(nil) - mockPlugin.(*mocks.Plugin).On("DeleteNode", mock.Anything).Return(nil) - setResponse := &gpb.SetResponse{} type fields struct { @@ -309,16 +285,6 @@ func TestGnmi_Set(t *testing.T) { args args wantErr bool }{ - { - name: "uninitialised", - fields: fields{transport: mockTransport(t)}, - args: args{ - payload: change.Payload{}, - path: "/", - ctx: context.WithValue(context.Background(), types.CtxKeyOperation, mnepb.ApiOperation_API_OPERATION_UPDATE), // nolint - }, - wantErr: true, - }, { name: "updateValue", fields: fields{ @@ -343,6 +309,18 @@ func TestGnmi_Set(t *testing.T) { payload: change.Payload{ Original: []byte("update"), Modified: []byte("update2"), + Diff: &gpb.Notification{ + Update: []*gpb.Update{ + { + Path: hostnamePath, + Val: &gpb.TypedValue{ + Value: &gpb.TypedValue_StringVal{ + StringVal: "newName", + }, + }, + }, + }, + }, }, path: "/system/config/hostname", ctx: context.WithValue(context.Background(), types.CtxKeyOperation, mnepb.ApiOperation_API_OPERATION_UPDATE), // nolint @@ -365,6 +343,11 @@ func TestGnmi_Set(t *testing.T) { payload: change.Payload{ Original: []byte("delete"), Modified: []byte("delete2"), + Diff: &gpb.Notification{ + Delete: []*gpb.Path{ + hostnamePath, + }, + }, }, path: "/system/config/hostname", ctx: context.WithValue(context.Background(), types.CtxKeyOperation, mnepb.ApiOperation_API_OPERATION_DELETE), // nolint @@ -376,7 +359,7 @@ func TestGnmi_Set(t *testing.T) { t.Run(tt.name, func(t *testing.T) { tt.fields.transport.client.(*mocks.GNMIClient). On("Set", mockContext, tt.fields.mockArgumentMatcher).Return(setResponse, nil) - err := tt.fields.transport.Set(tt.args.ctx, tt.args.payload, tt.args.path, mockPlugin) + err := tt.fields.transport.Set(tt.args.ctx, tt.args.payload) if (err != nil) != tt.wantErr { t.Errorf("Set() error = %v, wantErr %v", err, tt.wantErr) } diff --git a/controller/nucleus/util/gnmi/notification.go b/controller/nucleus/util/gnmi/notification.go new file mode 100644 index 0000000000000000000000000000000000000000..407faf3b1f7eb4d470657f2b7c7c3fbd0334e7ba --- /dev/null +++ b/controller/nucleus/util/gnmi/notification.go @@ -0,0 +1,14 @@ +package gnmi + +import "github.com/openconfig/gnmi/proto/gnmi" + +// IsGNMINotificationEmpty checks if the given gnmi.Notification does not +// contain any updates or deletes. +func IsGNMINotificationEmpty(n *gnmi.Notification) bool { + if n.Update == nil || len(n.Update) == 0 { + if n.Delete == nil || len(n.Delete) == 0 { + return true + } + } + return false +}