Bug 3618: Tenant.OPER not removed when Tenant.CONF removed

Change-Id: I010489727f36c390ff4fe59f259e79c9a06968ae
Signed-off-by: Konstantin Blagov <kblagov@cisco.com>

diff --git a/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/resolver/PolicyResolver.java b/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/resolver/PolicyResolver.java
old mode 100644 (file)
new mode 100755 (executable)
index 32642d2..aa293f3
--- a/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/resolver/PolicyResolver.java
+++ b/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/resolver/PolicyResolver.java
@@ -24,17 +24,15 @@ import javax.annotation.concurrent.Immutable;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.DataChangeListener;
 import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
+import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;
+import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker.DataChangeScope;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.ActionDefinitionId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.TenantId;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.has.action.refs.ActionRef;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.tenants.Tenant;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.tenants.tenant.Contract;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.tenants.tenant.contract.Subject;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.tenants.tenant.contract.subject.Rule;
 import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
@@ -302,7 +300,9 @@ public class PolicyResolver implements AutoCloseable {
             @Override
             public void onSuccess(Optional<Tenant> result) {
                 if (!result.isPresent()) {
-                    LOG.warn("Tenant {} not found", tenantId);
+                    LOG.info("Tenant {} not found in CONF; check&delete from OPER", tenantId);
+                    deleteOperTenantIfExists(tiid, tenantId);
+                    return;
                 }
 
                 Tenant t = InheritanceUtils.resolveTenant(result.get());
@@ -328,6 +328,30 @@ public class PolicyResolver implements AutoCloseable {
         }, executor);
     }
 
