Quick fix RPCs and DevicCtx.close

Change-Id: I1cd92d91e1025ae3aee00418b86912f2e86580b4
Signed-off-by: Vaclav Demcak <vdemcak@cisco.com>
Signed-off-by: Jozef Bacigal <jbacigal@cisco.com>

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/HandshakeListenerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/HandshakeListenerImpl.java
index 3ea654fe22364c722947a7f8c553f3f45b9f1380..61480de5f0f91aabee7f0171a976a626d8860933 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/HandshakeListenerImpl.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/connection/listener/HandshakeListenerImpl.java
@@ -34,21 +34,21 @@ public class HandshakeListenerImpl implements HandshakeListener {
 
     private static final Logger LOG = LoggerFactory.getLogger(HandshakeListenerImpl.class);
 
-    private ConnectionContext connectionContext;
-    private DeviceConnectedHandler deviceConnectedHandler;
+    private final ConnectionContext connectionContext;
+    private final DeviceConnectedHandler deviceConnectedHandler;
     private HandshakeContext handshakeContext;
 
     /**
      * @param connectionContext
      * @param deviceConnectedHandler
      */
-    public HandshakeListenerImpl(ConnectionContext connectionContext, DeviceConnectedHandler deviceConnectedHandler) {
+    public HandshakeListenerImpl(final ConnectionContext connectionContext, final DeviceConnectedHandler deviceConnectedHandler) {
         this.connectionContext = connectionContext;
         this.deviceConnectedHandler = deviceConnectedHandler;
     }
 
     @Override
-    public void onHandshakeSuccessfull(GetFeaturesOutput featureOutput, Short version) {
+    public void onHandshakeSuccessfull(final GetFeaturesOutput featureOutput, final Short version) {
         LOG.debug("handshake succeeded: {}", connectionContext.getConnectionAdapter().getRemoteAddress());
         closeHandshakeContext();
         connectionContext.changeStateToWorking();
@@ -65,11 +65,11 @@ public class HandshakeListenerImpl implements HandshakeListener {
                     deviceConnectedHandler.deviceConnected(connectionContext);
                     SessionStatistics.countEvent(connectionContext.getNodeId().toString(),
                             SessionStatistics.ConnectionStatus.CONNECTION_CREATED);
-                } catch (Exception e) {
+                } catch (final Exception e) {
                     LOG.info("ConnectionContext initial processing failed: {}", e.getMessage());
                     SessionStatistics.countEvent(connectionContext.getNodeId().toString(),
                             SessionStatistics.ConnectionStatus.CONNECTION_DISCONNECTED_BY_OFP);
-                    connectionContext.closeConnection(false);
+                    connectionContext.closeConnection(true);
                 }
             }
 
@@ -100,14 +100,14 @@ public class HandshakeListenerImpl implements HandshakeListener {
     private void closeHandshakeContext() {
         try {
             handshakeContext.close();
-        } catch (Exception e) {
+        } catch (final Exception e) {
             LOG.warn("Closing handshake context failed: {}", e.getMessage());
             LOG.debug("Detail in hanshake context close:", e);
         }
     }
 
     @Override
-    public void setHandshakeContext(HandshakeContext handshakeContext) {
+    public void setHandshakeContext(final HandshakeContext handshakeContext) {
         this.handshakeContext = handshakeContext;
     }
 }

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java
index 524e223f45622fab0d1b49984cf01fc0b05f61aa..e5c62d7e657a82bcfac6debb342a7f2d548fbc10 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java
@@ -14,6 +14,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.AsyncFunction;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.FutureCallback;
+import com.google.common.util.concurrent.FutureFallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import io.netty.util.HashedWheelTimer;
@@ -29,6 +30,8 @@ import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService;
 import org.opendaylight.controller.md.sal.binding.api.NotificationService;
@@ -244,6 +247,8 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
             return Futures.immediateFuture(null);
         }
         if (OfpRole.BECOMEMASTER.equals(role)) {
+            MdSalRegistratorUtils.registerMasterServices(getRpcContext(), DeviceContextImpl.this, role);
+            getRpcContext().registerStatCompatibilityServices();
             if (!deviceState.deviceSynchronized()) {
                 //TODO: no necessary code for yet - it needs for initialization phase only
                 LOG.debug("Setup Empty TxManager {} for initialization phase", getDeviceState().getNodeId());
@@ -251,7 +256,23 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
                 return Futures.immediateCheckedFuture(null);
             }
             /* Relevant for no initial Slave-to-Master scenario in cluster */
-            return asyncClusterRoleChange(role);
+            final ListenableFuture<Void> deviceInitialization = asyncClusterRoleChange();
+            Futures.addCallback(deviceInitialization, new FutureCallback<Void>() {
+
+                @Override
+                public void onSuccess(@Nullable Void aVoid) {
+                    //No operation
+                }
+
+                @Override
+                public void onFailure(Throwable throwable) {
+                    LOG.debug("Device {} init unexpected fail. Unregister RPCs", getDeviceState().getNodeId());
+                    MdSalRegistratorUtils.unregisterServices(getRpcContext());
+                }
+
+            });
+
+            return deviceInitialization;
 
         } else if (OfpRole.BECOMESLAVE.equals(role)) {
             if (null != rpcContext) {
@@ -272,14 +293,14 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
      * check all NodeInformation for statistics otherwise statistics will not contains
      * all possible MultipartTypes for polling in StatTypeList
      */
-    private ListenableFuture<Void> asyncClusterRoleChange(final OfpRole role) {
+    private ListenableFuture<Void> asyncClusterRoleChange() {
         if (statCtx == null) {
-            final String errMsg = String.format("DeviceCtx {} is up but we are missing StatisticsContext", deviceState.getNodeId());
+            final String errMsg = String.format("DeviceCtx %s is up but we are missing StatisticsContext", deviceState.getNodeId());
             LOG.warn(errMsg);
             return Futures.immediateFailedFuture(new IllegalStateException(errMsg));
         }
         if (rpcContext == null) {
-            final String errMsg = String.format("DeviceCtx {} is up but we are missing RpcContext", deviceState.getNodeId());
+            final String errMsg = String.format("DeviceCtx %s is up but we are missing RpcContext", deviceState.getNodeId());
             LOG.warn(errMsg);
             return Futures.immediateFailedFuture(new IllegalStateException(errMsg));
         }
@@ -322,14 +343,14 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
             @Override
             public Void apply(final Boolean input) {
                 if (ConnectionContext.CONNECTION_STATE.RIP.equals(getPrimaryConnectionContext().getConnectionState())) {
-                    final String errMsg = String.format("We lost connection for Device {}, context has to be closed.",
+                    final String errMsg = String.format("We lost connection for Device %s, context has to be closed.",
                             getDeviceState().getNodeId());
                     LOG.warn(errMsg);
                     transactionChainManager.clearUnsubmittedTransaction();
                     throw new IllegalStateException(errMsg);
                 }
                 if (!input.booleanValue()) {
-                    final String errMsg = String.format("Get Initial Device {} information fails",
+                    final String errMsg = String.format("Get Initial Device %s information fails",
                             getDeviceState().getNodeId());
                     LOG.warn(errMsg);
                     transactionChainManager.clearUnsubmittedTransaction();
@@ -338,8 +359,6 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
                 LOG.debug("Get Initial Device {} information is successful", getDeviceState().getNodeId());
                 getDeviceState().setDeviceSynchronized(true);
                 transactionChainManager.activateTransactionManager();
-                MdSalRegistratorUtils.registerMasterServices(getRpcContext(), DeviceContextImpl.this, role);
-                getRpcContext().registerStatCompatibilityServices();
                 initialSubmitTransaction();
                 getDeviceState().setStatisticsPollingEnabledProp(true);
                 return null;
@@ -542,14 +561,15 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
     }
 
     @Override
-    public void close() {
+    public synchronized void close() {
         LOG.debug("closing deviceContext: {}, nodeId:{}",
                 getPrimaryConnectionContext().getConnectionAdapter().getRemoteAddress(),
                 getDeviceState().getNodeId());
 
-        tearDown();
-
-        primaryConnectionContext.closeConnection(false);
+        if (deviceState.isValid()) {
+            primaryConnectionContext.closeConnection(false);
+            tearDown();
+        }
     }
 
     private synchronized void tearDown() {
@@ -583,9 +603,8 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi
         }
     }
 
-    protected void tearDownClean() {
+    private void tearDownClean() {
         LOG.info("Closing transaction chain manager without cleaning inventory operational");
-        Preconditions.checkState(!deviceState.isValid());
         transactionChainManager.close();
 
         final LinkedList<DeviceContextClosedHandler> reversedCloseHandlers = new LinkedList<>(closeHandlers);

diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MdSalRegistratorUtils.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MdSalRegistratorUtils.java
index 6b7d45f7f3a1b20fec90ebaf16ab1167acece5b1..15310237119be2b8c7bdb461f7893113e8fb13f8 100644 (file)
--- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MdSalRegistratorUtils.java
+++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/MdSalRegistratorUtils.java
@@ -53,6 +53,8 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.table.service.rev131026.Sal
 
 public class MdSalRegistratorUtils {
 
+    //TODO: Make one register and one unregister method for all services
+
     private static final TypeToken<Delegator<OpendaylightFlowStatisticsService>> COMPOSITE_SERVICE_TYPE_TOKEN =
             new TypeToken<Delegator<OpendaylightFlowStatisticsService>>() {
                 //NOBODY

diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java
index 7be1d00e1610a038baa1b1fc4743f0e357eca288..63cdfceb0542d89883356daa198c9265b2911b92 100644 (file)
--- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java
+++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImplTest.java
@@ -395,10 +395,9 @@ public class DeviceContextImplTest {
         deviceContext.addAuxiliaryConenctionContext(mockedAuxiliaryConnectionContext);
         DeviceContextClosedHandler mockedDeviceContextClosedHandler = mock(DeviceContextClosedHandler.class);
         deviceContext.addDeviceContextClosedHandler(mockedDeviceContextClosedHandler);
+        when(deviceState.isValid()).thenReturn(true);
         deviceContext.close();
-        verify(connectionContext).closeConnection(eq(false));
-//        verify(deviceState).setValid(eq(false));
-//        verify(mockedAuxiliaryConnectionContext).closeConnection(eq(false));
+        verify(connectionContext).closeConnection(false);
     }
 
     @Test