diff --git a/api/initialise_test.go b/api/initialise_test.go index cc90b5709b6bd4d17bbfdc4cbf44545a32cd48d3..11ea37d71cc0c0c4464ac9bbf54c403b43c51e26 100644 --- a/api/initialise_test.go +++ b/api/initialise_test.go @@ -87,7 +87,7 @@ func bootstrapUnitTest() { mockPnd.On("Confirm", mock.Anything).Return(nil) mockPnd.On("Devices").Return([]uuid.UUID{deviceUUID}) mockPnd.On("GetSBIs").Return(sbiStore) - mockPnd.On("ChangeOND", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + mockPnd.On("ChangeOND", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(uuid.Nil, nil) if err := pndStore.Add(&mockPnd); err != nil { log.Fatal(err) diff --git a/api/pnd.go b/api/pnd.go index 976fdb0ac17efa80e945babe8a83f040adaf6080..fb6203ba995df495fe5b3cc2fd456009d56bf844 100644 --- a/api/pnd.go +++ b/api/pnd.go @@ -11,6 +11,7 @@ import ( "code.fbi.h-da.de/cocsn/gosdn/nucleus/errors" "github.com/google/uuid" log "github.com/sirupsen/logrus" + "google.golang.org/protobuf/proto" ) // PrincipalNetworkDomainAdapter is an API adapter to reflect the NetworkDomain @@ -91,28 +92,27 @@ func (p *PrincipalNetworkDomainAdapter) Devices() []uuid.UUID { // ChangeOND sends an API call to the controller requesting the creation of // a change from the provided Operation, path and value. The Change is marked // as Pending and times out after the specified timeout period -func (p *PrincipalNetworkDomainAdapter) ChangeOND(uuid uuid.UUID, operation ppb.ApiOperation, path string, value ...string) error { +func (p *PrincipalNetworkDomainAdapter) ChangeOND(duid uuid.UUID, operation ppb.ApiOperation, path string, value ...string) (uuid.UUID, error) { var v string if len(value) != 0 { v = value[0] } - resp, err := changeRequest(p.endpoint, uuid.String(), p.id.String(), path, v, operation) + resp, err := changeRequest(p.endpoint, duid.String(), p.id.String(), path, v, operation) if err != nil { - return err + return uuid.Nil, err } log.Info(resp) - return err + return uuid.Nil, err } // Request sends an API call to the controller requesting the specified path // for the specified device -func (p *PrincipalNetworkDomainAdapter) Request(did uuid.UUID, path string) error { +func (p *PrincipalNetworkDomainAdapter) Request(did uuid.UUID, path string) (proto.Message, error) { resp, err := getPath(p.endpoint, p.id.String(), did.String(), path) if err != nil { - return err + return nil, err } - log.Info(resp) - return nil + return resp, nil } // RequestAll sends an API call to the controller requesting the specified path diff --git a/api/pnd_test.go b/api/pnd_test.go index f10c4d7b364185ffa44ea8d845d7e490a50922e8..1e11ec1c703832ae737c1d322ec0d47978e35d3a 100644 --- a/api/pnd_test.go +++ b/api/pnd_test.go @@ -269,7 +269,8 @@ func TestPrincipalNetworkDomainAdapter_ChangeOND(t *testing.T) { id: tt.fields.id, endpoint: tt.fields.endpoint, } - if err := p.ChangeOND(tt.args.uuid, tt.args.operation, tt.args.path, tt.args.value...); (err != nil) != tt.wantErr { + _, err := p.ChangeOND(tt.args.uuid, tt.args.operation, tt.args.path, tt.args.value...) + if (err != nil) != tt.wantErr { t.Errorf("PrincipalNetworkDomainAdapter.ChangeOND() error = %v, wantErr %v", err, tt.wantErr) } }) @@ -299,7 +300,8 @@ func TestPrincipalNetworkDomainAdapter_Request(t *testing.T) { id: tt.fields.id, endpoint: tt.fields.endpoint, } - if err := p.Request(tt.args.did, tt.args.path); (err != nil) != tt.wantErr { + _, err := p.Request(tt.args.did, tt.args.path) + if (err != nil) != tt.wantErr { t.Errorf("PrincipalNetworkDomainAdapter.Request() error = %v, wantErr %v", err, tt.wantErr) } }) diff --git a/interfaces/change/change.go b/interfaces/change/change.go index e195c02b16a0ac5fe1ef38c61f8a4a437d1bddbe..493c2183fb3cb3b416badaf39d1c9e449f7e1db4 100644 --- a/interfaces/change/change.go +++ b/interfaces/change/change.go @@ -3,6 +3,7 @@ package change import ( ppb "code.fbi.h-da.de/cocsn/api/go/gosdn/pnd" "github.com/google/uuid" + "github.com/openconfig/ygot/ygot" ) // Change is an intended change to an OND. It is unique and immutable. @@ -15,3 +16,10 @@ type Change interface { Confirm() error State() ppb.Change_State } + +// Payload contains two ygot.GoStructs, the first represents the original state +// before the change was applied and the second repesents the modified state. +type Payload struct { + Original ygot.GoStruct + Modified ygot.GoStruct +} diff --git a/interfaces/networkdomain/pnd.go b/interfaces/networkdomain/pnd.go index b723d723470af129644b2a0f62f882abf0163614..6438f192e6f6189612743931a7acacee7e128b17 100644 --- a/interfaces/networkdomain/pnd.go +++ b/interfaces/networkdomain/pnd.go @@ -8,6 +8,7 @@ import ( "code.fbi.h-da.de/cocsn/gosdn/interfaces/southbound" "code.fbi.h-da.de/cocsn/gosdn/interfaces/store" "github.com/google/uuid" + "google.golang.org/protobuf/proto" ) // NetworkDomain provides an interface for network domain implementations @@ -20,8 +21,8 @@ type NetworkDomain interface { GetDevice(identifier string) (device.Device, error) RemoveDevice(uuid.UUID) error Devices() []uuid.UUID - ChangeOND(uuid uuid.UUID, operation ppb.ApiOperation, path string, value ...string) error - Request(uuid.UUID, string) error + ChangeOND(uuid uuid.UUID, operation ppb.ApiOperation, path string, value ...string) (uuid.UUID, error) + Request(uuid.UUID, string) (proto.Message, error) RequestAll(string) error GetName() string GetDescription() string diff --git a/interfaces/transport/transport.go b/interfaces/transport/transport.go index 24bc2ab02884dbb6fd8fe446e0cfca2aa0ce3bc4..280233b67cc25a8714d4fec24331891c204ee9f0 100644 --- a/interfaces/transport/transport.go +++ b/interfaces/transport/transport.go @@ -3,6 +3,8 @@ package transport import ( "context" + "code.fbi.h-da.de/cocsn/gosdn/interfaces/change" + "github.com/openconfig/ygot/ytypes" ) @@ -10,7 +12,7 @@ import ( // like RESTCONF or gnmi type Transport interface { Get(ctx context.Context, params ...string) (interface{}, error) - Set(ctx context.Context, params ...interface{}) error + Set(ctx context.Context, payload change.Payload) error Subscribe(ctx context.Context, params ...string) error Type() string ProcessResponse(resp interface{}, root interface{}, models *ytypes.Schema) error diff --git a/mocks/NetworkDomain.go b/mocks/NetworkDomain.go index 86e84069386bcc0247d8d9b10b3b5c8563c32bf7..6d29a808f7463e9ccfbc6bbf453da8e7a03339dd 100644 --- a/mocks/NetworkDomain.go +++ b/mocks/NetworkDomain.go @@ -10,6 +10,8 @@ import ( pnd "code.fbi.h-da.de/cocsn/api/go/gosdn/pnd" + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + southbound "code.fbi.h-da.de/cocsn/gosdn/interfaces/southbound" store "code.fbi.h-da.de/cocsn/gosdn/interfaces/store" @@ -53,7 +55,7 @@ func (_m *NetworkDomain) AddSbi(s southbound.SouthboundInterface) error { } // ChangeOND provides a mock function with given fields: _a0, operation, path, value -func (_m *NetworkDomain) ChangeOND(_a0 uuid.UUID, operation pnd.ApiOperation, path string, value ...string) error { +func (_m *NetworkDomain) ChangeOND(_a0 uuid.UUID, operation pnd.ApiOperation, path string, value ...string) (uuid.UUID, error) { _va := make([]interface{}, len(value)) for _i := range value { _va[_i] = value[_i] @@ -63,14 +65,23 @@ func (_m *NetworkDomain) ChangeOND(_a0 uuid.UUID, operation pnd.ApiOperation, pa _ca = append(_ca, _va...) ret := _m.Called(_ca...) - var r0 error - if rf, ok := ret.Get(0).(func(uuid.UUID, pnd.ApiOperation, string, ...string) error); ok { + var r0 uuid.UUID + if rf, ok := ret.Get(0).(func(uuid.UUID, pnd.ApiOperation, string, ...string) uuid.UUID); ok { r0 = rf(_a0, operation, path, value...) } else { - r0 = ret.Error(0) + if ret.Get(0) != nil { + r0 = ret.Get(0).(uuid.UUID) + } } - return r0 + var r1 error + if rf, ok := ret.Get(1).(func(uuid.UUID, pnd.ApiOperation, string, ...string) error); ok { + r1 = rf(_a0, operation, path, value...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } // Commit provides a mock function with given fields: _a0 @@ -333,17 +344,26 @@ func (_m *NetworkDomain) RemoveSbi(_a0 uuid.UUID) error { } // Request provides a mock function with given fields: _a0, _a1 -func (_m *NetworkDomain) Request(_a0 uuid.UUID, _a1 string) error { +func (_m *NetworkDomain) Request(_a0 uuid.UUID, _a1 string) (protoreflect.ProtoMessage, error) { ret := _m.Called(_a0, _a1) - var r0 error - if rf, ok := ret.Get(0).(func(uuid.UUID, string) error); ok { + var r0 protoreflect.ProtoMessage + if rf, ok := ret.Get(0).(func(uuid.UUID, string) protoreflect.ProtoMessage); ok { r0 = rf(_a0, _a1) } else { - r0 = ret.Error(0) + if ret.Get(0) != nil { + r0 = ret.Get(0).(protoreflect.ProtoMessage) + } } - return r0 + var r1 error + if rf, ok := ret.Get(1).(func(uuid.UUID, string) error); ok { + r1 = rf(_a0, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } // RequestAll provides a mock function with given fields: _a0 diff --git a/mocks/Transport.go b/mocks/Transport.go index 4ba9ae182ce9df5909230a780182e161c6b4052e..0651c3872fe103856d439b68be58e6552977be53 100644 --- a/mocks/Transport.go +++ b/mocks/Transport.go @@ -5,6 +5,8 @@ package mocks import ( context "context" + change "code.fbi.h-da.de/cocsn/gosdn/interfaces/change" + mock "github.com/stretchr/testify/mock" ytypes "github.com/openconfig/ygot/ytypes" @@ -59,16 +61,13 @@ func (_m *Transport) ProcessResponse(resp interface{}, root interface{}, models return r0 } -// Set provides a mock function with given fields: ctx, params -func (_m *Transport) Set(ctx context.Context, params ...interface{}) error { - var _ca []interface{} - _ca = append(_ca, ctx) - _ca = append(_ca, params...) - ret := _m.Called(_ca...) +// 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, ...interface{}) error); ok { - r0 = rf(ctx, params...) + 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/northbound/server/pnd.go b/northbound/server/pnd.go index 8b69680c016502ef5084815319b8cd9a5d21118e..42168323f949af46682f7246ca54cd7882a0aaaa 100644 --- a/northbound/server/pnd.go +++ b/northbound/server/pnd.go @@ -157,7 +157,8 @@ func handleGetPath(pid uuid.UUID, req *ppb.GetRequest_Path) (*ppb.GetResponse, e log.Error(err) return nil, status.Errorf(codes.Aborted, "%v", err) } - if err := pnd.Request(duid, req.Path.Path); err != nil { + _, err = pnd.Request(duid, req.Path.Path) + if err != nil { log.Error(err) return nil, status.Errorf(codes.Aborted, "%v", err) } @@ -399,7 +400,9 @@ func handleChangeRequest(pnd networkdomain.NetworkDomain, req []*ppb.ChangeReque log.Error(err) return nil, status.Errorf(codes.Aborted, "%v", err) } - if err := pnd.ChangeOND(did, r.ApiOp, r.Path, r.Value); err != nil { + // TODO: Return CUID in API + _, err = pnd.ChangeOND(did, r.ApiOp, r.Path, r.Value) + if err != nil { log.Error(err) return nil, status.Errorf(codes.Aborted, "%v", err) } diff --git a/northbound/server/pnd_test.go b/northbound/server/pnd_test.go index 0d25e79ca28e1b778b452ad2781e7a0c8169f6a1..0137cde1a7d54bd6aab8bd48a563a3a7f55111c5 100644 --- a/northbound/server/pnd_test.go +++ b/northbound/server/pnd_test.go @@ -88,7 +88,7 @@ func TestMain(m *testing.M) { mockPnd.On("GetDevice", mock.Anything).Return(mockDevice, nil) mockPnd.On("Commit", mock.Anything).Return(nil) mockPnd.On("Confirm", mock.Anything).Return(nil) - mockPnd.On("ChangeOND", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + mockPnd.On("ChangeOND", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(uuid.Nil, nil) pndc = nucleus.NewPndStore() if err := pndc.Add(mockPnd); err != nil { diff --git a/nucleus/change.go b/nucleus/change.go index 75c54d37850780f011ffcc7330b03bbec3573a4d..391a90bfe4da99142f820e840e6b0f262151ac32 100644 --- a/nucleus/change.go +++ b/nucleus/change.go @@ -79,6 +79,9 @@ func (c *Change) ID() uuid.UUID { // and starts the timeout-timer for the Change. If the timer expires // the change is rolled back. func (c *Change) Commit() error { + if c.committed { + return fmt.Errorf("change %v already committed", c.cuid) + } c.commit <- c select { case err := <-c.errChan: diff --git a/nucleus/change_test.go b/nucleus/change_test.go index 4f35b89e4c6c29d746c91bdc5f3bf11994d4d81e..422b172619329a65189cf6acb130fc554ca410ed 100644 --- a/nucleus/change_test.go +++ b/nucleus/change_test.go @@ -132,7 +132,6 @@ func TestChange_CommitError(t *testing.T) { } func TestChange_Commit(t *testing.T) { - wantErr := false want := ppb.Change_COMMITTED commit, confirm, out := stateManager(time.Millisecond * 100) @@ -152,8 +151,8 @@ func TestChange_Commit(t *testing.T) { }, errChan: make(chan error, 10), } - if err := c.Commit(); (err != nil) != wantErr { - t.Errorf("Commit() error = %v, wantErr %v", err, wantErr) + if err := c.Commit(); err != nil { + t.Errorf("Commit() error = %v", err) } got := c.State() if !reflect.DeepEqual(got, want) { diff --git a/nucleus/gnmi_transport.go b/nucleus/gnmi_transport.go index 68568f7033cea220059862a228264e481a4a2f6e..fc736ac7adfa01cdd3e7f031d9554f70ff910ea4 100644 --- a/nucleus/gnmi_transport.go +++ b/nucleus/gnmi_transport.go @@ -5,6 +5,8 @@ import ( "fmt" "reflect" + "code.fbi.h-da.de/cocsn/gosdn/interfaces/change" + "code.fbi.h-da.de/cocsn/gosdn/interfaces/southbound" "google.golang.org/grpc" @@ -16,7 +18,6 @@ import ( "code.fbi.h-da.de/cocsn/gosdn/nucleus/types" pathutils "code.fbi.h-da.de/cocsn/gosdn/nucleus/util/path" gpb "github.com/openconfig/gnmi/proto/gnmi" - "github.com/openconfig/gnmi/proto/gnmi_ext" "github.com/openconfig/goyang/pkg/yang" "github.com/openconfig/ygot/ygot" "github.com/openconfig/ygot/ytypes" @@ -25,12 +26,6 @@ import ( tpb "code.fbi.h-da.de/cocsn/api/go/gosdn/transport" ) -var opmap = map[ppb.ApiOperation]string{ - ppb.ApiOperation_UPDATE: "update", - ppb.ApiOperation_REPLACE: "replace", - ppb.ApiOperation_DELETE: "delete", -} - // Gnmi implements the Transport interface and provides an SBI with the // possibility to access a gNMI endpoint. type Gnmi struct { @@ -85,117 +80,23 @@ func (g *Gnmi) Get(ctx context.Context, params ...string) (interface{}, error) { if g.client == nil { return nil, &errors.ErrNilClient{} } + ctx = gnmi.NewContext(ctx, g.config) paths := gnmi.SplitPaths(params) return g.get(ctx, paths, "") } -// Set takes a slice of params. This slice must contain at least one operation. -// It can contain an additional arbitrary amount of operations and extensions. -func (g *Gnmi) Set(ctx context.Context, args ...interface{}) error { +// Set takes a change.Payload struct. +func (g *Gnmi) Set(ctx context.Context, payload change.Payload) error { if g.client == nil { return &errors.ErrNilClient{} } - if len(args) == 0 { - return &errors.ErrInvalidParameters{ - Func: "nucleus.Set()", - Param: "no parameters provided", - } - } - - // look at args, unpack any GoStructs present - goStructs := make([]ygot.GoStruct, 0) - for _, arg := range args { - switch a := arg.(type) { - case ygot.GoStruct: - goStructs = append(goStructs, a) - default: - } - } - - if len(goStructs) == 2 { - return g.applyDiff(ctx, goStructs) - } - - opts := make([]interface{}, 0) - for _, o := range args { - attrs, ok := o.([]string) - if !ok { - return &errors.ErrInvalidTypeAssertion{ - Value: o, - Type: reflect.TypeOf("placeholder"), - } - } else if len(attrs) == 0 { - return &errors.ErrInvalidParameters{ - Func: "nucleus.Set()", - Param: "no parameters provided", - } - } - opts = append(opts, &gnmi.Operation{ - // Hardcoded TransportUpdate until multiple operations are supported - Type: opmap[ppb.ApiOperation_UPDATE], - Origin: "", - Target: "", - Path: gnmi.SplitPath(attrs[0]), - Val: attrs[1], - }) - } - - // Loop over args and create ops and exts - // Invalid args cause unhealable error - ops := make([]*gnmi.Operation, 0) - exts := make([]*gnmi_ext.Extension, 0) - for _, p := range opts { - switch p := p.(type) { - case *gnmi.Operation: - op := p - if op.Target == "" { - op.Target = g.Options.Address - } - ops = append(ops, op) - case *gnmi_ext.Extension: - exts = append(exts, p) - default: - return &errors.ErrInvalidParameters{ - Func: "nucleus.Set()", - Param: "args contain invalid type", - } - } - } - if len(ops) == 0 { - return &errors.ErrInvalidParameters{ - Func: "nucleus.Set()", - Param: "no operations provided", - } - } - resp, err := g.set(ctx, ops, exts...) - if err != nil { - return err - } - log.Info(resp) - return nil + return g.applyDiff(ctx, payload) } -func (g *Gnmi) applyDiff(ctx context.Context, payload []ygot.GoStruct) error { - if len(payload) != 2 { - return &errors.ErrInvalidParameters{} - } +func (g *Gnmi) applyDiff(ctx context.Context, payload change.Payload) error { op := ctx.Value(types.CtxKeyOperation) - oldstate, ok := payload[0].(ygot.GoStruct) - if !ok { - return &errors.ErrInvalidTypeAssertion{ - Value: payload[0], - Type: reflect.TypeOf("ygot.GoStruct"), - } - } - newstate, ok := payload[1].(ygot.GoStruct) - if !ok { - return &errors.ErrInvalidTypeAssertion{ - Value: payload[1], - Type: reflect.TypeOf("ygot.GoStruct"), - } - } - diff, err := ygot.Diff(oldstate, newstate) + diff, err := ygot.Diff(payload.Original, payload.Modified) if err != nil { return err } @@ -293,8 +194,6 @@ func (g *Gnmi) Capabilities(ctx context.Context) (interface{}, error) { // get calls GNMI get func (g *Gnmi) get(ctx context.Context, paths [][]string, origin string) (interface{}, error) { - - ctx = gnmi.NewContext(ctx, g.config) ctx = context.WithValue(ctx, types.CtxKeyConfig, g.config) //nolint req, err := gnmi.NewGetRequest(ctx, paths, origin) if err != nil { @@ -321,26 +220,6 @@ func (g *Gnmi) getWithRequest(ctx context.Context, req *gpb.GetRequest) (interfa return resp, nil } -// Set calls GNMI set -func (g *Gnmi) set(ctx context.Context, setOps []*gnmi.Operation, - exts ...*gnmi_ext.Extension) (*gpb.SetResponse, error) { - ctx = gnmi.NewContext(ctx, g.config) - targets := make([]string, len(setOps)) - paths := make([][]string, len(setOps)) - values := make([]string, len(setOps)) - for i, v := range setOps { - targets[i] = v.Target - paths[i] = v.Path - values[i] = v.Val - } - log.WithFields(log.Fields{ - "targets": targets, - "paths": paths, - "values": values, - }).Info("sending gNMI set request") - return gnmi.Set(ctx, g.client, setOps, exts...) -} - // Subscribe calls GNMI subscribe func (g *Gnmi) subscribe(ctx context.Context) error { ctx = gnmi.NewContext(ctx, g.config) diff --git a/nucleus/gnmi_transport_test.go b/nucleus/gnmi_transport_test.go index cb15e1eea8da2f3d3c483c6f1d74bc6a9b280ad4..113570ad72e9e5ca5584421f41b76d401162e266 100644 --- a/nucleus/gnmi_transport_test.go +++ b/nucleus/gnmi_transport_test.go @@ -6,6 +6,8 @@ import ( "reflect" "testing" + "code.fbi.h-da.de/cocsn/gosdn/interfaces/change" + "code.fbi.h-da.de/cocsn/gosdn/interfaces/southbound" spb "code.fbi.h-da.de/cocsn/api/go/gosdn/southbound" @@ -15,7 +17,6 @@ import ( "code.fbi.h-da.de/cocsn/gosdn/mocks" "code.fbi.h-da.de/cocsn/yang-models/generated/openconfig" gpb "github.com/openconfig/gnmi/proto/gnmi" - "github.com/openconfig/gnmi/proto/gnmi_ext" "github.com/openconfig/goyang/pkg/yang" "github.com/openconfig/ygot/ytypes" "github.com/stretchr/testify/mock" @@ -274,7 +275,7 @@ func TestGnmi_Set(t *testing.T) { transport *Gnmi } type args struct { - params []interface{} + payload change.Payload } tests := []struct { name string @@ -286,7 +287,7 @@ func TestGnmi_Set(t *testing.T) { name: "uninitialised", fields: fields{&Gnmi{}}, args: args{ - params: nil, + payload: change.Payload{}, }, wantErr: true, }, @@ -294,7 +295,7 @@ func TestGnmi_Set(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.fields.transport.Set(context.Background(), tt.args.params...) + err := tt.fields.transport.Set(context.Background(), tt.args.payload) if (err != nil) != tt.wantErr { t.Errorf("Set() error = %v, wantErr %v", err, tt.wantErr) } @@ -479,63 +480,3 @@ func TestNewGnmiTransport(t *testing.T) { }) } } - -func TestGnmi_set(t *testing.T) { - transport := mockTransport() - mockResponse := &gpb.SetResponse{} - - transport.client.(*mocks.GNMIClient). - On("NewContext", mockContext, mock.Anything). - Return(mockContext) - - transport.client.(*mocks.GNMIClient). - On("Set", mockContext, mock.Anything, mock.Anything). - Return(mockResponse, nil) - - type fields struct { - transport *Gnmi - } - type args struct { - ctx context.Context - setOps []*gnmi.Operation - exts []*gnmi_ext.Extension - } - tests := []struct { - name string - fields fields - args args - want *gpb.SetResponse - - wantErr bool - }{ - { - name: "default", - fields: fields{transport: &transport}, - args: args{ - ctx: context.Background(), - setOps: []*gnmi.Operation{ - { - Type: "update", - Path: []string{"interfaces", "interface", "name"}, - Val: "test0", - }, - }, - exts: nil, - }, - want: &gpb.SetResponse{}, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := tt.fields.transport.set(tt.args.ctx, tt.args.setOps, tt.args.exts...) - if (err != nil) != tt.wantErr { - t.Errorf("set() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("set() got = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/nucleus/principalNetworkDomain.go b/nucleus/principalNetworkDomain.go index 92b3bdec99b8c97e0a1dd461c7adc44bbe3bd5d9..4655a0b074dcfdbf9dcbfb1187ab3c3a36b04cd6 100644 --- a/nucleus/principalNetworkDomain.go +++ b/nucleus/principalNetworkDomain.go @@ -232,27 +232,32 @@ func (pnd *pndImplementation) MarshalDevice(identifier string) (string, error) { } // Request sends a get request to a specific device -func (pnd *pndImplementation) Request(uuid uuid.UUID, path string) error { +func (pnd *pndImplementation) Request(uuid uuid.UUID, path string) (proto.Message, error) { d, err := pnd.devices.GetDevice(FromString(uuid.String())) if err != nil { - return err + return nil, err } ctx := context.Background() res, err := d.Transport().Get(ctx, path) if err != nil { - return err + return nil, err } - err = d.ProcessResponse(res.(proto.Message)) + resp, ok := res.(proto.Message) + if !ok { + return nil, &errors.ErrInvalidTypeAssertion{} + } + err = d.ProcessResponse(resp) if err != nil { - return err + return nil, err } - return nil + return resp, nil } // RequestAll sends a request for all registered devices func (pnd *pndImplementation) RequestAll(path string) error { for _, k := range pnd.devices.UUIDs() { - if err := pnd.Request(k, path); err != nil { + _, err := pnd.Request(k, path) + if err != nil { return err } } @@ -265,24 +270,24 @@ func (pnd *pndImplementation) RequestAll(path string) error { // ChangeOND creates a change from the provided Operation, path and value. // The Change is Pending and times out after the specified timeout period -func (pnd *pndImplementation) ChangeOND(uuid uuid.UUID, operation ppb.ApiOperation, path string, value ...string) error { - d, err := pnd.devices.GetDevice(uuid) +func (pnd *pndImplementation) ChangeOND(duid uuid.UUID, operation ppb.ApiOperation, path string, value ...string) (uuid.UUID, error) { + d, err := pnd.devices.GetDevice(duid) if err != nil { - return err + return uuid.Nil, err } cpy, err := ygot.DeepCopy(d.Model()) ygot.BuildEmptyTree(cpy) if err != nil { - return err + return uuid.Nil, err } p, err := ygot.StringToStructuredPath(path) if err != nil { - return err + return uuid.Nil, err } if operation != ppb.ApiOperation_DELETE && len(value) != 1 { - return &errors.ErrInvalidParameters{ + return uuid.Nil, &errors.ErrInvalidParameters{ Func: pnd.ChangeOND, Param: value, } @@ -291,27 +296,31 @@ func (pnd *pndImplementation) ChangeOND(uuid uuid.UUID, operation ppb.ApiOperati case ppb.ApiOperation_UPDATE, ppb.ApiOperation_REPLACE: typedValue := gnmi.TypedValue(value[0]) if err := ytypes.SetNode(d.SBI().Schema().RootSchema(), cpy, p, typedValue); err != nil { - return err + return uuid.Nil, err } case ppb.ApiOperation_DELETE: if err := ytypes.DeleteNode(d.SBI().Schema().RootSchema(), cpy, p); err != nil { - return err + return uuid.Nil, err } default: - return &errors.ErrOperationNotSupported{Op: operation} + return uuid.Nil, &errors.ErrOperationNotSupported{Op: operation} } ygot.PruneEmptyBranches(cpy) - callback := func(state ygot.GoStruct, change ygot.GoStruct) error { + callback := func(original ygot.GoStruct, modified ygot.GoStruct) error { ctx := context.WithValue(context.Background(), types.CtxKeyOperation, operation) // nolint - return d.Transport().Set(ctx, state, change) + payload := change.Payload{Original: original, Modified: modified} + return d.Transport().Set(ctx, payload) } errChan := make(chan error) - ch := NewChange(uuid, d.Model(), cpy, callback, errChan) + ch := NewChange(duid, d.Model(), cpy, callback, errChan) pnd.errChans[ch.ID()] = errChan - return pnd.changes.Add(ch) + if err := pnd.changes.Add(ch); err != nil { + return uuid.Nil, err + } + return ch.cuid, nil } func handleRollbackError(id uuid.UUID, err error) { diff --git a/nucleus/principalNetworkDomain_test.go b/nucleus/principalNetworkDomain_test.go index 5819d0e3b0735957683b41528ebcf7864c223ac9..536d4d71993971b2cedd071dd4438230a9f61cfb 100644 --- a/nucleus/principalNetworkDomain_test.go +++ b/nucleus/principalNetworkDomain_test.go @@ -460,7 +460,8 @@ func Test_pndImplementation_Request(t *testing.T) { tr.On("Get", mockContext, mock.Anything).Return(&gpb.GetResponse{}, tt.args.rErr) tr.On("ProcessResponse", mock.Anything, mock.Anything, mock.Anything).Return(tt.args.rErr) _ = pnd.addDevice(deviceWithMockTransport) - if err := pnd.Request(tt.args.uuid, tt.args.path); (err != nil) != tt.wantErr { + _, err := pnd.Request(tt.args.uuid, tt.args.path) + if (err != nil) != tt.wantErr { t.Errorf("Request() error = %v, wantErr %v", err, tt.wantErr) } if err := pnd.devices.Delete(mdid); err != nil { @@ -629,7 +630,8 @@ func Test_pndImplementation_ChangeOND(t *testing.T) { return } - if err := pnd.ChangeOND(did, tt.args.operation, tt.args.path, tt.args.value...); (err != nil) != tt.wantErr { + _, err := pnd.ChangeOND(did, tt.args.operation, tt.args.path, tt.args.value...) + if (err != nil) != tt.wantErr { t.Errorf("ChangeOND() error = %v, wantErr %v", err, tt.wantErr) return } @@ -768,7 +770,8 @@ func Test_pndImplementation_Confirm(t *testing.T) { t.Error(err) return } - if err := pnd.ChangeOND(d.ID(), ppb.ApiOperation_UPDATE, "system/config/hostname", "ceos3000"); err != nil { + _, err := pnd.ChangeOND(d.ID(), ppb.ApiOperation_UPDATE, "system/config/hostname", "ceos3000") + if err != nil { t.Error(err) return } diff --git a/test/integration/nucleusIntegration_test.go b/test/integration/nucleusIntegration_test.go index 25ee1d4df99ff6e5fc74cbc47fec996333224d11..42aaa88e034f01e2bf353a216644be2da72c2776 100644 --- a/test/integration/nucleusIntegration_test.go +++ b/test/integration/nucleusIntegration_test.go @@ -2,14 +2,17 @@ package integration import ( "context" + "github.com/google/uuid" "os" "reflect" "sort" "testing" "time" + ppb "code.fbi.h-da.de/cocsn/api/go/gosdn/pnd" spb "code.fbi.h-da.de/cocsn/api/go/gosdn/southbound" tpb "code.fbi.h-da.de/cocsn/api/go/gosdn/transport" + "code.fbi.h-da.de/cocsn/gosdn/interfaces/change" "code.fbi.h-da.de/cocsn/gosdn/forks/goarista/gnmi" "code.fbi.h-da.de/cocsn/gosdn/nucleus" @@ -24,6 +27,7 @@ import ( const unreachable = "203.0.113.10:6030" const testPath = "/system/config/hostname" +var modifiedHostname = "ceos3000" var testAddress = "141.100.70.170:6030" var testUsername = "admin" var testPassword = "arista" @@ -86,7 +90,7 @@ func testSetupIntegration() { } } -func TestGnmi_SetIntegration(t *testing.T) { +func TestGnmi_SetInvalidIntegration(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } @@ -94,8 +98,8 @@ func TestGnmi_SetIntegration(t *testing.T) { opt *tpb.TransportOption } type args struct { - ctx context.Context - params []string + ctx context.Context + payload change.Payload } tests := []struct { name string @@ -107,32 +111,23 @@ func TestGnmi_SetIntegration(t *testing.T) { name: "destination unreachable", fields: fields{ opt: &tpb.TransportOption{ - Address: "203.0.113.10:6030", + Address: unreachable, TransportOption: &tpb.TransportOption_GnmiTransportOption{ GnmiTransportOption: &tpb.GnmiTransportOption{}}, }, }, args: args{ - ctx: context.Background(), - params: []string{"/system/config/hostname", "ceos3000"}, + ctx: context.Background(), + payload: change.Payload{}, }, wantErr: true, }, - { - name: "valid update", - fields: fields{opt: opt}, - args: args{ - ctx: context.Background(), - params: []string{"/system/config/hostname", "ceos3000"}, - }, - wantErr: false, - }, { name: "invalid update", fields: fields{opt: opt}, args: args{ - ctx: context.Background(), - params: nil, + ctx: context.Background(), + payload: change.Payload{}, }, wantErr: true, }, @@ -144,7 +139,7 @@ func TestGnmi_SetIntegration(t *testing.T) { t.Errorf("NewGnmiTransport() error = %v, wantErr %v", err, tt.wantErr) return } - err = g.Set(tt.args.ctx, tt.args.params) + err = g.Set(tt.args.ctx, tt.args.payload) if (err != nil) != tt.wantErr { t.Errorf("Set() error = %v, wantErr %v", err, tt.wantErr) return @@ -153,6 +148,97 @@ func TestGnmi_SetIntegration(t *testing.T) { } } +func TestGnmi_SetValidIntegration(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + sbi := nucleus.NewSBI(spb.Type_OPENCONFIG) + opt := &tpb.TransportOption{ + Address: testAddress, + Username: testUsername, + Password: testPassword, + TransportOption: &tpb.TransportOption_GnmiTransportOption{ + GnmiTransportOption: &tpb.GnmiTransportOption{}, + }, + } + pnd, err := nucleus.NewPND("test", "test", uuid.New(), sbi, nil, nil) + if err != nil { + t.Error(err) + return + } + if err := pnd.AddDevice("test", opt, sbi.ID()); err != nil { + t.Error(err) + return + } + device, err := pnd.GetDevice("test") + if err != nil { + t.Error(err) + return + } + + tests := []struct { + name string + apiOp ppb.ApiOperation + path string + value string + want string + }{ + { + name: "update", + apiOp: ppb.ApiOperation_UPDATE, + path: testPath, + value: modifiedHostname, + want: modifiedHostname, + }, + { + name: "replace", + apiOp: ppb.ApiOperation_REPLACE, + path: "/system/config/domain-name", + value: modifiedHostname, + want: modifiedHostname, + }, + { + name: "delete", + apiOp: ppb.ApiOperation_DELETE, + path: testPath, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cuid, err := pnd.ChangeOND(device.ID(), tt.apiOp, tt.path, tt.value) + if err != nil { + t.Error(err) + return + } + if err := pnd.Commit(cuid); err != nil { + t.Error(err) + return + } + if err := pnd.Confirm(cuid); err != nil { + t.Error(err) + return + } + if tt.name != "delete" { + resp, err := pnd.Request(device.ID(), tt.path) + if err != nil { + t.Error(err) + return + } + r, ok := resp.(*gpb.GetResponse) + if !ok { + t.Error(&errors.ErrInvalidTypeAssertion{}) + return + } + got := r.Notification[0].Update[0].Val.GetStringVal() + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetDevice() got = %v, want %v", got, tt.want) + } + } + }) + } +} + func TestGnmi_GetIntegration(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") @@ -190,7 +276,7 @@ func TestGnmi_GetIntegration(t *testing.T) { name: "destination unreachable", fields: fields{ opt: &tpb.TransportOption{ - Address: "203.0.113.10:6030", + Address: unreachable, TransportOption: &tpb.TransportOption_GnmiTransportOption{ GnmiTransportOption: &tpb.GnmiTransportOption{}}, },