Skip to content

Commit

Permalink
Enable che server to use internal address for wsagent (CHE-2030)
Browse files Browse the repository at this point in the history 
Add ability to make che-server use internal address of
workspaces when che-server is running in docker through
environment variable CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS.

When env var is set to "true", internal address is set
to the internal address of the relevant wsagent docker
container. Otherwise, behavior is unchanged.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
  • Loading branch information
@amisevsk
amisevsk committed Oct 31, 2016
1 parent ba7c0bb commit 339eb71
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 12 deletions.
39 changes: 38 additions & 1 deletion ...achine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstanceRuntimeInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ public class DockerInstanceRuntimeInfo implements MachineRuntimeInfo {
*/
public static final String CHE_WORKSPACE_ID = "CHE_WORKSPACE_ID";

/**
* Env variable that indicates to DockerInstanceRuntimeInfo that the direct address of wsagent containers
* should be used, as available in ContainerInfo.
*/
public static final String CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS = "CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS";

/**
* Default HOSTNAME that will be added in all docker containers that are started. This host will container the Docker host's ip
* reachable inside the container.
Expand Down Expand Up @@ -310,16 +316,33 @@ protected Map<String, ServerImpl> addRefAndUrlToServers(final Map<String, Server
protected Map<String, ServerImpl> getServersWithFilledPorts(final String externalHostame, final String internalHostname, final Map<String, List<PortBinding>> exposedPorts) {
final HashMap<String, ServerImpl> servers = new LinkedHashMap<>();

boolean useMappedPorts = true;
if (useInternalContainerAddresses()) {
if (info.getNetworkSettings() != null && internalHostname.equals(info.getNetworkSettings().getIpAddress())) {
useMappedPorts = false;
}
}

for (Map.Entry<String, List<PortBinding>> portEntry : exposedPorts.entrySet()) {
// in form 1234/tcp
String portProtocol = portEntry.getKey();
// we are assigning ports automatically, so have 1 to 1 binding (at least per protocol)
String externalPort = portEntry.getValue().get(0).getHostPort();

// If we are sending messages to the container directly, we don't want to use the mapped ports.
String internalHostnameAndPort;
if (useMappedPorts) {
internalHostnameAndPort = internalHostname + ":" + externalPort;
} else {
String internalPort = portEntry.getKey().split("/")[0];
internalHostnameAndPort = internalHostname + ":" + internalPort;
}

servers.put(portProtocol, new ServerImpl(null,
null,
externalHostame + ":" + externalPort,
null,
new ServerPropertiesImpl(null, internalHostname + ":" + externalPort, null)));
new ServerPropertiesImpl(null, internalHostnameAndPort, null)));
}

return servers;
Expand Down Expand Up @@ -355,4 +378,18 @@ private Map<String, ServerConfImpl> getServersConfFromLabels(final Set<String> p

return serversConf;
}

/**
* Checks environment variable {@code CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS} to determine if
* direct addresses of containers should be used.
* @return true if {@code CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS} is "true", false otherwise.
*/
protected boolean useInternalContainerAddresses() {
String useInternalContainerAddresses = System.getenv(CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS);
if (useInternalContainerAddresses == null) {
return false;
} else {
return useInternalContainerAddresses.equals("true");
}
}
}
45 changes: 34 additions & 11 deletions ...main/java/org/eclipse/che/plugin/docker/machine/local/LocalDockerInstanceRuntimeInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,21 @@
import org.eclipse.che.api.core.model.machine.ServerConf;
import org.eclipse.che.commons.annotation.Nullable;
import org.eclipse.che.plugin.docker.client.json.ContainerInfo;
import org.eclipse.che.plugin.docker.client.json.NetworkSettings;
import org.eclipse.che.plugin.docker.machine.DockerInstanceRuntimeInfo;

import javax.inject.Inject;
import javax.inject.Named;
import java.util.Set;

/**
* Gets predefined docker containers host (internal and external addresses) for machine servers instead of evaluating it
* By default, gets predefined docker containers host (internal and external addresses) for machine servers instead of evaluating it
* from docker configuration. External address is needed when clients (UD, IDE, etc...) can't ping machine servers
* directly (e.g. when running on Docker for Mac or behind a NAT). If the external address is not provided it defaults
* to the internal one.
*
* <p>If environment variable {@code CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS} is true, the IP Address from
* {@link ContainerInfo}, if available, is used for internal address instead.<br>
* <p>Value of host can be retrieved from property ${code machine.docker.local_node_host} or
* from environment variable {@code CHE_DOCKER_MACHINE_HOST}.<br>
* <p>Value of external hostname can be retrieved from property ${code machine.docker.local_node_host.external} or
Expand All @@ -50,6 +53,12 @@ public class LocalDockerInstanceRuntimeInfo extends DockerInstanceRuntimeInfo {
*/
public static final String CHE_DOCKER_MACHINE_HOST_EXTERNAL = "CHE_DOCKER_MACHINE_HOST_EXTERNAL";

