Skip to content

Commit

Permalink
#2642 fixed check to consider removed containers during modification
Browse files Browse the repository at this point in the history
  • Loading branch information
Dilshat Aliev committed Sep 11, 2018
1 parent 4078fc7 commit f0bfb85
Show file tree
Hide file tree
Showing 15 changed files with 139 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@


import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -75,7 +74,7 @@ Set<EnvironmentContainerHost> growEnvironment( final String environmentId, final
* new quota
* @param async true - env will be created in background, false - caller will block
*/
EnvironmentCreationRef modifyEnvironment( String environmentId, Topology topology, List<String> removedContainers,
EnvironmentCreationRef modifyEnvironment( String environmentId, Topology topology, Set<String> removedContainers,
Map<String, ContainerQuota> changedContainers, boolean async )
throws EnvironmentModificationException, EnvironmentNotFoundException;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ public Set<EnvironmentContainerHost> growEnvironment( final String environmentId

@Override
public EnvironmentCreationRef modifyEnvironment( final String environmentId, final Topology topology,
final List<String> removedContainers,
final Set<String> removedContainers,
final Map<String, ContainerQuota> changedContainers,
final boolean async )
throws EnvironmentModificationException, EnvironmentNotFoundException
Expand Down Expand Up @@ -633,7 +633,7 @@ public EnvironmentCreationRef modifyEnvironment( final String environmentId, fin
{
try
{
if ( !peer.canAccommodate( new Nodes( newNodes, changedQuotas ) ) )
if ( !peer.canAccommodate( new Nodes( newNodes, removedContainers, changedQuotas ) ) )
{
operationTracker.addLogFailed(
String.format( "Peer %s can not accommodate the requested containers",
Expand Down Expand Up @@ -716,33 +716,64 @@ public void run()
}


private Map<String, ContainerQuota> getPeerRemovedQuotas( final String peerId, final List<String> removedNodes,
final LocalEnvironment environment )
throws EnvironmentModificationException
{

if ( CollectionUtil.isCollectionEmpty( removedNodes ) )
{
return Maps.newHashMap();
}

try
{
Map<String, ContainerQuota> peerRemovedNodes = Maps.newHashMap();

for ( String containerId : removedNodes )
{
final ContainerHost containerHost = environment.getContainerHostById( containerId );

if ( Objects.equals( containerHost.getPeerId(), peerId ) )
{
peerRemovedNodes.put( containerHost.getResourceHostId().getId(), containerHost.getQuota() );
}
}

return peerRemovedNodes;
}
catch ( Exception e )
{
throw new EnvironmentModificationException( e );
}
}


private Map<String, ContainerQuota> getPeerChangedContainers( final String peerId,
final Map<String, ContainerQuota> allChangedContainers,
final LocalEnvironment environment )
throws EnvironmentModificationException
{
if ( allChangedContainers == null )
{
return Maps.newHashMap();
}

try
{
if ( allChangedContainers == null )
{
return Maps.newHashMap();
}
else
{
Map<String, ContainerQuota> peerChangedContainers = Maps.newHashMap();
Map<String, ContainerQuota> peerChangedContainers = Maps.newHashMap();

for ( Map.Entry<String, ContainerQuota> entry : allChangedContainers.entrySet() )
for ( Map.Entry<String, ContainerQuota> entry : allChangedContainers.entrySet() )
{
String containerId = entry.getKey();
EnvironmentContainerHost containerHost = environment.getContainerHostById( containerId );
if ( Objects.equals( containerHost.getPeerId(), peerId ) )
{
String containerId = entry.getKey();
EnvironmentContainerHost containerHost = environment.getContainerHostById( containerId );
if ( Objects.equals( containerHost.getPeerId(), peerId ) )
{
peerChangedContainers.put( containerId, entry.getValue() );
}
peerChangedContainers.put( containerId, entry.getValue() );
}

return peerChangedContainers;
}

return peerChangedContainers;
}
catch ( Exception e )
{
Expand Down Expand Up @@ -1642,7 +1673,7 @@ protected EnvironmentCreationWorkflow getEnvironmentCreationWorkflow( final Loca
protected EnvironmentModifyWorkflow getEnvironmentModifyingWorkflow( final LocalEnvironment environment,
final Topology topology,
final TrackerOperation operationTracker,
final List<String> removedContainers,
final Set<String> removedContainers,
final Map<String, ContainerQuota> changedContainers )

{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public EnvironmentCreationRef createEnvironment( final Topology topology, final
@Override
@RolesAllowed( "Environment-Management|Write" )
public EnvironmentCreationRef modifyEnvironment( final String environmentId, final Topology topology,
final List<String> removedContainers,
final Set<String> removedContainers,
final Map<String, ContainerQuota> changedContainers,
final boolean async )
throws EnvironmentModificationException, EnvironmentNotFoundException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@


import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import io.subutai.common.environment.EnvironmentStatus;
import io.subutai.common.environment.Topology;
Expand Down Expand Up @@ -34,7 +34,7 @@ public class EnvironmentModifyWorkflow extends CancellableWorkflow<EnvironmentMo
private final PeerManager peerManager;
private LocalEnvironment environment;
private final Topology topology;
private List<String> removedContainers;
private Set<String> removedContainers;
private Map<String, ContainerQuota> changedContainers;
private final String defaultDomain;
private final TrackerOperation operationTracker;
Expand All @@ -59,7 +59,7 @@ public enum EnvironmentGrowingPhase

public EnvironmentModifyWorkflow( String defaultDomain, IdentityManager identityManager, PeerManager peerManager,
SecurityManager securityManager, LocalEnvironment environment, Topology topology,
List<String> removedContainers, Map<String, ContainerQuota> changedContainers,
Set<String> removedContainers, Map<String, ContainerQuota> changedContainers,
TrackerOperation operationTracker, EnvironmentManagerImpl environmentManager )
{

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.subutai.core.environment.impl.workflow.modification.steps;


import java.util.List;
import java.util.Set;

import io.subutai.common.environment.ContainerHostNotFoundException;
import io.subutai.common.environment.Environment;
Expand All @@ -22,13 +22,13 @@ public class DestroyContainersStep
{
private final LocalEnvironment environment;
private final EnvironmentManagerImpl environmentManager;
private final List<String> removedContainers;
private final Set<String> removedContainers;
private final TrackerOperation trackerOperation;
protected TaskUtil<Object> destroyUtil = new TaskUtil<>();


public DestroyContainersStep( final LocalEnvironment environment, final EnvironmentManagerImpl environmentManager,
final List<String> removedContainers, TrackerOperation trackerOperation )
final Set<String> removedContainers, TrackerOperation trackerOperation )
{
this.environment = environment;
this.environmentManager = environmentManager;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package io.subutai.core.environment.impl;


import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
Expand Down Expand Up @@ -98,9 +97,9 @@
import static junit.framework.TestCase.fail;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyList;
import static org.mockito.Matchers.anyLong;
import static org.mockito.Matchers.anyMap;
import static org.mockito.Matchers.anySet;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Matchers.isA;
Expand Down Expand Up @@ -449,20 +448,20 @@ public void testGrowEnvironment() throws Exception
{

doReturn( environmentCreationRef ).when( environmentManager )
.modifyEnvironment( eq( TestHelper.ENV_ID ), eq( topology ), anyList(),
.modifyEnvironment( eq( TestHelper.ENV_ID ), eq( topology ), anySet(),
anyMap(), anyBoolean() );

environmentManager.growEnvironment( TestHelper.ENV_ID, topology, false );

verify( environmentManager )
.modifyEnvironment( eq( TestHelper.ENV_ID ), eq( topology ), anyList(), anyMap(), anyBoolean() );
.modifyEnvironment( eq( TestHelper.ENV_ID ), eq( topology ), anySet(), anyMap(), anyBoolean() );
}


@Test
public void testModifyEnvironment() throws Exception
{
List<String> removedContainers = Lists.newArrayList( TestHelper.CONTAINER_ID );
Set<String> removedContainers = Sets.newHashSet( TestHelper.CONTAINER_ID );
Map<String, ContainerQuota> changedContainers = Maps.newHashMap();
changedContainers.put( TestHelper.CONTAINER_ID, new ContainerQuota( ContainerSize.LARGE ) );
EnvironmentModifyWorkflow environmentModifyWorkflow = mock( EnvironmentModifyWorkflow.class );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;

import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;

Expand Down Expand Up @@ -245,11 +244,11 @@ public void testModifyEnvironment() throws Exception
Map<String, ContainerQuota> changedContainers = Maps.newHashMap();
changedContainers.put( TestHelper.CONTAINER_ID, new ContainerQuota( ContainerSize.LARGE ) );

proxy.modifyEnvironment( TestHelper.ENV_ID, topology, Lists.newArrayList( TestHelper.CONTAINER_ID ),
proxy.modifyEnvironment( TestHelper.ENV_ID, topology, Sets.newHashSet( TestHelper.CONTAINER_ID ),
changedContainers, true );

verify( environmentManager )
.modifyEnvironment( TestHelper.ENV_ID, topology, Lists.newArrayList( TestHelper.CONTAINER_ID ),
.modifyEnvironment( TestHelper.ENV_ID, topology, Sets.newHashSet( TestHelper.CONTAINER_ID ),
changedContainers, true );
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
package io.subutai.core.environment.impl.workflow.modification;


import java.util.List;
import java.util.Map;
import java.util.Set;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;

import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;

import io.subutai.common.environment.Topology;
import io.subutai.common.peer.Peer;
Expand Down Expand Up @@ -46,7 +46,7 @@ class EnvironmentModifyWorkflowSUT extends EnvironmentModifyWorkflow
public EnvironmentModifyWorkflowSUT( final String defaultDomain, final IdentityManager identityManager,
final PeerManager peerManager, final SecurityManager securityManager,
final LocalEnvironment environment, final Topology topology,
final List<String> removedContainers,
final Set<String> removedContainers,
final Map<String, ContainerQuota> changedContainers,
final TrackerOperation operationTracker,
final EnvironmentManagerImpl environmentManager )
Expand Down Expand Up @@ -94,8 +94,8 @@ public void setUp() throws Exception
changedContainers.put( TestHelper.CONTAINER_ID, new ContainerQuota( ContainerSize.LARGE ) );

workflow = new EnvironmentModifyWorkflowSUT( Common.DEFAULT_DOMAIN_NAME, identityManager, peerManager,
securityManager, environment, topology, Lists.newArrayList( TestHelper.CONTAINER_ID ),
changedContainers, trackerOperation, environmentManager );
securityManager, environment, topology, Sets.newHashSet( TestHelper.CONTAINER_ID ), changedContainers,
trackerOperation, environmentManager );

doReturn( environment ).when( environmentManager ).update( environment );
doReturn( environmentContainer ).when( environment ).getContainerHostById( TestHelper.CONTAINER_ID );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;

import com.google.common.collect.Lists;
import com.google.common.collect.Sets;

import io.subutai.common.security.relation.RelationManager;
import io.subutai.common.tracker.TrackerOperation;
Expand Down Expand Up @@ -49,8 +49,8 @@ public class DestroyContainersStepTest
@Before
public void setUp() throws Exception
{
step = new DestroyContainersStep( environment, environmentManager,
Lists.newArrayList( TestHelper.CONTAINER_ID ), trackerOperation );
step = new DestroyContainersStep( environment, environmentManager, Sets.newHashSet( TestHelper.CONTAINER_ID ),
trackerOperation );
step.destroyUtil = taskUtil;
TestHelper.bind( taskUtil, taskResults, taskResult );
doReturn( task ).when( taskResult ).getTask();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ public Response modify( final String environmentId, final String topologyJson, f
}


List<String> removedContainersList = JsonUtil.fromJson( removedContainers, new TypeToken<List<String>>()
Set<String> removedContainersList = JsonUtil.fromJson( removedContainers, new TypeToken<Set<String>>()
{
}.getType() );

Expand Down Expand Up @@ -418,7 +418,7 @@ public Response modifyAdvanced( final String environmentId, final String topolog

topology.addAllNodePlacement( nodes );

List<String> containersToRemove = JsonUtil.fromJson( removedContainers, new TypeToken<List<String>>()
Set<String> containersToRemove = JsonUtil.fromJson( removedContainers, new TypeToken<Set<String>>()
{
}.getType() );

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.subutai.core.hubmanager.impl.environment.state.change;


import com.google.common.collect.Lists;
import com.google.common.collect.Sets;

import io.subutai.common.environment.Environment;
import io.subutai.common.peer.ContainerId;
Expand Down Expand Up @@ -76,7 +76,7 @@ else if ( nodeDto.getState().equals( ContainerStateDto.ABORTING ) )
if ( envDto.isCreatedOnSS() )
{
ctx.envManager.modifyEnvironment( nodeDto.getEnvironmentId(), null,
Lists.newArrayList( nodeDto.getContainerId() ), null, false );
Sets.newHashSet( nodeDto.getContainerId() ), null, false );
}
else
{
Expand Down Expand Up @@ -117,7 +117,7 @@ else if ( nodeDto.getState().equals( ContainerStateDto.ABORTING ) )
if ( envDto.isCreatedOnSS() )
{
ctx.envManager.modifyEnvironment( nodeDto.getEnvironmentId(), null,
Lists.newArrayList( nodeDto.getContainerId() ), null, false );
Sets.newHashSet( nodeDto.getContainerId() ), null, false );
}

nodeDto.setState( ContainerStateDto.FROZEN );
Expand Down
Loading

0 comments on commit f0bfb85

Please sign in to comment.