Skip to content

Commit

Permalink
Multi-container workspace Support (eclipse-che#5110)
Browse files Browse the repository at this point in the history 
* Fix 2 NPE that prevented using *non-dev* additional machines

In the context of https://issues.jboss.org/browse/CHE-175

Signed-off-by: David Festal <dfestal@redhat.com>

* Name openshift resources based on the machine name for non-dev machines

This fixes https://issues.jboss.org/browse/CHE-259
and https://issues.jboss.org/browse/CHE-258

Signed-off-by: David Festal <dfestal@redhat.com>
  • Loading branch information
@davidfestal @sunix
davidfestal authored and sunix committed Jun 16, 2017
1 parent 42b3867 commit b9cf80a
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 40 deletions.
98 changes: 63 additions & 35 deletions ...hift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import org.eclipse.che.plugin.docker.client.json.NetworkSettings;
import org.eclipse.che.plugin.docker.client.json.PortBinding;
import org.eclipse.che.plugin.docker.client.json.network.ContainerInNetwork;
import org.eclipse.che.plugin.docker.client.json.network.EndpointConfig;
import org.eclipse.che.plugin.docker.client.json.network.Ipam;
import org.eclipse.che.plugin.docker.client.json.network.IpamConfig;
import org.eclipse.che.plugin.docker.client.json.network.Network;
Expand Down Expand Up @@ -167,6 +168,7 @@ public class OpenShiftConnector extends DockerConnector {
private static final String CHE_CONTAINER_IDENTIFIER_LABEL_KEY = "cheContainerIdentifier";
private static final String CHE_DEFAULT_EXTERNAL_ADDRESS = "172.17.0.1";
private static final String CHE_WORKSPACE_ID_ENV_VAR = "CHE_WORKSPACE_ID";
private static final String CHE_IS_DEV_MACHINE_ENV_VAR = "CHE_IS_DEV_MACHINE";
private static final int CHE_WORKSPACE_AGENT_PORT = 4401;
private static final int CHE_TERMINAL_AGENT_PORT = 4411;
private static final String DOCKER_PROTOCOL_PORT_DELIMITER = "/";
Expand Down Expand Up @@ -303,9 +305,6 @@ public ContainerCreated createContainer(CreateContainerParams createContainerPar
String containerName = KubernetesStringUtils.convertToContainerName(createContainerParams.getContainerName());
String workspaceID = getCheWorkspaceId(createContainerParams);

// Generate workspaceID if CHE_WORKSPACE_ID env var does not exist
workspaceID = workspaceID.isEmpty() ? KubernetesStringUtils.generateWorkspaceID() : workspaceID;

// imageForDocker is the docker version of the image repository. It's needed for other
// OpenShiftConnector API methods, but is not acceptable as an OpenShift name
String imageForDocker = createContainerParams.getContainerConfig().getImage();
Expand Down Expand Up @@ -350,7 +349,10 @@ public ContainerCreated createContainer(CreateContainerParams createContainerPar
String[] volumes = createContainerParams.getContainerConfig().getHostConfig().getBinds();

Map<String, String> additionalLabels = createContainerParams.getContainerConfig().getLabels();

String networkName = createContainerParams.getContainerConfig().getHostConfig().getNetworkMode();
EndpointConfig endpointConfig = createContainerParams.getContainerConfig().getNetworkingConfig().getEndpointsConfig().get(networkName);
String[] endpointAliases = endpointConfig != null ? endpointConfig.getAliases() : new String[0];

Map<String, Quantity> resourceLimits = new HashMap<>();
if (!isNullOrEmpty(cheWorkspaceMemoryLimit)) {
LOG.info("Che property 'che.openshift.workspace.memory.override' "
Expand All @@ -368,18 +370,32 @@ public ContainerCreated createContainer(CreateContainerParams createContainerPar
resourceRequests.put("memory", new Quantity(cheWorkspaceMemoryRequest));
}

String deploymentName;
String serviceName;
if (isDevMachine(createContainerParams)) {
serviceName = deploymentName = CHE_OPENSHIFT_RESOURCES_PREFIX + workspaceID;
} else {
if (endpointAliases.length > 0) {
serviceName = endpointAliases[0];
deploymentName = CHE_OPENSHIFT_RESOURCES_PREFIX + serviceName;
} else {
// Should never happen
serviceName = deploymentName = CHE_OPENSHIFT_RESOURCES_PREFIX + KubernetesStringUtils.generateWorkspaceID();
}
}

String containerID;
OpenShiftClient openShiftClient = new DefaultOpenShiftClient();
try {
createOpenShiftService(workspaceID, exposedPorts, additionalLabels);
String deploymentName = createOpenShiftDeployment(workspaceID,
dockerPullSpec,
containerName,
exposedPorts,
envVariables,
volumes,
resourceLimits,
resourceRequests);
try {
createOpenShiftService(deploymentName, serviceName, exposedPorts, additionalLabels, endpointAliases);
createOpenShiftDeployment(deploymentName,
dockerPullSpec,
containerName,
exposedPorts,
envVariables,
volumes,
resourceLimits,
resourceRequests);

containerID = waitAndRetrieveContainerID(deploymentName);
if (containerID == null) {
Expand All @@ -390,7 +406,6 @@ public ContainerCreated createContainer(CreateContainerParams createContainerPar
// in an inconsistent state.
LOG.info("Error while creating Pod, removing deployment");
LOG.info(e.getMessage());
String deploymentName = CHE_OPENSHIFT_RESOURCES_PREFIX + workspaceID;
cleanUpWorkspaceResources(deploymentName);
openShiftClient.resource(imageStreamTag).delete();
throw e;
Expand Down Expand Up @@ -1096,13 +1111,20 @@ protected String getCheWorkspaceId(CreateContainerParams createContainerParams)
return workspaceID.replaceFirst("workspace","");
}

private void createOpenShiftService(String workspaceID,
private boolean isDevMachine(CreateContainerParams createContainerParams) {
Stream<String> env = Arrays.stream(createContainerParams.getContainerConfig().getEnv());
return Boolean.parseBoolean(env.filter(v -> v.startsWith(CHE_IS_DEV_MACHINE_ENV_VAR) && v.contains("="))
.map(v -> v.split("=",2)[1])
.findFirst()
.orElse("false"));
}

private void createOpenShiftService(String deploymentName,
String serviceName,
Set<String> exposedPorts,
Map<String, String> additionalLabels) {

String serviceName = CHE_OPENSHIFT_RESOURCES_PREFIX + workspaceID;

Map<String, String> selector = Collections.singletonMap(OPENSHIFT_DEPLOYMENT_LABEL, serviceName);
Map<String, String> additionalLabels,
String[] endpointAliases) {
Map<String, String> selector = Collections.singletonMap(OPENSHIFT_DEPLOYMENT_LABEL, deploymentName);
List<ServicePort> ports = KubernetesService.getServicePortsFrom(exposedPorts);

try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient()) {
Expand All @@ -1123,19 +1145,25 @@ private void createOpenShiftService(String workspaceID,
LOG.info("OpenShift service {} created", service.getMetadata().getName());

for (ServicePort port : ports) {
createOpenShiftRoute(serviceName, port.getName(), workspaceID);
createOpenShiftRoute(serviceName, deploymentName, port.getName());
}
}
}

private void createOpenShiftRoute(String serviceName,
String serverRef,
String workspaceName) {

OpenShiftRouteCreator.createRoute(openShiftCheProjectName, workspaceName, cheServerExternalAddress, serverRef, serviceName, secureRoutes);
String deploymentName,
String serverRef) {
String routeId = serviceName.replaceFirst(CHE_OPENSHIFT_RESOURCES_PREFIX, "");
OpenShiftRouteCreator.createRoute(openShiftCheProjectName,
cheServerExternalAddress,
serverRef,
serviceName,
deploymentName,
routeId,
secureRoutes);
}

private String createOpenShiftDeployment(String workspaceID,
private void createOpenShiftDeployment(String deploymentName,
String imageName,
String sanitizedContainerName,
Set<String> exposedPorts,
Expand All @@ -1144,7 +1172,6 @@ private String createOpenShiftDeployment(String workspaceID,
Map<String, Quantity> resourceLimits,
Map<String, Quantity> resourceRequests) throws OpenShiftException {

String deploymentName = CHE_OPENSHIFT_RESOURCES_PREFIX + workspaceID;
LOG.info("Creating OpenShift deployment {}", deploymentName);

Map<String, String> selector = Collections.singletonMap(OPENSHIFT_DEPLOYMENT_LABEL, deploymentName);
Expand All @@ -1165,7 +1192,7 @@ private String createOpenShiftDeployment(String workspaceID,
.withPrivileged(false)
.endSecurityContext()
.withLivenessProbe(getLivenessProbeFrom(exposedPorts))
.withVolumeMounts(getVolumeMountsFrom(volumes, workspaceID))
.withVolumeMounts(getVolumeMountsFrom(volumes))
.withNewResources()
.withLimits(resourceLimits)
.withRequests(resourceRequests)
Expand All @@ -1174,7 +1201,7 @@ private String createOpenShiftDeployment(String workspaceID,

PodSpec podSpec = new PodSpecBuilder()
.withContainers(container)
.withVolumes(getVolumesFrom(volumes, workspaceID))
.withVolumes(getVolumesFrom(volumes))
.build();

Deployment deployment = new DeploymentBuilder()
Expand Down Expand Up @@ -1204,7 +1231,6 @@ private String createOpenShiftDeployment(String workspaceID,
}

LOG.info("OpenShift deployment {} created", deploymentName);
return deployment.getMetadata().getName();
}

/**
Expand Down Expand Up @@ -1453,22 +1479,24 @@ private String getWorkspaceSubpath(String[] volumes) {
return workspaceSubpath;
}

private List<VolumeMount> getVolumeMountsFrom(String[] volumes, String workspaceID) {
private List<VolumeMount> getVolumeMountsFrom(String[] volumes) {
List<VolumeMount> vms = new ArrayList<>();
PersistentVolumeClaim pvc = getClaimCheWorkspace();
if (pvc != null) {
String subPath = getWorkspaceSubpath(volumes);
VolumeMount vm = new VolumeMountBuilder()
if (subPath != null) {
VolumeMount vm = new VolumeMountBuilder()
.withMountPath(cheWorkspaceProjectsStorage)
.withName(workspacesPersistentVolumeClaim)
.withSubPath(subPath)
.build();
vms.add(vm);
vms.add(vm);
}
}
return vms;
}

private List<Volume> getVolumesFrom(String[] volumes, String workspaceID) {
private List<Volume> getVolumesFrom(String[] volumes) {
List<Volume> vs = new ArrayList<>();
PersistentVolumeClaim pvc = getClaimCheWorkspace();
if (pvc != null) {
Expand Down
11 changes: 6 additions & 5 deletions ...t-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftRouteCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,19 @@ public class OpenShiftRouteCreator {
private static final String REDIRECT_INSECURE_EDGE_TERMINATION_POLICY = "Redirect";

public static void createRoute (final String namespace,
final String workspaceName,
final String openShiftNamespaceExternalAddress,
final String serverRef,
final String serviceName,
final String deploymentName,
final String routeId,
final boolean enableTls) {

if (openShiftNamespaceExternalAddress == null) {
throw new IllegalArgumentException("Property che.docker.ip.external must be set when using openshift.");
}

try (OpenShiftClient openShiftClient = new DefaultOpenShiftClient()) {
String routeName = generateRouteName(workspaceName, serverRef);
String routeName = generateRouteName(routeId, serverRef);
String serviceHost = generateRouteHost(routeName, openShiftNamespaceExternalAddress);

SpecNested<DoneableRoute> routeSpec = openShiftClient
Expand All @@ -45,7 +46,7 @@ public static void createRoute (final String namespace,
.createNew()
.withNewMetadata()
.withName(routeName)
.addToLabels(OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, serviceName)
.addToLabels(OpenShiftConnector.OPENSHIFT_DEPLOYMENT_LABEL, deploymentName)
.endMetadata()
.withNewSpec()
.withHost(serviceHost)
Expand All @@ -72,8 +73,8 @@ public static void createRoute (final String namespace,
}
}

private static String generateRouteName(final String workspaceName, final String serverRef) {
return serverRef + "-" + workspaceName;
private static String generateRouteName(final String serviceName, final String serverRef) {
return serverRef + "-" + serviceName;
}

private static String generateRouteHost(final String routeName, final String openShiftNamespaceExternalAddress) {
Expand Down
6 changes: 6 additions & 0 deletions ...ain/java/org/eclipse/che/plugin/openshift/client/kubernetes/KubernetesLabelConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ public static String getCheServerLabelPrefix() {
*/
public static Map<String, String> labelsToNames(Map<String, String> labels) {
Map<String, String> names = new HashMap<>();
if (labels == null) {
return names;
}
for (Map.Entry<String, String> label : labels.entrySet()) {

if (!hasConversionProblems(label)) {
Expand Down Expand Up @@ -103,6 +106,9 @@ public static Map<String, String> labelsToNames(Map<String, String> labels) {
*/
public static Map<String, String> namesToLabels(Map<String, String> names) {
Map<String, String> labels = new HashMap<>();
if (names == null) {
return labels;
}
for (Map.Entry<String, String> entry: names.entrySet()){
String key = entry.getKey();
String value = entry.getValue();
Expand Down

0 comments on commit b9cf80a

Please sign in to comment.