[VOL-4495] Handling both cases in which the NNI goes down

Change-Id: I57e93b91e5a80f5afe9bae9e5f734186b328b965
diff --git a/impl/src/main/java/org/opencord/olt/impl/Olt.java b/impl/src/main/java/org/opencord/olt/impl/Olt.java
index af3eed8..8f035bf 100644
--- a/impl/src/main/java/org/opencord/olt/impl/Olt.java
+++ b/impl/src/main/java/org/opencord/olt/impl/Olt.java
@@ -819,11 +819,15 @@
                     portWithName(port), port.isEnabled() ? "ENABLED" : "DISABLED", device.id());
             if (port.isEnabled()) {
                 if (oltDeviceService.isNniPort(device, port.number())) {
-                    // NOTE in the NNI case we receive a PORT_REMOVED event with status ENABLED, thus we need to
-                    // pass the floeAction to the handleNniFlows method
                     OltFlowService.FlowOperation action = OltFlowService.FlowOperation.ADD;
+                    // NOTE the NNI is only disabled if the OLT shuts down (reboot or failure).
+                    // In that case the flows are purged anyway, so there's no need to deal with them,
+                    // it would actually be counter-productive as the openflow connection is severed and they won't
+                    // be correctly processed
                     if (type == DeviceEvent.Type.PORT_REMOVED) {
-                        action = OltFlowService.FlowOperation.REMOVE;
+                        log.debug("NNI port went down, " +
+                                "ignoring event as flows will be removed in the generic device cleanup");
+                        return;
                     }
                     oltFlowService.handleNniFlows(device, port, action);
                 } else {
diff --git a/impl/src/main/java/org/opencord/olt/impl/OltFlowService.java b/impl/src/main/java/org/opencord/olt/impl/OltFlowService.java
index 477e3a3..376056d 100644
--- a/impl/src/main/java/org/opencord/olt/impl/OltFlowService.java
+++ b/impl/src/main/java/org/opencord/olt/impl/OltFlowService.java
@@ -422,33 +422,36 @@
     public void handleNniFlows(Device device, Port port, FlowOperation action) {
 
         // always handle the LLDP flow
+        log.debug("{} LLDP trap flow on NNI {} for device {}", portWithName(port), device.id(), flowOpToString(action));
         processLldpFilteringObjective(device.id(), port, action);
 
         if (enableDhcpOnNni) {
             if (enableDhcpV4) {
-                log.debug("Installing DHCPv4 trap flow on NNI {} for device {}", portWithName(port), device.id());
+                log.debug("{} DHCPv4 trap flow on NNI {} for device {}", portWithName(port), device.id(),
+                        flowOpToString(action));
                 processDhcpFilteringObjectives(device.id(), port, action, FlowDirection.DOWNSTREAM,
                         67, 68, EthType.EtherType.IPV4.ethType(), IPv4.PROTOCOL_UDP,
                         null, null, nniUniTag);
             }
             if (enableDhcpV6) {
-                log.debug("Installing DHCPv6 trap flow on NNI {} for device {}", portWithName(port), device.id());
+                log.debug("{} DHCPv6 trap flow on NNI {} for device {}", portWithName(port), device.id(),
+                        flowOpToString(action));
                 processDhcpFilteringObjectives(device.id(), port, action, FlowDirection.DOWNSTREAM,
                         546, 547, EthType.EtherType.IPV6.ethType(), IPv6.PROTOCOL_UDP,
                         null, null, nniUniTag);
             }
         } else {
-            log.info("DHCP is not required on NNI {} for device {}", portWithName(port), device.id());
+            log.debug("DHCP is not required on NNI {} for device {}", portWithName(port), device.id());
         }
 
         if (enableIgmpOnNni) {
-            log.debug("Installing IGMP flow on NNI {} for device {}", portWithName(port), device.id());
+            log.debug("{} IGMP flow on NNI {} for device {}", portWithName(port), device.id(), flowOpToString(action));
             processIgmpFilteringObjectives(device.id(), port, action, FlowDirection.DOWNSTREAM,
                     null, null, NONE_TP_ID, VlanId.NONE, VlanId.ANY, -1);
         }
 
         if (enablePppoe) {
-            log.debug("Installing PPPoE flow on NNI {} for device {}", port.number(), device.id());
+            log.debug("{} PPPoE flow on NNI {} for device {}", port.number(), device.id(), flowOpToString(action));
             processPPPoEDFilteringObjectives(device.id(), port, action, FlowDirection.DOWNSTREAM,
                     null, null, NONE_TP_ID, VlanId.NONE, VlanId.ANY, null);
         }
diff --git a/impl/src/test/java/org/opencord/olt/impl/OltDeviceListenerTest.java b/impl/src/test/java/org/opencord/olt/impl/OltDeviceListenerTest.java
index 0d5204f..6af50f5 100644
--- a/impl/src/test/java/org/opencord/olt/impl/OltDeviceListenerTest.java
+++ b/impl/src/test/java/org/opencord/olt/impl/OltDeviceListenerTest.java
@@ -155,14 +155,15 @@
         verify(olt.oltFlowService, never())
                 .handleNniFlows(testDevice, disabledNniPort, OltFlowService.FlowOperation.REMOVE);
 
-        // when we disable the device we receive a PORT_REMOVED event with status ENABLED
-        // make sure we're removing the flows correctly
-        DeviceEvent nniRemoveEvent = new DeviceEvent(DeviceEvent.Type.PORT_REMOVED, testDevice, enabledNniPort);
-        oltDeviceListener.event(nniRemoveEvent);
+        // if the NNI is removed we ignore the event
+        Port removedNniPort = new OltPort(testDevice, true, PortNumber.portNumber(1048576),
+                DefaultAnnotations.builder().set(AnnotationKeys.PORT_NAME, "nni-1").build());
+        DeviceEvent nniRemovedEvent = new DeviceEvent(DeviceEvent.Type.PORT_UPDATED, testDevice, removedNniPort);
+        oltDeviceListener.event(nniRemovedEvent);
 
         assert olt.eventsQueues.isEmpty();
-        verify(olt.oltFlowService, times(1))
-                .handleNniFlows(testDevice, enabledNniPort, OltFlowService.FlowOperation.REMOVE);
+        verify(olt.oltFlowService, never())
+                .handleNniFlows(testDevice, removedNniPort, OltFlowService.FlowOperation.REMOVE);
     }
 
     @Test