From ec461af679cd9d4f62f089fc2dda6ce7a7251455 Mon Sep 17 00:00:00 2001 From: Anton Korneta Date: Fri, 24 Jun 2016 20:16:57 +0300 Subject: [PATCH] CHE-1276: Add exception handler during creation machine from snapshot --- .../plugin/docker/client/DockerConnector.java | 26 ++++++-- .../{ => exception}/DockerException.java | 4 +- .../exception/ImageNotFoundException.java | 23 ++++++++ .../docker/client/DockerConnectorTest.java | 1 + .../machine/DockerInstanceProvider.java | 25 +++++--- .../api/machine/server/MachineManager.java | 26 ++++++-- .../exception/SourceNotFoundException.java | 36 +++++++++++ .../machine/server/spi/InstanceProvider.java | 2 + .../machine/server/MachineManagerTest.java | 59 ++++++++++++++++++- 9 files changed, 181 insertions(+), 21 deletions(-) rename plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/{ => exception}/DockerException.java (91%) create mode 100644 plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/exception/ImageNotFoundException.java create mode 100644 wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/exception/SourceNotFoundException.java diff --git a/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/DockerConnector.java b/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/DockerConnector.java index 00a20258ce..a7877973d8 100644 --- a/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/DockerConnector.java +++ b/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/DockerConnector.java @@ -21,6 +21,7 @@ import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.commons.json.JsonHelper; import org.eclipse.che.commons.json.JsonNameConvention; import org.eclipse.che.commons.json.JsonParseException; +import org.eclipse.che.commons.lang.IoUtil; import org.eclipse.che.commons.lang.Pair; import org.eclipse.che.commons.lang.TarUtils; import org.eclipse.che.commons.lang.ws.rs.ExtMediaType; @@ -29,6 +30,8 @@ import org.eclipse.che.plugin.docker.client.connection.DockerConnection; import org.eclipse.che.plugin.docker.client.connection.DockerConnectionFactory; import org.eclipse.che.plugin.docker.client.connection.DockerResponse; import org.eclipse.che.plugin.docker.client.dto.AuthConfigs; +import org.eclipse.che.plugin.docker.client.exception.DockerException; +import org.eclipse.che.plugin.docker.client.exception.ImageNotFoundException; import org.eclipse.che.plugin.docker.client.json.ContainerCommitted; import org.eclipse.che.plugin.docker.client.json.ContainerConfig; import org.eclipse.che.plugin.docker.client.json.ContainerCreated; @@ -375,7 +378,8 @@ public class DockerConnector { * @deprecated use {@link #tag(TagParams)} instead */ @Deprecated - public void tag(String image, String repository, String tag) throws IOException { + public void tag(String image, String repository, String tag) throws ImageNotFoundException, + IOException { tag(TagParams.create(image, repository).withTag(tag), dockerDaemonUri); } @@ -1242,21 +1246,29 @@ public class DockerConnector { * @deprecated use {@link #tag(TagParams)} */ @Deprecated - protected void doTag(String image, String repository, String tag, URI dockerDaemonUri) throws IOException { + protected void doTag(String image, + String repository, + String tag, + URI dockerDaemonUri) throws ImageNotFoundException, + IOException { tag(TagParams.create(image, repository).withTag(tag), dockerDaemonUri); } /** * Tag the docker image into a repository. * + * @throws ImageNotFoundException + * when docker api return 404 status * @throws IOException - * when a problem occurs with docker api calls + * when a problem occurs with docker api calls */ - public void tag(final TagParams params) throws IOException { + public void tag(final TagParams params) throws ImageNotFoundException, + IOException { tag(params, dockerDaemonUri); } - private void tag(final TagParams params, URI dockerDaemonUri) throws IOException { + private void tag(final TagParams params, URI dockerDaemonUri) throws ImageNotFoundException, + IOException { try (DockerConnection connection = connectionFactory.openConnection(dockerDaemonUri) .method("POST") .path("/images/" + params.getImage() + "/tag") @@ -1264,7 +1276,9 @@ public class DockerConnector { addQueryParamIfNotNull(connection, "force", params.isForce()); addQueryParamIfNotNull(connection, "tag", params.getTag()); final DockerResponse response = connection.request(); - if (response.getStatus() / 100 != 2) { + if (response.getStatus() == 404) { + throw new ImageNotFoundException(IoUtil.readStream(response.getInputStream())); + } else if (response.getStatus() / 100 != 2) { throw getDockerException(response); } } diff --git a/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/DockerException.java b/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/exception/DockerException.java similarity index 91% rename from plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/DockerException.java rename to plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/exception/DockerException.java index 9a28969e9b..d2056d4032 100644 --- a/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/DockerException.java +++ b/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/exception/DockerException.java @@ -8,14 +8,14 @@ * Contributors: * Codenvy, S.A. - initial API and implementation *******************************************************************************/ -package org.eclipse.che.plugin.docker.client; +package org.eclipse.che.plugin.docker.client.exception; import java.io.IOException; /** * @author andrew00x */ -public final class DockerException extends IOException { +public class DockerException extends IOException { private final int status; private final String originError; diff --git a/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/exception/ImageNotFoundException.java b/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/exception/ImageNotFoundException.java new file mode 100644 index 0000000000..3ac3a7584e --- /dev/null +++ b/plugins/plugin-docker/che-plugin-docker-client/src/main/java/org/eclipse/che/plugin/docker/client/exception/ImageNotFoundException.java @@ -0,0 +1,23 @@ +/******************************************************************************* + * Copyright (c) 2012-2016 Codenvy, S.A. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Codenvy, S.A. - initial API and implementation + *******************************************************************************/ +package org.eclipse.che.plugin.docker.client.exception; + +/** + * Occurs when docker image is not found. + * + * @author Anton Korneta + */ +public class ImageNotFoundException extends DockerException { + + public ImageNotFoundException(String message) { + super(message, 404); + } +} diff --git a/plugins/plugin-docker/che-plugin-docker-client/src/test/java/org/eclipse/che/plugin/docker/client/DockerConnectorTest.java b/plugins/plugin-docker/che-plugin-docker-client/src/test/java/org/eclipse/che/plugin/docker/client/DockerConnectorTest.java index c4bc630c5b..ac025b91b4 100644 --- a/plugins/plugin-docker/che-plugin-docker-client/src/test/java/org/eclipse/che/plugin/docker/client/DockerConnectorTest.java +++ b/plugins/plugin-docker/che-plugin-docker-client/src/test/java/org/eclipse/che/plugin/docker/client/DockerConnectorTest.java @@ -23,6 +23,7 @@ import org.eclipse.che.plugin.docker.client.connection.DockerConnectionFactory; import org.eclipse.che.plugin.docker.client.connection.DockerResponse; import org.eclipse.che.plugin.docker.client.dto.AuthConfig; import org.eclipse.che.plugin.docker.client.dto.AuthConfigs; +import org.eclipse.che.plugin.docker.client.exception.DockerException; import org.eclipse.che.plugin.docker.client.json.ContainerCommitted; import org.eclipse.che.plugin.docker.client.json.ContainerConfig; import org.eclipse.che.plugin.docker.client.json.ContainerCreated; diff --git a/plugins/plugin-docker/che-plugin-docker-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstanceProvider.java b/plugins/plugin-docker/che-plugin-docker-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstanceProvider.java index e792261271..6dade9e097 100644 --- a/plugins/plugin-docker/che-plugin-docker-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstanceProvider.java +++ b/plugins/plugin-docker/che-plugin-docker-machine/src/main/java/org/eclipse/che/plugin/docker/machine/DockerInstanceProvider.java @@ -28,6 +28,7 @@ import org.eclipse.che.api.core.util.SystemInfo; import org.eclipse.che.api.machine.server.exception.InvalidRecipeException; import org.eclipse.che.api.machine.server.exception.MachineException; import org.eclipse.che.api.machine.server.exception.SnapshotException; +import org.eclipse.che.api.machine.server.exception.SourceNotFoundException; import org.eclipse.che.api.machine.server.exception.UnsupportedRecipeException; import org.eclipse.che.api.machine.server.spi.Instance; import org.eclipse.che.api.machine.server.spi.InstanceProvider; @@ -38,11 +39,12 @@ import org.eclipse.che.commons.lang.IoUtil; import org.eclipse.che.plugin.docker.client.DockerConnector; import org.eclipse.che.plugin.docker.client.DockerConnectorConfiguration; import org.eclipse.che.plugin.docker.client.DockerFileException; -import org.eclipse.che.plugin.docker.client.UserSpecificDockerRegistryCredentialsProvider; import org.eclipse.che.plugin.docker.client.Dockerfile; import org.eclipse.che.plugin.docker.client.DockerfileParser; import org.eclipse.che.plugin.docker.client.ProgressLineFormatterImpl; import org.eclipse.che.plugin.docker.client.ProgressMonitor; +import org.eclipse.che.plugin.docker.client.UserSpecificDockerRegistryCredentialsProvider; +import org.eclipse.che.plugin.docker.client.exception.ImageNotFoundException; import org.eclipse.che.plugin.docker.client.json.ContainerConfig; import org.eclipse.che.plugin.docker.client.json.HostConfig; import org.eclipse.che.plugin.docker.client.params.PullParams; @@ -269,8 +271,12 @@ public class DockerInstanceProvider implements InstanceProvider { * if other error occurs */ @Override - public Instance createInstance(final Machine machine, final LineConsumer creationLogsOutput) - throws UnsupportedRecipeException, InvalidRecipeException, NotFoundException, MachineException { + public Instance createInstance(Machine machine, + LineConsumer creationLogsOutput) throws UnsupportedRecipeException, + InvalidRecipeException, + SourceNotFoundException, + NotFoundException, + MachineException { // based on machine source, do the right steps MachineConfig machineConfig = machine.getConfig(); @@ -314,8 +320,11 @@ public class DockerInstanceProvider implements InstanceProvider { creationLogsOutput); } - protected Instance createInstanceFromImage(final Machine machine, String machineContainerName, - final LineConsumer creationLogsOutput) throws NotFoundException, MachineException { + protected Instance createInstanceFromImage(Machine machine, + String machineContainerName, + LineConsumer creationLogsOutput) throws NotFoundException, + MachineException, + SourceNotFoundException { final DockerMachineSource dockerMachineSource = new DockerMachineSource(machine.getConfig().getSource()); if (snapshotUseRegistry) { @@ -327,8 +336,10 @@ public class DockerInstanceProvider implements InstanceProvider { try { // tag image with generated name to allow sysadmin recognize it docker.tag(TagParams.create(fullNameOfPulledImage, machineImageName)); - } catch (IOException e) { - LOG.error(e.getLocalizedMessage(), e); + } catch (ImageNotFoundException nfEx) { + throw new SourceNotFoundException(nfEx.getLocalizedMessage(), nfEx); + } catch (IOException ioEx) { + LOG.error(ioEx.getLocalizedMessage(), ioEx); throw new MachineException("Can't create machine from snapshot."); } try { diff --git a/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/MachineManager.java b/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/MachineManager.java index e96e4b21f9..399e6f97c6 100644 --- a/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/MachineManager.java +++ b/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/MachineManager.java @@ -33,10 +33,12 @@ import org.eclipse.che.api.machine.server.dao.SnapshotDao; import org.eclipse.che.api.machine.server.event.InstanceStateEvent; import org.eclipse.che.api.machine.server.exception.MachineException; import org.eclipse.che.api.machine.server.exception.SnapshotException; +import org.eclipse.che.api.machine.server.exception.SourceNotFoundException; import org.eclipse.che.api.machine.server.exception.UnsupportedRecipeException; import org.eclipse.che.api.machine.server.model.impl.LimitsImpl; import org.eclipse.che.api.machine.server.model.impl.MachineConfigImpl; import org.eclipse.che.api.machine.server.model.impl.MachineImpl; +import org.eclipse.che.api.machine.server.model.impl.MachineSourceImpl; import org.eclipse.che.api.machine.server.model.impl.SnapshotImpl; import org.eclipse.che.api.machine.server.spi.Instance; import org.eclipse.che.api.machine.server.spi.InstanceProcess; @@ -277,6 +279,7 @@ public class MachineManager { ConflictException, BadRequestException, MachineException { + final MachineSourceImpl sourceCopy = machineConfig.getSource(); final InstanceProvider instanceProvider = machineInstanceProviders.getProvider(machineConfig.getType()); // Backward compatibility for source type 'Recipe'. @@ -329,10 +332,25 @@ public class MachineManager { .getOutput()); try { - machineRegistry.addMachine(machine); - - instanceCreator.createInstance(instanceProvider, machine, machineLogger); - + try { + machineRegistry.addMachine(machine); + instanceCreator.createInstance(instanceProvider, machine, machineLogger); + } catch (MachineException ex) { + if (snapshot == null) { + throw ex; + } + if (ex.getCause() instanceof SourceNotFoundException) { + final LineConsumer logger = getMachineLogger(machineId, + getMachineChannels(machine.getConfig().getName(), + machine.getWorkspaceId(), + machine.getEnvName()).getOutput()); + LOG.error("Image of snapshot for machine " + machineConfig.getName() + " not found. " + + "Machine will be created from origin source"); + machine.getConfig().setSource(sourceCopy); + machineRegistry.addMachine(machine); + instanceCreator.createInstance(instanceProvider, machine, logger); + } + } return machine; } catch (ConflictException e) { try { diff --git a/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/exception/SourceNotFoundException.java b/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/exception/SourceNotFoundException.java new file mode 100644 index 0000000000..28dd08d647 --- /dev/null +++ b/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/exception/SourceNotFoundException.java @@ -0,0 +1,36 @@ +/******************************************************************************* + * Copyright (c) 2012-2016 Codenvy, S.A. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Codenvy, S.A. - initial API and implementation + *******************************************************************************/ +package org.eclipse.che.api.machine.server.exception; + +import org.eclipse.che.api.core.rest.shared.dto.ServiceError; + +/** + * Occurs when machine sources is not found or is not available. + * + * @author Anton Korneta + */ +public class SourceNotFoundException extends MachineException { + public SourceNotFoundException(String message) { + super(message); + } + + public SourceNotFoundException(ServiceError serviceError) { + super(serviceError); + } + + public SourceNotFoundException(Throwable cause) { + super(cause); + } + + public SourceNotFoundException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/spi/InstanceProvider.java b/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/spi/InstanceProvider.java index 5fea18368c..b30924846a 100644 --- a/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/spi/InstanceProvider.java +++ b/wsmaster/che-core-api-machine/src/main/java/org/eclipse/che/api/machine/server/spi/InstanceProvider.java @@ -18,6 +18,7 @@ import org.eclipse.che.api.core.util.LineConsumer; import org.eclipse.che.api.machine.server.exception.InvalidRecipeException; import org.eclipse.che.api.machine.server.exception.MachineException; import org.eclipse.che.api.machine.server.exception.SnapshotException; +import org.eclipse.che.api.machine.server.exception.SourceNotFoundException; import org.eclipse.che.api.machine.server.exception.UnsupportedRecipeException; import java.util.Set; @@ -65,6 +66,7 @@ public interface InstanceProvider { Instance createInstance(Machine machine, LineConsumer creationLogsOutput) throws UnsupportedRecipeException, InvalidRecipeException, + SourceNotFoundException, NotFoundException, MachineException; diff --git a/wsmaster/che-core-api-machine/src/test/java/org/eclipse/che/api/machine/server/MachineManagerTest.java b/wsmaster/che-core-api-machine/src/test/java/org/eclipse/che/api/machine/server/MachineManagerTest.java index 8f8e1397bb..4c8f94348a 100644 --- a/wsmaster/che-core-api-machine/src/test/java/org/eclipse/che/api/machine/server/MachineManagerTest.java +++ b/wsmaster/che-core-api-machine/src/test/java/org/eclipse/che/api/machine/server/MachineManagerTest.java @@ -22,11 +22,13 @@ import org.eclipse.che.api.core.notification.EventService; import org.eclipse.che.api.core.util.LineConsumer; import org.eclipse.che.api.machine.server.dao.SnapshotDao; import org.eclipse.che.api.machine.server.exception.MachineException; +import org.eclipse.che.api.machine.server.exception.SourceNotFoundException; import org.eclipse.che.api.machine.server.model.impl.LimitsImpl; import org.eclipse.che.api.machine.server.model.impl.MachineConfigImpl; import org.eclipse.che.api.machine.server.model.impl.MachineImpl; import org.eclipse.che.api.machine.server.model.impl.MachineSourceImpl; import org.eclipse.che.api.machine.server.model.impl.ServerConfImpl; +import org.eclipse.che.api.machine.server.model.impl.SnapshotImpl; import org.eclipse.che.api.machine.server.recipe.RecipeImpl; import org.eclipse.che.api.machine.server.spi.Instance; import org.eclipse.che.api.machine.server.spi.InstanceProcess; @@ -61,6 +63,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; /** @@ -71,11 +74,15 @@ import static org.testng.Assert.assertNotNull; */ @Listeners(MockitoTestNGListener.class) public class MachineManagerTest { + private static final int DEFAULT_MACHINE_MEMORY_SIZE_MB = 1000; private static final String WS_ID = "testWsId"; private static final String ENVIRONMENT_NAME = "testEnvName"; private static final String USER_ID = "userId"; private static final String MACHINE_ID = "machineId"; + private static final String NAMESPACE = "namespace"; + + private static final SubjectImpl CREATOR = new SubjectImpl("name", USER_ID, "token", false); @Mock private MachineInstanceProviders machineInstanceProviders; @@ -95,12 +102,13 @@ public class MachineManagerTest { private InstanceProcess instanceProcess; @Mock private LineConsumer processLogger; + @Mock + private SnapshotDao snapshotDao; private MachineManager manager; @BeforeMethod public void setUp() throws Exception { - final SnapshotDao snapshotDao = mock(SnapshotDao.class); final EventService eventService = mock(EventService.class); final String machineLogsDir = targetDir().resolve("logs-dir").toString(); IoUtil.deleteRecursive(new File(machineLogsDir)); @@ -113,7 +121,7 @@ public class MachineManagerTest { wsAgentLauncher)); EnvironmentContext envCont = new EnvironmentContext(); - envCont.setSubject(new SubjectImpl(null, USER_ID, null, false)); + envCont.setSubject(CREATOR); EnvironmentContext.setCurrent(envCont); RecipeImpl recipe = new RecipeImpl().withScript("script").withType("Dockerfile"); @@ -123,6 +131,7 @@ public class MachineManagerTest { when(machineInstanceProviders.getProvider(anyString())).thenReturn(instanceProvider); HashSet recipeTypes = new HashSet<>(); recipeTypes.add("test type 1"); + recipeTypes.add("snapshot"); recipeTypes.add("dockerfile"); when(instanceProvider.getRecipeTypes()).thenReturn(recipeTypes); when(instanceProvider.createInstance(any(Machine.class), any(LineConsumer.class))).thenReturn(instance); @@ -259,6 +268,40 @@ public class MachineManagerTest { verify(machineLogger).close(); } + @Test + public void shouldCreateMachineFromOriginalSourceWhenMachineRecoverFails() throws Exception { + final SnapshotImpl snapshot = createSnapshot(); + final MachineConfigImpl config = createMachineConfig(); + when(manager.generateMachineId()).thenReturn(MACHINE_ID); + final MachineImpl machine = new MachineImpl(config, + MACHINE_ID, + WS_ID, + ENVIRONMENT_NAME, + CREATOR.getUserId(), + MachineStatus.CREATING, + null); + machine.getConfig().setSource(snapshot.getMachineSource()); + when(snapshotDao.getSnapshot(WS_ID, ENVIRONMENT_NAME, config.getName())).thenReturn(snapshot); + when(instanceProvider.createInstance(eq(machine), any(LineConsumer.class))).thenThrow(new SourceNotFoundException("")); + when(machineRegistry.getMachine(MACHINE_ID)).thenReturn(machine); + + final MachineImpl result = manager.recoverMachine(config, WS_ID, ENVIRONMENT_NAME); + + machine.getConfig().setSource(config.getSource()); + assertEquals(result, machine); + } + + @Test(expectedExceptions = MachineException.class) + public void shouldThrowExceptionWhenMachineCreationFromOriginSourceFailed() throws Exception { + final MachineConfigImpl config = createMachineConfig(); + when(manager.generateMachineId()).thenReturn(MACHINE_ID); + + when(instanceProvider.createInstance(any(MachineImpl.class), + any(LineConsumer.class))).thenThrow(new MachineException("")); + + manager.recoverMachine(config, WS_ID, ENVIRONMENT_NAME); + } + private void waitForExecutorIsCompletedTask() throws Exception { for (int i = 0; ((ThreadPoolExecutor)manager.executor).getCompletedTaskCount() == 0 && i < 10; i++) { Thread.sleep(300); @@ -287,4 +330,16 @@ public class MachineManagerTest { "/some/path")), Collections.singletonMap("key1", "value1")); } + + private SnapshotImpl createSnapshot() { + return SnapshotImpl.builder() + .setId("snapshot12") + .fromConfig(createMachineConfig()) + .setWorkspaceId(WS_ID) + .setEnvName(ENVIRONMENT_NAME) + .setNamespace(NAMESPACE) + .setMachineSource(new MachineSourceImpl("snapshot").setLocation("location")) + .setDev(true) + .build(); + } }