Skip to content

Commit

Permalink
CHE-4820: fix environment cleanup on env start error
Browse files Browse the repository at this point in the history 
Signed-off-by: Alexander Garagatyi <agaragatyi@codenvy.com>
  • Loading branch information
Alexander Garagatyi committed Apr 24, 2017
1 parent 43ac679 commit 7457950
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 4 deletions.
6 changes: 3 additions & 3 deletions ...-workspace/src/main/java/org/eclipse/che/api/environment/server/CheEnvironmentEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ private void startEnvironmentQueue(String namespace,
try (@SuppressWarnings("unused") Unlocker u = stripedLocks.readLock(workspaceId)) {
EnvironmentHolder environmentHolder = environments.get(workspaceId);
if (environmentHolder == null) {
throw new ServerException("Environment start is interrupted.");
throw new EnvironmentStartInterruptedException(workspaceId, envName);
}
service = environmentHolder.environment.getServices().get(machineName);
extendedMachine = environmentHolder.environmentConfig.getMachines().get(machineName);
Expand Down Expand Up @@ -868,7 +868,7 @@ private void startEnvironmentQueue(String namespace,

machineName = queuePeekOrFail(workspaceId);
}
} catch (RuntimeException | ServerException | EnvironmentStartInterruptedException e) {
} catch (Exception e) {
boolean interrupted = Thread.interrupted();
EnvironmentHolder env;
try (@SuppressWarnings("unused") Unlocker u = stripedLocks.writeLock(workspaceId)) {
Expand All @@ -886,7 +886,7 @@ private void startEnvironmentQueue(String namespace,
}
try {
throw e;
} catch (ServerException | EnvironmentStartInterruptedException rethrow) {
} catch (ServerException | EnvironmentException | AgentException rethrow) {
throw rethrow;
} catch (Exception wrap) {
throw new ServerException(wrap.getMessage(), wrap);
Expand Down
188 changes: 187 additions & 1 deletion ...kspace/src/test/java/org/eclipse/che/api/environment/server/CheEnvironmentEngineTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package org.eclipse.che.api.environment.server;

import org.eclipse.che.api.agent.server.AgentRegistry;
import org.eclipse.che.api.agent.server.exception.AgentException;
import org.eclipse.che.api.agent.shared.model.Agent;
import org.eclipse.che.api.agent.shared.model.AgentKey;
import org.eclipse.che.api.core.ConflictException;
Expand Down Expand Up @@ -82,6 +83,7 @@
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.inOrder;
Expand Down Expand Up @@ -1287,6 +1289,189 @@ public void shouldDestroyAndRemoveMachineFromEnvironmentIfEventAboutItsOOM() thr
}
}

@Test
public void shouldBeAbleToRestartEnvironmentAfterServerException() throws Exception {
// given
EnvironmentImpl env = createEnv();
String envName = "env-1";
String workspaceId = "wsId";
String testServerExcMessage = "test exception";
when(machineProvider.startService(anyString(),
eq(workspaceId),
eq(envName),
anyString(),
anyBoolean(),
anyString(),
any(CheServiceImpl.class),
any(LineConsumer.class)))
.thenThrow(new ServerException(testServerExcMessage));
when(environmentParser.parse(env)).thenReturn(createCheServicesEnv());

// when
// env start must fails because of server exception
try {
engine.start(workspaceId,
envName,
env,
false,
messageConsumer,
startedHandler);
fail("Server exception should be thrown");
} catch (ServerException e) {
assertEquals(e.getMessage(), testServerExcMessage);
}

when(machineProvider.startService(anyString(),
eq(workspaceId),
eq(envName),
anyString(),
anyBoolean(),
anyString(),
any(CheServiceImpl.class),
any(LineConsumer.class)))
.thenAnswer(invocationOnMock -> {
Object[] arguments = invocationOnMock.getArguments();
String machineName = (String)arguments[3];
boolean isDev = (boolean)arguments[4];
CheServiceImpl service = (CheServiceImpl)arguments[6];
Machine machine = createMachine(workspaceId,
envName,
service,
machineName,
isDev);
return new NoOpMachineInstance(machine);
});

// then
// env start succeeds
engine.start(workspaceId,
envName,
env,
false,
messageConsumer,
startedHandler);
}

@Test
public void shouldBeAbleToRestartEnvironmentAfterRuntimeException() throws Exception {
// given
EnvironmentImpl env = createEnv();
String envName = "env-1";
String workspaceId = "wsId";
String testEnvExcMessage = "test exception";
when(machineProvider.startService(anyString(),
eq(workspaceId),
eq(envName),
anyString(),
anyBoolean(),
anyString(),
any(CheServiceImpl.class),
any(LineConsumer.class)))
.thenThrow(new RuntimeException(testEnvExcMessage));
when(environmentParser.parse(env)).thenReturn(createCheServicesEnv());

// when
// env start must fails because of exception
try {
engine.start(workspaceId,
envName,
env,
false,
messageConsumer,
startedHandler);
fail("Environment exception should be thrown");
} catch (ServerException e) {
assertEquals(e.getMessage(), testEnvExcMessage);
}

when(machineProvider.startService(anyString(),
eq(workspaceId),
eq(envName),
anyString(),
anyBoolean(),
anyString(),
any(CheServiceImpl.class),
any(LineConsumer.class)))
.thenAnswer(invocationOnMock -> {
Object[] arguments = invocationOnMock.getArguments();
String machineName = (String)arguments[3];
boolean isDev = (boolean)arguments[4];
CheServiceImpl service = (CheServiceImpl)arguments[6];
Machine machine = createMachine(workspaceId,
envName,
service,
machineName,
isDev);
return new NoOpMachineInstance(machine);
});

// then
// env start succeeds
engine.start(workspaceId,
envName,
env,
false,
messageConsumer,
startedHandler);
}

@Test
public void shouldBeAbleToRestartEnvironmentAfterAgentException() throws Exception {
// given
EnvironmentImpl env = createEnv();
String envName = "env-1";
String workspaceId = "wsId";
when(machineProvider.startService(anyString(),
eq(workspaceId),
eq(envName),
anyString(),
anyBoolean(),
anyString(),
any(CheServiceImpl.class),
any(LineConsumer.class)))
.thenAnswer(invocationOnMock -> {
Object[] arguments = invocationOnMock.getArguments();
String machineName = (String)arguments[3];
boolean isDev = (boolean)arguments[4];
CheServiceImpl service = (CheServiceImpl)arguments[6];
Machine machine = createMachine(workspaceId,
envName,
service,
machineName,
isDev);
return new NoOpMachineInstance(machine);
});
when(environmentParser.parse(env)).thenReturn(createCheServicesEnv());
String testAgentExcMessage = "test agent exception";
doThrow(new AgentException(testAgentExcMessage)).when(startedHandler)
.started(any(Instance.class), any(ExtendedMachine.class));

// when
// env start must fails because of agent exception
try {
engine.start(workspaceId,
envName,
env,
false,
messageConsumer,
startedHandler);
fail("Agent exception should be thrown");
} catch (AgentException e) {
assertEquals(e.getMessage(), testAgentExcMessage);
}

doNothing().when(startedHandler).started(any(Instance.class), any(ExtendedMachine.class));

// then
// env start succeeds
engine.start(workspaceId,
envName,
env,
false,
messageConsumer,
startedHandler);
}

private List<Instance> startEnv() throws Exception {
EnvironmentImpl env = createEnv();
CheServicesEnvironmentImpl cheServicesEnv = createCheServicesEnv();
Expand Down Expand Up @@ -1372,7 +1557,8 @@ private CheServicesEnvironmentImpl createCheServicesEnv() {
CheServicesEnvironmentImpl cheServicesEnvironment = new CheServicesEnvironmentImpl();
Map<String, CheServiceImpl> services = new HashMap<>();

services.put("dev-machine", new CheServiceImpl().withBuild(new CheServiceBuildContextImpl().withContext("image")));
services.put("dev-machine",
new CheServiceImpl().withBuild(new CheServiceBuildContextImpl().withContext("image")));
services.put("machine2", new CheServiceImpl().withBuild(new CheServiceBuildContextImpl().withContext("image")));

cheServicesEnvironment.setServices(services);
Expand Down

0 comments on commit 7457950

Please sign in to comment.