VPNInstance IP Addr Family update is not working

Issue:
======
When router is getting associated with BGP-VPN(External BGP-VPN)
the corresponding external BGP-VPN
instance(odl-l3vpn:vpn-instance-op-data)
needs to be updated with IP address family based on the
attached subnets IP address family type either IPv4 or
IPv6 or both of them.

Currently updating the "vpn-instance-op-data" with
address family is not working as expected for some
times in odl cluster mode. (Sporadically this issue is happening)

If "vpn-instance-op-data" address family is not proper
then "ebgp:bgp" config data store also not proper as a
result address family (AFI/SAFI) is missing either IPv4
or IPv6 in ebgp data store in dual stack network use cases.

Solution:
============
As part of this issue fix reading "vpn-instance-op-data"
is avoided completely for updating VPN instance address family.
Since every time read and write into the DS is leading the
problem for dual stack network use cases.

So decided to re-implement the different approach of updating
the VPN Instance Op Data for address family. This will solve
race-condition between VPN Instance Op Data read and write.

Issue: NETVIRT-1410

Change-Id: I9dda8beeb74aaa273db800964b5e741478db36a3
Signed-off-by: Karthikeyan Krishnan <karthikeyangceb007@gmail.com>

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronPortChangeListener.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronPortChangeListener.java
index 13fbb7fea781477fa1a7320fe930f7e6ac300fe2..ec9611404a6ef73af877ffe7f2e0d41a94648b3f 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronPortChangeListener.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronPortChangeListener.java
@@ -310,24 +310,21 @@ public class NeutronPortChangeListener extends AsyncDataTreeChangeListenerBase<P
                 }
                 if (portIsIpv6) {
                     listVpnIds.add(internetVpnId);
-                    if (neutronvpnUtils.shouldVpnHandleIpVersionChoiceChangeToAdd(
-                                     IpVersionChoice.IPV6, internetVpnId)) {
-                        neutronvpnUtils.updateVpnInstanceWithIpFamily(internetVpnId.getValue(),
-                                                                     IpVersionChoice.IPV6, true);
+                    if (neutronvpnUtils.shouldVpnHandleIpVersionChoiceChange(
+                                     IpVersionChoice.IPV6, routerId, true)) {
+                        neutronvpnUtils.updateVpnInstanceWithIpFamily(internetVpnId.getValue(), IpVersionChoice.IPV6,
+                                true);
                         neutronvpnUtils.updateVpnInstanceWithFallback(internetVpnId.getValue(), true);
                     }
                 }
                 if (! subnetMapList.isEmpty()) {
                     nvpnManager.createVpnInterface(listVpnIds, routerPort, null);
                 }
+                IpVersionChoice ipVersion = IpVersionChoice.UNDEFINED;
                 for (FixedIps portIP : routerPort.getFixedIps()) {
                     String ipValue = portIP.getIpAddress().stringValue();
-                    IpVersionChoice version = NeutronvpnUtils.getIpVersionFromString(ipValue);
-                    if (neutronvpnUtils.shouldVpnHandleIpVersionChoiceChangeToAdd(version, vpnId)) {
-                        neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(),
-                               version, true);
-                    }
-                    if (version.isIpVersionChosen(IpVersionChoice.IPV4)) {
+                    ipVersion = NeutronvpnUtils.getIpVersionFromString(ipValue);
+                    if (ipVersion.isIpVersionChosen(IpVersionChoice.IPV4)) {
                         nvpnManager.addSubnetToVpn(vpnId, portIP.getSubnetId(),
                                                         null /* internet-vpn-id */);
                     } else {
@@ -337,6 +334,11 @@ public class NeutronPortChangeListener extends AsyncDataTreeChangeListenerBase<P
                             ipValue, routerPort.getMacAddress(),
                             routerPort.getUuid().getValue(), vpnId.getValue());
                 }
+                if (neutronvpnUtils.shouldVpnHandleIpVersionChoiceChange(ipVersion, routerId, true)) {
+                    LOG.debug("vpnInstanceOpDataEntry is getting update with ip address family {} for VPN {}",
+                            ipVersion, vpnId.getValue());
+                    neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), ipVersion, true);
+                }
                 nvpnManager.addToNeutronRouterInterfacesMap(routerId, routerPort.getUuid().getValue());
                 jobCoordinator.enqueueJob(routerId.toString(), () -> {
                     nvpnNatManager.handleSubnetsForExternalRouter(routerId);
@@ -388,21 +390,16 @@ public class NeutronPortChangeListener extends AsyncDataTreeChangeListenerBase<P
             // update RouterInterfaces map
             ListenableFutures.addErrorLogging(txRunner.callWithNewWriteOnlyTransactionAndSubmit(CONFIGURATION,
                 confTx -> {
-                    boolean vpnInstanceIpVersionRemoved = false;
-                    IpVersionChoice vpnInstanceIpVersionToRemove = IpVersionChoice.UNDEFINED;
+                    IpVersionChoice ipVersion = IpVersionChoice.UNDEFINED;
                     for (FixedIps portIP : portIps) {
                         Subnetmap sn = neutronvpnUtils.getSubnetmap(portIP.getSubnetId());
                         // router Port have either IPv4 or IPv6, never both
-                        if (neutronvpnUtils.shouldVpnHandleIpVersionChangeToRemove(sn, vpnId)) {
-                            vpnInstanceIpVersionRemoved = true;
-                            vpnInstanceIpVersionToRemove = NeutronvpnUtils.getIpVersionFromString(sn.getSubnetIp());
-                        }
+                        ipVersion = neutronvpnUtils.getIpVersionFromString(sn.getSubnetIp());
                         String ipValue = portIP.getIpAddress().stringValue();
                         neutronvpnUtils.removeVpnPortFixedIpToPort(vpnId.getValue(), ipValue, confTx);
                         // NOTE:  Please donot change the order of calls to removeSubnetFromVpn and
                         // and updateSubnetNodeWithFixedIP
-                        nvpnManager.removeSubnetFromVpn(vpnId, portIP.getSubnetId(),
-                                sn != null ? sn.getInternetVpnId() : null);
+                        nvpnManager.removeSubnetFromVpn(vpnId, portIP.getSubnetId(), sn.getInternetVpnId());
                         nvpnManager.updateSubnetNodeWithFixedIp(portIP.getSubnetId(), null, null,
                             null, null, null);
                     }
@@ -413,14 +410,15 @@ public class NeutronPortChangeListener extends AsyncDataTreeChangeListenerBase<P
                         nvpnNatManager.handleSubnetsForExternalRouter(routerId);
                         return Collections.emptyList();
                     });
-                    if (vpnInstanceIpVersionRemoved) {
-                        neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), vpnInstanceIpVersionToRemove,
-                                false);
+                    if (neutronvpnUtils.shouldVpnHandleIpVersionChoiceChange(ipVersion, routerId, false)) {
+                        LOG.debug("vpnInstanceOpDataEntry is getting update with ip address family {} for VPN {}",
+                                ipVersion, vpnId.getValue());
+                        neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), ipVersion, false);
                     }
                 }), LOG, "Error handling interface removal");
             if (vpnInstanceInternetIpVersionRemoved) {
-                neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnInstanceInternetUuid.getValue(),
-                        IpVersionChoice.IPV6, false);
+                neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnInstanceInternetUuid.getValue(), IpVersionChoice.IPV6,
+                         false);
                 neutronvpnUtils.updateVpnInstanceWithFallback(vpnInstanceInternetUuid.getValue(), false);
             }
         }

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java
index 1bd1742f781a298e5f84675afe3d091fb5d4ed0f..419da4fd9b7f9c2c4fef7b462583784b908fb118 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java
@@ -2260,14 +2260,20 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
     protected void associateRouterToVpn(Uuid vpnId, Uuid routerId) {
         updateVpnMaps(vpnId, null, routerId, null, null);
         LOG.debug("Updating association of subnets to external vpn {}", vpnId.getValue());
-        List<Uuid> routerSubnets = neutronvpnUtils.getNeutronRouterSubnetIds(routerId);
-        for (Uuid subnetId : routerSubnets) {
-            Subnetmap sn = updateVpnForSubnet(routerId, vpnId, subnetId, true);
-            if (neutronvpnUtils.shouldVpnHandleIpVersionChangeToAdd(sn, vpnId)) {
-                neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(),
-                          NeutronvpnUtils.getIpVersionFromString(sn.getSubnetIp()), true);
+        IpVersionChoice ipVersion = IpVersionChoice.UNDEFINED;
+        List<Subnetmap> subMapList = neutronvpnUtils.getNeutronRouterSubnetMapList(routerId);
+        for (Subnetmap sn : subMapList) {
+            updateVpnForSubnet(routerId, vpnId, sn.getId(), true);
+            IpVersionChoice ipVers = neutronvpnUtils.getIpVersionFromString(sn.getSubnetIp());
+            if (!ipVersion.isIpVersionChosen(ipVers)) {
+                ipVersion = ipVersion.addVersion(ipVers);
             }
         }
+        if (ipVersion != IpVersionChoice.UNDEFINED) {
+            LOG.debug("vpnInstanceOpDataEntry is getting update with ip address family {} for VPN {} ", ipVersion,
+                    vpnId);
+            neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), ipVersion, true);
+        }
 
         try {
             checkAndPublishRouterAssociatedtoVpnNotification(routerId, vpnId);
@@ -2298,22 +2304,21 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
     @SuppressWarnings("checkstyle:IllegalCatch")
     protected void dissociateRouterFromVpn(Uuid vpnId, Uuid routerId) {
 
-        List<Uuid> routerSubnets = neutronvpnUtils.getNeutronRouterSubnetIds(routerId);
-        boolean vpnInstanceIpVersionsRemoved = false;
-        IpVersionChoice vpnInstanceIpVersionsToRemove = IpVersionChoice.UNDEFINED;
-        for (Uuid subnetId : routerSubnets) {
-            Subnetmap sn = neutronvpnUtils.getSubnetmap(subnetId);
-            if (neutronvpnUtils.shouldVpnHandleIpVersionChangeToRemove(sn, vpnId)) {
-                vpnInstanceIpVersionsToRemove = vpnInstanceIpVersionsToRemove.addVersion(NeutronvpnUtils
-                        .getIpVersionFromString(sn.getSubnetIp()));
-                vpnInstanceIpVersionsRemoved = true;
+        List<Subnetmap> subMapList = neutronvpnUtils.getNeutronRouterSubnetMapList(routerId);
+        IpVersionChoice ipVersion = IpVersionChoice.UNDEFINED;
+        for (Subnetmap sn : subMapList) {
+            IpVersionChoice ipVers = neutronvpnUtils.getIpVersionFromString(sn.getSubnetIp());
+            if (ipVersion.isIpVersionChosen(ipVers)) {
+                ipVersion = ipVersion.addVersion(ipVers);
             }
             LOG.debug("Updating association of subnets to internal vpn {}", routerId.getValue());
-            updateVpnForSubnet(vpnId, routerId, subnetId, false);
+            updateVpnForSubnet(vpnId, routerId, sn.getId(), false);
         }
-
-        if (vpnInstanceIpVersionsRemoved) {
-            neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), vpnInstanceIpVersionsToRemove, false);
+        if (ipVersion != IpVersionChoice.UNDEFINED) {
+            LOG.debug("vpnInstanceOpDataEntry is getting update with ip address family {} for VPN {} ",
+                    ipVersion, vpnId);
+            neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), ipVersion,
+                    false);
         }
         clearFromVpnMaps(vpnId, routerId, null);
         try {
@@ -2398,6 +2403,7 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
                 if (vpnManager.checkForOverlappingSubnets(nw, subnetmapList, vpnId, routeTargets, failedNwList)) {
                     continue;
                 }
+                IpVersionChoice ipVersion = IpVersionChoice.UNDEFINED;
                 for (Subnetmap subnetmap : subnetmapList) {
                     Uuid subnetId = subnetmap.getId();
                     Uuid subnetVpnId = neutronvpnUtils.getVpnForSubnet(subnetId);
@@ -2408,9 +2414,9 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
                                 + " as it is already associated", subnetId.getValue(), vpnId.getValue()));
                         continue;
                     }
