Simplify KubernetesEnvironment#addPod() slightly + cleanup
Remove podName parameter from addPod method and simply get it from the pod's metadata. Additional cleanup and documentation changes: - Make some log messages level 'info' instead of 'debug' - Modify argument order in messages to to make more sense - Add docs for KubernetesEnvironment methods about how/when pods map can be modified - Fix issue where pods referencing nonexistent configmaps would emit unreadable error messages due to configmap renaming. - Clean up code according to PR Quality Review Signed-off-by: Angel Misevski <amisevsk@redhat.com>6.19.x
Angel Misevski
parent
e0f774c456
commit
a1417ac9c6
|
|
@ -718,9 +718,9 @@ public class KubernetesInternalRuntime<E extends KubernetesEnvironment>
|
|||
final KubernetesEnvironment environment = getContext().getEnvironment();
|
||||
final Map<String, InternalMachineConfig> machineConfigs = environment.getMachines();
|
||||
final String workspaceId = getContext().getIdentity().getWorkspaceId();
|
||||
LOG.debug("Begin pods creation for workspace '{}'", workspaceId);
|
||||
LOG.info("Begin pods creation for workspace '{}'", workspaceId);
|
||||
for (Pod toCreate : environment.getPodsCopy().values()) {
|
||||
startTracingContainersStartup(toCreate.getSpec(), toCreate.getMetadata());
|
||||
startTracingContainersStartup(toCreate.getMetadata(), toCreate.getSpec());
|
||||
ObjectMeta toCreateMeta = toCreate.getMetadata();
|
||||
final Pod createdPod = namespace.deployments().deploy(toCreate);
|
||||
LOG.debug("Creating pod '{}' in workspace '{}'", toCreateMeta.getName(), workspaceId);
|
||||
|
|
@ -728,7 +728,7 @@ public class KubernetesInternalRuntime<E extends KubernetesEnvironment>
|
|||
}
|
||||
for (Deployment toCreate : environment.getDeploymentsCopy().values()) {
|
||||
PodTemplateSpec template = toCreate.getSpec().getTemplate();
|
||||
startTracingContainersStartup(template.getSpec(), template.getMetadata());
|
||||
startTracingContainersStartup(template.getMetadata(), template.getSpec());
|
||||
ObjectMeta toCreateMeta = toCreate.getMetadata();
|
||||
final Pod createdPod = namespace.deployments().deploy(toCreate);
|
||||
LOG.debug("Creating deployment '{}' in workspace '{}'", toCreateMeta.getName(), workspaceId);
|
||||
|
|
@ -737,7 +737,7 @@ public class KubernetesInternalRuntime<E extends KubernetesEnvironment>
|
|||
final ObjectMeta templateMeta = toCreate.getSpec().getTemplate().getMetadata();
|
||||
storeStartingMachine(createdPod, templateMeta, machineConfigs, serverResolver);
|
||||
}
|
||||
LOG.debug("Pods creation finished in workspace '{}'", workspaceId);
|
||||
LOG.info("Pods creation finished in workspace '{}'", workspaceId);
|
||||
}
|
||||
|
||||
/** Puts createdPod in the {@code machines} map and sends the starting event for this machine */
|
||||
|
|
@ -766,7 +766,7 @@ public class KubernetesInternalRuntime<E extends KubernetesEnvironment>
|
|||
}
|
||||
}
|
||||
|
||||
private void startTracingContainersStartup(PodSpec podSpec, ObjectMeta podMeta) {
|
||||
private void startTracingContainersStartup(ObjectMeta podMeta, PodSpec podSpec) {
|
||||
if (tracer == null) {
|
||||
return;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -165,6 +165,9 @@ public class KubernetesEnvironment extends InternalEnvironment {
|
|||
*
|
||||
* <p>Note: This map <b>should not</b> be changed, as it will only return pods and not
|
||||
* deployments. If objects in the map need to be changed, see {@link #getPodsData()}
|
||||
*
|
||||
* <p>If pods need to be added to the environment, then {@link #addPod(Pod)} should be used
|
||||
* instead.
|
||||
*/
|
||||
public Map<String, Pod> getPodsCopy() {
|
||||
return ImmutableMap.copyOf(pods);
|
||||
|
|
@ -175,6 +178,9 @@ public class KubernetesEnvironment extends InternalEnvironment {
|
|||
*
|
||||
* <p>Note: This map <b>should not</b> be changed. If objects in the map need to be changed, see
|
||||
* {@link #getPodsData()}
|
||||
*
|
||||
* <p>If pods need to be added to the environment, then {@link #addPod(Pod)} should be used
|
||||
* instead.
|
||||
*/
|
||||
public Map<String, Deployment> getDeploymentsCopy() {
|
||||
return ImmutableMap.copyOf(deployments);
|
||||
|
|
@ -185,6 +191,9 @@ public class KubernetesEnvironment extends InternalEnvironment {
|
|||
* that should be created when environment starts. The data returned by this method represents all
|
||||
* deployment and pod objects that form the workspace, and should be used when provisioning or
|
||||
* performing any action that needs to see every object in the environment.
|
||||
*
|
||||
* <p>If pods need to be added to the environment, then {@link #addPod(Pod)} should be used
|
||||
* instead.
|
||||
*/
|
||||
public Map<String, PodData> getPodsData() {
|
||||
return ImmutableMap.copyOf(podData);
|
||||
|
|
@ -194,7 +203,8 @@ public class KubernetesEnvironment extends InternalEnvironment {
|
|||
* Add a pod to the current environment. This method is necessary as the map returned by {@link
|
||||
* #getPodsCopy()} is a copy. This method also adds the relevant data to {@link #getPodsData()}.
|
||||
*/
|
||||
public void addPod(String podName, Pod pod) {
|
||||
public void addPod(Pod pod) {
|
||||
String podName = pod.getMetadata().getName();
|
||||
pods.put(podName, pod);
|
||||
podData.put(podName, new PodData(pod.getSpec(), pod.getMetadata()));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -35,7 +35,8 @@ public class KubernetesEnvironmentValidator {
|
|||
* @throws ValidationException if the specified {@link KubernetesEnvironment} is invalid
|
||||
*/
|
||||
public void validate(KubernetesEnvironment env) throws ValidationException {
|
||||
checkArgument(!env.getPodsData().isEmpty(), "Environment should contain at least 1 pod");
|
||||
checkArgument(
|
||||
!env.getPodsData().isEmpty(), "Environment should contain at least 1 pod or deployment");
|
||||
|
||||
Set<String> missingMachines = new HashSet<>(env.getMachines().keySet());
|
||||
for (PodData pod : env.getPodsData().values()) {
|
||||
|
|
|
|||
|
|
@ -117,7 +117,12 @@ public class UniqueNamesProvisioner<T extends KubernetesEnvironment>
|
|||
env -> {
|
||||
ConfigMapKeySelector configMap = env.getValueFrom().getConfigMapKeyRef();
|
||||
String originalName = configMap.getName();
|
||||
configMap.setName(configMapNameTranslation.get(originalName));
|
||||
// Since pods can reference configmaps that don't exist, we only change the name
|
||||
// if the configmap does exist to aid debugging recipes (otherwise message is just
|
||||
// null).
|
||||
if (configMapNameTranslation.containsKey(originalName)) {
|
||||
configMap.setName(configMapNameTranslation.get(originalName));
|
||||
}
|
||||
});
|
||||
}
|
||||
if (container.getEnvFrom() != null) {
|
||||
|
|
@ -130,7 +135,9 @@ public class UniqueNamesProvisioner<T extends KubernetesEnvironment>
|
|||
envFrom -> {
|
||||
ConfigMapEnvSource configMapRef = envFrom.getConfigMapRef();
|
||||
String originalName = configMapRef.getName();
|
||||
configMapRef.setName(configMapNameTranslation.get(originalName));
|
||||
if (configMapNameTranslation.containsKey(originalName)) {
|
||||
configMapRef.setName(configMapNameTranslation.get(originalName));
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
@ -144,7 +151,9 @@ public class UniqueNamesProvisioner<T extends KubernetesEnvironment>
|
|||
volume -> {
|
||||
ConfigMapVolumeSource configMapVolume = volume.getConfigMap();
|
||||
String originalName = configMapVolume.getName();
|
||||
configMapVolume.setName(configMapNameTranslation.get(originalName));
|
||||
if (configMapNameTranslation.containsKey(originalName)) {
|
||||
configMapVolume.setName(configMapNameTranslation.get(originalName));
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -213,7 +213,7 @@ public class JwtProxyProvisioner {
|
|||
private void ensureJwtProxyInjected(KubernetesEnvironment k8sEnv) throws InfrastructureException {
|
||||
if (!k8sEnv.getMachines().containsKey(JWT_PROXY_MACHINE_NAME)) {
|
||||
k8sEnv.getMachines().put(JWT_PROXY_MACHINE_NAME, createJwtProxyMachine());
|
||||
k8sEnv.addPod(JWT_PROXY_POD_NAME, createJwtProxyPod());
|
||||
k8sEnv.addPod(createJwtProxyPod());
|
||||
|
||||
KeyPair keyPair;
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -127,7 +127,7 @@ public class KubernetesPluginsToolingApplier implements ChePluginsApplier {
|
|||
.endSpec()
|
||||
.build();
|
||||
|
||||
kubernetesEnvironment.addPod(CHE_WORKSPACE_POD, pod);
|
||||
kubernetesEnvironment.addPod(pod);
|
||||
}
|
||||
|
||||
private void populateWorkspaceEnvVars(
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ import static org.eclipse.che.workspace.infrastructure.kubernetes.Constants.MACH
|
|||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.Mockito.doReturn;
|
||||
import static org.testng.Assert.assertEquals;
|
||||
import static org.testng.Assert.assertFalse;
|
||||
import static org.testng.Assert.assertNotNull;
|
||||
import static org.testng.Assert.assertTrue;
|
||||
|
||||
|
|
@ -80,14 +81,11 @@ public class KubernetesBrokerInitContainerApplierTest {
|
|||
@Mock private InternalMachineConfig brokerMachine;
|
||||
private Volume brokerVolume;
|
||||
private ConfigMap brokerConfigMap;
|
||||
private KubernetesEnvironment brokerEnvironment;
|
||||
private Pod brokerPod;
|
||||
private Container brokerContainer;
|
||||
|
||||
// Workspace Environment mocks
|
||||
private KubernetesEnvironment workspaceEnvironment;
|
||||
private Pod workspacePod;
|
||||
private Map<String, ConfigMap> workspaceConfigMaps;
|
||||
|
||||
private KubernetesBrokerInitContainerApplier<KubernetesEnvironment> applier;
|
||||
|
||||
|
|
@ -97,7 +95,7 @@ public class KubernetesBrokerInitContainerApplierTest {
|
|||
ObjectMeta workspacePodMeta =
|
||||
new ObjectMetaBuilder().withAnnotations(workspacePodAnnotations).build();
|
||||
workspacePod = new PodBuilder().withMetadata(workspacePodMeta).withSpec(new PodSpec()).build();
|
||||
workspaceConfigMaps = new HashMap<>();
|
||||
Map<String, ConfigMap> workspaceConfigMaps = new HashMap<>();
|
||||
workspaceEnvironment =
|
||||
KubernetesEnvironment.builder()
|
||||
.setPods(ImmutableMap.of(WORKSPACE_POD_NAME, workspacePod))
|
||||
|
|
@ -110,7 +108,7 @@ public class KubernetesBrokerInitContainerApplierTest {
|
|||
new ObjectMetaBuilder().withAnnotations(brokerPodAnnotations).build();
|
||||
brokerContainer = new ContainerBuilder().withName(BROKER_CONTAINER_NAME).build();
|
||||
brokerVolume = new VolumeBuilder().build();
|
||||
brokerPod =
|
||||
Pod brokerPod =
|
||||
new PodBuilder()
|
||||
.withMetadata(brokerPodMeta)
|
||||
.withNewSpec()
|
||||
|
|
@ -119,7 +117,7 @@ public class KubernetesBrokerInitContainerApplierTest {
|
|||
.endSpec()
|
||||
.build();
|
||||
brokerConfigMap = new ConfigMapBuilder().addToData(brokerConfigMapData).build();
|
||||
brokerEnvironment =
|
||||
KubernetesEnvironment brokerEnvironment =
|
||||
KubernetesEnvironment.builder()
|
||||
.setPods(ImmutableMap.of(BROKER_POD_NAME, brokerPod))
|
||||
.setConfigMaps(ImmutableMap.of(BROKER_CONFIGMAP_NAME, brokerConfigMap))
|
||||
|
|
@ -144,7 +142,7 @@ public class KubernetesBrokerInitContainerApplierTest {
|
|||
|
||||
ConfigMap workspaceConfigMap = workspaceEnvironment.getConfigMaps().get(BROKER_CONFIGMAP_NAME);
|
||||
assertNotNull(workspaceConfigMap);
|
||||
assertTrue(!workspaceConfigMap.getData().isEmpty());
|
||||
assertFalse(workspaceConfigMap.getData().isEmpty());
|
||||
assertTrue(
|
||||
workspaceConfigMap
|
||||
.getData()
|
||||
|
|
|
|||
|
|
@ -49,7 +49,7 @@ public class KubernetesEnvironmentValidatorTest {
|
|||
|
||||
@Test(
|
||||
expectedExceptions = ValidationException.class,
|
||||
expectedExceptionsMessageRegExp = "Environment should contain at least 1 pod")
|
||||
expectedExceptionsMessageRegExp = "Environment should contain at least 1 pod or deployment")
|
||||
public void shouldThrowExceptionWhenEnvDoesNotHaveAnyPods() throws Exception {
|
||||
// given
|
||||
when(kubernetesEnvironment.getPodsData()).thenReturn(emptyMap());
|
||||
|
|
|
|||
|
|
@ -161,7 +161,6 @@ public class CommonPVCStrategyTest {
|
|||
mockName(pod, POD_NAME);
|
||||
mockName(pod2, POD_NAME_2);
|
||||
|
||||
Map<String, Pod> pods = ImmutableMap.of(POD_NAME, pod, POD_NAME_2, pod);
|
||||
Map<String, PodData> podData =
|
||||
ImmutableMap.of(
|
||||
POD_NAME, new PodData(pod.getSpec(), pod.getMetadata()),
|
||||
|
|
|
|||
|
|
@ -107,8 +107,8 @@ public class CertificateProvisionerTest {
|
|||
@Test
|
||||
public void shouldAddVolumeAndVolumeMountsToPodsAndContainersInEnvironment() throws Exception {
|
||||
// given
|
||||
k8sEnv.addPod("pod", createPod());
|
||||
k8sEnv.addPod("pod2", createPod());
|
||||
k8sEnv.addPod(createPod("pod"));
|
||||
k8sEnv.addPod(createPod("pod2"));
|
||||
|
||||
// when
|
||||
provisioner.provision(k8sEnv, runtimeId);
|
||||
|
|
@ -128,8 +128,8 @@ public class CertificateProvisionerTest {
|
|||
throws Exception {
|
||||
// given
|
||||
provisioner = new CertificateProvisioner("");
|
||||
k8sEnv.addPod("pod", createPod());
|
||||
k8sEnv.addPod("pod2", createPod());
|
||||
k8sEnv.addPod(createPod("pod"));
|
||||
k8sEnv.addPod(createPod("pod2"));
|
||||
|
||||
// when
|
||||
provisioner.provision(k8sEnv, runtimeId);
|
||||
|
|
@ -162,9 +162,10 @@ public class CertificateProvisionerTest {
|
|||
assertEquals(volumeMount.getMountPath(), CERT_MOUNT_PATH);
|
||||
}
|
||||
|
||||
private Pod createPod() {
|
||||
private Pod createPod(String podName) {
|
||||
return new PodBuilder()
|
||||
.withNewMetadata()
|
||||
.withName(podName)
|
||||
.endMetadata()
|
||||
.withNewSpec()
|
||||
.withContainers(new ContainerBuilder().build(), new ContainerBuilder().build())
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@ import com.google.common.collect.ImmutableMap;
|
|||
import com.google.gson.Gson;
|
||||
import io.fabric8.kubernetes.api.model.LocalObjectReference;
|
||||
import io.fabric8.kubernetes.api.model.LocalObjectReferenceBuilder;
|
||||
import io.fabric8.kubernetes.api.model.ObjectMeta;
|
||||
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
|
||||
import io.fabric8.kubernetes.api.model.Pod;
|
||||
import io.fabric8.kubernetes.api.model.PodSpec;
|
||||
import io.fabric8.kubernetes.api.model.Secret;
|
||||
|
|
@ -75,9 +77,11 @@ public class ImagePullSecretProvisionerTest {
|
|||
when(runtimeIdentity.getWorkspaceId()).thenReturn(WORKSPACE_ID);
|
||||
|
||||
k8sEnv = KubernetesEnvironment.builder().build();
|
||||
ObjectMeta podMeta = new ObjectMetaBuilder().withName("wksp").build();
|
||||
when(pod.getMetadata()).thenReturn(podMeta);
|
||||
when(pod.getSpec()).thenReturn(podSpec);
|
||||
when(podSpec.getImagePullSecrets()).thenReturn(ImmutableList.of(existingImagePullSecretRef));
|
||||
k8sEnv.addPod("wksp", pod);
|
||||
k8sEnv.addPod(pod);
|
||||
|
||||
when(credentialsProvider.getCredentials()).thenReturn(authConfigs);
|
||||
imagePullSecretProvisioner = new ImagePullSecretProvisioner(credentialsProvider);
|
||||
|
|
|
|||
|
|
@ -20,7 +20,6 @@ import static org.testng.Assert.assertEquals;
|
|||
|
||||
import io.fabric8.kubernetes.api.model.Container;
|
||||
import io.fabric8.kubernetes.api.model.ObjectMeta;
|
||||
import io.fabric8.kubernetes.api.model.Pod;
|
||||
import io.fabric8.kubernetes.api.model.PodSpec;
|
||||
import io.fabric8.kubernetes.api.model.Quantity;
|
||||
import io.fabric8.kubernetes.api.model.ResourceRequirements;
|
||||
|
|
@ -55,7 +54,6 @@ public class RamLimitRequestProvisionerTest {
|
|||
|
||||
@Mock private KubernetesEnvironment k8sEnv;
|
||||
@Mock private RuntimeIdentity identity;
|
||||
@Mock private Pod pod;
|
||||
@Mock private InternalMachineConfig internalMachineConfig;
|
||||
@Mock private MemoryAttributeProvisioner memoryAttributeProvisioner;
|
||||
|
||||
|
|
|
|||
|
|
@ -111,7 +111,7 @@ public class KubernetesPluginsToolingApplierTest {
|
|||
lenient().when(podSpec.getContainers()).thenReturn(containers);
|
||||
lenient().when(pod.getMetadata()).thenReturn(meta);
|
||||
lenient().when(meta.getName()).thenReturn(POD_NAME);
|
||||
internalEnvironment.addPod(POD_NAME, pod);
|
||||
internalEnvironment.addPod(pod);
|
||||
internalEnvironment.getMachines().putAll(machines);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue