From: Vladimir Lavor Date: Sat, 9 Jul 2016 16:31:07 +0000 (+0200) Subject: ios-xe renderer bugfix #2 X-Git-Tag: release/boron~49^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=dd09640a4f2951716fb1fa6e170f038930cd6418;p=groupbasedpolicy.git ios-xe renderer bugfix #2 - policyWriter is created per interface, not per node - removed class-default entry from policy-map - service-paths created/removed by gbp will not overwrite or remove service-paths from sfc - remote sff could be removed - no more duplicities when writing/removing policy/sfc Change-Id: I79a2a86dcf4af86064d33640d5978b0fb5ddf2cb Signed-off-by: Vladimir Lavor --- diff --git a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/manager/PolicyManagerImpl.java b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/manager/PolicyManagerImpl.java index 6873c6e05..5934b02a2 100644 --- a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/manager/PolicyManagerImpl.java +++ b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/manager/PolicyManagerImpl.java @@ -56,7 +56,7 @@ import com.google.common.util.concurrent.ListenableFuture; public class PolicyManagerImpl implements PolicyManager { private static final Logger LOG = LoggerFactory.getLogger(PolicyManagerImpl.class); - private static final String policyMapName = "service-chains"; + private static final String BASE_POLICY_MAP_NAME = "service-chains-"; private final DataBroker dataBroker; private final NodeManager nodeManager; @@ -103,9 +103,9 @@ public class PolicyManagerImpl implements PolicyManager { } final PolicyConfigurationContext context = new PolicyConfigurationContext(); - final Map policyWriterPerDeviceCache = new HashMap<>(); + final Map policyWriterPerDeviceCache = new HashMap<>(); for (RendererEndpoint rendererEndpoint : dataAfter.getRendererEndpoints().getRendererEndpoint()) { - // store the endpoint currently being configured + // Store the endpoint currently being configured context.setCurrentRendererEP(rendererEndpoint); if (dataAfter.getEndpoints() == null || dataAfter.getEndpoints().getAddressEndpointWithLocation() == null) { @@ -122,25 +122,31 @@ public class PolicyManagerImpl implements PolicyManager { context.appendUnconfiguredRendererEP(StatusUtil.assembleFullyNotConfigurableRendererEP(context, info)); continue; } - - // Find policy writer - PolicyWriter policyWriter = policyWriterPerDeviceCache.get(mountpoint); + // Generate policy writer key - policy map name, composed from base value, interface name and node id + final String interfaceName = PolicyManagerUtil.getInterfaceNameForPolicyMap(rendererEndpoint, endpointsWithLocation); + final NodeId nodeId = nodeManager.getNodeIdByMountpointIid(mountpointIid); + if (interfaceName == null || nodeId == null) { + LOG.warn("Cannot compose policy-map, missing value. Interface: {}, NodeId: {}", interfaceName, nodeId); + continue; + } + final String policyMapName = BASE_POLICY_MAP_NAME.concat(interfaceName); + final String policyWriterKey = policyMapName.concat(nodeId.getValue()); + // Find appropriate writer + PolicyWriter policyWriter = policyWriterPerDeviceCache.get(policyWriterKey); if (policyWriter == null) { // Initialize new policy writer - final String interfaceName = PolicyManagerUtil.getInterfaceNameForPolicyMap(rendererEndpoint, endpointsWithLocation); - final NodeId nodeId = nodeManager.getNodeIdByMountpointIid(mountpointIid); final String managementIpAddress = nodeManager.getNodeManagementIpByMountPointIid(mountpointIid); - if (interfaceName == null || managementIpAddress == null) { - final String info = String.format("can not create policyWriter: interface=%s, managementIpAddress=%s", - interfaceName, managementIpAddress); + if (managementIpAddress == null) { + final String info = String.format("can not create policyWriter, managementIpAddress for mountpoint %s is null", + mountpointIid); context.appendUnconfiguredRendererEP(StatusUtil.assembleFullyNotConfigurableRendererEP(context, info)); continue; } policyWriter = new PolicyWriter(mountpoint, interfaceName, managementIpAddress, policyMapName, nodeId); - policyWriterPerDeviceCache.put(mountpoint, policyWriter); + policyWriterPerDeviceCache.put(policyWriterKey, policyWriter); } - // assign policyWriter for current mount-point + // Assign policyWriter for current policy-map context.setPolicyWriter(policyWriter); final Sgt sourceSgt = PolicyManagerUtil.findSgtTag(rendererEndpoint, dataAfter.getEndpoints() @@ -163,6 +169,7 @@ public class PolicyManagerImpl implements PolicyManager { final List> allFutureResults = new ArrayList<>(); if (action.equals(Create)) { + // TODO ensure that last transaction is done before the next one starts policyWriterPerDeviceCache.values().forEach((pw) -> allFutureResults.add(pw.commitToDatastore())); } else if (action.equals(Delete)) { policyWriterPerDeviceCache.values().forEach((pw) -> allFutureResults.add(pw.removeFromDatastore())); diff --git a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/util/PolicyManagerUtil.java b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/util/PolicyManagerUtil.java index 520784b41..5280daee4 100644 --- a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/util/PolicyManagerUtil.java +++ b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/util/PolicyManagerUtil.java @@ -80,7 +80,6 @@ import org.slf4j.LoggerFactory; public class PolicyManagerUtil { private static final Logger LOG = LoggerFactory.getLogger(PolicyManagerUtil.class); - private static final String DEFAULT = "class-default"; public static void syncPolicyEntities(final Sgt sourceSgt, final Sgt destinationSgt, final PolicyConfigurationContext context, final Configuration dataAfter, final PeerEndpoint peerEndpoint, @@ -187,19 +186,16 @@ public class PolicyManagerUtil { return policyClassBuilder.build(); } - public static PolicyMap createPolicyMap(final String policyMapName, final List policyMapEntries) { - // Create default class entry - final ClassBuilder defaultBuilder = new ClassBuilder(); - defaultBuilder.setName(new ClassNameType(DEFAULT)) - .setKey(new ClassKey(new ClassNameType(DEFAULT))); - // TODO add pass-through value - policyMapEntries.add(defaultBuilder.build()); + public static PolicyMap createPolicyMap(final String policyMapName, final Set policyMapEntries) { + // TODO maybe could be better to create also class-default entry with pass-through value than not to create any default entry at all + final List policyMapEntriesList = new ArrayList<>(policyMapEntries); + // Construct policy map final PolicyMapBuilder policyMapBuilder = new PolicyMapBuilder(); policyMapBuilder.setName(policyMapName) .setKey(new PolicyMapKey(policyMapName)) .setType(PolicyMap.Type.ServiceChain) - .setXmlClass(policyMapEntries); + .setXmlClass(policyMapEntriesList); return policyMapBuilder.build(); } diff --git a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/util/ServiceChainingUtil.java b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/util/ServiceChainingUtil.java index 33c22b760..cef2b124c 100644 --- a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/util/ServiceChainingUtil.java +++ b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/util/ServiceChainingUtil.java @@ -57,6 +57,7 @@ import org.opendaylight.yang.gen.v1.urn.ios.rev160308._native.policy.map.Class; import org.opendaylight.yang.gen.v1.urn.ios.rev160308._native.service.chain.ServicePath; import org.opendaylight.yang.gen.v1.urn.ios.rev160308._native.service.chain.ServicePathBuilder; import org.opendaylight.yang.gen.v1.urn.ios.rev160308._native.service.chain.ServicePathKey; +import org.opendaylight.yang.gen.v1.urn.ios.rev160308._native.service.chain.service.function.forwarder.ServiceFfName; import org.opendaylight.yang.gen.v1.urn.ios.rev160308._native.service.chain.service.function.forwarder.ServiceFfNameBuilder; import org.opendaylight.yang.gen.v1.urn.ios.rev160308._native.service.chain.service.function.forwarder.ServiceFfNameKey; import org.opendaylight.yang.gen.v1.urn.ios.rev160308._native.service.chain.service.path.ConfigServiceChainPathModeBuilder; @@ -182,36 +183,72 @@ public class ServiceChainingUtil { if (tenantId == null) { return; } - // Cache class-maps, appropriate policy-map entries and service-chains + // Cache class-maps, appropriate policy-map entries and service-path (if there are some created by gbp) final List policyMapEntries = new ArrayList<>(); final String classMapName = PolicyManagerUtil.generateClassMapName(sourceSgt.getValue(), destinationSgt.getValue()); final ClassMap classMap = PolicyManagerUtil.createClassMap(classMapName, null); - final RspName rspName = generateRspName(servicePath, tenantId); - final ServiceChain serviceChain = findServiceChainToRsp(rspName); - policyMapEntries.add(PolicyManagerUtil.createPolicyEntry(classMapName, null, PolicyManagerImpl.ActionCase.CHAIN)); policyWriter.cache(classMap); - policyWriter.cache(serviceChain); + policyMapEntries.add(PolicyManagerUtil.createPolicyEntry(classMapName, null, PolicyManagerImpl.ActionCase.CHAIN)); + + final RspName rspName = generateRspName(servicePath, tenantId); + final RenderedServicePath renderedServicePath = SfcProviderRenderedPathAPI.readRenderedServicePath(rspName); + final ServiceFunctionForwarder firstHopSff = getFirstHopSff(renderedServicePath); + if (firstHopSff != null && firstHopSff.getIpMgmtAddress() != null && + firstHopSff.getIpMgmtAddress().getIpv4Address() != null) { + final String sffMgmtIpValue = firstHopSff.getIpMgmtAddress().getIpv4Address().getValue(); + if (sffMgmtIpValue.equals(policyWriter.getManagementIpAddress())) { + // Remove service chain and remote forwarder + final ServiceChain serviceChain = findServiceChainToRsp(renderedServicePath); + final ServiceFfName remoteForwarder = findRemoteForwarder(firstHopSff); + policyWriter.cache(serviceChain); + policyWriter.cache(remoteForwarder); + } + } if (servicePath.isSymmetric()) { final String oppositeClassMapName = PolicyManagerUtil.generateClassMapName(destinationSgt.getValue(), sourceSgt.getValue()); final ClassMap oppositeClassMap = PolicyManagerUtil.createClassMap(oppositeClassMapName, null); - final RspName reversedRspName = generateReversedRspName(servicePath, tenantId); - final ServiceChain reversedServiceChain = findServiceChainToRsp(reversedRspName); - policyMapEntries.add(PolicyManagerUtil.createPolicyEntry(oppositeClassMapName, null, PolicyManagerImpl.ActionCase.CHAIN)); policyWriter.cache(oppositeClassMap); - policyWriter.cache(reversedServiceChain); + policyMapEntries.add(PolicyManagerUtil.createPolicyEntry(oppositeClassMapName, null, PolicyManagerImpl.ActionCase.CHAIN)); + + final RspName reversedRspName = generateReversedRspName(servicePath, tenantId); + final RenderedServicePath reversedRenderedServicePath = SfcProviderRenderedPathAPI.readRenderedServicePath(reversedRspName); + final ServiceFunctionForwarder reversedFirstHopSff = getFirstHopSff(reversedRenderedServicePath); + if (reversedFirstHopSff != null && reversedFirstHopSff.getIpMgmtAddress() != null && + reversedFirstHopSff.getIpMgmtAddress().getIpv4Address() != null) { + final String reversedSffMgmtIpValue = reversedFirstHopSff.getIpMgmtAddress().getIpv4Address().getValue(); + if (reversedSffMgmtIpValue.equals(policyWriter.getManagementIpAddress())) { + // Remove service chain and remote forwarder + final ServiceChain serviceChain = findServiceChainToRsp(renderedServicePath); + final ServiceFfName remoteForwarder = findRemoteForwarder(reversedFirstHopSff); + policyWriter.cache(serviceChain); + policyWriter.cache(remoteForwarder); + } + } } policyWriter.cache(policyMapEntries); - // TODO remove other sfc stuff - forwarders, etc. } - private static ServiceChain findServiceChainToRsp(final RspName rspName) { - // Do not actually remove rsp from DS, could be used by someone else - final RenderedServicePath renderedServicePath = SfcProviderRenderedPathAPI.readRenderedServicePath(rspName); - if (renderedServicePath == null) { - LOG.debug("Rendered service path not found, if there is service-path created according to that rsp, " + - "it cannot be removed. Rendered path name: {} ", rspName.getValue()); + private static ServiceFfName findRemoteForwarder(ServiceFunctionForwarder firstHopSff) { + ServiceFfNameBuilder serviceFfNameBuilder = new ServiceFfNameBuilder(); + serviceFfNameBuilder.setName(firstHopSff.getName().getValue()) + .setKey(new ServiceFfNameKey(firstHopSff.getName().getValue())); + return serviceFfNameBuilder.build(); + } + + private static ServiceFunctionForwarder getFirstHopSff(RenderedServicePath renderedServicePath) { + if (renderedServicePath == null || renderedServicePath.getRenderedServicePathHop() == null || + renderedServicePath.getRenderedServicePathHop().isEmpty()) { return null; } + final RenderedServicePathHop firstHop = renderedServicePath.getRenderedServicePathHop().get(0); + final SffName firstHopSff = firstHop.getServiceFunctionForwarder(); + if (firstHopSff == null) { + return null; + } + return SfcProviderServiceForwarderAPI.readServiceFunctionForwarder(firstHopSff); + } + + private static ServiceChain findServiceChainToRsp(final RenderedServicePath renderedServicePath) { // Construct service chain with key final Long pathId = renderedServicePath.getPathId(); final ServicePathBuilder servicePathBuilder = new ServicePathBuilder(); @@ -259,40 +296,6 @@ public class ServiceChainingUtil { return reversedRenderedPath; } - /** - * Method checks up, if some {@link ServicePath} is present on device. - * - * @param mountpoint used to access specific device - * @return true if service chain does not exist, is null or does not contain any service path. False otherwise - */ - public static boolean checkServicePathPresence(DataBroker mountpoint) { - InstanceIdentifier serviceChainIid = InstanceIdentifier.builder(Native.class) - .child(ServiceChain.class).build(); - java.util.Optional optionalTransaction = - NetconfTransactionCreator.netconfReadOnlyTransaction(mountpoint); - if (!optionalTransaction.isPresent()) { - LOG.warn("Failed to create transaction, mountpoint: {}", mountpoint); - return false; - } - ReadOnlyTransaction transaction = optionalTransaction.get(); - CheckedFuture, ReadFailedException> submitFuture = transaction.read(LogicalDatastoreType.CONFIGURATION, - serviceChainIid); - try { - Optional optionalServiceChain = submitFuture.checkedGet(); - if (optionalServiceChain.isPresent()) { - ServiceChain chain = optionalServiceChain.get(); - return chain == null || chain.getServicePath() == null || chain.getServicePath().isEmpty(); - } else { - return true; - } - } catch (ReadFailedException e) { - LOG.warn("Read transaction failed to {} ", e); - } catch (Exception e) { - LOG.error("Failed to .. {}", e.getMessage()); - } - return false; - } - static ServiceFunctionPath findServiceFunctionPath(final SfcName chainName) { final ServiceFunctionPaths allPaths = SfcProviderServicePathAPI.readAllServiceFunctionPaths(); for (ServiceFunctionPath serviceFunctionPath : allPaths.getServiceFunctionPath()) { @@ -368,14 +371,7 @@ public class ServiceChainingUtil { } final SffName sffName = firstHop.getServiceFunctionForwarder(); - // Forwarders - // - // If classifier node is also forwarder, first entry in service path has to point to first service function - // (Local case) - // - // If first hop Sff is on different node, first service path entry has to point to that specific service - // forwarder (Remote case) - + // Create remote forwarder if necessary final java.util.Optional serviceFunctionForwarder = java.util.Optional.ofNullable( SfcProviderServiceForwarderAPI.readServiceFunctionForwarder(sffName)); if (!serviceFunctionForwarder.isPresent()) { @@ -402,7 +398,6 @@ public class ServiceChainingUtil { .map(org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress::getIpv4Address) .map(Ipv4Address::getValue) .map(addressValue -> { - // Set up choice. If remote, this choice is overwritten final ServiceTypeChoice serviceTypeChoice; if (!addressValue.equals(policyWriter.getManagementIpAddress())) { final ServiceFfNameBuilder remoteSffBuilder = new ServiceFfNameBuilder(); @@ -411,29 +406,25 @@ public class ServiceChainingUtil { .setIp(new IpBuilder().setAddress(new Ipv4Address(remoteForwarderStringIp)).build()); policyWriter.cache(remoteSffBuilder.build()); serviceTypeChoice = forwarderTypeChoice(sffName.getValue()); - } else { - serviceTypeChoice = functionTypeChoice(firstHop.getServiceFunctionName().getValue()); + // Service chain + final List services = new ArrayList<>(); + final ServicesBuilder servicesBuilder = new ServicesBuilder(); + servicesBuilder.setServiceIndexId(renderedServicePath.getStartingIndex()) + .setServiceTypeChoice(serviceTypeChoice); + services.add(servicesBuilder.build()); + final List servicePaths = new ArrayList<>(); + final ServicePathBuilder servicePathBuilder = new ServicePathBuilder(); + servicePathBuilder.setKey(new ServicePathKey(renderedServicePath.getPathId())) + .setServicePathId(renderedServicePath.getPathId()) + .setConfigServiceChainPathMode(new ConfigServiceChainPathModeBuilder() + .setServiceIndex(new ServiceIndexBuilder() + .setServices(services).build()).build()); + servicePaths.add(servicePathBuilder.build()); + final ServiceChainBuilder chainBuilder = new ServiceChainBuilder(); + chainBuilder.setServicePath(servicePaths); + final ServiceChain serviceChain = chainBuilder.build(); + policyWriter.cache(serviceChain); } - - // Service chain - final List services = new ArrayList<>(); - final ServicesBuilder servicesBuilder = new ServicesBuilder(); - servicesBuilder.setServiceIndexId(renderedServicePath.getStartingIndex()) - .setServiceTypeChoice(serviceTypeChoice); - services.add(servicesBuilder.build()); - final List servicePaths = new ArrayList<>(); - final ServicePathBuilder servicePathBuilder = new ServicePathBuilder(); - servicePathBuilder.setKey(new ServicePathKey(renderedServicePath.getPathId())) - .setServicePathId(renderedServicePath.getPathId()) - .setConfigServiceChainPathMode(new ConfigServiceChainPathModeBuilder() - .setServiceIndex(new ServiceIndexBuilder() - .setServices(services).build()).build()); - servicePaths.add(servicePathBuilder.build()); - final ServiceChainBuilder chainBuilder = new ServiceChainBuilder(); - chainBuilder.setServicePath(servicePaths); - final ServiceChain serviceChain = chainBuilder.build(); - policyWriter.cache(serviceChain); - return true; }).orElseGet(createNegativePathWithLogSupplier(sffName.getValue(), (value) -> LOG.error("Cannot create remote forwarder, SFF {} does not contain management ip address", diff --git a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriter.java b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriter.java index 711b57638..034da8ac6 100644 --- a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriter.java +++ b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriter.java @@ -21,8 +21,9 @@ import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology. import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; public class PolicyWriter { @@ -30,10 +31,10 @@ public class PolicyWriter { private final DataBroker mountpoint; // Local cache - private final List classMapEntries; - private final List policyMapEntries; - private final List remoteForwarders; - private final List serviceChains; + private final Set classMapEntries; + private final Set policyMapEntries; + private final Set remoteForwarders; + private final Set serviceChains; private final NodeId nodeId; private final String interfaceName; private final String policyMapName; @@ -41,10 +42,10 @@ public class PolicyWriter { public PolicyWriter(final DataBroker dataBroker, final String interfaceName, final String ipAddress, final String policyMapName, final NodeId nodeId) { - classMapEntries = new ArrayList<>(); - policyMapEntries = new ArrayList<>(); - remoteForwarders = new ArrayList<>(); - serviceChains = new ArrayList<>(); + classMapEntries = new HashSet<>(); + policyMapEntries = new HashSet<>(); + remoteForwarders = new HashSet<>(); + serviceChains = new HashSet<>(); this.nodeId = Preconditions.checkNotNull(nodeId); mountpoint = Preconditions.checkNotNull(dataBroker); @@ -71,6 +72,10 @@ public class PolicyWriter { public CheckedFuture commitToDatastore() { LOG.info("Configuring policy on node {} ... ", nodeId.getValue()); + if (policyMapEntries.isEmpty()) { + LOG.info("Policy map {} is empty, skipping", policyMapName); + return Futures.immediateCheckedFuture(true); + } // SFC boolean remoteResult = PolicyWriterUtil.writeRemote(remoteForwarders, nodeId, mountpoint); boolean servicePathsResult = PolicyWriterUtil.writeServicePaths(serviceChains, nodeId, mountpoint); @@ -86,6 +91,10 @@ public class PolicyWriter { public CheckedFuture removeFromDatastore() { LOG.info("Removing policy from node {} ... ", nodeId.getValue()); + if (policyMapEntries.isEmpty()) { + LOG.info("Policy map {} is empty, nothing to remove", policyMapName); + return Futures.immediateCheckedFuture(true); + } // GBP - maintain order! boolean policyMapEntriesResult = PolicyWriterUtil.removePolicyMapEntries(policyMapName, policyMapEntries, nodeId, mountpoint); @@ -93,10 +102,11 @@ public class PolicyWriter { // TODO remove class map? // SFC boolean servicePathsResult = PolicyWriterUtil.removeServicePaths(serviceChains, nodeId, mountpoint); - // TODO remove remote forwarders + boolean remoteSffResult = PolicyWriterUtil.removeRemote(remoteForwarders, nodeId, mountpoint); // Result LOG.info("Policy removed from node {}", nodeId.getValue()); - return Futures.immediateCheckedFuture(classMapResult && policyMapEntriesResult && servicePathsResult); + return Futures.immediateCheckedFuture(classMapResult && policyMapEntriesResult && servicePathsResult + && remoteSffResult); } public String getManagementIpAddress() { diff --git a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriterUtil.java b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriterUtil.java index 45e3bb8de..ec71181d3 100644 --- a/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriterUtil.java +++ b/renderers/ios-xe/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriterUtil.java @@ -8,6 +8,8 @@ package org.opendaylight.groupbasedpolicy.renderer.ios_xe_provider.impl.writer; +import java.util.List; +import java.util.Set; import com.google.common.base.Optional; import com.google.common.util.concurrent.CheckedFuture; import org.opendaylight.controller.md.sal.binding.api.DataBroker; @@ -17,7 +19,6 @@ import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException; import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; import org.opendaylight.groupbasedpolicy.renderer.ios_xe_provider.impl.util.PolicyManagerUtil; -import org.opendaylight.groupbasedpolicy.renderer.ios_xe_provider.impl.util.ServiceChainingUtil; import org.opendaylight.yang.gen.v1.urn.ios.rev160308.ClassNameType; import org.opendaylight.yang.gen.v1.urn.ios.rev160308.Native; import org.opendaylight.yang.gen.v1.urn.ios.rev160308._interface.common.grouping.ServicePolicy; @@ -44,8 +45,6 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.List; - /** * Purpose: Util class for every policy writer */ @@ -53,7 +52,7 @@ class PolicyWriterUtil { private static final Logger LOG = LoggerFactory.getLogger(PolicyWriterUtil.class); - static boolean writeClassMaps(final List classMapEntries, final NodeId nodeId, final DataBroker mountpoint) { + static boolean writeClassMaps(final Set classMapEntries, final NodeId nodeId, final DataBroker mountpoint) { if (classMapEntries == null || classMapEntries.isEmpty()) { return true; } @@ -83,7 +82,7 @@ class PolicyWriterUtil { return true; } - static boolean removeClassMaps(final List classMapEntries, final NodeId nodeId, final DataBroker mountpoint) { + static boolean removeClassMaps(final Set classMapEntries, final NodeId nodeId, final DataBroker mountpoint) { boolean result = true; if (classMapEntries == null || classMapEntries.isEmpty()) { return true; @@ -112,7 +111,7 @@ class PolicyWriterUtil { return result; } - static boolean writePolicyMap(final String policyMapName, final List policyMapEntries, NodeId nodeId, + static boolean writePolicyMap(final String policyMapName, final Set policyMapEntries, NodeId nodeId, final DataBroker mountpoint) { final java.util.Optional optionalWriteTransaction = NetconfTransactionCreator.netconfWriteOnlyTransaction(mountpoint); @@ -139,7 +138,7 @@ class PolicyWriterUtil { return true; } - static boolean removePolicyMapEntries(final String policyMapName, final List policyMapEntries, + static boolean removePolicyMapEntries(final String policyMapName, final Set policyMapEntries, final NodeId nodeId, final DataBroker mountpoint) { if (policyMapEntries == null || policyMapEntries.isEmpty()) { return true; @@ -177,42 +176,28 @@ class PolicyWriterUtil { return true; } - static boolean writeLocal(final Local localForwarder, final NodeId nodeId, final DataBroker mountpoint) { - if (localForwarder == null) { + static boolean writeRemote(final Set remoteForwarders, final NodeId nodeId, + final DataBroker mountpoint) { + if (remoteForwarders == null || remoteForwarders.isEmpty()) { return true; } - final java.util.Optional optionalWriteTransaction = - NetconfTransactionCreator.netconfWriteOnlyTransaction(mountpoint); - if (!optionalWriteTransaction.isPresent()) { - LOG.warn("Failed to create write-only transaction, mountpoint: {}", mountpoint); - return false; - } - final WriteTransaction writeTransaction = optionalWriteTransaction.get(); - final InstanceIdentifier localIid = localSffInstanceIdentifier(); - writeMergeTransaction(writeTransaction, localIid, localForwarder); - LOG.info("Local forwarder created on node {}", nodeId.getValue()); - return true; - } - - static boolean removeLocal(final NodeId nodeId, final DataBroker mountpoint) { - // Remove local forwarder only when there are no more service-paths - if (ServiceChainingUtil.checkServicePathPresence(mountpoint)) { + for (ServiceFfName forwarder : remoteForwarders) { final java.util.Optional optionalWriteTransaction = NetconfTransactionCreator.netconfWriteOnlyTransaction(mountpoint); if (!optionalWriteTransaction.isPresent()) { - LOG.warn("Failed to create write-only transaction, mountpoint: {}", mountpoint); + LOG.warn("Failed to create transaction, mountpoint: {}", mountpoint); return false; } final WriteTransaction writeTransaction = optionalWriteTransaction.get(); - final InstanceIdentifier localIid = localSffInstanceIdentifier(); - deleteTransaction(writeTransaction, localIid); - LOG.info("Local forwarder removed from node {}", nodeId.getValue()); + final InstanceIdentifier forwarderIid = remoteSffInstanceIdentifier(forwarder); + writeMergeTransaction(writeTransaction, forwarderIid, forwarder); + LOG.info("Remote forwarder {} created on node {}", forwarder.getName(), nodeId.getValue()); } return true; } - static boolean writeRemote(final List remoteForwarders, final NodeId nodeId, - final DataBroker mountpoint) { + static boolean removeRemote(final Set remoteForwarders, final NodeId nodeId, + final DataBroker mountpoint) { if (remoteForwarders == null || remoteForwarders.isEmpty()) { return true; } @@ -225,13 +210,13 @@ class PolicyWriterUtil { } final WriteTransaction writeTransaction = optionalWriteTransaction.get(); final InstanceIdentifier forwarderIid = remoteSffInstanceIdentifier(forwarder); - writeMergeTransaction(writeTransaction, forwarderIid, forwarder); - LOG.info("Remote forwarder {} created on node {}", forwarder.getName(), nodeId.getValue()); + deleteTransaction(writeTransaction, forwarderIid); + LOG.info("Remote forwarder {} removed from node {}", forwarder.getName(), nodeId.getValue()); } return true; } - static boolean writeServicePaths(final List serviceChains, final NodeId nodeId, + static boolean writeServicePaths(final Set serviceChains, final NodeId nodeId, final DataBroker mountpoint) { for (org.opendaylight.yang.gen.v1.urn.ios.rev160308._native.ServiceChain serviceChain : serviceChains) { for (ServicePath entry : serviceChain.getServicePath()) { @@ -250,7 +235,7 @@ class PolicyWriterUtil { return true; } - static boolean removeServicePaths(final List serviceChains, final NodeId nodeId, + static boolean removeServicePaths(final Set serviceChains, final NodeId nodeId, final DataBroker mountpoint) { if (serviceChains == null || serviceChains.isEmpty()) { return true; diff --git a/renderers/ios-xe/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriterUtilTest.java b/renderers/ios-xe/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriterUtilTest.java index 2356d814f..0cb4e7d9e 100644 --- a/renderers/ios-xe/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriterUtilTest.java +++ b/renderers/ios-xe/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ios_xe_provider/impl/writer/PolicyWriterUtilTest.java @@ -12,7 +12,10 @@ import com.google.common.base.Optional; import com.google.common.util.concurrent.Futures; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; + import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -77,7 +80,7 @@ public class PolicyWriterUtilTest { Assert.assertTrue(PolicyWriterUtil.writeClassMaps(null, NODE_ID, dataBroker)); LOG.debug("scenario: pass through with empty classMapEntries collection"); - final List classMapEntries = new ArrayList<>(); + final Set classMapEntries = new HashSet<>(); Assert.assertTrue(PolicyWriterUtil.writeClassMaps(classMapEntries, NODE_ID, dataBroker)); LOG.debug("scenario: fail with one entry, no writeOnlyTransaction"); @@ -107,7 +110,7 @@ public class PolicyWriterUtilTest { Assert.assertTrue(PolicyWriterUtil.removeClassMaps(null, NODE_ID, dataBroker)); LOG.debug("scenario: pass through with empty classMapEntries collection"); - final List classMapEntries = new ArrayList<>(); + final Set classMapEntries = new HashSet<>(); Assert.assertTrue(PolicyWriterUtil.removeClassMaps(classMapEntries, NODE_ID, dataBroker)); LOG.debug("scenario: fail with one entry, no writeOnlyTransaction"); @@ -132,7 +135,7 @@ public class PolicyWriterUtilTest { @Test public void testWritePolicyMap() throws Exception { - final List classEntries = new ArrayList<>(); + final Set classEntries = new HashSet<>(); LOG.debug("scenario: fail with one entry, no writeOnlyTransaction"); classEntries.add(new ClassBuilder().setName(new ClassNameType("unit-classEntry-name")).build()); @@ -172,7 +175,7 @@ public class PolicyWriterUtilTest { Assert.assertTrue(PolicyWriterUtil.removePolicyMapEntries(POLICY_MAP_NAME, null, NODE_ID, dataBroker)); LOG.debug("scenario: pass through with empty classEntries collection"); - final List classEntries = new ArrayList<>(); + final Set classEntries = new HashSet<>(); Assert.assertTrue(PolicyWriterUtil.removePolicyMapEntries(POLICY_MAP_NAME, classEntries, NODE_ID, dataBroker)); LOG.debug("scenario: fail with one entry, no writeOnlyTransaction"); @@ -205,52 +208,16 @@ public class PolicyWriterUtilTest { Assert.assertTrue(PolicyWriterUtil.writeInterface(POLICY_MAP_NAME, interfaceName, NODE_ID, dataBroker)); } - @Test - public void testWriteLocal() throws Exception { - LOG.debug("scenario: succeed with null localForwarder"); - Assert.assertTrue(PolicyWriterUtil.writeLocal(null, NODE_ID, dataBroker)); - - LOG.debug("scenario: fail with no writeOnlyTransaction"); - final Local localForwarder = new LocalBuilder().build(); - Assert.assertFalse(PolicyWriterUtil.writeLocal(localForwarder, NODE_ID, dataBroker)); - - LOG.debug("scenario: succeed - available writeOnlyTransaction, no submit future"); - PowerMockito.stub(PowerMockito.method(NetconfTransactionCreator.class, "netconfWriteOnlyTransaction")).toReturn(wTxOptional); - Assert.assertTrue(PolicyWriterUtil.writeLocal(localForwarder, NODE_ID, dataBroker)); - - LOG.debug("scenario: succeed - available writeOnlyTransaction, available future"); - Mockito.when(wTx.submit()).thenReturn(Futures.immediateCheckedFuture(null)); - Assert.assertTrue(PolicyWriterUtil.writeLocal(localForwarder, NODE_ID, dataBroker)); - } - - @Test - public void testRemoveLocal() throws Exception { - LOG.debug("scenario: succeed with no service path present"); - Assert.assertTrue(PolicyWriterUtil.removeLocal(NODE_ID, dataBroker)); - - LOG.debug("scenario: fail with service path present, no writeOnlyTransaction"); - PowerMockito.stub(PowerMockito.method(ServiceChainingUtil.class, "checkServicePathPresence")).toReturn(true); - Assert.assertFalse(PolicyWriterUtil.removeLocal(NODE_ID, dataBroker)); - - LOG.debug("scenario: fail with service path present, available writeOnlyTransaction, no submit future"); - PowerMockito.stub(PowerMockito.method(NetconfTransactionCreator.class, "netconfWriteOnlyTransaction")).toReturn(wTxOptional); - Assert.assertTrue(PolicyWriterUtil.removeLocal(NODE_ID, dataBroker)); - - LOG.debug("scenario: fail with service path present, available writeOnlyTransaction, available future"); - Mockito.when(wTx.submit()).thenReturn(Futures.immediateCheckedFuture(null)); - Assert.assertTrue(PolicyWriterUtil.removeLocal(NODE_ID, dataBroker)); - } - @Test public void testWriteRemote() throws Exception { LOG.debug("scenario: succeed with null List"); Assert.assertTrue(PolicyWriterUtil.writeRemote(null, NODE_ID, dataBroker)); LOG.debug("scenario: succeed with empty List"); - Assert.assertTrue(PolicyWriterUtil.writeRemote(Collections.emptyList(), NODE_ID, dataBroker)); + Assert.assertTrue(PolicyWriterUtil.writeRemote(Collections.emptySet(), NODE_ID, dataBroker)); LOG.debug("scenario: fail with no writeOnlyTransaction"); - final List remotes = Collections.singletonList(new ServiceFfNameBuilder().build()); + final Set remotes = Collections.singleton(new ServiceFfNameBuilder().build()); Assert.assertFalse(PolicyWriterUtil.writeRemote(remotes, NODE_ID, dataBroker)); LOG.debug("scenario: succeed - available writeOnlyTransaction, no submit future"); @@ -265,10 +232,10 @@ public class PolicyWriterUtilTest { @Test public void testWriteServicePaths() throws Exception { LOG.debug("scenario: succeed with empty List"); - Assert.assertTrue(PolicyWriterUtil.writeServicePaths(Collections.emptyList(), NODE_ID, dataBroker)); + Assert.assertTrue(PolicyWriterUtil.writeServicePaths(Collections.emptySet(), NODE_ID, dataBroker)); LOG.debug("scenario: fail with no writeOnlyTransaction"); - final List serviceChains = Collections.singletonList(new ServiceChainBuilder() + final Set serviceChains = Collections.singleton(new ServiceChainBuilder() .setServicePath(Collections.singletonList(new ServicePathBuilder() .setServicePathId(42L) .build())) @@ -290,10 +257,10 @@ public class PolicyWriterUtilTest { Assert.assertTrue(PolicyWriterUtil.removeServicePaths(null, NODE_ID, dataBroker)); LOG.debug("scenario: succeed with no service path present"); - Assert.assertTrue(PolicyWriterUtil.removeServicePaths(Collections.emptyList(), NODE_ID, dataBroker)); + Assert.assertTrue(PolicyWriterUtil.removeServicePaths(Collections.emptySet(), NODE_ID, dataBroker)); LOG.debug("scenario: fail with service path present, no writeOnlyTransaction"); - final List serviceChains = Collections.singletonList(new ServiceChainBuilder() + final Set serviceChains = Collections.singleton(new ServiceChainBuilder() .setServicePath(Collections.singletonList(new ServicePathBuilder() .setServicePathId(42L) .build()))