Skip to content

Commit

Permalink
[#879][#925] Docker network Pool overlaps with other one on this addr…
Browse files Browse the repository at this point in the history
…ess space (#1234)

### What changes were proposed in this pull request?

1. Added Docker network overlap detection
2. Reduced IP allocation ranges for lower conflict probability, Modified
to `10.20.30.0/28` only allocate IP ranger `10.20.30.1` ~ `10.20.30.14`
3. Docker network only created on MacOS and default Docker server
environment

### Why are the changes needed?

Fix: #879, #925

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

CI

Co-authored-by: Xun Liu <xun@datastrato.com>
  • Loading branch information
github-actions[bot] and xunliu authored Dec 21, 2023
1 parent 631b579 commit b2136bb
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 7 deletions.
2 changes: 1 addition & 1 deletion dev/docker/tools/docker-connector.conf
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@

# Fixed docker container network subnet in Gravitino integration testing module
# Generate command: `docker network ls --filter driver=bridge --format "{{.ID}}" | xargs docker network inspect --format "route {{range .IPAM.Config}}{{.Subnet}}{{end}}" > ${bin}/docker-connector.conf`
route 10.0.0.0/22
route 10.20.30.0/28
9 changes: 9 additions & 0 deletions integration-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,15 @@ tasks.test {
environment("PROJECT_VERSION", version)
environment("TRINO_CONF_DIR", buildDir.path + "/trino-conf")

val dockerRunning = project.extra["dockerRunning"] as? Boolean ?: false
val macDockerConnector = project.extra["macDockerConnector"] as? Boolean ?: false
if (OperatingSystem.current().isMacOsX() &&
dockerRunning &&
macDockerConnector
) {
environment("NEED_CREATE_DOCKER_NETWORK", "true")
}

// Gravitino CI Docker image
environment("GRAVITINO_CI_HIVE_DOCKER_IMAGE", "datastrato/gravitino-ci-hive:0.1.6")
environment("GRAVITINO_CI_TRINO_DOCKER_IMAGE", "datastrato/gravitino-ci-trino:0.1.2")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public SELF withEnvVars(Map<String, String> envVars) {
}

public SELF withNetwork(Network network) {
this.network = Optional.of(network);
this.network = Optional.ofNullable(network);
return self;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@

import com.datastrato.gravitino.integration.test.util.CloseableGroup;
import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.command.RemoveNetworkCmd;
import com.github.dockerjava.api.model.Info;
import com.github.dockerjava.api.model.Network.Ipam.Config;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import java.io.Closeable;
import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testcontainers.DockerClientFactory;
Expand All @@ -22,9 +27,11 @@ public class ContainerSuite implements Closeable {
private static volatile ContainerSuite instance = null;

// The subnet must match the configuration in `dev/docker/tools/mac-docker-connector.conf`
private static final String CONTAINER_NETWORK_SUBNET = "10.0.0.0/22";
private static final String CONTAINER_NETWORK_GATEWAY = "10.0.0.1";
private static final String CONTAINER_NETWORK_IPRANGE = "10.0.0.100/22";
public static final String CONTAINER_NETWORK_SUBNET = "10.20.30.0/28";
private static final String CONTAINER_NETWORK_GATEWAY = "10.20.30.1";
private static final String CONTAINER_NETWORK_IPRANGE = "10.20.30.14/28";
private static final String NETWORK_NAME = "gravitino-ci-network";

private static Network network = null;
private static HiveContainer hiveContainer;
private static TrinoContainer trinoContainer;
Expand All @@ -38,7 +45,9 @@ private ContainerSuite() {
Info info = dockerClient.infoCmd().exec();
LOG.info("Docker info: {}", info);

network = createDockerNetwork();
if ("true".equalsIgnoreCase(System.getenv("NEED_CREATE_DOCKER_NETWORK"))) {
network = createDockerNetwork();
}
} catch (Exception e) {
throw new RuntimeException("Failed to initialize ContainerSuite", e);
}
Expand Down Expand Up @@ -124,12 +133,53 @@ public HiveContainer getHiveContainer() {
// Let containers assign addresses in a fixed subnet to avoid `mac-docker-connector` needing to
// refresh the configuration
private static Network createDockerNetwork() {
DockerClient dockerClient = DockerClientFactory.instance().client();

// Remove the `gravitino-ci-network` if it exists
boolean networkExists =
dockerClient.listNetworksCmd().withNameFilter(NETWORK_NAME).exec().stream()
.anyMatch(network -> network.getName().equals(NETWORK_NAME));
if (networkExists) {
RemoveNetworkCmd removeNetworkCmd = dockerClient.removeNetworkCmd(NETWORK_NAME);
removeNetworkCmd.exec();
}

// Check if the subnet of the network conflicts with `gravitino-ci-network`
List<com.github.dockerjava.api.model.Network> networks = dockerClient.listNetworksCmd().exec();

for (com.github.dockerjava.api.model.Network network : networks) {
List<Config> ipamConfigs = network.getIpam().getConfig();
for (Config ipamConfig : ipamConfigs) {
try {
if (ipRangesOverlap(ipamConfig.getSubnet(), CONTAINER_NETWORK_SUBNET)) {
LOG.error(
"The Docker of the network {} subnet {} conflicts with the `gravitino-ci-network` {}, "
+ "You can either remove {} network from Docker, or modify the `ContainerSuite.CONTAINER_NETWORK_SUBNET` variable",
network.getName(),
ipamConfig.getSubnet(),
CONTAINER_NETWORK_SUBNET,
network.getName());
throw new RuntimeException(
"The Docker of the network "
+ network.getName()
+ " subnet "
+ ipamConfig.getSubnet()
+ " conflicts with the `gravitino-ci-network` "
+ CONTAINER_NETWORK_SUBNET);
}
} catch (Exception e) {
throw new RuntimeException(e);
}
}
}

com.github.dockerjava.api.model.Network.Ipam.Config ipamConfig =
new com.github.dockerjava.api.model.Network.Ipam.Config();
ipamConfig
.withSubnet(CONTAINER_NETWORK_SUBNET)
.withGateway(CONTAINER_NETWORK_GATEWAY)
.withIpRange(CONTAINER_NETWORK_IPRANGE);
.withIpRange(CONTAINER_NETWORK_IPRANGE)
.setNetworkID("gravitino-ci-network");

return closer.register(
Network.builder()
Expand All @@ -140,6 +190,61 @@ private static Network createDockerNetwork() {
.build());
}

public static boolean ipRangesOverlap(String cidr1, String cidr2) throws Exception {
long[] net1 = cidrToRange(cidr1);
long[] net2 = cidrToRange(cidr2);

long startIp1 = net1[0];
long endIp1 = net1[1];
long startIp2 = net2[0];
long endIp2 = net2[1];

LOG.info("Subnet1: {} allocate IP ranger [{} ~ {}]", cidr1, long2Ip(startIp1), long2Ip(endIp1));
LOG.info("Subnet2: {} allocate IP ranger [{} ~ {}]", cidr2, long2Ip(startIp2), long2Ip(endIp2));

if (startIp1 > endIp2 || endIp1 < startIp2) {
return false;
} else {
return true;
}
}

public static String long2Ip(final long ip) {
final StringBuilder result = new StringBuilder(15);
result.append(ip >> 24 & 0xff).append(".");
result.append(ip >> 16 & 0xff).append(".");
result.append(ip >> 8 & 0xff).append(".");
result.append(ip & 0xff);

return result.toString();
}

// Classless Inter-Domain Routing (CIDR)
// https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing
private static long[] cidrToRange(String cidr) throws Exception {
String[] parts = cidr.split("/");
InetAddress inetAddress = InetAddress.getByName(parts[0]);
int prefixLength = Integer.parseInt(parts[1]);

ByteBuffer buffer = ByteBuffer.wrap(inetAddress.getAddress());
long ip =
(inetAddress.getAddress().length == 4) ? buffer.getInt() & 0xFFFFFFFFL : buffer.getLong();
long mask = -(1L << (32 - prefixLength));

long startIp = ip & mask;

long endIp;
if (inetAddress.getAddress().length == 4) {
// IPv4
endIp = startIp + ((1L << (32 - prefixLength)) - 1);
} else {
// IPv6
endIp = startIp + ((1L << (128 - prefixLength)) - 1);
}

return new long[] {startIp, endIp};
}

@Override
public void close() throws IOException {
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright 2023 Datastrato Pvt Ltd.
* This software is licensed under the Apache License version 2.
*/
package com.datastrato.gravitino.integration.test.container;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

public class NetworksConflictTest {
@Test
public void networksConflictTest1() throws Exception {
final String subnet1 = "10.20.30.0/28"; // allocate IP ranger 10.20.30.1 ~ 10.20.30.14
final String subnet2 = "10.20.30.0/26"; // allocate IP ranger is 10.20.30.1 ~ 10.20.30.62
Assertions.assertTrue(ContainerSuite.ipRangesOverlap(subnet1, subnet2));
}

@Test
public void networksConflictTest2() throws Exception {
final String subnet1 = "10.20.30.0/28"; // allocate IP ranger is 10.20.30.1 ~ 10.20.30.14
final String subnet2 = "10.20.31.0/28"; // allocate IP ranger is 10.20.31.1 ~ 10.20.31.14
Assertions.assertFalse(ContainerSuite.ipRangesOverlap(subnet1, subnet2));
}
}

0 comments on commit b2136bb

Please sign in to comment.