-                    if (neutronvpnUtils.shouldVpnHandleIpVersionChangeToAdd(subnetmap, vpnId)) {
-                        neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(),
-                                NeutronvpnUtils.getIpVersionFromString(subnetmap.getSubnetIp()), true);
+                    IpVersionChoice ipVers = neutronvpnUtils.getIpVersionFromString(subnetmap.getSubnetIp());
+                    if (!ipVersion.isIpVersionChosen(ipVers)) {
+                        ipVersion = ipVersion.addVersion(ipVers);
                     }
                     if (!NeutronvpnUtils.getIsExternal(network)) {
                         LOG.debug("associateNetworksToVpn: Add subnet {} to VPN {}", subnetId.getValue(),
@@ -2421,6 +2427,11 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
                         passedNwList.add(nw);
                     }
                 }
+                if (ipVersion != IpVersionChoice.UNDEFINED) {
+                    LOG.debug("vpnInstanceOpDataEntry is getting update with ip address family {} for VPN {} ",
+                            ipVersion, vpnId);
+                    neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), ipVersion, true);
+                }
                 passedNwList.add(nw);
             }
         } catch (ReadFailedException e) {
@@ -2521,13 +2532,12 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
                 }
             }
             Set<VpnTarget> routeTargets = vpnManager.getRtListForVpn(vpnId.getValue());
