Fixes addressing test reports 20/53520/12
authorMichal Cmarada <mcmarada@cisco.com>
Fri, 24 Mar 2017 09:05:02 +0000 (10:05 +0100)
committerTomas Cechvala <tcechval@cisco.com>
Fri, 24 Mar 2017 11:08:24 +0000 (11:08 +0000)
 - bug 8069 caching security groups and group
   rules in case notifications are disordered
 - mapping ipv6 allocation pools in subnets
 - tiny fix in processing SNAT matching table

Change-Id: I9cf4617d493e50f448be29455186cb6067a730e4
Signed-off-by: Tomas Cechvala <tcechval@cisco.com>
Signed-off-by: Michal Cmarada <mcmarada@cisco.com>
neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/NeutronMapper.java
neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/mapping/NeutronSecurityGroupAware.java
neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/mapping/NeutronSubnetAware.java
neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/mapping/rule/NeutronSecurityRuleAware.java
neutron-mapper/src/test/java/org/opendaylight/groupbasedpolicy/neutron/mapper/mapping/NeutronSecurityGroupAwareDataStoreTest.java
renderers/vpp/src/main/java/org/opendaylight/groupbasedpolicy/renderer/vpp/nat/NatUtil.java

index f5f1939b3f1bdaa92e2506a03af862e938f421fb..f1cd873f3f6a60eae27c55c0f3b418c5c0287471 100644 (file)
@@ -108,8 +108,8 @@ public class NeutronMapper implements ClusteredDataTreeChangeListener<Neutron>,
             BaseEndpointService baseEpService) {
         EndpointRegistrator epRegistrator = new EndpointRegistrator(epService, baseEpService);
         networkAware = new NeutronNetworkAware(dataProvider);
-        securityGroupAware = new NeutronSecurityGroupAware(dataProvider);
         securityRuleAware = new NeutronSecurityRuleAware(dataProvider);
+        securityGroupAware = new NeutronSecurityGroupAware(dataProvider, securityRuleAware);
         subnetAware = new NeutronSubnetAware(dataProvider, epRegistrator);
         portAware = new NeutronPortAware(dataProvider, epRegistrator);
         routerAware = new NeutronRouterAware(dataProvider, epRegistrator);
index a1de94c8223d34f25c07d564e5017b26a30c640f..8478f59aee6b5194998c5473e527574afd727a95 100644 (file)
@@ -12,6 +12,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.groupbasedpolicy.neutron.mapper.mapping.rule.NeutronSecurityRuleAware;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.util.MappingUtils;
 import org.opendaylight.groupbasedpolicy.util.DataStoreHelper;
 import org.opendaylight.groupbasedpolicy.util.IidFactory;
@@ -39,9 +40,11 @@ public class NeutronSecurityGroupAware implements NeutronAware<SecurityGroup> {
     public static final InstanceIdentifier<SecurityGroup> SECURITY_GROUP_WILDCARD_IID =
             InstanceIdentifier.builder(Neutron.class).child(SecurityGroups.class).child(SecurityGroup.class).build();
     private final DataBroker dataProvider;
+    private final NeutronSecurityRuleAware ruleAware;
 
-    public NeutronSecurityGroupAware(DataBroker dataProvider) {
+    public NeutronSecurityGroupAware(DataBroker dataProvider, NeutronSecurityRuleAware ruleAware) {
         this.dataProvider = checkNotNull(dataProvider);
+        this.ruleAware = checkNotNull(ruleAware);
     }
 
     @Override
@@ -54,6 +57,7 @@ public class NeutronSecurityGroupAware implements NeutronAware<SecurityGroup> {
         } else {
             rwTx.cancel();
         }
+        ruleAware.flushPendingSecurityRulesFor(createdSecGroup.getKey(), neutron);
     }
 
     public boolean addNeutronSecurityGroup(SecurityGroup secGroup, ReadWriteTransaction rwTx) {
@@ -88,9 +92,24 @@ public class NeutronSecurityGroupAware implements NeutronAware<SecurityGroup> {
     @Override
     public void onDeleted(SecurityGroup deletedSecGroup, Neutron oldNeutron, Neutron newNeutron) {
         LOG.trace("deleted securityGroup - {}", deletedSecGroup);
-        ReadWriteTransaction rwTx = dataProvider.newReadWriteTransaction();
-        TenantId tenantId = new TenantId(deletedSecGroup.getTenantId().getValue());
-        EndpointGroupId epgId = new EndpointGroupId(deletedSecGroup.getUuid().getValue());
+        if (newNeutron != null && newNeutron.getSecurityRules() != null
+                && newNeutron.getSecurityRules().getSecurityRule() != null
+                && newNeutron.getSecurityRules()
+                    .getSecurityRule()
+                    .stream()
+                    .filter(sr -> sr.getSecurityGroupId().equals(deletedSecGroup.getUuid()))
+                    .findAny()
+                    .isPresent()) {
+            LOG.warn("Cannot remove security group {} before removing last security rule.", deletedSecGroup.getKey());
+            ruleAware.addPendingDeletedSecGroup(deletedSecGroup);
+            return;
+        }
+        deleteGbpEndpointGroup(dataProvider, new TenantId(deletedSecGroup.getTenantId().getValue()),
+                new EndpointGroupId(deletedSecGroup.getUuid().getValue()));
+    }
+
+    public static void deleteGbpEndpointGroup(DataBroker dataBroker, TenantId tenantId, EndpointGroupId epgId) {
+        ReadWriteTransaction rwTx = dataBroker.newReadWriteTransaction();
         Optional<EndpointGroup> potentialEpg = DataStoreHelper.removeIfExists(LogicalDatastoreType.CONFIGURATION,
                 IidFactory.endpointGroupIid(tenantId, epgId), rwTx);
         if (!potentialEpg.isPresent()) {
@@ -98,8 +117,6 @@ public class NeutronSecurityGroupAware implements NeutronAware<SecurityGroup> {
             rwTx.cancel();
             return;
         }
-
         DataStoreHelper.submitToDs(rwTx);
     }
-
 }
index 643bc0621c6892729e3765f52ca406d6068a7272..911aff6e3bd65042d9e5d65baafa861d3035af89 100644 (file)
@@ -11,6 +11,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import java.util.Collections;
 import java.util.List;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
@@ -44,6 +45,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.networks.rev150712.
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.ports.rev150712.ports.attributes.ports.Port;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.provider.ext.rev150712.NetworkProviderExtension;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.rev150712.Neutron;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.subnets.rev150712.subnet.attributes.AllocationPools;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.subnets.rev150712.subnets.attributes.Subnets;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
@@ -164,12 +166,24 @@ public class NeutronSubnetAware implements
             sb.setIsTenant(NetworkUtils.isTenantNetwork(potentialNetwork.get()));
         }
         if (subnet.getAllocationPools() != null) {
-            List<AllocationPool> pools = subnet.getAllocationPools()
-                .stream()
-                .map(s -> new AllocationPoolBuilder().setFirst(s.getStart().getIpv4Address().getValue())
-                    .setLast(s.getEnd().getIpv4Address().getValue())
-                    .build())
-                .collect(Collectors.toList());
+            List<AllocationPool> pools =
+                    subnet.getAllocationPools().stream().map(new Function<AllocationPools, AllocationPool>() {
+
+                        @Override
+                        public AllocationPool apply(AllocationPools ap) {
+                            IpAddress start = ap.getStart();
+                            IpAddress end = ap.getEnd();
+                            AllocationPoolBuilder ab = new AllocationPoolBuilder();
+                            if (start.getIpv4Address() != null || end.getIpv4Address() != null) {
+                                ab.setFirst(start.getIpv4Address().getValue());
+                                ab.setLast(end.getIpv4Address().getValue());
+                            } else {
+                                ab.setFirst(start.getIpv6Address().getValue());
+                                ab.setLast(end.getIpv6Address().getValue());
+                            }
+                            return ab.build();
+                        }
+                    }).collect(Collectors.toList());
             sb.setAllocationPool(pools);
         }
         NetworkDomainBuilder ndb = new NetworkDomainBuilder();
index 096f0ad5a4049aabdaaddffd394676504cefa347..86e21b876a58761e99ae47af0953c47e5f554c2a 100644 (file)
@@ -10,7 +10,14 @@ package org.opendaylight.groupbasedpolicy.neutron.mapper.mapping.rule;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collector;
+import java.util.stream.Collectors;
+
+import javax.annotation.Nonnull;
 
 import org.opendaylight.controller.config.yang.config.neutron_mapper.impl.NeutronMapperModule;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
@@ -20,6 +27,7 @@ import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.groupbasedpolicy.api.sf.ChainActionDefinition;
 import org.opendaylight.groupbasedpolicy.dto.EpgKeyDto;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.mapping.NeutronAware;
+import org.opendaylight.groupbasedpolicy.neutron.mapper.mapping.NeutronSecurityGroupAware;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.util.MappingUtils;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.util.SecurityGroupUtils;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.util.SecurityRuleUtils;
@@ -48,14 +56,17 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.tenants.tenant.policy.subject.feature.instances.ClassifierInstance;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.rev150712.Neutron;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.secgroups.rev150712.security.groups.attributes.security.groups.SecurityGroup;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.secgroups.rev150712.security.groups.attributes.security.groups.SecurityGroupKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.secgroups.rev150712.security.rules.attributes.SecurityRules;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.secgroups.rev150712.security.rules.attributes.security.rules.SecurityRule;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.neutron.secgroups.rev150712.security.rules.attributes.security.rules.SecurityRuleKey;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.HashMultiset;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Multiset;
@@ -70,6 +81,8 @@ public class NeutronSecurityRuleAware implements NeutronAware<SecurityRule> {
     private final DataBroker dataProvider;
     private final Multiset<InstanceIdentifier<ClassifierInstance>> createdClassifierInstances;
     private final Multiset<InstanceIdentifier<ActionInstance>> createdActionInstances;
+    private final Map<SecurityRuleKey, SecurityRule> pendingCreatedRules;
+    private final Map<SecurityGroupKey, SecurityGroup> pendingDeletedGroups;
     final static String PROVIDED_BY = "provided_by-";
     final static String POSSIBLE_CONSUMER = "possible_consumer-";
 
@@ -85,11 +98,25 @@ public class NeutronSecurityRuleAware implements NeutronAware<SecurityRule> {
         this.dataProvider = checkNotNull(dataProvider);
         this.createdClassifierInstances = checkNotNull(classifierInstanceNames);
         this.createdActionInstances = checkNotNull(createdActionInstances);
+        this.pendingCreatedRules = new HashMap<>();
+        this.pendingDeletedGroups = new HashMap<>();
     }
 
     @Override
     public void onCreated(SecurityRule secRule, Neutron neutron) {
         LOG.trace("created securityRule - {}", secRule);
+        if (neutron.getSecurityGroups() == null || neutron.getSecurityGroups().getSecurityGroup() == null
+                || !neutron.getSecurityGroups()
+                    .getSecurityGroup()
+                    .stream()
+                    .filter(sg -> sg.getKey().getUuid().equals(secRule.getSecurityGroupId()))
+                    .findFirst()
+                    .isPresent()) {
+            pendingCreatedRules.put(secRule.getKey(), secRule);
+            LOG.warn("Security group of security rule {} does not exist yet. The rule will be processed"
+                    + "when the missing security group is created.", secRule.getKey());
+            return;
+        }
         ReadWriteTransaction rwTx = dataProvider.newReadWriteTransaction();
         boolean isNeutronSecurityRuleAdded = addNeutronSecurityRule(secRule, neutron, rwTx);
         if (isNeutronSecurityRuleAdded) {
@@ -99,6 +126,24 @@ public class NeutronSecurityRuleAware implements NeutronAware<SecurityRule> {
         }
     }
 
+    public void flushPendingSecurityRulesFor(@Nonnull SecurityGroupKey secGroupKey, Neutron neutron) {
+        List<SecurityRule> rules = pendingCreatedRules.values()
+            .stream()
+            .filter(sr -> sr.getSecurityGroupId().equals(secGroupKey.getUuid()))
+            .collect(Collectors.toList());
+        rules.forEach(sr -> {
+            LOG.trace("Flushing pending security rule {}", sr);
+            ReadWriteTransaction rwTx = dataProvider.newReadWriteTransaction();
+            boolean isNeutronSecurityRuleAdded = addNeutronSecurityRule(sr, neutron, rwTx);
+            if (isNeutronSecurityRuleAdded) {
+                DataStoreHelper.submitToDs(rwTx);
+            } else {
+                rwTx.cancel();
+            }
+            pendingCreatedRules.remove(sr.getKey());
+        });
+    }
+
     public boolean addNeutronSecurityRule(SecurityRule secRule, Neutron neutron, ReadWriteTransaction rwTx) {
         return addNeutronSecurityRuleWithAction(secRule, neutron, MappingUtils.ALLOW_ACTION_CHOICE, rwTx);
     }
@@ -265,6 +310,11 @@ public class NeutronSecurityRuleAware implements NeutronAware<SecurityRule> {
                 newSecRule);
     }
 
+    public void addPendingDeletedSecGroup(SecurityGroup secGroup) {
+        LOG.trace("Caching pending deleted security group {}", secGroup.getKey());
+        pendingDeletedGroups.put(secGroup.getKey(), secGroup);
+    }
+
     @Override
     public void onDeleted(SecurityRule deletedSecRule, Neutron oldNeutron, Neutron newNeutron) {
         LOG.trace("deleted securityRule - {}", deletedSecRule);
@@ -272,6 +322,24 @@ public class NeutronSecurityRuleAware implements NeutronAware<SecurityRule> {
         boolean isNeutronSecurityRuleDeleted = deleteNeutronSecurityRule(deletedSecRule, oldNeutron, rwTx);
         if (isNeutronSecurityRuleDeleted) {
             DataStoreHelper.submitToDs(rwTx);
+            if (newNeutron == null || newNeutron.getSecurityRules() == null
+                || newNeutron.getSecurityRules().getSecurityRule() == null
+                || !newNeutron.getSecurityRules()
+                        .getSecurityRule()
+                        .stream()
+                        .filter(rule -> rule.getSecurityGroupId().equals(deletedSecRule.getSecurityGroupId()))
+                        .findAny()
+                        .isPresent()) {
+                SecurityGroupKey secGroupKey = new SecurityGroupKey(deletedSecRule.getSecurityGroupId());
+                SecurityGroup pendingSg = pendingDeletedGroups.get(secGroupKey);
+                if (pendingSg != null) {
+                    LOG.trace("Processing pending deleted security group {}", secGroupKey);
+                    NeutronSecurityGroupAware.deleteGbpEndpointGroup(dataProvider,
+                            new TenantId(deletedSecRule.getTenantId().getValue()),
+                            new EndpointGroupId(secGroupKey.getUuid().getValue()));
+                    pendingDeletedGroups.remove(secGroupKey);
+                }
+            }
         } else {
             rwTx.cancel();
         }
index 9cc2f53d4ceb7fa9a889572d4e655d7ffba4fd77..bb2f9695c7b9bad10cab0a54f774c8bd5007a8dc 100644 (file)
@@ -15,6 +15,7 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.test.NeutronMapperDataBrokerTest;
+import org.opendaylight.groupbasedpolicy.neutron.mapper.mapping.rule.NeutronSecurityRuleAware;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.test.NeutronEntityFactory;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.test.PolicyAssert;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.util.MappingUtils;
@@ -37,7 +38,7 @@ public class NeutronSecurityGroupAwareDataStoreTest extends NeutronMapperDataBro
     @Before
     public void init() {
         dataBroker = getDataBroker();
-        groupAware = new NeutronSecurityGroupAware(dataBroker);
+        groupAware = new NeutronSecurityGroupAware(dataBroker, new NeutronSecurityRuleAware(dataBroker));
 
         secGroup1 = NeutronEntityFactory.securityGroup(secGroupId1, tenantId);
         secGroup2 = NeutronEntityFactory.securityGroup(secGroupId2, tenantId);
@@ -98,7 +99,7 @@ public class NeutronSecurityGroupAwareDataStoreTest extends NeutronMapperDataBro
     @Test
     public void testConstructor_invalidArgument() throws NullPointerException {
         thrown.expect(NullPointerException.class);
-        new NeutronSecurityGroupAware(null);
+        new NeutronSecurityGroupAware(null, null);
     }
 
     @Test
index 2a67f3141405750422174c709508553d36e3d188..6887d6f79ed40195b6701bbf083860b31f987c3b 100644 (file)
@@ -31,6 +31,7 @@ import org.opendaylight.groupbasedpolicy.renderer.vpp.util.GbpNetconfTransaction
 import org.opendaylight.groupbasedpolicy.renderer.vpp.util.VppIidFactory;\r
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Address;\r
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv4Prefix;\r
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.Ipv6Prefix;\r
 import org.opendaylight.groupbasedpolicy.util.DataStoreHelper;\r
 import org.opendaylight.groupbasedpolicy.util.NetUtils;\r
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpPrefix;\r
@@ -169,10 +170,12 @@ public class NatUtil {
         if (natEntries != null) {\r
             for (MappingEntryBuilder natEntry : natEntries) {\r
                 Ipv4Address externalSrcAddress = natEntry.getExternalSrcAddress();\r
-                ext.remove(Integer.toUnsignedLong(subnet.getInfo().asInteger(externalSrcAddress.getValue())));\r
+                long id = Integer.toUnsignedLong(subnet.getInfo().asInteger(externalSrcAddress.getValue()));\r
+                if (ext.get(id) != null) {\r
+                    ext.remove(id);\r
+                }\r
             }\r
         }\r
-\r
         return ext;\r
     }\r
 }\r