Skip to content

Commit

Permalink
VBV-2234 - [Running unit tests locally without retry]
Browse files Browse the repository at this point in the history 
ContainerRemovalTaskServiceTest.testRemovingOfCompositeDescriptionAndContainerRemovals

The problem was that we had a couple of operations executing in parallel
when performing remove of a container: remove container state, update
placement, release ports...
During this sequence it was possible to have the case where a delete and
get to the container description are sent at almost the same time. In
this case we hit an issue in xenon and the description is not deleted
even when the delete request is successful. For more information about
the xenon issue check: https://review.ec.eng.vmware.com/#/c/17003/. The
issue is still reproducible with the new xenon
- Changed the removal task first to perform placement and port
operations and after that to delete the container state and the
description
- Checked that the case is fixed by running the test 100 times in a loop
with no failures

Change-Id: I3b420a58077ba7ff5f0aa07fddb043b88f6fe2b4
Reviewed-on: https://bellevue-ci.eng.vmware.com:8080/44425
Closures-Verified: jenkins <[email protected]>
Upgrade-Verified: jenkins <[email protected]>
Bellevue-Verified: jenkins <[email protected]>
PG-Verified: jenkins <[email protected]>
CS-Verified: jenkins <[email protected]>
Reviewed-by: Lazarin Lazarov <[email protected]>
  • Loading branch information
@gmuleshkov
gmuleshkov committed Sep 28, 2018
1 parent 8dc8e77 commit eaf8892
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 77 deletions.
128 changes: 51 additions & 77 deletions request/src/main/java/com/vmware/admiral/request/ContainerRemovalTaskService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import com.vmware.admiral.common.util.QueryUtil;
import com.vmware.admiral.common.util.ServiceDocumentQuery;
import com.vmware.admiral.common.util.ServiceUtils;
import com.vmware.admiral.compute.container.ContainerDescriptionService.ContainerDescription;
import com.vmware.admiral.compute.container.ContainerHostDataCollectionService;
import com.vmware.admiral.compute.container.ContainerHostDataCollectionService.ContainerHostDataCollectionState;
import com.vmware.admiral.compute.container.ContainerService.ContainerState;
Expand Down Expand Up @@ -411,14 +410,12 @@ private void doDeleteResource(ContainerRemovalTaskState state, String subTaskLin
skipDeleteDescriptionOperationException,
skipHostPortProfileNotFoundException);

Operation delContainerOpr = deleteContainer(cs);
Operation placementOpr = releaseResourcePlacement(state, cs, subTaskLink);
Operation releasePortsOpr = releasePorts(cs,
skipHostPortProfileNotFoundException);

// list of operations to execute to release container resources
List<Operation> operations = new ArrayList<>();
operations.add(delContainerOpr);
// add placementOpr only when needed
if (placementOpr != null) {
operations.add(placementOpr);
Expand All @@ -429,103 +426,80 @@ private void doDeleteResource(ContainerRemovalTaskState state, String subTaskLin
if (releasePortsOpr != null) {
operations.add(releasePortsOpr);
}
// delete container description when deleting all its containers
if (state.resourceLinks.containsAll(resourcesSharingDesc)
&& (state.customProperties != null && !state.customProperties
.containsKey(CONTAINER_REDEPLOYMENT_CUSTOM_PROP))) {
// there could be a race condition when containers are in cluster and
// the same description tries to be deleted multiple times, that's why
// we need to skipOperationException is the description is NOT FOUND
Operation deleteContainerDescription = deleteContainerDescription(cs,
skipDeleteDescriptionOperationException);
operations.add(deleteContainerDescription);
}

OperationJoin.create(operations).setCompletion((ops, exs) -> {
// remove skipped exceptions
if (exs != null) {
skipOperationExceptions
.stream()
.filter(p -> p.get() != 0)
.forEach(p -> exs.remove(p.get()));
}
// fail the task is there are exceptions in the children operations
if (exs != null && !exs.isEmpty()) {
failTask("Failed deleting container resources: "
+ Utils.toString(exs), null);
return;
}
if (operations.size() > 0) {
OperationJoin.create(operations).setCompletion((ops, exs) -> {
// remove skipped exceptions
if (exs != null) {
skipOperationExceptions
.stream()
.filter(p -> p.get() != 0)
.forEach(p -> exs.remove(p.get()));
}
// fail the task is there are exceptions in the children operations
if (exs != null && !exs.isEmpty()) {
failTask("Failed deleting container resources: "
+ Utils.toString(exs), null);
return;
}
deleteContainer(cs, subTaskLink, state, resourcesSharingDesc)
.sendWith(this);
}).sendWith(this);
} else {
deleteContainer(cs, subTaskLink, state, resourcesSharingDesc)
.sendWith(this);
}

// complete the counter task after all remove operations finished
// successfully
completeSubTasksCounter(subTaskLink, null);
}).sendWith(this);
}
});
}

