Skip to content

Commit

Permalink
core: make register-vm a sync operation
Browse files Browse the repository at this point in the history 
In 38d5120 we've changed adding vm from configuration to
be blocking/sync operations. Here we do the same for calls to
register-vm for the same reason - this operation doesn't involve
copying the disks, only add-lease is asynchronous and it is a
relatively short operation, so the caller won't wait much and
at the end of the call the vm is ready to be used.

This is particularly important for disaster recovery as we try
to start highly available vms right after registering them. It
might have been better to change the disaster recovery scripts
to specify async=false, but that would require more efforts.

That change revealed an issue with the tests of ExportVmTemplate
since it is monitored by polling the job but tested as if it
was monitored by polling the VDSM tasks. Therefore the API
tests of ExportVmTemplate are also changed here.

Bug-Url: https://bugzilla.redhat.com/1968433
Signed-off-by: Arik Hadas <ahadas@redhat.com>
  • Loading branch information
@ahadas
ahadas committed Sep 18, 2022
1 parent 97483d0 commit ce82106
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 64 deletions.
10 changes: 5 additions & 5 deletions ...rc/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendActionableResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected Response doAction(final ActionType task, final ActionParametersBase pa
}
return actionStatus(status, action, addLinks(newModel(id)));
} else {
return actionAsync(actionResult, action);
return actionAsync(actionResult, action, pollingType);
}
} else {
Object model = resolveCreated(actionResult, entityResolver, null);
Expand Down Expand Up @@ -81,12 +81,12 @@ protected Response doAction(final ActionType task, final ActionParametersBase pa
if (actionResult.getJobId() != null) {
setJobLink(action, actionResult);
}
if (actionResult.getHasAsyncTasks()) {
if (isAsyncTaskOrJobExists(pollingType, actionResult)) {
if (expectBlocking(action)) {
CreationStatus status = awaitCompletion(actionResult, pollingType);
return actionStatus(status, action, addLinks(newModel(id)));
} else {
return actionAsync(actionResult, action);
return actionAsync(actionResult, action, pollingType);
}
} else {
return actionSuccess(action, addLinks(newModel(id)));
Expand Down Expand Up @@ -183,7 +183,7 @@ private Response actionSuccess(Action action, Object result) {
return Response.ok().entity(action).build();
}

protected Response actionAsync(ActionReturnValue actionResult, Action action) {
protected Response actionAsync(ActionReturnValue actionResult, Action action, PollingType pollingType) {
action.setAsync(true);

String ids = asString(actionResult.getVdsmTaskIdList());
Expand All @@ -195,7 +195,7 @@ protected Response actionAsync(ActionReturnValue actionResult, Action action) {
addOrUpdateLink(action, "parent", path.substring(0, path.lastIndexOf("/")));
addOrUpdateLink(action, "replay", path);

CreationStatus status = getAsynchronousStatus(actionResult);
CreationStatus status = getAsynchronousStatus(actionResult, pollingType);
if (status != null) {
action.setStatus(status.value());
}
Expand Down
22 changes: 0 additions & 22 deletions ...rc/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,28 +263,6 @@ private <T> Response fetchCreatedEntity(IResolver<T, Q> entityResolver,
return response;
}

/**
* Returns true if there are still processes running in the
* background, associated with the current request.
*
* For vdsm-task polling, the indication is the existence of running
* vdsm tasks.
*
* For job polling, the indication is that the job is in PENDING or
* IN_PROGRESS status.
*/
private boolean isAsyncTaskOrJobExists(PollingType pollingType, ActionReturnValue createResult) {
if (pollingType==PollingType.VDSM_TASKS) {
//when the polling-type is vdsm_tasks, check for existing async-tasks
return createResult.getHasAsyncTasks();
} else if (pollingType==PollingType.JOB) {
//when the polling-type is job, check if the job is pending or in progress
CreationStatus status = getJobIdStatus(createResult);
return status==CreationStatus.PENDING || status==CreationStatus.IN_PROGRESS;
}
return false; //shouldn't reach here
}

/**
* In rare cases, after entity creation there is a need to make modifications
* to the created entity. Such changes should be done here
Expand Down
22 changes: 22 additions & 0 deletions ...pi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,28 @@ private CreationStatus getVdsmTasksStatus(ActionReturnValue result) {
return asyncStatus;
}

/**
* Returns true if there are still processes running in the
* background, associated with the current request.
*
* For vdsm-task polling, the indication is the existence of running
* vdsm tasks.
*
* For job polling, the indication is that the job is in PENDING or
* IN_PROGRESS status.
*/
protected boolean isAsyncTaskOrJobExists(PollingType pollingType, ActionReturnValue createResult) {
if (pollingType==PollingType.VDSM_TASKS) {
//when the polling-type is vdsm_tasks, check for existing async-tasks
return createResult.getHasAsyncTasks();
} else if (pollingType==PollingType.JOB) {
//when the polling-type is job, check if the job is pending or in progress
CreationStatus status = getJobIdStatus(createResult);
return status==CreationStatus.PENDING || status==CreationStatus.IN_PROGRESS;
}
return false; //shouldn't reach here
}

protected CreationStatus getJobIdStatus(ActionReturnValue result) {
Guid jobId = result.getJobId();
if (jobId == null || jobId.equals(Guid.Empty)) {
Expand Down
3 changes: 2 additions & 1 deletion ...s/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainVmResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public Response register(Action action) {
params.setClusterId(getClusterId(action));
}
params.setImagesExistOnTargetStorageDomain(true);
action.setAsync(false);

if (action.isSetClone()) {
params.setImportAsNewEntity(action.isClone());
Expand All @@ -117,7 +118,7 @@ public Response register(Action action) {
if (action.isSetName()) {
params.setName(action.getName());
}
return doAction(ActionType.ImportVmFromConfiguration, params, action);
return doAction(ActionType.ImportVmFromConfiguration, params, action, PollingType.JOB);
}

@Override
Expand Down
60 changes: 24 additions & 36 deletions ...axrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendTemplateResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@
import org.ovirt.engine.core.common.action.UpdateVmTemplateParameters;
import org.ovirt.engine.core.common.action.VmTemplateManagementParameters;
import org.ovirt.engine.core.common.businessentities.AsyncTaskStatus;
import org.ovirt.engine.core.common.businessentities.AsyncTaskStatusEnum;
import org.ovirt.engine.core.common.businessentities.GraphicsDevice;
import org.ovirt.engine.core.common.businessentities.VmIcon;
import org.ovirt.engine.core.common.businessentities.VmTemplate;
import org.ovirt.engine.core.common.job.JobExecutionStatus;
import org.ovirt.engine.core.common.queries.GetVmTemplateParameters;
import org.ovirt.engine.core.common.queries.IdQueryParameters;
import org.ovirt.engine.core.common.queries.NameQueryParameters;
Expand Down Expand Up @@ -302,7 +302,8 @@ protected void doTestExport(StorageDomain storageDomain, boolean exclusive) {
setUriInfo(setUpActionExpectations(ActionType.ExportVmTemplate,
MoveOrCopyParameters.class,
new String[]{"ContainerId", "StorageDomainId", "ForceOverride"},
new Object[]{GUIDS[0], GUIDS[2], exclusive}));
new Object[]{GUIDS[0], GUIDS[2], exclusive},
JobExecutionStatus.FINISHED));

Action action = new Action();
action.setStorageDomain(storageDomain);
Expand All @@ -312,19 +313,24 @@ protected void doTestExport(StorageDomain storageDomain, boolean exclusive) {
verifyActionResponse(resource.export(action));
}

@Test
public void testExportAsyncPending() {
doTestExportAsync(AsyncTaskStatusEnum.init, CreationStatus.PENDING);
}

@Test
public void testExportAsyncInProgress() {
doTestExportAsync(AsyncTaskStatusEnum.running, CreationStatus.IN_PROGRESS);
}
setUriInfo(setUpActionExpectations(ActionType.ExportVmTemplate,
MoveOrCopyParameters.class,
new String[] { "ContainerId", "StorageDomainId", "ForceOverride" },
new Object[] { GUIDS[0], GUIDS[2], false },
JobExecutionStatus.STARTED));

@Test
public void testExportAsyncFinished() {
doTestExportAsync(AsyncTaskStatusEnum.finished, CreationStatus.COMPLETE);
Action action = new Action();
StorageDomain storageDomain = new StorageDomain();
storageDomain.setId(GUIDS[2].toString());
action.setStorageDomain(storageDomain);

Response response = resource.export(action);
verifyActionResponse(response, "templates/" + GUIDS[0], true, null);
action = (Action)response.getEntity();
assertTrue(action.isSetStatus());
assertEquals(CreationStatus.IN_PROGRESS.value(), action.getStatus());
}

@Test
Expand Down Expand Up @@ -372,36 +378,18 @@ public void testUpdateSetAndUploadIconFailure() {
assertThrows(WebApplicationException.class, () -> verifyModel(resource.update(model), 0)), BAD_REQUEST);
}

private void doTestExportAsync(AsyncTaskStatusEnum asyncStatus, CreationStatus actionStatus) {
setUriInfo(setUpActionExpectations(ActionType.ExportVmTemplate,
MoveOrCopyParameters.class,
new String[] { "ContainerId", "StorageDomainId", "ForceOverride" },
new Object[] { GUIDS[0], GUIDS[2], false },
asList(GUIDS[1]),
asList(new AsyncTaskStatus(asyncStatus))));

Action action = new Action();
StorageDomain storageDomain = new StorageDomain();
storageDomain.setId(GUIDS[2].toString());
action.setStorageDomain(storageDomain);

Response response = resource.export(action);
verifyActionResponse(response, "templates/" + GUIDS[0], true, null);
action = (Action)response.getEntity();
assertTrue(action.isSetStatus());
assertEquals(actionStatus.value(), action.getStatus());
}

@Override
protected VmTemplate getEntity(int index) {
return setUpEntityExpectations(mock(VmTemplate.class), index);
}

protected UriInfo setUpActionExpectations(ActionType task,
Class<? extends ActionParametersBase> clz,
String[] names,
Object[] values) {
return setUpActionExpectations(task, clz, names, values, true, true, null, null, true);
Class<? extends ActionParametersBase> clz,
String[] names,
Object[] values,
JobExecutionStatus jobExecutionStatus) {
String uri = "templates/" + GUIDS[0] + "/action";
return setUpActionExpectations(task, clz, names, values, true, true, null, null, null, Guid.newGuid(), jobExecutionStatus, uri, true);
}

protected UriInfo setUpActionExpectations(ActionType task,
Expand Down

0 comments on commit ce82106

Please sign in to comment.