Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: devfile endpoint with single-host exposure #157

Merged
merged 3 commits into from
Oct 6, 2021
Merged

fix: devfile endpoint with single-host exposure #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71e95f5
does not set require subdomain to devfile endpoints when single-host …
sparkoo Oct 6, 2021
a96ffa2
fix openshift component applier
sparkoo Oct 6, 2021
cd4f9f1
tests
sparkoo Oct 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions ...e/workspace/infrastructure/kubernetes/devfile/DockerimageComponentToWorkspaceApplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
import static org.eclipse.che.api.core.model.workspace.config.Command.MACHINE_NAME_ATTRIBUTE;
import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.DEVFILE_COMPONENT_ALIAS_ATTRIBUTE;
import static org.eclipse.che.api.workspace.server.devfile.Constants.DOCKERIMAGE_COMPONENT_TYPE;
import static org.eclipse.che.api.workspace.server.devfile.convert.component.ComponentToWorkspaceApplier.convertEndpointsIntoServers;
import static org.eclipse.che.api.workspace.shared.Constants.PROJECTS_VOLUME_NAME;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostExternalServiceExposureStrategy.SINGLE_HOST_STRATEGY;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
Expand All @@ -32,13 +34,11 @@
import java.util.stream.Collectors;
import javax.inject.Inject;
import javax.inject.Named;
import org.eclipse.che.api.core.model.workspace.devfile.Endpoint;
import org.eclipse.che.api.workspace.server.devfile.Constants;
import org.eclipse.che.api.workspace.server.devfile.FileContentProvider;
import org.eclipse.che.api.workspace.server.devfile.convert.component.ComponentToWorkspaceApplier;
import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException;
import org.eclipse.che.api.workspace.server.model.impl.MachineConfigImpl;
import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl;
import org.eclipse.che.api.workspace.server.model.impl.VolumeImpl;
import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl;
import org.eclipse.che.api.workspace.server.model.impl.devfile.ComponentImpl;
Expand All @@ -65,15 +65,19 @@ public class DockerimageComponentToWorkspaceApplier implements ComponentToWorksp
private final String projectFolderPath;
private final String imagePullPolicy;
private final KubernetesEnvironmentProvisioner k8sEnvProvisioner;
private final String devfileEndpointsExposure;

@Inject
public DockerimageComponentToWorkspaceApplier(
@Named("che.workspace.projects.storage") String projectFolderPath,
@Named("che.workspace.sidecar.image_pull_policy") String imagePullPolicy,
@Named("che.infra.kubernetes.singlehost.workspace.devfile_endpoint_exposure")
String devfileEndpointsExposure,
KubernetesEnvironmentProvisioner k8sEnvProvisioner) {
this.projectFolderPath = projectFolderPath;
this.imagePullPolicy = imagePullPolicy;
this.k8sEnvProvisioner = k8sEnvProvisioner;
this.devfileEndpointsExposure = devfileEndpointsExposure;
}

