From: Shashidhar Raja Date: Fri, 10 Mar 2017 14:20:57 +0000 (+0530) Subject: Bug 7775: Updated to use DataStoreJobCoordinator for flow programming X-Git-Tag: release/carbon~243 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=ddfe3934e4df1dce0c54b32a1cd0b1106852852a;p=netvirt.git Bug 7775: Updated to use DataStoreJobCoordinator for flow programming Updated syncFlow() in AbstractAclServiceImpl.java to use DJC while flow is added/removed to serialize/parallelize flow programming based on device id. This also avoids possibility of OptimisticLockException getting generated from ACL while flows are programmed. Also, Interface state listener updated to use DJC to serialize/parallelize interface state processing changes. This review is dependent on below genius patches: https://git.opendaylight.org/gerrit/#/c/53143/ https://git.opendaylight.org/gerrit/#/c/53352/ Depends-On: I8c15af6607f83a08354520872e4a5ff0ea16d2bf Change-Id: I578cdf332d8e5575ac31b3b7c5fdf9d7ea86013d Signed-off-by: Shashidhar Raja --- diff --git a/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java b/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java index 93f3495534..ce9236767a 100644 --- a/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java +++ b/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java @@ -9,12 +9,14 @@ package org.opendaylight.netvirt.aclservice; import java.math.BigInteger; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; import org.opendaylight.controller.md.sal.binding.api.DataBroker; +import org.opendaylight.genius.datastoreutils.DataStoreJobCoordinator; import org.opendaylight.genius.mdsalutil.ActionInfo; import org.opendaylight.genius.mdsalutil.FlowEntity; import org.opendaylight.genius.mdsalutil.InstructionInfo; @@ -338,16 +340,23 @@ public abstract class AbstractAclServiceImpl implements AclServiceListener { protected void syncFlow(BigInteger dpId, short tableId, String flowId, int priority, String flowName, int idleTimeOut, int hardTimeOut, BigInteger cookie, List matches, List instructions, int addOrRemove) { + DataStoreJobCoordinator dataStoreCoordinator = DataStoreJobCoordinator.getInstance(); if (addOrRemove == NwConstants.DEL_FLOW) { FlowEntity flowEntity = MDSALUtil.buildFlowEntity(dpId, tableId, flowId, priority, flowName, idleTimeOut, hardTimeOut, cookie, matches, null); LOG.trace("Removing Acl Flow DpnId {}, flowId {}", dpId, flowId); - mdsalManager.removeFlow(flowEntity); + dataStoreCoordinator.enqueueJob(dpId.toString(), + () -> { + return Arrays.asList(mdsalManager.removeFlow(dpId, flowEntity)); + }); } else { FlowEntity flowEntity = MDSALUtil.buildFlowEntity(dpId, tableId, flowId, priority, flowName, idleTimeOut, hardTimeOut, cookie, matches, instructions); LOG.trace("Installing DpnId {}, flowId {}", dpId, flowId); - mdsalManager.installFlow(flowEntity); + dataStoreCoordinator.enqueueJob(dpId.toString(), + () -> { + return Arrays.asList(mdsalManager.installFlow(dpId, flowEntity)); + }); } } diff --git a/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceStateListener.java b/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceStateListener.java index 54e5ffc0a6..08ab241482 100644 --- a/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceStateListener.java +++ b/vpnservice/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceStateListener.java @@ -7,6 +7,9 @@ */ package org.opendaylight.netvirt.aclservice.listeners; +import com.google.common.util.concurrent.Futures; + +import java.util.Arrays; import java.util.List; import javax.annotation.PostConstruct; @@ -17,6 +20,7 @@ import org.opendaylight.controller.md.sal.binding.api.ClusteredDataTreeChangeLis 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.datastoreutils.DataStoreJobCoordinator; import org.opendaylight.netvirt.aclservice.api.AclServiceManager; import org.opendaylight.netvirt.aclservice.api.AclServiceManager.Action; import org.opendaylight.netvirt.aclservice.api.utils.AclInterface; @@ -68,17 +72,22 @@ public class AclInterfaceStateListener extends AsyncDataTreeChangeListenerBase key, Interface dataObjectModification) { String interfaceId = dataObjectModification.getName(); - AclInterface aclInterface = AclInterfaceCacheUtil.getAclInterfaceFromCache(interfaceId); - if (AclServiceUtils.isOfInterest(aclInterface)) { - AclInterfaceCacheUtil.removeAclInterfaceFromCache(interfaceId); - if (aclClusterUtil.isEntityOwner()) { - aclServiceManger.notify(aclInterface, null, Action.REMOVE); - } - List aclList = aclInterface.getSecurityGroups(); - if (aclList != null) { - aclDataUtil.removeAclInterfaceMap(aclList, aclInterface); - } - } + DataStoreJobCoordinator dataStoreCoordinator = DataStoreJobCoordinator.getInstance(); + dataStoreCoordinator.enqueueJob(interfaceId, + () -> { + AclInterface aclInterface = AclInterfaceCacheUtil.getAclInterfaceFromCache(interfaceId); + if (AclServiceUtils.isOfInterest(aclInterface)) { + AclInterfaceCacheUtil.removeAclInterfaceFromCache(interfaceId); + if (aclClusterUtil.isEntityOwner()) { + aclServiceManger.notify(aclInterface, null, Action.REMOVE); + } + List aclList = aclInterface.getSecurityGroups(); + if (aclList != null) { + aclDataUtil.removeAclInterfaceMap(aclList, aclInterface); + } + } + return Arrays.asList(Futures.immediateCheckedFuture(null)); + }); } @Override @@ -92,16 +101,21 @@ public class AclInterfaceStateListener extends AsyncDataTreeChangeListenerBase key, Interface dataObjectModification) { - AclInterface aclInterface = updateAclInterfaceCache(dataObjectModification); - if (AclServiceUtils.isOfInterest(aclInterface)) { - List aclList = aclInterface.getSecurityGroups(); - if (aclList != null) { - aclDataUtil.addAclInterfaceMap(aclList, aclInterface); - } - if (aclClusterUtil.isEntityOwner()) { - aclServiceManger.notify(aclInterface, null, Action.ADD); - } - } + DataStoreJobCoordinator dataStoreCoordinator = DataStoreJobCoordinator.getInstance(); + dataStoreCoordinator.enqueueJob(dataObjectModification.getName(), + () -> { + AclInterface aclInterface = updateAclInterfaceCache(dataObjectModification); + if (AclServiceUtils.isOfInterest(aclInterface)) { + List aclList = aclInterface.getSecurityGroups(); + if (aclList != null) { + aclDataUtil.addAclInterfaceMap(aclList, aclInterface); + } + if (aclClusterUtil.isEntityOwner()) { + aclServiceManger.notify(aclInterface, null, Action.ADD); + } + } + return Arrays.asList(Futures.immediateCheckedFuture(null)); + }); } @Override diff --git a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/LearnEgressAclServiceImplTest.java b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/LearnEgressAclServiceImplTest.java index 3296f65791..035b7dd9fc 100644 --- a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/LearnEgressAclServiceImplTest.java +++ b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/LearnEgressAclServiceImplTest.java @@ -15,6 +15,7 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; import com.google.common.base.Optional; +import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; import java.math.BigInteger; @@ -30,6 +31,7 @@ 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.mdsalutil.FlowEntity; import org.opendaylight.genius.mdsalutil.NwConstants; import org.opendaylight.genius.mdsalutil.NxMatchFieldType; @@ -77,8 +79,8 @@ public class LearnEgressAclServiceImplTest { @Mock AclserviceConfig config; @Mock IdManagerService idManager; - MethodInvocationParamSaver installFlowValueSaver = null; - MethodInvocationParamSaver removeFlowValueSaver = null; + MethodInvocationParamSaver> installFlowValueSaver = null; + MethodInvocationParamSaver> removeFlowValueSaver = null; final Integer tcpFinIdleTimeoutValue = 60; @@ -90,10 +92,10 @@ public class LearnEgressAclServiceImplTest { doReturn(Futures.immediateCheckedFuture(null)).when(mockWriteTx).submit(); doReturn(mockReadTx).when(dataBroker).newReadOnlyTransaction(); doReturn(mockWriteTx).when(dataBroker).newWriteOnlyTransaction(); - installFlowValueSaver = new MethodInvocationParamSaver<>(null); - doAnswer(installFlowValueSaver).when(mdsalManager).installFlow(any(FlowEntity.class)); - removeFlowValueSaver = new MethodInvocationParamSaver<>(null); - doAnswer(installFlowValueSaver).when(mdsalManager).removeFlow(any(FlowEntity.class)); + installFlowValueSaver = new MethodInvocationParamSaver<>(Futures.immediateCheckedFuture(null)); + doAnswer(installFlowValueSaver).when(mdsalManager).installFlow(any(BigInteger.class), any(FlowEntity.class)); + removeFlowValueSaver = new MethodInvocationParamSaver<>(Futures.immediateCheckedFuture(null)); + doAnswer(installFlowValueSaver).when(mdsalManager).removeFlow(any(BigInteger.class), any(FlowEntity.class)); doReturn(tcpFinIdleTimeoutValue).when(config).getSecurityGroupTcpFinIdleTimeout(); } @@ -115,9 +117,10 @@ public class LearnEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubTcpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 80); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(10, installFlowValueSaver.getNumOfInvocations()); - FlowEntity flow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(0); + FlowEntity flow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(1); AclServiceTestUtils.verifyMatchInfo(flow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "80", "65535"); AclServiceTestUtils.verifyActionTypeExist(flow.getInstructionInfoList().get(0), ActionLearn.class); @@ -141,9 +144,10 @@ public class LearnEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubAllowAllInterface(sgUuid, "if_name"); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(10, installFlowValueSaver.getNumOfInvocations()); - FlowEntity flow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(0); + FlowEntity flow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(1); AclServiceTestUtils.verifyActionTypeExist(flow.getInstructionInfoList().get(0), ActionLearn.class); } @@ -152,12 +156,13 @@ public class LearnEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubTcpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 84); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(11, installFlowValueSaver.getNumOfInvocations()); - FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(0); + FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(1); AclServiceTestUtils.verifyMatchInfo(firstRangeFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "80", "65532"); - FlowEntity secondRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(10).get(0); + FlowEntity secondRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(10).get(1); AclServiceTestUtils.verifyMatchInfo(secondRangeFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "84", "65535"); } @@ -167,8 +172,9 @@ public class LearnEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubUdpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 80); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(10, installFlowValueSaver.getNumOfInvocations()); - FlowEntity flow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(0); + FlowEntity flow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(1); AclServiceTestUtils.verifyMatchInfo(flow.getMatchInfoList(), NxMatchFieldType.nx_udp_dst_with_mask, "80", "65535"); AclServiceTestUtils.verifyActionTypeExist(flow.getInstructionInfoList().get(0), ActionLearn.class); @@ -193,6 +199,7 @@ public class LearnEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubTcpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 80); assertEquals(true, testedService.removeAcl(ai)); + Thread.sleep(1000); assertEquals(5, removeFlowValueSaver.getNumOfInvocations()); FlowEntity firstRangeFlow = (FlowEntity) removeFlowValueSaver.getInvocationParams(4).get(0); assertTrue(firstRangeFlow.getMatchInfoList().contains(new MatchTcpFlags(2))); diff --git a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/StatelessEgressAclServiceImplTest.java b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/StatelessEgressAclServiceImplTest.java index ea4573aa8f..3354a8d07b 100644 --- a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/StatelessEgressAclServiceImplTest.java +++ b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/StatelessEgressAclServiceImplTest.java @@ -15,6 +15,7 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; import com.google.common.base.Optional; +import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; import java.math.BigInteger; import java.util.Collections; @@ -27,6 +28,7 @@ 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.mdsalutil.FlowEntity; import org.opendaylight.genius.mdsalutil.NwConstants; import org.opendaylight.genius.mdsalutil.NxMatchFieldType; @@ -73,8 +75,8 @@ public class StatelessEgressAclServiceImplTest { @Mock AclserviceConfig config; @Mock IdManagerService idManager; - MethodInvocationParamSaver installFlowValueSaver = null; - MethodInvocationParamSaver removeFlowValueSaver = null; + MethodInvocationParamSaver> installFlowValueSaver = null; + MethodInvocationParamSaver> removeFlowValueSaver = null; @Before public void setUp() { @@ -84,10 +86,10 @@ public class StatelessEgressAclServiceImplTest { doReturn(Futures.immediateCheckedFuture(null)).when(mockWriteTx).submit(); doReturn(mockReadTx).when(dataBroker).newReadOnlyTransaction(); doReturn(mockWriteTx).when(dataBroker).newWriteOnlyTransaction(); - installFlowValueSaver = new MethodInvocationParamSaver<>(null); - doAnswer(installFlowValueSaver).when(mdsalManager).installFlow(any(FlowEntity.class)); - removeFlowValueSaver = new MethodInvocationParamSaver<>(null); - doAnswer(removeFlowValueSaver).when(mdsalManager).removeFlow(any(FlowEntity.class)); + installFlowValueSaver = new MethodInvocationParamSaver<>(Futures.immediateCheckedFuture(null)); + removeFlowValueSaver = new MethodInvocationParamSaver<>(Futures.immediateCheckedFuture(null)); + doAnswer(removeFlowValueSaver).when(mdsalManager).removeFlow(any(BigInteger.class), any(FlowEntity.class)); + doAnswer(installFlowValueSaver).when(mdsalManager).installFlow(any(BigInteger.class), any(FlowEntity.class)); } @Test @@ -108,15 +110,15 @@ public class StatelessEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubTcpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 80); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(10, installFlowValueSaver.getNumOfInvocations()); - FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(0); + FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(1); AclServiceTestUtils.verifyMatchInfo(firstRangeFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "80", "65535"); assertTrue(firstRangeFlow.getMatchInfoList().contains(new MatchTcpFlags(2))); AclServiceTestUtils.verifyActionInfo(firstRangeFlow.getInstructionInfoList().get(0), new ActionNxResubmit(NwConstants.LPORT_DISPATCHER_TABLE)); - } @Test @@ -124,9 +126,10 @@ public class StatelessEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubAllowAllInterface(sgUuid, "if_name"); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(10, installFlowValueSaver.getNumOfInvocations()); - FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(0); + FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(1); AclServiceTestUtils.verifyActionInfo(firstRangeFlow.getInstructionInfoList().get(0), new ActionNxResubmit(NwConstants.LPORT_DISPATCHER_TABLE)); } @@ -136,13 +139,14 @@ public class StatelessEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubTcpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 84); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(11, installFlowValueSaver.getNumOfInvocations()); - FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(0); + FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(9).get(1); AclServiceTestUtils.verifyMatchInfo(firstRangeFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "80", "65532"); assertTrue(firstRangeFlow.getMatchInfoList().contains(new MatchTcpFlags(2))); - FlowEntity secondRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(10).get(0); + FlowEntity secondRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(10).get(1); AclServiceTestUtils.verifyMatchInfo(secondRangeFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "84", "65535"); assertTrue(secondRangeFlow.getMatchInfoList().contains(new MatchTcpFlags(2))); @@ -153,6 +157,7 @@ public class StatelessEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubUdpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 80); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(9, installFlowValueSaver.getNumOfInvocations()); } @@ -161,8 +166,10 @@ public class StatelessEgressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubTcpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 80); assertEquals(true, testedService.removeAcl(ai)); + Thread.sleep(1000); + assertEquals(10, removeFlowValueSaver.getNumOfInvocations()); - FlowEntity firstRangeFlow = (FlowEntity) removeFlowValueSaver.getInvocationParams(9).get(0); + FlowEntity firstRangeFlow = (FlowEntity) removeFlowValueSaver.getInvocationParams(9).get(1); assertTrue(firstRangeFlow.getMatchInfoList().contains(new MatchTcpFlags(2))); AclServiceTestUtils.verifyMatchInfo(firstRangeFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "80", "65535"); diff --git a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/StatelessIngressAclServiceImplTest.java b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/StatelessIngressAclServiceImplTest.java index 76d39fc98d..3393295f09 100644 --- a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/StatelessIngressAclServiceImplTest.java +++ b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/StatelessIngressAclServiceImplTest.java @@ -15,6 +15,7 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; import com.google.common.base.Optional; +import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.Futures; import java.math.BigInteger; import java.util.Collections; @@ -27,6 +28,7 @@ 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.mdsalutil.FlowEntity; import org.opendaylight.genius.mdsalutil.NwConstants; import org.opendaylight.genius.mdsalutil.NxMatchFieldType; @@ -73,8 +75,8 @@ public class StatelessIngressAclServiceImplTest { @Mock AclserviceConfig config; @Mock IdManagerService idManager; - MethodInvocationParamSaver installFlowValueSaver = null; - MethodInvocationParamSaver removeFlowValueSaver = null; + MethodInvocationParamSaver> installFlowValueSaver = null; + MethodInvocationParamSaver> removeFlowValueSaver = null; @Before public void setUp() { @@ -84,10 +86,10 @@ public class StatelessIngressAclServiceImplTest { doReturn(Futures.immediateCheckedFuture(null)).when(mockWriteTx).submit(); doReturn(mockReadTx).when(dataBroker).newReadOnlyTransaction(); doReturn(mockWriteTx).when(dataBroker).newWriteOnlyTransaction(); - installFlowValueSaver = new MethodInvocationParamSaver<>(null); - doAnswer(installFlowValueSaver).when(mdsalManager).installFlow(any(FlowEntity.class)); - removeFlowValueSaver = new MethodInvocationParamSaver<>(null); - doAnswer(removeFlowValueSaver).when(mdsalManager).removeFlow(any(FlowEntity.class)); + installFlowValueSaver = new MethodInvocationParamSaver<>(Futures.immediateCheckedFuture(null)); + doAnswer(installFlowValueSaver).when(mdsalManager).installFlow(any(BigInteger.class), any(FlowEntity.class)); + removeFlowValueSaver = new MethodInvocationParamSaver<>(Futures.immediateCheckedFuture(null)); + doAnswer(removeFlowValueSaver).when(mdsalManager).removeFlow(any(BigInteger.class), any(FlowEntity.class)); } @Test @@ -108,9 +110,10 @@ public class StatelessIngressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubTcpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 80); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(7, installFlowValueSaver.getNumOfInvocations()); - FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(6).get(0); + FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(6).get(1); AclServiceTestUtils.verifyMatchInfo(firstRangeFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "80", "65535"); assertTrue(firstRangeFlow.getMatchInfoList().contains(new MatchTcpFlags(2))); @@ -123,9 +126,10 @@ public class StatelessIngressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubAllowAllInterface(sgUuid, "if_name"); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(7, installFlowValueSaver.getNumOfInvocations()); - FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(6).get(0); + FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(6).get(1); AclServiceTestUtils.verifyActionInfo(firstRangeFlow.getInstructionInfoList().get(0), new ActionNxResubmit(NwConstants.EGRESS_LPORT_DISPATCHER_TABLE)); } @@ -135,15 +139,16 @@ public class StatelessIngressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubTcpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 84); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(8, installFlowValueSaver.getNumOfInvocations()); - FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(6).get(0); + FlowEntity firstRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(6).get(1); // should have been 80-83 will be fixed as part of the port range support // https://bugs.opendaylight.org/show_bug.cgi?id=6200 AclServiceTestUtils.verifyMatchInfo(firstRangeFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "80", "65532"); assertTrue(firstRangeFlow.getMatchInfoList().contains(new MatchTcpFlags(2))); - FlowEntity secondRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(7).get(0); + FlowEntity secondRangeFlow = (FlowEntity) installFlowValueSaver.getInvocationParams(7).get(1); AclServiceTestUtils.verifyMatchInfo(secondRangeFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "84", "65535"); assertTrue(secondRangeFlow.getMatchInfoList().contains(new MatchTcpFlags(2))); @@ -154,6 +159,7 @@ public class StatelessIngressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubUdpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 80); assertEquals(true, testedService.applyAcl(ai)); + Thread.sleep(1000); assertEquals(6, installFlowValueSaver.getNumOfInvocations()); } @@ -162,8 +168,9 @@ public class StatelessIngressAclServiceImplTest { Uuid sgUuid = new Uuid("12345678-1234-1234-1234-123456789012"); AclInterface ai = stubTcpAclInterface(sgUuid, "if_name", "1.1.1.1/32", 80, 80); assertEquals(true, testedService.removeAcl(ai)); + Thread.sleep(1000); assertEquals(7, removeFlowValueSaver.getNumOfInvocations()); - FlowEntity firstSynFlow = (FlowEntity) removeFlowValueSaver.getInvocationParams(6).get(0); + FlowEntity firstSynFlow = (FlowEntity) removeFlowValueSaver.getInvocationParams(6).get(1); AclServiceTestUtils.verifyMatchInfo(firstSynFlow.getMatchInfoList(), NxMatchFieldType.nx_tcp_dst_with_mask, "80", "65535"); assertTrue(firstSynFlow.getMatchInfoList().contains(MatchTcpFlags.SYN)); diff --git a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/tests/AclServiceTestBase.java b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/tests/AclServiceTestBase.java index 103b7dc4a2..b15c1e6c1f 100644 --- a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/tests/AclServiceTestBase.java +++ b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/tests/AclServiceTestBase.java @@ -15,6 +15,7 @@ import java.util.Collections; import java.util.List; import java.util.stream.Collectors; import javax.inject.Inject; + import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -22,6 +23,7 @@ import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; import org.opendaylight.genius.datastoreutils.SingleTransactionDataBroker; import org.opendaylight.genius.datastoreutils.testutils.AsyncEventsWaiter; +import org.opendaylight.genius.datastoreutils.testutils.JobCoordinatorEventsWaiter; import org.opendaylight.genius.mdsalutil.FlowEntity; import org.opendaylight.genius.mdsalutil.NwConstants; import org.opendaylight.genius.mdsalutil.interfaces.testutils.TestIMdsalApiManager; @@ -90,6 +92,7 @@ public abstract class AclServiceTestBase { SingleTransactionDataBroker singleTransactionDataBroker; @Inject TestIMdsalApiManager mdsalApiManager; @Inject AsyncEventsWaiter asyncEventsWaiter; + @Inject JobCoordinatorEventsWaiter coordinatorEventsWaiter; @Before public void beforeEachTest() throws Exception { @@ -438,6 +441,7 @@ public abstract class AclServiceTestBase { abstract void newInterfaceWithAapIpv4AllCheck(); protected void assertFlowsInAnyOrder(Iterable expectedFlows) { + coordinatorEventsWaiter.awaitEventsConsumption(); mdsalApiManager.assertFlowsInAnyOrder(expectedFlows); } diff --git a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/tests/AclServiceTestModule.java b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/tests/AclServiceTestModule.java index b00fba81fc..13587e6ee2 100644 --- a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/tests/AclServiceTestModule.java +++ b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/tests/AclServiceTestModule.java @@ -17,6 +17,8 @@ import org.mockito.Mockito; import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.binding.test.DataBrokerTestModule; +import org.opendaylight.genius.datastoreutils.testutils.JobCoordinatorEventsWaiter; +import org.opendaylight.genius.datastoreutils.testutils.TestableJobCoordinatorEventsWaiter; import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager; import org.opendaylight.genius.mdsalutil.interfaces.testutils.TestIMdsalApiManager; import org.opendaylight.netvirt.aclservice.stats.TestOdlDirectStatisticsService; @@ -68,6 +70,7 @@ public class AclServiceTestModule extends AbstractModule { bind(IdManagerService.class).toInstance(Mockito.mock(TestIdManagerService.class, realOrException())); bind(OpendaylightDirectStatisticsService.class) .toInstance(Mockito.mock(TestOdlDirectStatisticsService.class, realOrException())); + bind(JobCoordinatorEventsWaiter.class).toInstance(new TestableJobCoordinatorEventsWaiter()); } private AclserviceConfig aclServiceConfig() { diff --git a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/utils/AclServiceTestUtils.java b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/utils/AclServiceTestUtils.java index 6f4ee687e1..f0cdc51ba8 100644 --- a/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/utils/AclServiceTestUtils.java +++ b/vpnservice/aclservice/impl/src/test/java/org/opendaylight/netvirt/aclservice/utils/AclServiceTestUtils.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.concurrent.ConcurrentMap; import java.util.stream.Collectors; import java.util.stream.Stream; + import org.junit.Assert; import org.opendaylight.genius.mdsalutil.ActionInfo; import org.opendaylight.genius.mdsalutil.InstructionInfo; @@ -203,7 +204,5 @@ public class AclServiceTestUtils { if (entityOwnerCache != null) { entityOwnerCache.put(entityName, true); } - } - }