From: karthikeyan <karthikeyan.k@altencalsoftlabs.com>
Date: Fri, 7 Apr 2017 09:40:17 +0000 (+0530)
Subject: Bug 8001: Data validation failed for path CSIT TCs
X-Git-Tag: release/carbon~110
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=c151da8ec33eedf6d1a42e91de2bd3357fc916e6;p=netvirt.git

Bug 8001: Data validation failed for path CSIT TCs

Problem Description:
====================
On external network update or internet VPN update NAT code will trigger
the ExternalRoutersListener.update() to handle the updated changes
in NAT feature. During this invocation, for getting NAPT switch-id is
retrieved from the "router-to-napt-switch" DS. After retrieved the NAPT
switch-id from the DS, it contains the value as "0". There is no check for
NAPT switch-id validation.

clrRtsFromBgpAndDelFibTs() method is invoked with passing NAPT switch-id
as "0" value is throwing the data validation exceptions.

Solution:
===========
As part of this issue fix, invoked NAPT Switch selection if dpnId value is
"null" or "0" from "router-to-napt-switch" DS then go for electing new
Napt switch for existing router. This will resolve the data validation
exception. Since clrRtsFromBgpAndDelFibTs() method is never invoked with
NAPT switch-id as "0".

Change-Id: I851c4049397369871616834f59aff1eb16ce4472
Signed-off-by: karthikeyan <karthikeyan.k@altencalsoftlabs.com>
---

diff --git a/vpnservice/natservice/natservice-impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java b/vpnservice/natservice/natservice-impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java
index 52b279a7cd..063b209594 100644
--- a/vpnservice/natservice/natservice-impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java
+++ b/vpnservice/natservice/natservice-impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java
@@ -1128,7 +1128,14 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
             LOG.error("NAT Service : Update external router event - Invalid routerId for routerName {}", routerName);
             return;
         }
-        BigInteger dpnId = NatUtil.getPrimaryNaptfromRouterName(dataBroker, routerName);
+        /* Get Primary Napt Switch for existing router from "router-to-napt-switch" DS.
+         * if dpnId value is null or zero then go for electing new Napt switch for existing router.
+         */
+        BigInteger dpnId = getPrimaryNaptSwitch(routerName, routerId);
+        if (dpnId == null || dpnId.equals(BigInteger.ZERO)) {
+            LOG.error("NAT Service: Failed to get or allocate NAPT switch for router {} during Update()", routerName);
+            return;
+        }
         Uuid networkId = original.getNetworkId();
 
         // Check if its update on SNAT flag
@@ -1155,15 +1162,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
                     List<String> externalIps = NatUtil.getExternalIpsForRouter(dataBroker, routerId);
                     handleDisableSnat(original, networkUuid, externalIps, false, null, dpnId);
                 } else {
-                    // Allocate Primary Napt Switch for existing router
-                    BigInteger primarySwitchId = getPrimaryNaptSwitch(routerName, routerId);
-                    if (primarySwitchId == null || primarySwitchId.equals(BigInteger.ZERO)) {
-                        LOG.error("NAT Service: Failed to get or allocated NAPT switch in"
-                                + " ExternalRouterListener.Update()");
-                        return;
-                    }
                     LOG.info("NAT Service : SNAT enabled for Router {}", original.getRouterName());
-                    handleEnableSnat(original, routerId, primarySwitchId);
+                    handleEnableSnat(original, routerId, dpnId);
                 }
             }
 
diff --git a/vpnservice/natservice/natservice-impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptManager.java b/vpnservice/natservice/natservice-impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptManager.java
index 50f035c8fb..dc623798af 100644
--- a/vpnservice/natservice/natservice-impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptManager.java
+++ b/vpnservice/natservice/natservice-impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptManager.java
@@ -703,26 +703,33 @@ public class NaptManager {
                     updateCounter(segmentId, externalIp, false);
                 }
             }
+            // remove from ipmap DS
+            LOG.debug("NAPT Service : Removing Ipmap for router {} from datastore", segmentId);
+            MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.OPERATIONAL, id);
         }
-        // remove from ipmap DS
-        LOG.debug("NAPT Service : Removing Ipmap for router {} from datastore", segmentId);
-        MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.OPERATIONAL, id);
     }
 
     void removeIpPortMappingForRouterID(long segmentId) {
         InstanceIdentifier<IpPortMapping> idBuilder = InstanceIdentifier.builder(IntextIpPortMap.class)
             .child(IpPortMapping.class, new IpPortMappingKey(segmentId)).build();
-        // remove from IntExtIpPortmap DS
-        LOG.debug("NAPT Service : Removing IntExtIpPort map for router {} from datastore", segmentId);
-        MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, idBuilder);
+        Optional<IpPortMapping> ipPortMapping = MDSALUtil.read(dataBroker, LogicalDatastoreType.CONFIGURATION,
+                idBuilder);
+        if (ipPortMapping.isPresent()) {
+            // remove from IntExtIpPortmap DS
+            LOG.debug("NAPT Service : Removing IntExtIpPort map for router {} from datastore", segmentId);
+            MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, idBuilder);
+        }
     }
 
     void removeIntIpPortMappingForRouterID(long segmentId) {
         InstanceIdentifier<IntipPortMap> intIp = InstanceIdentifier.builder(SnatintIpPortMap.class)
             .child(IntipPortMap.class, new IntipPortMapKey(segmentId)).build();
-        // remove from SnatIntIpPortmap DS
-        LOG.debug("NAPT Service : Removing SnatIntIpPort from datastore : {}", intIp);
-        MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, intIp);
+        Optional<IntipPortMap> intIpPortMap = MDSALUtil.read(dataBroker, LogicalDatastoreType.CONFIGURATION, intIp);
+        if (intIpPortMap.isPresent()) {
+            // remove from SnatIntIpPortmap DS
+            LOG.debug("NAPT Service : Removing SnatIntIpPort from datastore : {}", intIp);
+            MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.CONFIGURATION, intIp);
+        }
     }
 
     void removePortFromPool(String internalIpPort, String externalIp) {