summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMaros Marsalek <mmarsale@cisco.com>2016-06-27 11:49:12 +0200
committerMarek Gradzki <mgradzki@cisco.com>2016-06-27 12:04:57 +0000
commite187f2bd1301a3f20d5316c5a14a99b733f07550 (patch)
tree13d74dff06a9f5cc63e8875dacadd5579d8602cf
parent6e3de9c3e99fdf284e63f303d3a32bd4dfbd124e (diff)
Fix IPv4 read caching
Single cache key was used for each interface during a single read returing same IPs for all the interfaces Change-Id: I8cc05591b257d44a253cc23c9d79d9096459dcdd Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
-rw-r--r--v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizer.java29
-rw-r--r--v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizerTest.java77
2 files changed, 82 insertions, 24 deletions
diff --git a/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizer.java b/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizer.java
index 641516136..2121337ab 100644
--- a/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizer.java
+++ b/v3po/v3po2vpp/src/main/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizer.java
@@ -50,7 +50,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * Customizer for read operations for {@link Address} of {@link Ipv4}
+ * Customizer for read operations for {@link Address} of {@link Ipv4}.
*/
public class Ipv4AddressCustomizer extends FutureJVppCustomizer
implements ListReaderCustomizer<Address, AddressKey, AddressBuilder> {
@@ -92,8 +92,7 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer
builder.setIp(TranslateUtils.arrayToIpv4AddressNoZone(detail.ip))
.setSubnet(new PrefixLengthBuilder()
.setPrefixLength(Short.valueOf(detail.prefixLength)).build());
- LOG.info("Address read successfull");
-
+ LOG.info("Address read successful");
} else {
LOG.warn("No address dump present");
}
@@ -102,18 +101,22 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer
@Override
public List<AddressKey> getAllIds(@Nonnull InstanceIdentifier<Address> id, @Nonnull ReadContext context)
throws ReadFailedException {
+ // FIXME: this kind of logs provide very little information. At least the ID should be included so we know
+ // from the logs what exact data is being processed
+ // + Logs should be consistent in using punctuation
LOG.debug("Extracting keys..");
Optional<IpAddressDetailsReplyDump> dumpOptional = dumpAddresses(id, context);
if (dumpOptional.isPresent() && dumpOptional.get().ipAddressDetails != null) {
- List<IpAddressDetails> details = dumpOptional.get().ipAddressDetails;
-
- return details.stream()
+ return dumpOptional.get().ipAddressDetails.stream()
.map(detail -> new AddressKey(TranslateUtils.arrayToIpv4AddressNoZone(detail.ip)))
.collect(Collectors.toList());
} else {
+ // FIXME if this is expected then WARN should not be emitted
+ // FIXME if this is not expected, throw an exception instead
+ // Same in readCurrentAttributes()
LOG.warn("No dump present");
return Collections.emptyList();
}
@@ -124,11 +127,14 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer
((Ipv4Builder) builder).setAddress(readData);
}
+ // TODO refactor after there is a more generic implementation of cache operations
+ // FIXME update TODO with what exactly should be refactored and how
// TODO refactor after there is an more generic implementation of cache
// operations
private Optional<IpAddressDetailsReplyDump> dumpAddresses(InstanceIdentifier<Address> id, ReadContext ctx)
- throws ReadFailedException {
- Optional<IpAddressDetailsReplyDump> dumpFromCache = dumpAddressFromCache(ctx.getModificationCache());
+ throws ReadFailedException {
+ final String cacheKey = CACHE_KEY + id.firstKeyOf(Interface.class).getName();
+ Optional<IpAddressDetailsReplyDump> dumpFromCache = dumpAddressFromCache(cacheKey, ctx.getModificationCache());
if (dumpFromCache.isPresent()) {
return dumpFromCache;
@@ -142,15 +148,16 @@ public class Ipv4AddressCustomizer extends FutureJVppCustomizer
}
if (dumpFromOperational.isPresent()) {
- ctx.getModificationCache().put(CACHE_KEY, dumpFromOperational.get());
+ ctx.getModificationCache().put(cacheKey, dumpFromOperational.get());
}
return dumpFromOperational;
}
- private Optional<IpAddressDetailsReplyDump> dumpAddressFromCache(ModificationCache cache) {
+ private Optional<IpAddressDetailsReplyDump> dumpAddressFromCache(final String cacheKey,
+ ModificationCache cache) {
LOG.debug("Dumping from cache...");
- return Optional.fromNullable((IpAddressDetailsReplyDump) cache.get(CACHE_KEY));
+ return Optional.fromNullable((IpAddressDetailsReplyDump) cache.get(cacheKey));
}
private Optional<IpAddressDetailsReplyDump> dumpAddressFromOperationalData(final InstanceIdentifier<Address> id,
diff --git a/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizerTest.java b/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizerTest.java
index 85f528e1e..584464645 100644
--- a/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizerTest.java
+++ b/v3po/v3po2vpp/src/test/java/io/fd/honeycomb/v3po/translate/v3po/interfacesstate/ip/Ipv4AddressCustomizerTest.java
@@ -19,7 +19,9 @@ package io.fd.honeycomb.v3po.translate.v3po.interfacesstate.ip;
import static io.fd.honeycomb.v3po.translate.v3po.test.ContextTestUtils.getMapping;
import static io.fd.honeycomb.v3po.translate.v3po.test.ContextTestUtils.getMappingIid;
import static io.fd.honeycomb.v3po.translate.v3po.util.TranslateUtils.reverseBytes;
+import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -38,6 +40,7 @@ import java.util.Arrays;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.stream.Collectors;
+import org.hamcrest.CoreMatchers;
import org.junit.Test;
import org.mockito.Mockito;
import org.opendaylight.yang.gen.v1.urn.honeycomb.params.xml.ns.yang.naming.context.rev160513.contexts.naming.context.Mappings;
@@ -64,7 +67,9 @@ import org.openvpp.jvpp.dto.IpAddressDump;
public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest<Address, AddressKey, AddressBuilder> {
private static final String IFACE_NAME = "eth0";
+ private static final String IFACE_2_NAME = "eth1";
private static final int IFACE_ID = 1;
+ private static final int IFACE_2_ID = 2;
private NamingContext interfacesContext;
@@ -80,24 +85,27 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest<Address,
@Override
protected RootReaderCustomizer<Address, AddressBuilder> initCustomizer() {
final KeyedInstanceIdentifier<Mapping, MappingKey> eth0Id = getMappingIid(IFACE_NAME, "test-instance");
+ final KeyedInstanceIdentifier<Mapping, MappingKey> eth1Id = getMappingIid(IFACE_2_NAME, "test-instance");
final Optional<Mapping> eth0 = getMapping(IFACE_NAME, IFACE_ID);
+ final Optional<Mapping> eth1 = getMapping(IFACE_2_NAME, IFACE_2_ID);
- final List<Mapping> allMappings = Lists.newArrayList(getMapping(IFACE_NAME, IFACE_ID).get());
+ final List<Mapping> allMappings = Lists.newArrayList(eth0.get(), eth1.get());
final Mappings allMappingsBaObject = new MappingsBuilder().setMapping(allMappings).build();
doReturn(Optional.of(allMappingsBaObject)).when(mappingContext).read(eth0Id.firstIdentifierOf(Mappings.class));
doReturn(eth0).when(mappingContext).read(eth0Id);
+ doReturn(eth1).when(mappingContext).read(eth1Id);
return new Ipv4AddressCustomizer(api, interfacesContext);
}
- private static InstanceIdentifier<Address> getId(final String address) {
+ private static InstanceIdentifier<Address> getId(final String address, final String ifaceName) {
return InstanceIdentifier.builder(InterfacesState.class)
- .child(Interface.class, new InterfaceKey(IFACE_NAME))
- .augmentation(Interface2.class)
- .child(Ipv4.class)
- .child(Address.class, new AddressKey(new Ipv4AddressNoZone(new Ipv4Address(address))))
- .build();
+ .child(Interface.class, new InterfaceKey(ifaceName))
+ .augmentation(Interface2.class)
+ .child(Ipv4.class)
+ .child(Address.class, new AddressKey(new Ipv4AddressNoZone(new Ipv4Address(address))))
+ .build();
}
@Test
@@ -118,11 +126,11 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest<Address,
IpAddressDetailsReplyDump reply = new IpAddressDetailsReplyDump();
reply.ipAddressDetails = ImmutableList.of(detail1, detail2, detail3);
- cache.put(Ipv4AddressCustomizer.class.getName(), reply);
+ cache.put(Ipv4AddressCustomizer.class.getName() + IFACE_NAME, reply);
when(ctx.getModificationCache()).thenReturn(cache);
final AddressBuilder builder = new AddressBuilder();
- final InstanceIdentifier<Address> id = getId("192.168.2.1");
+ final InstanceIdentifier<Address> id = getId("192.168.2.1", IFACE_NAME);
getCustomizer().readCurrentAttributes(id, builder, ctx);
@@ -130,6 +138,49 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest<Address,
}
@Test
+ public void testReadCurrentAttributesFor2Ifcs() throws ReadFailedException {
+ ModificationCache cache = new ModificationCache();
+
+ IpAddressDetails detail1 = new IpAddressDetails();
+ IpAddressDetails detail2 = new IpAddressDetails();
+
+ detail1.ip = reverseBytes(
+ TranslateUtils.ipv4AddressNoZoneToArray(new Ipv4AddressNoZone(new Ipv4Address("192.168.2.1"))));
+ detail2.ip = reverseBytes(
+ TranslateUtils.ipv4AddressNoZoneToArray(new Ipv4AddressNoZone(new Ipv4Address("192.168.2.2"))));
+
+ IpAddressDetailsReplyDump reply = new IpAddressDetailsReplyDump();
+ reply.ipAddressDetails = ImmutableList.of(detail1);
+ IpAddressDetailsReplyDump reply2 = new IpAddressDetailsReplyDump();
+ reply2.ipAddressDetails = ImmutableList.of(detail2);
+
+ CompletableFuture<IpAddressDetailsReplyDump> future = new CompletableFuture<>();
+ future.complete(reply);
+ CompletableFuture<IpAddressDetailsReplyDump> future2 = new CompletableFuture<>();
+ future2.complete(reply2);
+
+ when(api.ipAddressDump(Mockito.any(IpAddressDump.class))).thenReturn(future).thenReturn(future2);
+ when(ctx.getModificationCache()).thenReturn(cache);
+
+ final InstanceIdentifier<Address> id = getId("192.168.2.1", IFACE_NAME);
+ final InstanceIdentifier<Address> id2 = getId("192.168.2.2", IFACE_2_NAME);
+
+ final List<AddressKey> ifc1Ids = getCustomizer().getAllIds(id, ctx);
+ assertThat(ifc1Ids.size(), is(1));
+ assertThat(ifc1Ids, CoreMatchers.hasItem(new AddressKey(new Ipv4AddressNoZone("192.168.2.1"))));
+ final List<AddressKey> ifc2Ids = getCustomizer().getAllIds(id2, ctx);
+ assertThat(ifc2Ids.size(), is(1));
+ assertThat(ifc2Ids, CoreMatchers.hasItem(new AddressKey(new Ipv4AddressNoZone("192.168.2.2"))));
+
+ AddressBuilder builder = new AddressBuilder();
+ getCustomizer().readCurrentAttributes(id, builder, ctx);
+ assertEquals(builder.getIp().getValue(), "192.168.2.1");
+ builder = new AddressBuilder();
+ getCustomizer().readCurrentAttributes(id2, builder, ctx);
+ assertEquals(builder.getIp().getValue(), "192.168.2.2");
+ }
+
+ @Test
public void testReadCurrentAttributesFromOperationalData() throws ReadFailedException {
ModificationCache cache = new ModificationCache();
@@ -155,7 +206,7 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest<Address,
final AddressBuilder builder = new AddressBuilder();
- final InstanceIdentifier<Address> id = getId("192.168.2.1");
+ final InstanceIdentifier<Address> id = getId("192.168.2.1", IFACE_NAME);
getCustomizer().readCurrentAttributes(id, builder, ctx);
@@ -180,10 +231,10 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest<Address,
IpAddressDetailsReplyDump reply = new IpAddressDetailsReplyDump();
reply.ipAddressDetails = ImmutableList.of(detail1, detail2, detail3);
- cache.put(Ipv4AddressCustomizer.class.getName(), reply);
+ cache.put(Ipv4AddressCustomizer.class.getName() + IFACE_NAME, reply);
when(ctx.getModificationCache()).thenReturn(cache);
- final InstanceIdentifier<Address> id = getId("192.168.2.1");
+ final InstanceIdentifier<Address> id = getId("192.168.2.1", IFACE_NAME);
List<Ipv4AddressNoZone> ids = getCustomizer().getAllIds(id, ctx).stream()
.map(key -> key.getIp())
@@ -220,7 +271,7 @@ public class Ipv4AddressCustomizerTest extends ListReaderCustomizerTest<Address,
when(api.ipAddressDump(Mockito.any(IpAddressDump.class))).thenReturn(future);
when(ctx.getModificationCache()).thenReturn(cache);
- final InstanceIdentifier<Address> id = getId("192.168.2.1");
+ final InstanceIdentifier<Address> id = getId("192.168.2.1", IFACE_NAME);
List<Ipv4AddressNoZone> ids = getCustomizer().getAllIds(id, ctx).stream()
.map(key -> key.getIp())