+    private void deleteOperTenantIfExists(final InstanceIdentifier<Tenant> tiid, final TenantId tenantId) {
+        final ReadWriteTransaction rwTx = dataProvider.newReadWriteTransaction();
+
+        ListenableFuture<Optional<Tenant>> readFuture = rwTx.read(LogicalDatastoreType.OPERATIONAL, tiid);
+        Futures.addCallback(readFuture, new FutureCallback<Optional<Tenant>>() {
+            @Override
+            public void onSuccess(Optional<Tenant> result) {
+                if(result.isPresent()){
+                    unsubscribeTenant(tenantId);
+                    rwTx.delete(LogicalDatastoreType.OPERATIONAL, tiid);
+                    rwTx.submit();
+                    updatePolicy();
+                }
+            }
+
+            @Override
+            public void onFailure(Throwable t) {
+                LOG.error("Failed to read operational datastore: {}", t);
+                rwTx.cancel();
+            }
+        }, executor);
+
+    }
+
     protected void updatePolicy() {
         try {
             Map<EgKey, Set<ConditionSet>> egConditions = new HashMap<>();

diff --git a/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapper.java b/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapper.java
index 77a205c76fa98dda2dce39e2d0637b3e9bfa3b4f..5b63354f090fcd85f9d8371c1a02ae637a70a315 100755 (executable)
--- a/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapper.java
+++ b/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapper.java
@@ -21,6 +21,7 @@ import org.opendaylight.groupbasedpolicy.renderer.ofoverlay.PolicyManager.FlowMa
 import org.opendaylight.groupbasedpolicy.renderer.ofoverlay.flow.FlowUtils.RegMatch;
 import org.opendaylight.groupbasedpolicy.renderer.ofoverlay.flow.OrdinalFactory.EndpointFwdCtxOrdinals;
 import org.opendaylight.groupbasedpolicy.resolver.EgKey;
+import org.opendaylight.groupbasedpolicy.resolver.IndexedTenant;
 import org.opendaylight.groupbasedpolicy.resolver.PolicyInfo;
 import org.opendaylight.groupbasedpolicy.resolver.TenantUtils;
 import org.opendaylight.groupbasedpolicy.util.IidFactory;
@@ -485,7 +486,12 @@ public class DestinationMapper extends FlowTable {
     }
 
     private L3Context getL3ContextForSubnet(TenantId tenantId, Subnet sn) {
-        L3Context l3c = ctx.getPolicyResolver().getTenant(tenantId).resolveL3Context(sn.getId());
+        IndexedTenant indexedTenant = ctx.getPolicyResolver().getTenant(tenantId);
+        if (indexedTenant == null) {
+            LOG.debug("Tenant {} is null, cannot get L3 context", tenantId);
+            return null;
+        }
+        L3Context l3c = indexedTenant.resolveL3Context(sn.getId());
         return l3c;
     }
 
@@ -602,7 +608,11 @@ public class DestinationMapper extends FlowTable {
         EndpointFwdCtxOrdinals srcEpFwdCtxOrds = OrdinalFactory.getEndpointFwdCtxOrdinals(ctx, policyInfo, srcEp);
 
         if (destEp.getTenant() == null || (destEp.getEndpointGroup() == null && destEp.getEndpointGroups() == null)) {
-            LOG.trace("Didn't process endpoint due to either tenant, or EPG(s) being null", destEp.getKey());
+            if (destEp.getTenant() == null) {
+                LOG.debug("Didn't process endpoint {} due to tenant being null", destEp.getKey());
+            } else {
+                LOG.debug("Didn't process endpoint {} due to EPG(s) being null", destEp.getKey());
+            }
             return;
         }
         OfOverlayContext ofc = destEp.getAugmentation(OfOverlayContext.class);
@@ -1155,8 +1165,8 @@ public class DestinationMapper extends FlowTable {
         for (Endpoint endpoint : endpointsForNode) {
             HashSet<Subnet> subnets = getSubnets(endpoint.getTenant());
             if (subnets == null) {
-                LOG.error("No local subnets.");
-                return null;
+                LOG.debug("No local subnets in tenant {} for EP {}.", endpoint.getTenant(), endpoint.getKey());
+                continue;
             }
             NetworkDomainId epNetworkContainment = getEPNetworkContainment(endpoint);
             for (Subnet subnet : subnets) {

diff --git a/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/OrdinalFactory.java b/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/OrdinalFactory.java
old mode 100644 (file)
new mode 100755 (executable)
index 7b77ccb..0931226
--- a/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/OrdinalFactory.java
+++ b/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/OrdinalFactory.java
@@ -186,6 +186,10 @@ public class OrdinalFactory {
             this.ep = new EpKey(ep.getL2Context(), ep.getMacAddress());
 
             IndexedTenant tenant = ctx.getPolicyResolver().getTenant(ep.getTenant());
+            if (tenant == null) {
+                LOG.debug("Tenant {} is null", ep.getTenant());
+                return;
+            }
             // Set network containment either from ep, or from primary EPG
             if (ep.getNetworkContainment() != null) {
                 this.networkContainment = ep.getNetworkContainment();

diff --git a/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/PolicyEnforcer.java b/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/PolicyEnforcer.java
index 5b3d2d268dcdc9968e647590f22219d80e645fc2..f444256fe5a92a8af123a0a9b325be000c85caf0 100755 (executable)
--- a/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/PolicyEnforcer.java
+++ b/renderers/ofoverlay/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/PolicyEnforcer.java
@@ -199,19 +199,21 @@ public class PolicyEnforcer extends FlowTable {
             for (EgKey srcEpgKey : ctx.getEndpointManager().getEgKeysForEndpoint(srcEp)) {
 
                 IndexedTenant tenant = ctx.getPolicyResolver().getTenant(srcEpgKey.getTenantId());
-                EndpointGroup group = tenant.getEndpointGroup(srcEpgKey.getEgId());
-                IntraGroupPolicy igp = group.getIntraGroupPolicy();
-
-                if (igp == null || igp.equals(IntraGroupPolicy.Allow)) {
-                    for (Endpoint dstEp : ctx.getEndpointManager().getEndpointsForGroup(srcEpgKey)) {
-                        EndpointFwdCtxOrdinals srcEpFwdCxtOrds = OrdinalFactory.getEndpointFwdCtxOrdinals(ctx,
-                                policyInfo, srcEp);
-                        EndpointFwdCtxOrdinals dstEpFwdCxtOrds = OrdinalFactory.getEndpointFwdCtxOrdinals(ctx,
-                                policyInfo, dstEp);
-                        int depgId = dstEpFwdCxtOrds.getEpgId();
-                        int sepgId = srcEpFwdCxtOrds.getEpgId();
-                        flowMap.writeFlow(nodeId, TABLE_ID, allowSameEpg(sepgId, depgId));
-                        flowMap.writeFlow(nodeId, TABLE_ID, allowSameEpg(depgId, sepgId));
+                if (tenant != null) {
+                    EndpointGroup group = tenant.getEndpointGroup(srcEpgKey.getEgId());
+                    IntraGroupPolicy igp = group.getIntraGroupPolicy();
+
+                    if (igp == null || igp.equals(IntraGroupPolicy.Allow)) {
+                        for (Endpoint dstEp : ctx.getEndpointManager().getEndpointsForGroup(srcEpgKey)) {
+                            EndpointFwdCtxOrdinals srcEpFwdCxtOrds =
+                                OrdinalFactory.getEndpointFwdCtxOrdinals(ctx, policyInfo, srcEp);
+                            EndpointFwdCtxOrdinals dstEpFwdCxtOrds =
+                                OrdinalFactory.getEndpointFwdCtxOrdinals(ctx, policyInfo, dstEp);
+                            int depgId = dstEpFwdCxtOrds.getEpgId();
+                            int sepgId = srcEpFwdCxtOrds.getEpgId();
+                            flowMap.writeFlow(nodeId, TABLE_ID, allowSameEpg(sepgId, depgId));
+                            flowMap.writeFlow(nodeId, TABLE_ID, allowSameEpg(depgId, sepgId));
+                        }
                     }
                 }
             }

diff --git a/renderers/ofoverlay/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapperTest.java b/renderers/ofoverlay/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapperTest.java
index 48843f7de4d3745032e40569abe4259b625c3def..85b231dc126eef07dc8dfa6fb146e3a255705f8c 100755 (executable)
--- a/renderers/ofoverlay/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapperTest.java
+++ b/renderers/ofoverlay/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapperTest.java
@@ -15,6 +15,7 @@ import java.util.List;
 import java.util.Objects;\r
 \r
 import org.junit.Before;\r
+import org.junit.Ignore;\r
 import org.junit.Test;\r
 import org.opendaylight.groupbasedpolicy.renderer.ofoverlay.PolicyManager.FlowMap;\r
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924.IpAddress;\r
@@ -73,6 +74,7 @@ public class DestinationMapperTest extends FlowTableTest {
         FlowMap fm = dosync(null);\r
         assertNotEquals(0, fm.getTableForNode(nodeId, ctx.getPolicyManager().getTABLEID_DESTINATION_MAPPER()).getFlow().size());\r
 \r
+        // presumably counts flows that have correct matches set up\r
         int count = 0;\r
         HashMap<String, Flow> flowMap = new HashMap<>();\r
         for (Flow f : fm.getTableForNode(nodeId, ctx.getPolicyManager().getTABLEID_DESTINATION_MAPPER()).getFlow()) {\r
@@ -142,9 +144,8 @@ public class DestinationMapperTest extends FlowTableTest {
                     if (ins.getInstruction() instanceof ApplyActionsCase) {\r
                         long p = getOfPortNum(tunnelId);\r
                         List<Action> actions = ((ApplyActionsCase) ins.getInstruction()).getApplyActions().getAction();\r
-                        assertEquals(nxLoadRegAction(NxmNxReg7.class,\r
-                                BigInteger.valueOf(p)),\r
-                                actions.get(4).getAction());\r
+                        assertEquals(nxLoadRegAction(NxmNxReg7.class, BigInteger.valueOf(p)),\r
+                                actions.get(actions.size() - 1).getAction());\r
                         icount += 1;\r
                     } else if (ins.getInstruction() instanceof GoToTableCase) {\r
                         assertEquals(gotoTableIns((short) (table.getTableId() + 1)),\r
@@ -218,7 +219,7 @@ public class DestinationMapperTest extends FlowTableTest {
         // TODO Li alagalah: Due to subnet checking this test is no longer setup\r
         // correct. Must address before Li.\r
         // assertEquals(8, count);\r
-        assertEquals(1, count);\r
+        assertEquals(fm.getTableForNode(nodeId, ctx.getPolicyManager().getTABLEID_DESTINATION_MAPPER()).getFlow().size(), count);\r
         int numberOfFlows = fm.getTableForNode(nodeId, (short) 2).getFlow().size();\r
         fm = dosync(flowMap);\r
         assertEquals(numberOfFlows, fm.getTableForNode(nodeId, (short) 2).getFlow().size());\r