From 618519cef8ff4e11227b74ca1d1700406079388f Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Fri, 4 May 2018 17:46:09 +0200 Subject: [PATCH] Use managed transactions in vpnmanager-impl This also enforces restrictions on newReadWriteTransaction and newWriteOnlyTransaction calls, to prevent new code introducing unmanaged transactions. Change-Id: Ib496ce580fa2fa4a27d1d2a8d4eb52b25b6d569a Signed-off-by: Stephen Kitt --- vpnmanager/impl/pom.xml | 4 +- .../CentralizedSwitchChangeListener.java | 31 +- .../vpnmanager/DpnInVpnChangeListener.java | 8 +- .../vpnmanager/FibEntriesListener.java | 27 +- .../LearntVpnVipToPortEventProcessor.java | 23 +- .../netvirt/vpnmanager/TransactionUtil.java | 76 ---- .../TunnelInterfaceStateListener.java | 88 +++-- .../VpnElanInterfaceChangeListener.java | 9 +- .../vpnmanager/VpnFootprintService.java | 232 ++++++------ .../vpnmanager/VpnInstanceListener.java | 106 +++--- .../vpnmanager/VpnInterfaceManager.java | 344 +++++++++--------- .../vpnmanager/VpnInterfaceOpListener.java | 36 +- .../netvirt/vpnmanager/VpnManagerImpl.java | 47 +-- .../netvirt/vpnmanager/VpnNodeListener.java | 22 +- .../vpnmanager/VpnOpStatusListener.java | 147 ++++---- .../netvirt/vpnmanager/VpnUtil.java | 302 ++++----------- .../intervpnlink/IVpnLinkServiceImpl.java | 9 +- .../intervpnlink/InterVpnLinkListener.java | 17 +- .../intervpnlink/InterVpnLinkNodeAddTask.java | 13 +- .../intervpnlink/InterVpnLinkUtil.java | 78 ---- .../tasks/InterVpnLinkCreatorTask.java | 21 +- .../tasks/InterVpnLinkRemoverTask.java | 13 +- 22 files changed, 684 insertions(+), 969 deletions(-) delete mode 100644 vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/TransactionUtil.java diff --git a/vpnmanager/impl/pom.xml b/vpnmanager/impl/pom.xml index fc5bdcf229..b09af85c03 100644 --- a/vpnmanager/impl/pom.xml +++ b/vpnmanager/impl/pom.xml @@ -11,9 +11,9 @@ and is available at http://www.eclipse.org/legal/epl-v10.html org.opendaylight.netvirt - binding-parent + managed-tx-parent 0.7.0-SNAPSHOT - ../../commons/binding-parent + ../../commons/managed-tx-parent vpnmanager-impl diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/CentralizedSwitchChangeListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/CentralizedSwitchChangeListener.java index e1e605e8ac..18d1f3ca4f 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/CentralizedSwitchChangeListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/CentralizedSwitchChangeListener.java @@ -10,6 +10,7 @@ package org.opendaylight.netvirt.vpnmanager; import java.math.BigInteger; import java.util.List; +import java.util.Objects; import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.inject.Singleton; @@ -17,7 +18,10 @@ import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.mdsalutil.NwConstants; +import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.netvirt.vpnmanager.api.IVpnManager; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.natservice.rev160111.NaptSwitches; @@ -44,6 +48,7 @@ public class CentralizedSwitchChangeListener private static final Logger LOG = LoggerFactory.getLogger(CentralizedSwitchChangeListener.class); private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; private final IVpnManager vpnManager; private final ExternalRouterDataUtil externalRouterDataUtil; @@ -52,6 +57,7 @@ public class CentralizedSwitchChangeListener ExternalRouterDataUtil externalRouterDataUtil) { super(RouterToNaptSwitch.class, CentralizedSwitchChangeListener.class); this.dataBroker = dataBroker; + this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker); this.vpnManager = vpnManager; this.externalRouterDataUtil = externalRouterDataUtil; } @@ -71,31 +77,30 @@ public class CentralizedSwitchChangeListener @Override protected void remove(InstanceIdentifier key, RouterToNaptSwitch routerToNaptSwitch) { LOG.debug("Removing {}", routerToNaptSwitch); - WriteTransaction writeTx = dataBroker.newWriteOnlyTransaction(); - setupRouterGwFlows(routerToNaptSwitch, writeTx, NwConstants.DEL_FLOW); - writeTx.submit(); + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + setupRouterGwFlows(routerToNaptSwitch, tx, NwConstants.DEL_FLOW)), LOG, + "Error processing switch removal for {}", routerToNaptSwitch); } @Override protected void update(InstanceIdentifier key, RouterToNaptSwitch origRouterToNaptSwitch, RouterToNaptSwitch updatedRouterToNaptSwitch) { LOG.debug("Updating old {} new {}", origRouterToNaptSwitch, updatedRouterToNaptSwitch); - if (updatedRouterToNaptSwitch.getPrimarySwitchId() != origRouterToNaptSwitch.getPrimarySwitchId()) { - WriteTransaction removeTx = dataBroker.newWriteOnlyTransaction(); - setupRouterGwFlows(origRouterToNaptSwitch, removeTx, NwConstants.DEL_FLOW); - removeTx.submit(); - WriteTransaction writeTx = dataBroker.newWriteOnlyTransaction(); - setupRouterGwFlows(updatedRouterToNaptSwitch, writeTx, NwConstants.ADD_FLOW); - writeTx.submit(); + if (!Objects.equals(updatedRouterToNaptSwitch.getPrimarySwitchId(), + origRouterToNaptSwitch.getPrimarySwitchId())) { + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> { + setupRouterGwFlows(origRouterToNaptSwitch, tx, NwConstants.DEL_FLOW); + setupRouterGwFlows(updatedRouterToNaptSwitch, tx, NwConstants.ADD_FLOW); + }), LOG, "Error updating switch {} to {}", origRouterToNaptSwitch, updatedRouterToNaptSwitch); } } @Override protected void add(InstanceIdentifier key, RouterToNaptSwitch routerToNaptSwitch) { LOG.debug("Adding {}", routerToNaptSwitch); - WriteTransaction writeTx = dataBroker.newWriteOnlyTransaction(); - setupRouterGwFlows(routerToNaptSwitch, writeTx, NwConstants.ADD_FLOW); - writeTx.submit(); + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + setupRouterGwFlows(routerToNaptSwitch, tx, NwConstants.ADD_FLOW)), LOG, + "Error processing switch addition for {}", routerToNaptSwitch); } @Override diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/DpnInVpnChangeListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/DpnInVpnChangeListener.java index 70aa74f767..afb9c54bd9 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/DpnInVpnChangeListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/DpnInVpnChangeListener.java @@ -17,6 +17,8 @@ import javax.inject.Singleton; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.netvirt.vpnmanager.api.VpnHelper; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.AddDpnEvent; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.AddInterfaceToDpnOnVpnEvent; @@ -34,10 +36,12 @@ import org.slf4j.LoggerFactory; public class DpnInVpnChangeListener implements OdlL3vpnListener { private static final Logger LOG = LoggerFactory.getLogger(DpnInVpnChangeListener.class); private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; @Inject public DpnInVpnChangeListener(DataBroker dataBroker) { this.dataBroker = dataBroker; + this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker); } @Override @@ -71,10 +75,8 @@ public class DpnInVpnChangeListener implements OdlL3vpnListener { } } if (flushDpnsOnVpn) { - WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction(); - deleteDpn(vpnToDpnList, rd, writeTxn); try { - writeTxn.submit().get(); + txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> deleteDpn(vpnToDpnList, rd, tx)).get(); } catch (InterruptedException | ExecutionException e) { LOG.error("Error removing dpnToVpnList for vpn {} ", vpnName); throw new RuntimeException(e.getMessage(), e); diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/FibEntriesListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/FibEntriesListener.java index bc8d4025d7..f378896dbf 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/FibEntriesListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/FibEntriesListener.java @@ -16,6 +16,9 @@ import javax.inject.Singleton; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; +import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.FibEntries; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.fibentries.VrfTables; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.fibentries.VrfTablesKey; @@ -31,12 +34,14 @@ import org.slf4j.LoggerFactory; public class FibEntriesListener extends AsyncDataTreeChangeListenerBase { private static final Logger LOG = LoggerFactory.getLogger(FibEntriesListener.class); private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; private final VpnInstanceListener vpnInstanceListener; @Inject public FibEntriesListener(final DataBroker dataBroker, final VpnInstanceListener vpnInstanceListener) { super(VrfEntry.class, FibEntriesListener.class); this.dataBroker = dataBroker; + this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker); this.vpnInstanceListener = vpnInstanceListener; } @@ -105,10 +110,12 @@ public class FibEntriesListener extends AsyncDataTreeChangeListenerBase + tx.put(LogicalDatastoreType.OPERATIONAL, + VpnUtil.getVpnInstanceOpDataIdentifier(rd), + new VpnInstanceOpDataEntryBuilder(vpnInstanceOpData).setRouteEntryId(routeIds) + .build())), + LOG, "Error adding label to VPN instance"); } else { LOG.warn("No VPN Instance found for RD: {}", rd); } @@ -125,12 +132,12 @@ public class FibEntriesListener extends AsyncDataTreeChangeListenerBase + tx.put(LogicalDatastoreType.OPERATIONAL, + VpnUtil.getVpnInstanceOpDataIdentifier(rd), + new VpnInstanceOpDataEntryBuilder(vpnInstanceOpData).setRouteEntryId(routeIds) + .build())), + LOG, "Error removing label from VPN instance"); } } else { LOG.warn("No VPN Instance found for RD: {}", rd); diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/LearntVpnVipToPortEventProcessor.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/LearntVpnVipToPortEventProcessor.java index cfa86032fd..24a2354fdf 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/LearntVpnVipToPortEventProcessor.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/LearntVpnVipToPortEventProcessor.java @@ -21,10 +21,11 @@ import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.controller.md.sal.binding.api.DataBroker; -import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.genius.arputil.api.ArpConstants; import org.opendaylight.genius.datastoreutils.AsyncClusteredDataTreeChangeListenerBase; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.interfacemanager.interfaces.IInterfaceManager; import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager; import org.opendaylight.genius.utils.clustering.EntityOwnershipUtils; @@ -58,6 +59,7 @@ public class LearntVpnVipToPortEventProcessor extends AsyncClusteredDataTreeChangeListenerBase { private static final Logger LOG = LoggerFactory.getLogger(LearntVpnVipToPortEventProcessor.class); private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; private final OdlInterfaceRpcService interfaceRpc; private final IMdsalApiManager mdsalManager; private final AlivenessMonitorService alivenessManager; @@ -77,6 +79,7 @@ public class LearntVpnVipToPortEventProcessor IdManagerService idManagerService, final JobCoordinator jobCoordinator) { super(LearntVpnVipToPortEvent.class, LearntVpnVipToPortEventProcessor.class); this.dataBroker = dataBroker; + this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker); this.interfaceRpc = interfaceRpc; this.mdsalManager = mdsalManager; this.alivenessManager = alivenessManager; @@ -173,17 +176,13 @@ public class LearntVpnVipToPortEventProcessor } @Override - public List> call() throws Exception { - List> futures = new ArrayList<>(); - WriteTransaction writeOperTxn = dataBroker.newWriteOnlyTransaction(); - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); - addMipAdjacency(vpnName, interfaceName, - srcIpAddress, macAddress, destIpAddress); - VpnUtil.createLearntVpnVipToPort(dataBroker, vpnName, srcIpAddress, - interfaceName, macAddress, writeOperTxn); - futures.add(writeConfigTxn.submit()); - futures.add(writeOperTxn.submit()); - return futures; + public List> call() { + return Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(operTx -> { + addMipAdjacency(vpnName, interfaceName, + srcIpAddress, macAddress, destIpAddress); + VpnUtil.createLearntVpnVipToPort(dataBroker, vpnName, srcIpAddress, + interfaceName, macAddress, operTx); + })); } private void addMipAdjacency(String vpnInstName, String vpnInterface, String srcPrefix, String mipMacAddress, diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/TransactionUtil.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/TransactionUtil.java deleted file mode 100644 index 1fc965c6b8..0000000000 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/TransactionUtil.java +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright (c) 2015 - 2017 Ericsson India Global Services Pvt Ltd. 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.netvirt.vpnmanager; - -import com.google.common.base.Optional; -import com.google.common.util.concurrent.FutureCallback; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.MoreExecutors; -import java.util.concurrent.ExecutionException; -import org.opendaylight.controller.md.sal.binding.api.DataBroker; -import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction; -import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; -import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; -import org.opendaylight.yangtools.yang.binding.DataObject; -import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public final class TransactionUtil { - private static final Logger LOG = LoggerFactory.getLogger(TransactionUtil.class); - - private TransactionUtil() { - } - - public static final FutureCallback DEFAULT_CALLBACK = new FutureCallback() { - @Override - public void onSuccess(Void result) { - LOG.debug("onSuccess: Success in Datastore operation"); - } - - @Override - public void onFailure(Throwable error) { - LOG.error("onFailure: Error in Datastore operation", error); - } - }; - - public static Optional read(DataBroker dataBroker, LogicalDatastoreType datastoreType, - InstanceIdentifier path) { - - ReadOnlyTransaction tx = dataBroker.newReadOnlyTransaction(); - - Optional result; - try { - result = tx.read(datastoreType, path).get(); - } catch (InterruptedException | ExecutionException e) { - LOG.debug("read: Error while reading data from path {}", path); - throw new RuntimeException(e); - } - return result; - } - - public static void asyncWrite(DataBroker dataBroker, LogicalDatastoreType datastoreType, - InstanceIdentifier path, T data, FutureCallback callback) { - WriteTransaction tx = dataBroker.newWriteOnlyTransaction(); - tx.put(datastoreType, path, data, true); - Futures.addCallback(tx.submit(), callback, MoreExecutors.directExecutor()); - } - - public static void syncWrite(DataBroker dataBroker, LogicalDatastoreType datastoreType, - InstanceIdentifier path, T data) { - WriteTransaction tx = dataBroker.newWriteOnlyTransaction(); - tx.put(datastoreType, path, data, WriteTransaction.CREATE_MISSING_PARENTS); - try { - tx.submit().get(); - } catch (InterruptedException | ExecutionException e) { - LOG.error("syncWrite: Error writing VPN instance to ID info to datastore (path, data) : ({}, {})", path, - data); - throw new RuntimeException(e.getMessage(), e); - } - } -} diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/TunnelInterfaceStateListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/TunnelInterfaceStateListener.java index 942dab2125..7bedb847ef 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/TunnelInterfaceStateListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/TunnelInterfaceStateListener.java @@ -27,12 +27,14 @@ import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.controller.md.sal.binding.api.DataBroker; -import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.mdsalutil.MDSALUtil; import org.opendaylight.genius.mdsalutil.NwConstants; import org.opendaylight.infrautils.jobcoordinator.JobCoordinator; +import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.netvirt.fibmanager.api.IFibManager; import org.opendaylight.netvirt.vpnmanager.api.InterfaceUtils; import org.opendaylight.netvirt.vpnmanager.api.VpnExtraRouteHelper; @@ -80,6 +82,7 @@ public class TunnelInterfaceStateListener extends AsyncDataTreeChangeListenerBas private static final Logger LOG = LoggerFactory.getLogger(TunnelInterfaceStateListener.class); private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; private final IFibManager fibManager; private final OdlInterfaceRpcService intfRpcService; private final VpnInterfaceManager vpnInterfaceManager; @@ -108,6 +111,7 @@ public class TunnelInterfaceStateListener extends AsyncDataTreeChangeListenerBas final JobCoordinator jobCoordinator) { super(StateTunnelList.class, TunnelInterfaceStateListener.class); this.dataBroker = dataBroker; + this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker); this.fibManager = fibManager; this.intfRpcService = ifaceMgrRpcService; this.vpnInterfaceManager = vpnInterfaceManager; @@ -166,36 +170,35 @@ public class TunnelInterfaceStateListener extends AsyncDataTreeChangeListenerBas LOG.trace("update: No vpnInstanceOpdata present"); return; } - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); if (tunOpStatus == TunnelOperStatus.Up) { handleTunnelEventForDPN(update, TunnelAction.TUNNEL_EP_ADD); } else { - vpnInstanceOpData.stream().filter(opData -> { - if (opData.getVpnToDpnList() == null) { - return false; - } - return opData.getVpnToDpnList().stream().anyMatch(vpnToDpn -> vpnToDpn.getDpnId().equals(srcDpnId)); - }).forEach(opData -> { - List prefixes = VpnExtraRouteHelper.getExtraRouteDestPrefixes(dataBroker, - opData.getVpnId()); - prefixes.forEach(destPrefix -> { - VrfEntry vrfEntry = VpnUtil.getVrfEntry(dataBroker, opData.getVrfId(), - destPrefix.getDestPrefix()); - if (vrfEntry == null || vrfEntry.getRoutePaths() == null) { - return; - } - List routePaths = vrfEntry.getRoutePaths(); - routePaths.forEach(routePath -> { - if (routePath.getNexthopAddress().equals(srcTepIp)) { - fibManager.updateRoutePathForFibEntry(opData.getVrfId(), - destPrefix.getDestPrefix(), srcTepIp, routePath.getLabel(), - false, writeConfigTxn); - } - }); - }); - }); + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(confTx -> + vpnInstanceOpData.stream() + .filter(opData -> opData.getVpnToDpnList() != null + && opData.getVpnToDpnList().stream().anyMatch( + vpnToDpn -> vpnToDpn.getDpnId().equals(srcDpnId))) + .forEach(opData -> { + List prefixes = VpnExtraRouteHelper.getExtraRouteDestPrefixes(dataBroker, + opData.getVpnId()); + prefixes.forEach(destPrefix -> { + VrfEntry vrfEntry = VpnUtil.getVrfEntry(dataBroker, opData.getVrfId(), + destPrefix.getDestPrefix()); + if (vrfEntry == null || vrfEntry.getRoutePaths() == null) { + return; + } + List routePaths = vrfEntry.getRoutePaths(); + routePaths.forEach(routePath -> { + if (routePath.getNexthopAddress().equals(srcTepIp)) { + fibManager.updateRoutePathForFibEntry(opData.getVrfId(), + destPrefix.getDestPrefix(), srcTepIp, routePath.getLabel(), + false, confTx); + } + }); + }); + }) + ), LOG, "Error updating route paths for FIB entries"); } - writeConfigTxn.submit(); } @Override @@ -507,26 +510,19 @@ public class TunnelInterfaceStateListener extends AsyncDataTreeChangeListenerBas @Override public List> call() { - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); - WriteTransaction writeOperTxn = dataBroker.newWriteOnlyTransaction(); - List> futures = new ArrayList<>(); - - if (tunnelAction == TunnelAction.TUNNEL_EP_ADD) { - vpnInterfaceManager.updateVpnInterfaceOnTepAdd(vpnInterface, - stateTunnelList, - writeConfigTxn, - writeOperTxn); - } - - if (tunnelAction == TunnelAction.TUNNEL_EP_DELETE && isTepDeletedOnDpn) { - vpnInterfaceManager.updateVpnInterfaceOnTepDelete(vpnInterface, - stateTunnelList, - writeConfigTxn, - writeOperTxn); - } + List> futures = new ArrayList<>(2); + futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit(confTx -> + futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit(operTx -> { + if (tunnelAction == TunnelAction.TUNNEL_EP_ADD) { + vpnInterfaceManager.updateVpnInterfaceOnTepAdd(vpnInterface, stateTunnelList, confTx, + operTx); + } - futures.add(writeOperTxn.submit()); - futures.add(writeConfigTxn.submit()); + if (tunnelAction == TunnelAction.TUNNEL_EP_DELETE && isTepDeletedOnDpn) { + vpnInterfaceManager.updateVpnInterfaceOnTepDelete(vpnInterface, stateTunnelList, confTx, + operTx); + } + })))); return futures; } } diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnElanInterfaceChangeListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnElanInterfaceChangeListener.java index c6c058eb02..0575d20ab6 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnElanInterfaceChangeListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnElanInterfaceChangeListener.java @@ -18,6 +18,9 @@ import javax.inject.Singleton; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; +import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.netvirt.elanmanager.api.IElanService; import org.opendaylight.netvirt.vpnmanager.api.VpnHelper; import org.opendaylight.yang.gen.v1.urn.huawei.params.xml.ns.yang.l3vpn.rev140815.vpn.interfaces.VpnInterface; @@ -38,12 +41,14 @@ public class VpnElanInterfaceChangeListener private static final Logger LOG = LoggerFactory.getLogger(VpnElanInterfaceChangeListener.class); private final DataBroker broker; + private final ManagedNewTransactionRunner txRunner; private final IElanService elanService; @Inject public VpnElanInterfaceChangeListener(final DataBroker broker, final IElanService elanService) { super(ElanInterface.class, VpnElanInterfaceChangeListener.class); this.broker = broker; + this.txRunner = new ManagedNewTransactionRunnerImpl(broker); this.elanService = elanService; } @@ -72,7 +77,9 @@ public class VpnElanInterfaceChangeListener } InstanceIdentifier vpnInterfaceIdentifier = VpnUtil.getVpnInterfaceIdentifier(interfaceName); - VpnUtil.delete(broker, LogicalDatastoreType.CONFIGURATION, vpnInterfaceIdentifier); + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + tx.delete(LogicalDatastoreType.CONFIGURATION, vpnInterfaceIdentifier)), LOG, + "Error removing VPN interface {}", vpnInterfaceIdentifier); LOG.info("remove: Removed VPN interface {}", interfaceName); } diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnFootprintService.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnFootprintService.java index 8349b42f5a..453a4b83b7 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnFootprintService.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnFootprintService.java @@ -17,6 +17,7 @@ import java.math.BigInteger; import java.util.ArrayList; import java.util.List; import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicBoolean; import javax.inject.Inject; import javax.inject.Singleton; import org.apache.commons.lang3.tuple.ImmutablePair; @@ -24,6 +25,8 @@ import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.interfacemanager.interfaces.IInterfaceManager; import org.opendaylight.netvirt.fibmanager.api.IFibManager; import org.opendaylight.netvirt.vpnmanager.api.IVpnFootprintService; @@ -63,6 +66,7 @@ public class VpnFootprintService implements IVpnFootprintService { private static final Logger LOG = LoggerFactory.getLogger(VpnFootprintService.class); private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; private final IFibManager fibManager; private final VpnOpDataSyncer vpnOpDataSyncer; private final NotificationPublishService notificationPublishService; @@ -73,6 +77,7 @@ public class VpnFootprintService implements IVpnFootprintService { final NotificationPublishService notificationPublishService, final VpnOpDataSyncer vpnOpDataSyncer, final IInterfaceManager interfaceManager) { this.dataBroker = dataBroker; + this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker); this.fibManager = fibManager; this.vpnOpDataSyncer = vpnOpDataSyncer; this.notificationPublishService = notificationPublishService; @@ -114,49 +119,51 @@ public class VpnFootprintService implements IVpnFootprintService { private void createOrUpdateVpnToDpnListForInterfaceName(long vpnId, String primaryRd, BigInteger dpnId, String intfName, String vpnName) { - Boolean newDpnOnVpn = Boolean.FALSE; + AtomicBoolean newDpnOnVpn = new AtomicBoolean(false); synchronized (vpnName.intern()) { - WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction(); InstanceIdentifier id = VpnHelper.getVpnToDpnListIdentifier(primaryRd, dpnId); Optional dpnInVpn = VpnUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL, id); VpnInterfaces vpnInterface = new VpnInterfacesBuilder().setInterfaceName(intfName).build(); - if (dpnInVpn.isPresent()) { - VpnToDpnList vpnToDpnList = dpnInVpn.get(); - List vpnInterfaces = vpnToDpnList.getVpnInterfaces(); - if (vpnInterfaces == null) { - vpnInterfaces = new ArrayList<>(); - } - vpnInterfaces.add(vpnInterface); - VpnToDpnListBuilder vpnToDpnListBuilder = new VpnToDpnListBuilder(vpnToDpnList); - vpnToDpnListBuilder.setDpnState(VpnToDpnList.DpnState.Active).setVpnInterfaces(vpnInterfaces); - - writeTxn.put(LogicalDatastoreType.OPERATIONAL, id, vpnToDpnListBuilder.build(), - WriteTransaction.CREATE_MISSING_PARENTS); - /* - * If earlier state was inactive, it is considered new DPN coming back to the - * same VPN - */ - if (vpnToDpnList.getDpnState() == VpnToDpnList.DpnState.Inactive) { - newDpnOnVpn = Boolean.TRUE; + ListenableFuture future = txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> { + if (dpnInVpn.isPresent()) { + VpnToDpnList vpnToDpnList = dpnInVpn.get(); + List vpnInterfaces = vpnToDpnList.getVpnInterfaces(); + if (vpnInterfaces == null) { + vpnInterfaces = new ArrayList<>(); + } + vpnInterfaces.add(vpnInterface); + VpnToDpnListBuilder vpnToDpnListBuilder = new VpnToDpnListBuilder(vpnToDpnList); + vpnToDpnListBuilder.setDpnState(VpnToDpnList.DpnState.Active).setVpnInterfaces(vpnInterfaces); + + tx.put(LogicalDatastoreType.OPERATIONAL, id, vpnToDpnListBuilder.build(), + WriteTransaction.CREATE_MISSING_PARENTS); + /* + * If earlier state was inactive, it is considered new DPN coming back to the + * same VPN + */ + if (vpnToDpnList.getDpnState() == VpnToDpnList.DpnState.Inactive) { + newDpnOnVpn.set(true); + } + LOG.debug("createOrUpdateVpnToDpnList: Updating vpn footprint for vpn {} vpnId {} interface {}" + + " on dpn {}", vpnName, vpnId, intfName, dpnId); + } else { + List vpnInterfaces = new ArrayList<>(); + vpnInterfaces.add(vpnInterface); + VpnToDpnListBuilder vpnToDpnListBuilder = new VpnToDpnListBuilder().setDpnId(dpnId); + vpnToDpnListBuilder.setDpnState(VpnToDpnList.DpnState.Active).setVpnInterfaces(vpnInterfaces); + + tx.put(LogicalDatastoreType.OPERATIONAL, id, vpnToDpnListBuilder.build(), + WriteTransaction.CREATE_MISSING_PARENTS); + newDpnOnVpn.set(true); + LOG.debug("createOrUpdateVpnToDpnList: Creating vpn footprint for vpn {} vpnId {} interface {}" + + " on dpn {}", vpnName, vpnId, intfName, dpnId); } - LOG.debug("createOrUpdateVpnToDpnList: Updating vpn footprint for vpn {} vpnId {} interface {}" - + " on dpn {}", vpnName, vpnId, intfName, dpnId); - } else { - List vpnInterfaces = new ArrayList<>(); - vpnInterfaces.add(vpnInterface); - VpnToDpnListBuilder vpnToDpnListBuilder = new VpnToDpnListBuilder().setDpnId(dpnId); - vpnToDpnListBuilder.setDpnState(VpnToDpnList.DpnState.Active).setVpnInterfaces(vpnInterfaces); - - writeTxn.put(LogicalDatastoreType.OPERATIONAL, id, vpnToDpnListBuilder.build(), - WriteTransaction.CREATE_MISSING_PARENTS); - newDpnOnVpn = Boolean.TRUE; - LOG.debug("createOrUpdateVpnToDpnList: Creating vpn footprint for vpn {} vpnId {} interface {}" - + " on dpn {}", vpnName, vpnId, intfName, dpnId); - } + }); + try { - writeTxn.submit().get(); + future.get(); } catch (InterruptedException | ExecutionException e) { LOG.error("createOrUpdateVpnToDpnList: Error adding to dpnToVpnList for vpn {} vpnId {} interface {}" + " dpn {}", vpnName, vpnId, intfName, dpnId, e); @@ -168,7 +175,7 @@ public class VpnFootprintService implements IVpnFootprintService { /* * Informing the FIB only after writeTxn is submitted successfully. */ - if (newDpnOnVpn) { + if (newDpnOnVpn.get()) { if (VpnUtil.isVlan(dataBroker ,intfName)) { if (!VpnUtil.shouldPopulateFibForVlan(dataBroker, vpnName, null, dpnId, interfaceManager)) { return; @@ -183,10 +190,9 @@ public class VpnFootprintService implements IVpnFootprintService { private void createOrUpdateVpnToDpnListForIPAddress(long vpnId, String primaryRd, BigInteger dpnId, ImmutablePair ipAddressSourceValuePair, String vpnName) { - Boolean newDpnOnVpn = Boolean.FALSE; + AtomicBoolean newDpnOnVpn = new AtomicBoolean(false); synchronized (vpnName.intern()) { - WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction(); InstanceIdentifier id = VpnHelper.getVpnToDpnListIdentifier(primaryRd, dpnId); Optional dpnInVpn = VpnUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL, id); IpAddressesBuilder ipAddressesBldr = new IpAddressesBuilder() @@ -194,35 +200,37 @@ public class VpnFootprintService implements IVpnFootprintService { ipAddressesBldr.setKey(new IpAddressesKey(ipAddressSourceValuePair.getValue())); ipAddressesBldr.setIpAddress(ipAddressSourceValuePair.getValue()); - if (dpnInVpn.isPresent()) { - VpnToDpnList vpnToDpnList = dpnInVpn.get(); - List ipAddresses = vpnToDpnList.getIpAddresses(); - if (ipAddresses == null) { - ipAddresses = new ArrayList<>(); - } - ipAddresses.add(ipAddressesBldr.build()); - VpnToDpnListBuilder vpnToDpnListBuilder = new VpnToDpnListBuilder(vpnToDpnList); - vpnToDpnListBuilder.setDpnState(VpnToDpnList.DpnState.Active).setIpAddresses(ipAddresses); - - writeTxn.put(LogicalDatastoreType.OPERATIONAL, id, vpnToDpnListBuilder.build(), true); - /* - * If earlier state was inactive, it is considered new DPN coming back to the - * same VPN - */ - if (vpnToDpnList.getDpnState() == VpnToDpnList.DpnState.Inactive) { - newDpnOnVpn = Boolean.TRUE; - } - } else { - List ipAddresses = new ArrayList<>(); - ipAddresses.add(ipAddressesBldr.build()); - VpnToDpnListBuilder vpnToDpnListBuilder = new VpnToDpnListBuilder().setDpnId(dpnId); - vpnToDpnListBuilder.setDpnState(VpnToDpnList.DpnState.Active).setIpAddresses(ipAddresses); + ListenableFuture future = txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> { + if (dpnInVpn.isPresent()) { + VpnToDpnList vpnToDpnList = dpnInVpn.get(); + List ipAddresses = vpnToDpnList.getIpAddresses(); + if (ipAddresses == null) { + ipAddresses = new ArrayList<>(); + } + ipAddresses.add(ipAddressesBldr.build()); + VpnToDpnListBuilder vpnToDpnListBuilder = new VpnToDpnListBuilder(vpnToDpnList); + vpnToDpnListBuilder.setDpnState(VpnToDpnList.DpnState.Active).setIpAddresses(ipAddresses); + + tx.put(LogicalDatastoreType.OPERATIONAL, id, vpnToDpnListBuilder.build(), true); + /* + * If earlier state was inactive, it is considered new DPN coming back to the + * same VPN + */ + if (vpnToDpnList.getDpnState() == VpnToDpnList.DpnState.Inactive) { + newDpnOnVpn.set(true); + } + } else { + List ipAddresses = new ArrayList<>(); + ipAddresses.add(ipAddressesBldr.build()); + VpnToDpnListBuilder vpnToDpnListBuilder = new VpnToDpnListBuilder().setDpnId(dpnId); + vpnToDpnListBuilder.setDpnState(VpnToDpnList.DpnState.Active).setIpAddresses(ipAddresses); - writeTxn.put(LogicalDatastoreType.OPERATIONAL, id, vpnToDpnListBuilder.build(), true); - newDpnOnVpn = Boolean.TRUE; - } + tx.put(LogicalDatastoreType.OPERATIONAL, id, vpnToDpnListBuilder.build(), true); + newDpnOnVpn.set(true); + } + }); try { - writeTxn.submit().get(); + future.get(); } catch (InterruptedException | ExecutionException e) { LOG.error("Error adding to dpnToVpnList for vpn {} ipAddresses {} dpn {}", vpnName, ipAddressSourceValuePair.getValue(), dpnId, e); @@ -232,7 +240,7 @@ public class VpnFootprintService implements IVpnFootprintService { /* * Informing the Fib only after writeTxn is submitted successfuly. */ - if (newDpnOnVpn) { + if (newDpnOnVpn.get()) { LOG.debug("Sending populateFib event for new dpn {} in VPN {}", dpnId, vpnName); fibManager.populateFibOnNewDpn(dpnId, vpnId, primaryRd, new DpnEnterExitVpnWorker(dpnId, vpnName, primaryRd, true /* entered */)); @@ -241,7 +249,7 @@ public class VpnFootprintService implements IVpnFootprintService { private void removeOrUpdateVpnToDpnListForInterfaceName(long vpnId, String rd, BigInteger dpnId, String intfName, String vpnName) { - Boolean lastDpnOnVpn = Boolean.FALSE; + AtomicBoolean lastDpnOnVpn = new AtomicBoolean(false); synchronized (vpnName.intern()) { InstanceIdentifier id = VpnHelper.getVpnToDpnListIdentifier(rd, dpnId); VpnToDpnList dpnInVpn = VpnUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL, id).orNull(); @@ -259,30 +267,31 @@ public class VpnFootprintService implements IVpnFootprintService { VpnInterfaces currVpnInterface = new VpnInterfacesBuilder().setInterfaceName(intfName).build(); if (vpnInterfaces.remove(currVpnInterface)) { - WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction(); - if (vpnInterfaces.isEmpty()) { - List ipAddresses = dpnInVpn.getIpAddresses(); - VpnToDpnListBuilder dpnInVpnBuilder = new VpnToDpnListBuilder(dpnInVpn).setVpnInterfaces(null); - if (ipAddresses == null || ipAddresses.isEmpty()) { - dpnInVpnBuilder.setDpnState(VpnToDpnList.DpnState.Inactive); - lastDpnOnVpn = Boolean.TRUE; - } else { - LOG.error("removeOrUpdateVpnToDpnList: vpn interfaces are empty but ip addresses are present" - + " for the vpn {} in dpn {} interface {}", vpnName, dpnId, intfName); - } - LOG.debug("removeOrUpdateVpnToDpnList: Removing vpn footprint for vpn {} vpnId {} interface {}," - + " on dpn {}", vpnName, vpnName, intfName, dpnId); - writeTxn.put(LogicalDatastoreType.OPERATIONAL, id, dpnInVpnBuilder.build(), - WriteTransaction.CREATE_MISSING_PARENTS); - - } else { - writeTxn.delete(LogicalDatastoreType.OPERATIONAL, - id.child(VpnInterfaces.class, new VpnInterfacesKey(intfName))); - LOG.debug("removeOrUpdateVpnToDpnList: Updating vpn footprint for vpn {} vpnId {} interface {}," - + " on dpn {}", vpnName, vpnName, intfName, dpnId); - } try { - writeTxn.submit().get(); + txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> { + if (vpnInterfaces.isEmpty()) { + List ipAddresses = dpnInVpn.getIpAddresses(); + VpnToDpnListBuilder dpnInVpnBuilder = + new VpnToDpnListBuilder(dpnInVpn).setVpnInterfaces(null); + if (ipAddresses == null || ipAddresses.isEmpty()) { + dpnInVpnBuilder.setDpnState(VpnToDpnList.DpnState.Inactive); + lastDpnOnVpn.set(true); + } else { + LOG.error("removeOrUpdateVpnToDpnList: vpn interfaces are empty but ip addresses are " + + "present for the vpn {} in dpn {} interface {}", vpnName, dpnId, intfName); + } + LOG.debug("removeOrUpdateVpnToDpnList: Removing vpn footprint for vpn {} vpnId {} " + + "interface {}, on dpn {}", vpnName, vpnName, intfName, dpnId); + tx.put(LogicalDatastoreType.OPERATIONAL, id, dpnInVpnBuilder.build(), + WriteTransaction.CREATE_MISSING_PARENTS); + + } else { + tx.delete(LogicalDatastoreType.OPERATIONAL, + id.child(VpnInterfaces.class, new VpnInterfacesKey(intfName))); + LOG.debug("removeOrUpdateVpnToDpnList: Updating vpn footprint for vpn {} vpnId {} " + + "interface {}, on dpn {}", vpnName, vpnName, intfName, dpnId); + } + }).get(); } catch (InterruptedException | ExecutionException e) { LOG.error("removeOrUpdateVpnToDpnList: Error removing from dpnToVpnList for vpn {} vpnId {}" + " interface {} dpn {}", vpnName, vpnId, intfName, dpnId, e); @@ -293,7 +302,7 @@ public class VpnFootprintService implements IVpnFootprintService { LOG.info("removeOrUpdateVpnToDpnList: Updated/Removed vpn footprint for vpn {} vpnId {} interface {}," + " on dpn {}", vpnName, vpnName, intfName, dpnId); - if (lastDpnOnVpn) { + if (lastDpnOnVpn.get()) { fibManager.cleanUpDpnForVpn(dpnId, vpnId, rd, new DpnEnterExitVpnWorker(dpnId, vpnName, rd, false /* exited */)); LOG.info("removeOrUpdateVpnToDpnList: Sent cleanup event for dpn {} in VPN {} vpnId {} interface {}", dpnId, @@ -303,7 +312,7 @@ public class VpnFootprintService implements IVpnFootprintService { private void removeOrUpdateVpnToDpnListForIpAddress(long vpnId, String rd, BigInteger dpnId, ImmutablePair ipAddressSourceValuePair, String vpnName) { - Boolean lastDpnOnVpn = Boolean.FALSE; + AtomicBoolean lastDpnOnVpn = new AtomicBoolean(false); synchronized (vpnName.intern()) { InstanceIdentifier id = VpnHelper.getVpnToDpnListIdentifier(rd, dpnId); VpnToDpnList dpnInVpn = VpnUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL, id).orNull(); @@ -323,25 +332,26 @@ public class VpnFootprintService implements IVpnFootprintService { .setKey(new IpAddressesKey(ipAddressSourceValuePair.getValue())) .setIpAddressSource(ipAddressSourceValuePair.getKey()).build(); if (ipAddresses.remove(currIpAddress)) { - WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction(); - if (ipAddresses.isEmpty()) { - List vpnInterfaces = dpnInVpn.getVpnInterfaces(); - VpnToDpnListBuilder dpnInVpnBuilder = new VpnToDpnListBuilder(dpnInVpn).setIpAddresses(null); - if (vpnInterfaces == null || vpnInterfaces.isEmpty()) { - dpnInVpnBuilder.setDpnState(VpnToDpnList.DpnState.Inactive); - lastDpnOnVpn = Boolean.TRUE; - } else { - LOG.warn("ip addresses are empty but vpn interfaces are present for the vpn {} in dpn {}", - vpnName, dpnId); - } - writeTxn.put(LogicalDatastoreType.OPERATIONAL, id, dpnInVpnBuilder.build(), true); - - } else { - writeTxn.delete(LogicalDatastoreType.OPERATIONAL, - id.child(IpAddresses.class, new IpAddressesKey(ipAddressSourceValuePair.getValue()))); - } try { - writeTxn.submit().get(); + txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> { + if (ipAddresses.isEmpty()) { + List vpnInterfaces = dpnInVpn.getVpnInterfaces(); + VpnToDpnListBuilder dpnInVpnBuilder = + new VpnToDpnListBuilder(dpnInVpn).setIpAddresses(null); + if (vpnInterfaces == null || vpnInterfaces.isEmpty()) { + dpnInVpnBuilder.setDpnState(VpnToDpnList.DpnState.Inactive); + lastDpnOnVpn.set(true); + } else { + LOG.warn("ip addresses are empty but vpn interfaces are present for the vpn {} in " + + "dpn {}", vpnName, dpnId); + } + tx.put(LogicalDatastoreType.OPERATIONAL, id, dpnInVpnBuilder.build(), true); + + } else { + tx.delete(LogicalDatastoreType.OPERATIONAL, id.child(IpAddresses.class, + new IpAddressesKey(ipAddressSourceValuePair.getValue()))); + } + }).get(); } catch (InterruptedException | ExecutionException e) { LOG.error("Error removing from dpnToVpnList for vpn {} Ipaddress {} dpn {}", vpnName, ipAddressSourceValuePair.getValue(), dpnId, e); @@ -350,7 +360,7 @@ public class VpnFootprintService implements IVpnFootprintService { } } // Ends synchronized block - if (lastDpnOnVpn) { + if (lastDpnOnVpn.get()) { LOG.debug("Sending cleanup event for dpn {} in VPN {}", dpnId, vpnName); fibManager.cleanUpDpnForVpn(dpnId, vpnId, rd, new DpnEnterExitVpnWorker(dpnId, vpnName, rd, false /* exited */)); diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInstanceListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInstanceListener.java index d6250e5b80..900e3bdd7f 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInstanceListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInstanceListener.java @@ -18,7 +18,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.inject.Singleton; @@ -28,6 +27,8 @@ import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase; import org.opendaylight.genius.datastoreutils.SingleTransactionDataBroker; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.mdsalutil.FlowEntity; import org.opendaylight.genius.mdsalutil.InstructionInfo; import org.opendaylight.genius.mdsalutil.MDSALUtil; @@ -41,6 +42,7 @@ import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager; import org.opendaylight.genius.mdsalutil.matches.MatchTunnelId; import org.opendaylight.genius.utils.SystemPropertyReader; import org.opendaylight.infrautils.jobcoordinator.JobCoordinator; +import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.netvirt.fibmanager.api.IFibManager; import org.opendaylight.yang.gen.v1.urn.huawei.params.xml.ns.yang.l3vpn.rev140815.VpnAfConfig; import org.opendaylight.yang.gen.v1.urn.huawei.params.xml.ns.yang.l3vpn.rev140815.VpnInstances; @@ -68,6 +70,7 @@ public class VpnInstanceListener extends AsyncDataTreeChangeListenerBase { - VpnInstanceOpDataEntryBuilder builder = new VpnInstanceOpDataEntryBuilder().setVrfId(primaryRd) - .setVpnState(VpnInstanceOpDataEntry.VpnState.PendingDelete); - InstanceIdentifier id = VpnUtil.getVpnInstanceOpDataIdentifier(primaryRd); - WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction(); - writeTxn.merge(LogicalDatastoreType.OPERATIONAL, id, builder.build()); - - LOG.info("{} call: Operational status set to PENDING_DELETE for vpn {} with rd {}", - LOGGING_PREFIX_DELETE, vpnName, primaryRd); - return Collections.singletonList(writeTxn.submit()); - }, SystemPropertyReader.getDataStoreJobCoordinatorMaxRetries()); + jobCoordinator.enqueueJob("VPN-" + vpnName, () -> + Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> { + VpnInstanceOpDataEntryBuilder builder = new VpnInstanceOpDataEntryBuilder().setVrfId(primaryRd) + .setVpnState(VpnInstanceOpDataEntry.VpnState.PendingDelete); + InstanceIdentifier id = + VpnUtil.getVpnInstanceOpDataIdentifier(primaryRd); + tx.merge(LogicalDatastoreType.OPERATIONAL, id, builder.build()); + + LOG.info("{} call: Operational status set to PENDING_DELETE for vpn {} with rd {}", + LOGGING_PREFIX_DELETE, vpnName, primaryRd); + })), SystemPropertyReader.getDataStoreJobCoordinatorMaxRetries()); } } @@ -173,19 +177,15 @@ public class VpnInstanceListener extends AsyncDataTreeChangeListenerBase> call() { // If another renderer(for eg : CSS) needs to be supported, check can be performed here // to call the respective helpers. - WriteTransaction writeConfigTxn = broker.newWriteOnlyTransaction(); - WriteTransaction writeOperTxn = broker.newWriteOnlyTransaction(); - addVpnInstance(vpnInstance, writeConfigTxn, writeOperTxn); - try { - writeOperTxn.submit().get(); - } catch (InterruptedException | ExecutionException e) { - log.error("{} call: Error creating vpn {} ", LOGGING_PREFIX_ADD, vpnInstance.getVpnInstanceName()); - throw new RuntimeException(e.getMessage(), e); - } - List> futures = new ArrayList<>(); - futures.add(writeConfigTxn.submit()); - ListenableFuture> listenableFuture = Futures.allAsList(futures); - Futures.addCallback(listenableFuture, + List> futures = new ArrayList<>(2); + futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit(confTx -> { + ListenableFuture future = txRunner.callWithNewWriteOnlyTransactionAndSubmit(operTx -> + addVpnInstance(vpnInstance, confTx, operTx)); + ListenableFutures.addErrorLogging(future, LOG, "{} call: error creating VPN {}", LOGGING_PREFIX_ADD, + vpnInstance.getVpnInstanceName()); + futures.add(future); + })); + Futures.addCallback(Futures.allAsList(futures), new PostAddVpnInstanceWorker(vpnInstance , vpnInstance.getVpnInstanceName()), MoreExecutors.directExecutor()); return futures; @@ -196,6 +196,16 @@ public class VpnInstanceListener extends AsyncDataTreeChangeListenerBase + addVpnInstance(value, tx, writeOperTxn)), LOG, "Error adding VPN instance {}", value); + return; + } + if (writeOperTxn == null) { + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + addVpnInstance(value, writeConfigTxn, tx)), LOG, "Error adding VPN instance {}", value); + return; + } VpnAfConfig config = value.getIpv4Family(); String vpnInstanceName = value.getVpnInstanceName(); @@ -211,28 +221,16 @@ public class VpnInstanceListener extends AsyncDataTreeChangeListenerBase id = VpnUtil.getVpnInstanceOpDataIdentifier(rd); - Optional vpnInstanceOpData = - TransactionUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL, id); - if (vpnInstanceOpData.isPresent()) { - return vpnInstanceOpData.get(); + try { + return SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.OPERATIONAL, + id).orNull(); + } catch (ReadFailedException e) { + throw new RuntimeException("Error reading VPN instance data for " + rd, e); } - return null; } private List getDcGatewayTunnelInterfaceNameList() { diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java index fd43bf0bb2..86e457468a 100755 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java @@ -13,6 +13,7 @@ import com.google.common.collect.Iterators; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.math.BigInteger; import java.util.ArrayList; @@ -23,7 +24,6 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.ExecutionException; import java.util.function.Consumer; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -266,50 +266,49 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase { - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); - WriteTransaction writeOperTxn = dataBroker.newWriteOnlyTransaction(); - WriteTransaction writeInvTxn = dataBroker.newWriteOnlyTransaction(); - LOG.info("addVpnInterface: VPN Interface add event - intfName {} vpnName {} on dpn {}" , - vpnInterface.getName(), vpnName, vpnInterface.getDpnId()); - processVpnInterfaceUp(dpnId, vpnInterface, primaryRd, ifIndex, false, writeConfigTxn, - writeOperTxn, writeInvTxn, interfaceState, vpnName); - if (oldAdjs != null && !oldAdjs.equals(newAdjs)) { - LOG.info("addVpnInterface: Adjacency changed upon VPNInterface {}" - + " Update for swapping VPN {} case.", interfaceName, vpnName); - if (newAdjs != null) { - for (Adjacency adj : newAdjs) { - if (oldAdjs.contains(adj)) { - oldAdjs.remove(adj); - } else { - if (!isBgpVpnInternetVpn || VpnUtil.isAdjacencyEligibleToVpnInternet( - dataBroker, adj)) { - addNewAdjToVpnInterface(vpnInterfaceOpIdentifier, primaryRd, - adj, dpnId, writeOperTxn, writeConfigTxn); - } - } - } - } - for (Adjacency adj : oldAdjs) { - if (!isBgpVpnInternetVpn || VpnUtil.isAdjacencyEligibleToVpnInternet( - dataBroker, adj)) { - delAdjFromVpnInterface(vpnInterfaceOpIdentifier, adj, dpnId, - writeOperTxn, writeConfigTxn); - } - } - } - ListenableFuture operFuture = writeOperTxn.submit(); - try { - operFuture.get(); - } catch (ExecutionException e) { - LOG.error("addVpnInterface: Exception encountered while submitting operational future for" - + " addVpnInterface {} on vpn {}", vpnInterface.getName(), vpnName, e); - return null; - } + // TODO Deal with sequencing — the config tx must only submitted if the oper tx goes in + // (the inventory tx goes in last) List> futures = new ArrayList<>(); - ListenableFuture configFuture = writeConfigTxn.submit(); - futures.add(configFuture); - Futures.addCallback(configFuture, new PostVpnInterfaceWorker(interfaceName, true, "Config")); - futures.add(writeInvTxn.submit()); + ListenableFuture confFuture = txRunner.callWithNewWriteOnlyTransactionAndSubmit( + confTx -> futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit( + operTx -> futures.add( + txRunner.callWithNewWriteOnlyTransactionAndSubmit(invTx -> { + LOG.info( + "addVpnInterface: VPN Interface add event - intfName {} vpnName {}" + + " on dpn {}", + vpnInterface.getName(), vpnName, vpnInterface.getDpnId()); + processVpnInterfaceUp(dpnId, vpnInterface, primaryRd, ifIndex, false, + confTx, operTx, invTx, interfaceState, vpnName); + if (oldAdjs != null && !oldAdjs.equals(newAdjs)) { + LOG.info("addVpnInterface: Adjacency changed upon VPNInterface {}" + + " Update for swapping VPN {} case.", interfaceName, vpnName); + if (newAdjs != null) { + for (Adjacency adj : newAdjs) { + if (oldAdjs.contains(adj)) { + oldAdjs.remove(adj); + } else { + if (!isBgpVpnInternetVpn + || VpnUtil.isAdjacencyEligibleToVpnInternet( + dataBroker, adj)) { + addNewAdjToVpnInterface(vpnInterfaceOpIdentifier, + primaryRd, adj, dpnId, operTx, confTx); + } + } + } + } + for (Adjacency adj : oldAdjs) { + if (!isBgpVpnInternetVpn + || VpnUtil.isAdjacencyEligibleToVpnInternet( + dataBroker, adj)) { + delAdjFromVpnInterface(vpnInterfaceOpIdentifier, adj, dpnId, + operTx, confTx); + } + } + } + }))))); + futures.add(confFuture); + Futures.addCallback(confFuture, new PostVpnInterfaceWorker(interfaceName, true, "Config"), + MoreExecutors.directExecutor()); LOG.info("addVpnInterface: Addition of interface {} in VPN {} on dpn {}" + " processed successfully", interfaceName, vpnName, dpnId); return futures; @@ -323,16 +322,15 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase { - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); - createFibEntryForRouterInterface(primaryRd, vpnInterface, interfaceName, - writeConfigTxn, vpnName); - LOG.info("addVpnInterface: Router interface {} for vpn {} on dpn {}", interfaceName, - vpnName, vpnInterface.getDpnId()); - ListenableFuture futures = writeConfigTxn.submit(); - String errorText = "addVpnInterfaceCall: Exception encountered while submitting writeConfigTxn" - + " for interface " + vpnInterface.getName() + " on vpn " + vpnName; - ListenableFutures.addErrorLogging(futures, LOG, errorText); - return Collections.singletonList(futures); + ListenableFuture future = txRunner.callWithNewWriteOnlyTransactionAndSubmit(confTx -> { + createFibEntryForRouterInterface(primaryRd, vpnInterface, interfaceName, + confTx, vpnName); + LOG.info("addVpnInterface: Router interface {} for vpn {} on dpn {}", interfaceName, + vpnName, vpnInterface.getDpnId()); + }); + ListenableFutures.addErrorLogging(future, LOG, + "Error creating FIB entry for interface {} on VPN {}", vpnInterface.getName(), vpnName); + return Collections.singletonList(future); }); } else { LOG.info("addVpnInterface: Handling addition of VPN interface {} on vpn {} skipped as interfaceState" @@ -1123,47 +1121,47 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase vpnsToExportRoute = getVpnsExportingMyRoute(vpnName); for (VpnInstanceOpDataEntry vpn : vpnsToExportRoute) { List vrfEntries = VpnUtil.getAllVrfEntries(dataBroker, vpn.getVrfId()); - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); if (vrfEntries != null) { - for (VrfEntry vrfEntry : vrfEntries) { - try { - if (!FibHelper.isControllerManagedNonInterVpnLinkRoute( - RouteOrigin.value(vrfEntry.getOrigin()))) { - LOG.info("handleVpnsExportingRoutes: vrfEntry with rd {} prefix {}" - + " is not a controller managed non intervpn link route. Ignoring.", - vpn.getVrfId(), vrfEntry.getDestPrefix()); - continue; - } - String prefix = vrfEntry.getDestPrefix(); - String gwMac = vrfEntry.getGatewayMacAddress(); - vrfEntry.getRoutePaths().forEach(routePath -> { - String nh = routePath.getNexthopAddress(); - int label = routePath.getLabel().intValue(); - if (FibHelper.isControllerManagedVpnInterfaceRoute(RouteOrigin.value( - vrfEntry.getOrigin()))) { - LOG.info("handleVpnsExportingRoutesImporting: Importing fib entry rd {} prefix {}" - + " nexthop {} label {} to vpn {} vpnRd {}", vpn.getVrfId(), prefix, nh, label, - vpnName, vpnRd); - fibManager.addOrUpdateFibEntry(vpnRd, null /*macAddress*/, prefix, - Collections.singletonList(nh), VrfEntry.EncapType.Mplsgre, label, - 0 /*l3vni*/, gwMac, vpn.getVrfId(), RouteOrigin.SELF_IMPORTED, - writeConfigTxn); - } else { - LOG.info("handleVpnsExportingRoutes: Importing subnet route fib entry rd {} prefix {}" - + " nexthop {} label {} to vpn {} vpnRd {}", vpn.getVrfId(), prefix, nh, label, - vpnName, vpnRd); - SubnetRoute route = vrfEntry.getAugmentation(SubnetRoute.class); - importSubnetRouteForNewVpn(vpnRd, prefix, nh, label, route, vpn.getVrfId(), - writeConfigTxn); + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(confTx -> { + for (VrfEntry vrfEntry : vrfEntries) { + try { + if (!FibHelper.isControllerManagedNonInterVpnLinkRoute( + RouteOrigin.value(vrfEntry.getOrigin()))) { + LOG.info("handleVpnsExportingRoutes: vrfEntry with rd {} prefix {}" + + " is not a controller managed non intervpn link route. Ignoring.", + vpn.getVrfId(), vrfEntry.getDestPrefix()); + continue; } - }); - } catch (RuntimeException e) { - LOG.error("getNextHopAddressList: Exception occurred while importing route with rd {}" - + " prefix {} routePaths {} to vpn {} vpnRd {}", vpn.getVrfId(), - vrfEntry.getDestPrefix(), vrfEntry.getRoutePaths(), vpnName, vpnRd); + String prefix = vrfEntry.getDestPrefix(); + String gwMac = vrfEntry.getGatewayMacAddress(); + vrfEntry.getRoutePaths().forEach(routePath -> { + String nh = routePath.getNexthopAddress(); + int label = routePath.getLabel().intValue(); + if (FibHelper.isControllerManagedVpnInterfaceRoute(RouteOrigin.value( + vrfEntry.getOrigin()))) { + LOG.info("handleVpnsExportingRoutesImporting: Importing fib entry rd {} prefix {}" + + " nexthop {} label {} to vpn {} vpnRd {}", + vpn.getVrfId(), prefix, nh, label, vpnName, vpnRd); + fibManager.addOrUpdateFibEntry(vpnRd, null /*macAddress*/, prefix, + Collections.singletonList(nh), VrfEntry.EncapType.Mplsgre, label, + 0 /*l3vni*/, gwMac, vpn.getVrfId(), RouteOrigin.SELF_IMPORTED, + confTx); + } else { + LOG.info("handleVpnsExportingRoutes: Importing subnet route fib entry rd {} " + + "prefix {} nexthop {} label {} to vpn {} vpnRd {}", + vpn.getVrfId(), prefix, nh, label, vpnName, vpnRd); + SubnetRoute route = vrfEntry.getAugmentation(SubnetRoute.class); + importSubnetRouteForNewVpn(vpnRd, prefix, nh, label, route, vpn.getVrfId(), + confTx); + } + }); + } catch (RuntimeException e) { + LOG.error("getNextHopAddressList: Exception occurred while importing route with rd {}" + + " prefix {} routePaths {} to vpn {} vpnRd {}", vpn.getVrfId(), + vrfEntry.getDestPrefix(), vrfEntry.getRoutePaths(), vpnName, vpnRd); + } } - } - writeConfigTxn.submit(); + }), LOG, "Error handing VPN exporting routes"); } else { LOG.info("getNextHopAddressList: No vrf entries to import from vpn {} with rd {} to vpn {} with rd {}", vpn.getVpnInstanceName(), vpn.getVrfId(), vpnName, vpnRd); @@ -1171,7 +1169,6 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase identifier, VpnInterface vpnInterface) { final VpnInterfaceKey key = identifier.firstKeyOf(VpnInterface.class, VpnInterfaceKey.class); @@ -1187,14 +1184,13 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase { - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); - deleteFibEntryForRouterInterface(vpnInterface, writeConfigTxn, vpnName); - LOG.info("remove: Router interface {} for vpn {}", interfaceName, vpnName); - ListenableFuture futures = writeConfigTxn.submit(); - String errorText = "removeVpnInterfaceCall: Exception encountered while submitting writeConfigTxn" - + " for interface " + vpnInterface.getName() + " on vpn " + vpnName; - ListenableFutures.addErrorLogging(futures, LOG, errorText); - return Collections.singletonList(futures); + ListenableFuture future = txRunner.callWithNewWriteOnlyTransactionAndSubmit(confTx -> { + deleteFibEntryForRouterInterface(vpnInterface, confTx, vpnName); + LOG.info("remove: Router interface {} for vpn {}", interfaceName, vpnName); + }); + ListenableFutures.addErrorLogging(future, LOG, "Error removing call for interface {} on VPN {}", + vpnInterface.getName(), vpnName); + return Collections.singletonList(future); }, DJC_MAX_RETRIES); } else { Interface interfaceState = InterfaceUtils.getInterfaceStateFromOperDS(dataBroker, interfaceName); @@ -1486,76 +1482,70 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase { - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); - WriteTransaction writeOperTxn = dataBroker.newWriteOnlyTransaction(); - InstanceIdentifier vpnInterfaceOpIdentifier = - VpnUtil.getVpnInterfaceOpDataEntryIdentifier(vpnInterfaceName, newVpnName); - LOG.info("VPN Interface update event - intfName {} onto vpnName {} running config-driven", - update.getName(), newVpnName); - //handle both addition and removal of adjacencies - //currently, new adjacency may be an extra route - boolean isBgpVpnInternetVpn = VpnUtil.isBgpVpnInternet(dataBroker, newVpnName); - if (!oldAdjs.equals(newAdjs)) { - for (Adjacency adj : copyNewAdjs) { - if (copyOldAdjs.contains(adj)) { - copyOldAdjs.remove(adj); - } else { - // add new adjacency - right now only extra route will hit this path - if (!isBgpVpnInternetVpn || VpnUtil.isAdjacencyEligibleToVpnInternet(dataBroker, adj)) { - addNewAdjToVpnInterface(vpnInterfaceOpIdentifier, primaryRd, adj, - dpnId, writeOperTxn, writeConfigTxn); + // TODO Deal with sequencing — the config tx must only submitted if the oper tx goes in + List> futures = new ArrayList<>(); + futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit(confTx -> { + futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit(operTx -> { + InstanceIdentifier vpnInterfaceOpIdentifier = + VpnUtil.getVpnInterfaceOpDataEntryIdentifier(vpnInterfaceName, newVpnName); + LOG.info("VPN Interface update event - intfName {} onto vpnName {} running config-driven", + update.getName(), newVpnName); + //handle both addition and removal of adjacencies + //currently, new adjacency may be an extra route + boolean isBgpVpnInternetVpn = VpnUtil.isBgpVpnInternet(dataBroker, newVpnName); + if (!oldAdjs.equals(newAdjs)) { + for (Adjacency adj : copyNewAdjs) { + if (copyOldAdjs.contains(adj)) { + copyOldAdjs.remove(adj); + } else { + // add new adjacency - right now only extra route will hit this path + if (!isBgpVpnInternetVpn || VpnUtil.isAdjacencyEligibleToVpnInternet(dataBroker, + adj)) { + addNewAdjToVpnInterface(vpnInterfaceOpIdentifier, primaryRd, adj, + dpnId, operTx, confTx); + } + LOG.info("update: new Adjacency {} with nextHop {} label {} subnet {} added to" + + " vpn interface {} on vpn {} dpnId {}", + adj.getIpAddress(), adj.getNextHopIpList(), + adj.getLabel(), adj.getSubnetId(), update.getName(), + newVpnName, dpnId); + } } - LOG.info("update: new Adjacency {} with nextHop {} label {} subnet {} added to vpn " - + "interface {} on vpn {} dpnId {}", - adj.getIpAddress(), adj.getNextHopIpList(), - adj.getLabel(), adj.getSubnetId(), update.getName(), - newVpnName, dpnId); - } - } - for (Adjacency adj : copyOldAdjs) { - if (!isBgpVpnInternetVpn || VpnUtil.isAdjacencyEligibleToVpnInternet(dataBroker, adj)) { - if (adj.getAdjacencyType() == AdjacencyType.PrimaryAdjacency - && !adj.isPhysNetworkFunc()) { - delAdjFromVpnInterface(vpnInterfaceOpIdentifier, adj, dpnId, - writeOperTxn, writeConfigTxn); - Optional optVpnInterface = VpnUtil.read(dataBroker, - LogicalDatastoreType.OPERATIONAL, vpnInterfaceOpIdentifier); - VpnInterfaceOpDataEntry vpnInterfaceOpDataEntry = optVpnInterface.get(); - long vpnId = VpnUtil.getVpnId(dataBroker, newVpnName); - VpnUtil.removePrefixToInterfaceAdj(dataBroker, adj, vpnId, vpnInterfaceOpDataEntry, - writeOperTxn); - //remove FIB entry - String vpnRd = VpnUtil.getVpnRd(dataBroker, newVpnName); - LOG.debug("update: remove prefix {} from the FIB and BGP entry " - + "for the Vpn-Rd {} ", adj.getIpAddress(), vpnRd); - //remove BGP entry - fibManager.removeFibEntry(vpnRd, adj.getIpAddress(), writeConfigTxn); - if (vpnRd != null && !vpnRd.equalsIgnoreCase(newVpnName)) { - bgpManager.withdrawPrefix(vpnRd, adj.getIpAddress()); + for (Adjacency adj : copyOldAdjs) { + if (!isBgpVpnInternetVpn || VpnUtil.isAdjacencyEligibleToVpnInternet(dataBroker, + adj)) { + if (adj.getAdjacencyType() == AdjacencyType.PrimaryAdjacency + && !adj.isPhysNetworkFunc()) { + delAdjFromVpnInterface(vpnInterfaceOpIdentifier, adj, dpnId, + operTx, confTx); + Optional optVpnInterface = VpnUtil.read(dataBroker, + LogicalDatastoreType.OPERATIONAL, vpnInterfaceOpIdentifier); + VpnInterfaceOpDataEntry vpnInterfaceOpDataEntry = optVpnInterface.get(); + long vpnId = VpnUtil.getVpnId(dataBroker, newVpnName); + VpnUtil.removePrefixToInterfaceAdj(dataBroker, adj, vpnId, + vpnInterfaceOpDataEntry, operTx); + //remove FIB entry + String vpnRd = VpnUtil.getVpnRd(dataBroker, newVpnName); + LOG.debug("update: remove prefix {} from the FIB and BGP entry " + + "for the Vpn-Rd {} ", adj.getIpAddress(), vpnRd); + //remove BGP entry + fibManager.removeFibEntry(vpnRd, adj.getIpAddress(), confTx); + if (vpnRd != null && !vpnRd.equalsIgnoreCase(newVpnName)) { + bgpManager.withdrawPrefix(vpnRd, adj.getIpAddress()); + } + } else { + delAdjFromVpnInterface(vpnInterfaceOpIdentifier, adj, dpnId, + operTx, confTx); + } } - } else { - delAdjFromVpnInterface(vpnInterfaceOpIdentifier, adj, dpnId, - writeOperTxn, writeConfigTxn); + LOG.info("update: Adjacency {} with nextHop {} label {} subnet {} removed from" + + " vpn interface {} on vpn {}", adj.getIpAddress(), adj + .getNextHopIpList(), + adj.getLabel(), adj.getSubnetId(), update.getName(), newVpnName); } } - LOG.info("update: Adjacency {} with nextHop {} label {} subnet {} removed from" - + " vpn interface {} on vpn {}", adj.getIpAddress(), adj.getNextHopIpList(), - adj.getLabel(), adj.getSubnetId(), update.getName(), newVpnName); - } - } - ListenableFuture operFuture = writeOperTxn.submit(); - try { - operFuture.get(); - } catch (ExecutionException e) { - LOG.error("Exception encountered while submitting operational future for update" - + " VpnInterface {} on vpn {}", vpnInterfaceName, newVpnName, e); - return null; - } - List> futures = new ArrayList<>(); - futures.add(writeConfigTxn.submit()); - LOG.info("update: vpn interface updated for interface {} oldVpn(s) {} newVpn {}" - + "processed successfully", update.getName(), - VpnHelper.getVpnInterfaceVpnInstanceNamesString(original.getVpnInstanceNames()), newVpnName); + })); + })); return futures; }); } else { @@ -2186,22 +2176,16 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase { - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); - WriteTransaction writeOperTxn = dataBroker.newWriteOnlyTransaction(); + // TODO Deal with sequencing — the config tx must only submitted if the oper tx goes in if (VpnUtil.isAdjacencyEligibleToVpn(dataBroker, adjacency, vpnName)) { - addNewAdjToVpnInterface(existingVpnInterfaceId, primaryRd, adjacency, - vpnInterfaceOptional.get().getDpnId(), writeConfigTxn, writeOperTxn); - ListenableFuture operFuture = writeOperTxn.submit(); - try { - operFuture.get(); - } catch (ExecutionException | InterruptedException e) { - LOG.error("Exception encountered while submitting operational" - + " future for vpnInterface {}", vpnInterface, e); - } List> futures = new ArrayList<>(); - futures.add(writeConfigTxn.submit()); + futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit(operTx -> futures.add( + txRunner.callWithNewWriteOnlyTransactionAndSubmit( + confTx -> addNewAdjToVpnInterface(existingVpnInterfaceId, primaryRd, + adjacency, vpnInterfaceOptional.get().getDpnId(), confTx, + operTx))))); return futures; - } else { + } else { return Collections.emptyList(); } }); diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceOpListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceOpListener.java index 97bf738efd..b85871cf04 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceOpListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceOpListener.java @@ -8,8 +8,8 @@ package org.opendaylight.netvirt.vpnmanager; import com.google.common.base.Optional; -import com.google.common.util.concurrent.ListenableFuture; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -20,7 +20,10 @@ import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.infrautils.jobcoordinator.JobCoordinator; +import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.AdjacenciesOp; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.VpnInterfaceOpData; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.adjacency.list.Adjacency; @@ -38,6 +41,7 @@ public class VpnInterfaceOpListener extends AsyncDataTreeChangeListenerBase { private static final Logger LOG = LoggerFactory.getLogger(VpnInterfaceOpListener.class); private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; private final VpnInterfaceManager vpnInterfaceManager; private final VpnFootprintService vpnFootprintService; private final JobCoordinator jobCoordinator; @@ -53,6 +57,7 @@ public class VpnInterfaceOpListener extends AsyncDataTreeChangeListenerBase { - WriteTransaction writeOperTxn = dataBroker.newWriteOnlyTransaction(); - postProcessVpnInterfaceRemoval(identifier, del, writeOperTxn); - List> futures = new ArrayList<>(); - futures.add(writeOperTxn.submit()); + () -> Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> { + postProcessVpnInterfaceRemoval(identifier, del, tx); LOG.info("remove: Removed vpn operational data for interface {} on dpn {} vpn {}", del.getName(), del.getDpnId(), del.getVpnInstanceName()); - return futures; - }); + }))); } private void postProcessVpnInterfaceRemoval(InstanceIdentifier identifier, VpnInterfaceOpDataEntry del, WriteTransaction writeOperTxn) { + if (writeOperTxn == null) { + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + postProcessVpnInterfaceRemoval(identifier, del, tx)), LOG, + "Error post-processing VPN interface removal"); + return; + } final VpnInterfaceOpDataEntryKey key = identifier.firstKeyOf(VpnInterfaceOpDataEntry.class, VpnInterfaceOpDataEntryKey.class); String interfaceName = key.getName(); @@ -109,8 +116,7 @@ public class VpnInterfaceOpListener extends AsyncDataTreeChangeListenerBase + addExtraRoute(vpnName, destination, finalNextHop, rd, routerID, l3vni, origin, intfName, operationalAdj, + encapType, tx)), + LOG, "Error adding extra route"); + return; } //add extra route to vpn mapping; advertise with nexthop as tunnel ip @@ -254,10 +258,6 @@ public class VpnManagerImpl implements IVpnManager { } } } - - if (!writeConfigTxnPresent) { - writeConfigTxn.submit(); - } } @Override @@ -269,12 +269,13 @@ public class VpnManagerImpl implements IVpnManager { @Override public void delExtraRoute(String vpnName, String destination, String nextHop, String rd, String routerID, String intfName, WriteTransaction writeConfigTxn) { - Boolean writeConfigTxnPresent = true; - BigInteger dpnId = null; if (writeConfigTxn == null) { - writeConfigTxnPresent = false; - writeConfigTxn = dataBroker.newWriteOnlyTransaction(); + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + delExtraRoute(vpnName, destination, nextHop, rd, routerID, intfName, tx)), + LOG, "Error deleting extra route"); + return; } + BigInteger dpnId = null; String tunnelIp = nextHop; if (intfName != null && !intfName.isEmpty()) { dpnId = InterfaceUtils.getDpnForInterface(ifaceMgrRpcService, intfName); @@ -296,9 +297,6 @@ public class VpnManagerImpl implements IVpnManager { LOG.info("delExtraRoute: Removed extra route {} from interface {} for rd {}", destination, intfName, routerID); } - if (!writeConfigTxnPresent) { - writeConfigTxn.submit(); - } } // TODO Clean up the exception handling @@ -356,13 +354,18 @@ public class VpnManagerImpl implements IVpnManager { @Override public boolean isVPNConfigured() { InstanceIdentifier vpnsIdentifier = InstanceIdentifier.builder(VpnInstances.class).build(); - Optional optionalVpns = TransactionUtil.read(dataBroker, LogicalDatastoreType.CONFIGURATION, - vpnsIdentifier); - if (!optionalVpns.isPresent() - || optionalVpns.get().getVpnInstance() == null - || optionalVpns.get().getVpnInstance().isEmpty()) { - LOG.trace("isVPNConfigured: No VPNs configured."); - return false; + try { + Optional optionalVpns = + SingleTransactionDataBroker.syncReadOptional(dataBroker, LogicalDatastoreType.CONFIGURATION, + vpnsIdentifier); + if (!optionalVpns.isPresent() + || optionalVpns.get().getVpnInstance() == null + || optionalVpns.get().getVpnInstance().isEmpty()) { + LOG.trace("isVPNConfigured: No VPNs configured."); + return false; + } + } catch (ReadFailedException e) { + throw new RuntimeException("Error reading VPN " + vpnsIdentifier, e); } LOG.trace("isVPNConfigured: VPNs are configured on the system."); return true; diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnNodeListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnNodeListener.java index 6b133df8f1..fd416527a2 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnNodeListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnNodeListener.java @@ -20,6 +20,8 @@ import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.genius.datastoreutils.AsyncClusteredDataTreeChangeListenerBase; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.mdsalutil.ActionInfo; import org.opendaylight.genius.mdsalutil.FlowEntity; import org.opendaylight.genius.mdsalutil.InstructionInfo; @@ -48,6 +50,7 @@ public class VpnNodeListener extends AsyncClusteredDataTreeChangeListenerBase connectedDpnIds; @@ -56,6 +59,7 @@ public class VpnNodeListener extends AsyncClusteredDataTreeChangeListenerBase(); @@ -101,17 +105,15 @@ public class VpnNodeListener extends AsyncClusteredDataTreeChangeListenerBase { - WriteTransaction writeFlowTx = broker.newWriteOnlyTransaction(); + () -> Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> { LOG.debug("Received notification to install TableMiss entries for dpn {} ", dpId); - makeTableMissFlow(writeFlowTx, dpId, NwConstants.ADD_FLOW); - makeL3IntfTblMissFlow(writeFlowTx, dpId, NwConstants.ADD_FLOW); - makeSubnetRouteTableMissFlow(writeFlowTx, dpId, NwConstants.ADD_FLOW); - createTableMissForVpnGwFlow(writeFlowTx, dpId); - createL3GwMacArpFlows(writeFlowTx, dpId); - programTableMissForVpnVniDemuxTable(writeFlowTx, dpId, NwConstants.ADD_FLOW); - return Collections.singletonList(writeFlowTx.submit()); - }, 3); + makeTableMissFlow(tx, dpId, NwConstants.ADD_FLOW); + makeL3IntfTblMissFlow(tx, dpId, NwConstants.ADD_FLOW); + makeSubnetRouteTableMissFlow(tx, dpId, NwConstants.ADD_FLOW); + createTableMissForVpnGwFlow(tx, dpId); + createL3GwMacArpFlows(tx, dpId); + programTableMissForVpnVniDemuxTable(tx, dpId, NwConstants.ADD_FLOW); + })), 3); } private void makeTableMissFlow(WriteTransaction writeFlowTx, BigInteger dpnId, int addOrRemove) { diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnOpStatusListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnOpStatusListener.java index 031f5d64b7..c6c2cf4e09 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnOpStatusListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnOpStatusListener.java @@ -8,22 +8,21 @@ package org.opendaylight.netvirt.vpnmanager; import com.google.common.base.Optional; -import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.concurrent.ExecutionException; import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.controller.md.sal.binding.api.DataBroker; -import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; -import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager; import org.opendaylight.genius.utils.SystemPropertyReader; import org.opendaylight.infrautils.jobcoordinator.JobCoordinator; @@ -48,6 +47,7 @@ import org.slf4j.LoggerFactory; public class VpnOpStatusListener extends AsyncDataTreeChangeListenerBase { private static final Logger LOG = LoggerFactory.getLogger(VpnOpStatusListener.class); private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; private final IBgpManager bgpManager; private final IdManagerService idManager; private final IFibManager fibManager; @@ -62,6 +62,7 @@ public class VpnOpStatusListener extends AsyncDataTreeChangeListenerBase { - WriteTransaction writeConfigTxn = dataBroker.newWriteOnlyTransaction(); - WriteTransaction writeOperTxn = dataBroker.newWriteOnlyTransaction(); - // Clean up VpnInstanceToVpnId from Config DS - VpnUtil.removeVpnIdToVpnInstance(dataBroker, vpnId, writeConfigTxn); - VpnUtil.removeVpnInstanceToVpnId(dataBroker, vpnName, writeConfigTxn); - LOG.trace("Removed vpnIdentifier for rd{} vpnname {}", primaryRd, vpnName); - - // Clean up FIB Entries Config DS - synchronized (vpnName.intern()) { - fibManager.removeVrfTable(primaryRd, null); - } - - // Clean up VPNExtraRoutes Operational DS - if (VpnUtil.isBgpVpn(vpnName, primaryRd)) { - if (update.getType() == VpnInstanceOpDataEntry.Type.L2) { - rds.parallelStream().forEach(rd -> bgpManager.deleteVrf(rd, false, AddressFamily.L2VPN)); + // Two transactions are used, one for operational, one for config; we only submit the config + // transaction if the operational transaction succeeds + ListenableFuture operationalFuture = txRunner.callWithNewWriteOnlyTransactionAndSubmit(operTx -> { + // Clean up VPNExtraRoutes Operational DS + if (VpnUtil.isBgpVpn(vpnName, primaryRd)) { + if (update.getType() == VpnInstanceOpDataEntry.Type.L2) { + rds.parallelStream().forEach(rd -> bgpManager.deleteVrf(rd, false, AddressFamily.L2VPN)); + } + if (update.isIpv4Configured()) { + rds.parallelStream().forEach(rd -> bgpManager.deleteVrf(rd, false, AddressFamily.IPV4)); + } + if (update.isIpv6Configured()) { + rds.parallelStream().forEach(rd -> bgpManager.deleteVrf(rd, false, AddressFamily.IPV6)); + } } - if (update.isIpv4Configured()) { - rds.parallelStream().forEach(rd -> bgpManager.deleteVrf(rd, false, AddressFamily.IPV4)); + InstanceIdentifier vpnToExtraroute = + VpnExtraRouteHelper.getVpnToExtrarouteVpnIdentifier(vpnName); + Optional optVpnToExtraroute = VpnUtil.read(dataBroker, + LogicalDatastoreType.OPERATIONAL, vpnToExtraroute); + if (optVpnToExtraroute.isPresent()) { + VpnUtil.removeVpnExtraRouteForVpn(vpnName, operTx); } - if (update.isIpv6Configured()) { - rds.parallelStream().forEach(rd -> bgpManager.deleteVrf(rd, false, AddressFamily.IPV6)); + + if (VpnUtil.isL3VpnOverVxLan(update.getL3vni())) { + VpnUtil.removeExternalTunnelDemuxFlows(vpnName, dataBroker, mdsalManager); } - } - InstanceIdentifier vpnToExtraroute = VpnExtraRouteHelper.getVpnToExtrarouteVpnIdentifier(vpnName); - Optional optVpnToExtraroute = VpnUtil.read(dataBroker, - LogicalDatastoreType.OPERATIONAL, vpnToExtraroute); - if (optVpnToExtraroute.isPresent()) { - VpnUtil.removeVpnExtraRouteForVpn(dataBroker, vpnName, writeOperTxn); - } - if (VpnUtil.isL3VpnOverVxLan(update.getL3vni())) { - VpnUtil.removeExternalTunnelDemuxFlows(vpnName, dataBroker, mdsalManager); - } + // Clean up PrefixToInterface Operational DS + Optional optPrefixToIntf = VpnUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL, + VpnUtil.getPrefixToInterfaceIdentifier(vpnId)); + if (optPrefixToIntf.isPresent()) { + VpnUtil.removePrefixToInterfaceForVpnId(vpnId, operTx); + } - // Clean up PrefixToInterface Operational DS - Optional optPrefixToIntf = VpnUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL, - VpnUtil.getPrefixToInterfaceIdentifier(vpnId)); - if (optPrefixToIntf.isPresent()) { - VpnUtil.removePrefixToInterfaceForVpnId(dataBroker, vpnId, writeOperTxn); - } + // Clean up L3NextHop Operational DS + InstanceIdentifier vpnNextHops = InstanceIdentifier.builder(L3nexthop.class).child( + VpnNexthops.class, new VpnNexthopsKey(vpnId)).build(); + Optional optL3nexthopForVpnId = VpnUtil.read(dataBroker, + LogicalDatastoreType.OPERATIONAL, + vpnNextHops); + if (optL3nexthopForVpnId.isPresent()) { + VpnUtil.removeL3nexthopForVpnId(vpnId, operTx); + } - // Clean up L3NextHop Operational DS - InstanceIdentifier vpnNextHops = InstanceIdentifier.builder(L3nexthop.class).child( - VpnNexthops.class, new VpnNexthopsKey(vpnId)).build(); - Optional optL3nexthopForVpnId = VpnUtil.read(dataBroker, - LogicalDatastoreType.OPERATIONAL, - vpnNextHops); - if (optL3nexthopForVpnId.isPresent()) { - VpnUtil.removeL3nexthopForVpnId(dataBroker, vpnId, writeOperTxn); - } + // Clean up VPNInstanceOpDataEntry + VpnUtil.removeVpnOpInstance(primaryRd, operTx); + }); - // Clean up VPNInstanceOpDataEntry - VpnUtil.removeVpnOpInstance(dataBroker, primaryRd, writeOperTxn); + Futures.addCallback(operationalFuture, new FutureCallback() { + @Override + public void onSuccess(Void result) { + Futures.addCallback(txRunner.callWithNewWriteOnlyTransactionAndSubmit(confTx -> { + // Clean up VpnInstanceToVpnId from Config DS + VpnUtil.removeVpnIdToVpnInstance(vpnId, confTx); + VpnUtil.removeVpnInstanceToVpnId(vpnName, confTx); + LOG.trace("Removed vpnIdentifier for rd{} vpnname {}", primaryRd, vpnName); - // Note: Release the of VpnId will happen in PostDeleteVpnInstancWorker only if - // operationalTxn/Config succeeds. + // Clean up FIB Entries Config DS + synchronized (vpnName.intern()) { + fibManager.removeVrfTable(primaryRd, confTx); + } + }), new VpnOpStatusListener.PostDeleteVpnInstanceWorker(vpnName), + MoreExecutors.directExecutor()); + // Note: Release the of VpnId will happen in PostDeleteVpnInstancWorker only if + // operationalTxn/Config succeeds. + } + + @Override + public void onFailure(Throwable throwable) { + LOG.error("Error deleting VPN {}", vpnName, throwable); + } + }, MoreExecutors.directExecutor()); - CheckedFuture checkFutures = writeOperTxn.submit(); - try { - checkFutures.get(); - } catch (InterruptedException | ExecutionException e) { - LOG.error("Error deleting vpn {} ", vpnName); - writeConfigTxn.cancel(); - throw new RuntimeException(e); - } - List> futures = new ArrayList<>(); - futures.add(writeConfigTxn.submit()); - ListenableFuture> listenableFuture = Futures.allAsList(futures); - Futures.addCallback(listenableFuture, new VpnOpStatusListener.PostDeleteVpnInstanceWorker(vpnName)); LOG.info("Removed vpn data for vpnname {}", vpnName); - return futures; + return Collections.singletonList(operationalFuture); }, SystemPropertyReader.getDataStoreJobCoordinatorMaxRetries()); } else if (update.getVpnState() == VpnInstanceOpDataEntry.VpnState.Created) { final String vpnName = update.getVpnInstanceName(); @@ -216,8 +220,7 @@ public class VpnOpStatusListener extends AsyncDataTreeChangeListenerBase { - WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction(); - long primaryRdAddFailed = rds.parallelStream().filter(rd -> { + rds.parallelStream().forEach(rd -> { try { LOG.info("VpnOpStatusListener.update: updating BGPVPN for vpn {} with RD {}" + " Type is {}, IPv4 is {}, IPv6 is {}", vpnName, primaryRd, update.getType(), @@ -240,10 +243,8 @@ public class VpnOpStatusListener extends AsyncDataTreeChangeListenerBase> { + private class PostDeleteVpnInstanceWorker implements FutureCallback { private final Logger log = LoggerFactory.getLogger(VpnOpStatusListener.PostDeleteVpnInstanceWorker.class); String vpnName; @@ -268,7 +269,7 @@ public class VpnOpStatusListener extends AsyncDataTreeChangeListenerBase voids) { + public void onSuccess(Void ignored) { VpnUtil.releaseId(idManager, VpnConstants.VPN_IDPOOL_NAME, vpnName); log.info("onSuccess: VpnId for VpnName {} is released to IdManager successfully.", vpnName); } diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnUtil.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnUtil.java index bb94820554..51b4352572 100755 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnUtil.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnUtil.java @@ -11,9 +11,7 @@ package org.opendaylight.netvirt.vpnmanager; import com.google.common.base.Optional; import com.google.common.collect.Iterators; import com.google.common.util.concurrent.FutureCallback; -import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.MoreExecutors; import java.math.BigInteger; import java.net.Inet4Address; import java.net.Inet6Address; @@ -41,7 +39,10 @@ import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; +import org.opendaylight.genius.datastoreutils.SingleTransactionDataBroker; import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.interfacemanager.interfaces.IInterfaceManager; import org.opendaylight.genius.mdsalutil.FlowEntity; import org.opendaylight.genius.mdsalutil.FlowEntityBuilder; @@ -60,6 +61,7 @@ import org.opendaylight.genius.mdsalutil.matches.MatchMetadata; import org.opendaylight.genius.utils.ServiceIndex; import org.opendaylight.genius.utils.SystemPropertyReader; import org.opendaylight.infrautils.jobcoordinator.JobCoordinator; +import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.netvirt.bgpmanager.api.IBgpManager; import org.opendaylight.netvirt.elanmanager.api.ElanHelper; import org.opendaylight.netvirt.fibmanager.api.FibHelper; @@ -225,7 +227,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.subnets.rev150712.s import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.common.RpcResult; -import org.opendaylight.yangtools.yang.data.impl.schema.tree.SchemaValidationFailedException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -632,14 +633,15 @@ public final class VpnUtil { Optional vrfTablesOpc = read(broker, LogicalDatastoreType.CONFIGURATION, vpnVrfTableIid); if (vrfTablesOpc.isPresent()) { VrfTables vrfTables = vrfTablesOpc.get(); - WriteTransaction tx = broker.newWriteOnlyTransaction(); - for (VrfEntry vrfEntry : vrfTables.getVrfEntry()) { - if (origin == RouteOrigin.value(vrfEntry.getOrigin())) { - tx.delete(LogicalDatastoreType.CONFIGURATION, - vpnVrfTableIid.child(VrfEntry.class, vrfEntry.getKey())); - } - } - tx.submit(); + ListenableFutures.addErrorLogging( + new ManagedNewTransactionRunnerImpl(broker).callWithNewWriteOnlyTransactionAndSubmit(tx -> { + for (VrfEntry vrfEntry : vrfTables.getVrfEntry()) { + if (origin == RouteOrigin.value(vrfEntry.getOrigin())) { + tx.delete(LogicalDatastoreType.CONFIGURATION, + vpnVrfTableIid.child(VrfEntry.class, vrfEntry.getKey())); + } + } + }), LOG, "Error removing VRF entries by origin"); } } @@ -664,11 +666,13 @@ public final class VpnUtil { public static void removeVrfEntries(DataBroker broker, String rd, List vrfEntries) { InstanceIdentifier vpnVrfTableIid = InstanceIdentifier.builder(FibEntries.class).child(VrfTables.class, new VrfTablesKey(rd)).build(); - WriteTransaction tx = broker.newWriteOnlyTransaction(); - for (VrfEntry vrfEntry : vrfEntries) { - tx.delete(LogicalDatastoreType.CONFIGURATION, vpnVrfTableIid.child(VrfEntry.class, vrfEntry.getKey())); - } - tx.submit(); + ListenableFutures.addErrorLogging( + new ManagedNewTransactionRunnerImpl(broker).callWithNewWriteOnlyTransactionAndSubmit(tx -> { + for (VrfEntry vrfEntry : vrfEntries) { + tx.delete(LogicalDatastoreType.CONFIGURATION, + vpnVrfTableIid.child(VrfEntry.class, vrfEntry.getKey())); + } + }), LOG, "Error removing VRF entries"); } // TODO Clean up the exception handling @@ -848,6 +852,7 @@ public final class VpnUtil { }; + @Deprecated public static Optional read(DataBroker broker, LogicalDatastoreType datastoreType, InstanceIdentifier path) { try (ReadOnlyTransaction tx = broker.newReadOnlyTransaction()) { @@ -857,78 +862,24 @@ public final class VpnUtil { } } - public static void asyncUpdate(DataBroker broker, LogicalDatastoreType datastoreType, - InstanceIdentifier path, T data) { - asyncUpdate(broker, datastoreType, path, data, DEFAULT_CALLBACK); - } - - public static void asyncUpdate(DataBroker broker, LogicalDatastoreType datastoreType, - InstanceIdentifier path, T data, FutureCallback callback) { - WriteTransaction tx = broker.newWriteOnlyTransaction(); - tx.merge(datastoreType, path, data, true); - Futures.addCallback(tx.submit(), callback, MoreExecutors.directExecutor()); - } - - public static void asyncWrite(DataBroker broker, LogicalDatastoreType datastoreType, - InstanceIdentifier path, T data) { - asyncWrite(broker, datastoreType, path, data, DEFAULT_CALLBACK); - } - - public static void asyncWrite(DataBroker broker, LogicalDatastoreType datastoreType, - InstanceIdentifier path, T data, FutureCallback callback) { - WriteTransaction tx = broker.newWriteOnlyTransaction(); - tx.put(datastoreType, path, data, WriteTransaction.CREATE_MISSING_PARENTS); - Futures.addCallback(tx.submit(), callback, MoreExecutors.directExecutor()); - } - - // TODO Clean up the exception handling - @SuppressWarnings("checkstyle:IllegalCatch") - public static void tryDelete(DataBroker broker, LogicalDatastoreType datastoreType, - InstanceIdentifier path) { - try { - delete(broker, datastoreType, path, DEFAULT_CALLBACK); - } catch (SchemaValidationFailedException sve) { - LOG.info("tryDelete: Could not delete {}. SchemaValidationFailedException: {}", path, sve.getMessage()); - } catch (Exception e) { - LOG.info("tryDelete: Could not delete {}. Unhandled error: {}", path, e.getMessage()); - } - } - - public static void delete(DataBroker broker, LogicalDatastoreType datastoreType, - InstanceIdentifier path) { - delete(broker, datastoreType, path, DEFAULT_CALLBACK); - } - - - public static void delete(DataBroker broker, LogicalDatastoreType datastoreType, - InstanceIdentifier path, FutureCallback callback) { - WriteTransaction tx = broker.newWriteOnlyTransaction(); - tx.delete(datastoreType, path); - Futures.addCallback(tx.submit(), callback, MoreExecutors.directExecutor()); - } - + @Deprecated public static void syncWrite(DataBroker broker, LogicalDatastoreType datastoreType, InstanceIdentifier path, T data) { - WriteTransaction tx = broker.newWriteOnlyTransaction(); - tx.put(datastoreType, path, data, WriteTransaction.CREATE_MISSING_PARENTS); - try { - tx.submit().get(); - } catch (InterruptedException | ExecutionException e) { - LOG.error("syncWrite: Error writing to datastore (path, data) : ({}, {})", path, data); + SingleTransactionDataBroker.syncWrite(broker, datastoreType, path, data); + } catch (TransactionCommitFailedException e) { + LOG.error("syncWrite: Error writing to datastore (path, data) : ({}, {})", path, data, e); throw new RuntimeException(e.getMessage(), e); } } + @Deprecated public static void syncUpdate(DataBroker broker, LogicalDatastoreType datastoreType, InstanceIdentifier path, T data) { - WriteTransaction tx = broker.newWriteOnlyTransaction(); - tx.merge(datastoreType, path, data, true); - try { - tx.submit().get(); - } catch (InterruptedException | ExecutionException e) { - LOG.error("syncUpdate: Error writing to datastore (path, data) : ({}, {})", path, data); + SingleTransactionDataBroker.syncUpdate(broker, datastoreType, path, data); + } catch (TransactionCommitFailedException e) { + LOG.error("syncUpdate: Error writing to datastore (path, data) : ({}, {})", path, data, e); throw new RuntimeException(e.getMessage(), e); } } @@ -990,138 +941,37 @@ public final class VpnUtil { .build(); } - // TODO Clean up the exception handling - @SuppressWarnings("checkstyle:IllegalCatch") - public static void removePrefixToInterfaceForVpnId(DataBroker broker, long vpnId, WriteTransaction writeTxn) { - try { - // Clean up PrefixToInterface Operational DS - if (writeTxn != null) { - writeTxn.delete(LogicalDatastoreType.OPERATIONAL, - InstanceIdentifier.builder(PrefixToInterface.class).child( - VpnIds.class, new VpnIdsKey(vpnId)).build()); - } else { - delete(broker, LogicalDatastoreType.OPERATIONAL, - InstanceIdentifier.builder(PrefixToInterface.class).child(VpnIds.class, - new VpnIdsKey(vpnId)).build(), - DEFAULT_CALLBACK); - } - } catch (Exception e) { - LOG.error("removePrefixToInterfaceForVpnId: Exception during cleanup of PrefixToInterface for VPN ID {}", - vpnId, e); - } - } - - // TODO Clean up the exception handling - @SuppressWarnings("checkstyle:IllegalCatch") - public static void removeVpnExtraRouteForVpn(DataBroker broker, String vpnName, WriteTransaction writeTxn) { - try { - // Clean up VPNExtraRoutes Operational DS - if (writeTxn != null) { - writeTxn.delete(LogicalDatastoreType.OPERATIONAL, - InstanceIdentifier.builder(VpnToExtraroutes.class) - .child(Vpn.class, new VpnKey(vpnName)).build()); - } else { - delete(broker, LogicalDatastoreType.OPERATIONAL, - InstanceIdentifier.builder(VpnToExtraroutes.class) - .child(Vpn.class, new VpnKey(vpnName)).build(), - DEFAULT_CALLBACK); - } - } catch (Exception e) { - LOG.error("removeVpnExtraRouteForVpna: Exception during cleanup of VPNToExtraRoute for VPN {}", - vpnName, e); - } + public static void removePrefixToInterfaceForVpnId(long vpnId, @Nonnull WriteTransaction operTx) { + // Clean up PrefixToInterface Operational DS + operTx.delete(LogicalDatastoreType.OPERATIONAL, + InstanceIdentifier.builder(PrefixToInterface.class).child(VpnIds.class, new VpnIdsKey(vpnId)).build()); } - // TODO Clean up the exception handling - @SuppressWarnings("checkstyle:IllegalCatch") - public static void removeVpnOpInstance(DataBroker broker, String vpnName, WriteTransaction writeTxn) { - try { - // Clean up VPNInstanceOpDataEntry - if (writeTxn != null) { - writeTxn.delete(LogicalDatastoreType.OPERATIONAL, getVpnInstanceOpDataIdentifier(vpnName)); - } else { - delete(broker, LogicalDatastoreType.OPERATIONAL, getVpnInstanceOpDataIdentifier(vpnName), - DEFAULT_CALLBACK); - } - } catch (Exception e) { - LOG.error("removeVpnOpInstance: Exception during cleanup of VPNInstanceOpDataEntry for VPN {}", - vpnName, e); - } + public static void removeVpnExtraRouteForVpn(String vpnName, @Nonnull WriteTransaction operTx) { + // Clean up VPNExtraRoutes Operational DS + operTx.delete(LogicalDatastoreType.OPERATIONAL, + InstanceIdentifier.builder(VpnToExtraroutes.class).child(Vpn.class, new VpnKey(vpnName)).build()); } // TODO Clean up the exception handling @SuppressWarnings("checkstyle:IllegalCatch") - public static void removeVpnInstanceToVpnId(DataBroker broker, String vpnName, WriteTransaction writeTxn) { - try { - if (writeTxn != null) { - writeTxn.delete(LogicalDatastoreType.CONFIGURATION, - VpnOperDsUtils.getVpnInstanceToVpnIdIdentifier(vpnName)); - } else { - delete(broker, LogicalDatastoreType.CONFIGURATION, - VpnOperDsUtils.getVpnInstanceToVpnIdIdentifier(vpnName), - DEFAULT_CALLBACK); - } - } catch (Exception e) { - LOG.error("removeVpnInstanceToVpnId: Exception during clean up of VpnInstanceToVpnId for VPN {}", - vpnName, e); - } + public static void removeVpnOpInstance(String vpnName, @Nonnull WriteTransaction operTx) { + // Clean up VPNInstanceOpDataEntry + operTx.delete(LogicalDatastoreType.OPERATIONAL, getVpnInstanceOpDataIdentifier(vpnName)); } - // TODO Clean up the exception handling - @SuppressWarnings("checkstyle:IllegalCatch") - public static void removeVpnIdToVpnInstance(DataBroker broker, long vpnId, WriteTransaction writeTxn) { - try { - if (writeTxn != null) { - writeTxn.delete(LogicalDatastoreType.CONFIGURATION, getVpnIdToVpnInstanceIdentifier(vpnId)); - } else { - delete(broker, LogicalDatastoreType.CONFIGURATION, getVpnIdToVpnInstanceIdentifier(vpnId), - DEFAULT_CALLBACK); - } - } catch (Exception e) { - LOG.error("removeVpnIdToVpnInstance: Exception during clean up of VpnIdToVpnInstance for VPNID {}", - vpnId, e); - } + public static void removeVpnInstanceToVpnId(String vpnName, @Nonnull WriteTransaction confTx) { + confTx.delete(LogicalDatastoreType.CONFIGURATION, VpnOperDsUtils.getVpnInstanceToVpnIdIdentifier(vpnName)); } - // TODO Clean up the exception handling - @SuppressWarnings("checkstyle:IllegalCatch") - public static void removeVrfTableForVpn(DataBroker broker, String vpnName, WriteTransaction writeTxn) { - // Clean up FIB Entries Config DS - try { - if (writeTxn != null) { - writeTxn.delete(LogicalDatastoreType.CONFIGURATION, - InstanceIdentifier.builder(FibEntries.class).child(VrfTables.class, - new VrfTablesKey(vpnName)).build()); - } else { - delete(broker, LogicalDatastoreType.CONFIGURATION, - InstanceIdentifier.builder(FibEntries.class).child(VrfTables.class, - new VrfTablesKey(vpnName)).build(), - DEFAULT_CALLBACK); - } - } catch (Exception e) { - LOG.error("removeVrfTableForVpn: Exception during clean up of VrfTable from FIB for VPN {}", - vpnName, e); - } + public static void removeVpnIdToVpnInstance(long vpnId, @Nonnull WriteTransaction confTx) { + confTx.delete(LogicalDatastoreType.CONFIGURATION, getVpnIdToVpnInstanceIdentifier(vpnId)); } - // TODO Clean up the exception handling - @SuppressWarnings("checkstyle:IllegalCatch") - public static void removeL3nexthopForVpnId(DataBroker broker, long vpnId, WriteTransaction writeTxn) { - try { - // Clean up L3NextHop Operational DS - if (writeTxn != null) { - writeTxn.delete(LogicalDatastoreType.OPERATIONAL, - InstanceIdentifier.builder(L3nexthop.class).child(VpnNexthops.class, - new VpnNexthopsKey(vpnId)).build()); - } else { - delete(broker, LogicalDatastoreType.OPERATIONAL, - InstanceIdentifier.builder(L3nexthop.class).child(VpnNexthops.class, - new VpnNexthopsKey(vpnId)).build(), - DEFAULT_CALLBACK); - } - } catch (Exception e) { - LOG.error("removeL3nexthopForVpnId: Exception during cleanup of L3NextHop for VPN ID {}", vpnId, e); - } + public static void removeL3nexthopForVpnId(long vpnId, @Nonnull WriteTransaction operTx) { + // Clean up L3NextHop Operational DS + operTx.delete(LogicalDatastoreType.OPERATIONAL, + InstanceIdentifier.builder(L3nexthop.class).child(VpnNexthops.class, new VpnNexthopsKey(vpnId)).build()); } public static void scheduleVpnInterfaceForRemoval(DataBroker broker,String interfaceName, BigInteger dpnId, @@ -1709,16 +1559,16 @@ public final class VpnUtil { static void bindService(final String vpnInstanceName, final String interfaceName, DataBroker dataBroker, boolean isTunnelInterface, JobCoordinator jobCoordinator) { jobCoordinator.enqueueJob(interfaceName, - () -> { - WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction(); - BoundServices serviceInfo = isTunnelInterface - ? VpnUtil.getBoundServicesForTunnelInterface(vpnInstanceName, interfaceName) - : getBoundServicesForVpnInterface(dataBroker, vpnInstanceName, interfaceName); - writeTxn.put(LogicalDatastoreType.CONFIGURATION, InterfaceUtils.buildServiceId(interfaceName, - ServiceIndex.getIndex(NwConstants.L3VPN_SERVICE_NAME, NwConstants.L3VPN_SERVICE_INDEX)), - serviceInfo, WriteTransaction.CREATE_MISSING_PARENTS); - return Collections.singletonList(writeTxn.submit()); - }, SystemPropertyReader.getDataStoreJobCoordinatorMaxRetries()); + () -> Collections.singletonList( + new ManagedNewTransactionRunnerImpl(dataBroker).callWithNewReadWriteTransactionAndSubmit(tx -> { + BoundServices serviceInfo = isTunnelInterface + ? VpnUtil.getBoundServicesForTunnelInterface(vpnInstanceName, interfaceName) + : getBoundServicesForVpnInterface(dataBroker, vpnInstanceName, interfaceName); + tx.put(LogicalDatastoreType.CONFIGURATION, InterfaceUtils.buildServiceId(interfaceName, + ServiceIndex.getIndex(NwConstants.L3VPN_SERVICE_NAME, + NwConstants.L3VPN_SERVICE_INDEX)), + serviceInfo, WriteTransaction.CREATE_MISSING_PARENTS); + })), SystemPropertyReader.getDataStoreJobCoordinatorMaxRetries()); } static BoundServices getBoundServicesForVpnInterface(DataBroker broker, String vpnName, String interfaceName) { @@ -1757,17 +1607,13 @@ public final class VpnUtil { JobCoordinator jobCoordinator) { if (!isInterfaceStateDown) { jobCoordinator.enqueueJob(vpnInterfaceName, - () -> { - WriteTransaction writeTxn = dataBroker.newWriteOnlyTransaction(); - writeTxn.delete(LogicalDatastoreType.CONFIGURATION, - InterfaceUtils.buildServiceId(vpnInterfaceName, - ServiceIndex.getIndex(NwConstants.L3VPN_SERVICE_NAME, - NwConstants.L3VPN_SERVICE_INDEX))); - - List> futures = new ArrayList<>(); - futures.add(writeTxn.submit()); - return futures; - }, SystemPropertyReader.getDataStoreJobCoordinatorMaxRetries()); + () -> Collections.singletonList(new ManagedNewTransactionRunnerImpl( + dataBroker).callWithNewWriteOnlyTransactionAndSubmit(tx -> + tx.delete(LogicalDatastoreType.CONFIGURATION, + InterfaceUtils.buildServiceId(vpnInterfaceName, + ServiceIndex.getIndex(NwConstants.L3VPN_SERVICE_NAME, + NwConstants.L3VPN_SERVICE_INDEX))))), + SystemPropertyReader.getDataStoreJobCoordinatorMaxRetries()); } } @@ -2385,6 +2231,14 @@ public final class VpnUtil { public static void removePrefixToInterfaceAdj(DataBroker dataBroker, Adjacency adj, long vpnId, VpnInterfaceOpDataEntry vpnInterfaceOpDataEntry, WriteTransaction writeOperTxn) { + if (writeOperTxn == null) { + ListenableFutures.addErrorLogging( + new ManagedNewTransactionRunnerImpl(dataBroker).callWithNewWriteOnlyTransactionAndSubmit(tx -> + removePrefixToInterfaceAdj(dataBroker, adj, vpnId, vpnInterfaceOpDataEntry, tx)), LOG, + "Error removing prefix"); + return; + } + Optional prefix = VpnUtil.read(dataBroker, LogicalDatastoreType.OPERATIONAL, VpnUtil.getPrefixToInterfaceIdentifier(vpnId, VpnUtil.getIpPrefix(adj.getIpAddress()))); @@ -2408,14 +2262,8 @@ public final class VpnUtil { } for (Prefixes pref : prefixToInterface) { if (VpnUtil.isMatchedPrefixToInterface(pref, vpnInterfaceOpDataEntry)) { - if (writeOperTxn != null) { - writeOperTxn.delete(LogicalDatastoreType.OPERATIONAL, - VpnUtil.getPrefixToInterfaceIdentifier(vpnId, pref.getIpAddress())); - } else { - VpnUtil.delete(dataBroker, LogicalDatastoreType.OPERATIONAL, - VpnUtil.getPrefixToInterfaceIdentifier(vpnId, pref.getIpAddress()), - VpnUtil.DEFAULT_CALLBACK); - } + writeOperTxn.delete(LogicalDatastoreType.OPERATIONAL, + VpnUtil.getPrefixToInterfaceIdentifier(vpnId, pref.getIpAddress())); } } } diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/IVpnLinkServiceImpl.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/IVpnLinkServiceImpl.java index 1f388bb663..8256b715f0 100755 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/IVpnLinkServiceImpl.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/IVpnLinkServiceImpl.java @@ -23,8 +23,11 @@ import javax.inject.Inject; import javax.inject.Singleton; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.mdsalutil.MDSALUtil; import org.opendaylight.genius.mdsalutil.NwConstants; +import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.netvirt.bgpmanager.api.IBgpManager; import org.opendaylight.netvirt.fibmanager.api.FibHelper; import org.opendaylight.netvirt.fibmanager.api.IFibManager; @@ -59,6 +62,7 @@ public class IVpnLinkServiceImpl implements IVpnLinkService, AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(IVpnLinkServiceImpl.class); private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; private final IdManagerService idManager; private final IBgpManager bgpManager; private final IFibManager fibManager; @@ -68,6 +72,7 @@ public class IVpnLinkServiceImpl implements IVpnLinkService, AutoCloseable { public IVpnLinkServiceImpl(final DataBroker dataBroker, final IdManagerService idMgr, final IBgpManager bgpMgr, final IFibManager fibMgr, final InterVpnLinkCache interVpnLinkCache) { this.dataBroker = dataBroker; + this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker); this.idManager = idMgr; this.bgpManager = bgpMgr; this.fibManager = fibMgr; @@ -172,7 +177,9 @@ public class IVpnLinkServiceImpl implements IVpnLinkService, AutoCloseable { .child(VrfTables.class, new VrfTablesKey(dstVpnRd)) .child(VrfEntry.class, new VrfEntryKey(newVrfEntry.getDestPrefix())) .build(); - VpnUtil.asyncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION, newVrfEntryIid, newVrfEntry); + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + tx.put(LogicalDatastoreType.CONFIGURATION, newVrfEntryIid, newVrfEntry)), + LOG, "Error adding VRF entry {}", newVrfEntry); // Finally, route is advertised it to the DC-GW. But while in the FibEntries the nexthop is the other // endpoint's IP, in the DC-GW the nexthop for those prefixes are the IPs of those DPNs where the target diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkListener.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkListener.java index c78b5a03e1..0034b61564 100755 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkListener.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkListener.java @@ -26,11 +26,14 @@ import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.mdsalutil.MDSALUtil; import org.opendaylight.genius.mdsalutil.NwConstants; import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager; import org.opendaylight.infrautils.jobcoordinator.JobCoordinator; import org.opendaylight.infrautils.utils.concurrent.JdkFutures; +import org.opendaylight.infrautils.utils.concurrent.ListenableFutures; import org.opendaylight.netvirt.bgpmanager.api.IBgpManager; import org.opendaylight.netvirt.fibmanager.api.IFibManager; import org.opendaylight.netvirt.fibmanager.api.RouteOrigin; @@ -82,6 +85,7 @@ public class InterVpnLinkListener extends AsyncDataTreeChangeListenerBase interVpnLinkStateIid = InterVpnLinkUtil.getInterVpnLinkStateIid(del.getName()); - VpnUtil.delete(dataBroker, LogicalDatastoreType.CONFIGURATION, interVpnLinkStateIid); + ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + tx.delete(LogicalDatastoreType.CONFIGURATION, interVpnLinkStateIid)), LOG, + "Error deleting inter-VPN link state {}", interVpnLinkStateIid); } // We're catching Exception here to continue deleting as much as possible @@ -451,10 +458,10 @@ public class InterVpnLinkListener extends AsyncDataTreeChangeListenerBase + tx.put(LogicalDatastoreType.CONFIGURATION, vpnLinkStateIid, vpnLinkErrorState, + WriteTransaction.CREATE_MISSING_PARENTS)), + LOG, "Error storing the VPN link error state for {}, {}", vpnLinkStateIid, vpnLinkErrorState); // Sending out an error Notification InterVpnLinkCreationErrorMessage errMsg = diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkNodeAddTask.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkNodeAddTask.java index 07bed23d5e..fc0eb84739 100755 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkNodeAddTask.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkNodeAddTask.java @@ -17,6 +17,8 @@ import java.util.concurrent.Callable; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager; import org.opendaylight.netvirt.vpnmanager.VpnFootprintService; import org.opendaylight.netvirt.vpnmanager.VpnUtil; @@ -42,6 +44,7 @@ public class InterVpnLinkNodeAddTask implements Callable + tx.merge(LogicalDatastoreType.CONFIGURATION, + InterVpnLinkUtil.getInterVpnLinkStateIid(interVpnLinkState.getInterVpnLinkName()), + newInterVpnLinkState, WriteTransaction.CREATE_MISSING_PARENTS)); } private void installLPortDispatcherTable(InterVpnLinkState interVpnLinkState, List firstDpnList, diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkUtil.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkUtil.java index 80b11e6755..0c9635da7a 100755 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkUtil.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/InterVpnLinkUtil.java @@ -8,7 +8,6 @@ package org.opendaylight.netvirt.vpnmanager.intervpnlink; import com.google.common.base.Optional; -import com.google.common.base.Preconditions; import com.google.common.util.concurrent.ListenableFuture; import java.math.BigInteger; import java.util.ArrayList; @@ -25,7 +24,6 @@ import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager; import org.opendaylight.genius.mdsalutil.matches.MatchMetadata; import org.opendaylight.genius.utils.ServiceIndex; import org.opendaylight.netvirt.bgpmanager.api.IBgpManager; -import org.opendaylight.netvirt.fibmanager.api.FibHelper; import org.opendaylight.netvirt.fibmanager.api.IFibManager; import org.opendaylight.netvirt.fibmanager.api.RouteOrigin; import org.opendaylight.netvirt.vpnmanager.VpnConstants; @@ -37,11 +35,7 @@ import org.opendaylight.netvirt.vpnmanager.api.intervpnlink.InterVpnLinkDataComp import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.Flow; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.instruction.list.Instruction; -import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.FibEntries; -import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.fibentries.VrfTables; -import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.fibentries.VrfTablesKey; import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.vrfentries.VrfEntry; -import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.vrfentries.VrfEntryKey; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.netvirt.inter.vpn.link.rev160311.InterVpnLinkStates; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.netvirt.inter.vpn.link.rev160311.InterVpnLinks; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.netvirt.inter.vpn.link.rev160311.inter.vpn.link.states.InterVpnLinkState; @@ -363,78 +357,6 @@ public final class InterVpnLinkUtil { : new ArrayList<>(); } - /** - * Leaks a route from one VPN to another. By default, the origin for this leaked route is INTERVPN. - * - * @param broker dataBroker service reference - * @param bgpManager Used to advertise routes to the BGP Router - * @param interVpnLink Reference to the object that holds the info about the link between the 2 VPNs - * @param srcVpnUuid UUID of the VPN that has the route that is going to be leaked to the other VPN - * @param dstVpnUuid UUID of the VPN that is going to receive the route - * @param prefix Prefix of the route - * @param label Label of the route in the original VPN - */ - // TODO Clean up the exception handling - @SuppressWarnings("checkstyle:IllegalCatch") - public static void leakRoute(DataBroker broker, IBgpManager bgpManager, InterVpnLink interVpnLink, - String srcVpnUuid, String dstVpnUuid, String prefix, Long label) { - Preconditions.checkNotNull(interVpnLink); - - // The source VPN must participate in the InterVpnLink - Preconditions.checkArgument(interVpnLink.getFirstEndpoint().getVpnUuid().getValue().equals(srcVpnUuid) - || interVpnLink.getSecondEndpoint().getVpnUuid().getValue().equals(srcVpnUuid), - "The source VPN {} does not participate in the interVpnLink {}", - srcVpnUuid, interVpnLink.getName()); - // The destination VPN must participate in the InterVpnLink - Preconditions.checkArgument(interVpnLink.getFirstEndpoint().getVpnUuid().getValue().equals(dstVpnUuid) - || interVpnLink.getSecondEndpoint().getVpnUuid().getValue().equals(dstVpnUuid), - "The destination VPN {} does not participate in the interVpnLink {}", - dstVpnUuid, interVpnLink.getName()); - - boolean destinationIs1stEndpoint = interVpnLink.getFirstEndpoint().getVpnUuid().getValue().equals(dstVpnUuid); - - String endpointIp = destinationIs1stEndpoint ? interVpnLink.getSecondEndpoint().getIpAddress().getValue() - : interVpnLink.getFirstEndpoint().getIpAddress().getValue(); - - VrfEntry newVrfEntry = FibHelper.getVrfEntryBuilder(prefix, label, endpointIp, RouteOrigin.INTERVPN, - null /* parentVpnRd */).build(); - - String dstVpnRd = VpnUtil.getVpnRd(broker, dstVpnUuid); - InstanceIdentifier newVrfEntryIid = - InstanceIdentifier.builder(FibEntries.class) - .child(VrfTables.class, new VrfTablesKey(dstVpnRd)) - .child(VrfEntry.class, new VrfEntryKey(newVrfEntry.getDestPrefix())) - .build(); - VpnUtil.asyncWrite(broker, LogicalDatastoreType.CONFIGURATION, newVrfEntryIid, newVrfEntry); - - // Finally, route is advertised it to the DC-GW. But while in the FibEntries the nexthop is the other - // endpoint's IP, in the DC-GW the nexthop for those prefixes are the IPs of those DPNs where the target - // VPN has been instantiated - Optional optVpnLinkState = getInterVpnLinkState(broker, interVpnLink.getName()); - if (optVpnLinkState.isPresent()) { - InterVpnLinkState vpnLinkState = optVpnLinkState.get(); - List dpnIdList = destinationIs1stEndpoint ? vpnLinkState.getFirstEndpointState().getDpId() - : vpnLinkState.getSecondEndpointState().getDpId(); - List nexthops = new ArrayList<>(); - for (BigInteger dpnId : dpnIdList) { - nexthops.add(InterfaceUtils.getEndpointIpAddressForDPN(broker, dpnId)); - } - try { - LOG.debug("Advertising route in VPN={} [prefix={} label={} nexthops={}] to DC-GW", - dstVpnRd, newVrfEntry.getDestPrefix(), label.intValue(), nexthops); - bgpManager.advertisePrefix(dstVpnRd, null /*macAddress*/, newVrfEntry.getDestPrefix(), nexthops, - VrfEntry.EncapType.Mplsgre, label.intValue(), 0 /*l3vni*/, 0 /*l2vni*/, - null /*gatewayMacAddress*/); - } catch (Exception ex) { - LOG.error("Could not advertise prefix {} with label {} to VPN rd={}", - newVrfEntry.getDestPrefix(), label.intValue(), dstVpnRd, ex); - } - } else { - LOG.warn("Error when advertising leaked routes: Could not find State for InterVpnLink={}", - interVpnLink.getName()); - } - } - public static void handleStaticRoute(InterVpnLinkDataComposite interVpnLink, String vpnName, String destination, String nexthop, int label, DataBroker dataBroker, IFibManager fibManager, IBgpManager bgpManager) throws Exception { diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/tasks/InterVpnLinkCreatorTask.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/tasks/InterVpnLinkCreatorTask.java index 1f8863a9b7..d8b881e454 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/tasks/InterVpnLinkCreatorTask.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/tasks/InterVpnLinkCreatorTask.java @@ -8,12 +8,14 @@ package org.opendaylight.netvirt.vpnmanager.intervpnlink.tasks; import com.google.common.util.concurrent.ListenableFuture; -import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.Callable; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.netvirt.vpnmanager.intervpnlink.InterVpnLinkUtil; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.netvirt.inter.vpn.link.rev160311.inter.vpn.links.InterVpnLink; import org.slf4j.Logger; @@ -23,11 +25,11 @@ public class InterVpnLinkCreatorTask implements Callable> result = new ArrayList<>(); - - WriteTransaction writeTx = dataBroker.newWriteOnlyTransaction(); - writeTx.merge(LogicalDatastoreType.CONFIGURATION, - InterVpnLinkUtil.getInterVpnLinkPath(interVpnLinkToPersist.getName()), - interVpnLinkToPersist, - true /* create missing parents */); - result.add(writeTx.submit()); - return result; + return Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + tx.merge(LogicalDatastoreType.CONFIGURATION, + InterVpnLinkUtil.getInterVpnLinkPath(interVpnLinkToPersist.getName()), + interVpnLinkToPersist, WriteTransaction.CREATE_MISSING_PARENTS))); } } diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/tasks/InterVpnLinkRemoverTask.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/tasks/InterVpnLinkRemoverTask.java index 32e574c140..4ee7e93df4 100644 --- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/tasks/InterVpnLinkRemoverTask.java +++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/intervpnlink/tasks/InterVpnLinkRemoverTask.java @@ -12,8 +12,9 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.Callable; import org.opendaylight.controller.md.sal.binding.api.DataBroker; -import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.genius.infra.ManagedNewTransactionRunner; +import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.netvirt.inter.vpn.link.rev160311.inter.vpn.links.InterVpnLink; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.slf4j.Logger; @@ -24,20 +25,18 @@ public class InterVpnLinkRemoverTask implements Callable interVpnLinkIid; private final String interVpnLinkName; - private final DataBroker dataBroker; + private final ManagedNewTransactionRunner txRunner; public InterVpnLinkRemoverTask(DataBroker dataBroker, InstanceIdentifier interVpnLinkPath) { this.interVpnLinkIid = interVpnLinkPath; this.interVpnLinkName = interVpnLinkPath.firstKeyOf(InterVpnLink.class).getName(); - this.dataBroker = dataBroker; + this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker); } @Override public List> call() { LOG.debug("Removing InterVpnLink {} from storage", interVpnLinkName); - WriteTransaction removeTx = dataBroker.newWriteOnlyTransaction(); - removeTx.delete(LogicalDatastoreType.CONFIGURATION, this.interVpnLinkIid); - - return Collections.singletonList(removeTx.submit()); + return Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> + tx.delete(LogicalDatastoreType.CONFIGURATION, this.interVpnLinkIid))); } } -- 2.36.6