Resolve "Restarting the controller after devices are registered is throwing a panic"

See merge request !373

Co-authored-by: Malte Bauch <malte.bauch@extern.h-da.de>

controller/interfaces/device/device.go

@@ -51,18 +51,4 @@ type LoadedDevice struct {
 	// SBI indicates the southbound interface, which is used by this device as UUID.
 	SBI   string `json:"sbi"`
 	Model string `json:"model,omitempty" bson:"model,omitempty"`
-
-	convertFunc func(LoadedDevice) (Device, error)
-}
-
-// SetConvertFunction allows to set the LoadedDevice's convert function. This
-// function should take a LoadedDevice and returns a Device.
-func (ld *LoadedDevice) SetConvertFunction(cf func(LoadedDevice) (Device, error)) {
-	ld.convertFunc = cf
-}
-
-// ConvertToDevice calls the LoadedDevice's convert function and converts the
-// LoadedDevice into a Device.
-func (ld LoadedDevice) ConvertToDevice() (Device, error) {
-	return ld.convertFunc(ld)
 }

controller/interfaces/device/deviceService.go

@@ -11,5 +11,6 @@ type Service interface {
 	UpdateModel(Device, string) error
 	Delete(Device) error
 	Get(store.Query) (Device, error)
-	GetAll() ([]LoadedDevice, error)
+	GetAll() ([]Device, error)
+	GetAllAsLoaded() ([]LoadedDevice, error)
 }

controller/interfaces/networkdomain/pnd.go

@@ -20,7 +20,8 @@ type NetworkDomain interface {
 	GetDevice(identifier string) (device.Device, error)
 	RemoveDevice(uuid.UUID) error
 	UpdateDevice(device.Device, string) error
-	Devices() []device.LoadedDevice
+	Devices() []device.Device
+	FlattenedDevices() []device.LoadedDevice
 	ChangeOND(uuid uuid.UUID, operation ppb.ApiOperation, path string, value ...string) (uuid.UUID, error)
 	Request(uuid.UUID, string) (proto.Message, error)
 	RequestAll(string) error

controller/mocks/NetworkDomain.go

@@ -150,7 +150,23 @@ func (_m *NetworkDomain) Destroy() error {
 }
 
 // Devices provides a mock function with given fields:
-func (_m *NetworkDomain) Devices() []device.LoadedDevice {
+func (_m *NetworkDomain) Devices() []device.Device {
+	ret := _m.Called()
+
+	var r0 []device.Device
+	if rf, ok := ret.Get(0).(func() []device.Device); ok {
+		r0 = rf()
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).([]device.Device)
+		}
+	}
+
+	return r0
+}
+
+// FlattenedDevices provides a mock function with given fields:
+func (_m *NetworkDomain) FlattenedDevices() []device.LoadedDevice {
 	ret := _m.Called()
 
 	var r0 []device.LoadedDevice

controller/mocks/Service.go

@@ -3,10 +3,9 @@
 package mocks
 
 import (
-	device "code.fbi.h-da.de/danet/gosdn/controller/interfaces/device"
-	mock "github.com/stretchr/testify/mock"
+	controllerevent "code.fbi.h-da.de/danet/gosdn/controller/event"
 
-	store "code.fbi.h-da.de/danet/gosdn/controller/store"
+	mock "github.com/stretchr/testify/mock"
 )
 
 // Service is an autogenerated mock type for the Service type
@@ -14,101 +13,18 @@ type Service struct {
 	mock.Mock
 }
 
-// Add provides a mock function with given fields: _a0
-func (_m *Service) Add(_a0 device.Device) error {
-	ret := _m.Called(_a0)
-
-	var r0 error
-	if rf, ok := ret.Get(0).(func(device.Device) error); ok {
-		r0 = rf(_a0)
-	} else {
-		r0 = ret.Error(0)
-	}
-
-	return r0
-}
-
-// Delete provides a mock function with given fields: _a0
-func (_m *Service) Delete(_a0 device.Device) error {
-	ret := _m.Called(_a0)
-
-	var r0 error
-	if rf, ok := ret.Get(0).(func(device.Device) error); ok {
-		r0 = rf(_a0)
-	} else {
-		r0 = ret.Error(0)
-	}
-
-	return r0
-}
-
-// Get provides a mock function with given fields: _a0
-func (_m *Service) Get(_a0 store.Query) (device.Device, error) {
-	ret := _m.Called(_a0)
-
-	var r0 device.Device
-	if rf, ok := ret.Get(0).(func(store.Query) device.Device); ok {
-		r0 = rf(_a0)
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(device.Device)
-		}
-	}
-
-	var r1 error
-	if rf, ok := ret.Get(1).(func(store.Query) error); ok {
-		r1 = rf(_a0)
-	} else {
-		r1 = ret.Error(1)
-	}
-
-	return r0, r1
-}
-
-// GetAll provides a mock function with given fields:
-func (_m *Service) GetAll() ([]device.Device, error) {
-	ret := _m.Called()
-
-	var r0 []device.Device
-	if rf, ok := ret.Get(0).(func() []device.Device); ok {
-		r0 = rf()
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).([]device.Device)
-		}
-	}
-
-	var r1 error
-	if rf, ok := ret.Get(1).(func() error); ok {
-		r1 = rf()
-	} else {
-		r1 = ret.Error(1)
-	}
-
-	return r0, r1
-}
-
-// Update provides a mock function with given fields: _a0
-func (_m *Service) Update(_a0 device.Device) error {
-	ret := _m.Called(_a0)
-
-	var r0 error
-	if rf, ok := ret.Get(0).(func(device.Device) error); ok {
-		r0 = rf(_a0)
-	} else {
-		r0 = ret.Error(0)
-	}
-
-	return r0
+// CloseConnection provides a mock function with given fields:
+func (_m *Service) CloseConnection() {
+	_m.Called()
 }
 
-// UpdateModel provides a mock function with given fields: _a0, _a1
-func (_m *Service) UpdateModel(_a0 device.Device, _a1 string) error {
-	ret := _m.Called(_a0, _a1)
+// PublishEvent provides a mock function with given fields: topic, _a1
+func (_m *Service) PublishEvent(topic string, _a1 controllerevent.Event) error {
+	ret := _m.Called(topic, _a1)
 
 	var r0 error
-	if rf, ok := ret.Get(0).(func(device.Device, string) error); ok {
-		r0 = rf(_a0, _a1)
+	if rf, ok := ret.Get(0).(func(string, controllerevent.Event) error); ok {
+		r0 = rf(topic, _a1)
 	} else {
 		r0 = ret.Error(0)
 	}

controller/mocks/Store.go

@@ -3,8 +3,8 @@
 package mocks
 
 import (
-	southbound "code.fbi.h-da.de/danet/gosdn/controller/interfaces/southbound"
-	store "code.fbi.h-da.de/danet/gosdn/controller/store"
+	store "code.fbi.h-da.de/danet/gosdn/controller/interfaces/store"
+	uuid "github.com/google/uuid"
 	mock "github.com/stretchr/testify/mock"
 )
 
@@ -13,13 +13,13 @@ type Store struct {
 	mock.Mock
 }
 
-// Add provides a mock function with given fields: _a0
-func (_m *Store) Add(_a0 southbound.SouthboundInterface) error {
-	ret := _m.Called(_a0)
+// Add provides a mock function with given fields: item
+func (_m *Store) Add(item store.Storable) error {
+	ret := _m.Called(item)
 
 	var r0 error
-	if rf, ok := ret.Get(0).(func(southbound.SouthboundInterface) error); ok {
-		r0 = rf(_a0)
+	if rf, ok := ret.Get(0).(func(store.Storable) error); ok {
+		r0 = rf(item)
 	} else {
 		r0 = ret.Error(0)
 	}
@@ -27,13 +27,13 @@ func (_m *Store) Add(_a0 southbound.SouthboundInterface) error {
 	return r0
 }
 
-// Delete provides a mock function with given fields: _a0
-func (_m *Store) Delete(_a0 southbound.SouthboundInterface) error {
-	ret := _m.Called(_a0)
+// Delete provides a mock function with given fields: id
+func (_m *Store) Delete(id uuid.UUID) error {
+	ret := _m.Called(id)
 
 	var r0 error
-	if rf, ok := ret.Get(0).(func(southbound.SouthboundInterface) error); ok {
-		r0 = rf(_a0)
+	if rf, ok := ret.Get(0).(func(uuid.UUID) error); ok {
+		r0 = rf(id)
 	} else {
 		r0 = ret.Error(0)
 	}
@@ -41,20 +41,36 @@ func (_m *Store) Delete(_a0 southbound.SouthboundInterface) error {
 	return r0
 }
 
-// Get provides a mock function with given fields: _a0
-func (_m *Store) Get(_a0 store.Query) (southbound.LoadedSbi, error) {
-	ret := _m.Called(_a0)
+// Exists provides a mock function with given fields: id
+func (_m *Store) Exists(id uuid.UUID) bool {
+	ret := _m.Called(id)
 
-	var r0 southbound.LoadedSbi
-	if rf, ok := ret.Get(0).(func(store.Query) southbound.LoadedSbi); ok {
-		r0 = rf(_a0)
+	var r0 bool
+	if rf, ok := ret.Get(0).(func(uuid.UUID) bool); ok {
+		r0 = rf(id)
 	} else {
-		r0 = ret.Get(0).(southbound.LoadedSbi)
+		r0 = ret.Get(0).(bool)
+	}
+
+	return r0
+}
+
+// Get provides a mock function with given fields: id
+func (_m *Store) Get(id uuid.UUID) (store.Storable, error) {
+	ret := _m.Called(id)
+
+	var r0 store.Storable
+	if rf, ok := ret.Get(0).(func(uuid.UUID) store.Storable); ok {
+		r0 = rf(id)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(store.Storable)
+		}
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(store.Query) error); ok {
-		r1 = rf(_a0)
+	if rf, ok := ret.Get(1).(func(uuid.UUID) error); ok {
+		r1 = rf(id)
 	} else {
 		r1 = ret.Error(1)
 	}
@@ -62,27 +78,20 @@ func (_m *Store) Get(_a0 store.Query) (southbound.LoadedSbi, error) {
 	return r0, r1
 }
 
-// GetAll provides a mock function with given fields:
-func (_m *Store) GetAll() ([]southbound.LoadedSbi, error) {
+// UUIDs provides a mock function with given fields:
+func (_m *Store) UUIDs() []uuid.UUID {
 	ret := _m.Called()
 
-	var r0 []southbound.LoadedSbi
-	if rf, ok := ret.Get(0).(func() []southbound.LoadedSbi); ok {
+	var r0 []uuid.UUID
+	if rf, ok := ret.Get(0).(func() []uuid.UUID); ok {
 		r0 = rf()
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).([]southbound.LoadedSbi)
+			r0 = ret.Get(0).([]uuid.UUID)
 		}
 	}
 
-	var r1 error
-	if rf, ok := ret.Get(1).(func() error); ok {
-		r1 = rf()
-	} else {
-		r1 = ret.Error(1)
-	}
-
-	return r0, r1
+	return r0
 }
 
 type mockConstructorTestingTNewStore interface {

controller/northbound/server/device.go

@@ -54,10 +54,6 @@ func (d *DeviceServer) GetAll(ctx context.Context, request *dpb.GetAllDeviceRequ
 
 	onds := []*dpb.Device{}
 	for _, device := range devices {
-		device, err := device.ConvertToDevice()
-		if err != nil {
-			return nil, status.Errorf(codes.Aborted, "%v", err)
-		}
 		ygotStructAsJSON, err := device.GetModelAsString()
 		if err != nil {
 			log.Error(err)

controller/northbound/server/pnd.go

@@ -58,7 +58,7 @@ func (p PndServer) GetOnd(ctx context.Context, request *ppb.GetOndRequest) (*ppb
 		return nil, status.Errorf(codes.Aborted, "%v", err)
 	}
 
-	ond, err := fillOndBySpecificPath(pnd, device, "/")
+	ond, err := fillOndBySpecificPath(device, "/")
 	if err != nil {
 		log.Error(err)
 		return nil, status.Errorf(codes.Aborted, "%v", err)
@@ -92,12 +92,8 @@ func (p PndServer) GetOndList(ctx context.Context, request *ppb.GetOndListReques
 	}
 
 	onds := make([]*ppb.OrchestratedNetworkingDevice, len(pnd.Devices()))
-	for i, loadedDevice := range pnd.Devices() {
-		device, err := loadedDevice.ConvertToDevice()
-		if err != nil {
-			return nil, status.Errorf(codes.Aborted, "%v", err)
-		}
-		ond, err := fillOndBySpecificPath(pnd, device, "/")
+	for i, device := range pnd.Devices() {
+		ond, err := fillOndBySpecificPath(device, "/")
 		if err != nil {
 			log.Error(err)
 			return nil, status.Errorf(codes.Aborted, "%v", err)
@@ -132,7 +128,7 @@ func (p PndServer) GetFlattenedOndList(ctx context.Context, request *ppb.GetOndL
 		return nil, status.Errorf(codes.Aborted, "%v", err)
 	}
 
-	onds := pnd.Devices()
+	onds := pnd.FlattenedDevices()
 	flattenedOnds := make([]*ppb.FlattenedOrchestratedNetworkingDevice, len(onds))
 	for i, ond := range onds {
 		ond := &ppb.FlattenedOrchestratedNetworkingDevice{
@@ -155,7 +151,7 @@ func (p PndServer) GetFlattenedOndList(ctx context.Context, request *ppb.GetOndL
 	}, nil
 }
 
-func fillOndBySpecificPath(pnd networkdomain.NetworkDomain, d device.Device, path string) (*ppb.OrchestratedNetworkingDevice, error) {
+func fillOndBySpecificPath(d device.Device, path string) (*ppb.OrchestratedNetworkingDevice, error) {
 	gnmiPath, err := ygot.StringToStructuredPath(path)
 	if err != nil {
 		log.Error(err)
@@ -354,7 +350,7 @@ func (p PndServer) GetPath(ctx context.Context, request *ppb.GetPathRequest) (*p
 		return nil, status.Errorf(codes.Aborted, "%v", err)
 	}
 
-	ond, err := fillOndBySpecificPath(pnd, device, path)
+	ond, err := fillOndBySpecificPath(device, path)
 	if err != nil {
 		log.Error(err)
 		return nil, status.Errorf(codes.Aborted, "%v", err)

controller/nucleus/deviceService.go

@@ -49,8 +49,6 @@ func (s *DeviceService) Get(query store.Query) (device.Device, error) {
 		return nil, err
 	}
 
-	loadedDevice.SetConvertFunction(s.createDeviceFromStore)
-
 	device, err := s.createDeviceFromStore(loadedDevice)
 	if err != nil {
 		return nil, err
@@ -60,14 +58,34 @@ func (s *DeviceService) Get(query store.Query) (device.Device, error) {
 }
 
 // GetAll returns all stored devices.
-func (s *DeviceService) GetAll() ([]device.LoadedDevice, error) {
+func (s *DeviceService) GetAll() ([]device.Device, error) {
+	var devices []device.Device
+
 	loadedDevices, err := s.deviceStore.GetAll()
 	if err != nil {
 		return nil, err
 	}
 
 	for _, loadedDevice := range loadedDevices {
-		loadedDevice.SetConvertFunction(s.createDeviceFromStore)
+		device, err := s.createDeviceFromStore(loadedDevice)
+		if err != nil {
+			return nil, err
+		}
+
+		devices = append(devices, device)
+	}
+
+	return devices, nil
+}
+
+// GetAllAsLoaded returns all stored devices as LoadedDevice.
+// This method should be used if there is no need for a device.Device, since
+// requesting device information through this method is a lot faster than the
+// usual `GetAll` method.
+func (s *DeviceService) GetAllAsLoaded() ([]device.LoadedDevice, error) {
+	loadedDevices, err := s.deviceStore.GetAll()
+	if err != nil {
+		return nil, err
 	}
 
 	return loadedDevices, nil

controller/nucleus/deviceServiceMock.go

@@ -89,7 +89,18 @@ func (t *DeviceServiceMock) Get(query store.Query) (device.Device, error) {
 }
 
 // GetAll gets all items.
-func (t *DeviceServiceMock) GetAll() ([]device.LoadedDevice, error) {
+func (t *DeviceServiceMock) GetAll() ([]device.Device, error) {
+	var allItems []device.Device
+
+	for _, item := range t.Store {
+		allItems = append(allItems, item)
+	}
+
+	return allItems, nil
+}
+
+// GetAllAsLoaded gets all items as `device.LoadedDevice`.
+func (t *DeviceServiceMock) GetAllAsLoaded() ([]device.LoadedDevice, error) {
 	var allItems []device.LoadedDevice
 
 	for _, item := range t.Store {

controller/nucleus/deviceWatcher.go

@@ -68,8 +68,7 @@ func (d *DeviceWatcher) SubToDevices(paths [][]string, opts *gnmi.SubscribeOptio
 }
 
 func (d *DeviceWatcher) subscribeToPndDevices(pndID string, pnd networkdomain.NetworkDomain, opts *gnmi.SubscribeOptions) {
-	for _, loadedDevice := range pnd.Devices() {
-		device, _ := loadedDevice.ConvertToDevice()
+	for _, device := range pnd.Devices() {
 		subID := uuid.New()
 
 		stopContext, cancel := context.WithCancel(context.Background())

controller/nucleus/principalNetworkDomain.go

@@ -157,12 +157,18 @@ func (pnd *pndImplementation) ID() uuid.UUID {
 	return pnd.Id
 }
 
-func (pnd *pndImplementation) Devices() []device.LoadedDevice {
+func (pnd *pndImplementation) Devices() []device.Device {
 	allDevices, _ := pnd.deviceService.GetAll()
 
 	return allDevices
 }
 
+func (pnd *pndImplementation) FlattenedDevices() []device.LoadedDevice {
+	allDevices, _ := pnd.deviceService.GetAllAsLoaded()
+
+	return allDevices
+}
+
 // GetName returns the name of the PND.
 func (pnd *pndImplementation) GetName() string {
 	return pnd.Name
@@ -206,7 +212,7 @@ func (pnd *pndImplementation) AddSbi(s southbound.SouthboundInterface) error {
 func (pnd *pndImplementation) RemoveSbi(sid uuid.UUID) error {
 	var associatedDevices []device.LoadedDevice
 
-	allExistingDevices, err := pnd.deviceService.GetAll()
+	allExistingDevices, err := pnd.deviceService.GetAllAsLoaded()
 	if err != nil {
 		return err
 	}
@@ -436,7 +442,7 @@ func (pnd *pndImplementation) Request(uuid uuid.UUID, path string) (proto.Messag
 
 // RequestAll sends a request for all registered devices.
 func (pnd *pndImplementation) RequestAll(path string) error {
-	allDevices, err := pnd.deviceService.GetAll()
+	allDevices, err := pnd.deviceService.GetAllAsLoaded()
 	if err != nil {
 		return err
 	}

controller/nucleus/principalNetworkDomain_test.go

@@ -785,7 +785,7 @@ func Test_pndImplementation_ChangeOND(t *testing.T) {
 				return
 			}
 
-			devices, err := pnd.deviceService.GetAll()
+			devices, err := pnd.deviceService.GetAllAsLoaded()
 			if err != nil {
 				err := errors.New("error fetching device")
 				t.Error(err)