From 2dbb22c1d9f4bf53a74752ced0f2e4eff58b6454 Mon Sep 17 00:00:00 2001 From: Malte Bauch <malte.bauch@stud.h-da.de> Date: Thu, 26 Jan 2023 13:56:50 +0100 Subject: [PATCH] Remove schema field from southbound struct --- controller/api/initialise_test.go | 6 +++- controller/interfaces/southbound/sbi.go | 2 +- controller/northbound/server/pnd.go | 7 +++- controller/nucleus/gnmi_transport_test.go | 5 ++- controller/nucleus/initialise_test.go | 10 ++++-- controller/nucleus/networkElement.go | 18 ++++++++-- .../nucleus/networkElementService_test.go | 22 +++++++++---- controller/nucleus/networkElementWatcher.go | 7 +++- controller/nucleus/principalNetworkDomain.go | 23 +++++++++---- .../nucleus/principalNetworkDomain_test.go | 23 ++++++++++--- controller/nucleus/southbound.go | 20 +++++------ controller/nucleus/southbound_test.go | 33 ++++++++++--------- controller/test/plugin/csbiAdditions.go | 18 +++++----- 13 files changed, 130 insertions(+), 64 deletions(-) diff --git a/controller/api/initialise_test.go b/controller/api/initialise_test.go index 9ec8959c0..a750f866b 100644 --- a/controller/api/initialise_test.go +++ b/controller/api/initialise_test.go @@ -122,11 +122,15 @@ func bootstrapUnitTest() { }) sbi := &nucleus.OpenConfig{} + schema, err := sbi.Schema() + if err != nil { + log.Fatal(err) + } mockNetworkElement := &mocks.NetworkElement{} mockNetworkElement.On("SBI").Return(sbi) mockNetworkElement.On("ID").Return(mneUUID) - mockNetworkElement.On("GetModel").Return(sbi.Schema().Root) + mockNetworkElement.On("GetModel").Return(schema.Root) mockNetworkElement.On("Name").Return("openconfig") mockNetworkElement.On("TransportAddress").Return("127.0.0.1:6030") mockNetworkElement.On("GetMetadata").Return(conflict.Metadata{ResourceVersion: 0}) diff --git a/controller/interfaces/southbound/sbi.go b/controller/interfaces/southbound/sbi.go index 2667d5051..9ddf92b99 100644 --- a/controller/interfaces/southbound/sbi.go +++ b/controller/interfaces/southbound/sbi.go @@ -17,7 +17,7 @@ type SouthboundInterface interface { // nolint // representation to the transport. // Needed for type assertion. SetNode(schema *yang.Entry, root interface{}, path *gpb.Path, val interface{}, opts ...ytypes.SetNodeOpt) error - Schema() *ytypes.Schema + Schema() (*ytypes.Schema, error) SchemaTreeGzip() []byte ID() uuid.UUID SetID(id uuid.UUID) diff --git a/controller/northbound/server/pnd.go b/controller/northbound/server/pnd.go index 64d3e1634..41e7339e2 100644 --- a/controller/northbound/server/pnd.go +++ b/controller/northbound/server/pnd.go @@ -164,7 +164,12 @@ func fillMneBySpecificPath(nme networkelement.NetworkElement, path string) (*ppb &ytypes.GetHandleWildcards{}, &ytypes.GetPartialKeyMatch{}, } - nodes, err := ytypes.GetNode(nme.SBI().Schema().RootSchema(), nme.GetModel(), gnmiPath, opts...) + schema, err := nme.SBI().Schema() + if err != nil { + log.Error(err) + return nil, status.Errorf(codes.Aborted, "%v", err) + } + nodes, err := ytypes.GetNode(schema.RootSchema(), nme.GetModel(), gnmiPath, opts...) if err != nil { log.Error(err) return nil, status.Errorf(codes.Aborted, "%v", err) diff --git a/controller/nucleus/gnmi_transport_test.go b/controller/nucleus/gnmi_transport_test.go index 146b1ce8b..dd90cd69d 100644 --- a/controller/nucleus/gnmi_transport_test.go +++ b/controller/nucleus/gnmi_transport_test.go @@ -264,7 +264,10 @@ func TestGnmi_ProcessResponse(t *testing.T) { SetNode: tt.fields.Sbi.SetNode, Unmarshal: tt.fields.Sbi.(*OpenConfig).Unmarshal, } - s := tt.fields.Sbi.Schema() + s, err := tt.fields.Sbi.Schema() + if err != nil { + t.Errorf("ProcessResponse() error = %v", err) + } resp := gnmiMessages[tt.args.path] if err := g.ProcessResponse(resp, tt.args.root, s); (err != nil) != tt.wantErr { t.Errorf("ProcessResponse() error = %v, wantErr %v", err, tt.wantErr) diff --git a/controller/nucleus/initialise_test.go b/controller/nucleus/initialise_test.go index 6a66fc3fa..45e5c987f 100644 --- a/controller/nucleus/initialise_test.go +++ b/controller/nucleus/initialise_test.go @@ -142,14 +142,18 @@ func readTestUUIDs() { } } -func mockNetworkElement() networkelement.NetworkElement { +func mockNetworkElement() (networkelement.NetworkElement, error) { sbi := &OpenConfig{} + schema, err := sbi.Schema() + if err != nil { + return nil, err + } return &CommonNetworkElement{ UUID: mdid, - Model: sbi.Schema().Root, + Model: schema.Root, sbi: sbi, transport: &mocks.Transport{}, - } + }, nil } func newPnd() pndImplementation { diff --git a/controller/nucleus/networkElement.go b/controller/nucleus/networkElement.go index 692253286..f8381601f 100644 --- a/controller/nucleus/networkElement.go +++ b/controller/nucleus/networkElement.go @@ -43,7 +43,11 @@ func NewNetworkElement( // We want a representation of the MNE's config (the SBI-schema's root created through ygot), // but do not want to work on the sbi.Schema() directly. // So the root of sbi.Schema() is never changed when a set or get on a network element will be called. - root, err := ygot.DeepCopy(sbi.Schema().Root) + schema, err := sbi.Schema() + if err != nil { + return nil, err + } + root, err := ygot.DeepCopy(schema.Root) if err != nil { return nil, err } @@ -154,7 +158,11 @@ func (n *CommonNetworkElement) SetSBI(sbi southbound.SouthboundInterface) { // ProcessResponse processes a response for the Network Element. func (n *CommonNetworkElement) ProcessResponse(resp proto.Message) error { - return n.transport.ProcessResponse(resp, n.Model, n.sbi.Schema()) + schema, err := n.sbi.Schema() + if err != nil { + return err + } + return n.transport.ProcessResponse(resp, n.Model, schema) } // IsTransportValid returns a boolean if the transport of a network element is valid. @@ -214,7 +222,11 @@ func (n *CsbiNetworkElement) GetMetadata() conflict.Metadata { // ProcessResponse processes a response for the Network Element. func (n *CsbiNetworkElement) ProcessResponse(resp proto.Message) error { // TODO: callback to send response to caller - return n.transport.ProcessResponse(resp, n.Model, n.sbi.Schema()) + schema, err := n.SBI().Schema() + if err != nil { + return err + } + return n.transport.ProcessResponse(resp, n.Model, schema) } func createValidatedCopy(n networkelement.NetworkElement) (ygot.ValidatedGoStruct, error) { diff --git a/controller/nucleus/networkElementService_test.go b/controller/nucleus/networkElementService_test.go index 39aad46fa..5118c3d69 100644 --- a/controller/nucleus/networkElementService_test.go +++ b/controller/nucleus/networkElementService_test.go @@ -13,13 +13,17 @@ import ( spb "code.fbi.h-da.de/danet/gosdn/api/go/gosdn/southbound" ) -func getMockNetworkElement(mneID uuid.UUID, sbi southbound.SouthboundInterface) networkelement.NetworkElement { +func getMockNetworkElement(mneID uuid.UUID, sbi southbound.SouthboundInterface) (networkelement.NetworkElement, error) { + schema, err := sbi.Schema() + if err != nil { + return nil, err + } return &CommonNetworkElement{ UUID: mneID, - Model: sbi.Schema().Root, + Model: schema.Root, sbi: sbi, transport: &mocks.Transport{}, - } + }, nil } func getNetworkElementTestStores(t *testing.T, mneID uuid.UUID) (networkelement.Service, southbound.Service, networkelement.NetworkElement, southbound.SouthboundInterface) { @@ -43,7 +47,10 @@ func getNetworkElementTestStores(t *testing.T, mneID uuid.UUID) (networkelement. t.Error("could not add sbi") } - mockNetworkElement := getMockNetworkElement(mneID, sbi) + mockNetworkElement, err := getMockNetworkElement(mneID, sbi) + if err != nil { + t.Log(err) + } err = networkElementService.Add(mockNetworkElement) if err != nil { t.Error("could not add network element") @@ -94,9 +101,12 @@ func TestNetworkElementService_GetAll(t *testing.T) { mneID2 := uuid.New() networkElementService, _, _, sbi := getNetworkElementTestStores(t, mneID) - mockNetworkElement2 := getMockNetworkElement(mneID2, sbi) + mockNetworkElement2, err := getMockNetworkElement(mneID2, sbi) + if err != nil { + t.Log(err) + } - err := networkElementService.Add(mockNetworkElement2) + err = networkElementService.Add(mockNetworkElement2) if err != nil { t.Error("could not add network element") } diff --git a/controller/nucleus/networkElementWatcher.go b/controller/nucleus/networkElementWatcher.go index 44775ef8b..06c44cb3a 100644 --- a/controller/nucleus/networkElementWatcher.go +++ b/controller/nucleus/networkElementWatcher.go @@ -162,7 +162,12 @@ func (n *NetworkElementWatcher) handleSubscribeResponseUpdate(resp *gpb.Subscrib log.Error(err) } - err = mne.Transport().ProcessControlPlaneSubscribeResponse(resp, mne.GetModel(), mne.SBI().Schema()) + schema, err := mne.SBI().Schema() + if err != nil { + log.Error(err) + } + + err = mne.Transport().ProcessControlPlaneSubscribeResponse(resp, mne.GetModel(), schema) if err != nil { log.Error(err) } diff --git a/controller/nucleus/principalNetworkDomain.go b/controller/nucleus/principalNetworkDomain.go index f2662933b..fab2bf7e9 100644 --- a/controller/nucleus/principalNetworkDomain.go +++ b/controller/nucleus/principalNetworkDomain.go @@ -156,14 +156,18 @@ func (pnd *pndImplementation) Commit(u uuid.UUID) error { if err != nil { return err } + schema, err := networkElement.SBI().Schema() + if err != nil { + return err + } for _, update := range diff.GetUpdate() { opts := []ytypes.SetNodeOpt{&ytypes.InitMissingElements{}, &ytypes.TolerateJSONInconsistencies{}} - if err := ytypes.SetNode(networkElement.SBI().Schema().RootSchema(), networkElement.GetModel(), update.GetPath(), update.GetVal(), opts...); err != nil { + if err := ytypes.SetNode(schema.RootSchema(), networkElement.GetModel(), update.GetPath(), update.GetVal(), opts...); err != nil { return err } } for _, deletePath := range diff.GetDelete() { - if err := ytypes.DeleteNode(networkElement.SBI().Schema().RootSchema(), networkElement.GetModel(), deletePath); err != nil { + if err := ytypes.DeleteNode(schema.RootSchema(), networkElement.GetModel(), deletePath); err != nil { return err } } @@ -551,6 +555,11 @@ func (pnd *pndImplementation) ChangeMNE(duid uuid.UUID, operation ppb.ApiOperati return uuid.Nil, err } + schema, err := mne.SBI().Schema() + if err != nil { + return uuid.Nil, err + } + validatedCpy, err := mne.CreateModelCopy() if err != nil { return uuid.Nil, err @@ -569,7 +578,7 @@ func (pnd *pndImplementation) ChangeMNE(duid uuid.UUID, operation ppb.ApiOperati } switch operation { case ppb.ApiOperation_API_OPERATION_UPDATE, ppb.ApiOperation_API_OPERATION_REPLACE: - _, entry, err := ytypes.GetOrCreateNode(mne.SBI().Schema().RootSchema(), validatedCpy, p) + _, entry, err := ytypes.GetOrCreateNode(schema.RootSchema(), validatedCpy, p) if err != nil { return uuid.Nil, err } @@ -585,12 +594,12 @@ func (pnd *pndImplementation) ChangeMNE(duid uuid.UUID, operation ppb.ApiOperati return uuid.Nil, err } opts := []ytypes.SetNodeOpt{&ytypes.InitMissingElements{}, &ytypes.TolerateJSONInconsistencies{}} - if err := ytypes.SetNode(mne.SBI().Schema().RootSchema(), validatedCpy, p, typedValue, opts...); err != nil { + if err := ytypes.SetNode(schema.RootSchema(), validatedCpy, p, typedValue, opts...); err != nil { return uuid.Nil, err } } case ppb.ApiOperation_API_OPERATION_DELETE: - if err := ytypes.DeleteNode(mne.SBI().Schema().RootSchema(), validatedCpy, p); err != nil { + if err := ytypes.DeleteNode(schema.RootSchema(), validatedCpy, p); err != nil { return uuid.Nil, err } default: @@ -602,8 +611,8 @@ func (pnd *pndImplementation) ChangeMNE(duid uuid.UUID, operation ppb.ApiOperati ctx := context.WithValue(context.Background(), types.CtxKeyOperation, operation) // nolint payload := change.Payload{Original: original, Modified: modified} pathToSet := path - schema := mne.SBI().Schema() - return mne.Transport().Set(ctx, payload, pathToSet, schema) + sbiSchema := schema + return mne.Transport().Set(ctx, payload, pathToSet, sbiSchema) } ch := NewChange(duid, mne.GetModel(), validatedCpy, callback) diff --git a/controller/nucleus/principalNetworkDomain_test.go b/controller/nucleus/principalNetworkDomain_test.go index f880431f9..9d07cfc93 100644 --- a/controller/nucleus/principalNetworkDomain_test.go +++ b/controller/nucleus/principalNetworkDomain_test.go @@ -830,6 +830,11 @@ func Test_pndImplementation_GetNetworkElement(t *testing.T) { t.Errorf("NewSBI() error = %v", err) return } + sbiSchema, err := sbi.Schema() + if err != nil { + t.Errorf("NewSBI() error = %v", err) + return + } err = pnd.addSbi(sbi) if err != nil { t.Error(err) @@ -858,7 +863,7 @@ func Test_pndImplementation_GetNetworkElement(t *testing.T) { { name: "default", args: args{uuid: mneid}, - want: sbi.Schema().Root, + want: sbiSchema.Root, wantErr: false, }, { @@ -902,6 +907,11 @@ func Test_pndImplementation_GetNetworkElementByName(t *testing.T) { t.Errorf("NewSBI() error = %v", err) return } + sbiSchema, err := sbi.Schema() + if err != nil { + t.Errorf("NewSBI() error = %v", err) + return + } err = pnd.addSbi(sbi) if err != nil { t.Error(err) @@ -929,7 +939,7 @@ func Test_pndImplementation_GetNetworkElementByName(t *testing.T) { { name: "default", args: args{name: mne.Name()}, - want: sbi.Schema().Root, + want: sbiSchema.Root, wantErr: false, }, { @@ -986,14 +996,19 @@ func Test_pndImplementation_Confirm(t *testing.T) { Id: defaultPndID, } - mne := mockNetworkElement() + mne, err := mockNetworkElement() + if err != nil { + t.Error(err) + return + } tr, ok := mne.Transport().(*mocks.Transport) if !ok { log.Errorf("Confirm(), failed type conversion: %v", ok) + return } tr.On("Set", mockContext, mock.Anything, mock.Anything, mock.Anything).Return(nil) - _, err := pnd.addNetworkElement(mne) + _, err = pnd.addNetworkElement(mne) if err != nil { t.Error(err) return diff --git a/controller/nucleus/southbound.go b/controller/nucleus/southbound.go index de92fad6e..8f709cca4 100644 --- a/controller/nucleus/southbound.go +++ b/controller/nucleus/southbound.go @@ -76,8 +76,7 @@ func NewSouthboundPlugin(id uuid.UUID, path string, build bool) (*SouthboundPlug // The struct holds the YANG schema and a function that // returns an SBI specific SetNode function. type OpenConfig struct { - schema *ytypes.Schema - id uuid.UUID + id uuid.UUID // nolint:unused path string } @@ -95,13 +94,8 @@ func (oc *OpenConfig) Name() string { } // Schema returns a ygot generated openconfig Schema as ytypes.Schema. -func (oc *OpenConfig) Schema() *ytypes.Schema { - schema, err := openconfig.Schema() - oc.schema = schema - if err != nil { - log.Fatal(err) - } - return schema +func (oc *OpenConfig) Schema() (*ytypes.Schema, error) { + return openconfig.Schema() } // SchemaTreeGzip returns the ygot generated SchemaTree compressed as gzip byte @@ -119,7 +113,11 @@ func (oc *OpenConfig) SetNode(schema *yang.Entry, root interface{}, path *gpb.Pa // Unmarshal injects OpenConfig specific model representation to the transport. // Needed for type assertion. func (oc *OpenConfig) Unmarshal(bytes []byte, path *gpb.Path, goStruct ygot.GoStruct, opt ...ytypes.UnmarshalOpt) error { - return unmarshal(oc.Schema(), bytes, path, goStruct, opt...) + schema, err := openconfig.Schema() + if err != nil { + return err + } + return unmarshal(schema, bytes, path, goStruct, opt...) } // unmarshal parses a gNMI response to a go struct. @@ -200,7 +198,7 @@ func (p *SouthboundPlugin) SetNode(schema *yang.Entry, root interface{}, path *g } // Schema returns a ygot generated Schema as ytypes.Schema. -func (p *SouthboundPlugin) Schema() *ytypes.Schema { +func (p *SouthboundPlugin) Schema() (*ytypes.Schema, error) { return p.sbi.Schema() } diff --git a/controller/nucleus/southbound_test.go b/controller/nucleus/southbound_test.go index d6a09f2d9..07e25cf6c 100644 --- a/controller/nucleus/southbound_test.go +++ b/controller/nucleus/southbound_test.go @@ -16,8 +16,7 @@ import ( func TestOpenConfig_Id(t *testing.T) { type fields struct { - schema *ytypes.Schema - id uuid.UUID + id uuid.UUID } tests := []struct { name string @@ -27,8 +26,7 @@ func TestOpenConfig_Id(t *testing.T) { { name: "default", fields: fields{ - schema: nil, - id: ocUUID, + id: ocUUID, }, want: ocUUID, }, @@ -36,8 +34,7 @@ func TestOpenConfig_Id(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { oc := &OpenConfig{ - schema: tt.fields.schema, - id: tt.fields.id, + id: tt.fields.id, } if got := oc.ID(); !reflect.DeepEqual(got, tt.want) { t.Errorf("ID() = %v, want %v", got, tt.want) @@ -48,8 +45,7 @@ func TestOpenConfig_Id(t *testing.T) { func TestOpenConfig_SbiIdentifier(t *testing.T) { type fields struct { - schema *ytypes.Schema - id uuid.UUID + id uuid.UUID } tests := []struct { name string @@ -61,8 +57,7 @@ func TestOpenConfig_SbiIdentifier(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { oc := &OpenConfig{ - schema: tt.fields.schema, - id: tt.fields.id, + id: tt.fields.id, } if got := oc.SbiIdentifier(); got != tt.want { t.Errorf("SbiIdentifier() = %v, want %v", got, tt.want) @@ -77,8 +72,7 @@ func TestOpenConfig_Schema(t *testing.T) { t.Error(err) } type fields struct { - schema *ytypes.Schema - id uuid.UUID + id uuid.UUID } tests := []struct { name string @@ -90,10 +84,13 @@ func TestOpenConfig_Schema(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { oc := &OpenConfig{ - schema: tt.fields.schema, - id: tt.fields.id, + id: tt.fields.id, } - got := oc.Schema().SchemaTree + schema, err := oc.Schema() + if err != nil { + t.Errorf("Schema() error = %v", err) + } + got := schema.SchemaTree if !reflect.DeepEqual(got, tt.want.SchemaTree) { t.Errorf("Schema() = %v, want %v", got, tt.want) } @@ -170,7 +167,11 @@ func Test_unmarshal(t *testing.T) { t.Error(err) return } - if err := unmarshal(oc.Schema(), bytes, resp.Notification[0].Update[0].Path, tt.args.goStruct, tt.args.opt...); err != nil { + schema, err := oc.Schema() + if err != nil { + t.Errorf("unmarshal() error = %v", err) + } + if err := unmarshal(schema, bytes, resp.Notification[0].Update[0].Path, tt.args.goStruct, tt.args.opt...); err != nil { if !tt.wantErr { t.Errorf("unmarshal() error = %v, wantErr %v", err, tt.wantErr) } diff --git a/controller/test/plugin/csbiAdditions.go b/controller/test/plugin/csbiAdditions.go index d56abea58..b592400fc 100644 --- a/controller/test/plugin/csbiAdditions.go +++ b/controller/test/plugin/csbiAdditions.go @@ -27,10 +27,15 @@ func (csbi *Csbi) SetNode(schema *yang.Entry, root interface{}, path *gpb.Path, // Unmarshal injects schema specific model representation to the transport. // Needed for type assertion. func (csbi *Csbi) Unmarshal(bytes []byte, path *gpb.Path, goStruct ygot.GoStruct, opt ...ytypes.UnmarshalOpt) error { - return unmarshal(csbi.Schema(), bytes, path, goStruct, opt...) + schema, err := Schema() + if err != nil { + return err + } + + return unmarshal(schema, bytes, path, goStruct, opt...) } -//unmarshal parses a gNMI response to a go struct. +// unmarshal parses a gNMI response to a go struct. func unmarshal(schema *ytypes.Schema, bytes []byte, path *gpb.Path, goStruct ygot.GoStruct, opt ...ytypes.UnmarshalOpt) error { defer func() { if r := recover(); r != nil { @@ -67,13 +72,8 @@ func unmarshal(schema *ytypes.Schema, bytes []byte, path *gpb.Path, goStruct ygo return ygot.MergeStructInto(goStruct, validatedDeepCopy, opts...) } -func (csbi *Csbi) Schema() *ytypes.Schema { - schema, err := Schema() - csbi.schema = schema - if err != nil { - log.Fatal(err) - } - return schema +func (csbi *Csbi) Schema() (*ytypes.Schema, error) { + return Schema() } func (csbi *Csbi) SchemaTreeGzip() []byte { -- GitLab