[VOL-3767] Removing device flows during deletion of device and completing deletion regardless of devices not found
Change-Id: I803003d42ecfce98240efe22559c28ca2180e9ca
diff --git a/rw_core/core/device/logical_agent_flow.go b/rw_core/core/device/logical_agent_flow.go
index fa1a19a..eda4fa6 100644
--- a/rw_core/core/device/logical_agent_flow.go
+++ b/rw_core/core/device/logical_agent_flow.go
@@ -356,9 +356,20 @@
devicesInFlows = []string{agent.rootDeviceID}
}
- if err := agent.deviceMgr.canMultipleAdapterRequestProceed(ctx, devicesInFlows); err != nil {
- logger.Warnw(ctx, "adapters-not-ready", log.Fields{"logical-device-id": agent.logicalDeviceID, "flow": toDelete, "error": err})
- return err
+ for _, deviceID := range devicesInFlows {
+ if err := agent.deviceMgr.canAdapterRequestProceed(ctx, deviceID); err != nil {
+ //If the error has code.NotFound the device is not there anymore, there is no need to delete flows, just ignore it
+ if status.Code(err) != codes.NotFound {
+ logger.Warnw(ctx, "adapters-not-ready", log.Fields{"logical-device-id": agent.logicalDeviceID, "flow": toDelete, "error": err})
+ return err
+ } else {
+ logger.Debugw(ctx, "flow-delete-for-nil-device-proceeding-deletion", log.Fields{"deviceID": deviceID})
+ if deviceRules != nil {
+ deviceRules.RemoveRule(deviceID)
+ partialRoute = true
+ }
+ }
+ }
}
// Update the devices
@@ -456,9 +467,20 @@
devicesInFlows = []string{agent.rootDeviceID}
}
- if err := agent.deviceMgr.canMultipleAdapterRequestProceed(ctx, devicesInFlows); err != nil {
- logger.Warnw(ctx, "adapters-not-ready", log.Fields{"logical-device-id": agent.logicalDeviceID, "flow": flowsToDelete, "error": err})
- return err
+ for _, deviceID := range devicesInFlows {
+ if err := agent.deviceMgr.canAdapterRequestProceed(ctx, deviceID); err != nil {
+ //If the error has code.NotFound the device is not there anymore, there is no need to delete flows, just ignore it
+ if status.Code(err) != codes.NotFound {
+ logger.Warnw(ctx, "adapters-not-ready", log.Fields{"logical-device-id": agent.logicalDeviceID, "flow": flowsToDelete, "error": err})
+ return err
+ } else {
+ logger.Debugw(ctx, "flow-delete-strict-for-nil-device-proceeding-deletion", log.Fields{"deviceID": deviceID})
+ if deviceRules != nil {
+ deviceRules.RemoveRule(deviceID)
+ partialRoute = true
+ }
+ }
+ }
}
// Update the devices
diff --git a/rw_core/core/device/manager.go b/rw_core/core/device/manager.go
index 07e92d0..c1148b5 100755
--- a/rw_core/core/device/manager.go
+++ b/rw_core/core/device/manager.go
@@ -465,37 +465,44 @@
return status.Errorf(codes.NotFound, "%s", deviceID)
}
-func (dMgr *Manager) canMultipleAdapterRequestProceed(ctx context.Context, deviceIDs []string) error {
- ready := len(deviceIDs) > 0
- for _, deviceID := range deviceIDs {
- agent := dMgr.getDeviceAgent(ctx, deviceID)
- if agent == nil {
- logger.Errorw(ctx, "adapter-nil", log.Fields{"device-id": deviceID})
- return status.Errorf(codes.Unavailable, "adapter-nil-for-%s", deviceID)
+func (dMgr *Manager) canAdapterRequestProceed(ctx context.Context, deviceID string) error {
+ agent := dMgr.getDeviceAgent(ctx, deviceID)
+ if agent == nil {
+ logger.Errorw(ctx, "device-nil", log.Fields{"device-id": deviceID})
+ return status.Errorf(codes.NotFound, "device-nil-for-%s", deviceID)
+ }
+ if !agent.isAdapterConnectionUp(ctx) {
+ return status.Errorf(codes.Unavailable, "adapter-connection-down-for-%s", deviceID)
+ }
+ if err := agent.canDeviceRequestProceed(ctx); err != nil {
+ return err
+ }
+ // Perform the same checks for parent device
+ if !agent.isRootDevice {
+ parentDeviceAgent := dMgr.getDeviceAgent(ctx, agent.parentID)
+ if parentDeviceAgent == nil {
+ logger.Errorw(ctx, "parent-device-adapter-nil", log.Fields{"parent-id": agent.parentID})
+ return status.Errorf(codes.NotFound, "parent-device-adapter-nil-for-%s", deviceID)
}
- ready = ready && agent.isAdapterConnectionUp(ctx)
- if !ready {
- return status.Errorf(codes.Unavailable, "adapter-connection-down-for-%s", deviceID)
- }
- if err := agent.canDeviceRequestProceed(ctx); err != nil {
+ if err := parentDeviceAgent.canDeviceRequestProceed(ctx); err != nil {
return err
}
- // Perform the same checks for parent device
- if !agent.isRootDevice {
- parentDeviceAgent := dMgr.getDeviceAgent(ctx, agent.parentID)
- if parentDeviceAgent == nil {
- logger.Errorw(ctx, "parent-device-adapter-nil", log.Fields{"parent-id": agent.parentID})
- return status.Errorf(codes.Unavailable, "parent-device-adapter-nil-for-%s", deviceID)
- }
- if err := parentDeviceAgent.canDeviceRequestProceed(ctx); err != nil {
- return err
- }
- }
-
}
- if !ready {
+
+ return nil
+}
+
+func (dMgr *Manager) canMultipleAdapterRequestProceed(ctx context.Context, deviceIDs []string) error {
+ if len(deviceIDs) == 0 {
return status.Error(codes.Unavailable, "adapter(s)-not-ready")
}
+
+ for _, deviceID := range deviceIDs {
+ if err := dMgr.canAdapterRequestProceed(ctx, deviceID); err != nil {
+ return err
+ }
+ }
+
return nil
}
diff --git a/rw_core/core/device/state/transitions.go b/rw_core/core/device/state/transitions.go
index ed0a59d..b8e429c 100644
--- a/rw_core/core/device/state/transitions.go
+++ b/rw_core/core/device/state/transitions.go
@@ -201,13 +201,13 @@
deviceType: child,
previousState: deviceState{Admin: voltha.AdminState_UNKNOWN, Connection: voltha.ConnectStatus_UNKNOWN, Operational: voltha.OperStatus_UNKNOWN, Transient: core.DeviceTransientState_ANY},
currentState: deviceState{Admin: voltha.AdminState_UNKNOWN, Connection: voltha.ConnectStatus_UNKNOWN, Operational: voltha.OperStatus_UNKNOWN, Transient: core.DeviceTransientState_FORCE_DELETING},
- handlers: []transitionHandler{dMgr.ChildDeviceLost, dMgr.DeleteLogicalPorts, dMgr.RunPostDeviceDelete}})
+ handlers: []transitionHandler{dMgr.DeleteAllDeviceFlows, dMgr.ChildDeviceLost, dMgr.DeleteLogicalPorts, dMgr.RunPostDeviceDelete}})
transitionMap.transitions = append(transitionMap.transitions,
transition{ //DELETE after adapter response case
deviceType: child,
previousState: deviceState{Admin: voltha.AdminState_UNKNOWN, Connection: voltha.ConnectStatus_UNKNOWN, Operational: voltha.OperStatus_UNKNOWN, Transient: core.DeviceTransientState_ANY},
currentState: deviceState{Admin: voltha.AdminState_UNKNOWN, Connection: voltha.ConnectStatus_UNKNOWN, Operational: voltha.OperStatus_UNKNOWN, Transient: core.DeviceTransientState_DELETING_POST_ADAPTER_RESPONSE},
- handlers: []transitionHandler{dMgr.ChildDeviceLost, dMgr.DeleteLogicalPorts, dMgr.RunPostDeviceDelete}})
+ handlers: []transitionHandler{dMgr.DeleteAllDeviceFlows, dMgr.ChildDeviceLost, dMgr.DeleteLogicalPorts, dMgr.RunPostDeviceDelete}})
transitionMap.transitions = append(transitionMap.transitions,
transition{ //DELETE wait for adapter response(no operation)
deviceType: child,
diff --git a/rw_core/core/device/state/transitions_test.go b/rw_core/core/device/state/transitions_test.go
index cef7af9..d503efa 100644
--- a/rw_core/core/device/state/transitions_test.go
+++ b/rw_core/core/device/state/transitions_test.go
@@ -199,10 +199,11 @@
device = getDevice(false, voltha.AdminState_ENABLED, voltha.ConnectStatus_UNKNOWN, voltha.OperStatus_FAILED)
handlers = transitionMap.getTransitionHandler(ctx, device, previousDevice, core.DeviceTransientState_FORCE_DELETING,
core.DeviceTransientState_ANY)
- assert.Equal(t, 3, len(handlers))
- assert.True(t, reflect.ValueOf(tdm.ChildDeviceLost).Pointer() == reflect.ValueOf(handlers[0]).Pointer())
- assert.True(t, reflect.ValueOf(tdm.DeleteLogicalPorts).Pointer() == reflect.ValueOf(handlers[1]).Pointer())
- assert.True(t, reflect.ValueOf(tdm.RunPostDeviceDelete).Pointer() == reflect.ValueOf(handlers[2]).Pointer())
+ assert.Equal(t, 4, len(handlers))
+ assert.True(t, reflect.ValueOf(tdm.DeleteAllDeviceFlows).Pointer() == reflect.ValueOf(handlers[0]).Pointer())
+ assert.True(t, reflect.ValueOf(tdm.ChildDeviceLost).Pointer() == reflect.ValueOf(handlers[1]).Pointer())
+ assert.True(t, reflect.ValueOf(tdm.DeleteLogicalPorts).Pointer() == reflect.ValueOf(handlers[2]).Pointer())
+ assert.True(t, reflect.ValueOf(tdm.RunPostDeviceDelete).Pointer() == reflect.ValueOf(handlers[3]).Pointer())
previousDevice = getDevice(true, voltha.AdminState_ENABLED, voltha.ConnectStatus_REACHABLE, voltha.OperStatus_ACTIVE)
device = getDevice(true, voltha.AdminState_ENABLED, voltha.ConnectStatus_UNREACHABLE, voltha.OperStatus_UNKNOWN)
@@ -337,6 +338,7 @@
tdm.RunPostDeviceDelete,
},
expectedChildHandlers: []transitionHandler{
+ tdm.DeleteAllDeviceFlows,
tdm.ChildDeviceLost,
tdm.DeleteLogicalPorts,
tdm.RunPostDeviceDelete,
@@ -365,7 +367,7 @@
t.Run(testName, func(t *testing.T) {
handlers = transitionMap.getTransitionHandler(ctx, device, previousDevice, core.DeviceTransientState_FORCE_DELETING,
core.DeviceTransientState_ANY)
- assert.Equal(t, 3, len(handlers))
+ assert.Equal(t, 4, len(handlers))
for idx, expHandler := range deleteDeviceTest.expectedChildHandlers {
assert.True(t, reflect.ValueOf(expHandler).Pointer() == reflect.ValueOf(handlers[idx]).Pointer())
}
diff --git a/rw_core/flowdecomposition/flow_decomposer.go b/rw_core/flowdecomposition/flow_decomposer.go
index d9b81a0..134cb9d 100644
--- a/rw_core/flowdecomposition/flow_decomposer.go
+++ b/rw_core/flowdecomposition/flow_decomposer.go
@@ -70,7 +70,7 @@
}
// Handles special case of any controller-bound flow for a parent device
-func (fd *FlowDecomposer) updateOutputPortForControllerBoundFlowForParentDevide(ctx context.Context, dr *fu.DeviceRules) (*fu.DeviceRules, error) {
+func (fd *FlowDecomposer) updateOutputPortForControllerBoundFlowForParentDevice(ctx context.Context, dr *fu.DeviceRules) (*fu.DeviceRules, error) {
EAPOL := fu.EthType(0x888e)
PPPoED := fu.EthType(0x8863)
IGMP := fu.IpProto(2)
@@ -542,6 +542,6 @@
return deviceRules, status.Errorf(codes.Aborted, "unknown downstream flow %v", *flow)
}
}
- deviceRules, err = fd.updateOutputPortForControllerBoundFlowForParentDevide(ctx, deviceRules)
+ deviceRules, err = fd.updateOutputPortForControllerBoundFlowForParentDevice(ctx, deviceRules)
return deviceRules, err
}