private Operation deleteContainer(ContainerState cs) {
private Operation deleteContainer(ContainerState cs, String subTaskLink,
ContainerRemovalTaskState state, List<String> resourcesSharingDesc) {
return Operation
.createDelete(this, cs.documentSelfLink)
.setBody(new ServiceDocument())
.setCompletion((op, ex) -> {
if (ex != null) {
logWarning("Failed deleting ContainerState: %s. Error: %s",
cs.documentSelfLink, Utils.toString(ex));
failTask("Failed deleting container resources: "
+ Utils.toString(ex), null);
return;
} else {
logInfo("Deleted ContainerState: %s", cs.documentSelfLink);
// When removing container state, remove also if there are any logs created.
// This is workaround for:
// https://www.pivotaltracker.com/n/projects/1471320/stories/143794415
sendRequest(Operation.createDelete(this, UriUtils.buildUriPath(
LogService.FACTORY_LINK, Service.getId(cs.documentSelfLink))));
if (state.resourceLinks.containsAll(resourcesSharingDesc)
&& (state.customProperties != null && !state.customProperties
.containsKey(CONTAINER_REDEPLOYMENT_CUSTOM_PROP))) {
deleteContainerDescription(cs, subTaskLink).sendWith(this);
} else {
completeSubTasksCounter(subTaskLink, null);
}
}
logInfo("Deleted ContainerState: %s", cs.documentSelfLink);
// When removing container state, remove also if there are any logs created.
// This is workaround for:
// https://www.pivotaltracker.com/n/projects/1471320/stories/143794415
sendRequest(Operation.createDelete(this, UriUtils.buildUriPath(
LogService.FACTORY_LINK, Service.getId(cs.documentSelfLink))));
});
}

private Operation deleteContainerDescription(ContainerState cs,
AtomicLong skipOperationException) {
private Operation deleteContainerDescription(ContainerState cs, String subTaskLink) {

Operation deleteContainerDesc = Operation
.createGet(this, cs.descriptionLink)
.setCompletion((o, e) -> {
if (o.getStatusCode() == Operation.STATUS_CODE_NOT_FOUND ||
e instanceof CancellationException) {
logFine("Resource [%s] not found, it will not be removed!",
cs.descriptionLink);
skipOperationException.set(o.getId());
return;
}

if (e != null) {
logWarning("Failed retrieving ContainerDescription: %s. Error: %s",
cs.descriptionLink, Utils.toString(e));
return;
}

ContainerDescription cd = o.getBody(ContainerDescription.class);

if (cd.parentDescriptionLink == null) {
logFine("Resource [%s] will not be removed because it doesn't contain"
+ " parentDescriptionLink!",
o.getBody(ContainerDescription.class).documentSelfLink);
.createDelete(this, cs.descriptionLink)
.setBody(new ServiceDocument())
.setCompletion((op, ex) -> {
if (ex != null) {
logWarning("Failed deleting ContainerDescription: %s. Error: %s",
cs.descriptionLink, Utils.toString(ex));
failTask("Failed deleting container resources: "
+ Utils.toString(ex), null);
return;
}

sendRequest(Operation
.createDelete(this, cd.documentSelfLink)
.setBody(new ServiceDocument())
.setCompletion((op, ex) -> {
if (ex != null) {
logWarning("Failed deleting ContainerDescription: %s."
+ " Error: %s",
cd.documentSelfLink, Utils.toString(ex));
return;
}
logInfo("Deleted ContainerDescription: %s", cd.documentSelfLink);
}));
logInfo("Deleted ContainerDescription: %s", cs.descriptionLink);
completeSubTasksCounter(subTaskLink, null);
});

return deleteContainerDesc;
}

Expand Down
112 changes: 112 additions & 0 deletions request/src/test/java/com/vmware/admiral/request/ContainerRemovalTaskServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@
import com.vmware.admiral.compute.container.ContainerFactoryService;
import com.vmware.admiral.compute.container.ContainerService.ContainerState;
import com.vmware.admiral.compute.container.ContainerService.ContainerState.PowerState;
import com.vmware.admiral.compute.container.ContainerStatsServiceTest.MockAdapterService;
import com.vmware.admiral.compute.container.GroupResourcePlacementService;
import com.vmware.admiral.compute.container.GroupResourcePlacementService.GroupResourcePlacementState;
import com.vmware.admiral.compute.container.HealthChecker.HealthConfig;
import com.vmware.admiral.compute.container.HealthChecker.HealthConfig.RequestProtocol;
import com.vmware.admiral.compute.container.HostPortProfileService;
import com.vmware.admiral.compute.container.SystemContainerDescriptions;
import com.vmware.admiral.request.ContainerAllocationTaskService.ContainerAllocationTaskState;
Expand Down Expand Up @@ -321,6 +324,63 @@ public void testSystemContainerRemoveOperation() throws Throwable {
assertTrue("System container should not be removed.", !containerStateLinks.isEmpty());
}

@Test
public void testContainerRemoveOperationAdapterNotAvailable() throws Throwable {
ContainerState container = TestRequestStateFactory.createContainer();
container.id = "test";
container.descriptionLink = containerDesc.documentSelfLink;
container.adapterManagementReference = containerDesc.instanceAdapterReference;
container.groupResourcePlacementLink = groupPlacementState.documentSelfLink;
container = doPost(container, ContainerFactoryService.SELF_LINK);

List<String> containerStateLinks = new ArrayList<>(1);
containerStateLinks.add(container.documentSelfLink);

RequestBrokerState removalRequest = new RequestBrokerState();
removalRequest.documentSelfLink = extractId(container.documentSelfLink) + "-removal";
removalRequest.resourceType = request.resourceType;
removalRequest.resourceLinks = new HashSet<>(containerStateLinks);
removalRequest.operation = ContainerOperationType.DELETE.id;

// stop the adapter service
// verify the container have not been removed:
MockAdapterService mockAdapterService = new MockAdapterService();
mockAdapterService.setSelfLink(MockAdapterService.SELF_LINK);
stopService(mockAdapterService);
removalRequest = startRequest(removalRequest);
waitForRequestToFail(removalRequest);
containerStateLinks = findResourceLinks(ContainerState.class, containerStateLinks);
assertTrue("Container should not be removed.", !containerStateLinks.isEmpty());
}

@Test
public void testSystemContainerRemoveOperationWithId() throws Throwable {
ContainerState container = TestRequestStateFactory.createContainer();
container.id = "systemContainer";
container.descriptionLink = containerDesc.documentSelfLink;
container.adapterManagementReference = containerDesc.instanceAdapterReference;
container.groupResourcePlacementLink = groupPlacementState.documentSelfLink;
container.system = Boolean.TRUE;
container = doPost(container, ContainerFactoryService.SELF_LINK);

// try to delete system container
List<String> containerStateLinks = new ArrayList<>(1);
containerStateLinks.add(container.documentSelfLink);

RequestBrokerState removalRequest = new RequestBrokerState();
removalRequest.documentSelfLink = extractId(container.documentSelfLink) + "-removal";
removalRequest.resourceType = request.resourceType;
removalRequest.resourceLinks = new HashSet<>(containerStateLinks);
removalRequest.operation = ContainerOperationType.DELETE.id;

removalRequest = startRequest(removalRequest);
waitForRequestToComplete(removalRequest);

// verify the system container have not been removed:
containerStateLinks = findResourceLinks(ContainerState.class, containerStateLinks);
assertTrue("System container should not be removed.", !containerStateLinks.isEmpty());
}

@Test
public void testAgentContainerRemoveOperation() throws Throwable {

Expand Down Expand Up @@ -823,6 +883,58 @@ public void testRemoveApplicationWithScaledContainer() throws Throwable {
assertNull(compositeComp);
}

@Test
public void testRequestWithAutoRedeploymentShouldRemoveDescription()
throws Throwable {
ContainerDescription desc = TestRequestStateFactory.createContainerDescription("name1",
true, true);
desc.healthConfig = new HealthConfig();
desc.healthConfig.autoredeploy = true;
desc.healthConfig.protocol = RequestProtocol.HTTP;
CompositeDescription compositeDesc = createCompositeDesc(true, desc);

RequestBrokerState request = TestRequestStateFactory.createRequestState(
ResourceType.COMPOSITE_COMPONENT_TYPE.getName(), compositeDesc.documentSelfLink);
request.tenantLinks = groupPlacementState.tenantLinks;
request = startRequest(request);
request = waitForRequestToComplete(request);

assertEquals(1, request.resourceLinks.size());
CompositeComponent cc = getDocument(CompositeComponent.class,
request.resourceLinks.iterator().next());

List<String> containerLinks = cc.componentLinks;
ContainerState container = getDocument(ContainerState.class, containerLinks.get(0));
assertNotNull(container);

CompositeComponent compositeComp = getDocument(CompositeComponent.class,
container.compositeComponentLink);
assertNotNull(compositeComp);

// Remove Containers
request = TestRequestStateFactory.createRequestState();
request.tenantLinks = groupPlacementState.tenantLinks;
request.resourceLinks = new HashSet<>(containerLinks);
request.operation = ContainerOperationType.DELETE.id;
request.customProperties = new HashMap<>();
request.customProperties.put("test", "test");

request = startRequest(request);

waitForRequestToComplete(request);

List<String> containerDescriptionLinks = new ArrayList<>(1);
containerDescriptionLinks.add(container.descriptionLink);
containerDescriptionLinks = findResourceLinks(ContainerDescription.class,
containerDescriptionLinks);
assertTrue("Description should be deleted.", containerDescriptionLinks.isEmpty());
container = searchForDocument(ContainerState.class, containerLinks.get(0));
assertNull(container);

compositeComp = searchForDocument(CompositeComponent.class, compositeComp.documentSelfLink);
assertNull(compositeComp);
}

/**
* Validate ports are released when container is removed
*/
Expand Down

0 comments on commit eaf8892

Please sign in to comment.