/**
Expand Down Expand Up @@ -137,12 +141,9 @@ private MachineConfigImpl createMachineConfig(
machineConfig
.getServers()
.putAll(
dockerimageComponent
.getEndpoints()
.stream()
.collect(
Collectors.toMap(
Endpoint::getName, e -> ServerConfigImpl.createFromEndpoint(e, true))));
convertEndpointsIntoServers(
dockerimageComponent.getEndpoints(),
!SINGLE_HOST_STRATEGY.equals(devfileEndpointsExposure)));

dockerimageComponent
.getVolumes()
Expand Down
19 changes: 10 additions & 9 deletions ...he/workspace/infrastructure/kubernetes/devfile/KubernetesComponentToWorkspaceApplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
import static org.eclipse.che.api.core.model.workspace.config.Command.MACHINE_NAME_ATTRIBUTE;
import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.DEVFILE_COMPONENT_ALIAS_ATTRIBUTE;
import static org.eclipse.che.api.workspace.server.devfile.Components.getIdentifiableComponentName;
import static org.eclipse.che.api.workspace.server.devfile.convert.component.ComponentToWorkspaceApplier.convertEndpointsIntoServers;
import static org.eclipse.che.api.workspace.shared.Constants.PROJECTS_VOLUME_NAME;
import static org.eclipse.che.workspace.infrastructure.kubernetes.Names.machineName;
import static org.eclipse.che.workspace.infrastructure.kubernetes.devfile.KubernetesDevfileBindings.KUBERNETES_BASED_COMPONENTS_KEY_NAME;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.newPVC;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.newVolume;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesObjectUtil.newVolumeMount;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostExternalServiceExposureStrategy.SINGLE_HOST_STRATEGY;

import io.fabric8.kubernetes.api.model.Container;
import io.fabric8.kubernetes.api.model.HasMetadata;
Expand All @@ -39,22 +41,19 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.inject.Inject;
import javax.inject.Named;
import org.eclipse.che.api.core.model.workspace.WorkspaceConfig;
import org.eclipse.che.api.core.model.workspace.config.Command;
import org.eclipse.che.api.core.model.workspace.devfile.Component;
import org.eclipse.che.api.core.model.workspace.devfile.Endpoint;
import org.eclipse.che.api.core.model.workspace.devfile.Entrypoint;
import org.eclipse.che.api.workspace.server.devfile.Constants;
import org.eclipse.che.api.workspace.server.devfile.DevfileRecipeFormatException;
import org.eclipse.che.api.workspace.server.devfile.FileContentProvider;
import org.eclipse.che.api.workspace.server.devfile.convert.component.ComponentToWorkspaceApplier;
import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException;
import org.eclipse.che.api.workspace.server.model.impl.MachineConfigImpl;
import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl;
import org.eclipse.che.api.workspace.server.model.impl.VolumeImpl;
import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl;
import org.eclipse.che.api.workspace.server.model.impl.devfile.ComponentImpl;
Expand All @@ -80,6 +79,7 @@ public class KubernetesComponentToWorkspaceApplier implements ComponentToWorkspa
private final String defaultPVCAccessMode;
private final String pvcStorageClassName;
private final EnvVars envVars;
private final String devfileEndpointsExposure;

@Inject
public KubernetesComponentToWorkspaceApplier(
Expand All @@ -91,6 +91,8 @@ public KubernetesComponentToWorkspaceApplier(
@Named("che.infra.kubernetes.pvc.access_mode") String defaultPVCAccessMode,
@Named("che.infra.kubernetes.pvc.storage_class_name") String pvcStorageClassName,
@Named("che.workspace.sidecar.image_pull_policy") String imagePullPolicy,
@Named("che.infra.kubernetes.singlehost.workspace.devfile_endpoint_exposure")
String devfileEndpointsExposure,
@Named(KUBERNETES_BASED_COMPONENTS_KEY_NAME) Set<String> kubernetesBasedComponentTypes) {
this(
objectsParser,
Expand All @@ -102,6 +104,7 @@ public KubernetesComponentToWorkspaceApplier(
defaultPVCAccessMode,
pvcStorageClassName,
imagePullPolicy,
devfileEndpointsExposure,
kubernetesBasedComponentTypes);
}

Expand All @@ -115,6 +118,7 @@ protected KubernetesComponentToWorkspaceApplier(
String defaultPVCAccessMode,
String pvcStorageClassName,
String imagePullPolicy,
String devfileEndpointsExposure,
Set<String> kubernetesBasedComponentTypes) {
this.objectsParser = objectsParser;
this.k8sEnvProvisioner = k8sEnvProvisioner;
Expand All @@ -126,6 +130,7 @@ protected KubernetesComponentToWorkspaceApplier(
this.imagePullPolicy = imagePullPolicy;
this.kubernetesBasedComponentTypes = kubernetesBasedComponentTypes;
this.envVars = envVars;
this.devfileEndpointsExposure = devfileEndpointsExposure;
}

/**
Expand Down Expand Up @@ -267,12 +272,8 @@ private void provisionEndpoints(Component component, MachineConfigImpl config) {
config
.getServers()
.putAll(
component
.getEndpoints()
.stream()
.collect(
Collectors.toMap(
Endpoint::getName, e -> ServerConfigImpl.createFromEndpoint(e, true))));
convertEndpointsIntoServers(
component.getEndpoints(), !SINGLE_HOST_STRATEGY.equals(devfileEndpointsExposure)));
}

private void provisionVolumes(
Expand Down
80 changes: 78 additions & 2 deletions ...rkspace/infrastructure/kubernetes/devfile/DockerimageComponentToWorkspaceApplierTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import static org.eclipse.che.api.workspace.server.devfile.Constants.PUBLIC_ENDPOINT_ATTRIBUTE;
import static org.eclipse.che.api.workspace.shared.Constants.PROJECTS_VOLUME_NAME;
import static org.eclipse.che.workspace.infrastructure.kubernetes.devfile.DockerimageComponentToWorkspaceApplier.CHE_COMPONENT_NAME_LABEL;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.MultiHostExternalServiceExposureStrategy.MULTI_HOST_STRATEGY;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.external.SingleHostExternalServiceExposureStrategy.SINGLE_HOST_STRATEGY;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -80,7 +82,7 @@ public class DockerimageComponentToWorkspaceApplierTest {
public void setUp() throws Exception {
dockerimageComponentApplier =
new DockerimageComponentToWorkspaceApplier(
PROJECTS_MOUNT_PATH, "Always", k8sEnvProvisioner);
PROJECTS_MOUNT_PATH, "Always", MULTI_HOST_STRATEGY, k8sEnvProvisioner);
workspaceConfig = new WorkspaceConfigImpl();
}

Expand Down Expand Up @@ -135,7 +137,8 @@ public void shouldBeAbleToSetupSpecificImagePullPolicy() throws DevfileException
dockerimageComponent.setImage("eclipse/ubuntu_jdk8:latest");
dockerimageComponent.setMemoryLimit("1G");
dockerimageComponentApplier =
new DockerimageComponentToWorkspaceApplier(PROJECTS_MOUNT_PATH, "Never", k8sEnvProvisioner);
new DockerimageComponentToWorkspaceApplier(
PROJECTS_MOUNT_PATH, "Never", MULTI_HOST_STRATEGY, k8sEnvProvisioner);

// when
dockerimageComponentApplier.apply(workspaceConfig, dockerimageComponent, null);
Expand Down Expand Up @@ -570,6 +573,79 @@ public void shouldProvisionCommandAndArgs() throws Exception {
assertEquals(container.getArgs(), args);
}

@Test
public void serverMustHaveRequireSubdomainWhenNonSinglehostDevfileExpose()
throws DevfileException {
dockerimageComponentApplier =
new DockerimageComponentToWorkspaceApplier(
PROJECTS_MOUNT_PATH, "Always", MULTI_HOST_STRATEGY, k8sEnvProvisioner);

// given
EndpointImpl endpoint = new EndpointImpl("jdk-ls", 4923, emptyMap());
ComponentImpl dockerimageComponent = new ComponentImpl();
dockerimageComponent.setAlias("jdk");
dockerimageComponent.setType(DOCKERIMAGE_COMPONENT_TYPE);
dockerimageComponent.setImage("eclipse/ubuntu_jdk8:latest");
dockerimageComponent.setMemoryLimit("100M");
dockerimageComponent.setEndpoints(
Arrays.asList(
new EndpointImpl("e1", 1111, emptyMap()), new EndpointImpl("e2", 2222, emptyMap())));

// when
dockerimageComponentApplier.apply(workspaceConfig, dockerimageComponent, null);

// then
verify(k8sEnvProvisioner)
.provision(
eq(workspaceConfig),
eq(KubernetesEnvironment.TYPE),
objectsCaptor.capture(),
machinesCaptor.capture());
MachineConfigImpl machineConfig = machinesCaptor.getValue().get("jdk");
assertNotNull(machineConfig);
assertEquals(machineConfig.getServers().size(), 2);
assertTrue(
ServerConfig.isRequireSubdomain(machineConfig.getServers().get("e1").getAttributes()));
assertTrue(
ServerConfig.isRequireSubdomain(machineConfig.getServers().get("e2").getAttributes()));
}

@Test
public void serverCantHaveRequireSubdomainWhenSinglehostDevfileExpose() throws DevfileException {
dockerimageComponentApplier =
new DockerimageComponentToWorkspaceApplier(
PROJECTS_MOUNT_PATH, "Always", SINGLE_HOST_STRATEGY, k8sEnvProvisioner);

// given
EndpointImpl endpoint = new EndpointImpl("jdk-ls", 4923, emptyMap());
ComponentImpl dockerimageComponent = new ComponentImpl();
dockerimageComponent.setAlias("jdk");
dockerimageComponent.setType(DOCKERIMAGE_COMPONENT_TYPE);
dockerimageComponent.setImage("eclipse/ubuntu_jdk8:latest");
dockerimageComponent.setMemoryLimit("100M");
dockerimageComponent.setEndpoints(
Arrays.asList(
new EndpointImpl("e1", 1111, emptyMap()), new EndpointImpl("e2", 2222, emptyMap())));

// when
dockerimageComponentApplier.apply(workspaceConfig, dockerimageComponent, null);

// then
verify(k8sEnvProvisioner)
.provision(
eq(workspaceConfig),
eq(KubernetesEnvironment.TYPE),
objectsCaptor.capture(),
machinesCaptor.capture());
MachineConfigImpl machineConfig = machinesCaptor.getValue().get("jdk");
assertNotNull(machineConfig);
assertEquals(machineConfig.getServers().size(), 2);
assertFalse(
ServerConfig.isRequireSubdomain(machineConfig.getServers().get("e1").getAttributes()));
assertFalse(
ServerConfig.isRequireSubdomain(machineConfig.getServers().get("e2").getAttributes()));
}

@Test(dataProvider = "imageNames")
public void testGeneratesValidMachineNameFromImageName(String imageName)
throws ValidationException, DevfileException {
Expand Down
Loading