+            IpVersionChoice ipVersion = IpVersionChoice.UNDEFINED;
             for (Uuid subnet : networkSubnets) {
                 Subnetmap subnetmap = neutronvpnUtils.getSubnetmap(subnet);
-                if (neutronvpnUtils.shouldVpnHandleIpVersionChangeToRemove(subnetmap, vpnId)) {
-                    IpVersionChoice ipVersionsToRemove = IpVersionChoice.UNDEFINED;
-                    IpVersionChoice ipVersion = NeutronvpnUtils.getIpVersionFromString(subnetmap.getSubnetIp());
-                    neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(),
-                        ipVersionsToRemove.addVersion(ipVersion), false);
+                IpVersionChoice ipVers = neutronvpnUtils.getIpVersionFromString(subnetmap.getSubnetIp());
+                if (!ipVersion.isIpVersionChosen(ipVers)) {
+                    ipVersion = ipVersion.addVersion(ipVers);
                 }
                 LOG.debug("dissociateNetworksFromVpn: Withdraw subnet {} from VPN {}", subnet.getValue(),
                           vpnId.getValue());
@@ -2536,6 +2546,11 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
                         vpnId.getValue());
                 passedNwList.add(nw);
             }
+            if (ipVersion != IpVersionChoice.UNDEFINED) {
+                LOG.debug("vpnInstanceOpDataEntry is getting update with ip address family {} for VPN {}",
+                        ipVersion, vpnId);
+                neutronvpnUtils.updateVpnInstanceWithIpFamily(vpnId.getValue(), ipVersion, false);
+            }
         }
         clearFromVpnMaps(vpnId, null, new ArrayList<>(passedNwList));
         LOG.info("Network(s) {} disassociated from L3VPN {} successfully", passedNwList.toString(),

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
index ecc3e76cd6e7f7913106023144ba2915e7c35b67..992ec7e70691247c5411dce9cbd2a21a7e59f760 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java
@@ -780,6 +780,21 @@ public class NeutronvpnUtils {
         return subnet;
     }
 
+    protected List<Subnetmap> getNeutronRouterSubnetMapList(Uuid routerId) {
+        List<Subnetmap> subnetMapList = new ArrayList<>();
+        Optional<Subnetmaps> subnetMaps = read(LogicalDatastoreType.CONFIGURATION,
+                InstanceIdentifier.builder(Subnetmaps.class).build());
+        if (subnetMaps.isPresent() && subnetMaps.get().getSubnetmap() != null) {
+            for (Subnetmap subnetmap : subnetMaps.get().getSubnetmap()) {
+                if (routerId.equals(subnetmap.getRouterId())) {
+                    subnetMapList.add(subnetmap);
+                }
+            }
+        }
+        LOG.debug("getNeutronRouterSubnetMapList returns {}", subnetMapList);
+        return subnetMapList;
+    }
+
     @Nonnull
     protected List<Uuid> getNeutronRouterSubnetIds(Uuid routerId) {
         LOG.debug("getNeutronRouterSubnetIds for {}", routerId.getValue());
@@ -1438,40 +1453,29 @@ public class NeutronvpnUtils {
                 .child(VpnInstanceOpDataEntry.class, new VpnInstanceOpDataEntryKey(primaryRd)).build();
     }
 
-    public boolean shouldVpnHandleIpVersionChangeToAdd(Subnetmap sm, Uuid vpnId) {
-        if (sm == null) {
-            return false;
-        }
-        IpVersionChoice ipVersion = getIpVersionFromString(sm.getSubnetIp());
-        return shouldVpnHandleIpVersionChoiceChangeToAdd(ipVersion, vpnId);
-    }
-
-    public boolean shouldVpnHandleIpVersionChoiceChangeToAdd(IpVersionChoice ipVersion, Uuid vpnId) {
-        VpnInstanceOpDataEntry vpnInstanceOpDataEntry = getVpnInstanceOpDataEntryFromVpnId(vpnId.getValue());
-        if (vpnInstanceOpDataEntry == null) {
-            return false;
-        }
-        if (vpnInstanceOpDataEntry.getType() == VpnInstanceOpDataEntry.Type.L2) {
-            LOG.error("shouldVpnHandleIpVersionChangeToAdd: {} "
-                    + "VpnInstanceOpDataEntry is L2 instance. Do nothing.", vpnId.getValue());
+    public boolean shouldVpnHandleIpVersionChoiceChange(IpVersionChoice ipVersion, Uuid routerId, boolean add) {
+        int subnetCount = -1;
+        if (ipVersion.isIpVersionChosen(IpVersionChoice.IPV4)) {
+            subnetCount = getSubnetCountFromRouter(routerId, ipVersion);
+        } else if (ipVersion.isIpVersionChosen(IpVersionChoice.IPV6)) {
+            subnetCount = getSubnetCountFromRouter(routerId, ipVersion);
+        } else {
+            //Possible value of ipversion choice is either V4 or V6 only. Not accepted V4andV6 and Undefined
             return false;
         }
-        boolean isIpv4Configured = vpnInstanceOpDataEntry.isIpv4Configured();
-        boolean isVpnInstanceIpv4Changed = false;
-        if (ipVersion.isIpVersionChosen(IpVersionChoice.IPV4) && !isIpv4Configured) {
-            isVpnInstanceIpv4Changed = true;
-        }
-        boolean isIpv6Configured = vpnInstanceOpDataEntry.isIpv6Configured();
-        boolean isVpnInstanceIpv6Changed = false;
-        if (ipVersion.isIpVersionChosen(IpVersionChoice.IPV6) && !isIpv6Configured) {
-            isVpnInstanceIpv6Changed = true;
-        }
-        if (!isVpnInstanceIpv4Changed && !isVpnInstanceIpv6Changed) {
-            LOG.debug("shouldVpnHandleIpVersionChangeToAdd: VPN {} did not change with IpFamily {}",
-                  vpnId.getValue(), ipVersion.toString());
+        /* ADD: Update vpnInstanceOpDataEntry with address family only on first IPv4/IPv6 subnet
+         * for the VPN Instance.
+         *
+         * REMOVE: Update vpnInstanceOpDataEntry with address family only on last IPv4/IPv6 subnet
+         * for the VPN Instance.
+         */
+        if (add && subnetCount == 1) {
+            return true;
+        } else if (!add && subnetCount == 0) {
+            return true;
+        } else {
             return false;
         }
-        return true;
     }
 
     public boolean shouldVpnHandleIpVersionChangeToRemove(Subnetmap sm, Uuid vpnId) {
@@ -1500,6 +1504,21 @@ public class NeutronvpnUtils {
         return false;
     }
 
+    public int getSubnetCountFromRouter(Uuid routerId, IpVersionChoice ipVer) {
+        List<Subnetmap> subnetMapList = getNeutronRouterSubnetMapList(routerId);
+        int subnetCount = 0;
+        for (Subnetmap subMap : subnetMapList) {
+            IpVersionChoice ipVersion = getIpVersionFromString(subMap.getSubnetIp());
+            if (ipVersion.isIpVersionChosen(ipVer)) {
+                subnetCount++;
+            }
+            if (subnetCount > 1) {
+                break;
+            }
+        }
+        return subnetCount;
+    }
+
     public void updateVpnInstanceWithIpFamily(String vpnName, IpVersionChoice ipVersion, boolean add) {
         VpnInstanceOpDataEntry vpnInstanceOpDataEntry = getVpnInstanceOpDataEntryFromVpnId(vpnName);
         if (vpnInstanceOpDataEntry == null) {
@@ -1507,33 +1526,33 @@ public class NeutronvpnUtils {
         }
         if (vpnInstanceOpDataEntry.getType() == VpnInstanceOpDataEntry.Type.L2) {
             LOG.debug("updateVpnInstanceWithIpFamily: Update VpnInstance {} with ipFamily {}."
-                            + "VpnInstanceOpDataEntry is L2 instance. Do nothing.", vpnName,
-                    ipVersion.toString());
+                    + "VpnInstanceOpDataEntry is L2 instance. Do nothing.", vpnName, ipVersion);
+            return;
+        }
+        if (ipVersion == IpVersionChoice.UNDEFINED) {
+            LOG.debug("updateVpnInstanceWithIpFamily: Update VpnInstance {} with Undefined address family"
+                    + "is not allowed. Do nothing", vpnName);
             return;
         }
-        final boolean isFinalVpnInstanceIpv6Changed = ipVersion
-                .isIpVersionChosen(IpVersionChoice.IPV6) ? true : false;
-        final boolean isFinalVpnInstanceIpv4Changed = ipVersion
-                .isIpVersionChosen(IpVersionChoice.IPV4) ? true : false;
-        final boolean finalIsIpv4Configured = ipVersion.isIpVersionChosen(IpVersionChoice.IPV4) ? add : false;
-        final boolean finalIsIpv6Configured = ipVersion.isIpVersionChosen(IpVersionChoice.IPV6) ? add : false;
         jobCoordinator.enqueueJob("VPN-" + vpnName, () -> {
             VpnInstanceOpDataEntryBuilder builder = new VpnInstanceOpDataEntryBuilder(vpnInstanceOpDataEntry);
-            if (isFinalVpnInstanceIpv4Changed) {
-                builder.setIpv4Configured(finalIsIpv4Configured);
-            }
-            if (isFinalVpnInstanceIpv6Changed) {
-                builder.setIpv6Configured(finalIsIpv6Configured);
+            boolean ipConfigured = add;
+            if (ipVersion.isIpVersionChosen(IpVersionChoice.IPV4AND6)) {
+                builder.setIpv4Configured(ipConfigured);
+                builder.setIpv6Configured(ipConfigured);
+            } else if (ipVersion.isIpVersionChosen(IpVersionChoice.IPV4)) {
+                builder.setIpv4Configured(ipConfigured);
+            } else if (ipVersion.isIpVersionChosen(IpVersionChoice.IPV6)) {
+                builder.setIpv6Configured(ipConfigured);
             }
             return Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(
-                OPERATIONAL, tx -> {
-                    InstanceIdentifier<VpnInstanceOpDataEntry> id = InstanceIdentifier.builder(VpnInstanceOpData.class)
-                        .child(VpnInstanceOpDataEntry.class,
-                                new VpnInstanceOpDataEntryKey(vpnInstanceOpDataEntry.getVrfId())).build();
+                    OPERATIONAL, tx -> {
+                    InstanceIdentifier<VpnInstanceOpDataEntry> id = InstanceIdentifier
+                            .builder(VpnInstanceOpData.class).child(VpnInstanceOpDataEntry.class,
+                                        new VpnInstanceOpDataEntryKey(vpnInstanceOpDataEntry.getVrfId())).build();
                     tx.merge(id, builder.build(), false);
                     LOG.info("updateVpnInstanceWithIpFamily: Successfully {} {} to Vpn {}",
-                            add ? "added" : "removed",
-                            ipVersion.toString(), vpnName);
+                            add == true ? "added" : "removed", ipVersion, vpnName);
                 }));
         });
     }