BUG-7505: Conflict Modification

When closing Peer and Rib, can cause
race condition between the removal
of the peer and the rib when updating DS.
Fix by close peers on blocking mode, and once
they are closed proceed with rib.
Same its applied in case that peer/rib is updated.
First we close it and then once DS is updated
we proceed with creating the new instance.

Change-Id: Ibe70e0324ae12bcbb88b2e7f039671141447ddb1
Signed-off-by: Claudio D. Gasparini <claudio.gasparini@pantheon.tech>

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java
index 099bce0b0a3686f381afaead9bb7081cce2cf1a7..aa4f538e2b55864f79d91a0629c3165e1277aa46 100755 (executable)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/RIBImpl.java
@@ -14,6 +14,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.CheckedFuture;
+import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -23,6 +24,7 @@ import java.util.Map.Entry;
 import java.util.Set;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.GuardedBy;
 import javax.annotation.concurrent.ThreadSafe;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
@@ -119,6 +121,8 @@ public final class RIBImpl extends DefaultRibReference implements ClusterSinglet
     private final Map<TablesKey, ExportPolicyPeerTracker> exportPolicyPeerTrackerMap;
 
     private DOMTransactionChain domChain;
+    @GuardedBy("this")
+    private boolean isServiceInstantiated;
 
     public RIBImpl(final ClusterSingletonServiceProvider provider, final RibId ribId, final AsNumber localAs, final BgpId localBgpId,
         final ClusterIdentifier clusterId, final RIBExtensionConsumerContext extensions, final BGPDispatcher dispatcher,
@@ -161,7 +165,7 @@ public final class RIBImpl extends DefaultRibReference implements ClusterSinglet
         }
         this.exportPolicyPeerTrackerMap = exportPolicies.build();
 
-        LOG.info("RIB Singleton Service {} registered", getIdentifier());
+        LOG.info("RIB Singleton Service {} registered, RIB {}", getIdentifier().getValue(), this.ribId.getValue());
         //this need to be always the last step
         this.registration = registerClusterSingletonService(this);
     }
@@ -331,12 +335,13 @@ public final class RIBImpl extends DefaultRibReference implements ClusterSinglet
     }
 
     @Override
