From 4ac927548dfd7f66ded8a636b518fbac3f17ec8c Mon Sep 17 00:00:00 2001 From: Colin Dixon Date: Mon, 3 Oct 2016 17:43:08 +0200 Subject: [PATCH] Improve cleanup after device disconnected event - Ensure that all contexts are removed from managers at once at end of disconnect chain right before unregistration from cluster singleton service - Prevent closing some services multiple times during device disconnect chain - Prevent some NPEs in DeviceInitializationUtils what was caused by connecting and disconnecting device with no time in between - Unify all openflow managers to implement OFPManager interface - Ensure that when we will fail to set device to SLAVE we will wait until device is removed from operational DS and then continue Resolves: bug 6672 See also: bug 6710, bug 5271 Actual work done by Tomas Slusny. Colin Dixon merely fixed a merge conflict. Change-Id: I3845c7d2a9147125b185d3abcc3fc8dd63ba5da8 Depends-On: I15f89cbd43310b0a8e4b7ac78595eeaaa82a65ee Co-author: Tomas Slusny Signed-off-by: Tomas Slusny Signed-off-by: Colin Dixon (cherry picked from commit 4a89e389589676a2b6975096ba721cd4c354e1bd) --- .../api/openflow/OFPContext.java | 14 ++- .../api/openflow/OFPManager.java | 36 ++++++ .../connection/ConnectionManager.java | 2 +- .../api/openflow/device/DeviceContext.java | 4 - .../api/openflow/device/DeviceManager.java | 12 +- .../handlers/DeviceConnectedHandler.java | 3 +- .../handlers/DeviceDisconnectedHandler.java | 5 +- .../DeviceInitializationPhaseHandler.java | 3 +- .../handlers/DeviceLifecycleSupervisor.java | 6 +- .../device/handlers/DeviceRemovedHandler.java | 22 ++++ .../DeviceTerminationPhaseHandler.java | 6 +- .../openflow/lifecycle/LifecycleService.java | 16 ++- .../api/openflow/rpc/RpcContext.java | 7 +- .../api/openflow/rpc/RpcManager.java | 5 +- .../statistics/StatisticsContext.java | 6 +- .../statistics/StatisticsManager.java | 7 +- .../impl/device/DeviceContextImpl.java | 54 +++++---- .../impl/device/DeviceManagerImpl.java | 59 ++++------ .../impl/lifecycle/LifecycleServiceImpl.java | 111 ++++++++++++------ .../impl/rpc/RpcContextImpl.java | 66 ++++++----- .../impl/rpc/RpcManagerImpl.java | 21 ++-- .../statistics/StatisticsContextImpl.java | 44 ++++--- .../statistics/StatisticsManagerImpl.java | 24 ++-- .../impl/util/DeviceInitializationUtils.java | 39 +++--- .../lifecycle/LifecycleServiceImplTest.java | 7 +- .../impl/rpc/RpcContextImplTest.java | 1 - .../impl/rpc/RpcManagerImplTest.java | 4 +- .../statistics/StatisticsManagerImplTest.java | 3 +- 28 files changed, 355 insertions(+), 232 deletions(-) create mode 100644 openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/OFPManager.java create mode 100644 openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceRemovedHandler.java diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/OFPContext.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/OFPContext.java index fad1402451..bd05be9e44 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/OFPContext.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/OFPContext.java @@ -18,10 +18,7 @@ import org.opendaylight.openflowplugin.api.openflow.device.handlers.ClusterLifec /** * General API for all OFP Context */ -public interface OFPContext extends ClusterLifecycleSupervisor, ClusterInitializationPhaseHandler { - - void setState(CONTEXT_STATE contextState); - +public interface OFPContext extends AutoCloseable, ClusterLifecycleSupervisor, ClusterInitializationPhaseHandler { /** * Context state */ @@ -35,6 +32,7 @@ public interface OFPContext extends ClusterLifecycleSupervisor, ClusterInitializ } /** + * Get actual context state * @return actual context state */ CONTEXT_STATE getState(); @@ -42,20 +40,24 @@ public interface OFPContext extends ClusterLifecycleSupervisor, ClusterInitializ /** * About to stop services in cluster not master anymore or going down * @return Future most of services need time to be closed - * @param deviceDisconnected true if clustering services stopping by device disconnect + * @param connectionInterrupted true if clustering services stopping by device disconnect */ - default ListenableFuture stopClusterServices(final boolean deviceDisconnected){ + default ListenableFuture stopClusterServices(boolean connectionInterrupted) { return Futures.immediateFailedFuture(new RejectedExecutionException("Cannot stop abstract services, check implementation of cluster services")); } /** + * Get cluster singleton service identifier * @return cluster singleton service identifier */ ServiceGroupIdentifier getServiceIdentifier(); /** + * Get device info * @return device info */ DeviceInfo getDeviceInfo(); + @Override + void close(); } diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/OFPManager.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/OFPManager.java new file mode 100644 index 0000000000..a857cb201e --- /dev/null +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/OFPManager.java @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2016 Pantheon Technologies s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ + +package org.opendaylight.openflowplugin.api.openflow; + +import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceInitializationPhaseHandler; +import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceLifecycleSupervisor; +import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceRemovedHandler; +import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceTerminationPhaseHandler; + +/** + * This interface is responsible for managing lifecycle of itself and all it's associated contexts. + * Every manager that implements this interface must handle connection initialization and termination chain + * by implementing methods from {@link org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceLifecycleSupervisor}, + * then it must handle initialization and termination chain of it's associated context by implementing methods from + * {@link org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceInitializationPhaseHandler} and + * {@link org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceTerminationPhaseHandler} and also removal + * of these contexts from it's internal map of contexts by implementing methods from + * {@link org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceRemovedHandler}. And at last, it must + * handle it's own full termination by implementing {@link AutoCloseable#close()} + */ +public interface OFPManager extends + DeviceLifecycleSupervisor, + DeviceInitializationPhaseHandler, + DeviceTerminationPhaseHandler, + DeviceRemovedHandler, + AutoCloseable { + + @Override + void close(); +} diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/connection/ConnectionManager.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/connection/ConnectionManager.java index 045fdcc760..4d421fc8d6 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/connection/ConnectionManager.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/connection/ConnectionManager.java @@ -24,7 +24,7 @@ public interface ConnectionManager extends SwitchConnectionHandler { * Method registers handler responsible handling operations related to connected device after * device is connected. * - * @param deviceConnectedHandler + * @param deviceConnectedHandler device connected handler */ void setDeviceConnectedHandler(DeviceConnectedHandler deviceConnectedHandler); void setEchoReplyTimeout(long echoReplyTimeout); diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/DeviceContext.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/DeviceContext.java index 1f8c692b49..b4e0b098fd 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/DeviceContext.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/DeviceContext.java @@ -42,7 +42,6 @@ import org.opendaylight.yangtools.yang.common.RpcResult; */ public interface DeviceContext extends OFPContext, - AutoCloseable, DeviceReplyProcessor, TxFacade, DeviceRegistry, @@ -127,9 +126,6 @@ public interface DeviceContext extends */ ItemLifeCycleRegistry getItemLifeCycleSourceRegistry(); - @Override - void close(); - void setSwitchFeaturesMandatory(boolean switchFeaturesMandatory); void putLifecycleServiceIntoTxChainManager(LifecycleService lifecycleService); diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/DeviceManager.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/DeviceManager.java index c56409a6da..58bb6fc021 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/DeviceManager.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/DeviceManager.java @@ -10,11 +10,9 @@ package org.opendaylight.openflowplugin.api.openflow.device; import com.google.common.util.concurrent.CheckedFuture; import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; +import org.opendaylight.openflowplugin.api.openflow.OFPManager; import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceConnectedHandler; import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceDisconnectedHandler; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceInitializationPhaseHandler; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceLifecycleSupervisor; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceTerminationPhaseHandler; import org.opendaylight.openflowplugin.api.openflow.translator.TranslatorLibrarian; /** @@ -22,9 +20,11 @@ import org.opendaylight.openflowplugin.api.openflow.translator.TranslatorLibrari * registering transaction chain for each DeviceContext. Each device * has its own device context managed by this manager. */ -public interface DeviceManager extends DeviceConnectedHandler, DeviceDisconnectedHandler, DeviceLifecycleSupervisor, - DeviceInitializationPhaseHandler, DeviceTerminationPhaseHandler, TranslatorLibrarian, AutoCloseable { - +public interface DeviceManager extends + OFPManager, + DeviceConnectedHandler, + DeviceDisconnectedHandler, + TranslatorLibrarian { /** * invoked after all services injected diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceConnectedHandler.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceConnectedHandler.java index 1587f77627..65ff8a6599 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceConnectedHandler.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceConnectedHandler.java @@ -12,7 +12,8 @@ import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionStatus; /** - * Created by Martin Bobak <mbobak@cisco.com> on 26.2.2015. + * Represents handler for new connected device that will propagate information about + * established connection with device. It is important for correct order in device connection chain. */ public interface DeviceConnectedHandler { diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceDisconnectedHandler.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceDisconnectedHandler.java index 3cdf031f4a..a5d520a270 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceDisconnectedHandler.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceDisconnectedHandler.java @@ -11,10 +11,11 @@ package org.opendaylight.openflowplugin.api.openflow.device.handlers; import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext; /** - * Created by Martin Bobak <mbobak@cisco.com> on 22.4.2015. + * Represents handler for just disconnected device that will propagate device's + * connection context. It is important for correct order in device disconnection chain. */ - public interface DeviceDisconnectedHandler { + /** * Method is used to propagate information about closed connection with device. * It propagates connected device's connection context. diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceInitializationPhaseHandler.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceInitializationPhaseHandler.java index 7c16d3e384..2468818ad1 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceInitializationPhaseHandler.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceInitializationPhaseHandler.java @@ -31,5 +31,6 @@ public interface DeviceInitializationPhaseHandler { * @param lifecycleService - cluster singleton service * @throws Exception - needs to be catch in ConnectionHandler implementation */ - void onDeviceContextLevelUp(final DeviceInfo deviceInfo, final LifecycleService lifecycleService) throws Exception; + void onDeviceContextLevelUp(final @CheckForNull DeviceInfo deviceInfo, + final @CheckForNull LifecycleService lifecycleService) throws Exception; } diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceLifecycleSupervisor.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceLifecycleSupervisor.java index 9e818a65c5..a537b3edb5 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceLifecycleSupervisor.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceLifecycleSupervisor.java @@ -15,7 +15,7 @@ package org.opendaylight.openflowplugin.api.openflow.device.handlers; * Interface has to implement all relevant manager to correctly handling * device initialization and termination phase. Methods are used for order * handlers in initialization/termination phase. Ordering is easily changed - * programicaly by definition. + * by definition. * */ public interface DeviceLifecycleSupervisor { @@ -24,7 +24,7 @@ public interface DeviceLifecycleSupervisor { * Method sets relevant {@link DeviceInitializationPhaseHandler} for building * handler's chain for new Device initial phase. * - * @param handler + * @param handler initialization phase handler */ void setDeviceInitializationPhaseHandler(DeviceInitializationPhaseHandler handler); @@ -32,7 +32,7 @@ public interface DeviceLifecycleSupervisor { * Method sets relevant {@link DeviceInitializationPhaseHandler} for annihilating * handler's chain for dead Device termination phase. * - * @param handler + * @param handler termination phase handler */ void setDeviceTerminationPhaseHandler(DeviceTerminationPhaseHandler handler); } diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceRemovedHandler.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceRemovedHandler.java new file mode 100644 index 0000000000..168522b42e --- /dev/null +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceRemovedHandler.java @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2016 Pantheon Technologies s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ + +package org.opendaylight.openflowplugin.api.openflow.device.handlers; + +import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo; + +/** + * Represents handler for device that was disconnected but needs to be removed from it's manager. + */ +public interface DeviceRemovedHandler { + + /** + * Method is used to propagate information about device being removed from manager. + */ + void onDeviceRemoved(DeviceInfo deviceInfo); +} diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceTerminationPhaseHandler.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceTerminationPhaseHandler.java index e1c140e92b..afbf3040eb 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceTerminationPhaseHandler.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/device/handlers/DeviceTerminationPhaseHandler.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015 Cisco Systems, Inc. and others. All rights reserved. + * Copyright (c) 2016 Cisco Systems, Inc. and others. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v1.0 which accompanies this distribution, @@ -21,8 +21,8 @@ public interface DeviceTerminationPhaseHandler { /** * Method represents a termination cycle for {@link DeviceContext}. + * @param deviceInfo {@link org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo} * - * @param deviceInfo - {@link DeviceInfo} */ - void onDeviceContextLevelDown(@CheckForNull DeviceInfo deviceInfo); + void onDeviceContextLevelDown(final @CheckForNull DeviceInfo deviceInfo); } diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/lifecycle/LifecycleService.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/lifecycle/LifecycleService.java index c3e35491c1..931aaf0cc1 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/lifecycle/LifecycleService.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/lifecycle/LifecycleService.java @@ -8,18 +8,19 @@ package org.opendaylight.openflowplugin.api.openflow.lifecycle; -import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider; +import javax.annotation.CheckForNull; import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService; +import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider; +import org.opendaylight.openflowplugin.api.openflow.OFPContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.ClusterInitializationPhaseHandler; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.ClusterLifecycleSupervisor; +import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceRemovedHandler; import org.opendaylight.openflowplugin.api.openflow.rpc.RpcContext; import org.opendaylight.openflowplugin.api.openflow.statistics.StatisticsContext; /** * Service for starting or stopping all services in plugin in cluster */ -public interface LifecycleService extends ClusterSingletonService, AutoCloseable, ClusterLifecycleSupervisor, ClusterInitializationPhaseHandler { +public interface LifecycleService extends ClusterSingletonService, OFPContext { /** * This method registers lifecycle service to the given provider @@ -27,6 +28,13 @@ public interface LifecycleService extends ClusterSingletonService, AutoCloseable */ void registerService(final ClusterSingletonServiceProvider singletonServiceProvider); + /** + * This method registers device removed handler what will be executed when device should be removed + * from managers, + * @param deviceRemovedHandler device removed handler + */ + void registerDeviceRemovedHandler(final @CheckForNull DeviceRemovedHandler deviceRemovedHandler); + /** * Setter for device context * @param deviceContext actual device context created per device diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/rpc/RpcContext.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/rpc/RpcContext.java index 36965c38ee..1dddefbef1 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/rpc/RpcContext.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/rpc/RpcContext.java @@ -17,16 +17,11 @@ import org.opendaylight.yangtools.yang.binding.RpcService; * {@link org.opendaylight.openflowplugin.api.openflow.device.RequestContext} to perform requests. *

*/ -public interface RpcContext extends RequestContextStack, AutoCloseable, OFPContext { +public interface RpcContext extends RequestContextStack, OFPContext { void registerRpcServiceImplementation(Class serviceClass, S serviceInstance); S lookupRpcService(Class serviceClass); void unregisterRpcServiceImplementation(Class serviceClass); - @Override - void close(); - void setStatisticsRpcEnabled(boolean isStatisticsRpcEnabled); - - boolean isStatisticsRpcEnabled(); } diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/rpc/RpcManager.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/rpc/RpcManager.java index eae6757584..505e8eb353 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/rpc/RpcManager.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/rpc/RpcManager.java @@ -8,17 +8,16 @@ package org.opendaylight.openflowplugin.api.openflow.rpc; +import org.opendaylight.openflowplugin.api.openflow.OFPManager; import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo; import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceInitializationPhaseHandler; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceLifecycleSupervisor; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceTerminationPhaseHandler; /** * The RPC Manager will maintain an RPC Context for each online switch. RPC context for device is created when * {@link DeviceInitializationPhaseHandler#onDeviceContextLevelUp(DeviceInfo, org.opendaylight.openflowplugin.api.openflow.lifecycle.LifecycleService)} * is called. */ -public interface RpcManager extends DeviceLifecycleSupervisor, DeviceInitializationPhaseHandler, AutoCloseable, DeviceTerminationPhaseHandler { +public interface RpcManager extends OFPManager { void setStatisticsRpcEnabled(boolean statisticsRpcEnabled); } diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/statistics/StatisticsContext.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/statistics/StatisticsContext.java index 4a954c00fb..9c50b65da7 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/statistics/StatisticsContext.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/statistics/StatisticsContext.java @@ -15,13 +15,12 @@ import org.opendaylight.openflowplugin.api.openflow.OFPContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceState; import org.opendaylight.openflowplugin.api.openflow.device.RequestContextStack; -import org.opendaylight.openflowplugin.api.openflow.lifecycle.LifecycleService; import org.opendaylight.openflowplugin.api.openflow.rpc.listener.ItemLifecycleListener; /** * Context for statistics */ -public interface StatisticsContext extends RequestContextStack, AutoCloseable, OFPContext { +public interface StatisticsContext extends RequestContextStack, OFPContext { /** * Gather data from device @@ -60,9 +59,6 @@ public interface StatisticsContext extends RequestContextStack, AutoCloseable, O */ ItemLifecycleListener getItemLifeCycleListener(); - @Override - void close(); - /** * On / Off scheduling * @param schedulingEnabled true if scheduling should be enabled diff --git a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/statistics/StatisticsManager.java b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/statistics/StatisticsManager.java index 1c3c6fcd7c..0cb22a5160 100644 --- a/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/statistics/StatisticsManager.java +++ b/openflowplugin-api/src/main/java/org/opendaylight/openflowplugin/api/openflow/statistics/StatisticsManager.java @@ -8,16 +8,13 @@ package org.opendaylight.openflowplugin.api.openflow.statistics; +import org.opendaylight.openflowplugin.api.openflow.OFPManager; import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceInitializationPhaseHandler; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceLifecycleSupervisor; -import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceTerminationPhaseHandler; /** * Manager to start or stop scheduling statistics */ -public interface StatisticsManager extends DeviceLifecycleSupervisor, DeviceInitializationPhaseHandler, - DeviceTerminationPhaseHandler, AutoCloseable { +public interface StatisticsManager extends OFPManager { /** * Start scheduling statistic gathering for given device info 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 dd13831464..fcc711bb16 100644 --- 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 @@ -117,7 +117,7 @@ import org.opendaylight.yangtools.yang.common.RpcResult; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class DeviceContextImpl implements DeviceContext, ExtensionConverterProviderKeeper{ +public class DeviceContextImpl implements DeviceContext, ExtensionConverterProviderKeeper { private static final Logger LOG = LoggerFactory.getLogger(DeviceContextImpl.class); @@ -504,15 +504,6 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi return translatorLibrary; } - @Override - public synchronized void close() { - LOG.debug("closing deviceContext: {}, nodeId:{}", - getPrimaryConnectionContext().getConnectionAdapter().getRemoteAddress(), - getDeviceInfo().getLOGValue()); - // NOOP - throw new UnsupportedOperationException("Autocloseble.close will be removed soon"); - } - @Override public void setCurrentBarrierTimeout(final Timeout timeout) { barrierTaskTimeout = timeout; @@ -536,7 +527,7 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi @Override public void onPublished() { Verify.verify(CONTEXT_STATE.INITIALIZATION.equals(getState())); - setState(CONTEXT_STATE.WORKING); + this.state = CONTEXT_STATE.WORKING; primaryConnectionContext.getConnectionAdapter().setPacketInFiltering(false); for (final ConnectionContext switchAuxConnectionContext : auxiliaryConnectionContexts.values()) { switchAuxConnectionContext.getConnectionAdapter().setPacketInFiltering(false); @@ -623,18 +614,13 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi } @Override - public void setState(CONTEXT_STATE state) { - this.state = state; - } - - @Override - public ListenableFuture stopClusterServices(boolean deviceDisconnected) { - - ListenableFuture deactivateTxManagerFuture = - initialized ? transactionChainManager.deactivateTransactionManager() : Futures.immediateFuture(null); + public ListenableFuture stopClusterServices(boolean connectionInterrupted) { + final ListenableFuture deactivateTxManagerFuture = initialized + ? transactionChainManager.deactivateTransactionManager() + : Futures.immediateFuture(null); - if (!deviceDisconnected) { - ListenableFuture makeSlaveFuture = Futures.transform(makeDeviceSlave(), new Function, Void>() { + if (!connectionInterrupted) { + final ListenableFuture makeSlaveFuture = Futures.transform(makeDeviceSlave(), new Function, Void>() { @Nullable @Override public Void apply(@Nullable RpcResult setRoleOutputRpcResult) { @@ -654,14 +640,15 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi public void onFailure(final Throwable throwable) { LOG.warn("Was not able to set role SLAVE to device on node {} ", deviceInfo.getLOGValue()); LOG.trace("Error occurred on device role setting, probably connection loss: ", throwable); - myManager.removeDeviceFromOperationalDS(deviceInfo); } }); return Futures.transform(deactivateTxManagerFuture, new AsyncFunction() { @Override public ListenableFuture apply(Void aVoid) throws Exception { - return makeSlaveFuture; + // Add fallback to remove device from operational DS if setting slave fails + return Futures.withFallback(makeSlaveFuture, t -> + myManager.removeDeviceFromOperationalDS(deviceInfo)); } }); } else { @@ -684,6 +671,17 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi return this.deviceInfo; } + @Override + public void close() { + if (CONTEXT_STATE.TERMINATION.equals(getState())){ + if (LOG.isDebugEnabled()) { + LOG.debug("DeviceContext for node {} is already in TERMINATION state.", getDeviceInfo().getLOGValue()); + } + } else { + this.state = CONTEXT_STATE.TERMINATION; + } + } + @Override public void putLifecycleServiceIntoTxChainManager(final LifecycleService lifecycleService){ if (initialized) { @@ -694,7 +692,7 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi @Override public void replaceConnectionContext(final ConnectionContext connectionContext){ // Act like we are initializing the context - setState(CONTEXT_STATE.INITIALIZATION); + this.state = CONTEXT_STATE.INITIALIZATION; this.primaryConnectionContext = connectionContext; this.onPublished(); } @@ -769,22 +767,28 @@ public class DeviceContextImpl implements DeviceContext, ExtensionConverterProvi if (LOG.isDebugEnabled()) { LOG.debug("Sending new role {} to device {}", newRole, deviceInfo.getNodeId()); } + final Future> setRoleOutputFuture; + if (deviceInfo.getVersion() >= OFConstants.OFP_VERSION_1_3) { final SetRoleInput setRoleInput = (new SetRoleInputBuilder()).setControllerRole(newRole) .setNode(new NodeRef(deviceInfo.getNodeInstanceIdentifier())).build(); + setRoleOutputFuture = this.salRoleService.setRole(setRoleInput); + final TimerTask timerTask = timeout -> { if (!setRoleOutputFuture.isDone()) { LOG.warn("New role {} was not propagated to device {} during {} sec", newRole, deviceInfo.getLOGValue(), SET_ROLE_TIMEOUT); setRoleOutputFuture.cancel(true); } }; + hashedWheelTimer.newTimeout(timerTask, SET_ROLE_TIMEOUT, TimeUnit.SECONDS); } else { LOG.info("Device: {} with version: {} does not support role", deviceInfo.getLOGValue(), deviceInfo.getVersion()); return Futures.immediateFuture(null); } + return JdkFutureAdapters.listenInPoolThread(setRoleOutputFuture); } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceManagerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceManagerImpl.java index 964ad0ba6b..f81ae826fe 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceManagerImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceManagerImpl.java @@ -149,14 +149,15 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi LOG.debug("Final phase of DeviceContextLevelUp for Node: {} ", deviceInfo.getNodeId()); DeviceContext deviceContext = Preconditions.checkNotNull(deviceContexts.get(deviceInfo)); deviceContext.onPublished(); + lifecycleService.registerDeviceRemovedHandler(this); lifecycleService.registerService(this.singletonServiceProvider); } @Override public ConnectionStatus deviceConnected(@CheckForNull final ConnectionContext connectionContext) throws Exception { Preconditions.checkArgument(connectionContext != null); + final DeviceInfo deviceInfo = connectionContext.getDeviceInfo(); - DeviceInfo deviceInfo = connectionContext.getDeviceInfo(); /* * This part prevent destroy another device context. Throwing here an exception result to propagate close connection * in {@link org.opendaylight.openflowplugin.impl.connection.org.opendaylight.openflowplugin.impl.connection.HandshakeContextImpl} @@ -178,11 +179,12 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi connectionContext.getConnectionAdapter().getRemoteAddress(), deviceInfo.getNodeId()); // Add Disconnect handler - connectionContext.setDeviceDisconnectedHandler(DeviceManagerImpl.this); + connectionContext.setDeviceDisconnectedHandler(this); + // Cache this for clarity final ConnectionAdapter connectionAdapter = connectionContext.getConnectionAdapter(); - //FIXME: as soon as auxiliary connection are fully supported then this is needed only before device context published + // FIXME: as soon as auxiliary connection are fully supported then this is needed only before device context published connectionAdapter.setPacketInFiltering(true); final OutboundQueueProvider outboundQueueProvider = new OutboundQueueProviderImpl(deviceInfo.getVersion()); @@ -256,27 +258,8 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi @Override public void onDeviceContextLevelDown(final DeviceInfo deviceInfo) { - - LifecycleService lifecycleService = lifecycleServices.remove(deviceInfo); - if (LOG.isDebugEnabled()) { - LOG.debug("Lifecycle service removed for node {}", deviceInfo.getLOGValue()); - } - updatePacketInRateLimiters(); - if (Objects.nonNull(lifecycleService)) { - try { - lifecycleService.close(); - LOG.debug("Lifecycle service successfully closed for node {}", deviceInfo.getLOGValue()); - } catch (Exception e) { - LOG.warn("Closing lifecycle service for node {} was unsuccessful ", deviceInfo.getLOGValue(), e); - } - } - - deviceContexts.remove(deviceInfo); - if (LOG.isDebugEnabled()) { - LOG.debug("Device context removed for node {}", deviceInfo.getLOGValue()); - } - + Optional.ofNullable(lifecycleServices.get(deviceInfo)).ifPresent(OFPContext::close); } @Override @@ -305,7 +288,7 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi final DeviceInfo deviceInfo = connectionContext.getDeviceInfo(); final DeviceContext deviceCtx = this.deviceContexts.get(deviceInfo); - if (null == deviceCtx) { + if (Objects.isNull(deviceCtx)) { LOG.info("DeviceContext for Node {} was not found. Connection is terminated without OFP context suite.", deviceInfo.getLOGValue()); return; } @@ -315,18 +298,18 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi return; } - deviceCtx.setState(OFPContext.CONTEXT_STATE.TERMINATION); + deviceCtx.close(); if (!connectionContext.equals(deviceCtx.getPrimaryConnectionContext())) { LOG.debug("Node {} disconnected, but not primary connection.", connectionContext.getDeviceInfo().getLOGValue()); - /* Connection is not PrimaryConnection so try to remove from Auxiliary Connections */ + // Connection is not PrimaryConnection so try to remove from Auxiliary Connections deviceCtx.removeAuxiliaryConnectionContext(connectionContext); } - //TODO: Auxiliary connections supported ? - /* Device is disconnected and so we need to close TxManager */ + + // TODO: Auxiliary connections supported ? + // Device is disconnected and so we need to close TxManager final ListenableFuture future = deviceCtx.shuttingDownDataStoreTransactions(); Futures.addCallback(future, new FutureCallback() { - @Override public void onSuccess(final Void result) { LOG.debug("TxChainManager for device {} is closed successful.", deviceInfo.getLOGValue()); @@ -340,13 +323,15 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi deviceTerminPhaseHandler.onDeviceContextLevelDown(deviceInfo); } }); - /* Add timer for Close TxManager because it could fain ind cluster without notification */ + + // Add timer for Close TxManager because it could fail in cluster without notification final TimerTask timerTask = timeout -> { if (!future.isDone()) { LOG.warn("Shutting down TxChain for node {} not completed during 10 sec. Continue anyway.", deviceInfo.getLOGValue()); future.cancel(false); } }; + hashedWheelTimer.newTimeout(timerTask, 10, TimeUnit.SECONDS); } @@ -440,15 +425,11 @@ public class DeviceManagerImpl implements DeviceManager, ExtensionConverterProvi } } - @VisibleForTesting - void setDeviceContext(final DeviceInfo deviceInfo, final DeviceContext deviceContext) { - this.deviceContexts.putIfAbsent(deviceInfo, deviceContext); - } + public void onDeviceRemoved(DeviceInfo deviceInfo) { + deviceContexts.remove(deviceInfo); + LOG.debug("Device context removed for node {}", deviceInfo.getLOGValue()); - @VisibleForTesting - int getDeviceContextCount() { - return this.deviceContexts.size(); + lifecycleServices.remove(deviceInfo); + LOG.debug("Lifecycle service removed for node {}", deviceInfo.getLOGValue()); } - - } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/lifecycle/LifecycleServiceImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/lifecycle/LifecycleServiceImpl.java index 885438df44..7522fe54b0 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/lifecycle/LifecycleServiceImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/lifecycle/LifecycleServiceImpl.java @@ -9,6 +9,7 @@ package org.opendaylight.openflowplugin.impl.lifecycle; import com.google.common.base.Function; import com.google.common.base.Optional; +import com.google.common.base.Verify; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; @@ -21,8 +22,11 @@ import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvid import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceRegistration; import org.opendaylight.mdsal.singleton.common.api.ServiceGroupIdentifier; import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext; +import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext.CONNECTION_STATE; import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext; +import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo; import org.opendaylight.openflowplugin.api.openflow.device.handlers.ClusterInitializationPhaseHandler; +import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceRemovedHandler; import org.opendaylight.openflowplugin.api.openflow.lifecycle.LifecycleService; import org.opendaylight.openflowplugin.api.openflow.rpc.RpcContext; import org.opendaylight.openflowplugin.api.openflow.statistics.StatisticsContext; @@ -33,57 +37,43 @@ import org.slf4j.LoggerFactory; public class LifecycleServiceImpl implements LifecycleService { private static final Logger LOG = LoggerFactory.getLogger(LifecycleServiceImpl.class); - - private boolean inClosing = false; private DeviceContext deviceContext; private RpcContext rpcContext; private StatisticsContext statContext; private ClusterSingletonServiceRegistration registration; private ClusterInitializationPhaseHandler clusterInitializationPhaseHandler; + private final List deviceRemovedHandlers = new ArrayList<>(); + private volatile CONTEXT_STATE state = CONTEXT_STATE.INITIALIZATION; @Override public void instantiateServiceInstance() { - LOG.info("Starting clustering MASTER services for node {}", this.deviceContext.getDeviceInfo().getLOGValue()); + LOG.info("Starting clustering MASTER services for node {}", getDeviceInfo().getLOGValue()); - if (!this.clusterInitializationPhaseHandler.onContextInstantiateService(null)) { - this.closeConnection(); + if (!clusterInitializationPhaseHandler.onContextInstantiateService(null)) { + closeConnection(); } - } @Override public ListenableFuture closeServiceInstance() { - LOG.info("Closing clustering MASTER services for node {}", this.deviceContext.getDeviceInfo().getLOGValue()); - final boolean connectionInterrupted = this.deviceContext .getPrimaryConnectionContext() .getConnectionState() .equals(ConnectionContext.CONNECTION_STATE.RIP); - // If connection was interrupted and we are not trying to close service, then we received something - // we do not wanted to receive, so do not continue - if (connectionInterrupted && !inClosing) { - LOG.warn("Failed to close clustering MASTER services for node {} because they are already closed", - LifecycleServiceImpl.this.deviceContext.getDeviceInfo().getLOGValue()); - - return Futures.immediateCancelledFuture(); - } - // Chain all jobs that will stop our services final List> futureList = new ArrayList<>(); futureList.add(statContext.stopClusterServices(connectionInterrupted)); futureList.add(rpcContext.stopClusterServices(connectionInterrupted)); futureList.add(deviceContext.stopClusterServices(connectionInterrupted)); - // When we stopped all jobs then we are not in closing state anymore (at least from plugin perspective) return Futures.transform(Futures.successfulAsList(futureList), new Function, Void>() { @Nullable @Override public Void apply(@Nullable List input) { - LOG.debug("Closed clustering MASTER services for node {}", - LifecycleServiceImpl.this.deviceContext.getDeviceInfo().getLOGValue()); + LOG.debug("Closed clustering MASTER services for node {}", getDeviceInfo().getLOGValue()); return null; } }); @@ -91,33 +81,76 @@ public class LifecycleServiceImpl implements LifecycleService { @Override public ServiceGroupIdentifier getIdentifier() { + return getServiceIdentifier(); + } + + @Override + public CONTEXT_STATE getState() { + return this.state; + } + + @Override + public ServiceGroupIdentifier getServiceIdentifier() { return deviceContext.getServiceIdentifier(); } + @Override + public DeviceInfo getDeviceInfo() { + return deviceContext.getDeviceInfo(); + } @Override - public void close() throws Exception { - // If we are still registered and we are not already closing, then close the registration - if (Objects.nonNull(registration) && !inClosing) { - inClosing = true; - registration.close(); - registration = null; + public void close() { + if (CONTEXT_STATE.TERMINATION.equals(getState())){ + if (LOG.isDebugEnabled()) { + LOG.debug("LifecycleService is already in TERMINATION state."); + } + } else { + this.state = CONTEXT_STATE.TERMINATION; + + // We are closing, so cleanup all managers now + deviceRemovedHandlers.forEach(h -> h.onDeviceRemoved(getDeviceInfo())); + + // If we are still registered and we are not already closing, then close the registration + if (Objects.nonNull(registration)) { + try { + LOG.debug("Closing clustering MASTER services for node {}", getDeviceInfo().getLOGValue()); + registration.close(); + } catch (Exception e) { + LOG.debug("Failed to close clustering MASTER services for node {} with exception: ", + getDeviceInfo().getLOGValue(), e); + } + } } } @Override public void registerService(final ClusterSingletonServiceProvider singletonServiceProvider) { - LOG.info("Registering clustering MASTER services for node {}", this.deviceContext.getDeviceInfo().getLOGValue()); + LOG.debug("Registered clustering MASTER services for node {}", getDeviceInfo().getLOGValue()); - //lifecycle service -> device context -> statistics context -> rpc context -> role context -> lifecycle service + // lifecycle service -> device context -> statistics context -> rpc context -> role context -> lifecycle service this.clusterInitializationPhaseHandler = deviceContext; this.deviceContext.setLifecycleInitializationPhaseHandler(this.statContext); this.statContext.setLifecycleInitializationPhaseHandler(this.rpcContext); this.rpcContext.setLifecycleInitializationPhaseHandler(this); //Set initial submit handler this.statContext.setInitialSubmitHandler(this.deviceContext); - //Register cluster singleton service - this.registration = singletonServiceProvider.registerClusterSingletonService(this); + + // Register cluster singleton service + try { + this.registration = Verify.verifyNotNull(singletonServiceProvider.registerClusterSingletonService(this)); + LOG.info("Registered clustering MASTER services for node {}", getDeviceInfo().getLOGValue()); + } catch (Exception e) { + LOG.warn("Failed to register cluster singleton service for node {}, with exception: {}", getDeviceInfo(), e); + closeConnection(); + } + } + + @Override + public void registerDeviceRemovedHandler(final DeviceRemovedHandler deviceRemovedHandler) { + if (!deviceRemovedHandlers.contains(deviceRemovedHandler)) { + deviceRemovedHandlers.add(deviceRemovedHandler); + } } @Override @@ -142,6 +175,10 @@ public class LifecycleServiceImpl implements LifecycleService { @Override public void closeConnection() { + if (LOG.isDebugEnabled()) { + LOG.debug("Closing connection for node {}.", getDeviceInfo().getLOGValue()); + } + this.deviceContext.shutdownConnection(); } @@ -157,11 +194,11 @@ public class LifecycleServiceImpl implements LifecycleService { @Override public boolean onContextInstantiateService(final ConnectionContext connectionContext) { - - if (ConnectionContext.CONNECTION_STATE.RIP.equals(connectionContext.getConnectionState())) { + if (CONNECTION_STATE.RIP.equals(connectionContext.getConnectionState())) { if (LOG.isDebugEnabled()) { - LOG.debug("Connection to the device {} was interrupted.", this.deviceContext.getDeviceInfo().getLOGValue()); + LOG.debug("Connection to the device {} was interrupted.", getDeviceInfo().getLOGValue()); } + return false; } @@ -172,7 +209,7 @@ public class LifecycleServiceImpl implements LifecycleService { private class DeviceFlowRegistryCallback implements FutureCallback>> { private final ListenableFuture>> deviceFlowRegistryFill; - public DeviceFlowRegistryCallback(ListenableFuture>> deviceFlowRegistryFill) { + DeviceFlowRegistryCallback(ListenableFuture>> deviceFlowRegistryFill) { this.deviceFlowRegistryFill = deviceFlowRegistryFill; } @@ -195,7 +232,7 @@ public class LifecycleServiceImpl implements LifecycleService { .filter(Objects::nonNull) .count(); - LOG.debug("Finished filling flow registry with {} flows for node: {}", flowCount, deviceContext.getDeviceInfo().getLOGValue()); + LOG.debug("Finished filling flow registry with {} flows for node: {}", flowCount, getDeviceInfo().getLOGValue()); } } @@ -203,10 +240,10 @@ public class LifecycleServiceImpl implements LifecycleService { public void onFailure(Throwable t) { if (deviceFlowRegistryFill.isCancelled()) { if (LOG.isDebugEnabled()) { - LOG.debug("Cancelled filling flow registry with flows for node: {}", deviceContext.getDeviceInfo().getLOGValue()); + LOG.debug("Cancelled filling flow registry with flows for node: {}", getDeviceInfo().getLOGValue()); } } else { - LOG.warn("Failed filling flow registry with flows for node: {} with exception: {}", deviceContext.getDeviceInfo().getLOGValue(), t); + LOG.warn("Failed filling flow registry with flows for node: {} with exception: {}", getDeviceInfo().getLOGValue(), t); } } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/rpc/RpcContextImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/rpc/RpcContextImpl.java index fec32b5489..6953784a3f 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/rpc/RpcContextImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/rpc/RpcContextImpl.java @@ -8,6 +8,7 @@ package org.opendaylight.openflowplugin.impl.rpc; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.Iterators; import com.google.common.util.concurrent.Futures; @@ -17,6 +18,7 @@ import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Semaphore; +import javax.annotation.Nullable; import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService; import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.RoutedRpcRegistration; import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry; @@ -49,7 +51,7 @@ class RpcContextImpl implements RpcContext { // TODO: add private Sal salBroker private final ConcurrentMap, RoutedRpcRegistration> rpcRegistrations = new ConcurrentHashMap<>(); private final KeyedInstanceIdentifier nodeInstanceIdentifier; - private CONTEXT_STATE state; + private volatile CONTEXT_STATE state = CONTEXT_STATE.INITIALIZATION; private final DeviceInfo deviceInfo; private final DeviceContext deviceContext; private final ExtensionConverterProvider extensionConverterProvider; @@ -69,11 +71,9 @@ class RpcContextImpl implements RpcContext { this.messageSpy = Preconditions.checkNotNull(messageSpy); this.rpcProviderRegistry = Preconditions.checkNotNull(rpcProviderRegistry); this.nodeInstanceIdentifier = nodeInstanceIdentifier; - - tracker = new Semaphore(maxRequests, true); + this.tracker = new Semaphore(maxRequests, true); this.extensionConverterProvider = extensionConverterProvider; this.notificationPublishService = notificationPublishService; - setState(CONTEXT_STATE.WORKING); this.deviceInfo = deviceInfo; this.deviceContext = deviceContext; this.convertorExecutor = convertorExecutor; @@ -111,20 +111,16 @@ class RpcContextImpl implements RpcContext { public void close() { if (CONTEXT_STATE.TERMINATION.equals(getState())){ if (LOG.isDebugEnabled()) { - LOG.debug("RpcContext is already in TERMINATION state."); + LOG.debug("RpcContext for node {} is already in TERMINATION state.", getDeviceInfo().getLOGValue()); } } else { - setState(CONTEXT_STATE.TERMINATION); - for (final Iterator, RoutedRpcRegistration>> iterator = Iterators - .consumingIterator(rpcRegistrations.entrySet().iterator()); iterator.hasNext(); ) { - final RoutedRpcRegistration rpcRegistration = iterator.next().getValue(); - rpcRegistration.unregisterPath(NodeContext.class, nodeInstanceIdentifier); - rpcRegistration.close(); - if (LOG.isDebugEnabled()) { - LOG.debug("Closing RPC Registration of service {} for device {}.", rpcRegistration.getServiceType().getSimpleName(), - nodeInstanceIdentifier.getKey().getId().getValue()); - } + try { + stopClusterServices(true).get(); + } catch (Exception e) { + LOG.debug("Failed to close RpcContext for node {} with exception: ", getDeviceInfo().getLOGValue(), e); } + + this.state = CONTEXT_STATE.TERMINATION; } } @@ -176,21 +172,11 @@ class RpcContextImpl implements RpcContext { this.isStatisticsRpcEnabled = isStatisticsRpcEnabled; } - @Override - public boolean isStatisticsRpcEnabled() { - return isStatisticsRpcEnabled; - } - @Override public CONTEXT_STATE getState() { return this.state; } - @Override - public void setState(CONTEXT_STATE state) { - this.state = state; - } - @Override public ServiceGroupIdentifier getServiceIdentifier() { return this.deviceInfo.getServiceIdentifier(); @@ -202,9 +188,30 @@ class RpcContextImpl implements RpcContext { } @Override - public ListenableFuture stopClusterServices(boolean deviceDisconnected) { - MdSalRegistrationUtils.unregisterServices(this); - return Futures.immediateFuture(null); + public ListenableFuture stopClusterServices(boolean connectionInterrupted) { + if (CONTEXT_STATE.TERMINATION.equals(getState())) { + return Futures.immediateCancelledFuture(); + } + + return Futures.transform(Futures.immediateFuture(null), new Function() { + @Nullable + @Override + public Void apply(@Nullable Object input) { + for (final Iterator, RoutedRpcRegistration>> iterator = Iterators + .consumingIterator(rpcRegistrations.entrySet().iterator()); iterator.hasNext(); ) { + final RoutedRpcRegistration rpcRegistration = iterator.next().getValue(); + rpcRegistration.unregisterPath(NodeContext.class, nodeInstanceIdentifier); + rpcRegistration.close(); + + if (LOG.isDebugEnabled()) { + LOG.debug("Closing RPC Registration of service {} for device {}.", rpcRegistration.getServiceType().getSimpleName(), + nodeInstanceIdentifier.getKey().getId().getValue()); + } + } + + return null; + } + }); } @Override @@ -214,13 +221,13 @@ class RpcContextImpl implements RpcContext { @Override public boolean onContextInstantiateService(final ConnectionContext connectionContext) { - if (connectionContext.getConnectionState().equals(ConnectionContext.CONNECTION_STATE.RIP)) { LOG.warn("Connection on device {} was interrupted, will stop starting master services.", deviceInfo.getLOGValue()); return false; } MdSalRegistrationUtils.registerServices(this, deviceContext, extensionConverterProvider, convertorExecutor); + if (isStatisticsRpcEnabled) { MdSalRegistrationUtils.registerStatCompatibilityServices( this, @@ -228,6 +235,7 @@ class RpcContextImpl implements RpcContext { notificationPublishService, convertorExecutor); } + return this.clusterInitializationPhaseHandler.onContextInstantiateService(connectionContext); } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/rpc/RpcManagerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/rpc/RpcManagerImpl.java index 11f30f644f..f8cf02e331 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/rpc/RpcManagerImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/rpc/RpcManagerImpl.java @@ -12,10 +12,12 @@ import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.collect.Iterators; import java.util.Iterator; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService; import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry; +import org.opendaylight.openflowplugin.api.openflow.OFPContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo; import org.opendaylight.openflowplugin.api.openflow.device.handlers.DeviceInitializationPhaseHandler; @@ -33,7 +35,7 @@ public class RpcManagerImpl implements RpcManager { private static final Logger LOG = LoggerFactory.getLogger(RpcManagerImpl.class); private final RpcProviderRegistry rpcProviderRegistry; private DeviceInitializationPhaseHandler deviceInitPhaseHandler; - private DeviceTerminationPhaseHandler deviceTerminPhaseHandler; + private DeviceTerminationPhaseHandler deviceTerminationPhaseHandler; private final int maxRequestsQuota; private final ConcurrentMap contexts = new ConcurrentHashMap<>(); private boolean isStatisticsRpcEnabled; @@ -78,6 +80,7 @@ public class RpcManagerImpl implements RpcManager { Verify.verify(contexts.putIfAbsent(deviceInfo, rpcContext) == null, "RpcCtx still not closed for node {}", deviceInfo.getNodeId()); lifecycleService.setRpcContext(rpcContext); + lifecycleService.registerDeviceRemovedHandler(this); rpcContext.setStatisticsRpcEnabled(isStatisticsRpcEnabled); // finish device initialization cycle back to DeviceManager @@ -94,17 +97,13 @@ public class RpcManagerImpl implements RpcManager { @Override public void onDeviceContextLevelDown(final DeviceInfo deviceInfo) { - final RpcContext removedContext = contexts.remove(deviceInfo); - if (removedContext != null) { - LOG.debug("Unregister RPCs services for node {}", deviceInfo.getLOGValue()); - removedContext.close(); - } - deviceTerminPhaseHandler.onDeviceContextLevelDown(deviceInfo); + Optional.ofNullable(contexts.get(deviceInfo)).ifPresent(OFPContext::close); + deviceTerminationPhaseHandler.onDeviceContextLevelDown(deviceInfo); } @Override public void setDeviceTerminationPhaseHandler(final DeviceTerminationPhaseHandler handler) { - this.deviceTerminPhaseHandler = handler; + this.deviceTerminationPhaseHandler = handler; } /** @@ -122,4 +121,10 @@ public class RpcManagerImpl implements RpcManager { public void setStatisticsRpcEnabled(boolean statisticsRpcEnabled) { isStatisticsRpcEnabled = statisticsRpcEnabled; } + + @Override + public void onDeviceRemoved(DeviceInfo deviceInfo) { + contexts.remove(deviceInfo); + LOG.debug("Rpc context removed for node {}", deviceInfo.getLOGValue()); + } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsContextImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsContextImpl.java index 8eade85cb8..eab57c2795 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsContextImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsContextImpl.java @@ -9,6 +9,7 @@ package org.opendaylight.openflowplugin.impl.statistics; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; @@ -94,7 +95,7 @@ class StatisticsContextImpl implements StatisticsContext { statisticsGatheringOnTheFlyService = new StatisticsGatheringOnTheFlyService(this, deviceContext, convertorExecutor); itemLifeCycleListener = new ItemLifecycleListenerImpl(deviceContext); statListForCollectingInitialization(); - setState(CONTEXT_STATE.INITIALIZATION); + this.state = CONTEXT_STATE.INITIALIZATION; this.deviceInfo = deviceInfo; this.myManager = myManager; this.lastDataGathering = null; @@ -228,16 +229,22 @@ class StatisticsContextImpl implements StatisticsContext { public void close() { if (CONTEXT_STATE.TERMINATION.equals(getState())) { if (LOG.isDebugEnabled()) { - LOG.debug("Statistics context is already in state TERMINATION."); + LOG.debug("StatisticsContext for node {} is already in TERMINATION state.", getDeviceInfo().getLOGValue()); } } else { - stopGatheringData(); - setState(CONTEXT_STATE.TERMINATION); - schedulingEnabled = false; + try { + stopClusterServices(true).get(); + } catch (Exception e) { + LOG.debug("Failed to close StatisticsContext for node {} with exception: ", getDeviceInfo().getLOGValue(), e); + } + + this.state = CONTEXT_STATE.TERMINATION; + for (final Iterator> iterator = Iterators.consumingIterator(requestContexts.iterator()); iterator.hasNext(); ) { RequestContextUtil.closeRequestContextWithRpcError(iterator.next(), CONNECTION_CLOSED); } + if (null != pollTimeout && !pollTimeout.isExpired()) { pollTimeout.cancel(); } @@ -421,11 +428,6 @@ class StatisticsContextImpl implements StatisticsContext { return this.state; } - @Override - public void setState(CONTEXT_STATE state) { - this.state = state; - } - @Override public ServiceGroupIdentifier getServiceIdentifier() { return this.deviceInfo.getServiceIdentifier(); @@ -437,10 +439,20 @@ class StatisticsContextImpl implements StatisticsContext { } @Override - public ListenableFuture stopClusterServices(boolean deviceDisconnected) { - stopGatheringData(); - myManager.stopScheduling(deviceInfo); - return Futures.immediateFuture(null); + public ListenableFuture stopClusterServices(boolean connectionInterrupted) { + if (CONTEXT_STATE.TERMINATION.equals(getState())) { + return Futures.immediateCancelledFuture(); + } + + return Futures.transform(Futures.immediateFuture(null), new Function() { + @Nullable + @Override + public Void apply(@Nullable Object input) { + schedulingEnabled = false; + stopGatheringData(); + return null; + } + }); } @Override @@ -459,7 +471,8 @@ class StatisticsContextImpl implements StatisticsContext { if (LOG.isDebugEnabled()) { LOG.debug("Stop the running statistics gathering for node {}", this.deviceInfo.getLOGValue()); } - this.lastDataGathering.cancel(true); + + lastDataGathering.cancel(true); } } @@ -470,7 +483,6 @@ class StatisticsContextImpl implements StatisticsContext { @Override public boolean onContextInstantiateService(final ConnectionContext connectionContext) { - if (connectionContext.getConnectionState().equals(ConnectionContext.CONNECTION_STATE.RIP)) { LOG.warn("Connection on device {} was interrupted, will stop starting master services.", deviceInfo.getLOGValue()); return false; diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImpl.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImpl.java index c42a210752..26f635d0c0 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImpl.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImpl.java @@ -29,6 +29,7 @@ import java.util.concurrent.TimeUnit; import javax.annotation.Nonnull; import org.opendaylight.controller.sal.binding.api.BindingAwareBroker; import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry; +import org.opendaylight.openflowplugin.api.openflow.OFPContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo; import org.opendaylight.openflowplugin.api.openflow.device.DeviceState; @@ -58,7 +59,7 @@ public class StatisticsManagerImpl implements StatisticsManager, StatisticsManag private final ConvertorExecutor converterExecutor; private DeviceInitializationPhaseHandler deviceInitPhaseHandler; - private DeviceTerminationPhaseHandler deviceTerminPhaseHandler; + private DeviceTerminationPhaseHandler deviceTerminationPhaseHandler; private final ConcurrentMap contexts = new ConcurrentHashMap<>(); @@ -102,11 +103,14 @@ public class StatisticsManagerImpl implements StatisticsManager, StatisticsManag lifecycleService, converterExecutor, this); + Verify.verify( contexts.putIfAbsent(deviceInfo, statisticsContext) == null, "StatisticsCtx still not closed for Node {}", deviceInfo.getLOGValue() ); + lifecycleService.setStatContext(statisticsContext); + lifecycleService.registerDeviceRemovedHandler(this); deviceInitPhaseHandler.onDeviceContextLevelUp(deviceInfo, lifecycleService); } @@ -126,6 +130,7 @@ public class StatisticsManagerImpl implements StatisticsManager, StatisticsManag if (LOG.isDebugEnabled()) { LOG.debug("POLLING ALL STATISTICS for device: {}", deviceInfo.getNodeId()); } + timeCounter.markStart(); final ListenableFuture deviceStatisticsCollectionFuture = statisticsContext.gatherDynamicData(); Futures.addCallback(deviceStatisticsCollectionFuture, new FutureCallback() { @@ -209,12 +214,8 @@ public class StatisticsManagerImpl implements StatisticsManager, StatisticsManag @Override public void onDeviceContextLevelDown(final DeviceInfo deviceInfo) { - final StatisticsContext statisticsContext = contexts.remove(deviceInfo); - if (null != statisticsContext) { - LOG.debug("Removing device context from stack. No more statistics gathering for device: {}", deviceInfo.getLOGValue()); - statisticsContext.close(); - } - deviceTerminPhaseHandler.onDeviceContextLevelDown(deviceInfo); + Optional.ofNullable(contexts.get(deviceInfo)).ifPresent(OFPContext::close); + deviceTerminationPhaseHandler.onDeviceContextLevelDown(deviceInfo); } @Override @@ -304,12 +305,14 @@ public class StatisticsManagerImpl implements StatisticsManager, StatisticsManag if (LOG.isDebugEnabled()) { LOG.debug("Stopping statistics scheduling for device: {}", deviceInfo.getNodeId()); } + final StatisticsContext statisticsContext = contexts.get(deviceInfo); if (statisticsContext == null) { LOG.warn("Statistics context not found for device: {}", deviceInfo.getNodeId()); return; } + statisticsContext.setSchedulingEnabled(false); } @@ -319,6 +322,7 @@ public class StatisticsManagerImpl implements StatisticsManager, StatisticsManag controlServiceRegistration.close(); controlServiceRegistration = null; } + for (final Iterator iterator = Iterators.consumingIterator(contexts.values().iterator()); iterator.hasNext();) { iterator.next().close(); @@ -327,7 +331,7 @@ public class StatisticsManagerImpl implements StatisticsManager, StatisticsManag @Override public void setDeviceTerminationPhaseHandler(final DeviceTerminationPhaseHandler handler) { - this.deviceTerminPhaseHandler = handler; + this.deviceTerminationPhaseHandler = handler; } @Override @@ -335,4 +339,8 @@ public class StatisticsManagerImpl implements StatisticsManager, StatisticsManag this.isStatisticsPollingOn = isStatisticsPollingOn; } + public void onDeviceRemoved(DeviceInfo deviceInfo) { + contexts.remove(deviceInfo); + LOG.debug("Statistics context removed for node {}", deviceInfo.getLOGValue()); + } } diff --git a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/DeviceInitializationUtils.java b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/DeviceInitializationUtils.java index 9a8548c82a..0bdbe08a9c 100644 --- a/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/DeviceInitializationUtils.java +++ b/openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/util/DeviceInitializationUtils.java @@ -241,20 +241,26 @@ public class DeviceInitializationUtils { static void translateAndWriteReply(final MultipartType type, final DeviceContext dContext, final InstanceIdentifier nodeII, final Collection result, final ConvertorExecutor convertorExecutor) { - try { - result.stream() - .map(MultipartReply::getMultipartReplyBody) - .forEach(multipartReplyBody -> { - if (!(writeDesc(type, multipartReplyBody, dContext, nodeII) - || writeTableFeatures(type, multipartReplyBody, dContext, nodeII, convertorExecutor) - || writeMeterFeatures(type, multipartReplyBody, dContext, nodeII) - || writeGroupFeatures(type, multipartReplyBody, dContext, nodeII) - || writePortDesc(type, multipartReplyBody, dContext, nodeII))) { - throw new IllegalArgumentException("Unexpected MultipartType " + type); - } - }); - } catch (final Exception e) { - LOG.debug("translateAndWriteReply: Failed to write node {} to DS ", dContext.getDeviceInfo().getNodeId().toString(), e); + if (Objects.nonNull(result)) { + try { + result.stream() + .map(MultipartReply::getMultipartReplyBody) + .forEach(multipartReplyBody -> { + if (!(writeDesc(type, multipartReplyBody, dContext, nodeII) + || writeTableFeatures(type, multipartReplyBody, dContext, nodeII, convertorExecutor) + || writeMeterFeatures(type, multipartReplyBody, dContext, nodeII) + || writeGroupFeatures(type, multipartReplyBody, dContext, nodeII) + || writePortDesc(type, multipartReplyBody, dContext, nodeII))) { + throw new IllegalArgumentException("Unexpected MultipartType " + type); + } + }); + } catch (final Exception e) { + LOG.debug("translateAndWriteReply: Failed to write node {} to DS ", dContext.getDeviceInfo().getNodeId().toString(), e); + } + } else { + LOG.debug("translateAndWriteReply: Failed to write node {} to DS because we failed to gather device" + + "info.", + dContext.getDeviceInfo().getNodeId().toString()); } } @@ -492,6 +498,11 @@ public class DeviceInitializationUtils { final Xid xid = requestContext.getXid(); + if (Objects.isNull(xid)) { + LOG.debug("Xid is not present, so cancelling node static info gathering."); + return Futures.immediateCancelledFuture(); + } + LOG.trace("Hooking xid {} to device context - precaution.", reserved); final MultiMsgCollector multiMsgCollector = deviceContext.getMultiMsgCollector(requestContext); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/lifecycle/LifecycleServiceImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/lifecycle/LifecycleServiceImplTest.java index 8ac5337d5f..7d02cdb2e1 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/lifecycle/LifecycleServiceImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/lifecycle/LifecycleServiceImplTest.java @@ -16,6 +16,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceProvider; +import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceRegistration; import org.opendaylight.mdsal.singleton.common.api.ServiceGroupIdentifier; import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext; @@ -45,6 +46,8 @@ public class LifecycleServiceImplTest { private DeviceFlowRegistry deviceFlowRegistry; @Mock private ClusterSingletonServiceProvider clusterSingletonServiceProvider; + @Mock + private ClusterSingletonServiceRegistration clusterSingletonServiceRegistration; private LifecycleService lifecycleService; @@ -57,6 +60,8 @@ public class LifecycleServiceImplTest { Mockito.when(deviceFlowRegistry.fill()).thenReturn(Futures.immediateFuture(null)); Mockito.when(connectionContext.getConnectionState()).thenReturn(ConnectionContext.CONNECTION_STATE.WORKING); Mockito.when(deviceInfo.getLOGValue()).thenReturn(TEST_NODE); + Mockito.when(clusterSingletonServiceProvider.registerClusterSingletonService(Mockito.any())) + .thenReturn(clusterSingletonServiceRegistration); Mockito.when(deviceContext.stopClusterServices(Mockito.anyBoolean())).thenReturn(Futures.immediateFuture(null)); Mockito.when(statContext.stopClusterServices(Mockito.anyBoolean())).thenReturn(Futures.immediateFuture(null)); @@ -80,7 +85,7 @@ public class LifecycleServiceImplTest { @Test public void closeServiceInstance() throws Exception { - lifecycleService.closeServiceInstance(); + lifecycleService.closeServiceInstance().get(); Mockito.verify(statContext).stopClusterServices(false); Mockito.verify(deviceContext).stopClusterServices(false); Mockito.verify(rpcContext).stopClusterServices(false); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/rpc/RpcContextImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/rpc/RpcContextImplTest.java index d38ff06800..68e3dfdc91 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/rpc/RpcContextImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/rpc/RpcContextImplTest.java @@ -10,7 +10,6 @@ package org.opendaylight.openflowplugin.impl.rpc; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/rpc/RpcManagerImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/rpc/RpcManagerImplTest.java index f3c6dabb6b..62ce9bdade 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/rpc/RpcManagerImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/rpc/RpcManagerImplTest.java @@ -123,7 +123,7 @@ public class RpcManagerImplTest { Mockito.when(deviceContext.getMessageSpy()).thenReturn(messageSpy); Mockito.when(deviceInfo.getNodeId()).thenReturn(nodeKey.getId()); Mockito.when(rpcProviderRegistry.addRoutedRpcImplementation( - Matchers.>any(), Matchers.any(RpcService.class))) + Matchers.any(), Matchers.any(RpcService.class))) .thenReturn(routedRpcRegistration); Mockito.when(contexts.remove(deviceInfo)).thenReturn(removedContexts); Mockito.when(lifecycleService.getDeviceContext()).thenReturn(deviceContext); @@ -165,7 +165,7 @@ public class RpcManagerImplTest { */ @Test public void onDeviceContextLevelDown1() { - rpcManager.addRecordToContexts(deviceInfo,removedContexts); + rpcManager.addRecordToContexts(deviceInfo, removedContexts); rpcManager.onDeviceContextLevelDown(deviceInfo); verify(removedContexts,times(1)).close(); verify(deviceTerminationPhaseHandler,times(1)).onDeviceContextLevelDown(deviceInfo); diff --git a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImplTest.java b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImplTest.java index 0b9c455559..2ae19e9632 100644 --- a/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImplTest.java +++ b/openflowplugin-impl/src/test/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsManagerImplTest.java @@ -40,7 +40,6 @@ import org.opendaylight.controller.sal.binding.api.BindingAwareBroker; import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry; import org.opendaylight.openflowjava.protocol.api.connection.ConnectionAdapter; import org.opendaylight.openflowjava.protocol.api.connection.OutboundQueue; -import org.opendaylight.openflowplugin.api.OFConstants; import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext; import org.opendaylight.openflowplugin.api.openflow.device.DeviceInfo; @@ -221,7 +220,7 @@ public class StatisticsManagerImplTest { statisticsManager.onDeviceContextLevelDown(deviceInfo); verify(statisticContext).close(); verify(mockedTerminationPhaseHandler).onDeviceContextLevelDown(deviceInfo); - Assert.assertEquals(0, contextsMap.size()); + Assert.assertEquals(1, contextsMap.size()); } private static Map getContextsMap(final StatisticsManagerImpl statisticsManager) -- 2.36.6