/**
* Env variable that indicates to DockerInstanceRuntimeInfo that the direct address of wsagent containers
* should be used, as available in ContainerInfo.
*/
public static final String CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS = "CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS";

@Inject
public LocalDockerInstanceRuntimeInfo(@Assisted ContainerInfo containerInfo,
@Assisted("externalhost") @Nullable String containerExternalHostname,
Expand All @@ -60,23 +69,23 @@ public LocalDockerInstanceRuntimeInfo(@Assisted ContainerInfo containerInfo,
@Named("machine.docker.dev_machine.machine_servers") Set<ServerConf> devMachineServers,
@Named("machine.docker.machine_servers") Set<ServerConf> allMachinesServers) {


super(containerInfo,
externalHostnameWithPrecedence(dockerNodeExternalHostname,
containerExternalHostname,
dockerNodeInternalHostname,
containerInternalHostname),
containerExternalHostname,
dockerNodeInternalHostname,
containerInternalHostname),
internalHostnameWithPrecedence(dockerNodeInternalHostname,
containerInternalHostname),
containerInternalHostname,
containerInfo.getNetworkSettings()),
machineConfig,
devMachineServers,
allMachinesServers);
}

private static String externalHostnameWithPrecedence(String externalHostnameProperty,
String externalHostnameAssisted,
String internalHostnameProperty,
String internalHostnameAssisted) {
String externalHostnameAssisted,
String internalHostnameProperty,
String internalHostnameAssisted) {

String externalHostnameEnvVar = System.getenv(CHE_DOCKER_MACHINE_HOST_EXTERNAL);
if (externalHostnameEnvVar != null) {
Expand All @@ -86,12 +95,26 @@ private static String externalHostnameWithPrecedence(String externalHostnameProp
} else if (externalHostnameAssisted != null) {
return externalHostnameAssisted;
} else {
return internalHostnameWithPrecedence(internalHostnameProperty,internalHostnameAssisted);
return internalHostnameWithPrecedence(internalHostnameProperty,
internalHostnameAssisted,
null);
}
}

private static String internalHostnameWithPrecedence(String internalHostnameProperty,
String internalHostnameAssisted) {
String internalHostnameAssisted,
NetworkSettings networkSettings) {

String useInternalContainerAddress = System.getenv(CHE_DOCKER_USE_INTERNAL_CONTAINER_ADDRESS);
if (useInternalContainerAddress != null && useInternalContainerAddress.equals("true")) {
String containerHostName = null;
if (networkSettings != null) {
containerHostName = networkSettings.getIpAddress();
}
if (containerHostName != null && !containerHostName.isEmpty()) {
return containerHostName;
}
}

String internalHostNameEnvVariable = System.getenv(CHE_DOCKER_MACHINE_HOST_INTERNAL);
if (internalHostNameEnvVariable != null) {
Expand Down
98 changes: 98 additions & 0 deletions ...ne/src/test/java/org/eclipse/che/plugin/docker/machine/DockerInstanceRuntimeInfoTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.eclipse.che.plugin.docker.client.json.NetworkSettings;
import org.eclipse.che.plugin.docker.client.json.PortBinding;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.testng.MockitoTestNGListener;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Listeners;
Expand Down Expand Up @@ -47,6 +48,7 @@ public class DockerInstanceRuntimeInfoTest {
private static final String CONTAINER_HOST = "container-host.com";
private static final String CONTAINER_HOST_EXTERNAL = "container-host-ext.com";
private static final String DEFAULT_ADDRESS = "192.168.1.1";
private static final String DEFAULT_ADDRESS_INTERNAL = "172.17.0.1";

@Mock
private ContainerInfo containerInfo;
Expand Down Expand Up @@ -678,4 +680,100 @@ public void shouldSetExternalAddressDistinctFromInternalWhenExternalHostnameIsNo
// then
assertEquals(servers, expectedServers);
}

@Test
public void shouldUseUnmappedPortsWhenUsingInternalAddresses() throws Exception {
//given
when(networkSettings.getIpAddress()).thenReturn(DEFAULT_ADDRESS_INTERNAL);
Map<String, List<PortBinding>> ports = new HashMap<>();
when(networkSettings.getPorts()).thenReturn(ports);
ports.put("4301/tcp", Collections.singletonList(new PortBinding().withHostIp(DEFAULT_ADDRESS)
.withHostPort("32100")));
ports.put("4305/udp", Collections.singletonList(new PortBinding().withHostIp(DEFAULT_ADDRESS)
.withHostPort("32103")));
Set<ServerConf> commonSystemServersConfigs = new HashSet<>();
commonSystemServersConfigs.add(new ServerConfImpl("sysServer1-tcp", "4301/tcp", "http", "/some/path1"));
Set<ServerConf> devSystemServersConfigs = new HashSet<>();
devSystemServersConfigs.add(new ServerConfImpl("devSysServer1-udp", "4305/udp", null, "some/path4"));
when(machineConfig.isDev()).thenReturn(true);

runtimeInfo = Mockito.spy(new DockerInstanceRuntimeInfo(containerInfo,
DEFAULT_ADDRESS,
DEFAULT_ADDRESS_INTERNAL,
machineConfig,
devSystemServersConfigs,
commonSystemServersConfigs));

doReturn(true).when(runtimeInfo).useInternalContainerAddresses();

final HashMap<String, ServerImpl> expectedServers = new HashMap<>();
expectedServers.put("4301/tcp", new ServerImpl("sysServer1-tcp",
"http",
DEFAULT_ADDRESS + ":32100",
"http://" + DEFAULT_ADDRESS + ":32100/some/path1",
new ServerPropertiesImpl("/some/path1",
DEFAULT_ADDRESS_INTERNAL + ":4301",
"http://" + DEFAULT_ADDRESS_INTERNAL + ":4301/some/path1")));
expectedServers.put("4305/udp", new ServerImpl("devSysServer1-udp",
null,
DEFAULT_ADDRESS + ":32103",
null,
new ServerPropertiesImpl("some/path4",
DEFAULT_ADDRESS_INTERNAL + ":4305",
null)));

//when
final Map<String, ServerImpl> servers = runtimeInfo.getServers();

//then
assertEquals(servers, expectedServers, "Expected servers to use internal container address and ports");
}

@Test
public void shouldUseMappedPortsWhenNotUsingInternalAddresses() throws Exception {
//given
when(networkSettings.getIpAddress()).thenReturn(DEFAULT_ADDRESS_INTERNAL);
Map<String, List<PortBinding>> ports = new HashMap<>();
when(networkSettings.getPorts()).thenReturn(ports);
ports.put("4301/tcp", Collections.singletonList(new PortBinding().withHostIp(DEFAULT_ADDRESS)
.withHostPort("32100")));
ports.put("4305/udp", Collections.singletonList(new PortBinding().withHostIp(DEFAULT_ADDRESS)
.withHostPort("32103")));
Set<ServerConf> commonSystemServersConfigs = new HashSet<>();
commonSystemServersConfigs.add(new ServerConfImpl("sysServer1-tcp", "4301/tcp", "http", "/some/path1"));
Set<ServerConf> devSystemServersConfigs = new HashSet<>();
devSystemServersConfigs.add(new ServerConfImpl("devSysServer1-udp", "4305/udp", null, "some/path4"));
when(machineConfig.isDev()).thenReturn(true);

runtimeInfo = Mockito.spy(new DockerInstanceRuntimeInfo(containerInfo,
DEFAULT_ADDRESS,
DEFAULT_ADDRESS_INTERNAL,
machineConfig,
devSystemServersConfigs,
commonSystemServersConfigs));

doReturn(false).when(runtimeInfo).useInternalContainerAddresses();

final HashMap<String, ServerImpl> expectedServers = new HashMap<>();
expectedServers.put("4301/tcp", new ServerImpl("sysServer1-tcp",
"http",
DEFAULT_ADDRESS + ":32100",
"http://" + DEFAULT_ADDRESS + ":32100/some/path1",
new ServerPropertiesImpl("/some/path1",
DEFAULT_ADDRESS_INTERNAL + ":32100",
"http://" + DEFAULT_ADDRESS_INTERNAL + ":32100/some/path1")));
expectedServers.put("4305/udp", new ServerImpl("devSysServer1-udp",
null,
DEFAULT_ADDRESS + ":32103",
null,
new ServerPropertiesImpl("some/path4",
DEFAULT_ADDRESS_INTERNAL + ":32103",
null)));

//when
final Map<String, ServerImpl> servers = runtimeInfo.getServers();

//then
assertEquals(servers, expectedServers, "Expected servers to not use internal container address and ports");
}
}

0 comments on commit 339eb71

Please sign in to comment.