Merge branch...

Merge branch '163-removing-an-sbi-from-the-pnd-s-sbi-store-should-also-remove-the-devices-using-that-specific-sbi' into 'develop'

Resolve "Removing an SBI from the PND's SBI store should also remove the devices using that specific SBI"

See merge request !234

go.mod

@@ -8,6 +8,7 @@ require (
 	code.fbi.h-da.de/danet/forks/google v0.0.0-20210709163519-47ee8958ef40
 	code.fbi.h-da.de/danet/yang-models v0.1.0
 	github.com/docker/docker v20.10.11+incompatible
+	github.com/google/go-cmp v0.5.6
 	github.com/google/uuid v1.2.0
 	github.com/openconfig/gnmi v0.0.0-20210914185457-51254b657b7d
 	github.com/openconfig/goyang v0.3.1
@@ -29,7 +30,6 @@ require (
 	github.com/fsnotify/fsnotify v1.5.1 // indirect
 	github.com/golang/glog v1.0.0 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
-	github.com/google/go-cmp v0.5.6 // indirect
 	github.com/hashicorp/hcl v1.0.0 // indirect
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
 	github.com/kylelemons/godebug v1.1.0 // indirect
@@ -49,6 +49,7 @@ require (
 	golang.org/x/net v0.0.0-20211123203042-d83791d6bcd9 // indirect
 	golang.org/x/sys v0.0.0-20211123173158-ef496fb156ab // indirect
 	golang.org/x/text v0.3.7 // indirect
+	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
 	google.golang.org/genproto v0.0.0-20211118181313-81c1377c94b1 // indirect
 	gopkg.in/ini.v1 v1.64.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect

nucleus/principalNetworkDomain.go

@@ -142,11 +142,18 @@ func (pnd *pndImplementation) AddSbi(s southbound.SouthboundInterface) error {
 	return pnd.addSbi(s)
 }
 
-// RemoveSbi removes a SBI from the PND
-// TODO: this should to recursively through
-// devices and remove the devices using
-// this SBI
+// RemoveSbi removes a SBI and all the associated devices from the PND
 func (pnd *pndImplementation) RemoveSbi(id uuid.UUID) error {
+	associatedDevices, err := pnd.devices.GetDevicesAssociatedWithSbi(id)
+	if err != nil {
+		return err
+	}
+	// range over associated devices and remove each one of them
+	for _, d := range associatedDevices {
+		if err := pnd.removeDevice(d.ID()); err != nil {
+			return err
+		}
+	}
 	return pnd.removeSbi(id)
 }
 

nucleus/principalNetworkDomain_test.go

@@ -409,6 +409,11 @@ func Test_pndImplementation_RemoveDevice(t *testing.T) {
 }
 
 func Test_pndImplementation_RemoveSbi(t *testing.T) {
+	opts := &tpb.TransportOption{
+		TransportOption: &tpb.TransportOption_GnmiTransportOption{
+			GnmiTransportOption: &tpb.GnmiTransportOption{},
+		},
+	}
 	type args struct {
 		id uuid.UUID
 	}
@@ -420,6 +425,7 @@ func Test_pndImplementation_RemoveSbi(t *testing.T) {
 		{name: "default", args: args{id: defaultSbiID}, wantErr: false},
 		{name: "fails", args: args{id: uuid.New()}, wantErr: true},
 		{name: "fails empty", args: args{id: defaultSbiID}, wantErr: true},
+		{name: "exclusively remove associated devices", args: args{id: defaultSbiID}, wantErr: false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -430,16 +436,42 @@ func Test_pndImplementation_RemoveSbi(t *testing.T) {
 				devices:     store.NewDeviceStore(defaultPndID),
 				Id:          defaultPndID,
 			}
-			if tt.name != "fails empty" {
+			if tt.name != "fails empty" && tt.name != "fails" {
 				if err := pnd.addSbi(&OpenConfig{id: defaultSbiID}); err != nil {
 					t.Error(err)
 				}
+				if err := pnd.AddDevice("associatedDevice", opts, tt.args.id); err != nil {
+					t.Error(err)
+				}
+				if err := pnd.AddDevice("associatedDevice2", opts, tt.args.id); err != nil {
+					t.Error(err)
+				}
+				if tt.name == "exclusively remove associated devices" {
+					newID := uuid.New()
+					if err := pnd.addSbi(&OpenConfig{id: newID}); err != nil {
+						t.Error(err)
+					}
+					if err := pnd.AddDevice("associatedDevice2", opts, newID); err != nil {
+						t.Error(err)
+					}
+				}
 			}
+
 			if err := pnd.RemoveSbi(tt.args.id); (err != nil) != tt.wantErr {
 				t.Errorf("RemoveSbi() error = %v, wantErr %v", err, tt.wantErr)
 			}
 			if pnd.sbic.Exists(tt.args.id) {
-				t.Errorf("RemoveDevice() SBI still in SBI store %v", pnd.sbic)
+				t.Errorf("RemoveSbi() SBI still in SBI store %v", pnd.sbic)
+			}
+
+			if tt.name == "exclusively remove associated devices" {
+				if len(pnd.devices.Store) != 1 {
+					t.Errorf("RemoveSbi() non associated devices should remain in the storage %v", pnd.devices)
+				}
+			} else {
+				if len(pnd.devices.Store) != 0 {
+					t.Errorf("RemoveSbi() associated devices have not been removed correctly %v", len(pnd.devices.Store))
+				}
 			}
 		})
 	}

store/deviceStore.go

@@ -37,16 +37,15 @@ func (s *DeviceStore) GetDevice(id uuid.UUID, parseErrors ...error) (device.Devi
 
 	foundID = id
 
-	for _, parseErrs := range parseErrors {
-		if parseErrs != nil {
-			switch e := parseErrs.(type) {
+	for _, parseErr := range parseErrors {
+		if parseErr != nil {
+			switch e := parseErr.(type) {
 			case *errors.ErrInvalidUUID:
 				myID, ok := s.DeviceNameToUUIDLookup[e.DeviceName]
 				if !ok {
 					log.Debug(fmt.Sprintf("no device named %s found", foundID))
 					return nil, &errors.ErrNotFound{ID: foundID}
 				}
-
 				foundID = myID
 			}
 		}
@@ -71,6 +70,30 @@ func (s *DeviceStore) GetDevice(id uuid.UUID, parseErrors ...error) (device.Devi
 	return d, nil
 }
 
+// GetDevicesAssociatedWithSbi ranges over devices within the device store and
+// checks if they are associated with the provided SBI. Returns a slice of
+// device.Device with all associated devices.
+func (s *DeviceStore) GetDevicesAssociatedWithSbi(sid uuid.UUID) ([]device.Device, error) {
+	var devices []device.Device
+	// range over all storable items within the device store
+	for _, item := range s.Store {
+		d, ok := item.(device.Device)
+		if !ok {
+			return nil, &errors.ErrInvalidTypeAssertion{
+				Value: d,
+				Type:  reflect.TypeOf((*device.Device)(nil)),
+			}
+		}
+		// check if the device uses the provided SBI and add it to the devices
+		// slice.
+		if d.SBI().ID() == sid {
+			devices = append(devices, d)
+		}
+	}
+
+	return devices, nil
+}
+
 // Add adds a device to the device store.
 // It also adds the name of the device to the lookup table.
 func (s *DeviceStore) Add(item store.Storable, name string) error {

store/device_store_test.go

@@ -8,6 +8,8 @@ import (
 	"code.fbi.h-da.de/danet/gosdn/interfaces/device"
 	"code.fbi.h-da.de/danet/gosdn/interfaces/store"
 	"code.fbi.h-da.de/danet/gosdn/mocks"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/google/uuid"
 )
 
@@ -95,3 +97,146 @@ func Test_deviceStore_get(t *testing.T) {
 		})
 	}
 }
+
+func Test_deviceStore_GetDevicesAsssociatedWithSbi(t *testing.T) {
+	mockSBI := &mocks.SouthboundInterface{}
+	mockSBI.On("ID").Return(defaultSbiID)
+
+	createDeviceMock := func(name string, sid uuid.UUID) *mocks.Device {
+		dm := &mocks.Device{}
+		dm.On("ID").Return(uuid.New())
+		dm.On("Name").Return("did")
+		dm.On("SBI").Return(mockSBI)
+		return dm
+	}
+
+	associatedDevices := []device.Device{
+		createDeviceMock("mockDevice1", defaultSbiID),
+		createDeviceMock("mockDevice2", defaultSbiID),
+		createDeviceMock("mockDevice3", defaultSbiID),
+	}
+
+	nonAssociatedDevice := createDeviceMock("mockDevice4", uuid.New())
+
+	pndMock := &mocks.NetworkDomain{}
+	pndMock.On("ID").Return(did)
+
+	// options to apply to cmp.Equal
+	opts := []cmp.Option{
+		// create custom comparer that simply checks if the device ID's are the
+		// same.
+		cmp.Comparer(func(x, y device.Device) bool {
+			return x.ID() == y.ID()
+		}),
+		// compare option to treat slices of length zero as equal
+		cmpopts.EquateEmpty(),
+		// sort the slices based on the ID
+		cmpopts.SortSlices(func(x, y device.Device) bool {
+			return x.ID().ID() < y.ID().ID()
+		}),
+	}
+
+	type fields struct {
+		genericStore *genericStore
+	}
+	type args struct {
+		sid uuid.UUID
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		args    args
+		want    []device.Device
+		wantErr bool
+	}{
+		{
+			name: "return devices associated with SBI",
+			fields: fields{
+				&genericStore{
+					Store: map[uuid.UUID]store.Storable{
+						associatedDevices[0].ID(): associatedDevices[0],
+						associatedDevices[1].ID(): associatedDevices[1],
+						associatedDevices[2].ID(): associatedDevices[2],
+					},
+					storeLock: sync.RWMutex{},
+				},
+			},
+			args: args{
+				sid: defaultSbiID,
+			},
+			want:    associatedDevices,
+			wantErr: false,
+		},
+		{
+			name: "non associated devices should not be part of the returned slice",
+			fields: fields{
+				&genericStore{
+					Store: map[uuid.UUID]store.Storable{
+						associatedDevices[0].ID(): associatedDevices[0],
+						associatedDevices[1].ID(): associatedDevices[1],
+						associatedDevices[2].ID(): associatedDevices[2],
+						nonAssociatedDevice.ID():  nonAssociatedDevice,
+					},
+					storeLock: sync.RWMutex{},
+				},
+			},
+			args: args{
+				sid: defaultSbiID,
+			},
+			want:    append(associatedDevices, nonAssociatedDevice),
+			wantErr: false,
+		},
+		{
+			name: "empty",
+			fields: fields{
+				&genericStore{
+					Store:     map[uuid.UUID]store.Storable{},
+					storeLock: sync.RWMutex{},
+				},
+			},
+			want:    []device.Device{},
+			wantErr: false,
+		},
+		{
+			name: "no device associated",
+			fields: fields{
+				&genericStore{
+					Store: map[uuid.UUID]store.Storable{
+						nonAssociatedDevice.ID(): nonAssociatedDevice,
+					},
+					storeLock: sync.RWMutex{},
+				},
+			},
+			want:    []device.Device{},
+			wantErr: false,
+		},
+		{
+			name: "fails wrong type",
+			fields: fields{
+				&genericStore{
+					Store: map[uuid.UUID]store.Storable{
+						defaultPndID: pndMock,
+					},
+					storeLock: sync.RWMutex{},
+				},
+			},
+			args:    args{sid: defaultSbiID},
+			want:    nil,
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s := DeviceStore{genericStore: tt.fields.genericStore}
+
+			got, err := s.GetDevicesAssociatedWithSbi(tt.args.sid)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("GetDevicesAssociatedWithSbi() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !cmp.Equal(got, tt.want, opts...) {
+				t.Errorf("GetDevicesAssociatedWithSbi() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}