-    public void instantiateServiceInstance() {
+    public synchronized void instantiateServiceInstance() {
+        this.isServiceInstantiated = true;
         this.domChain = this.domDataBroker.createTransactionChain(this);
         if(this.configurationWriter != null) {
             this.configurationWriter.apply();
         }
-        LOG.info("RIB Singleton Service {} instantiated", getIdentifier());
+        LOG.info("RIB Singleton Service {} instantiated, RIB {}", getIdentifier().getValue(), this.ribId.getValue());
         LOG.debug("Instantiating RIB table {} at {}", this.ribId , this.yangRibId);
 
         final ContainerNode bgpRib = Builders.containerBuilder().withNodeIdentifier(new NodeIdentifier(BgpRib.QNAME))
@@ -373,7 +378,14 @@ public final class RIBImpl extends DefaultRibReference implements ClusterSinglet
 
     @Override
     public synchronized ListenableFuture<Void> closeServiceInstance() {
-        LOG.info("RIB {} closing instance", this.ribId.getValue());
+        if(!this.isServiceInstantiated) {
+            LOG.trace("RIB Singleton Service {} already closed, RIB {}", getIdentifier().getValue(),
+                this.ribId.getValue());
+            return Futures.immediateFuture(null);
+        }
+        LOG.info("Close RIB Singleton Service {}, RIB {}", getIdentifier().getValue(), this.ribId.getValue());
+        this.isServiceInstantiated = false;
+
         this.txChainToLocRibWriter.values().forEach(LocRibWriter::close);
         this.txChainToLocRibWriter.clear();
 

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java
index 70156ccad6ea537ab453659ccb18e9492b6c8d56..1000f777f555e66c90bdb623d2c8b0e4884855f5 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/AppPeer.java
@@ -10,8 +10,10 @@ package org.opendaylight.protocol.bgp.rib.impl.config;
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
+import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import java.util.Objects;
+import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataTreeChangeService;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataTreeIdentifier;
@@ -30,6 +32,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.rib.rev130925.rib.Tables;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.osgi.framework.ServiceRegistration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -39,6 +42,7 @@ public final class AppPeer implements PeerBean {
     private Neighbor currentConfiguration;
     private BgpAppPeerSingletonService bgpAppPeerSingletonService;
     private BGPOpenConfigMappingService mappingService;
+    private ServiceRegistration<?> serviceRegistration;
 
     @Override
     public void start(final RIB rib, final Neighbor neighbor, final BGPOpenConfigMappingService mappingService,
@@ -64,6 +68,19 @@ public final class AppPeer implements PeerBean {
         } catch (final Exception e) {
             LOG.warn("Failed to close application peer instance", e);
         }
+        if (this.serviceRegistration != null) {
+            this.serviceRegistration.unregister();
+            this.serviceRegistration = null;
+        }
+    }
+
+    @Override
+    public ListenableFuture<Void> closeServiceInstance() {
+        if (this.bgpAppPeerSingletonService != null) {
+            return this.bgpAppPeerSingletonService.closeServiceInstance();
+        }
+
+        return Futures.immediateFuture(null);
     }
 
     @Override
@@ -80,13 +97,21 @@ public final class AppPeer implements PeerBean {
         return new ApplicationRibId(neighbor.getNeighborAddress().getIpv4Address().getValue());
     }
 
-    private final class BgpAppPeerSingletonService implements ClusterSingletonService, AutoCloseable {
+
+    void setServiceRegistration(final ServiceRegistration<?> serviceRegistration) {
+        this.serviceRegistration = serviceRegistration;
+    }
+
+    private final class BgpAppPeerSingletonService implements ClusterSingletonService,
+        AutoCloseable {
         private final ApplicationPeer applicationPeer;
         private final DOMDataTreeChangeService dataTreeChangeService;
         private final ApplicationRibId appRibId;
         private ClusterSingletonServiceRegistration singletonServiceRegistration;
         private final ServiceGroupIdentifier serviceGroupIdentifier;
         private final WriteConfiguration configurationWriter;
+        @GuardedBy("this")
+        private boolean isServiceInstantiated;
 
         BgpAppPeerSingletonService(final RIB rib, final ApplicationRibId appRibId, final Ipv4Address neighborAddress,
             final WriteConfiguration configurationWriter) {
@@ -95,7 +120,8 @@ public final class AppPeer implements PeerBean {
             this.dataTreeChangeService = rib.getService();
             this.serviceGroupIdentifier = rib.getRibIServiceGroupIdentifier();
             this.configurationWriter = configurationWriter;
-            LOG.info("Application Peer Singleton Service {} registered", getIdentifier());
+            LOG.info("Application Peer Singleton Service {} registered, Application peer {}",
+                getIdentifier().getValue(), this.appRibId.getValue());
             //this need to be always the last step
             this.singletonServiceRegistration = rib.registerClusterSingletonService(this);
         }
@@ -109,20 +135,30 @@ public final class AppPeer implements PeerBean {
         }
 
         @Override
-        public void instantiateServiceInstance() {
+        public synchronized void instantiateServiceInstance() {
+            this.isServiceInstantiated = true;
             if(this.configurationWriter != null) {
                 this.configurationWriter.apply();
             }
-            LOG.info("Application Peer Singleton Service {} instantiated", getIdentifier());
+            LOG.info("Application Peer Singleton Service {} instantiated, Application peer {}",
+                getIdentifier().getValue(), this.appRibId.getValue());
             final YangInstanceIdentifier yangIId = YangInstanceIdentifier.builder().node(ApplicationRib.QNAME)
-                .nodeWithKey(ApplicationRib.QNAME, APP_ID_QNAME, this.appRibId.getValue()).node(Tables.QNAME).node(Tables.QNAME).build();
+                .nodeWithKey(ApplicationRib.QNAME, APP_ID_QNAME, this.appRibId.getValue()).node(Tables.QNAME)
+                .node(Tables.QNAME).build();
             this.applicationPeer.instantiateServiceInstance(this.dataTreeChangeService,
                 new DOMDataTreeIdentifier(LogicalDatastoreType.CONFIGURATION, yangIId));
         }
 
         @Override
-        public ListenableFuture<Void> closeServiceInstance() {
-            LOG.info("Application Peer Singleton Service {} instance closed", getIdentifier());
+        public synchronized ListenableFuture<Void> closeServiceInstance() {
+            if(!this.isServiceInstantiated) {
+                LOG.trace("Application Peer Singleton Service {} instance already closed, Application peer {}",
+                    getIdentifier().getValue(), this.appRibId.getValue());
+                return Futures.immediateFuture(null);
+            }
+            LOG.info("Application Peer Singleton Service {} instance closed, Application peer {}",
+                getIdentifier().getValue(), this.appRibId.getValue());
+            this.isServiceInstantiated = false;
             return this.applicationPeer.close();
         }
 

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java
index ea6d71e8d0dfb492a5b196f626e23d7e8802fcc6..8b536b0f0f78cce8d2232c26399b7c1670fc9110 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImpl.java
@@ -8,10 +8,12 @@
 
 package org.opendaylight.protocol.bgp.rib.impl.config;
 
+import static com.google.common.util.concurrent.Futures.transform;
 import static org.opendaylight.protocol.bgp.rib.impl.config.OpenConfigMappingUtil.getNeighborInstanceIdentifier;
 import static org.opendaylight.protocol.bgp.rib.impl.config.OpenConfigMappingUtil.getNeighborInstanceName;
 import static org.opendaylight.protocol.bgp.rib.impl.config.OpenConfigMappingUtil.getRibInstanceName;
 
+import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.FutureCallback;
@@ -24,6 +26,7 @@ import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.controller.md.sal.binding.api.ClusteredDataTreeChangeListener;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
@@ -72,7 +75,8 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
     @GuardedBy("this")
     private boolean closed;
 
-    public BgpDeployerImpl(final String networkInstanceName, final BlueprintContainer container, final BundleContext bundleContext, final DataBroker dataBroker,
+    public BgpDeployerImpl(final String networkInstanceName, final BlueprintContainer container,
+        final BundleContext bundleContext, final DataBroker dataBroker,
         final BGPOpenConfigMappingService mappingService) {
         this.dataBroker = Preconditions.checkNotNull(dataBroker);
         this.container = Preconditions.checkNotNull(container);
@@ -91,8 +95,9 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
                 LOG.error("Failed to initialize Network Instance {}.", networkInstanceName, t);
             }
         });
-        this.registration = dataBroker.registerDataTreeChangeListener(new DataTreeIdentifier<>(LogicalDatastoreType.CONFIGURATION,
-            this.networkInstanceIId.child(Protocols.class).child(Protocol.class).augmentation(Protocol1.class).child(Bgp.class)), this);
+        this.registration = dataBroker.registerDataTreeChangeListener(
+            new DataTreeIdentifier<>(LogicalDatastoreType.CONFIGURATION, this.networkInstanceIId.child(Protocols.class)
+                .child(Protocol.class).augmentation(Protocol1.class).child(Bgp.class)), this);
         LOG.info("BGP Deployer {} started.", networkInstanceName);
     }
 
@@ -123,12 +128,32 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
 
     @Override
     public synchronized void close() throws Exception {
+        LOG.info("Closing BGP Deployer.");
         this.registration.close();
-        this.peers.values().forEach(PeerBean::close);
-        this.peers.clear();
-        this.ribs.values().forEach(RibImpl::close);
-        this.ribs.clear();
         this.closed = true;
+
+        final List<ListenableFuture<Void>> futurePeerCloseList = this.peers.values().stream()
+            .map(PeerBean::closeServiceInstance).collect(Collectors.toList());
+        final ListenableFuture<List<Void>> futureOfRelevance = Futures.allAsList(futurePeerCloseList);
+
+        final ListenableFuture<ListenableFuture<List<Void>>> maxRelevanceFuture = transform(futureOfRelevance,
+            (Function<List<Void>, ListenableFuture<List<Void>>>) futurePeersClose -> {
+                this.peers.values().forEach(PeerBean::close);
+                BgpDeployerImpl.this.peers.clear();
+
+                final List<ListenableFuture<Void>> futureRIBCloseList = BgpDeployerImpl.this.ribs.values().stream()
+                    .map(RibImpl::closeServiceInstance).collect(Collectors.toList());
+                return Futures.allAsList(futureRIBCloseList);
+            });
+
+        final ListenableFuture<Void> ribFutureClose = transform(maxRelevanceFuture,
+            (Function<ListenableFuture<List<Void>>, Void>) futurePeersClose -> {
+                BgpDeployerImpl.this.ribs.values().forEach(RibImpl::close);
+                this.ribs.clear();
+                return null;
+            });
+
+        ribFutureClose.get();
     }
 
     private static CheckedFuture<Void, TransactionCommitFailedException> initializeNetworkInstance(
@@ -169,7 +194,13 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
     private synchronized List<PeerBean> closeAllBindedPeers(final InstanceIdentifier<Bgp> rootIdentifier) {
         final List<PeerBean> filtered = new ArrayList<>();
         this.peers.entrySet().stream().filter(entry -> entry.getKey().firstIdentifierOf(Bgp.class)
-            .contains(rootIdentifier)).forEach(entry -> {final PeerBean peer = entry.getValue();
+            .contains(rootIdentifier)).forEach(entry -> {
+            final PeerBean peer = entry.getValue();
+            try {
+                peer.closeServiceInstance().get();
+            } catch (final Exception e) {
+                LOG.error("Peer instance failed to close service instance", e);
+            }
             peer.close();
             filtered.add(peer);
         });
@@ -189,6 +220,11 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
         final RibImpl ribImpl, final WriteConfiguration configurationWriter) {
         LOG.debug("Modifying RIB instance with configuration: {}", global);
         final List<PeerBean> closedPeers = closeAllBindedPeers(rootIdentifier);
+        try {
+            ribImpl.closeServiceInstance().get();
+        } catch (final Exception e) {
+            LOG.error("RIB instance failed to close service instance", e);
+        }
         ribImpl.close();
         initiateRibInstance(rootIdentifier, global, ribImpl, configurationWriter);
         closedPeers.forEach(peer -> peer.restart(ribImpl, this.mappingService));
@@ -201,6 +237,8 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
         final RibImpl ribImpl = this.ribs.remove(rootIdentifier);
         if (ribImpl != null) {
             LOG.debug("RIB instance removed {}", ribImpl);
+            closeAllBindedPeers(rootIdentifier);
+            ribImpl.closeServiceInstance();
             ribImpl.close();
         }
     }
@@ -208,7 +246,8 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
     private synchronized void registerRibInstance(final RibImpl ribImpl, final String ribInstanceName) {
         final Dictionary<String, String> properties = new Hashtable<>();
         properties.put(InstanceType.RIB.getBeanName(), ribInstanceName);
-        final ServiceRegistration<?> serviceRegistration = this.bundleContext.registerService(InstanceType.RIB.getServices(), ribImpl, properties);
+        final ServiceRegistration<?> serviceRegistration = this.bundleContext.registerService(
+            InstanceType.RIB.getServices(), ribImpl, properties);
         ribImpl.setServiceRegistration(serviceRegistration);
     }
 
@@ -285,11 +324,21 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
     private synchronized void registerPeerInstance(final BgpPeer bgpPeer, final String peerInstanceName) {
         final Dictionary<String, String> properties = new Hashtable<>();
         properties.put(InstanceType.PEER.getBeanName(), peerInstanceName);
-        final ServiceRegistration<?> serviceRegistration = this.bundleContext.registerService(InstanceType.PEER.getServices(), bgpPeer, properties);
+        final ServiceRegistration<?> serviceRegistration = this.bundleContext
+            .registerService(InstanceType.PEER.getServices(), bgpPeer, properties);
         bgpPeer.setServiceRegistration(serviceRegistration);
     }
 
-    private synchronized void initiatePeerInstance(final InstanceIdentifier<Bgp> rootIdentifier, final InstanceIdentifier<Neighbor> neighborIdentifier, final Neighbor neighbor,
+    private synchronized void registerAppPeerInstance(final AppPeer appPeer, final String peerInstanceName) {
+        final Dictionary<String, String> properties = new Hashtable<>();
+        properties.put(InstanceType.PEER.getBeanName(), peerInstanceName);
+        final ServiceRegistration<?> serviceRegistration = this.bundleContext
+            .registerService(InstanceType.APP_PEER.getServices(), appPeer, properties);
+        appPeer.setServiceRegistration(serviceRegistration);
+    }
+
+    private synchronized void initiatePeerInstance(final InstanceIdentifier<Bgp> rootIdentifier,
+        final InstanceIdentifier<Neighbor> neighborIdentifier, final Neighbor neighbor,
         final PeerBean bgpPeer, final WriteConfiguration configurationWriter) {
         final String peerInstanceName = getNeighborInstanceName(neighborIdentifier);
         final RibImpl rib = this.ribs.get(rootIdentifier);
@@ -297,6 +346,8 @@ public final class BgpDeployerImpl implements BgpDeployer, ClusteredDataTreeChan
             bgpPeer.start(rib, neighbor, this.mappingService, configurationWriter);
             if (bgpPeer instanceof BgpPeer) {
                 registerPeerInstance((BgpPeer) bgpPeer, peerInstanceName);
+            } else if(bgpPeer instanceof AppPeer) {
+                registerAppPeerInstance((AppPeer) bgpPeer, peerInstanceName);
             }
         }
     }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
index d0365212a47a4eac6c80ecb0de5fe8a4cbfd08b4..330546826bbdd3be8ab6cf057883f5cccaa79e33 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpPeer.java
@@ -15,6 +15,7 @@ import static org.opendaylight.protocol.bgp.rib.impl.config.OpenConfigMappingUti
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Iterables;
+import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import io.netty.util.concurrent.Future;
 import java.net.InetSocketAddress;
@@ -22,6 +23,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
+import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.controller.config.yang.bgp.rib.impl.BGPPeerRuntimeMXBean;
 import org.opendaylight.controller.config.yang.bgp.rib.impl.BgpPeerState;
 import org.opendaylight.controller.config.yang.bgp.rib.impl.BgpSessionState;
@@ -99,6 +101,14 @@ public final class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
         }
     }
 
+    @Override
+    public ListenableFuture<Void> closeServiceInstance() {
+        if (this.bgpPeerSingletonService != null) {
+            return this.bgpPeerSingletonService.closeServiceInstance();
+        }
+        return Futures.immediateFuture(null);
+    }
+
     private void closeSingletonService() {
         try {
             this.bgpPeerSingletonService.close();
@@ -207,6 +217,8 @@ public final class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
         private final IpAddress neighborAddress;
         private final BGPSessionPreferences prefs;
         private Future<Void> connection;
+        @GuardedBy("this")
+        private boolean isServiceInstantiated;
 
         private BgpPeerSingletonService(final RIB rib, final Neighbor neighbor, final BGPOpenConfigMappingService mappingService,
             final WriteConfiguration configurationWriter) {
@@ -215,8 +227,8 @@ public final class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
                     mappingService.toPeerRole(neighbor), getSimpleRoutingPolicy(neighbor), BgpPeer.this.rpcRegistry);
             final List<BgpParameters> bgpParameters = getBgpParameters(neighbor, rib, mappingService);
             final KeyMapping keyMapping = OpenConfigMappingUtil.getNeighborKey(neighbor);
-            this.prefs = new BGPSessionPreferences(rib.getLocalAs(), getHoldTimer(neighbor), rib.getBgpIdentifier(), getPeerAs(neighbor, rib),
-                bgpParameters, getPassword(keyMapping));
+            this.prefs = new BGPSessionPreferences(rib.getLocalAs(), getHoldTimer(neighbor), rib.getBgpIdentifier(),
+                getPeerAs(neighbor, rib), bgpParameters, getPassword(keyMapping));
             this.activeConnection = OpenConfigMappingUtil.isActive(neighbor);
             this.dispatcher = rib.getDispatcher();
             this.inetAddress = Ipv4Util.toInetSocketAddress(this.neighborAddress, OpenConfigMappingUtil.getPort(neighbor));
@@ -224,7 +236,7 @@ public final class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
             this.key = Optional.fromNullable(keyMapping);
             this.configurationWriter = configurationWriter;
             this.serviceGroupIdentifier = rib.getRibIServiceGroupIdentifier();
-            LOG.info("Peer Singleton Service {} registered", this.serviceGroupIdentifier);
+            LOG.info("Peer Singleton Service {} registered", this.serviceGroupIdentifier.getValue());
             //this need to be always the last step
             this.registration = rib.registerClusterSingletonService(this);
         }
@@ -238,11 +250,12 @@ public final class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
         }
 
         @Override
-        public void instantiateServiceInstance() {
+        public synchronized void instantiateServiceInstance() {
+            this.isServiceInstantiated = true;
             if(this.configurationWriter != null) {
                 this.configurationWriter.apply();
             }
-            LOG.info("Peer Singleton Service {} instantiated", getIdentifier());
+            LOG.info("Peer Singleton Service {} instantiated, Peer {}", getIdentifier().getValue(), this.neighborAddress);
             this.bgpPeer.instantiateServiceInstance();
             BgpPeer.this.peerRegistry.addPeer(this.neighborAddress, this.bgpPeer, this.prefs);
             if (this.activeConnection) {
@@ -251,8 +264,14 @@ public final class BgpPeer implements PeerBean, BGPPeerRuntimeMXBean {
         }
 
         @Override
-        public ListenableFuture<Void> closeServiceInstance() {
-            LOG.info("Close Peer Singleton Service {}", getIdentifier());
+        public synchronized ListenableFuture<Void> closeServiceInstance() {
+            if(!this.isServiceInstantiated) {
+                LOG.info("Peer Singleton Service {} already closed, Peer {}", getIdentifier().getValue(),
+                    this.neighborAddress);
+                return Futures.immediateFuture(null);
+            }
+            LOG.info("Close Peer Singleton Service {}, Peer {}", getIdentifier().getValue(), this.neighborAddress);
+            this.isServiceInstantiated = false;
             if (this.connection != null) {
                 this.connection.cancel(true);
                 this.connection = null;

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/PeerBean.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/PeerBean.java
index a640494490eb626f9a93b3c0e4d53aabf3b77d01..967db288274a6c8624edd7e53adf730b7672a66e 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/PeerBean.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/PeerBean.java
@@ -8,6 +8,7 @@
 
 package org.opendaylight.protocol.bgp.rib.impl.config;
 
+import com.google.common.util.concurrent.ListenableFuture;
 import org.opendaylight.protocol.bgp.openconfig.spi.BGPOpenConfigMappingService;
 import org.opendaylight.protocol.bgp.rib.impl.spi.BgpDeployer.WriteConfiguration;
 import org.opendaylight.protocol.bgp.rib.impl.spi.RIB;
@@ -26,5 +27,7 @@ public interface PeerBean extends AutoCloseable {
     @Override
     void close();
 
+    ListenableFuture<Void> closeServiceInstance();
+
     Boolean containsEqualConfiguration(Neighbor neighbor);
 }

diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/RibImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/RibImpl.java
index 603094a0be79ee4216c1dfe62e1d7120742b19a4..627f8884ec8f6fc5d7934e53754dacda174e9401 100644 (file)
--- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/RibImpl.java
+++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/config/RibImpl.java
@@ -12,6 +12,8 @@ import static org.opendaylight.protocol.bgp.rib.impl.config.OpenConfigMappingUti
 
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
+import com.google.common.util.concurrent.Futures;
+import com.google.common.util.concurrent.ListenableFuture;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -167,6 +169,13 @@ public final class RibImpl implements RIB, AutoCloseable {
         return this.ribImpl.getService();
     }
 
+    public ListenableFuture<Void> closeServiceInstance() {
+        if (this.ribImpl != null) {
+            return this.ribImpl.closeServiceInstance();
+        }
+        return Futures.immediateFuture(null);
+    }
+
     @Override
     public void close() {
         if (this.ribImpl != null) {

diff --git a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImplTest.java b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImplTest.java
index 64a8e79afc2afc1b29fd3da9fe864aa7fb4f9ffd..27d3a0729cfcf709c00777f5cd816cd798e828b2 100644 (file)
--- a/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImplTest.java
+++ b/bgp/rib-impl/src/test/java/org/opendaylight/protocol/bgp/rib/impl/config/BgpDeployerImplTest.java
@@ -299,7 +299,7 @@ public class BgpDeployerImplTest {
         verifyPrivate(spyDeployer).invoke("onGlobalCreated", any(InstanceIdentifier.class), any(Global.class),
             any(BgpDeployer.WriteConfiguration.class));
         verifyPrivate(spyDeployer).invoke("onGlobalUpdated", any(InstanceIdentifier.class), any(Global.class), any(RibImpl.class), any(BgpDeployer.WriteConfiguration.class));
-        verifyPrivate(spyDeployer).invoke("closeAllBindedPeers", any(InstanceIdentifier.class));
+        verifyPrivate(spyDeployer, times(2)).invoke("closeAllBindedPeers", any(InstanceIdentifier.class));
         verifyPrivate(spyDeployer, times(2)).invoke("initiateRibInstance", any(InstanceIdentifier.class), any(Global.class), any(RibImpl.class), any
             (BgpDeployer.WriteConfiguration.class));
         verifyPrivate(spyDeployer, times(2)).invoke("registerRibInstance", any(RibImpl.class), anyString());