diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7f6645f4af4d..0ee24ed4c300 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,7 @@ jobs: smoke/test_affinity_groups_projects smoke/test_annotations smoke/test_async_job + smoke/test_async_jobs smoke/test_attach_multiple_volumes smoke/test_backup_recovery_dummy smoke/test_certauthority_root diff --git a/agent/src/main/java/com/cloud/agent/Agent.java b/agent/src/main/java/com/cloud/agent/Agent.java index 275fd41edc34..db5ded18ab1e 100644 --- a/agent/src/main/java/com/cloud/agent/Agent.java +++ b/agent/src/main/java/com/cloud/agent/Agent.java @@ -807,9 +807,7 @@ protected void processRequest(final Request request, final Link link) { } commandsInProgress.incrementAndGet(); try { - if (cmd.isReconcile()) { - cmd.setRequestSequence(request.getSequence()); - } + cmd.setRequestSequence(request.getSequence()); answer = serverResource.executeRequest(cmd); } finally { commandsInProgress.decrementAndGet(); @@ -1103,9 +1101,7 @@ public void processOtherTask(final Task task) { Answer answer = null; commandsInProgress.incrementAndGet(); try { - if (command.isReconcile()) { - command.setRequestSequence(req.getSequence()); - } + command.setRequestSequence(req.getSequence()); answer = serverResource.executeRequest(command); } finally { commandsInProgress.decrementAndGet(); diff --git a/api/src/main/java/com/cloud/agent/api/Answer.java b/api/src/main/java/com/cloud/agent/api/Answer.java index 1166c075a78e..861a38d79da6 100644 --- a/api/src/main/java/com/cloud/agent/api/Answer.java +++ b/api/src/main/java/com/cloud/agent/api/Answer.java @@ -21,6 +21,8 @@ public class Answer extends Command { protected boolean result; protected String details; + // set when the command was stopped because its async job was cancelled + protected boolean cancelled; protected Answer() { this(null); @@ -47,6 +49,20 @@ public String getDetails() { return details; } + public boolean isCancelled() { + return cancelled; + } + + public void setCancelled(final boolean cancelled) { + this.cancelled = cancelled; + } + + public static Answer createCancelledAnswer(final Command command, final String details) { + Answer answer = new Answer(command, false, details); + answer.setCancelled(true); + return answer; + } + @Override public boolean executeInSequence() { return false; @@ -69,6 +85,7 @@ public boolean equals(Object o) { Answer answer = (Answer) o; if (result != answer.result) return false; + if (cancelled != answer.cancelled) return false; if (details != null ? !details.equals(answer.details) : answer.details != null) return false; return true; @@ -78,6 +95,7 @@ public boolean equals(Object o) { public int hashCode() { int result1 = super.hashCode(); result1 = 31 * result1 + (result ? 1 : 0); + result1 = 31 * result1 + (cancelled ? 1 : 0); result1 = 31 * result1 + (details != null ? details.hashCode() : 0); return result1; } diff --git a/api/src/main/java/com/cloud/event/EventTypes.java b/api/src/main/java/com/cloud/event/EventTypes.java index f7d13343d469..334da23ef289 100644 --- a/api/src/main/java/com/cloud/event/EventTypes.java +++ b/api/src/main/java/com/cloud/event/EventTypes.java @@ -899,6 +899,8 @@ public class EventTypes { public static final String EVENT_DNS_RECORD_DELETE = "DNS.RECORD.DELETE"; public static final String EVENT_DNS_NAME_COLLISION = "DNS.NAME.COLLISION"; + public static final String EVENT_JOB_CANCEL = "JOB.CANCEL"; + static { // TODO: need a way to force author adding event types to declare the entity details as well, with out braking diff --git a/api/src/main/java/com/cloud/exception/OperationCancelledException.java b/api/src/main/java/com/cloud/exception/OperationCancelledException.java new file mode 100644 index 000000000000..9bcbe5603332 --- /dev/null +++ b/api/src/main/java/com/cloud/exception/OperationCancelledException.java @@ -0,0 +1,73 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.exception; + +import com.cloud.agent.api.Command; +import com.cloud.utils.SerialVersionUID; +import com.cloud.utils.exception.CloudRuntimeException; + +import java.util.Arrays; + +/** Thrown when an operation was stopped because its async job was cancelled. Unchecked: intermediate callers must not relabel it as a timeout. */ +public class OperationCancelledException extends CloudRuntimeException { + private static final long serialVersionUID = SerialVersionUID.OperationCancelledException; + long _agentId; + long _seqId; + int _time; + boolean _isActive; + String _reason; + + transient Command[] _cmds; + + public OperationCancelledException(Command[] cmds, long agentId, long seqId, int time, boolean isActive, String reason) { + super("Commands: " + Arrays.toString(cmds) + " to Host " + agentId + " with seqId " + seqId + " cancelled after " + time + " secs"); + _agentId = agentId; + _seqId = seqId; + _time = time; + _cmds = cmds; + _isActive = isActive; + _reason = reason; + } + + public OperationCancelledException(Command[] cmds, long agentId, long seqId, int time, boolean isActive) { + this(cmds, agentId, seqId, time, isActive, null); + } + + public long getAgentId() { + return _agentId; + } + + public long getSequenceId() { + return _seqId; + } + + public int getWaitTime() { + return _time; + } + + public boolean isActive() { + return _isActive; + } + + public String getReason() { + return _reason; + } + + public Command[] getCommands() { + return _cmds; + } +} diff --git a/api/src/main/java/com/cloud/vm/VirtualMachine.java b/api/src/main/java/com/cloud/vm/VirtualMachine.java index 3adcc85d28a1..4b437b4bd8e9 100644 --- a/api/src/main/java/com/cloud/vm/VirtualMachine.java +++ b/api/src/main/java/com/cloud/vm/VirtualMachine.java @@ -105,6 +105,7 @@ public static StateMachine2 getStat s_fsm.addTransition(new Transition(State.Starting, VirtualMachine.Event.AgentReportRunning, State.Running, Arrays.asList(new Impact[]{Impact.USAGE}))); s_fsm.addTransition(new Transition(State.Starting, VirtualMachine.Event.AgentReportStopped, State.Stopped, null)); s_fsm.addTransition(new Transition(State.Starting, VirtualMachine.Event.AgentReportShutdowned, State.Stopped, null)); + s_fsm.addTransition(new Transition(State.Starting, VirtualMachine.Event.StopRequested, State.Stopping, null)); s_fsm.addTransition(new Transition(State.Destroyed, VirtualMachine.Event.RecoveryRequested, State.Stopped, Arrays.asList(new Impact[]{Impact.USAGE}))); s_fsm.addTransition(new Transition(State.Destroyed, VirtualMachine.Event.ExpungeOperation, State.Expunging, null)); s_fsm.addTransition(new Transition(State.Running, VirtualMachine.Event.MigrationRequested, State.Migrating, null)); diff --git a/api/src/main/java/org/apache/cloudstack/api/APICommand.java b/api/src/main/java/org/apache/cloudstack/api/APICommand.java index b77649046ca9..ee841b4e0ffa 100644 --- a/api/src/main/java/org/apache/cloudstack/api/APICommand.java +++ b/api/src/main/java/org/apache/cloudstack/api/APICommand.java @@ -52,4 +52,6 @@ Class[] entityType() default {}; String httpMethod() default ""; + + boolean cancellable() default false; } diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiCommandResourceType.java b/api/src/main/java/org/apache/cloudstack/api/ApiCommandResourceType.java index 2aa97b65a3d5..d13b30cf0f3d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiCommandResourceType.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiCommandResourceType.java @@ -91,7 +91,8 @@ public enum ApiCommandResourceType { Extension(org.apache.cloudstack.extension.Extension.class), ExtensionCustomAction(org.apache.cloudstack.extension.ExtensionCustomAction.class), KmsKey(org.apache.cloudstack.kms.KMSKey.class), - HsmProfile(org.apache.cloudstack.kms.HSMProfile.class); + HsmProfile(org.apache.cloudstack.kms.HSMProfile.class), + Job(org.apache.cloudstack.jobs.JobInfo.class); private final Class clazz; diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index f74c46161180..8d82a8034f2d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -675,6 +675,7 @@ public class ApiConstants { public static final String USAGE_ID = "usageid"; public static final String USAGE_NAME = "usagename"; public static final String USAGE_TYPE = "usagetype"; + public static final String USAGE_SERVER = "usageserver"; public static final String INCLUDE_TAGS = "includetags"; public static final String VLAN = "vlan"; diff --git a/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java b/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java index 6e880c89432f..e4b0e5d97184 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java +++ b/api/src/main/java/org/apache/cloudstack/api/ResponseGenerator.java @@ -22,6 +22,7 @@ import java.util.Map; import java.util.Set; +import org.apache.cloudstack.api.command.user.job.CancelAsyncJobCmd; import org.apache.cloudstack.api.response.ConsoleSessionResponse; import org.apache.cloudstack.consoleproxy.ConsoleSession; import org.apache.cloudstack.acl.apikeypair.ApiKeyPair; @@ -397,6 +398,8 @@ TemplatePermissionsResponse createTemplatePermissionsResponse(ResponseView view, AsyncJobResponse queryJobResult(QueryAsyncJobResultCmd cmd); + AsyncJobResponse cancelJobResponse(CancelAsyncJobCmd cmd); + NetworkOfferingResponse createNetworkOfferingResponse(NetworkOffering offering); NetworkResponse createNetworkResponse(ResponseView view, Network network); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/usage/ListUsageJobsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/usage/ListUsageJobsCmd.java new file mode 100644 index 000000000000..eecbe27ebc9a --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/usage/ListUsageJobsCmd.java @@ -0,0 +1,77 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.admin.usage; + +import java.util.Date; + +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseListCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.response.ListResponse; +import org.apache.cloudstack.api.response.UsageJobResponse; + +@APICommand(name = "listUsageJobs", description = "Lists the usage jobs.", responseObject = UsageJobResponse.class, + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "24.0") +public class ListUsageJobsCmd extends BaseListCmd { + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, description = "The start date from which the usage jobs should be listed. Only jobs started on or after this date will be included. (use format \"yyyy-MM-dd'T'HH:mm:ss'+'SSSS\")") + private Date startDate; + + @Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, description = "The end date up to which the usage jobs should be listed. Only jobs started on or before this date will be included. (use format \"yyyy-MM-dd'T'HH:mm:ss'+'SSSS\")") + private Date endDate; + + @Parameter(name = ApiConstants.USAGE_SERVER, type = CommandType.STRING, description = "The usage server host name or ip") + private String usageServer; + + @Parameter(name = ApiConstants.DURATION, type = CommandType.INTEGER, description = "the duration in hours to list the usage jobs started or completed within that period up to now.") + private Integer duration; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public Date getStartDate() { + return startDate; + } + + public Date getEndDate() { + return endDate; + } + + public String getUsageServer() { + return usageServer; + } + + public Integer getDuration() { + return duration; + } + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + @Override + public void execute() { + ListResponse response = _usageService.getUsageJobs(this); + response.setResponseName(getCommandName()); + this.setResponseObject(response); + } +} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DestroyVMCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DestroyVMCmdByAdmin.java index cbe6494d4004..7ab285b18ed4 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DestroyVMCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/DestroyVMCmdByAdmin.java @@ -28,7 +28,8 @@ @APICommand(name = "destroyVirtualMachine", description = "Destroys an Instance. Once destroyed, only the administrator can recover it.", responseObject = UserVmResponse.class, responseView = ResponseView.Full, entityType = {VirtualMachine.class}, requestHasSensitiveInfo = false, - responseHasSensitiveInfo = true) + responseHasSensitiveInfo = true, + cancellable = true) public class DestroyVMCmdByAdmin extends DestroyVMCmd implements AdminCmd { @Parameter( name = ApiConstants.FORCED, diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java index 467b92d415d2..45ac0a27bd36 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVMCmd.java @@ -48,10 +48,10 @@ description = "Attempts Migration of an Instance to a different host or Root volume of the Instance to a different storage pool", responseObject = UserVmResponse.class, entityType = {VirtualMachine.class}, requestHasSensitiveInfo = false, - responseHasSensitiveInfo = true) + responseHasSensitiveInfo = true, + cancellable = true) public class MigrateVMCmd extends BaseAsyncCmd { - ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java index ad298513ac0b..85c2f5c20274 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java @@ -49,9 +49,9 @@ description = "Attempts Migration of an Instance with its volumes to a different host", responseObject = UserVmResponse.class, entityType = {VirtualMachine.class}, requestHasSensitiveInfo = false, - responseHasSensitiveInfo = true) -public class MigrateVirtualMachineWithVolumeCmd extends BaseAsyncCmd { - + responseHasSensitiveInfo = true, + cancellable = true) +public class MigrateVirtualMachineWithVolumeCmd extends BaseAsyncCmd { ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RebootVMCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RebootVMCmdByAdmin.java index 36fdc9d8ec18..c55aaaf0501b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RebootVMCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/RebootVMCmdByAdmin.java @@ -25,5 +25,5 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "rebootVirtualMachine", description = "Reboots an Instance.", responseObject = UserVmResponse.class, responseView = ResponseView.Full, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, cancellable = true) public class RebootVMCmdByAdmin extends RebootVMCmd implements AdminCmd {} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StartVMCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StartVMCmdByAdmin.java index 30fab3b9382f..cecff0bd1c15 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StartVMCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StartVMCmdByAdmin.java @@ -25,5 +25,5 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "startVirtualMachine", responseObject = UserVmResponse.class, description = "Starts an Instance.", responseView = ResponseView.Full, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, cancellable = true) public class StartVMCmdByAdmin extends StartVMCmd implements AdminCmd {} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StopVMCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StopVMCmdByAdmin.java index 6dc1947712d6..98ea91fb62ed 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StopVMCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/StopVMCmdByAdmin.java @@ -25,5 +25,5 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "stopVirtualMachine", responseObject = UserVmResponse.class, description = "Stops an Instance.", responseView = ResponseView.Full, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, cancellable = true) public class StopVMCmdByAdmin extends StopVMCmd implements AdminCmd {} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vmsnapshot/RevertToVMSnapshotCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vmsnapshot/RevertToVMSnapshotCmdByAdmin.java index a57362d62056..0c25c6c693e0 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vmsnapshot/RevertToVMSnapshotCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vmsnapshot/RevertToVMSnapshotCmdByAdmin.java @@ -23,5 +23,5 @@ import org.apache.cloudstack.api.response.UserVmResponse; @APICommand(name = "revertToVMSnapshot", description = "Revert Instance from a vmsnapshot.", responseObject = UserVmResponse.class, since = "4.2.0", responseView = ResponseView.Full, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, cancellable = true) public class RevertToVMSnapshotCmdByAdmin extends RevertToVMSnapshotCmd implements AdminCmd {} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/AttachVolumeCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/AttachVolumeCmdByAdmin.java index 20418d589405..5a6ee0f19d16 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/AttachVolumeCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/AttachVolumeCmdByAdmin.java @@ -25,5 +25,5 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "attachVolume", description = "Attaches a disk volume to an Instance.", responseObject = VolumeResponse.class, responseView = ResponseView.Full, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class AttachVolumeCmdByAdmin extends AttachVolumeCmd implements AdminCmd {} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/CreateVolumeCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/CreateVolumeCmdByAdmin.java index e156475b29ae..40b950bfbb33 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/CreateVolumeCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/CreateVolumeCmdByAdmin.java @@ -27,5 +27,5 @@ @APICommand(name = "createVolume", responseObject = VolumeResponse.class, description = "Creates a disk volume from a disk offering. This disk volume must still be attached to an Instance to make use of it.", responseView = ResponseView.Full, entityType = { Volume.class, VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class CreateVolumeCmdByAdmin extends CreateVolumeCmd implements AdminCmd {} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DestroyVolumeCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DestroyVolumeCmdByAdmin.java index de90fee102de..5273d562dc49 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DestroyVolumeCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DestroyVolumeCmdByAdmin.java @@ -33,7 +33,8 @@ since = "4.14.0", authorized = {RoleType.Admin}, requestHasSensitiveInfo = false, - responseHasSensitiveInfo = true) + responseHasSensitiveInfo = true, + cancellable = true) public class DestroyVolumeCmdByAdmin extends DestroyVolumeCmd implements AdminCmd { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DetachVolumeCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DetachVolumeCmdByAdmin.java index 05f9fe6a31a6..c39e487fc04f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DetachVolumeCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/DetachVolumeCmdByAdmin.java @@ -25,5 +25,5 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "detachVolume", description = "Detaches a disk volume from an Instance.", responseObject = VolumeResponse.class, responseView = ResponseView.Full, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class DetachVolumeCmdByAdmin extends DetachVolumeCmd implements AdminCmd {} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/MigrateVolumeCmdByAdmin.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/MigrateVolumeCmdByAdmin.java index 9dd544cae581..f30f3f788fbc 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/MigrateVolumeCmdByAdmin.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/volume/MigrateVolumeCmdByAdmin.java @@ -25,6 +25,6 @@ import com.cloud.storage.Volume; @APICommand(name = "migrateVolume", description = "Migrate volume", responseObject = VolumeResponse.class, since = "3.0.0", responseView = ResponseView.Full, entityType = { - Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class MigrateVolumeCmdByAdmin extends MigrateVolumeCmd implements AdminCmd { } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/event/ListEventsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/event/ListEventsCmd.java index b86d1f8b956a..2587044d7199 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/event/ListEventsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/event/ListEventsCmd.java @@ -22,6 +22,7 @@ import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd; import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.response.AsyncJobResponse; import org.apache.cloudstack.api.response.EventResponse; import org.apache.cloudstack.api.response.ListResponse; @@ -64,6 +65,9 @@ public class ListEventsCmd extends BaseListProjectAndAccountResourcesCmd { @Parameter(name = ApiConstants.START_ID, type = CommandType.UUID, entityType = EventResponse.class, description = "The parent/start ID of the event, when provided this will list all the events with the start/parent ID including the parent event") private Long startId; + @Parameter(name = ApiConstants.JOB_ID, type = CommandType.UUID, entityType = AsyncJobResponse.class, description = "the ID of the async job that raised the events", since = "24.0") + private Long jobId; + @Parameter(name = ApiConstants.RESOURCE_ID, type = CommandType.STRING, description = "The ID of the resource associated with the event", since="4.17.0") private String resourceId; @@ -112,6 +116,10 @@ public Long getStartId() { return startId; } + public Long getJobId() { + return jobId; + } + public String getResourceId() { return resourceId; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/job/CancelAsyncJobCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/job/CancelAsyncJobCmd.java new file mode 100644 index 000000000000..bdcfbe4305c1 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/job/CancelAsyncJobCmd.java @@ -0,0 +1,102 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.job; + + +import com.cloud.event.EventTypes; +import com.cloud.utils.StringUtils; +import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiCommandResourceType; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.AsyncJobResponse; + +import com.cloud.user.Account; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.jobs.AsyncJobService; + +import javax.inject.Inject; + +@APICommand(name = CancelAsyncJobCmd.APINAME, description = "Cancels the asynchronous job.", responseObject = AsyncJobResponse.class, + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, authorized = {RoleType.Admin}, since = "24.0") +public class CancelAsyncJobCmd extends BaseCmd { + public static final String APINAME = "cancelAsyncJob"; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.JOB_ID, type = CommandType.UUID, entityType = AsyncJobResponse.class, required = true, description = "the ID of the asynchronous job") + private Long id; + + @Inject + private AsyncJobService asyncJobService; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public Long getId() { + return id; + } + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public long getEntityOwnerId() { + return Account.ACCOUNT_ID_SYSTEM; + } + + @Override + public String getCommandName() { + return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX; + } + + public String getEventType() { + return EventTypes.EVENT_JOB_CANCEL; + } + + public String getEventDescription() { + return "Cancelling job with id: " + id; + } + + public ApiCommandResourceType getInstanceType() { + return ApiCommandResourceType.Job; + } + + public Long getInstanceId() { + return getId(); + } + + @Override + public void execute() { + String status = asyncJobService.cancelAsyncJob(id, "Cancel requested by " + CallContext.current().getCallingUser().toString()); + if (StringUtils.isBlank(status)) { + AsyncJobResponse response = _responseGenerator.cancelJobResponse(this); + response.setResponseName(getCommandName()); + setResponseObject(response); + } else { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, status); + } + } +} diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/job/ListAsyncJobsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/job/ListAsyncJobsCmd.java index 2c8401831132..289aa8b8ec8b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/job/ListAsyncJobsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/job/ListAsyncJobsCmd.java @@ -16,7 +16,14 @@ // under the License. package org.apache.cloudstack.api.command.user.job; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; +import java.util.List; +import java.util.Locale; + +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.StringUtils; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiArgValidator; @@ -26,8 +33,9 @@ import org.apache.cloudstack.api.response.AsyncJobResponse; import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.api.response.ManagementServerResponse; +import org.apache.cloudstack.jobs.JobInfo; -@APICommand(name = "listAsyncJobs", description = "Lists all pending asynchronous jobs for the Account.", responseObject = AsyncJobResponse.class, +@APICommand(name = "listAsyncJobs", description = "Lists asynchronous jobs for the Account.", responseObject = AsyncJobResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) public class ListAsyncJobsCmd extends BaseListAccountResourcesCmd { @@ -35,9 +43,12 @@ public class ListAsyncJobsCmd extends BaseListAccountResourcesCmd { //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// - @Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, description = "The start date of the async job (use format \"yyyy-MM-dd'T'HH:mm:ss'+'SSSS\")") + @Parameter(name = ApiConstants.START_DATE, type = CommandType.DATE, description = "The start date from which the async jobs should be listed. Only jobs created on or after this date will be included. (use format \"yyyy-MM-dd'T'HH:mm:ss'+'SSSS\")") private Date startDate; + @Parameter(name = ApiConstants.END_DATE, type = CommandType.DATE, description = "The end date up to which the async jobs should be listed. Only jobs created on or before this date will be included. (use format \"yyyy-MM-dd'T'HH:mm:ss'+'SSSS\")", since = "24.0") + private Date endDate; + @Parameter(name = ApiConstants.MANAGEMENT_SERVER_ID, type = CommandType.UUID, entityType = ManagementServerResponse.class, description = "The id of the management server", since="4.19") private Long managementServerId; @@ -47,6 +58,13 @@ public class ListAsyncJobsCmd extends BaseListAccountResourcesCmd { @Parameter(name = ApiConstants.RESOURCE_TYPE, type = CommandType.STRING, description = "the type of the resource associated with the job", since="4.22.1") private String resourceType; + @Parameter(name = ApiConstants.JOB_STATUS, type = CommandType.LIST, collectionType = CommandType.STRING, description = "Comma-separated list of job statuses to list the async jobs by. " + + "Accepts names (IN_PROGRESS, SUCCEEDED, FAILED, CANCELLED) or ordinals (0, 1, 2, 3). Only pending jobs are listed by default.", since = "24.0") + private List jobStatuses; + + @Parameter(name = ApiConstants.DURATION, type = CommandType.INTEGER, description = "the duration in hours to list the async jobs started or completed within that period up to now.", since = "24.0") + private Integer duration; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -55,6 +73,10 @@ public Date getStartDate() { return startDate; } + public Date getEndDate() { + return endDate; + } + public Long getManagementServerId() { return managementServerId; } @@ -67,15 +89,56 @@ public String getResourceType() { return resourceType; } + public List getJobStatuses() { + if (jobStatuses == null) { + return null; + } + + if (jobStatuses.isEmpty()) { + throw new InvalidParameterValueException("Empty job status"); + } + + List statuses = new ArrayList<>(jobStatuses.size()); + for (String status : jobStatuses) { + statuses.add((long)parseJobStatus(status).value()); + } + return statuses; + } + + private JobInfo.Status parseJobStatus(String status) { + if (StringUtils.isBlank(status)) { + throw new InvalidParameterValueException("Empty job status"); + } + + String value = status.trim(); + try { + return JobInfo.Status.fromValue(Integer.parseInt(value)); + } catch (NumberFormatException e) { + // not an ordinal, fall through and try the name + } catch (IllegalArgumentException e) { + throw new InvalidParameterValueException(String.format("Invalid job status: %s. Valid values are %s or their ordinals", + status, Arrays.toString(JobInfo.Status.values()))); + } + + try { + return JobInfo.Status.valueOf(value.toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + throw new InvalidParameterValueException(String.format("Invalid job status: %s. Valid values are %s or their ordinals", + status, Arrays.toString(JobInfo.Status.values()))); + } + } + + public Integer getDuration() { + return duration; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @Override public void execute() { - ListResponse response = _queryService.searchForAsyncJobs(this); response.setResponseName(getCommandName()); this.setResponseObject(response); - } } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java index d03df501847a..8af8a7a2b8dd 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java @@ -50,7 +50,7 @@ import com.cloud.utils.exception.CloudRuntimeException; @APICommand(name = "createSnapshot", description = "Creates an instant Snapshot of a volume.", responseObject = SnapshotResponse.class, entityType = {Snapshot.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class CreateSnapshotCmd extends BaseAsyncCreateCmd { // /////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/DeleteSnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/DeleteSnapshotCmd.java index b4eaceb61ba6..b99e186ae034 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/DeleteSnapshotCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/DeleteSnapshotCmd.java @@ -36,7 +36,7 @@ import com.cloud.user.Account; @APICommand(name = "deleteSnapshot", description = "Deletes a Snapshot of a disk volume.", responseObject = SuccessResponse.class, entityType = {Snapshot.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class DeleteSnapshotCmd extends BaseAsyncCmd { ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java index 59881dfdabe2..17a84f080490 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java @@ -36,7 +36,7 @@ import com.cloud.user.Account; @APICommand(name = "revertSnapshot", description = "This is supposed to revert a volume Snapshot. This command is only supported with KVM so far", responseObject = SnapshotResponse.class, entityType = {Snapshot.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class RevertSnapshotCmd extends BaseAsyncCmd { ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java index 0a3e510d3d53..5be01287137a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DestroyVMCmd.java @@ -43,7 +43,8 @@ @APICommand(name = "destroyVirtualMachine", description = "Destroys an Instance.", responseObject = UserVmResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, requestHasSensitiveInfo = false, - responseHasSensitiveInfo = true) + responseHasSensitiveInfo = true, + cancellable = true) public class DestroyVMCmd extends BaseAsyncCmd implements UserCmd { private static final String s_name = "destroyvirtualmachineresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/RebootVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/RebootVMCmd.java index 6f4431547848..35f776f08242 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/RebootVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/RebootVMCmd.java @@ -40,7 +40,7 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "rebootVirtualMachine", description = "Reboots an Instance.", responseObject = UserVmResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, cancellable = true) public class RebootVMCmd extends BaseAsyncCmd implements UserCmd { private static final String s_name = "rebootvirtualmachineresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/StartVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/StartVMCmd.java index 40ae91d4c264..272f8852215a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/StartVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/StartVMCmd.java @@ -48,7 +48,7 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "startVirtualMachine", responseObject = UserVmResponse.class, description = "Starts an Instance.", responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, cancellable = true) public class StartVMCmd extends BaseAsyncCmd implements UserCmd { private static final String s_name = "startvirtualmachineresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/StopVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/StopVMCmd.java index 232eeebd34be..963da6cf116b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/StopVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/StopVMCmd.java @@ -38,7 +38,7 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "stopVirtualMachine", responseObject = UserVmResponse.class, description = "Stops an Instance.", responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, cancellable = true) public class StopVMCmd extends BaseAsyncCmd implements UserCmd { private static final String s_name = "stopvirtualmachineresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java index 35723a3bbb74..fd7a93e02aa1 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java @@ -38,10 +38,9 @@ @APICommand(name = "createVMSnapshot", description = "Creates Snapshot for an Instance. Running KVM UEFI disk-only snapshots briefly suspend the Instance while copying NVRAM state.", responseObject = VMSnapshotResponse.class, since = "4.2.0", entityType = {VMSnapshot.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class CreateVMSnapshotCmd extends BaseAsyncCreateCmd { - @ACL(accessType = AccessType.OperateEntry) @Parameter(name = ApiConstants.VIRTUAL_MACHINE_ID, type = CommandType.UUID, required = true, entityType = UserVmResponse.class, description = "The ID of the Instance") private Long vmId; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/DeleteVMSnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/DeleteVMSnapshotCmd.java index 3373ac534cc8..a1f05919e5bb 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/DeleteVMSnapshotCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/DeleteVMSnapshotCmd.java @@ -36,7 +36,7 @@ import com.cloud.vm.snapshot.VMSnapshot; @APICommand(name = "deleteVMSnapshot", description = "Deletes an Instance Snapshot.", responseObject = SuccessResponse.class, since = "4.2.0", entityType = {VMSnapshot.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class DeleteVMSnapshotCmd extends BaseAsyncCmd { @ACL(accessType = AccessType.OperateEntry) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/RevertToVMSnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/RevertToVMSnapshotCmd.java index d44cefca5027..3cff47d43ed7 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/RevertToVMSnapshotCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/RevertToVMSnapshotCmd.java @@ -42,7 +42,7 @@ import com.cloud.vm.snapshot.VMSnapshot; @APICommand(name = "revertToVMSnapshot", description = "Revert Instance from a vmsnapshot.", responseObject = UserVmResponse.class, since = "4.2.0", responseView = ResponseView.Restricted, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, cancellable = true) public class RevertToVMSnapshotCmd extends BaseAsyncCmd implements UserCmd { private static final String s_name = "reverttovmsnapshotresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java index 8624043afc51..d89c92319509 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java @@ -38,7 +38,7 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "attachVolume", description = "Attaches a disk volume to an Instance.", responseObject = VolumeResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class AttachVolumeCmd extends BaseAsyncCmd implements UserCmd { private static final String s_name = "attachvolumeresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java index 538e263ae9de..ca83e2e880eb 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java @@ -47,7 +47,7 @@ @APICommand(name = "createVolume", responseObject = VolumeResponse.class, description = "Creates a disk volume from a disk offering. This disk volume must still be attached to an Instance to make use of it.", responseView = ResponseView.Restricted, entityType = { Volume.class, VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class CreateVolumeCmd extends BaseAsyncCreateCustomIdCmd implements UserCmd { private static final String s_name = "createvolumeresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/DestroyVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/DestroyVolumeCmd.java index e9e388436642..73a393d73b21 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/DestroyVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/DestroyVolumeCmd.java @@ -39,7 +39,8 @@ since = "4.14.0", authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}, requestHasSensitiveInfo = false, - responseHasSensitiveInfo = true) + responseHasSensitiveInfo = true, + cancellable = true) public class DestroyVolumeCmd extends BaseAsyncCmd { private static final String s_name = "destroyvolumeresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/DetachVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/DetachVolumeCmd.java index f6e811da6055..16534943bfe7 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/DetachVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/DetachVolumeCmd.java @@ -39,7 +39,7 @@ import com.cloud.vm.VirtualMachine; @APICommand(name = "detachVolume", description = "Detaches a disk volume from an Instance.", responseObject = VolumeResponse.class, responseView = ResponseView.Restricted, entityType = {VirtualMachine.class}, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class DetachVolumeCmd extends BaseAsyncCmd implements UserCmd { private static final String s_name = "detachvolumeresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/MigrateVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/MigrateVolumeCmd.java index 9927978ad55e..4bf446ee38ff 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/MigrateVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/MigrateVolumeCmd.java @@ -34,7 +34,7 @@ import com.cloud.user.Account; @APICommand(name = "migrateVolume", description = "Migrate volume", responseObject = VolumeResponse.class, since = "3.0.0", responseView = ResponseView.Restricted, entityType = { - Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, cancellable = true) public class MigrateVolumeCmd extends BaseAsyncCmd implements UserCmd { private static final String s_name = "migratevolumeresponse"; diff --git a/api/src/main/java/org/apache/cloudstack/api/response/UsageJobResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/UsageJobResponse.java new file mode 100644 index 000000000000..40b13c472516 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/response/UsageJobResponse.java @@ -0,0 +1,93 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.response; + +import java.util.Date; + +import com.google.gson.annotations.SerializedName; + +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseResponse; + +import com.cloud.serializer.Param; + +public class UsageJobResponse extends BaseResponse { + + @SerializedName(ApiConstants.USAGE_SERVER) + @Param(description = "the usage server host") + private String usageServer; + + @SerializedName("jobtype") + @Param(description = "the job type (0 - Recurring, 1 - Single)") + private Integer jobType; + + @SerializedName("scheduled") + @Param(description = "the job is scheduled or not") + private Integer scheduled; + + @SerializedName(ApiConstants.START_DATE) + @Param(description = " the start date of the job") + private Date startDate; + + @SerializedName(ApiConstants.END_DATE) + @Param(description = " the end date of the job") + private Date endDate; + + @SerializedName("executiontime") + @Param(description = " the execution time of the job") + private Long executionTime; + + @SerializedName(ApiConstants.SUCCESS) + @Param(description = "the job is success or not") + private Boolean success; + + @SerializedName("heartbeat") + @Param(description = "the job heartbeat") + private Date heartbeat; + + public void setUsageServer(String usageServer) { + this.usageServer = usageServer; + } + + public void setJobType(Integer jobType) { + this.jobType = jobType; + } + + public void setScheduled(Integer scheduled) { + this.scheduled = scheduled; + } + + public void setStartDate(Date startDate) { + this.startDate = startDate; + } + + public void setEndDate(final Date endDate) { + this.endDate = endDate; + } + + public void setExecutionTime(Long executionTime) { + this.executionTime = executionTime; + } + + public void setSuccess(Boolean success) { + this.success = success; + } + + public void setHeartbeat(final Date heartbeat) { + this.heartbeat = heartbeat; + } +} diff --git a/api/src/main/java/org/apache/cloudstack/jobs/AsyncJobService.java b/api/src/main/java/org/apache/cloudstack/jobs/AsyncJobService.java new file mode 100644 index 000000000000..fc50e4364d23 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/jobs/AsyncJobService.java @@ -0,0 +1,27 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.jobs; + +import com.cloud.utils.component.PluggableService; +import org.apache.cloudstack.framework.config.ConfigKey; + +public interface AsyncJobService extends PluggableService { + ConfigKey CancelledJobInterval = new ConfigKey<>("Advanced", Integer.class, "job.cancelled.interval", "1", + "Interval in seconds to check the jobs cancelled", false); + + String cancelAsyncJob(long jobId, String reason); +} diff --git a/api/src/main/java/org/apache/cloudstack/jobs/JobCancellationHandler.java b/api/src/main/java/org/apache/cloudstack/jobs/JobCancellationHandler.java new file mode 100644 index 000000000000..0a3d8f5b87df --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/jobs/JobCancellationHandler.java @@ -0,0 +1,27 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.jobs; + +/** Lets the job layer ask whoever runs a job's backend work whether it can be stopped, and stop it. An interface, not AgentManager: the job framework is built first. */ +public interface JobCancellationHandler { + + /** Asked before the job is marked cancelled; a job whose work cannot be stopped is refused, not recorded as cancelled. */ + boolean isJobExecutionCancellable(long jobId); + + /** Returns true only if everything in flight was actually stopped. */ + boolean cancelJobExecution(long jobId, String reason); +} diff --git a/api/src/main/java/org/apache/cloudstack/jobs/JobInfo.java b/api/src/main/java/org/apache/cloudstack/jobs/JobInfo.java index 5b63e627d622..998f8a20a130 100644 --- a/api/src/main/java/org/apache/cloudstack/jobs/JobInfo.java +++ b/api/src/main/java/org/apache/cloudstack/jobs/JobInfo.java @@ -23,17 +23,32 @@ public interface JobInfo extends Identity, InternalIdentity { public enum Status { - IN_PROGRESS(false), SUCCEEDED(true), FAILED(true), CANCELLED(true); + IN_PROGRESS(0, false), SUCCEEDED(1, true), FAILED(2, true), CANCELLED(3, true); + private final int value; private final boolean done; - private Status(boolean done) { + private Status(int value, boolean done) { + this.value = value; this.done = done; } + public int value() { + return value; + } + public boolean done() { return done; } + + public static Status fromValue(int value) { + for (Status status : Status.values()) { + if (status.value() == value) { + return status; + } + } + throw new IllegalArgumentException("Invalid status value: " + value); + } } String getType(); diff --git a/api/src/main/java/org/apache/cloudstack/usage/UsageService.java b/api/src/main/java/org/apache/cloudstack/usage/UsageService.java index 00e8b431f8fe..32256c538fc3 100644 --- a/api/src/main/java/org/apache/cloudstack/usage/UsageService.java +++ b/api/src/main/java/org/apache/cloudstack/usage/UsageService.java @@ -18,8 +18,11 @@ import com.cloud.utils.Pair; import org.apache.cloudstack.api.command.admin.usage.GenerateUsageRecordsCmd; +import org.apache.cloudstack.api.command.admin.usage.ListUsageJobsCmd; import org.apache.cloudstack.api.command.admin.usage.ListUsageRecordsCmd; import org.apache.cloudstack.api.command.admin.usage.RemoveRawUsageRecordsCmd; +import org.apache.cloudstack.api.response.ListResponse; +import org.apache.cloudstack.api.response.UsageJobResponse; import java.util.List; import java.util.TimeZone; @@ -61,4 +64,6 @@ public interface UsageService { TimeZone getUsageTimezone(); boolean removeRawUsageRecords(RemoveRawUsageRecordsCmd cmd); + + ListResponse getUsageJobs(ListUsageJobsCmd cmd); } diff --git a/core/src/main/java/com/cloud/agent/api/CancelCommand.java b/core/src/main/java/com/cloud/agent/api/CancelCommand.java index 5adace2d334e..ce63e4bce403 100644 --- a/core/src/main/java/com/cloud/agent/api/CancelCommand.java +++ b/core/src/main/java/com/cloud/agent/api/CancelCommand.java @@ -19,16 +19,23 @@ package com.cloud.agent.api; +/** Asks the executor of a request sequence to stop it; between management servers as a control request, to an agent as an ordinary command (checkOnly = just ask). */ public class CancelCommand extends Command { protected long sequence; protected String reason; + protected boolean checkOnly; protected CancelCommand() { } public CancelCommand(long sequence, String reason) { + this(sequence, reason, false); + } + + public CancelCommand(long sequence, String reason, boolean checkOnly) { this.sequence = sequence; this.reason = reason; + this.checkOnly = checkOnly; } public long getSequence() { @@ -39,6 +46,10 @@ public String getReason() { return reason; } + public boolean isCheckOnly() { + return checkOnly; + } + @Override public boolean executeInSequence() { return false; diff --git a/core/src/main/java/com/cloud/agent/transport/Request.java b/core/src/main/java/com/cloud/agent/transport/Request.java index 3769dbbd612c..199a7e8b9865 100644 --- a/core/src/main/java/com/cloud/agent/transport/Request.java +++ b/core/src/main/java/com/cloud/agent/transport/Request.java @@ -116,6 +116,8 @@ public static Version get(final byte ver) throws UnsupportedVersionException { protected String _content; protected String _agentName; + private volatile boolean _cancelled = false; + protected Request() { } @@ -262,6 +264,14 @@ public Command[] getCommands() { return _cmds; } + public void cancel() { + _cancelled = true; + } + + public boolean isCancelled() { + return _cancelled; + } + protected String getType() { return "Cmd "; } diff --git a/core/src/main/java/com/cloud/resource/RequestExecutionContext.java b/core/src/main/java/com/cloud/resource/RequestExecutionContext.java new file mode 100644 index 000000000000..0f0cc4d442c4 --- /dev/null +++ b/core/src/main/java/com/cloud/resource/RequestExecutionContext.java @@ -0,0 +1,38 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.resource; + +/** The agent request sequence the current thread executes a command for; set by the direct-agent attache so resources can key backend work by it. */ +public final class RequestExecutionContext { + + private static final ThreadLocal REQUEST_SEQUENCE = new ThreadLocal<>(); + + private RequestExecutionContext() { + } + + public static void setRequestSequence(final long sequence) { + REQUEST_SEQUENCE.set(sequence); + } + + public static Long getRequestSequence() { + return REQUEST_SEQUENCE.get(); + } + + public static void clear() { + REQUEST_SEQUENCE.remove(); + } +} diff --git a/core/src/main/java/com/cloud/resource/ServerResource.java b/core/src/main/java/com/cloud/resource/ServerResource.java index 23d200942a27..1dad2a0b7cf3 100644 --- a/core/src/main/java/com/cloud/resource/ServerResource.java +++ b/core/src/main/java/com/cloud/resource/ServerResource.java @@ -92,4 +92,14 @@ default boolean isAppendAgentNameToLogs() { } default void processPingAnswer(PingAnswer answer) {}; + + /** Defaults to false: a resource that has not implemented cancellation must not claim it. */ + default boolean isRequestSequenceCancellable(long sequence) { + return false; + } + + /** Returns true only when the backend operation was actually stopped. */ + default boolean cancelRequestSequence(long sequence) { + return false; + } } diff --git a/core/src/test/java/org/apache/cloudstack/api/agent/test/CancelCommandTest.java b/core/src/test/java/org/apache/cloudstack/api/agent/test/CancelCommandTest.java index 65fc5bc9e069..2aad6ee56427 100644 --- a/core/src/test/java/org/apache/cloudstack/api/agent/test/CancelCommandTest.java +++ b/core/src/test/java/org/apache/cloudstack/api/agent/test/CancelCommandTest.java @@ -41,6 +41,12 @@ public void testGetReason() { assertTrue(r.equals("goodreason")); } + @Test + public void testCheckOnlyDefaultsToFalse() { + assertFalse(cc.isCheckOnly()); + assertTrue(new CancelCommand(1L, "probe", true).isCheckOnly()); + } + @Test public void testExecuteInSequence() { boolean b = cc.executeInSequence(); diff --git a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java b/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java index 4d63fae33560..ce776f836f9d 100644 --- a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java +++ b/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java @@ -33,10 +33,12 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.resource.ServerResource; +import org.apache.cloudstack.jobs.JobCancellationHandler; + /** * AgentManager manages hosts. It directly coordinates between the DAOs and the connections it manages. */ -public interface AgentManager { +public interface AgentManager extends JobCancellationHandler { ConfigKey Wait = new ConfigKey("Advanced", Integer.class, "wait", "1800", "Time in seconds to wait for control commands to return", true); ConfigKey EnableKVMAutoEnableDisable = new ConfigKey<>(Boolean.class, @@ -178,4 +180,10 @@ enum TapAgentsAction { boolean transferDirectAgentsFromMS(String fromMsUuid, long fromMsId, long timeoutDurationInMs, boolean excludeHostsInMaintenance); int getHostSshPort(HostVO host); + + Long getAsyncJobId(); + + /** Answered from an in-memory view kept by the cancelled-jobs poller. */ + boolean isJobCancelled(Long jobId); + } diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java index 402bd2b6b9b9..5a08b02de26e 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java @@ -33,6 +33,7 @@ import java.util.concurrent.TimeUnit; import com.cloud.agent.api.CleanupPersistentNetworkResourceCommand; +import com.cloud.exception.OperationCancelledException; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; @@ -128,6 +129,8 @@ public int compare(final Object o1, final Object o2) { protected long _nextSequence; protected AgentManagerImpl _agentMgr; + // cancelled because the job was cancelled, as opposed to timed out + private final Set _cancelledSequences = ConcurrentHashMap.newKeySet(); public final static String[] s_commandsAllowedInMaintenanceMode = new String[] { MaintainCommand.class.toString(), MigrateCommand.class.toString(), StopCommand.class.toString(), CheckVirtualMachineCommand.class.toString(), PingTestCommand.class.toString(), CheckHealthCommand.class.toString(), @@ -231,6 +234,28 @@ protected synchronized void cancel(final long seq) { } } + /** Default false: an attache that cannot ask its resource must not claim it. */ + protected boolean isExecutionCancellable(final long seq) { + return false; + } + + /** Returns true only when the resource's work was stopped (or nothing was left to stop). */ + protected boolean cancelRunning(final long seq) { + return false; + } + + /** Job-cancel path; distinct from cancel(seq), which is also the timeout path and never reaches the hypervisor. */ + public boolean cancelExecution(final long seq) { + _cancelledSequences.add(seq); + if (!cancelRunning(seq)) { + // refused: the command runs on and its sender must see the real answer + _cancelledSequences.remove(seq); + return false; + } + cancel(seq); + return true; + } + protected synchronized int findRequest(final Request req) { return Collections.binarySearch(_requests, req, s_reqComparator); } @@ -411,13 +436,15 @@ public void send(final Request req, final Listener listener) throws AgentUnavail public Answer[] send(final Request req, final int wait) throws AgentUnavailableException, OperationTimedoutException { SynchronousListener sl = new SynchronousListener(null); - long seq = req.getSequence(); send(req, sl); try { for (int i = 0; i < 2; i++) { Answer[] answers = null; + if (_cancelledSequences.contains(seq)) { + throw new OperationCancelledException(req.getCommands(), _id, seq, wait, false); + } Command[] cmds = req.getCommands(); if (cmds != null && cmds.length == 1 && (cmds[0] != null) && cmds[0].isReconcile() && !sl.isDisconnected() && _agentMgr.isReconcileCommandsEnabled(_hypervisorType)) { @@ -427,14 +454,28 @@ public Answer[] send(final Request req, final int wait) throws AgentUnavailableE try { answers = sl.waitFor(wait); } catch (final InterruptedException e) { - logger.debug(LOG_SEQ_FORMATTED_STRING, seq, "Interrupted"); + logger.debug(LOG_SEQ_FORMATTED_STRING, seq, "Interrupted while waiting for the answer"); + Thread.currentThread().interrupt(); + if (_cancelledSequences.contains(seq)) { + throw new OperationCancelledException(req.getCommands(), _id, seq, wait, true, "Cancelled while waiting for the answer"); + } } } if (answers != null) { + for (Answer answer : answers) { + if (answer != null && answer.isCancelled()) { + throw new OperationCancelledException(req.getCommands(), _id, seq, wait, true, answer.getDetails()); + } + } + new Response(req, answers).logD("Received: ", false); return answers; } + if (_cancelledSequences.contains(seq)) { + throw new OperationCancelledException(req.getCommands(), _id, seq, wait, true, "Cancelled while waiting for the answer"); + } + answers = sl.getAnswers(); // Try it again. if (answers != null) { new Response(req, answers).logD("Received after timeout: ", true); @@ -463,6 +504,14 @@ public Answer[] send(final Request req, final int wait) throws AgentUnavailableE sendNext(seq); } throw e; + } catch (OperationCancelledException e) { + logger.warn(LOG_SEQ_FORMATTED_STRING, seq, "Cancelled: " + req.toString()); + cancel(seq); + final Long current = _currentSequence; + if (req.executeInSequence() && (current != null && current == seq)) { + sendNext(seq); + } + throw e; } catch (Exception e) { logger.warn(LOG_SEQ_FORMATTED_STRING, seq, "Exception while waiting for answer", e); cancel(seq); @@ -473,6 +522,7 @@ public Answer[] send(final Request req, final int wait) throws AgentUnavailableE _agentMgr.updateReconcileCommandsIfNeeded(req.getSequence(), req.getCommands(), Command.State.TIMED_OUT); throw new OperationTimedoutException(req.getCommands(), _id, seq, wait, false); } finally { + _cancelledSequences.remove(seq); unregisterListener(seq); } } diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index d3225f2fda5e..935f48575a7f 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -42,6 +42,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.exception.OperationCancelledException; import com.cloud.utils.StringUtils; import org.apache.cloudstack.agent.lb.IndirectAgentLB; import org.apache.cloudstack.ca.CAManager; @@ -55,6 +56,8 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.jobs.AsyncJob; import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext; +import org.apache.cloudstack.framework.jobs.AsyncJobManager; +import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; import org.apache.cloudstack.maintenance.ManagementServerMaintenanceListener; import org.apache.cloudstack.maintenance.ManagementServerMaintenanceManager; import org.apache.cloudstack.managed.context.ManagedContextRunnable; @@ -62,6 +65,7 @@ import org.apache.cloudstack.outofbandmanagement.dao.OutOfBandManagementDao; import org.apache.cloudstack.utils.identity.ManagementServerNode; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.ObjectUtils; @@ -143,6 +147,8 @@ import com.cloud.utils.nio.Task; import com.cloud.utils.time.InaccurateClock; +import static org.apache.cloudstack.jobs.AsyncJobService.CancelledJobInterval; + /** * Implementation of the Agent Manager. This class controls the connection to the agents. **/ @@ -159,6 +165,11 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl protected List _loadingAgents = new ArrayList<>(); protected Map _commandTimeouts = new HashMap<>(); private int _monitorId = 0; + // a job can have several commands in flight on several hosts + protected final Map>> _jobToHostIdAndReqSequenceMap = new ConcurrentHashMap<>(); + // sticky on purpose: workers consult it after the job row has been finalised + protected final Map _cancelledJobs = new ConcurrentHashMap<>(); + private static final long CANCELLED_JOB_MEMORY_MS = TimeUnit.HOURS.toMillis(1); @Inject protected CAManager caService; @@ -182,6 +193,8 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl protected ConfigurationDao _configDao = null; @Inject protected ClusterDao _clusterDao = null; + @Inject + protected AsyncJobManager asyncJobManager = null; @Inject protected HighAvailabilityManager _haMgr = null; @@ -210,6 +223,7 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl protected ScheduledExecutorService _directAgentExecutor; protected ScheduledExecutorService _cronJobExecutor; protected ScheduledExecutorService _monitorExecutor; + protected ScheduledExecutorService _cancelledJobsCheckExecutor; private int _directAgentThreadCap; @@ -390,6 +404,7 @@ public void onManagementServerCancelPreparingForMaintenance() { public void onManagementServerMaintenance() { logger.debug("Management server maintenance enabled"); _monitorExecutor.shutdownNow(); + _cancelledJobsCheckExecutor.shutdownNow(); newAgentConnectionsMonitor.shutdownNow(); if (_connection != null) { _connection.stop(); @@ -421,6 +436,9 @@ public void onManagementServerCancelMaintenance() { if (_monitorExecutor.isShutdown()) { initAndScheduleMonitorExecutor(); } + if (_cancelledJobsCheckExecutor.isShutdown()) { + initAndScheduleCancelJobExecutor(); + } if (newAgentConnectionsMonitor.isShutdown()) { initAndScheduleAgentConnectionsMonitor(); } @@ -437,6 +455,11 @@ private void initAndScheduleMonitorExecutor() { _monitorExecutor.scheduleWithFixedDelay(new MonitorTask(), mgmtServiceConf.getPingInterval(), mgmtServiceConf.getPingInterval(), TimeUnit.SECONDS); } + private void initAndScheduleCancelJobExecutor() { + _cancelledJobsCheckExecutor = new ScheduledThreadPoolExecutor(1, new NamedThreadFactory("CancelledJobsCheck")); + _cancelledJobsCheckExecutor.scheduleWithFixedDelay(new CancelledJobsCheckTask(), CancelledJobInterval.value(), CancelledJobInterval.value(), TimeUnit.SECONDS); + } + private void initAndScheduleAgentConnectionsMonitor() { final int cleanupTimeInSecs = Wait.value(); newAgentConnectionsMonitor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("NewAgentConnectionsMonitor")); @@ -637,6 +660,12 @@ public Answer[] send(final Long hostId, final Commands commands, int timeout) th throw new AgentUnavailableException(-1); } + final Long jobId = getAsyncJobId(); + if (jobId != null && isJobCancelled(jobId)) { + logger.debug("job-{} for host: {}, with commands: {} is cancelled", jobId, hostId, commands); + throw new OperationCancelledException(commands.toCommands(), hostId, 0, 0, false); + } + int wait = getTimeout(commands, timeout); logger.debug("Wait time setting on {} is {} seconds", commands, wait); for (Command cmd : commands) { @@ -666,16 +695,29 @@ public Answer[] send(final Long hostId, final Commands commands, int timeout) th final Request req = new Request(hostId, agent.getName(), _nodeId, cmds, commands.stopOnError(), true); req.setSequence(agent.getNextSequence()); + final Pair inFlight = new Pair<>(hostId, req.getSequence()); + if (jobId != null) { + _jobToHostIdAndReqSequenceMap.computeIfAbsent(jobId, k -> ConcurrentHashMap.newKeySet()).add(inFlight); + } - reconcileCommandService.persistReconcileCommands(hostId, req.getSequence(), cmds); + try { + reconcileCommandService.persistReconcileCommands(hostId, req.getSequence(), cmds); - final Answer[] answers = agent.send(req, wait); + final Answer[] answers = agent.send(req, wait); - reconcileCommandService.processAnswers(req.getSequence(), cmds, answers); + reconcileCommandService.processAnswers(req.getSequence(), cmds, answers); - notifyAnswersToMonitors(hostId, req.getSequence(), answers); - commands.setAnswers(answers); - return answers; + notifyAnswersToMonitors(hostId, req.getSequence(), answers); + commands.setAnswers(answers); + return answers; + } finally { + if (jobId != null) { + _jobToHostIdAndReqSequenceMap.computeIfPresent(jobId, (k, v) -> { + v.remove(inFlight); + return v.isEmpty() ? null : v; + }); + } + } } protected Status investigate(final AgentAttache agent) { @@ -891,6 +933,7 @@ public boolean start() { ManagementServerHostVO msHost = _mshostDao.findByMsid(_nodeId); if (msHost != null && (ManagementServerHost.State.Maintenance.equals(msHost.getState()) || ManagementServerHost.State.PreparingForMaintenance.equals(msHost.getState()))) { _monitorExecutor.shutdownNow(); + _cancelledJobsCheckExecutor.shutdownNow(); newAgentConnectionsMonitor.shutdownNow(); return true; } @@ -907,6 +950,7 @@ public boolean start() { } initAndScheduleMonitorExecutor(); + initAndScheduleCancelJobExecutor(); initAndScheduleAgentConnectionsMonitor(); return true; } @@ -1069,6 +1113,7 @@ public boolean stop() { _connectExecutor.shutdownNow(); _monitorExecutor.shutdownNow(); + _cancelledJobsCheckExecutor.shutdownNow(); newAgentConnectionsMonitor.shutdownNow(); return true; } @@ -2064,6 +2109,100 @@ protected List findAgentsBehindOnPing() { } } + /** Stops in-flight work of jobs cancelled on this management server and acknowledges them. */ + protected class CancelledJobsCheckTask extends ManagedContextRunnable { + @Override + protected void runInContext() { + try { + final long now = System.currentTimeMillis(); + _cancelledJobs.values().removeIf(firstSeen -> now - firstSeen > CANCELLED_JOB_MEMORY_MS); + + for (final AsyncJobVO job : asyncJobManager.listCancelledJobsExecutingOn(_nodeId)) { + _cancelledJobs.putIfAbsent(job.getId(), now); + if (_jobToHostIdAndReqSequenceMap.containsKey(job.getId())) { + logger.info("Job-{} on {} {} was cancelled, stopping its in-flight commands", + job.getId(), job.getInstanceType(), job.getInstanceId()); + if (!cancelJobExecution(job.getId(), "Job was cancelled")) { + logger.warn("Not every in-flight command of cancelled job-{} could be stopped; retrying on the next check", job.getId()); + continue; + } + } + asyncJobManager.finalizeCancelledJob(job.getId()); + } + } catch (final Throwable e) { + logger.error("Unexpected exception in the cancelled jobs check task", e); + } + } + } + + @Override + public boolean isJobCancelled(final Long jobId) { + return jobId != null && _cancelledJobs.containsKey(jobId); + } + + @Override + public boolean isJobExecutionCancellable(final long jobId) { + final Set> inFlight = _jobToHostIdAndReqSequenceMap.get(jobId); + if (CollectionUtils.isEmpty(inFlight)) { + return true; + } + + for (final Pair request : inFlight) { + final AgentAttache attache = findAttache(request.first()); + if (attache == null) { + logger.debug("Agent {} for job-{} is no longer attached, treating sequence {} as not cancellable", + request.first(), jobId, request.second()); + return false; + } + if (!attache.isExecutionCancellable(request.second())) { + logger.info("Job-{} cannot be cancelled: sequence {} on host {} is not cancellable", + jobId, request.second(), request.first()); + return false; + } + } + return true; + } + + @Override + public boolean cancelJobExecution(final long jobId, final String reason) { + // mark first, so a worker that carries on to its next command is refused there + _cancelledJobs.putIfAbsent(jobId, System.currentTimeMillis()); + final Set> inFlight = _jobToHostIdAndReqSequenceMap.get(jobId); + if (CollectionUtils.isEmpty(inFlight)) { + return true; + } + + boolean allCancelled = true; + for (final Pair request : inFlight) { + final AgentAttache attache = findAttache(request.first()); + if (attache == null) { + logger.debug("Agent {} for job-{} is no longer attached, cannot cancel sequence {}", + request.first(), jobId, request.second()); + allCancelled = false; + continue; + } + if (!attache.isExecutionCancellable(request.second())) { + logger.info("Not cancelling sequence {} on host {} for job-{}: it is not cancellable, letting it run to completion", + request.second(), request.first(), jobId); + allCancelled = false; + continue; + } + logger.debug("Cancelling sequence {} on host {} for job-{} ({})", request.second(), request.first(), jobId, reason); + if (!attache.cancelExecution(request.second())) { + allCancelled = false; + } + } + return allCancelled; + } + + private AgentAttache findAttache(final Long hostId) { + try { + return getAttache(hostId); + } catch (final AgentUnavailableException e) { + return null; + } + } + protected class AgentNewConnectionsMonitorTask extends ManagedContextRunnable { @Override protected void runInContext() { @@ -2306,6 +2445,21 @@ public int getHostSshPort(HostVO host) { return Integer.parseInt(hostPort); } + @Override + public Long getAsyncJobId() { + Long jobId = null; + final AsyncJobExecutionContext context = AsyncJobExecutionContext.getCurrent(); + if (context != null && context.getJob() != null) { + AsyncJob job = context.getJob(); + if (StringUtils.isNotEmpty(job.getRelated())) { + jobId = Long.parseLong(job.getRelated()); + } else { + jobId = job.getId(); + } + } + return jobId; + } + private GlobalLock getHostJoinLock(Long hostId) { return GlobalLock.getInternLock(String.format("%s-%s", "Host-Join", hostId)); } diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/ConnectedAgentAttache.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/ConnectedAgentAttache.java index f4efaaa34a42..f8570e3f66b0 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/ConnectedAgentAttache.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/ConnectedAgentAttache.java @@ -19,8 +19,12 @@ import java.nio.channels.ClosedChannelException; +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CancelCommand; +import com.cloud.agent.api.UnsupportedAnswer; import com.cloud.agent.transport.Request; import com.cloud.exception.AgentUnavailableException; +import com.cloud.exception.OperationTimedoutException; import com.cloud.host.Status; import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.nio.Link; @@ -31,6 +35,7 @@ public class ConnectedAgentAttache extends AgentAttache { protected Link _link; + private static final int CANCEL_COMMAND_WAIT_SECONDS = 60; public ConnectedAgentAttache(final AgentManagerImpl agentMgr, final long id, final String uuid, final String name, final Hypervisor.HypervisorType hypervisorType, final Link link, final boolean maintenance) { super(agentMgr, id, uuid, name, hypervisorType, maintenance); @@ -51,6 +56,39 @@ public synchronized boolean isClosed() { return _link == null; } + /** Only the agent knows whether its backend work can be stopped; an agent that does not understand the question answers unsupported. */ + @Override + protected boolean isExecutionCancellable(final long seq) { + return askAgentToCancel(seq, true); + } + + @Override + protected boolean cancelRunning(final long seq) { + return askAgentToCancel(seq, false); + } + + private boolean askAgentToCancel(final long seq, final boolean checkOnly) { + if (_agentMgr == null) { + return false; + } + final CancelCommand cmd = new CancelCommand(seq, "Job cancelled", checkOnly); + cmd.setWait(CANCEL_COMMAND_WAIT_SECONDS); + try { + final Answer answer = _agentMgr.send(_id, cmd); + if (answer == null || answer instanceof UnsupportedAnswer) { + logger.debug(LOG_SEQ_FORMATTED_STRING, seq, "Agent does not support cancellation, treating as not cancellable"); + return false; + } + if (!answer.getResult()) { + logger.debug(LOG_SEQ_FORMATTED_STRING, seq, (checkOnly ? "Not cancellable: " : "Not cancelled: ") + answer.getDetails()); + } + return answer.getResult(); + } catch (final AgentUnavailableException | OperationTimedoutException e) { + logger.warn(LOG_SEQ_FORMATTED_STRING, seq, "Unable to ask the agent about cancellation: " + e.getMessage()); + return false; + } + } + @Override public void disconnect(final Status state) { synchronized (this) { diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java index 2edc9ad19bc3..2d703b024679 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java @@ -17,8 +17,12 @@ package com.cloud.agent.manager; import java.util.ArrayList; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Future; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -37,6 +41,7 @@ import com.cloud.exception.AgentUnavailableException; import com.cloud.host.Status; import com.cloud.hypervisor.Hypervisor; +import com.cloud.resource.RequestExecutionContext; import com.cloud.resource.ServerResource; import org.apache.logging.log4j.ThreadContext; @@ -47,9 +52,11 @@ public class DirectAgentAttache extends AgentAttache { protected final ConfigKey _HostPingRetryTimer = new ConfigKey("Advanced", Integer.class, "host.ping.retry.timer", "5", "Interval to wait before retrying a host ping while waiting for check results", true); ServerResource _resource; - List> _futures = new ArrayList>(); + List> _futures = new ArrayList<>(); + private final Map> _taskFutures = new ConcurrentHashMap<>(); + private final Map _taskRequests = new ConcurrentHashMap<>(); long _seq = 0; - LinkedList tasks = new LinkedList(); + LinkedList tasks = new LinkedList<>(); AtomicInteger _outstandingTaskCount; AtomicInteger _outstandingCronTaskCount; @@ -68,12 +75,103 @@ public void disconnect(Status state) { future.cancel(false); } + synchronized (this) { + for (Task task : tasks) { + task._req.cancel(); + } + tasks.clear(); + } + + for (Request request : _taskRequests.values()) { + request.cancel(); + } + _taskRequests.clear(); + + for (Future future : _taskFutures.values()) { + boolean cancelled = future.cancel(false); + logger.debug("Running task {} for [id: {}, uuid: {}, name: {}]", cancelled ? "cancelled" : "not cancelled", _id, _uuid, _name); + } + _taskFutures.clear(); + synchronized (this) { if (_resource != null) { _resource.disconnected(); _resource = null; } } + + cleanup(state); + } + + private synchronized boolean isQueued(final long seq) { + for (Task task : tasks) { + if (task._req.getSequence() == seq) { + return true; + } + } + return false; + } + + @Override + protected boolean isExecutionCancellable(final long seq) { + if (isQueued(seq)) { + return true; + } + + if (!_taskFutures.containsKey(seq)) { + return false; + } + + final ServerResource resource = _resource; + // not under the attache lock: this call goes out to the hypervisor + return resource != null && resource.isRequestSequenceCancellable(seq); + } + + @Override + protected boolean cancelRunning(final long seq) { + if (removeQueuedTask(seq)) { + logger.debug(LOG_SEQ_FORMATTED_STRING, seq, "Cancelled queued task."); + return true; + } + + final Future future = _taskFutures.get(seq); + if (future == null) { + return true; + } + + final ServerResource resource = _resource; + if (resource == null || !resource.isRequestSequenceCancellable(seq)) { + logger.info(LOG_SEQ_FORMATTED_STRING, seq, "Cancellation requested but the command is not cancellable, letting it run to completion."); + return false; + } + + logger.debug(LOG_SEQ_FORMATTED_STRING, seq, "Cancelling request sequence at the resource."); + if (!resource.cancelRequestSequence(seq)) { + logger.info(LOG_SEQ_FORMATTED_STRING, seq, "Resource could not cancel the command, letting it run to completion."); + return false; + } + + final Request request = _taskRequests.get(seq); + if (request != null) { + request.cancel(); + } + final boolean cancelled = future.cancel(true); + logger.debug(LOG_SEQ_FORMATTED_STRING, seq, "Running task " + (cancelled ? "cancelled" : "not cancelled")); + return true; + } + + private synchronized boolean removeQueuedTask(final long seq) { + final Iterator iterator = tasks.iterator(); + while (iterator.hasNext()) { + final Task task = iterator.next(); + if (task._req.getSequence() == seq) { + task._req.cancel(); + iterator.remove(); + _taskRequests.remove(seq); + return true; + } + } + return false; } @Override @@ -139,6 +237,7 @@ protected void finalize() throws Throwable { private synchronized void queueTask(Task task) { tasks.add(task); + _taskRequests.put(task._req.getSequence(), task._req); } private synchronized void scheduleFromQueue() { @@ -146,7 +245,13 @@ private synchronized void scheduleFromQueue() { _id, _uuid, _name, tasks.size(), _outstandingTaskCount.get()); while (!tasks.isEmpty() && _outstandingTaskCount.get() < _agentMgr.getDirectAgentThreadCap()) { _outstandingTaskCount.incrementAndGet(); - _agentMgr.getDirectAgentPool().execute(tasks.remove()); + Task task = tasks.remove(); + Future future = _agentMgr.getDirectAgentPool().submit(task); + _taskFutures.put(task._req.getSequence(), future); + if (future.isDone()) { + // the task may have finished (and cleaned up) before the future was recorded + _taskFutures.remove(task._req.getSequence()); + } } } @@ -227,6 +332,7 @@ private void bailout() { @Override protected void runInContext() { long seq = _req.getSequence(); + RequestExecutionContext.setRequestSequence(seq); try { if (_outstandingCronTaskCount.incrementAndGet() >= _agentMgr.getDirectAgentThreadCap()) { logger.warn( @@ -278,6 +384,7 @@ protected void runInContext() { } catch (Exception e) { logger.warn(LOG_SEQ_FORMATTED_STRING, seq, "Exception caught ", e); } finally { + RequestExecutionContext.clear(); _outstandingCronTaskCount.decrementAndGet(); } } @@ -293,14 +400,26 @@ public Task(Request req) { @Override protected void runInContext() { long seq = _req.getSequence(); + RequestExecutionContext.setRequestSequence(seq); try { - ServerResource resource = _resource; Command[] cmds = _req.getCommands(); + if (Thread.currentThread().isInterrupted() || _req.isCancelled()) { + throw new InterruptedException("Task execution cancelled"); + } + + ServerResource resource = _resource; boolean stopOnError = _req.stopOnError(); logger.debug(LOG_SEQ_FORMATTED_STRING, seq, "Executing request"); - ArrayList answers = new ArrayList(cmds.length); + ArrayList answers = new ArrayList<>(cmds.length); for (int i = 0; i < cmds.length; i++) { + if (Thread.currentThread().isInterrupted() || _req.isCancelled()) { + for (int j = i; j < cmds.length; j++) { + answers.add(Answer.createCancelledAnswer(cmds[j], "Command cancelled")); + } + break; + } + Answer answer = null; Command currentCmd = cmds[i]; if (currentCmd.getContextParam("logid") != null) { @@ -313,6 +432,16 @@ protected void runInContext() { logger.warn("Resource returned null answer!"); answer = new Answer(cmds[i], false, "Resource returned null answer"); } + + // cancelled mid-request: do not start the remaining commands + if ((Thread.currentThread().isInterrupted() || _req.isCancelled()) + && resource.isRequestSequenceCancellable(seq)) { + answers.add(answer); + for (int j = i + 1; j < cmds.length; j++) { + answers.add(Answer.createCancelledAnswer(cmds[j], "Command cancelled")); + } + break; + } } else { answer = new Answer(cmds[i], false, "Agent is disconnected"); } @@ -334,13 +463,34 @@ protected void runInContext() { logger.debug(LOG_SEQ_FORMATTED_STRING, seq, "Response Received: "); processAnswers(seq, resp); + } catch (InterruptedException e) { + logger.warn(LOG_SEQ_FORMATTED_STRING, seq, "Task interrupted"); + Thread.currentThread().interrupt(); + handleCancellation(seq, "Task interrupted: " + e.getMessage()); } catch (Throwable t) { // This is pretty serious as processAnswers might not be called and the calling process is stuck waiting for the full timeout logger.error(LOG_SEQ_FORMATTED_STRING, seq, "Throwable caught in runInContext, this will cause the management to become unpredictable", t); } finally { + RequestExecutionContext.clear(); + _taskFutures.remove(seq); + _taskRequests.remove(seq); _outstandingTaskCount.decrementAndGet(); scheduleFromQueue(); } } + + private void handleCancellation(long seq, String reason) { + try { + Command[] cmds = _req.getCommands(); + ArrayList answers = new ArrayList<>(cmds.length); + for (Command cmd : cmds) { + answers.add(Answer.createCancelledAnswer(cmd, reason != null ? reason : "Task cancelled")); + } + Response resp = new Response(_req, answers.toArray(new Answer[answers.size()])); + processAnswers(seq, resp); + } catch (Exception e) { + logger.warn(LOG_SEQ_FORMATTED_STRING, seq, "Exception while handling cancellation", e); + } + } } } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 8c9a1653a260..374c661945ca 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -210,6 +210,7 @@ import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InsufficientServerCapacityException; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.OperationCancelledException; import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; @@ -662,7 +663,7 @@ public void allocate(final String vmInstanceName, final VirtualMachineTemplate t } VirtualMachineGuru getVmGuru(final VirtualMachine vm) { - if(vm != null) { + if (vm != null) { return _vmGurus.get(vm.getType()); } return null; @@ -1005,7 +1006,6 @@ protected boolean checkWorkItems(final VMInstanceVO vm, final State state) throw } logger.debug("Waited some more to make sure there's no activity on " + vm); } - } @DB @@ -1295,7 +1295,6 @@ protected void updateExternalVmFromPrepareAnswer(VirtualMachineTO vmTO, VirtualM } updateExternalVmDataFromPrepareAnswer(vmTO, updatedTO); updateExternalVmNicsFromPrepareAnswer(vmTO, updatedTO); - return; } protected void processPrepareExternalProvisioning(boolean firstStart, Host host, @@ -1677,6 +1676,14 @@ public void orchestrateStart(final String vmUuid, final Map sendStop(final VirtualMachineGuru guru, final Vi final StopCommand stop = stpCmd; try { Answer answer = null; - if(vm.getHostId() != null) { + if (vm.getHostId() != null) { answer = _agentMgr.send(vm.getHostId(), stop); } if (answer != null && answer instanceof StopAnswer) { @@ -2288,8 +2294,7 @@ protected Pair sendStop(final VirtualMachineGuru guru, final Vi logger.error(errorMsg); return new Pair<>(false, errorMsg); } - - } catch (final AgentUnavailableException | OperationTimedoutException e) { + } catch (AgentUnavailableException | OperationTimedoutException e) { String errorMsg = String.format("Unable to stop %s due to [%s].", vm.toString(), e.getMessage()); logger.warn(errorMsg, e); if (!force) { @@ -2337,7 +2342,6 @@ protected Pair cleanup(final VirtualMachineGuru guru, final Vir } } } - } else if (state == State.Stopping) { if (vm.getHostId() != null) { Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); @@ -2407,7 +2411,6 @@ public void advanceStop(final String vmUuid, final boolean cleanUpEvenIfUnableTo _workJobDao.expunge(placeHolder.getId()); } } - } else { final Outcome outcome = stopVmThroughJobQueue(vmUuid, cleanUpEvenIfUnableToStop); @@ -2489,8 +2492,7 @@ private Pair getVMNetworkDetails(NetworkVO networkVO, boolean i return null; } - private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnableToStop) throws AgentUnavailableException, OperationTimedoutException, - ConcurrentOperationException { + private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnableToStop) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException { final State state = vm.getState(); if (state == State.Stopped) { logger.debug("VM is already stopped: {}", vm); @@ -2768,13 +2770,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) throws * @param expunge indicates if vm should be expunged */ private void deleteVMSnapshots(VMInstanceVO vm, boolean expunge) { - if (! vm.getHypervisorType().equals(HypervisorType.VMware)) { + if (!vm.getHypervisorType().equals(HypervisorType.VMware)) { if (!_vmSnapshotMgr.deleteAllVMSnapshots(vm.getId(), null)) { logger.debug("Unable to delete all Snapshots for {}", vm); throw new CloudRuntimeException("Unable to delete Instance Snapshots for " + vm); } - } - else { + } else { if (expunge) { _vmSnapshotMgr.deleteVMSnapshotsFromDB(vm.getId(), false); } @@ -3012,7 +3013,7 @@ private void checkDestinationForTags(StoragePool destPool, VMInstanceVO vm) { for(Volume vol : vols) { DiskOfferingVO diskOffering = _diskOfferingDao.findById(vol.getDiskOfferingId()); List volumeTags = StringUtils.csvTagsToList(diskOffering.getTags()); - if(! matches(volumeTags, storageTags)) { + if (!matches(volumeTags, storageTags)) { String msg = String.format("destination pool '%s' with tags '%s', does not support the volume diskoffering for volume '%s' (tags: '%s') ", destPool.getName(), StringUtils.listToCsvTags(storageTags), @@ -3204,8 +3205,8 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy throw new AgentUnavailableException(msg, dstHostId); } logger.debug("Successfully prepared destination host {} for migration of VM {} ", dstHostId, vm.getInstanceName()); - } catch (final OperationTimedoutException e1) { - throw new AgentUnavailableException("Operation timed out", dstHostId); + } catch (final OperationTimedoutException e) { + throw new AgentUnavailableException("Operation timed out ", dstHostId); } finally { if (pfma == null) { _networkMgr.rollbackNicForMigration(vmSrc, profile); @@ -3266,7 +3267,7 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy throw new CloudRuntimeException(details); } logger.info("Migration command successful for VM {}", vm.getInstanceName()); - } catch (final OperationTimedoutException e) { + } catch (OperationTimedoutException e) { boolean success = false; if (HypervisorType.KVM.equals(vm.getHypervisorType())) { try { @@ -3283,11 +3284,15 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy } } if (!success) { - if (e.isActive()) { + if (e instanceof OperationTimedoutException && ((OperationTimedoutException)e).isActive()) { logger.warn("Active migration command so scheduling a restart for {}", vm, e); _haMgr.scheduleRestart(vm, true); throw new AgentUnavailableException("Operation timed out on migrating " + vm, dstHostId); + } else { + _haMgr.scheduleRestart(vm, true); + + throw new AgentUnavailableException("Operation cancelled on migrating " + vm, dstHostId); } } } @@ -3552,7 +3557,7 @@ protected void createStoragePoolMappingsForVolumes(VirtualMachineProfile profile executeManagedStorageChecksWhenTargetStoragePoolNotProvided(targetHost, currentPool, volume); if (ScopeType.HOST.equals(currentPool.getScope()) || isStorageCrossClusterMigration(plan.getClusterId(), currentPool)) { createVolumeToStoragePoolMappingIfPossible(profile, plan, volumeToPoolObjectMap, volume, currentPool); - } else if (shouldMapVolume(profile, currentPool)){ + } else if (shouldMapVolume(profile, currentPool)) { volumeToPoolObjectMap.put(volume, currentPool); } } @@ -3589,7 +3594,7 @@ protected void executeManagedStorageChecksWhenTargetStoragePoolNotProvided(Host } /** - * Return true if the VM migration is a cross cluster migration. To execute that, we check if the volume current storage pool cluster is different from the target cluster. + * Return true if the VM migration is a cross-cluster migration. To execute that, we check if the volume current storage pool cluster is different from the target cluster. */ protected boolean isStorageCrossClusterMigration(Long clusterId, StoragePoolVO currentPool) { return clusterId != null && ScopeType.CLUSTER.equals(currentPool.getScope()) && !currentPool.getClusterId().equals(clusterId); @@ -3714,8 +3719,7 @@ public void migrateWithStorage(final String vmUuid, final long srcHostId, final } } - private void orchestrateMigrateWithStorage(final String vmUuid, final long srcHostId, final long destHostId, final Map volumeToPool) throws ResourceUnavailableException, - ConcurrentOperationException { + private void orchestrateMigrateWithStorage(final String vmUuid, final long srcHostId, final long destHostId, final Map volumeToPool) throws ResourceUnavailableException, ConcurrentOperationException { final VMInstanceVO vm = _vmDao.findByUuid(vmUuid); @@ -3795,8 +3799,7 @@ private void orchestrateMigrateWithStorage(final String vmUuid, final long srcHo _agentMgr.send(srcHost.getId(), dettachCommand); logger.debug("Deleted config drive ISO for vm {} in host {}", vm.getInstanceName(), srcHost); } catch (OperationTimedoutException e) { - logger.error("TIme out occurred while exeuting command AttachOrDettachConfigDrive {}", e.getMessage(), e); - + logger.error("TIme out occurred while executing command AttachOrDettachConfigDrive {}", e.getMessage(), e); } } } @@ -3817,7 +3820,7 @@ private void orchestrateMigrateWithStorage(final String vmUuid, final long srcHo String errorDetails = (cleanupResult.second() != null) ? " due to " + cleanupResult.second() : ""; throw new CloudRuntimeException("VM not found on destination host. Unable to complete migration for " + vm + errorDetails); } - } catch (final OperationTimedoutException e) { + } catch (OperationTimedoutException e) { logger.error("Error while checking the vm {} is on host {}", vm, destHost, e); } migrated = true; @@ -4121,7 +4124,7 @@ public void advanceReboot(final String vmUuid, final Map params) throws InsufficientCapacityException, ConcurrentOperationException, - ResourceUnavailableException { + private void orchestrateReboot(final String vmUuid, final Map params) throws ConcurrentOperationException, ResourceUnavailableException { final VMInstanceVO vm = _vmDao.findByUuid(vmUuid); if (_vmSnapshotMgr.hasActiveVMSnapshotTasks(vm.getId())) { logger.error("Unable to reboot Instance: {} due to: {} has active Instance Snapshot tasks", vm, vm.getInstanceName()); @@ -4187,8 +4189,8 @@ private void orchestrateReboot(final String vmUuid, final Map vmMetadatum) { boolean found = false; for(Pair, Pair> vmDetail : vmDetails ) { Pair vmNameTypePair = vmDetail.first(); - if(vmNameTypePair.first().equals(name)) { + if (vmNameTypePair.first().equals(name)) { found = true; - if(vmNameTypePair.second() == VirtualMachine.Type.User) { + if (vmNameTypePair.second() == VirtualMachine.Type.User) { Pair detailPair = vmDetail.second(); String platformDetail = detailPair.second(); @@ -4354,9 +4356,9 @@ public void syncVMMetaData(final Map vmMetadatum) { } } - if(!found) { + if (!found) { VMInstanceVO vm = _vmDao.findVMByInstanceName(name); - if(vm != null && vm.getType() == VirtualMachine.Type.User) { + if (vm != null && vm.getType() == VirtualMachine.Type.User) { updateVmMetaData(vm.getId(), platform); } } @@ -4366,12 +4368,12 @@ public void syncVMMetaData(final Map vmMetadatum) { private void updateVmMetaData(Long vmId, String platform) { UserVmVO userVm = _userVmDao.findById(vmId); _userVmDao.loadDetails(userVm); - if ( userVm.details.containsKey(VmDetailConstants.TIME_OFFSET)) { + if (userVm.details.containsKey(VmDetailConstants.TIME_OFFSET)) { userVm.details.remove(VmDetailConstants.TIME_OFFSET); } userVm.setDetail(VmDetailConstants.PLATFORM, platform); String pvdriver = "xenserver56"; - if ( platform.contains("device_id")) { + if (platform.contains("device_id")) { pvdriver = "xenserver61"; } if (!userVm.details.containsKey(VmDetailConstants.HYPERVISOR_TOOLS_VERSION) || !userVm.details.get(VmDetailConstants.HYPERVISOR_TOOLS_VERSION).equals(pvdriver)) { @@ -4388,7 +4390,7 @@ public boolean isRecurring() { @Override public boolean processAnswers(final long agentId, final long seq, final Answer[] answers) { for (final Answer answer : answers) { - if ( answer instanceof ClusterVMMetaDataSyncAnswer) { + if (answer instanceof ClusterVMMetaDataSyncAnswer) { final ClusterVMMetaDataSyncAnswer cvms = (ClusterVMMetaDataSyncAnswer)answer; if (!cvms.isExecuted()) { syncVMMetaData(cvms.getVMMetaDatum()); @@ -4578,7 +4580,7 @@ protected void checkIfNewOfferingStorageScopeMatchesStoragePool(VirtualMachine v public boolean isRootVolumeOnLocalStorage(long vmId) { ScopeType poolScope = ScopeType.ZONE; List volumes = _volsDao.findByInstanceAndType(vmId, Type.ROOT); - if(CollectionUtils.isNotEmpty(volumes)) { + if (CollectionUtils.isNotEmpty(volumes)) { VolumeVO rootDisk = volumes.get(0); Long poolId = rootDisk.getPoolId(); if (poolId != null) { @@ -4654,8 +4656,7 @@ private void checkIfNetworkExistsForUserVM(VirtualMachine virtualMachine, Networ } } - private NicProfile orchestrateAddVmToNetwork(final VirtualMachine vm, final Network network, final NicProfile requested) throws ConcurrentOperationException, ResourceUnavailableException, - InsufficientCapacityException { + private NicProfile orchestrateAddVmToNetwork(final VirtualMachine vm, final Network network, final NicProfile requested) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { final CallContext cctx = CallContext.current(); checkIfNetworkExistsForUserVM(vm, network); @@ -4688,7 +4689,7 @@ private NicProfile orchestrateAddVmToNetwork(final VirtualMachine vm, final Netw logger.debug("Nic is plugged successfully for vm {} in network {}. VM is a part of network now.", vm, network); final long isDefault = nic.isDefaultNic() ? 1 : 0; - if(VirtualMachine.Type.User.equals(vmVO.getType())) { + if (VirtualMachine.Type.User.equals(vmVO.getType())) { UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NETWORK_OFFERING_ASSIGN, vmVO.getAccountId(), vmVO.getDataCenterId(), vmVO.getId(), Long.toString(nic.getId()), network.getNetworkOfferingId(), null, isDefault, VirtualMachine.class.getName(), vmVO.getUuid(), vm.isDisplay()); } @@ -4875,8 +4876,7 @@ private boolean orchestrateRemoveVmFromNetwork(final VirtualMachine vm, final Ne } @Override - public void findHostAndMigrate(final String vmUuid, final Long newSvcOfferingId, final Map customParameters, final ExcludeList excludes) throws InsufficientCapacityException, ConcurrentOperationException, - ResourceUnavailableException { + public void findHostAndMigrate(final String vmUuid, final Long newSvcOfferingId, final Map customParameters, final ExcludeList excludes) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException { final VMInstanceVO vm = _vmDao.findByUuid(vmUuid); if (vm == null) { @@ -5028,7 +5028,7 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI throw new AgentUnavailableException(String.format("Unable to prepare for migration to destination host [%s] due to [%s].", dest.getHost(), details), dstHostId); } } catch (final OperationTimedoutException e1) { - throw new AgentUnavailableException("Operation timed out", dstHostId); + throw new AgentUnavailableException("Operation timed out ", dstHostId); } finally { if (pfma == null) { work.setStep(Step.Done); @@ -5091,7 +5091,7 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI String errorDetails = (cleanupResult.second() != null) ? " due to " + cleanupResult.second() : ""; throw new CloudRuntimeException("Unable to complete migration for " + vm + errorDetails); } - } catch (final OperationTimedoutException e) { + } catch (OperationTimedoutException e) { logger.debug("Error while checking the {} on {}", vm, dstHost, e); } @@ -5128,8 +5128,7 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI } @Override - public boolean replugNic(final Network network, final NicTO nic, final VirtualMachineTO vm, final Host host) throws ConcurrentOperationException, - ResourceUnavailableException, InsufficientCapacityException { + public boolean replugNic(final Network network, final NicTO nic, final VirtualMachineTO vm, final Host host) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { boolean result = true; final VMInstanceVO router = _vmDao.findById(vm.getId()); @@ -5144,7 +5143,7 @@ public boolean replugNic(final Network network, final NicTO nic, final VirtualMa logger.warn("Unable to replug nic for vm {}", vm.getName()); result = false; } - } catch (final OperationTimedoutException e) { + } catch (OperationTimedoutException e) { throw new AgentUnavailableException("Unable to plug nic for router " + vm.getName() + " in network " + network, host.getId(), e); } } else { @@ -5157,8 +5156,7 @@ public boolean replugNic(final Network network, final NicTO nic, final VirtualMa return result; } - public boolean plugNic(final Network network, final NicTO nic, final VirtualMachineTO vm, final ReservationContext context, final DeployDestination dest) throws ConcurrentOperationException, - ResourceUnavailableException, InsufficientCapacityException { + public boolean plugNic(final Network network, final NicTO nic, final VirtualMachineTO vm, final ReservationContext context, final DeployDestination dest) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { boolean result = true; final VMInstanceVO router = _vmDao.findById(vm.getId()); @@ -5180,7 +5178,7 @@ public boolean plugNic(final Network network, final NicTO nic, final VirtualMach logger.warn("Unable to plug nic for vm {}", vm.getName()); result = false; } - } catch (final OperationTimedoutException e) { + } catch (OperationTimedoutException e) { throw new AgentUnavailableException("Unable to plug nic for router " + vm.getName() + " in network " + network, dest.getHost().getId(), e); } } else { @@ -5194,8 +5192,7 @@ public boolean plugNic(final Network network, final NicTO nic, final VirtualMach return result; } - public boolean unplugNic(final Network network, final NicTO nic, final VirtualMachineTO vm, final ReservationContext context, final DeployDestination dest) throws ConcurrentOperationException, - ResourceUnavailableException { + public boolean unplugNic(final Network network, final NicTO nic, final VirtualMachineTO vm, final ReservationContext context, final DeployDestination dest) throws ConcurrentOperationException, ResourceUnavailableException { boolean result = true; final VMInstanceVO router = _vmDao.findById(vm.getId()); @@ -5220,7 +5217,7 @@ public boolean unplugNic(final Network network, final NicTO nic, final VirtualMa logger.warn("Unable to unplug nic from router {}", router); result = false; } - } catch (final OperationTimedoutException e) { + } catch (OperationTimedoutException e) { throw new AgentUnavailableException("Unable to unplug nic from rotuer " + router + " from network " + network, dest.getHost().getId(), e); } } else if (router.getState() == State.Stopped || router.getState() == State.Stopping) { @@ -5463,9 +5460,6 @@ private ApiCommandResourceType getApiCommandResourceTypeForVm(VirtualMachine vm) } private void handlePowerOnReportWithNoPendingJobsOnVM(final VMInstanceVO vm) { - Host host = _hostDao.findById(vm.getHostId()); - Host poweredHost = _hostDao.findById(vm.getPowerHostId()); - switch (vm.getState()) { case Starting: logger.info("VM {} is at {} and we received a power-on report while there is no pending jobs on it.", vm.getInstanceName(), vm.getState()); @@ -5488,6 +5482,7 @@ private void handlePowerOnReportWithNoPendingJobsOnVM(final VMInstanceVO vm) { if (vm.getHostId() != null && !vm.getHostId().equals(vm.getPowerHostId())) { logger.info("Detected out of band VM migration from host {} to host {}", () -> _hostDao.findById(vm.getHostId()), () -> _hostDao.findById(vm.getPowerHostId())); } + stateTransitTo(vm, VirtualMachine.Event.FollowAgentPowerOnReport, vm.getPowerHostId()); } catch (final NoTransitionException e) { logger.warn("Unexpected VM state transition exception, race-condition?", e); @@ -5546,7 +5541,7 @@ private void handlePowerOffReportWithNoPendingJobsOnVM(final VMInstanceVO vm) { case Migrating: logger.info("VM {} is at {} and we received a {} report while there is no pending jobs on it" , vm, vm.getState(), vm.getPowerState()); - if((HighAvailabilityManager.ForceHA.value() || vm.isHaEnabled()) && vm.getState() == State.Running + if ((HighAvailabilityManager.ForceHA.value() || vm.isHaEnabled()) && vm.getState() == State.Running && HaVmRestartHostUp.value() && vm.getHypervisorType() != HypervisorType.VMware && vm.getHypervisorType() != HypervisorType.Hyperv) { @@ -5832,7 +5827,7 @@ public Outcome migrateVmThroughJobQueue(final String vmUuid, fin Map volumeStorageMap = dest.getStorageForDisks(); if (volumeStorageMap != null) { for (Volume vol : volumeStorageMap.keySet()) { - checkConcurrentJobsPerDatastoreThreshhold(volumeStorageMap.get(vol)); + checkConcurrentJobsPerDatastoreThreshold(volumeStorageMap.get(vol)); } } @@ -5927,7 +5922,7 @@ public Outcome migrateVmForScaleThroughJobQueue( return new VmJobVirtualMachineOutcome(workJob, vmId); } - private void checkConcurrentJobsPerDatastoreThreshhold(final StoragePool destPool) { + private void checkConcurrentJobsPerDatastoreThreshold(final StoragePool destPool) { final Long threshold = VolumeApiService.ConcurrentMigrationsThresholdPerDatastore.value(); if (threshold != null && threshold > 0) { long count = _jobMgr.countPendingJobs("\"storageid\":\"" + destPool.getUuid() + "\"", MigrateVMCmd.class.getName(), MigrateVolumeCmd.class.getName(), MigrateVolumeCmdByAdmin.class.getName()); @@ -5942,7 +5937,7 @@ public Outcome migrateVmStorageThroughJobQueue(final String vmUu Set uniquePoolIds = new HashSet<>(poolIds); for (Long poolId : uniquePoolIds) { StoragePoolVO pool = _storagePoolDao.findById(poolId); - checkConcurrentJobsPerDatastoreThreshhold(pool); + checkConcurrentJobsPerDatastoreThreshold(pool); } String commandName = VmWorkStorageMigration.class.getName(); @@ -6116,7 +6111,7 @@ private Pair orchestrateStart(final VmWorkStart work) th try { orchestrateStart(vm.getUuid(), work.getParams(), work.getPlan(), _dpMgr.getDeploymentPlannerByName(work.getDeploymentPlanner())); - } catch (CloudRuntimeException e){ + } catch (CloudRuntimeException e) { logger.error("Unable to orchestrate start {} due to [{}].", vm, e.getMessage()); CloudRuntimeException ex = new CloudRuntimeException(String.format("Unable to orchestrate the start of VM instance %s.", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(vm, "instanceName", "uuid"))); @@ -6258,7 +6253,7 @@ private VmWorkJobVO createPlaceHolderWork(final long instanceId, String secondar workJob.setStep(VmWorkJobVO.Step.Starting); workJob.setVmType(VirtualMachine.Type.Instance); workJob.setVmInstanceId(instanceId); - if(org.apache.commons.lang3.StringUtils.isNotBlank(secondaryObjectIdentifier)) { + if (org.apache.commons.lang3.StringUtils.isNotBlank(secondaryObjectIdentifier)) { workJob.setSecondaryObjectIdentifier(secondaryObjectIdentifier); } workJob.setInitMsid(ManagementServerNode.getManagementServerId()); @@ -6451,7 +6446,7 @@ private boolean orchestrateUpdateVmNic(final VirtualMachine vm, final Nic nic, f logger.warn("Unable to update VM %s NIC [{}].", vm.getName(), nic.getUuid()); return false; } - } catch (final OperationTimedoutException e) { + } catch (OperationTimedoutException e) { throw new AgentUnavailableException(String.format("Unable to update NIC %s for VM %s.", nic.getUuid(), vm.getUuid()), vm.getHostId(), e); } } @@ -6569,7 +6564,7 @@ private void executePreMigrationCommand(VMInstanceVO vm, VirtualMachineTO to, lo throw new CloudRuntimeException(msg); } logger.info("Successfully prepared source host {} for migration of VM {}", srcHostUuid, vmInstanceName); - } catch (final AgentUnavailableException | OperationTimedoutException e) { + } catch (AgentUnavailableException | OperationTimedoutException e) { logger.error("Failed to send PreMigrationCommand to source host {}: {}", srcHostUuid, e.getMessage(), e); throw new CloudRuntimeException("Failed to prepare source host for migration: " + e.getMessage(), e); } diff --git a/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java index 2377bbaefbc8..290a19890468 100644 --- a/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java @@ -31,13 +31,17 @@ import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.Pair; +import org.apache.cloudstack.framework.jobs.AsyncJobManager; +import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; public class AgentManagerImplTest { @@ -172,4 +176,41 @@ public void testGetHostSshPortWithKVMHostCustomPort() { int hostSshPort = mgr.getHostSshPort(host); Assert.assertEquals(3922, hostSshPort); } + + private AsyncJobManager cancelledJobOnThisServer(final long jobId) { + final AsyncJobManager asyncJobManager = Mockito.mock(AsyncJobManager.class); + mgr.asyncJobManager = asyncJobManager; + final AsyncJobVO job = new AsyncJobVO(); + job.setId(jobId); + Mockito.when(asyncJobManager.listCancelledJobsExecutingOn(mgr._nodeId)).thenReturn(Collections.singletonList(job)); + return asyncJobManager; + } + + @Test + public void testCancelledJobWithoutInFlightCommandsIsFinalisedAtOnce() { + final AsyncJobManager asyncJobManager = cancelledJobOnThisServer(42L); + + mgr.new CancelledJobsCheckTask().runInContext(); + + Mockito.verify(asyncJobManager).finalizeCancelledJob(42L); + Assert.assertTrue(mgr.isJobCancelled(42L)); + } + + @Test + public void testCancelledJobStaysAssignedUntilItsCommandCanBeStopped() throws Exception { + final AsyncJobManager asyncJobManager = cancelledJobOnThisServer(42L); + final AgentAttache attache = Mockito.mock(AgentAttache.class); + Mockito.doReturn(attache).when(mgr).getAttache(1L); + mgr._jobToHostIdAndReqSequenceMap.put(42L, new HashSet<>(Collections.singleton(new Pair<>(1L, 11L)))); + + Mockito.when(attache.isExecutionCancellable(11L)).thenReturn(false); + mgr.new CancelledJobsCheckTask().runInContext(); + Mockito.verify(asyncJobManager, Mockito.never()).finalizeCancelledJob(Mockito.anyLong()); + Mockito.verify(attache, Mockito.never()).cancelExecution(Mockito.anyLong()); + + Mockito.when(attache.isExecutionCancellable(11L)).thenReturn(true); + Mockito.when(attache.cancelExecution(11L)).thenReturn(true); + mgr.new CancelledJobsCheckTask().runInContext(); + Mockito.verify(asyncJobManager).finalizeCancelledJob(42L); + } } diff --git a/engine/orchestration/src/test/java/com/cloud/agent/manager/ConnectedAgentAttacheTest.java b/engine/orchestration/src/test/java/com/cloud/agent/manager/ConnectedAgentAttacheTest.java index 72b217d0fdd9..62b8ce3e5393 100644 --- a/engine/orchestration/src/test/java/com/cloud/agent/manager/ConnectedAgentAttacheTest.java +++ b/engine/orchestration/src/test/java/com/cloud/agent/manager/ConnectedAgentAttacheTest.java @@ -19,8 +19,18 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CancelCommand; +import com.cloud.agent.api.Command; +import com.cloud.agent.api.UnsupportedAnswer; +import com.cloud.exception.AgentUnavailableException; import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.nio.Link; @@ -80,4 +90,60 @@ public void testEqualsFalseDiffClass() throws Exception { assertFalse(agentAttache1.equals("abc")); } + + @Test + public void isExecutionCancellableAsksTheAgentWithoutCancelling() throws Exception { + AgentManagerImpl agentMgr = mock(AgentManagerImpl.class); + ConnectedAgentAttache attache = new ConnectedAgentAttache(agentMgr, 5L, "uuid", "host", Hypervisor.HypervisorType.KVM, mock(Link.class), false); + when(agentMgr.send(Mockito.eq(5L), Mockito.any(Command.class))).thenReturn(new Answer(null, true, "can")); + + assertTrue(attache.isExecutionCancellable(77L)); + + ArgumentCaptor sent = ArgumentCaptor.forClass(Command.class); + verify(agentMgr).send(Mockito.eq(5L), sent.capture()); + CancelCommand cancel = (CancelCommand) sent.getValue(); + assertTrue(cancel.isCheckOnly()); + assertTrue(77L == cancel.getSequence()); + assertFalse(cancel.executeInSequence()); + } + + @Test + public void cancelRunningSendsARealCancelAndReportsTheAgentsAnswer() throws Exception { + AgentManagerImpl agentMgr = mock(AgentManagerImpl.class); + ConnectedAgentAttache attache = new ConnectedAgentAttache(agentMgr, 5L, "uuid", "host", Hypervisor.HypervisorType.KVM, mock(Link.class), false); + when(agentMgr.send(Mockito.eq(5L), Mockito.any(Command.class))).thenReturn(new Answer(null, false, "nothing to stop")); + + assertFalse(attache.cancelRunning(77L)); + + ArgumentCaptor sent = ArgumentCaptor.forClass(Command.class); + verify(agentMgr).send(Mockito.eq(5L), sent.capture()); + assertFalse(((CancelCommand) sent.getValue()).isCheckOnly()); + } + + @Test + public void anAgentThatDoesNotUnderstandCancellationIsNotCancellable() throws Exception { + AgentManagerImpl agentMgr = mock(AgentManagerImpl.class); + ConnectedAgentAttache attache = new ConnectedAgentAttache(agentMgr, 5L, "uuid", "host", Hypervisor.HypervisorType.KVM, mock(Link.class), false); + when(agentMgr.send(Mockito.eq(5L), Mockito.any(Command.class))).thenReturn(new UnsupportedAnswer(null, "unsupported")); + + assertFalse(attache.isExecutionCancellable(77L)); + assertFalse(attache.cancelRunning(77L)); + } + + @Test + public void anUnreachableAgentIsNotCancellable() throws Exception { + AgentManagerImpl agentMgr = mock(AgentManagerImpl.class); + ConnectedAgentAttache attache = new ConnectedAgentAttache(agentMgr, 5L, "uuid", "host", Hypervisor.HypervisorType.KVM, mock(Link.class), false); + when(agentMgr.send(Mockito.eq(5L), Mockito.any(Command.class))).thenThrow(new AgentUnavailableException(5L)); + + assertFalse(attache.isExecutionCancellable(77L)); + } + + @Test + public void withoutAnAgentManagerNothingIsAsked() { + ConnectedAgentAttache attache = new ConnectedAgentAttache(null, 5L, "uuid", "host", Hypervisor.HypervisorType.KVM, mock(Link.class), false); + + assertFalse(attache.isExecutionCancellable(77L)); + assertFalse(attache.cancelRunning(77L)); + } } diff --git a/engine/orchestration/src/test/java/com/cloud/agent/manager/DirectAgentAttacheTest.java b/engine/orchestration/src/test/java/com/cloud/agent/manager/DirectAgentAttacheTest.java index 4ba276460e3a..e5529e3a55d6 100644 --- a/engine/orchestration/src/test/java/com/cloud/agent/manager/DirectAgentAttacheTest.java +++ b/engine/orchestration/src/test/java/com/cloud/agent/manager/DirectAgentAttacheTest.java @@ -16,19 +16,24 @@ // under the License. package com.cloud.agent.manager; +import java.util.UUID; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; + +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; -import org.mockito.MockitoAnnotations; import org.mockito.junit.MockitoJUnitRunner; +import com.cloud.agent.Listener; +import com.cloud.agent.api.CheckHealthCommand; +import com.cloud.agent.transport.Request; import com.cloud.hypervisor.Hypervisor; import com.cloud.resource.ServerResource; -import java.util.UUID; - @RunWith(MockitoJUnitRunner.class) public class DirectAgentAttacheTest { @Mock @@ -37,17 +42,22 @@ public class DirectAgentAttacheTest { @Mock private ServerResource _resource; + @Mock + private ScheduledExecutorService directAgentPool; + + @Mock + private Future runningTask; + long _id = 0L; String _uuid = UUID.randomUUID().toString(); + private DirectAgentAttache directAgentAttache; + @Before public void setup() { directAgentAttache = new DirectAgentAttache(_agentMgr, _id, _uuid, "myDirectAgentAttache", Hypervisor.HypervisorType.KVM, _resource, false); - - MockitoAnnotations.initMocks(directAgentAttache); } - private DirectAgentAttache directAgentAttache; @Test public void testPingTask() throws Exception { @@ -56,4 +66,78 @@ public void testPingTask() throws Exception { pt.runInContext(); Mockito.verify(_resource, Mockito.times(1)).getCurrentStatus(_id); } + + @Test + public void testCancelQueuedTaskDropsItBeforeItReachesTheResource() throws Exception { + // no free thread, so the task stays queued + Mockito.doReturn(0).when(_agentMgr).getDirectAgentThreadCap(); + final Request request = newRequest(202L); + + directAgentAttache.send(request, null); + Assert.assertEquals(1, directAgentAttache.tasks.size()); + Assert.assertTrue(directAgentAttache.isExecutionCancellable(202L)); + + directAgentAttache.cancelExecution(202L); + + Assert.assertTrue(request.isCancelled()); + Assert.assertEquals(0, directAgentAttache.tasks.size()); + Mockito.verify(_resource, Mockito.never()).cancelRequestSequence(Mockito.anyLong()); + } + + @Test + public void testCancelRunningTaskStopsTheResourceThenTheThread() throws Exception { + final Request request = submitRunning(101L); + directAgentAttache.registerListener(101L, waiter()); + Mockito.doReturn(true).when(_resource).isRequestSequenceCancellable(101L); + Mockito.doReturn(true).when(_resource).cancelRequestSequence(101L); + + Assert.assertTrue(directAgentAttache.isExecutionCancellable(101L)); + Assert.assertTrue(directAgentAttache.cancelExecution(101L)); + + Mockito.verify(_resource).cancelRequestSequence(101L); + Mockito.verify(runningTask).cancel(true); + Assert.assertTrue(request.isCancelled()); + Assert.assertFalse(directAgentAttache._waitForList.containsKey(101L)); + } + + @Test + public void testCancelLeavesNonCancellableRunningTaskAlone() throws Exception { + final Request request = submitRunning(303L); + directAgentAttache.registerListener(303L, waiter()); + Mockito.doReturn(false).when(_resource).isRequestSequenceCancellable(303L); + + Assert.assertFalse(directAgentAttache.isExecutionCancellable(303L)); + Assert.assertFalse(directAgentAttache.cancelExecution(303L)); + + Mockito.verify(_resource, Mockito.never()).cancelRequestSequence(Mockito.anyLong()); + Mockito.verify(runningTask, Mockito.never()).cancel(Mockito.anyBoolean()); + Assert.assertFalse(request.isCancelled()); + Assert.assertTrue(directAgentAttache._waitForList.containsKey(303L)); + } + + @Test + public void testUnknownSequenceIsNotClaimedCancellable() { + Assert.assertFalse(directAgentAttache.isExecutionCancellable(999L)); + } + + private Request newRequest(final long seq) { + final Request request = new Request(_id, -1, new CheckHealthCommand(), false); + request.setSequence(seq); + return request; + } + + private Listener waiter() { + final Listener listener = Mockito.mock(Listener.class); + Mockito.when(listener.getTimeout()).thenReturn(-1); + return listener; + } + + private Request submitRunning(final long seq) throws Exception { + Mockito.doReturn(2).when(_agentMgr).getDirectAgentThreadCap(); + Mockito.doReturn(directAgentPool).when(_agentMgr).getDirectAgentPool(); + Mockito.doReturn(runningTask).when(directAgentPool).submit(Mockito.any(Runnable.class)); + final Request request = newRequest(seq); + directAgentAttache.send(request, null); + return request; + } } diff --git a/engine/schema/src/main/java/com/cloud/event/EventVO.java b/engine/schema/src/main/java/com/cloud/event/EventVO.java index 24c3e8cd0641..47c5cf36d383 100644 --- a/engine/schema/src/main/java/com/cloud/event/EventVO.java +++ b/engine/schema/src/main/java/com/cloud/event/EventVO.java @@ -73,6 +73,9 @@ public class EventVO implements Event { @Column(name = "start_id") private long startId; + @Column(name = "async_job_id") + private Long asyncJobId; + @Column(name = "parameters", length = 1024) private String parameters; @@ -209,6 +212,14 @@ public void setStartId(long startId) { this.startId = startId; } + public Long getAsyncJobId() { + return asyncJobId; + } + + public void setAsyncJobId(Long asyncJobId) { + this.asyncJobId = asyncJobId; + } + @Override public String getParameters() { return parameters; diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42300to2400.sql b/engine/schema/src/main/resources/META-INF/db/schema-42300to2400.sql index 7c11013a17d2..085122899248 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-42300to2400.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-42300to2400.sql @@ -18,3 +18,7 @@ --; -- Schema upgrade from 4.23.0.0 to 24.0.0 --; + +-- Link every action event to the API job that raised it, so a job can be traced to its events +CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.event', 'async_job_id', 'BIGINT UNSIGNED DEFAULT NULL COMMENT ''The async job that raised this event'' '); +CALL `cloud`.`IDEMPOTENT_ADD_KEY`('i_event__async_job_id', 'cloud.event', '(`async_job_id`)'); diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.async_job_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.async_job_view.sql index 8e941a04e8ea..0ac08bf40a63 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.async_job_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.async_job_view.sql @@ -33,6 +33,7 @@ select user.uuid user_uuid, async_job.id, async_job.uuid, + async_job.related, async_job.job_cmd, async_job.job_status, async_job.job_process_status, @@ -43,6 +44,7 @@ select async_job.instance_type, async_job.instance_id, async_job.job_executing_msid, + async_job.job_complete_msid, CASE WHEN async_job.instance_type = 'Volume' THEN volumes.uuid WHEN diff --git a/engine/schema/src/main/resources/META-INF/db/views/cloud.event_view.sql b/engine/schema/src/main/resources/META-INF/db/views/cloud.event_view.sql index 0a15ae4c0c91..1066f567e95e 100644 --- a/engine/schema/src/main/resources/META-INF/db/views/cloud.event_view.sql +++ b/engine/schema/src/main/resources/META-INF/db/views/cloud.event_view.sql @@ -34,6 +34,8 @@ select `event`.`parameters` AS `parameters`, `event`.`start_id` AS `start_id`, `eve`.`uuid` AS `start_uuid`, + `event`.`async_job_id` AS `async_job_id`, + `async_job`.`uuid` AS `async_job_uuid`, `event`.`user_id` AS `user_id`, `event`.`archived` AS `archived`, `event`.`display` AS `display`, @@ -50,7 +52,7 @@ select `projects`.`uuid` AS `project_uuid`, `projects`.`name` AS `project_name` from - (((((`event` + ((((((`event` join `account` on ((`event`.`account_id` = `account`.`id`))) join `domain` on @@ -60,4 +62,6 @@ join `user` on left join `projects` on ((`projects`.`project_account_id` = `event`.`account_id`))) left join `event` `eve` on - ((`event`.`start_id` = `eve`.`id`))); + ((`event`.`start_id` = `eve`.`id`))) +left join `async_job` on + ((`event`.`async_job_id` = `async_job`.`id`))); diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJobExecutionContext.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJobExecutionContext.java index 465a80b62c7f..2d39e18edea3 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJobExecutionContext.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJobExecutionContext.java @@ -128,7 +128,7 @@ public void disjoinJob(long joinedJobId) throws InsufficientCapacityException, AsyncJobJoinMapVO record = s_joinMapDao.getJoinRecord(_job.getId(), joinedJobId); s_jobMgr.disjoinJob(_job.getId(), joinedJobId); - if (record.getJoinStatus() == JobInfo.Status.FAILED) { + if (record != null && record.getJoinStatus() == JobInfo.Status.FAILED) { if (record.getJoinResult() != null) { Object exception = JobSerializerHelper.fromObjectSerializedString(record.getJoinResult()); if (exception != null && exception instanceof Exception) { diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJobManager.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJobManager.java index 9f50a545efc7..274c1f282719 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJobManager.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/AsyncJobManager.java @@ -40,6 +40,13 @@ public interface AsyncJobManager extends Manager { void completeAsyncJob(long jobId, JobInfo.Status jobStatus, int resultCode, String result); + /** With remove=false the row keeps executing_msid until the executing server has stopped the job's work and called finalizeCancelledJob. */ + void completeAsyncJob(long jobId, JobInfo.Status jobStatus, int resultCode, String result, boolean remove); + + List listCancelledJobsExecutingOn(long msid); + + void finalizeCancelledJob(long jobId); + void updateAsyncJobStatus(long jobId, int processStatus, String resultObject); void updateAsyncJobAttachment(long jobId, String instanceType, Long instanceId); diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDao.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDao.java index c334e91feb84..fcd201790fdc 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDao.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDao.java @@ -70,4 +70,10 @@ public interface AsyncJobDao extends GenericDao { long countPendingNonPseudoJobs(Long... msIds); List listPendingJobIdsForAccount(long accountId); + + AsyncJobVO getRelatedJob(String jobId); + + List listChildJobs(long parentJobId); + + List getCancelledJobs(long executingMsid); } diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java index d8385e9aecd1..846a832c9331 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/dao/AsyncJobDaoImpl.java @@ -50,6 +50,8 @@ public class AsyncJobDaoImpl extends GenericDaoBase implements private final SearchBuilder byIdResourceIdResourceTypeSearch; private final GenericSearchBuilder asyncJobTypeSearch; private final GenericSearchBuilder pendingNonPseudoAsyncJobsSearch; + private final SearchBuilder relatedAsyncJobSearch; + private final SearchBuilder cancelledAsyncJobSearch; public AsyncJobDaoImpl() { pendingAsyncJobSearch = createSearchBuilder(); @@ -116,6 +118,16 @@ public AsyncJobDaoImpl() { pendingNonPseudoAsyncJobsSearch.and("instanceTypeNEQ", pendingNonPseudoAsyncJobsSearch.entity().getInstanceType(), SearchCriteria.Op.NEQ); pendingNonPseudoAsyncJobsSearch.and("jobStatusEQ", pendingNonPseudoAsyncJobsSearch.entity().getStatus(), SearchCriteria.Op.EQ); pendingNonPseudoAsyncJobsSearch.and("executingMsidIN", pendingNonPseudoAsyncJobsSearch.entity().getExecutingMsid(), SearchCriteria.Op.IN); + + relatedAsyncJobSearch = createSearchBuilder(); + relatedAsyncJobSearch.and("related", relatedAsyncJobSearch.entity().getRelated(), SearchCriteria.Op.EQ); + relatedAsyncJobSearch.done(); + + cancelledAsyncJobSearch = createSearchBuilder(); + cancelledAsyncJobSearch.and("status", cancelledAsyncJobSearch.entity().getStatus(), SearchCriteria.Op.EQ); + cancelledAsyncJobSearch.and("completeMsId", cancelledAsyncJobSearch.entity().getCompleteMsid(), SearchCriteria.Op.NULL); + cancelledAsyncJobSearch.and("executingMsid", cancelledAsyncJobSearch.entity().getExecutingMsid(), SearchCriteria.Op.EQ); + cancelledAsyncJobSearch.done(); } @Override @@ -309,4 +321,26 @@ public List listPendingJobIdsForAccount(long accountId) { sc.setParameters("accountId", accountId); return customSearch(sc, null); } + + @Override + public AsyncJobVO getRelatedJob(String jobId) { + SearchCriteria sc = relatedAsyncJobSearch.create(); + sc.setParameters("related", jobId); + return findOneIncludingRemovedBy(sc); + } + + @Override + public List listChildJobs(long parentJobId) { + SearchCriteria sc = relatedAsyncJobSearch.create(); + sc.setParameters("related", String.valueOf(parentJobId)); + return listBy(sc); + } + + @Override + public List getCancelledJobs(long executingMsid) { + SearchCriteria sc = cancelledAsyncJobSearch.create(); + sc.setParameters("status", JobInfo.Status.CANCELLED); + sc.setParameters("executingMsid", executingMsid); + return listBy(sc); + } } diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java index 1cb1cb4e309f..9d8cfb82f13e 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java @@ -20,6 +20,7 @@ import static com.cloud.utils.HumanReadableJson.getHumanReadableBytesJson; import java.io.Serializable; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Date; @@ -35,8 +36,10 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.command.user.job.CancelAsyncJobCmd; import org.apache.cloudstack.command.ReconcileCommandService; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; @@ -60,6 +63,8 @@ import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.framework.messagebus.MessageDetector; import org.apache.cloudstack.framework.messagebus.PublishScope; +import org.apache.cloudstack.jobs.AsyncJobService; +import org.apache.cloudstack.jobs.JobCancellationHandler; import org.apache.cloudstack.jobs.JobInfo; import org.apache.cloudstack.jobs.JobInfo.Status; import org.apache.cloudstack.managed.context.ManagedContextRunnable; @@ -114,7 +119,7 @@ import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; -public class AsyncJobManagerImpl extends ManagerBase implements AsyncJobManager, ClusterManagerListener, Configurable { +public class AsyncJobManagerImpl extends ManagerBase implements AsyncJobManager, ClusterManagerListener, Configurable, AsyncJobService { // Advanced public static final ConfigKey JobExpireMinutes = new ConfigKey("Advanced", Long.class, "job.expire.minutes", "1440", @@ -180,6 +185,9 @@ public class AsyncJobManagerImpl extends ManagerBase implements AsyncJobManager, private NetworkOrchestrationService networkOrchestrationService; @Inject private ReconcileCommandService reconcileCommandService; + // a list: Spring leaves it empty when no agent layer is present + @Inject + private List jobCancellationHandlers; private volatile long _executionRunNumber = 1; @@ -198,7 +206,7 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {JobExpireMinutes, JobCancelThresholdMinutes, VmJobLockTimeout, HidePassword}; + return new ConfigKey[] {JobExpireMinutes, JobCancelThresholdMinutes, VmJobLockTimeout, HidePassword, CancelledJobInterval}; } @Override @@ -336,6 +344,12 @@ public Long doInTransaction(TransactionStatus status) { @Override @DB public void completeAsyncJob(final long jobId, final Status jobStatus, final int resultCode, final String resultObject) { + completeAsyncJob(jobId, jobStatus, resultCode, resultObject, true); + } + + @Override + @DB + public void completeAsyncJob(final long jobId, final Status jobStatus, final int resultCode, final String resultObject, final boolean remove) { String resultObj = null; if (logger.isDebugEnabled()) { resultObj = convertHumanReadableJson(obfuscatePassword(resultObject, HidePassword.value())); @@ -380,7 +394,6 @@ public List doInTransaction(final TransactionStatus status) { if (logger.isDebugEnabled()) { logger.debug("Update db status for job-" + jobId); } - job.setCompleteMsid(getMsid()); job.setStatus(jobStatus); job.setResultCode(resultCode); @@ -392,8 +405,11 @@ public List doInTransaction(final TransactionStatus status) { final Date currentGMTTime = DateUtil.currentGMTTime(); job.setLastUpdated(currentGMTTime); - job.setRemoved(currentGMTTime); - job.setExecutingMsid(null); + if (remove) { + job.setCompleteMsid(getMsid()); + job.setRemoved(currentGMTTime); + job.setExecutingMsid(null); + } _jobDao.update(jobId, job); if (logger.isDebugEnabled()) { @@ -902,6 +918,9 @@ protected void reallyRun() { List l = _queueMgr.dequeueFromAny(getMsid(), MAX_ONETIME_SCHEDULE_SIZE); if (l != null && l.size() > 0) { for (SyncQueueItemVO item : l) { + if (isChildOfFinishedJob(item)) { + continue; + } if (logger.isDebugEnabled()) { logger.debug("Execute sync-queue item: " + item.toString()); } @@ -913,8 +932,13 @@ protected void reallyRun() { for (Long jobId : standaloneWakeupJobs) { // TODO, we assume that all jobs in this category is API job only AsyncJobVO job = _jobDao.findById(jobId); - if (job != null && (job.getPendingSignals() & AsyncJob.Constants.SIGNAL_MASK_WAKEUP) != 0) - scheduleExecution(job, false); + if (job != null && (job.getPendingSignals() & AsyncJob.Constants.SIGNAL_MASK_WAKEUP) != 0) { + if (job.getStatus() == Status.CANCELLED) { + finalizeCancelledJob(job.getId()); + } else { + scheduleExecution(job, false); + } + } } } catch (Throwable e) { logger.error("Unexpected exception when trying to execute queue item, ", e); @@ -953,7 +977,7 @@ public void reallyRun() { try { if (item.getContentType().equalsIgnoreCase(SyncQueueItem.AsyncJobContentType)) { logger.info("Remove Job-" + item.getContentId() + " from Queue-" + item.getId() + " since it has been blocked for too long"); - completeAsyncJob(item.getContentId(), JobInfo.Status.FAILED, 0, "Job is cancelled as it has been blocking others for too long"); + completeAsyncJob(item.getContentId(), JobInfo.Status.CANCELLED, 0, "Job is cancelled as it has been blocking others for too long"); _jobMonitor.unregisterByJobId(item.getContentId()); } @@ -1453,6 +1477,161 @@ public boolean stop() { return true; } + void cancelQueuedChildJobs(final long parentJobId, final String reason) { + for (final AsyncJobVO child : _jobDao.listChildJobs(parentJobId)) { + if (child.getStatus() != null && child.getStatus().done()) { + continue; + } + final Long queueItemId = _queueItemDao.getQueueItemIdByContentIdAndType(child.getId(), SyncQueueItem.AsyncJobContentType); + if (queueItemId == null) { + continue; + } + final SyncQueueItemVO item = _queueItemDao.findById(queueItemId); + if (item == null || item.getLastProcessMsid() != null) { + // dequeued already: it is executing, and the in-flight path owns it + continue; + } + logger.info("Cancelling queued job-{}, its parent job-{} was cancelled before it was scheduled", child.getId(), parentJobId); + completeAsyncJob(child.getId(), JobInfo.Status.CANCELLED, 0, "Job is cancelled due to " + reason + " (parent job cancelled before this job was scheduled)"); + _jobMonitor.unregisterByJobId(child.getId()); + _queueMgr.purgeItem(queueItemId); + } + } + + /** A queued VM work job whose parent already finished is completed with the parent's status instead of run. */ + private boolean isChildOfFinishedJob(final SyncQueueItemVO item) { + if (!SyncQueueItem.AsyncJobContentType.equalsIgnoreCase(item.getContentType())) { + return false; + } + final AsyncJobVO job = _jobDao.findById(item.getContentId()); + if (job == null || StringUtils.isBlank(job.getRelated())) { + return false; + } + final AsyncJobVO parentJob = _jobDao.findById(Long.parseLong(job.getRelated())); + if (parentJob == null || !parentJob.getStatus().done() || isPseudoJob(parentJob)) { + return false; + } + + logger.debug("Not executing sync-queue item {}: parent job-{} is already {}", item, parentJob.getId(), parentJob.getStatus()); + completeAsyncJob(item.getContentId(), parentJob.getStatus(), 0, + "Job is not scheduled for execution as the parent job is done. Parent job state: " + parentJob.getStatus()); + _jobMonitor.unregisterByJobId(item.getContentId()); + _queueMgr.purgeItem(item.getId()); + if (parentJob.getStatus() == Status.CANCELLED) { + finalizeCancelledJob(parentJob.getId()); + } + return true; + } + + private boolean isPseudoJob(final AsyncJob job) { + return AsyncJobVO.JOB_DISPATCHER_PSEUDO.equals(job.getDispatcher()) && AsyncJobVO.PSEUDO_JOB_INSTANCE_TYPE.equals(job.getInstanceType()); + } + + @Override + public String cancelAsyncJob(final long jobId, final String reason) { + final AsyncJobVO job = _jobDao.findByIdIncludingRemoved(jobId); + String errMessage; + if (job == null) { + errMessage = "Cannot cancel, job no longer exists."; + logger.debug(errMessage); + _queueMgr.purgeAsyncJobQueueItemId(jobId); + return errMessage; + } + + if (job.getExecutingMsid() != null || isActiveJob(jobId)) { + try { + final Class cmdClass = Class.forName(job.getCmd()); + final APICommand apiCommand = cmdClass.getAnnotation(APICommand.class); + if (apiCommand == null || !apiCommand.cancellable()) { + errMessage = "Cannot cancel, job " + job.getUuid() + " is not cancellable."; + logger.debug(errMessage); + return errMessage; + } + } catch (final ClassNotFoundException e) { + errMessage = "Command " + job.getCmd() + " of jobid " + job.getUuid() + " not found."; + logger.error(errMessage, e); + return errMessage; + } + } + + if (job.getStatus() != null && job.getStatus().done()) { + errMessage = "Cannot cancel, job-" + jobId + " is not running. Current job status is " + job.getStatus() + "."; + logger.debug(errMessage); + _queueMgr.purgeAsyncJobQueueItemId(jobId); + return errMessage; + } + + // ask before changing state: a job whose work cannot be stopped is refused, not recorded as cancelled + for (final JobCancellationHandler handler : getJobCancellationHandlers()) { + if (!handler.isJobExecutionCancellable(jobId)) { + errMessage = "Cannot cancel job-" + jobId + ", the operation it is running cannot be stopped at this point."; + logger.info(errMessage); + return errMessage; + } + } + + logger.debug("Cancelling job-{} which is in progress.", jobId); + try { + for (final JobCancellationHandler handler : getJobCancellationHandlers()) { + if (!handler.cancelJobExecution(jobId, reason)) { + errMessage = "Cannot cancel job-" + jobId + ", the operation it is running could not be stopped."; + logger.info(errMessage); + return errMessage; + } + } + + // finalise here unless another management server is executing the job; its poller acknowledges + final Long executingMsid = job.getExecutingMsid(); + final boolean finalizeNow = executingMsid == null || executingMsid == getMsid(); + completeAsyncJob(jobId, JobInfo.Status.CANCELLED, 0, "Job is cancelled due to " + reason, finalizeNow); + cancelQueuedChildJobs(jobId, reason); + return ""; + } catch (final Throwable t) { + errMessage = "Unexpected exception when cancelling async job with id: " + jobId; + logger.error(errMessage, t); + } + return errMessage; + } + + @Override + public List listCancelledJobsExecutingOn(final long msid) { + return _jobDao.getCancelledJobs(msid); + } + + @Override + public void finalizeCancelledJob(final long jobId) { + final AsyncJobVO job = _jobDao.findById(jobId); + if (job == null || job.getStatus() != JobInfo.Status.CANCELLED) { + return; + } + final Date now = DateUtil.currentGMTTime(); + job.setCompleteMsid(getMsid()); + job.setLastUpdated(now); + job.setRemoved(now); + job.setExecutingMsid(null); + _jobDao.update(jobId, job); + } + + private List getJobCancellationHandlers() { + return jobCancellationHandlers != null ? jobCancellationHandlers : Collections.emptyList(); + } + + private boolean isActiveJob(final long jobId) { + // a VM work child runs on behalf of its API parent + final AsyncJobVO relatedJob = _jobDao.getRelatedJob(String.valueOf(jobId)); + if (relatedJob != null) { + return _jobMonitor.isActiveJob(relatedJob.getId()); + } + return _jobMonitor.isActiveJob(jobId); + } + + @Override + public List> getCommands() { + final List> cmdList = new ArrayList<>(); + cmdList.add(CancelAsyncJobCmd.class); + return cmdList; + } + private GenericSearchBuilder ContentIdsSearch; private GenericSearchBuilder JoinJobSearch; private SearchBuilder JobIdsSearch; diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobMonitor.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobMonitor.java index b2216cb75025..d69c5978456f 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobMonitor.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobMonitor.java @@ -17,8 +17,10 @@ package org.apache.cloudstack.framework.jobs.impl; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.Map; +import java.util.Set; import java.util.Timer; import java.util.concurrent.atomic.AtomicInteger; @@ -39,7 +41,8 @@ public class AsyncJobMonitor extends ManagerBase { @Inject private MessageBus _messageBus; - private final Map _activeTasks = new HashMap(); + private final Map _runNumberToActiveTasksMap = new HashMap<>(); + private final Set _activeJobs = new HashSet<>(); private final Timer _timer = new Timer(); private final AtomicInteger _activePoolThreads = new AtomicInteger(); @@ -72,7 +75,7 @@ public void setInactivityWarningThresholdMs(long thresholdMs) { public void onJobHeartbeatNotify(String subject, String senderAddress, Object args) { if (args != null && args instanceof Long) { synchronized (this) { - ActiveTaskRecord record = _activeTasks.get(args); + ActiveTaskRecord record = _runNumberToActiveTasksMap.get(args); if (record != null) { record.updateJobHeartbeatTick(); } @@ -82,7 +85,7 @@ public void onJobHeartbeatNotify(String subject, String senderAddress, Object ar private void heartbeat() { synchronized (this) { - for (Map.Entry entry : _activeTasks.entrySet()) { + for (Map.Entry entry : _runNumberToActiveTasksMap.entrySet()) { if (entry.getValue().millisSinceLastJobHeartbeat() > _inactivityWarningThresholdMs) { logger.warn("Task (job-" + entry.getValue().getJobId() + ") has been pending for " + entry.getValue().millisSinceLastJobHeartbeat() / 1000 + " seconds"); @@ -109,23 +112,24 @@ protected void runInContext() { public void registerActiveTask(long runNumber, long jobId) { synchronized (this) { logger.info("Add job-" + jobId + " into job monitoring"); - - assert (_activeTasks.get(runNumber) == null); + assert (_runNumberToActiveTasksMap.get(runNumber) == null); long threadId = Thread.currentThread().getId(); boolean fromPoolThread = Thread.currentThread().getName().contains(AsyncJobManager.API_JOB_POOL_THREAD_PREFIX); ActiveTaskRecord record = new ActiveTaskRecord(jobId, threadId, fromPoolThread); - _activeTasks.put(runNumber, record); - if (fromPoolThread) + _runNumberToActiveTasksMap.put(runNumber, record); + _activeJobs.add(jobId); + if (fromPoolThread) { _activePoolThreads.incrementAndGet(); - else + } else { _activeInplaceThreads.incrementAndGet(); + } } } public void unregisterActiveTask(long runNumber) { synchronized (this) { - ActiveTaskRecord record = _activeTasks.get(runNumber); + ActiveTaskRecord record = _runNumberToActiveTasksMap.get(runNumber); assert (record != null); if (record != null) { logger.info("Remove job-" + record.getJobId() + " from job monitoring"); @@ -135,18 +139,19 @@ public void unregisterActiveTask(long runNumber) { else _activeInplaceThreads.decrementAndGet(); - _activeTasks.remove(runNumber); + _runNumberToActiveTasksMap.remove(runNumber); + _activeJobs.remove(record.getJobId()); } } } public void unregisterByJobId(long jobId) { synchronized (this) { - Iterator> it = _activeTasks.entrySet().iterator(); + Iterator> it = _runNumberToActiveTasksMap.entrySet().iterator(); while (it.hasNext()) { Map.Entry entry = it.next(); if (entry.getValue().getJobId() == jobId) { - logger.info("Remove Job-" + entry.getValue().getJobId() + " from job monitoring due to job cancelling"); + logger.info("Remove Job-{} from job monitoring due to job cancelling", entry.getValue().getJobId()); if (entry.getValue().isPoolThread()) _activePoolThreads.decrementAndGet(); @@ -156,6 +161,13 @@ public void unregisterByJobId(long jobId) { it.remove(); } } + _activeJobs.remove(jobId); + } + } + + public boolean isActiveJob(long jobId) { + synchronized (this) { + return _activeJobs.contains(jobId); } } diff --git a/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImplTest.java b/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImplTest.java index 0be5dbc01cbd..7795266f0960 100644 --- a/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImplTest.java +++ b/framework/jobs/src/test/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImplTest.java @@ -27,10 +27,15 @@ import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.api.ApiCommandResourceType; +import org.apache.cloudstack.framework.jobs.dao.AsyncJobDao; +import org.apache.cloudstack.framework.jobs.dao.SyncQueueItemDao; +import org.apache.cloudstack.jobs.JobInfo; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.junit.Test; +import java.util.Arrays; +import java.util.Collections; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; @@ -56,6 +61,18 @@ public class AsyncJobManagerImplTest { @Mock NetworkOrchestrationService networkOrchestrationService; + @Mock + AsyncJobDao jobDao; + + @Mock + SyncQueueItemDao queueItemDao; + + @Mock + SyncQueueManager queueMgr; + + @Mock + AsyncJobMonitor jobMonitor; + @Test public void testCleanupVolumeResource() { AsyncJobVO job = new AsyncJobVO(); @@ -93,4 +110,58 @@ public void testCleanupNetworkResource() throws NoTransitionException { Mockito.verify(networkOrchestrationService, Mockito.times(1)).stateTransitTo(networkVO, Network.Event.OperationFailed); } + + private AsyncJobVO childJob(long id, JobInfo.Status status) { + AsyncJobVO child = new AsyncJobVO(); + child.setId(id); + child.setStatus(status); + child.setRelated("10"); + return child; + } + + private SyncQueueItemVO queueItem(long id, Long lastProcessMsid) { + SyncQueueItemVO item = new SyncQueueItemVO(); + item.setId(id); + item.setLastProcessMsid(lastProcessMsid); + return item; + } + + @Test + public void testCancelQueuedChildJobsCompletesAndPurgesWaitingChild() { + when(jobDao.listChildJobs(10L)).thenReturn(Collections.singletonList(childJob(11L, JobInfo.Status.IN_PROGRESS))); + when(queueItemDao.getQueueItemIdByContentIdAndType(11L, SyncQueueItem.AsyncJobContentType)).thenReturn(5L); + when(queueItemDao.findById(5L)).thenReturn(queueItem(5L, null)); + Mockito.doNothing().when(asyncJobManager).completeAsyncJob(Mockito.eq(11L), Mockito.eq(JobInfo.Status.CANCELLED), Mockito.eq(0), Mockito.anyString()); + + asyncJobManager.cancelQueuedChildJobs(10L, "user request"); + + Mockito.verify(asyncJobManager).completeAsyncJob(Mockito.eq(11L), Mockito.eq(JobInfo.Status.CANCELLED), Mockito.eq(0), Mockito.contains("user request")); + Mockito.verify(jobMonitor).unregisterByJobId(11L); + Mockito.verify(queueMgr).purgeItem(5L); + } + + @Test + public void testCancelQueuedChildJobsLeavesDequeuedChildToTheInFlightPath() { + when(jobDao.listChildJobs(10L)).thenReturn(Collections.singletonList(childJob(11L, JobInfo.Status.IN_PROGRESS))); + when(queueItemDao.getQueueItemIdByContentIdAndType(11L, SyncQueueItem.AsyncJobContentType)).thenReturn(5L); + when(queueItemDao.findById(5L)).thenReturn(queueItem(5L, 1L)); + + asyncJobManager.cancelQueuedChildJobs(10L, "user request"); + + Mockito.verify(asyncJobManager, Mockito.never()).completeAsyncJob(Mockito.anyLong(), Mockito.any(), Mockito.anyInt(), Mockito.anyString()); + Mockito.verify(jobMonitor, Mockito.never()).unregisterByJobId(Mockito.anyLong()); + Mockito.verify(queueMgr, Mockito.never()).purgeItem(Mockito.anyLong()); + } + + @Test + public void testCancelQueuedChildJobsSkipsFinishedAndUnqueuedChildren() { + when(jobDao.listChildJobs(10L)).thenReturn(Arrays.asList(childJob(11L, JobInfo.Status.SUCCEEDED), childJob(12L, JobInfo.Status.IN_PROGRESS))); + when(queueItemDao.getQueueItemIdByContentIdAndType(12L, SyncQueueItem.AsyncJobContentType)).thenReturn(null); + + asyncJobManager.cancelQueuedChildJobs(10L, "user request"); + + Mockito.verify(queueItemDao, Mockito.never()).getQueueItemIdByContentIdAndType(Mockito.eq(11L), Mockito.anyString()); + Mockito.verify(asyncJobManager, Mockito.never()).completeAsyncJob(Mockito.anyLong(), Mockito.any(), Mockito.anyInt(), Mockito.anyString()); + Mockito.verify(queueMgr, Mockito.never()).purgeItem(Mockito.anyLong()); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KvmCancellableRequests.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KvmCancellableRequests.java new file mode 100644 index 000000000000..dc182081bedf --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/KvmCancellableRequests.java @@ -0,0 +1,148 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.kvm.resource; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.function.Consumer; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import com.cloud.agent.api.Command; +import com.cloud.hypervisor.kvm.resource.disconnecthook.DisconnectHook; + +/** Per agent request sequence, the DisconnectHooks the wrappers registered for its libvirt jobs: they are what a cancellation runs. A request without a hook has nothing that can be stopped. */ +public class KvmCancellableRequests { + private static final Logger LOGGER = LogManager.getLogger(KvmCancellableRequests.class); + + private static final ThreadLocal CURRENT = new ThreadLocal<>(); + + private final Map requests = new ConcurrentHashMap<>(); + + private static final class RequestState { + final Command command; + final List hooks = new CopyOnWriteArrayList<>(); + volatile boolean cancelRequested; + + RequestState(final Command command) { + this.command = command; + } + } + + /** A sequence of zero (not from the agent layer) leaves the thread unscoped. */ + public void begin(final long sequence, final Command command) { + if (sequence <= 0) { + return; + } + requests.putIfAbsent(sequence, new RequestState(command)); + CURRENT.set(sequence); + } + + public void end(final long sequence) { + CURRENT.remove(); + if (sequence > 0) { + requests.remove(sequence); + } + } + + public boolean wasCancelRequested(final long sequence) { + final RequestState state = requests.get(sequence); + return state != null && state.cancelRequested; + } + + /** Associates a hook with the request the current thread is executing, if any. */ + public void attach(final DisconnectHook hook) { + final Long sequence = CURRENT.get(); + if (sequence == null || hook == null) { + return; + } + final RequestState state = requests.get(sequence); + if (state != null) { + state.hooks.add(hook); + } + } + + public void detach(final DisconnectHook hook) { + if (hook == null) { + return; + } + final Long sequence = CURRENT.get(); + final RequestState scoped = sequence != null ? requests.get(sequence) : null; + if (scoped != null && scoped.hooks.remove(hook)) { + return; + } + for (final RequestState state : requests.values()) { + state.hooks.remove(hook); + } + } + + /** A request can be stopped only while it has a hook that knows how to stop it. */ + public boolean isCancellable(final long sequence) { + final RequestState state = requests.get(sequence); + return state != null && !state.hooks.isEmpty(); + } + + /** Runs each hook once, bounded by its timeout; a Thread cannot be started twice, so ran hooks are handed back for removal from the disconnect list. */ + public boolean cancel(final long sequence, final Consumer onHookRan) { + final RequestState state = requests.get(sequence); + if (state == null) { + return false; + } + state.cancelRequested = true; + if (state.hooks.isEmpty()) { + LOGGER.debug("Request sequence {} ({}) has nothing that can be stopped", sequence, describe(state.command)); + return false; + } + + boolean allRan = true; + for (final DisconnectHook hook : state.hooks) { + try { + if (hook.getState() == Thread.State.NEW) { + hook.start(); + } + hook.join(hook.getTimeoutMs()); + if (hook.isAlive()) { + LOGGER.warn("Cancel hook {} for request sequence {} did not finish within {} ms", hook.getName(), sequence, hook.getTimeoutMs()); + allRan = false; + } else { + LOGGER.info("Ran cancel hook {} for request sequence {} ({})", hook.getName(), sequence, describe(state.command)); + } + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + allRan = false; + } catch (final Exception e) { + LOGGER.warn("Cancel hook {} for request sequence {} failed", hook.getName(), sequence, e); + allRan = false; + } finally { + onHookRan.accept(hook); + } + } + return allRan; + } + + int hookCount(final long sequence) { + final RequestState state = requests.get(sequence); + return state == null ? 0 : state.hooks.size(); + } + + private static String describe(final Command command) { + return command == null ? "unknown command" : command.getClass().getSimpleName(); + } +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 9009ec629ca3..42f5955245ef 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -654,6 +654,8 @@ public synchronized void registerStatusUpdater(AgentStatusUpdater updater) { } protected List _disconnectHooks = new CopyOnWriteArrayList<>(); + // the same hooks keyed by the request that registered them + private final KvmCancellableRequests cancellableRequests = new KvmCancellableRequests(); @Override public ExecutionResult executeInVR(final String routerIp, final String script, final String args) { @@ -2403,6 +2405,31 @@ public boolean stop() { */ @Override public Answer executeRequest(final Command cmd) { + final long requestSequence = cmd.getRequestSequence(); + cancellableRequests.begin(requestSequence, cmd); + try { + final Answer answer = executeRequestInternal(cmd); + // an aborted libvirt job surfaces as an ordinary failure; flag it as cancelled + if (answer != null && !answer.getResult() && cancellableRequests.wasCancelRequested(requestSequence)) { + answer.setCancelled(true); + } + return answer; + } finally { + cancellableRequests.end(requestSequence); + } + } + + @Override + public boolean isRequestSequenceCancellable(final long sequence) { + return cancellableRequests.isCancellable(sequence); + } + + @Override + public boolean cancelRequestSequence(final long sequence) { + return cancellableRequests.cancel(sequence, this::removeDisconnectHook); + } + + private Answer executeRequestInternal(final Command cmd) { if (isReconcileCommandsEnabled) { ReconcileCommandUtils.updateLogFileForCommand(COMMANDS_LOG_PATH, cmd, Command.State.STARTED); } @@ -6921,10 +6948,12 @@ public void disconnected() { public void addDisconnectHook(DisconnectHook hook) { LOGGER.debug("Adding disconnect hook " + hook); _disconnectHooks.add(hook); + cancellableRequests.attach(hook); } public void removeDisconnectHook(DisconnectHook hook) { LOGGER.debug("Removing disconnect hook " + hook); + cancellableRequests.detach(hook); if (_disconnectHooks.contains(hook)) { LOGGER.debug("Removing disconnect hook " + hook); _disconnectHooks.remove(hook); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCancelCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCancelCommandWrapper.java new file mode 100644 index 000000000000..d77b935d7a91 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCancelCommandWrapper.java @@ -0,0 +1,44 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.kvm.resource.wrapper; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CancelCommand; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; + +/** Can another request in flight on this agent be stopped (checkOnly), or stop it. Out of sequence, so it is not queued behind that request. */ +@ResourceWrapper(handles = CancelCommand.class) +public final class LibvirtCancelCommandWrapper extends CommandWrapper { + + @Override + public Answer execute(final CancelCommand command, final LibvirtComputingResource libvirtComputingResource) { + final long sequence = command.getSequence(); + if (command.isCheckOnly()) { + final boolean cancellable = libvirtComputingResource.isRequestSequenceCancellable(sequence); + return new Answer(command, cancellable, cancellable + ? "request sequence " + sequence + " can be cancelled" + : "request sequence " + sequence + " has no work that can be stopped on this host"); + } + + final boolean cancelled = libvirtComputingResource.cancelRequestSequence(sequence); + return new Answer(command, cancelled, cancelled + ? "request sequence " + sequence + " cancelled (" + command.getReason() + ")" + : "request sequence " + sequence + " could not be cancelled on this host"); + } +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/KvmCancellableRequestsTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/KvmCancellableRequestsTest.java new file mode 100644 index 000000000000..75de371f7a92 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/KvmCancellableRequestsTest.java @@ -0,0 +1,129 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.kvm.resource; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; + +import com.cloud.agent.api.CheckHealthCommand; +import com.cloud.hypervisor.kvm.resource.disconnecthook.DisconnectHook; + +public class KvmCancellableRequestsTest { + + private static final long SEQ = 42L; + + private final KvmCancellableRequests requests = new KvmCancellableRequests(); + private final List dropped = new ArrayList<>(); + + private static final class CountingHook extends DisconnectHook { + final AtomicInteger runs = new AtomicInteger(); + + CountingHook() { + super("counting", 2000); + } + + @Override + public void run() { + runs.incrementAndGet(); + } + } + + @After + public void tearDown() { + requests.end(SEQ); + } + + @Test + public void unknownSequenceIsNeitherCancellableNorCancelled() { + Assert.assertFalse(requests.isCancellable(999L)); + Assert.assertFalse(requests.cancel(999L, dropped::add)); + Assert.assertFalse(requests.wasCancelRequested(999L)); + } + + @Test + public void requestWithoutAHookHasNothingToStop() { + requests.begin(SEQ, new CheckHealthCommand()); + + Assert.assertFalse(requests.isCancellable(SEQ)); + Assert.assertFalse(requests.cancel(SEQ, dropped::add)); + Assert.assertTrue(requests.wasCancelRequested(SEQ)); + } + + @Test + public void hookRegisteredOnTheExecutingThreadIsRunOnceOnCancel() { + final CountingHook hook = new CountingHook(); + requests.begin(SEQ, new CheckHealthCommand()); + requests.attach(hook); + + Assert.assertTrue(requests.isCancellable(SEQ)); + Assert.assertTrue(requests.cancel(SEQ, dropped::add)); + Assert.assertEquals(1, hook.runs.get()); + Assert.assertEquals(1, dropped.size()); + + // a Thread runs only once + Assert.assertTrue(requests.cancel(SEQ, dropped::add)); + Assert.assertEquals(1, hook.runs.get()); + } + + @Test + public void detachedHookIsNotRun() { + final CountingHook hook = new CountingHook(); + requests.begin(SEQ, new CheckHealthCommand()); + requests.attach(hook); + requests.detach(hook); + + Assert.assertEquals(0, requests.hookCount(SEQ)); + Assert.assertFalse(requests.isCancellable(SEQ)); + Assert.assertEquals(0, hook.runs.get()); + } + + @Test + public void hookAttachedFromAnUnscopedThreadIsIgnored() throws Exception { + final CountingHook hook = new CountingHook(); + requests.begin(SEQ, new CheckHealthCommand()); + + final Thread other = new Thread(() -> requests.attach(hook)); + other.start(); + other.join(); + + Assert.assertEquals(0, requests.hookCount(SEQ)); + } + + @Test + public void zeroSequenceLeavesTheThreadUnscoped() { + requests.begin(0L, new CheckHealthCommand()); + requests.attach(new CountingHook()); + + Assert.assertFalse(requests.isCancellable(0L)); + requests.end(0L); + } + + @Test + public void endForgetsTheSequence() { + requests.begin(SEQ, new CheckHealthCommand()); + requests.attach(new CountingHook()); + requests.end(SEQ); + + Assert.assertFalse(requests.isCancellable(SEQ)); + Assert.assertEquals(0, requests.hookCount(SEQ)); + } +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCancelCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCancelCommandWrapperTest.java new file mode 100644 index 000000000000..9284c71af898 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCancelCommandWrapperTest.java @@ -0,0 +1,77 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.kvm.resource.wrapper; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CancelCommand; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; + +@RunWith(MockitoJUnitRunner.class) +public class LibvirtCancelCommandWrapperTest { + + @Mock + private LibvirtComputingResource resource; + + private final LibvirtCancelCommandWrapper wrapper = new LibvirtCancelCommandWrapper(); + + @Test + public void checkOnlyAsksWithoutCancelling() { + Mockito.when(resource.isRequestSequenceCancellable(7L)).thenReturn(true); + + final Answer answer = wrapper.execute(new CancelCommand(7L, "probe", true), resource); + + Assert.assertTrue(answer.getResult()); + Mockito.verify(resource, Mockito.never()).cancelRequestSequence(Mockito.anyLong()); + } + + @Test + public void checkOnlyReportsNothingToStop() { + Mockito.when(resource.isRequestSequenceCancellable(7L)).thenReturn(false); + + final Answer answer = wrapper.execute(new CancelCommand(7L, "probe", true), resource); + + Assert.assertFalse(answer.getResult()); + Assert.assertTrue(answer.getDetails(), answer.getDetails().contains("no work that can be stopped")); + } + + @Test + public void cancelDelegatesAndReportsTheOutcome() { + Mockito.when(resource.cancelRequestSequence(7L)).thenReturn(true); + + final Answer answer = wrapper.execute(new CancelCommand(7L, "job cancelled"), resource); + + Assert.assertTrue(answer.getResult()); + Assert.assertTrue(answer.getDetails(), answer.getDetails().contains("job cancelled")); + Mockito.verify(resource, Mockito.never()).isRequestSequenceCancellable(Mockito.anyLong()); + } + + @Test + public void failedCancelIsReportedAsSuch() { + Mockito.when(resource.cancelRequestSequence(7L)).thenReturn(false); + + final Answer answer = wrapper.execute(new CancelCommand(7L, "job cancelled"), resource); + + Assert.assertFalse(answer.getResult()); + } +} diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 89b5b111e14f..7756ba414f1b 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -267,11 +267,13 @@ import com.cloud.hypervisor.vmware.util.VmwareContext; import com.cloud.hypervisor.vmware.util.VmwareContextPool; import com.cloud.hypervisor.vmware.util.VmwareHelper; +import com.cloud.hypervisor.vmware.util.VmwareTaskRegistry; import com.cloud.network.Networks; import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.network.Networks.TrafficType; import com.cloud.network.VmwareTrafficLabel; import com.cloud.network.router.VirtualRouterAutoScale; +import com.cloud.resource.RequestExecutionContext; import com.cloud.resource.ServerResource; import com.cloud.resource.ServerResourceBase; import com.cloud.serializer.GsonHelper; @@ -429,6 +431,9 @@ public class VmwareResource extends ServerResourceBase implements StoragePoolRes protected volatile long _cmdSequence = 1; + // per resource, so two hosts cannot collide on a request sequence + private final VmwareTaskRegistry taskRegistry = new VmwareTaskRegistry(); + protected StorageSubsystemCommandHandler storageHandler; private VmwareStorageProcessor _storageProcessor; @@ -467,6 +472,31 @@ private String getCommandLogTitle(Command cmd) { @Override public Answer executeRequest(Command cmd) { + final Long requestSequence = RequestExecutionContext.getRequestSequence(); + taskRegistry.beginRequest(requestSequence); + try { + Answer answer = executeRequestInternal(cmd); + // a cancelled vCenter task surfaces as an ordinary failure; flag it as cancelled + if (answer != null && !answer.getResult() && taskRegistry.wasCancelRequested(requestSequence)) { + answer.setCancelled(true); + } + return answer; + } finally { + taskRegistry.endRequest(requestSequence); + } + } + + @Override + public boolean isRequestSequenceCancellable(long sequence) { + return taskRegistry.isCancellable(sequence); + } + + @Override + public boolean cancelRequestSequence(long sequence) { + return taskRegistry.cancel(sequence); + } + + private Answer executeRequestInternal(Command cmd) { logCommand(cmd); Answer answer; ThreadContext.push(getCommandLogTitle(cmd)); diff --git a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java index be7ee46f6dda..3ea921e560b6 100644 --- a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java +++ b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java @@ -874,4 +874,10 @@ public void testRemoveVirtualTPMDevice() throws Exception { Mockito.verify(vmwareResource, Mockito.times(1)).removeVirtualTPMDevice(vmConfigSpec, tpm); Mockito.verify(deviceChanges, Mockito.times(1)).add(any(VirtualDeviceConfigSpec.class)); } + + @Test + public void testUnknownRequestSequenceIsNotCancellable() { + assertFalse(vmwareResource.isRequestSequenceCancellable(4242L)); + assertFalse(vmwareResource.cancelRequestSequence(4242L)); + } } diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index cdb4d7434aef..aa171e617513 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -126,6 +126,7 @@ import com.cloud.network.Networks; import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.network.Networks.TrafficType; +import com.cloud.resource.RequestExecutionContext; import com.cloud.resource.ServerResource; import com.cloud.resource.ServerResourceBase; import com.cloud.resource.hypervisor.HypervisorResource; @@ -307,6 +308,9 @@ private static boolean isAlienVm(final VM vm, final Connection conn) throws XenA protected XenServerUtilitiesHelper xenServerUtilitiesHelper = new XenServerUtilitiesHelper(); + // per resource, so two hosts cannot collide on a request sequence + private final XenServerTaskRegistry taskRegistry = new XenServerTaskRegistry(); + protected int _wait; // Hypervisor specific params with generic value, may need to be overridden // for specific versions @@ -1775,6 +1779,31 @@ public ExecutionResult executeInVR(final String routerIP, final String script, f @Override public Answer executeRequest(final Command cmd) { + final Long requestSequence = RequestExecutionContext.getRequestSequence(); + taskRegistry.beginRequest(requestSequence); + try { + final Answer answer = executeRequestInternal(cmd); + // a cancelled XenAPI task surfaces as an ordinary failure; flag it as cancelled + if (answer != null && !answer.getResult() && taskRegistry.wasCancelRequested(requestSequence)) { + answer.setCancelled(true); + } + return answer; + } finally { + taskRegistry.endRequest(requestSequence); + } + } + + @Override + public boolean isRequestSequenceCancellable(final long sequence) { + return taskRegistry.isCancellable(sequence); + } + + @Override + public boolean cancelRequestSequence(final long sequence) { + return taskRegistry.cancel(sequence); + } + + private Answer executeRequestInternal(final Command cmd) { final CitrixRequestWrapper wrapper = CitrixRequestWrapper.getInstance(); try { return wrapper.execute(cmd, this); @@ -5283,21 +5312,33 @@ public void waitForTask(final Connection c, final Task task, final long pollInte if (logger.isTraceEnabled()) { logger.trace("Task " + task.getNameLabel(c) + " (" + task.getUuid(c) + ") sent to " + c.getSessionReference() + " is pending completion with a " + timeout + "ms timeout"); } - while (task.getStatus(c) == Types.TaskStatusType.PENDING) { - try { - if (logger.isTraceEnabled()) { - logger.trace("Task " + task.getNameLabel(c) + " (" + task.getUuid(c) + ") is pending, sleeping for " + pollInterval + "ms"); + // register with the executing request; a request already cancelled cancels the new task at once + if (XenServerTaskRegistry.taskStarted(task, c)) { + logger.info("Request executing on this thread was already cancelled, cancelling newly created task " + task); + XenServerTaskRegistry.cancelTask(task, c); + } + try { + // wait through CANCELLING too: callers read the final status right after this returns + Types.TaskStatusType status = task.getStatus(c); + while (status == Types.TaskStatusType.PENDING || status == Types.TaskStatusType.CANCELLING) { + try { + if (logger.isTraceEnabled()) { + logger.trace("Task " + task.getNameLabel(c) + " (" + task.getUuid(c) + ") is " + status + ", sleeping for " + pollInterval + "ms"); + } + Thread.sleep(pollInterval); + } catch (final InterruptedException ignored) { } - Thread.sleep(pollInterval); - } catch (final InterruptedException ignored) { - } - if (System.currentTimeMillis() - beginTime > timeout) { - final String msg = "Async " + timeout / 1000 + " seconds timeout for task " + task; - logger.warn(msg); - task.cancel(c); - task.destroy(c); - throw new TimeoutException(msg); + if (System.currentTimeMillis() - beginTime > timeout) { + final String msg = "Async " + timeout / 1000 + " seconds timeout for task " + task; + logger.warn(msg); + task.cancel(c); + task.destroy(c); + throw new TimeoutException(msg); + } + status = task.getStatus(c); } + } finally { + XenServerTaskRegistry.taskFinished(task); } } diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerTaskRegistry.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerTaskRegistry.java new file mode 100644 index 000000000000..15655f87e540 --- /dev/null +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerTaskRegistry.java @@ -0,0 +1,204 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.xenserver.resource; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import com.cloud.utils.Pair; +import com.xensource.xenapi.Connection; +import com.xensource.xenapi.Task; +import com.xensource.xenapi.Types; + +/** Per agent request sequence, the XenAPI tasks a resource is waiting on; one instance per host. waitForTask is the single registration point. */ +public class XenServerTaskRegistry { + private static final Logger LOGGER = LogManager.getLogger(XenServerTaskRegistry.class); + + private static final long CANCEL_TASK_WAIT_MS = 30000; + private static final long CANCEL_TASK_POLL_MS = 500; + + private static final ThreadLocal CURRENT = new ThreadLocal<>(); + + private final Map requests = new ConcurrentHashMap<>(); + + private static final class ActiveTask { + final Task task; + final Connection connection; + + ActiveTask(final Task task, final Connection connection) { + this.task = task; + this.connection = connection; + } + } + + private static final class RequestState { + final Map tasks = new ConcurrentHashMap<>(); + volatile boolean cancelRequested; + } + + private static final class RequestScope { + final XenServerTaskRegistry registry; + final long sequence; + + RequestScope(final XenServerTaskRegistry registry, final long sequence) { + this.registry = registry; + this.sequence = sequence; + } + } + + /** A null sequence (not from the agent layer) leaves the thread unscoped. */ + public void beginRequest(final Long sequence) { + if (sequence == null) { + return; + } + requests.putIfAbsent(sequence, new RequestState()); + CURRENT.set(new RequestScope(this, sequence)); + } + + public void endRequest(final Long sequence) { + CURRENT.remove(); + if (sequence != null) { + requests.remove(sequence); + } + } + + public boolean wasCancelRequested(final Long sequence) { + if (sequence == null) { + return false; + } + final RequestState state = requests.get(sequence); + return state != null && state.cancelRequested; + } + + /** Returns true when the request was already cancelled, so the caller cancels the new task at once. */ + static boolean taskStarted(final Task task, final Connection connection) { + final RequestScope scope = CURRENT.get(); + if (scope == null || task == null) { + return false; + } + final RequestState state = scope.registry.requests.get(scope.sequence); + if (state == null) { + return false; + } + state.tasks.put(refOf(task), new ActiveTask(task, connection)); + return state.cancelRequested; + } + + static void taskFinished(final Task task) { + final RequestScope scope = CURRENT.get(); + if (scope == null || task == null) { + return; + } + final RequestState state = scope.registry.requests.get(scope.sequence); + if (state != null) { + state.tasks.remove(refOf(task)); + } + } + + /** Unknown sequences are not cancellable; XenAPI has no cancelable flag, so tasks must still be pending. */ + public boolean isCancellable(final long sequence) { + final RequestState state = requests.get(sequence); + if (state == null) { + return false; + } + for (final ActiveTask active : state.tasks.values()) { + try { + if (active.task.getStatus(active.connection) != Types.TaskStatusType.PENDING) { + LOGGER.debug("XenAPI task {} of request sequence {} is no longer pending, not cancellable", refOf(active.task), sequence); + return false; + } + } catch (final Exception e) { + LOGGER.warn("Unable to check the status of XenAPI task {} of request sequence {}", refOf(active.task), sequence, e); + return false; + } + } + return true; + } + + /** Cancels the in-flight tasks and marks the request so later tasks are cancelled on arrival. */ + public boolean cancel(final long sequence) { + final RequestState state = requests.get(sequence); + if (state == null) { + return false; + } + state.cancelRequested = true; + + boolean allCancelled = true; + for (final ActiveTask active : state.tasks.values()) { + final Pair result = cancelTask(active.task, active.connection); + if (result.first()) { + LOGGER.info("Cancelled XenAPI task {} of request sequence {}", refOf(active.task), sequence); + } else { + LOGGER.info("Could not cancel XenAPI task {} of request sequence {}: {}", refOf(active.task), sequence, result.second()); + allCancelled = false; + } + } + return allCancelled; + } + + /** cancelAsync, then poll to a terminal state: the synchronous cancel would hold the caller on a XenServer round trip. */ + static Pair cancelTask(final Task task, final Connection connection) { + final String ref = refOf(task); + try { + final Types.TaskStatusType status = task.getStatus(connection); + if (status != Types.TaskStatusType.PENDING && status != Types.TaskStatusType.CANCELLING) { + return new Pair<>(false, "task " + ref + " is already " + status); + } + if (status == Types.TaskStatusType.PENDING) { + task.cancelAsync(connection); + } + + final long deadline = System.currentTimeMillis() + CANCEL_TASK_WAIT_MS; + while (System.currentTimeMillis() < deadline) { + final Types.TaskStatusType now = task.getStatus(connection); + if (now == Types.TaskStatusType.CANCELLED) { + return new Pair<>(true, "task " + ref + " cancelled"); + } + if (now == Types.TaskStatusType.SUCCESS) { + return new Pair<>(false, "task " + ref + " completed before it could be cancelled"); + } + if (now == Types.TaskStatusType.FAILURE) { + return new Pair<>(false, "task " + ref + " failed before it could be cancelled"); + } + Thread.sleep(CANCEL_TASK_POLL_MS); + } + return new Pair<>(false, "task " + ref + " did not stop within " + CANCEL_TASK_WAIT_MS / 1000 + " seconds of the cancel request"); + } catch (final Types.OperationNotAllowed e) { + return new Pair<>(false, "XenServer does not allow cancelling task " + ref); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + return new Pair<>(false, "interrupted while waiting for task " + ref + " to cancel"); + } catch (final Exception e) { + LOGGER.warn("Failed to cancel XenAPI task {}", ref, e); + return new Pair<>(false, "failed to cancel task " + ref + ": " + e.getMessage()); + } + } + + // no opaque ref before a round trip (or on a test double): fall back to identity + private static String refOf(final Task task) { + final String ref = task.toWireString(); + return ref != null ? ref : "task@" + Integer.toHexString(System.identityHashCode(task)); + } + + int activeTaskCount(final long sequence) { + final RequestState state = requests.get(sequence); + return state == null ? 0 : state.tasks.size(); + } +} diff --git a/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java b/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java index aac865622196..67d5d3ac3a79 100644 --- a/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java +++ b/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java @@ -68,6 +68,7 @@ import static com.cloud.hypervisor.xenserver.resource.CitrixResourceBase.PLATFORM_CORES_PER_SOCKET_KEY; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.doReturn; @RunWith(MockitoJUnitRunner.class) @@ -616,4 +617,10 @@ public void testListFilesAtPathWithFile() throws IOException, XmlRpcException { Assert.assertEquals(1024L, listAnswer.getSizes().get(0).longValue()); Assert.assertEquals(123456789000L, listAnswer.getLastModified().get(0).longValue()); } + + @Test + public void testUnknownRequestSequenceIsNotCancellable() { + assertFalse(citrixResourceBase.isRequestSequenceCancellable(4242L)); + assertFalse(citrixResourceBase.cancelRequestSequence(4242L)); + } } diff --git a/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/XenServerTaskRegistryTest.java b/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/XenServerTaskRegistryTest.java new file mode 100644 index 000000000000..29b359dae635 --- /dev/null +++ b/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/XenServerTaskRegistryTest.java @@ -0,0 +1,177 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.xenserver.resource; + +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.utils.Pair; +import com.xensource.xenapi.Connection; +import com.xensource.xenapi.Task; +import com.xensource.xenapi.Types; + +@RunWith(MockitoJUnitRunner.class) +public class XenServerTaskRegistryTest { + + private static final long SEQ = 42L; + + @Mock + private Connection connection; + + private final XenServerTaskRegistry registry = new XenServerTaskRegistry(); + + @After + public void tearDown() { + registry.endRequest(SEQ); + } + + @Test + public void unknownSequenceIsNeitherCancellableNorCancelled() { + Assert.assertFalse(registry.isCancellable(999L)); + Assert.assertFalse(registry.cancel(999L)); + Assert.assertFalse(registry.wasCancelRequested(999L)); + } + + @Test + public void requestWithNoTaskInFlightIsCancellableByInterruptAlone() { + registry.beginRequest(SEQ); + + Assert.assertTrue(registry.isCancellable(SEQ)); + Assert.assertTrue(registry.cancel(SEQ)); + Assert.assertTrue(registry.wasCancelRequested(SEQ)); + } + + @Test + public void taskCreatedAfterCancelIsReportedSoTheResourceCancelsItImmediately() throws Exception { + registry.beginRequest(SEQ); + registry.cancel(SEQ); + + Assert.assertTrue(XenServerTaskRegistry.taskStarted(task("OpaqueRef:late", Types.TaskStatusType.PENDING), connection)); + } + + @Test + public void pendingTaskIsCancelledAsynchronouslyAndConfirmed() throws Exception { + final Task task = task("OpaqueRef:1", Types.TaskStatusType.PENDING, Types.TaskStatusType.PENDING, + Types.TaskStatusType.CANCELLING, Types.TaskStatusType.CANCELLED); + + registry.beginRequest(SEQ); + Assert.assertFalse(XenServerTaskRegistry.taskStarted(task, connection)); + + Assert.assertTrue(registry.isCancellable(SEQ)); + Assert.assertTrue(registry.cancel(SEQ)); + Mockito.verify(task).cancelAsync(connection); + Mockito.verify(task, Mockito.never()).cancel(Mockito.any()); + } + + @Test + public void taskNoLongerPendingRefusesAndIsLeftAlone() throws Exception { + final Task task = task("OpaqueRef:2", Types.TaskStatusType.SUCCESS); + + registry.beginRequest(SEQ); + XenServerTaskRegistry.taskStarted(task, connection); + + Assert.assertFalse(registry.isCancellable(SEQ)); + Assert.assertFalse(registry.cancel(SEQ)); + Mockito.verify(task, Mockito.never()).cancelAsync(Mockito.any()); + } + + @Test + public void cancelReportsFailureWhenTheTaskCompletesFirst() throws Exception { + final Task task = task("OpaqueRef:3", Types.TaskStatusType.PENDING, Types.TaskStatusType.PENDING, Types.TaskStatusType.SUCCESS); + + registry.beginRequest(SEQ); + XenServerTaskRegistry.taskStarted(task, connection); + + Assert.assertFalse(registry.cancel(SEQ)); + } + + @Test + public void cancelReportsFailureWhenXenServerDoesNotAllowIt() throws Exception { + final Task task = task("OpaqueRef:4", Types.TaskStatusType.PENDING); + Mockito.doThrow(new Types.OperationNotAllowed("not allowed")).when(task).cancelAsync(connection); + + final Pair result = XenServerTaskRegistry.cancelTask(task, connection); + + Assert.assertFalse(result.first()); + Assert.assertTrue(result.second(), result.second().contains("does not allow")); + } + + @Test + public void finishedTaskIsNoLongerTracked() throws Exception { + final Task task = task("OpaqueRef:5", Types.TaskStatusType.PENDING); + registry.beginRequest(SEQ); + XenServerTaskRegistry.taskStarted(task, connection); + Assert.assertEquals(1, registry.activeTaskCount(SEQ)); + + XenServerTaskRegistry.taskFinished(task); + + Assert.assertEquals(0, registry.activeTaskCount(SEQ)); + Assert.assertTrue(registry.cancel(SEQ)); + Mockito.verify(task, Mockito.never()).cancelAsync(Mockito.any()); + } + + @Test + public void endRequestForgetsTheSequence() throws Exception { + registry.beginRequest(SEQ); + registry.endRequest(SEQ); + + Assert.assertFalse(registry.isCancellable(SEQ)); + Assert.assertFalse(XenServerTaskRegistry.taskStarted(task("OpaqueRef:6", Types.TaskStatusType.PENDING), connection)); + Assert.assertEquals(0, registry.activeTaskCount(SEQ)); + } + + @Test + public void unscopedThreadRegistersNothing() throws Exception { + registry.beginRequest(SEQ); + final Task task = task("OpaqueRef:7", Types.TaskStatusType.PENDING); + final AtomicBoolean registered = new AtomicBoolean(true); + + // scope is per thread + final Thread other = new Thread(() -> registered.set(XenServerTaskRegistry.taskStarted(task, connection))); + other.start(); + other.join(); + + Assert.assertFalse(registered.get()); + Assert.assertEquals(0, registry.activeTaskCount(SEQ)); + } + + @Test + public void taskWithoutAReferenceIsStillTracked() throws Exception { + final Task task = Mockito.mock(Task.class); + Mockito.when(task.getStatus(connection)).thenReturn(Types.TaskStatusType.PENDING); + + registry.beginRequest(SEQ); + XenServerTaskRegistry.taskStarted(task, connection); + + Assert.assertEquals(1, registry.activeTaskCount(SEQ)); + Assert.assertTrue(registry.isCancellable(SEQ)); + } + + private Task task(final String ref, final Types.TaskStatusType first, final Types.TaskStatusType... rest) throws Exception { + final Task task = Mockito.mock(Task.class); + Mockito.when(task.toWireString()).thenReturn(ref); + Mockito.when(task.getStatus(connection)).thenReturn(first, rest); + return task; + } +} diff --git a/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java b/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java index e70a6b4da639..8eabd6b62457 100644 --- a/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java +++ b/server/src/main/java/com/cloud/api/ApiAsyncJobDispatcher.java @@ -31,6 +31,7 @@ import org.apache.cloudstack.framework.jobs.AsyncJob; import org.apache.cloudstack.framework.jobs.AsyncJobDispatcher; import org.apache.cloudstack.framework.jobs.AsyncJobManager; +import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; import org.apache.cloudstack.jobs.JobInfo; import com.cloud.exception.InvalidParameterValueException; @@ -117,10 +118,17 @@ public void runJob(final AsyncJob job) { CallContext.unregister(); } } catch (Throwable e) { + // a job cancelled mid-execution is already terminal; do not overwrite CANCELLED with the failure it caused + AsyncJobVO jobFromDb = _asyncJobMgr.getAsyncJob(job.getId()); + if (jobFromDb != null && jobFromDb.getStatus().done()) { + logger.debug("Not recording failure for job-{}, it is already in {}", job.getId(), jobFromDb.getStatus()); + return; + } + String errorMsg = null; int errorCode = ApiErrorCode.INTERNAL_ERROR.getHttpCode(); if (!(e instanceof ServerApiException)) { - logger.error("Unexpected exception while executing " + job.getCmd(), e); + logger.error("Unexpected exception while executing {}", job.getCmd(), e); errorMsg = e.getMessage(); } else { ServerApiException sApiEx = (ServerApiException)e; diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index f56cda6e557a..b457c37f9e4c 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -58,6 +58,7 @@ import org.apache.cloudstack.api.BaseResponseWithAssociatedNetwork; import org.apache.cloudstack.api.ResponseGenerator; import org.apache.cloudstack.api.ResponseObject.ResponseView; +import org.apache.cloudstack.api.command.user.job.CancelAsyncJobCmd; import org.apache.cloudstack.api.command.user.job.QueryAsyncJobResultCmd; import org.apache.cloudstack.api.response.ASNRangeResponse; import org.apache.cloudstack.api.response.ASNumberResponse; @@ -2377,6 +2378,12 @@ public AsyncJobResponse queryJobResult(final QueryAsyncJobResultCmd cmd) { return createAsyncJobResponse(_jobMgr.queryJob(jobId, true)); } + @Override + public AsyncJobResponse cancelJobResponse(CancelAsyncJobCmd cmd) { + AsyncJob job = _jobMgr.queryJob(cmd.getId(), true); + return createAsyncJobResponse(job); + } + public AsyncJobResponse createAsyncJobResponse(AsyncJob job) { AsyncJobJoinVO vJob = ApiDBUtils.newAsyncJobView(job); return ApiDBUtils.newAsyncJobResponse(vJob); diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index d700fba5a787..0ed7b93f0061 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -923,6 +924,7 @@ private Pair, Integer> searchForEventIdsAndCount(ListEventsCmd cmd) { Integer entryTime = cmd.getEntryTime(); Integer duration = cmd.getDuration(); Long startId = cmd.getStartId(); + final Long jobId = cmd.getJobId(); final String resourceUuid = getResourceUuid(cmd.getResourceId()); final ApiCommandResourceType resourceType = getResourceType(cmd.getResourceType()); final String stateStr = cmd.getState(); @@ -980,6 +982,7 @@ private Pair, Integer> searchForEventIdsAndCount(ListEventsCmd cmd) { eventSearchBuilder.and("archived", eventSearchBuilder.entity().getArchived(), SearchCriteria.Op.EQ); eventSearchBuilder.and("resourceId", eventSearchBuilder.entity().getResourceId(), SearchCriteria.Op.EQ); eventSearchBuilder.and("resourceType", eventSearchBuilder.entity().getResourceType(), SearchCriteria.Op.EQ); + eventSearchBuilder.and("asyncJobId", eventSearchBuilder.entity().getAsyncJobId(), SearchCriteria.Op.EQ); if (keyword != null) { eventSearchBuilder.and().op("keywordType", eventSearchBuilder.entity().getType(), SearchCriteria.Op.LIKE); @@ -1008,6 +1011,10 @@ private Pair, Integer> searchForEventIdsAndCount(ListEventsCmd cmd) { } } + if (jobId != null) { + sc.setParameters("asyncJobId", jobId); + } + if (keyword != null) { sc.setParameters("keywordType", "%" + keyword + "%"); sc.setParameters("keywordDescription", "%" + keyword + "%"); @@ -3208,7 +3215,6 @@ public ListResponse searchForAsyncJobs(ListAsyncJobsCmd cmd) { } private Pair, Integer> searchForAsyncJobsInternal(ListAsyncJobsCmd cmd) { - Account caller = CallContext.current().getCallingAccount(); List permittedAccounts = new ArrayList<>(); @@ -3219,8 +3225,16 @@ private Pair, Integer> searchForAsyncJobsInternal(ListAsync Boolean isRecursive = domainIdRecursiveListProject.second(); ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + // completed jobs are soft-deleted; anything but the pending-only default must include removed rows + final boolean filterByStatus = cmd.getJobStatuses() != null; + final boolean includeRemoved = filterByStatus || cmd.getEndDate() != null || cmd.getDuration() != null; + Filter searchFilter = new Filter(AsyncJobJoinVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); SearchBuilder sb = _jobJoinDao.createSearchBuilder(); + + if (filterByStatus) { + sb.and("statuses", sb.entity().getStatus(), SearchCriteria.Op.IN); + } sb.and("instanceTypeNEQ", sb.entity().getInstanceType(), SearchCriteria.Op.NEQ); sb.and("accountIdIN", sb.entity().getAccountId(), SearchCriteria.Op.IN); boolean accountJoinIsDone = false; @@ -3231,7 +3245,6 @@ private Pair, Integer> searchForAsyncJobsInternal(ListAsync } if (listProjectResourcesCriteria != null) { - if (listProjectResourcesCriteria == Project.ListProjectResourcesCriteria.ListProjectResourcesOnly) { sb.and("type", sb.entity().getAccountType(), SearchCriteria.Op.EQ); } else if (listProjectResourcesCriteria == Project.ListProjectResourcesCriteria.SkipProjectResources) { @@ -3245,13 +3258,20 @@ private Pair, Integer> searchForAsyncJobsInternal(ListAsync } if (cmd.getManagementServerId() != null) { - sb.and("executingMsid", sb.entity().getExecutingMsid(), SearchCriteria.Op.EQ); + // a completed job keeps only the server that completed it + sb.and().op("executingMsid", sb.entity().getExecutingMsid(), SearchCriteria.Op.EQ); + sb.or("completeMsid", sb.entity().getCompleteMsid(), SearchCriteria.Op.EQ); + sb.cp(); } Object keyword = cmd.getKeyword(); Object startDate = cmd.getStartDate(); + Object endDate = cmd.getEndDate(); SearchCriteria sc = sb.create(); + if (filterByStatus) { + sc.setParameters("statuses", cmd.getJobStatuses().toArray()); + } sc.setParameters("instanceTypeNEQ", AsyncJobVO.PSEUDO_JOB_INSTANCE_TYPE); if (listProjectResourcesCriteria != null) { sc.setParameters("type", Account.Type.PROJECT); @@ -3276,9 +3296,18 @@ private Pair, Integer> searchForAsyncJobsInternal(ListAsync sc.addAnd("created", SearchCriteria.Op.GTEQ, startDate); } - if (cmd.getManagementServerId() != null) { - ManagementServerHostVO msHost = msHostDao.findById(cmd.getManagementServerId()); - sc.setParameters("executingMsid", msHost.getMsid()); + if (endDate != null) { + sc.addAnd("created", SearchCriteria.Op.LTEQ, endDate); + } + + if (cmd.getDuration() != null) { + // rows are written in GMT + Date lastDate = new Date(DateUtil.currentGMTTime().getTime() - TimeUnit.HOURS.toMillis(cmd.getDuration())); + + SearchCriteria scc = _jobJoinDao.createSearchCriteria(); + scc.addOr("created", SearchCriteria.Op.GTEQ, lastDate); + scc.addOr("removed", SearchCriteria.Op.GTEQ, lastDate); + sc.addAnd("created", SearchCriteria.Op.SC, scc); } if (cmd.getResourceType() != null) { @@ -3293,7 +3322,16 @@ private Pair, Integer> searchForAsyncJobsInternal(ListAsync throw new InvalidParameterValueException(String.format("%s parameter must be used with %s parameter", ApiConstants.RESOURCE_ID, ApiConstants.RESOURCE_TYPE)); } - return _jobJoinDao.searchAndCount(sc, searchFilter); + if (cmd.getManagementServerId() != null) { + ManagementServerHostVO msHost = msHostDao.findById(cmd.getManagementServerId()); + if (msHost == null) { + throw new InvalidParameterValueException("Unable to find a management server with the specified id"); + } + sc.setParameters("executingMsid", msHost.getMsid()); + sc.setParameters("completeMsid", msHost.getMsid()); + } + + return _jobJoinDao.searchAndCount(sc, searchFilter, includeRemoved); } @Override diff --git a/server/src/main/java/com/cloud/api/query/dao/AsyncJobJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/AsyncJobJoinDaoImpl.java index 93af9a04e144..c200ba2e1534 100644 --- a/server/src/main/java/com/cloud/api/query/dao/AsyncJobJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/AsyncJobJoinDaoImpl.java @@ -69,8 +69,10 @@ public AsyncJobResponse newAsyncJobResponse(final AsyncJobJoinVO job) { jobResponse.setJobId(job.getUuid()); jobResponse.setJobStatus(job.getStatus()); jobResponse.setJobProcStatus(job.getProcessStatus()); - if (job.getExecutingMsid() != null) { - ManagementServerHostVO managementServer = managementServerHostDao.findByMsid(job.getExecutingMsid()); + // a finished job no longer has an executing server; the one that completed it is the answer then + final Long msid = job.getExecutingMsid() != null ? job.getExecutingMsid() : job.getCompleteMsid(); + if (msid != null) { + ManagementServerHostVO managementServer = managementServerHostDao.findByMsid(msid); if (managementServer != null) { jobResponse.setManagementServerId(managementServer.getUuid()); jobResponse.setManagementServerName(managementServer.getName()); diff --git a/server/src/main/java/com/cloud/api/query/vo/AsyncJobJoinVO.java b/server/src/main/java/com/cloud/api/query/vo/AsyncJobJoinVO.java index b4714c75def4..7fea7eb1a25a 100644 --- a/server/src/main/java/com/cloud/api/query/vo/AsyncJobJoinVO.java +++ b/server/src/main/java/com/cloud/api/query/vo/AsyncJobJoinVO.java @@ -79,6 +79,9 @@ public class AsyncJobJoinVO extends BaseViewVO implements ControlledViewEntity { @Column(name = "job_executing_msid") private Long executingMsid; + @Column(name = "job_complete_msid") + private Long completeMsid; + @Column(name = "job_status") private int status; @@ -107,6 +110,9 @@ public class AsyncJobJoinVO extends BaseViewVO implements ControlledViewEntity { @Column(name = "instance_uuid") private String instanceUuid; + @Column(name = "related") + private String related; + public AsyncJobJoinVO() { } @@ -208,6 +214,10 @@ public String getInstanceUuid() { return instanceUuid; } + public String getRelated() { + return related; + } + @Override public Class getEntityType() { return AsyncJob.class; @@ -222,6 +232,10 @@ public Long getExecutingMsid() { return executingMsid; } + public Long getCompleteMsid() { + return completeMsid; + } + @Override public String getProjectUuid() { // TODO Auto-generated method stub diff --git a/server/src/main/java/com/cloud/api/query/vo/EventJoinVO.java b/server/src/main/java/com/cloud/api/query/vo/EventJoinVO.java index bf731e0dbd6a..450616a61d22 100644 --- a/server/src/main/java/com/cloud/api/query/vo/EventJoinVO.java +++ b/server/src/main/java/com/cloud/api/query/vo/EventJoinVO.java @@ -69,6 +69,12 @@ public class EventJoinVO extends BaseViewVO implements ControlledViewEntity { @Column(name = "start_uuid") private String startUuid; + @Column(name = "async_job_id") + private Long asyncJobId; + + @Column(name = "async_job_uuid") + private String asyncJobUuid; + @Column(name = "parameters", length = 1024) private String parameters; @@ -229,6 +235,14 @@ public String getStartUuid() { return startUuid; } + public Long getAsyncJobId() { + return asyncJobId; + } + + public String getAsyncJobUuid() { + return asyncJobUuid; + } + public String getParameters() { return parameters; } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index c68dc390df2a..01b76ada39b4 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -122,6 +122,7 @@ import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.framework.messagebus.MessageSubscriber; import org.apache.cloudstack.framework.messagebus.PublishScope; +import org.apache.cloudstack.jobs.AsyncJobService; import org.apache.cloudstack.network.RoutedIpv4Manager; import org.apache.cloudstack.query.QueryService; import org.apache.cloudstack.region.PortableIp; @@ -630,6 +631,7 @@ protected void populateConfigValuesForValidationSet() { configValuesForValidation.add(VMLeaseManager.InstanceLeaseExpiryEventSchedulerInterval.key()); configValuesForValidation.add(VMLeaseManager.InstanceLeaseExpiryEventDaysBefore.key()); configValuesForValidation.add(AutoScaleManager.AutoScaleErroredInstanceThreshold.key()); + configValuesForValidation.add(AsyncJobService.CancelledJobInterval.key()); } protected void weightBasedParametersForValidation() { diff --git a/server/src/main/java/com/cloud/event/ActionEventUtils.java b/server/src/main/java/com/cloud/event/ActionEventUtils.java index 0ebb266fd7c2..6031b110684d 100644 --- a/server/src/main/java/com/cloud/event/ActionEventUtils.java +++ b/server/src/main/java/com/cloud/event/ActionEventUtils.java @@ -32,6 +32,9 @@ import org.apache.cloudstack.api.Identity; import org.apache.cloudstack.api.InternalIdentity; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.jobs.AsyncJob; +import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext; +import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.events.EventDistributor; import org.apache.commons.lang3.ObjectUtils; @@ -183,6 +186,7 @@ private static Event persistActionEvent(Long userId, Long accountId, Long domain event.setState(state); event.setDescription(description); event.setDisplay(eventDisplayEnabled); + event.setAsyncJobId(currentJobId()); if (domainId != null) { event.setDomainId(domainId); @@ -205,6 +209,20 @@ private static Event persistActionEvent(Long userId, Long accountId, Long domain return event; } + /** The API job this thread works for (a VM work job's parent), or null outside a job. */ + private static Long currentJobId() { + final AsyncJobExecutionContext context = AsyncJobExecutionContext.getCurrent(); + if (context == null || context.getJob() == null) { + return null; + } + final AsyncJob job = context.getJob(); + if (AsyncJobVO.JOB_DISPATCHER_PSEUDO.equals(job.getDispatcher())) { + return null; + } + final String related = job.getRelated(); + return StringUtils.isNotEmpty(related) ? Long.valueOf(related) : job.getId(); + } + private static void publishOnEventBus(Event eventRecord, long userId, long accountId, Long domainId, String eventCategory, String eventType, Event.State state, String description, String resourceUuid, String resourceType) { diff --git a/server/src/main/java/com/cloud/event/dao/EventJoinDaoImpl.java b/server/src/main/java/com/cloud/event/dao/EventJoinDaoImpl.java index b4316051e439..00ec0b9faf15 100644 --- a/server/src/main/java/com/cloud/event/dao/EventJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/event/dao/EventJoinDaoImpl.java @@ -106,6 +106,7 @@ public EventResponse newEventResponse(EventJoinVO event) { responseEvent.setId(event.getUuid()); responseEvent.setLevel(event.getLevel()); responseEvent.setParentId(event.getStartUuid()); + responseEvent.setJobId(event.getAsyncJobUuid()); responseEvent.setState(event.getState()); responseEvent.setUsername(event.getUserName()); if (event.getArchived()) { diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index f32857d7cf04..cd37d7897d9c 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -275,6 +275,7 @@ import org.apache.cloudstack.api.command.admin.usage.ListTrafficMonitorsCmd; import org.apache.cloudstack.api.command.admin.usage.ListTrafficTypeImplementorsCmd; import org.apache.cloudstack.api.command.admin.usage.ListTrafficTypesCmd; +import org.apache.cloudstack.api.command.admin.usage.ListUsageJobsCmd; import org.apache.cloudstack.api.command.admin.usage.ListUsageRecordsCmd; import org.apache.cloudstack.api.command.admin.usage.ListUsageTypesCmd; import org.apache.cloudstack.api.command.admin.usage.RemoveRawUsageRecordsCmd; @@ -3973,6 +3974,7 @@ public List> getCommands() { cmdList.add(DeleteTrafficTypeCmd.class); cmdList.add(GenerateUsageRecordsCmd.class); cmdList.add(ListUsageRecordsCmd.class); + cmdList.add(ListUsageJobsCmd.class); cmdList.add(RemoveRawUsageRecordsCmd.class); cmdList.add(ListTrafficMonitorsCmd.class); cmdList.add(ListTrafficTypeImplementorsCmd.class); diff --git a/server/src/main/java/com/cloud/usage/UsageServiceImpl.java b/server/src/main/java/com/cloud/usage/UsageServiceImpl.java index de8d4633d224..014c31b367aa 100644 --- a/server/src/main/java/com/cloud/usage/UsageServiceImpl.java +++ b/server/src/main/java/com/cloud/usage/UsageServiceImpl.java @@ -21,14 +21,18 @@ import java.util.List; import java.util.Map; import java.util.TimeZone; +import java.util.concurrent.TimeUnit; import javax.inject.Inject; import javax.naming.ConfigurationException; import com.cloud.configuration.ConfigurationManagerImpl; import org.apache.cloudstack.api.command.admin.usage.GenerateUsageRecordsCmd; +import org.apache.cloudstack.api.command.admin.usage.ListUsageJobsCmd; import org.apache.cloudstack.api.command.admin.usage.ListUsageRecordsCmd; import org.apache.cloudstack.api.command.admin.usage.RemoveRawUsageRecordsCmd; +import org.apache.cloudstack.api.response.ListResponse; +import org.apache.cloudstack.api.response.UsageJobResponse; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.usage.Usage; @@ -500,4 +504,72 @@ public boolean removeRawUsageRecords(RemoveRawUsageRecordsCmd cmd) throws Invali _usageDao.expungeAllOlderThan(interval, ConfigurationManagerImpl.DELETE_QUERY_BATCH_SIZE.value()); return true; } + + @Override + public ListResponse getUsageJobs(ListUsageJobsCmd cmd) { + Filter usageJobFilter = new Filter(UsageJobVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); + SearchCriteria sc = _usageJobDao.createSearchCriteria(); + + if (StringUtils.isNotBlank(cmd.getUsageServer())) { + sc.addAnd("host", SearchCriteria.Op.EQ, cmd.getUsageServer().trim()); + } + + Object startDate = cmd.getStartDate(); + if (startDate != null) { + sc.addAnd("startDate", SearchCriteria.Op.GTEQ, startDate); + } + + Object endDate = cmd.getEndDate(); + if (endDate != null) { + sc.addAnd("startDate", SearchCriteria.Op.LTEQ, endDate); + } + + if (cmd.getDuration() != null) { + // usage job timestamps are GMT + Date lastDate = new Date(DateUtil.currentGMTTime().getTime() - TimeUnit.HOURS.toMillis(cmd.getDuration())); + + SearchCriteria scc = _usageJobDao.createSearchCriteria(); + scc.addOr("startDate", SearchCriteria.Op.GTEQ, lastDate); + scc.addOr("endDate", SearchCriteria.Op.GTEQ, lastDate); + sc.addAnd("startDate", SearchCriteria.Op.SC, scc); + } + + Pair, Integer> usageJobs = null; + TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB); + try { + usageJobs = _usageJobDao.searchAndCount(sc, usageJobFilter); + } finally { + txn.close(); + + // switch back to VMOPS_DB + TransactionLegacy swap = TransactionLegacy.open(TransactionLegacy.CLOUD_DB); + swap.close(); + } + + ListResponse response = new ListResponse<>(); + if (usageJobs != null) { + List responses = new ArrayList<>(); + for (UsageJobVO job : usageJobs.first()) { + UsageJobResponse jobResponse = createUsageJobResponse(job); + responses.add(jobResponse); + } + response.setResponses(responses, usageJobs.second()); + } + + return response; + } + + private UsageJobResponse createUsageJobResponse(UsageJobVO job) { + UsageJobResponse jobResponse = new UsageJobResponse(); + jobResponse.setUsageServer(job.getHost()); + jobResponse.setJobType(job.getJobType()); + jobResponse.setScheduled(job.getScheduled()); + jobResponse.setStartDate(job.getStartDate()); + jobResponse.setEndDate(job.getEndDate()); + jobResponse.setExecutionTime(job.getExecTime()); + jobResponse.setSuccess(job.getSuccess()); + jobResponse.setHeartbeat(job.getHeartbeat()); + jobResponse.setObjectName("usagejobs"); + return jobResponse; + } } diff --git a/test/integration/smoke/test_async_jobs.py b/test/integration/smoke/test_async_jobs.py new file mode 100644 index 000000000000..5fc57b9cd349 --- /dev/null +++ b/test/integration/smoke/test_async_jobs.py @@ -0,0 +1,243 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" Tests for listing async jobs by status, period, resource and management server, and for cancelling them """ + +import time +import uuid + +from marvin.cloudstackAPI import (cancelAsyncJob, + listAsyncJobs, + listEvents, + listUsageJobs, + migrateVirtualMachine, + queryAsyncJobResult) +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.codes import (JOB_CANCELLED, + JOB_FAILED, + JOB_INPROGRESS, + JOB_SUCCEEDED, + PASS) +from marvin.lib.base import (Account, + DiskOffering, + ServiceOffering, + VirtualMachine) +from marvin.lib.common import (get_domain, + get_test_template, + get_zone, + list_hosts) +from nose.plugins.attrib import attr + +ALL_STATUSES = "%d,%d,%d,%d" % (JOB_INPROGRESS, JOB_SUCCEEDED, JOB_FAILED, JOB_CANCELLED) +CANCELLABLE_HYPERVISORS = ["kvm", "vmware", "xenserver"] + + +class TestAsyncJobsManagement(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + cls.testClient = super(TestAsyncJobsManagement, cls).getClsTestClient() + cls.apiclient = cls.testClient.getApiClient() + cls.testdata = cls.testClient.getParsedTestDataConfig() + cls.domain = get_domain(cls.apiclient) + cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + cls.hypervisor = cls.testClient.getHypervisorInfo() + cls.template = get_test_template(cls.apiclient, cls.zone.id, cls.hypervisor) + cls._cleanup = [] + + cls.service_offering = ServiceOffering.create(cls.apiclient, cls.testdata["service_offering"]) + cls._cleanup.append(cls.service_offering) + cls.disk_offering = DiskOffering.create(cls.apiclient, cls.testdata["disk_offering"]) + cls._cleanup.append(cls.disk_offering) + cls.account = Account.create(cls.apiclient, cls.testdata["account"], domainid=cls.domain.id) + cls._cleanup.append(cls.account) + + cls.testdata["virtual_machine"]["zoneid"] = cls.zone.id + cls.testdata["virtual_machine"]["template"] = cls.template.id + cls.virtual_machine = VirtualMachine.create( + cls.apiclient, + cls.testdata["virtual_machine"], + accountid=cls.account.name, + domainid=cls.account.domainid, + serviceofferingid=cls.service_offering.id, + diskofferingid=cls.disk_offering.id, + hypervisor=cls.hypervisor + ) + cls._cleanup.append(cls.virtual_machine) + # the deploy job is the completed job every listing test looks for + cls.deploy_job_id = cls.virtual_machine.jobid + + @classmethod + def tearDownClass(cls): + super(TestAsyncJobsManagement, cls).tearDownClass() + + def setUp(self): + self.cleanup = [] + + def tearDown(self): + super(TestAsyncJobsManagement, self).tearDown() + + def list_jobs(self, **kwargs): + cmd = listAsyncJobs.listAsyncJobsCmd() + cmd.listall = True + for key, value in kwargs.items(): + setattr(cmd, key, value) + return self.apiclient.listAsyncJobs(cmd) or [] + + @staticmethod + def find_job(jobs, jobid): + return next((job for job in jobs if job.jobid == jobid), None) + + def query_job(self, jobid): + cmd = queryAsyncJobResult.queryAsyncJobResultCmd() + cmd.jobid = jobid + return self.apiclient.queryAsyncJobResult(cmd) + + def wait_for_job(self, jobid, timeout=300): + deadline = time.time() + timeout + while time.time() < deadline: + job = self.query_job(jobid) + if job.jobstatus != JOB_INPROGRESS: + return job + time.sleep(3) + self.fail("Job %s still in progress after %d seconds" % (jobid, timeout)) + + def cancel_job(self, jobid): + cmd = cancelAsyncJob.cancelAsyncJobCmd() + cmd.jobid = jobid + return self.apiclient.cancelAsyncJob(cmd) + + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_01_default_listing_is_pending_only(self): + """Without a status filter only pending jobs are listed, so a completed job is absent""" + jobs = self.list_jobs() + self.assertIsNone(self.find_job(jobs, self.deploy_job_id), + "Completed deploy job %s listed without a status filter" % self.deploy_job_id) + for job in jobs: + self.assertEqual(job.jobstatus, JOB_INPROGRESS, "Non-pending job %s in the default listing" % job.jobid) + + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_02_list_completed_jobs_by_status_and_duration(self): + """A completed job is reachable by status, with the period, by ordinal or by name""" + job = self.find_job(self.list_jobs(jobstatus=JOB_SUCCEEDED, duration=1), self.deploy_job_id) + self.assertIsNotNone(job, "Deploy job %s not listed with jobstatus=%d duration=1" % (self.deploy_job_id, JOB_SUCCEEDED)) + self.assertEqual(job.jobstatus, JOB_SUCCEEDED) + self.assertTrue(job.completed, "Completed job has no completion time") + self.assertIn("DeployVMCmd", job.cmd) + self.assertEqual(job.jobinstanceid, self.virtual_machine.id) + + by_name = self.find_job(self.list_jobs(jobstatus="SUCCEEDED", duration=1), self.deploy_job_id) + self.assertIsNotNone(by_name, "Status filter by name did not match the deploy job") + + failed = self.find_job(self.list_jobs(jobstatus=JOB_FAILED, duration=1), self.deploy_job_id) + self.assertIsNone(failed, "Succeeded deploy job listed under the failed status") + + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_03_list_jobs_by_resource(self): + """Jobs are reachable from the resource they acted on""" + jobs = self.list_jobs(resourcetype="VirtualMachine", resourceid=self.virtual_machine.id, + jobstatus=ALL_STATUSES, duration=1) + self.assertIsNotNone(self.find_job(jobs, self.deploy_job_id), "Deploy job not listed for its instance") + for job in jobs: + self.assertEqual(job.jobinstanceid, self.virtual_machine.id, "Job %s for another resource listed" % job.jobid) + + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_04_list_jobs_by_management_server(self): + """Jobs are reachable from the management server that ran them""" + job = self.find_job(self.list_jobs(jobstatus=JOB_SUCCEEDED, duration=1), self.deploy_job_id) + self.assertIsNotNone(job) + msid = getattr(job, "managementserverid", None) + if not msid: + self.skipTest("Deploy job carries no management server id") + + jobs = self.list_jobs(managementserverid=msid, jobstatus=JOB_SUCCEEDED, duration=1) + self.assertIsNotNone(self.find_job(jobs, self.deploy_job_id), "Deploy job not listed for its management server") + for entry in jobs: + self.assertEqual(entry.managementserverid, msid, "Job %s of another management server listed" % entry.jobid) + + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_05_events_link_to_job(self): + """The events a job raised are reachable by its id""" + cmd = listEvents.listEventsCmd() + cmd.listall = True + cmd.jobid = self.deploy_job_id + events = self.apiclient.listEvents(cmd) or [] + self.assertTrue(events, "No events listed for deploy job %s" % self.deploy_job_id) + self.assertTrue(any(event.type.startswith("VM.CREATE") for event in events), "No VM.CREATE event for the deploy job") + for event in events: + self.assertEqual(event.jobid, self.deploy_job_id, "Event %s of another job listed" % event.id) + + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_06_cancel_completed_job_is_refused(self): + """A finished job cannot be cancelled and is left as it was""" + with self.assertRaises(Exception): + self.cancel_job(self.deploy_job_id) + self.assertEqual(self.query_job(self.deploy_job_id).jobstatus, JOB_SUCCEEDED, "Refused cancel changed the job") + + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_07_cancel_unknown_job_is_refused(self): + with self.assertRaises(Exception): + self.cancel_job(str(uuid.uuid4())) + + @attr(tags=["advanced", "advancedns", "smoke"], required_hardware="true") + def test_08_cancel_running_migration(self): + """Cancelling a live migration either stops it or is refused; the instance ends Running either way""" + if self.hypervisor.lower() not in CANCELLABLE_HYPERVISORS: + self.skipTest("Cancellation of hypervisor jobs is not implemented for %s" % self.hypervisor) + + vm = VirtualMachine.list(self.apiclient, id=self.virtual_machine.id)[0] + source_host = list_hosts(self.apiclient, id=vm.hostid)[0] + targets = [h for h in (list_hosts(self.apiclient, clusterid=source_host.clusterid, type="Routing", + state="Up", resourcestate="Enabled") or []) if h.id != source_host.id] + if not targets: + self.skipTest("Need a second host in cluster %s to migrate to" % source_host.clusterid) + + cmd = migrateVirtualMachine.migrateVirtualMachineCmd() + cmd.virtualmachineid = vm.id + cmd.hostid = targets[0].id + cmd.isAsync = "false" + jobid = self.apiclient.migrateVirtualMachine(cmd, method="GET").jobid + + cancelled = True + try: + self.cancel_job(jobid) + except Exception as e: + # the migration finished, or the hypervisor would not stop it: a refusal, not a failure + self.debug("Cancel of migration job %s refused: %s" % (jobid, e)) + cancelled = False + + job = self.wait_for_job(jobid) + if cancelled: + self.assertEqual(job.jobstatus, JOB_CANCELLED, "Accepted cancel did not end the job as cancelled") + else: + self.assertIn(job.jobstatus, [JOB_SUCCEEDED, JOB_FAILED], "Refused cancel left the job in an odd state") + + response = self.virtual_machine.getState(self.apiclient, VirtualMachine.RUNNING) + self.assertEqual(response[0], PASS, response[1]) + if cancelled: + vm = VirtualMachine.list(self.apiclient, id=self.virtual_machine.id)[0] + self.assertEqual(vm.hostid, source_host.id, "Cancelled migration left the instance on the target host") + + @attr(tags=["advanced", "advancedns", "smoke", "basic"], required_hardware="false") + def test_09_list_usage_jobs(self): + """Usage server jobs list without error, and filter by usage server""" + cmd = listUsageJobs.listUsageJobsCmd() + cmd.duration = 24 + self.apiclient.listUsageJobs(cmd) + + cmd = listUsageJobs.listUsageJobsCmd() + cmd.usageserver = "no-such-usage-server" + self.assertFalse(self.apiclient.listUsageJobs(cmd), "Jobs listed for a usage server that does not exist") diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 99bf2cf7aef9..658cf9d773d5 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -4425,5 +4425,23 @@ "GB*Month": "GB * Month", "IP*Month": "IP * Month", "Policy*Month": "Policy * Month", -"message.kms.key.optional": "Optional: Select a KMS key for encryption. If not selected, legacy passphrase encryption will be used." +"message.kms.key.optional": "Optional: Select a KMS key for encryption. If not selected, legacy passphrase encryption will be used.", +"label.jobs": "Jobs", +"label.usage.jobs": "Usage Jobs", +"label.jobresult": "Result", +"label.pending": "Pending", +"label.cancelled": "Cancelled", +"label.last.hour": "Last hour", +"label.last.24.hours": "Last 24 hours", +"label.last.7.days": "Last 7 days", +"label.usage.server": "Usage server", +"label.job.type": "Job type", +"label.execution.time": "Execution time (ms)", +"label.heartbeat": "Heartbeat", +"label.recurring": "Recurring", +"label.single": "Single", +"label.cancel.job": "Cancel job", +"message.confirm.cancel.job": "Please confirm that you want to cancel this job. Where the hypervisor supports it, the operation in progress is stopped as well; if it cannot be stopped, the request is refused and the job is left to finish.", +"message.cancel.job.success": "Job cancelled", +"label.activity": "Activity" } diff --git a/ui/src/config/router.js b/ui/src/config/router.js index b9c60bcd0c21..b55e40abd28a 100644 --- a/ui/src/config/router.js +++ b/ui/src/config/router.js @@ -30,7 +30,7 @@ import network from '@/config/section/network' import image from '@/config/section/image' import kms from '@/config/section/kms' import project from '@/config/section/project' -import event from '@/config/section/event' +import activity from '@/config/section/activity' import user from '@/config/section/user' import keyPair from '@/config/section/keypair' import account from '@/config/section/account' @@ -221,7 +221,7 @@ export function asyncRouterMap () { generateRouterMap(network), generateRouterMap(image), generateRouterMap(kms), - generateRouterMap(event), + generateRouterMap(activity), generateRouterMap(project), generateRouterMap(user), generateRouterMap(keyPair), diff --git a/ui/src/config/section/activity.js b/ui/src/config/section/activity.js new file mode 100644 index 000000000000..f25ec0f862a6 --- /dev/null +++ b/ui/src/config/section/activity.js @@ -0,0 +1,144 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import store from '@/store' + +export default { + name: 'activity', + title: 'label.activity', + icon: 'AuditOutlined', + children: [ + { + name: 'jobs', + title: 'label.jobs', + icon: 'ClockCircleOutlined', + permission: ['listAsyncJobs'], + component: () => import('@/views/activity/JobsView.vue') + }, + { + name: 'event', + title: 'label.events', + icon: 'ScheduleOutlined', + docHelp: 'adminguide/events.html', + permission: ['listEvents'], + columns: () => { + var fields = ['level', 'type', 'state', 'description', 'resource', 'username', 'account'] + if (store.getters.listAllProjects) { + fields.push('project') + } + fields.push(...['domain', 'created']) + return fields + }, + details: ['username', 'id', 'description', 'resourcetype', 'resourceid', 'state', 'level', 'type', 'account', 'domain', 'created'], + searchFilters: ['level', 'domainid', 'account', 'keyword', 'resourcetype', 'state'], + related: [{ + name: 'event', + title: 'label.event.timeline', + param: 'startid' + }], + filters: () => { + return ['active', 'archived'] + }, + actions: [ + { + api: 'archiveEvents', + icon: 'book-outlined', + label: 'label.archive.events', + message: 'message.confirm.archive.selected.events', + docHelp: 'adminguide/events.html#deleting-and-archiving-events-and-alerts', + dataView: true, + successMessage: 'label.event.archived', + groupAction: true, + groupMap: (selection) => { return [{ ids: selection.join(',') }] }, + args: ['ids'], + mapping: { + ids: { + value: (record) => { return record.id } + } + }, + show: (record) => { + return !(record.archived) + }, + groupShow: (selectedItems) => { + return selectedItems.filter(x => { return !(x.archived) }).length > 0 + } + }, + { + api: 'deleteEvents', + icon: 'delete-outlined', + label: 'label.delete.events', + message: 'message.confirm.remove.selected.events', + docHelp: 'adminguide/events.html#deleting-and-archiving-events-and-alerts', + dataView: true, + successMessage: 'label.event.deleted', + groupAction: true, + groupMap: (selection) => { return [{ ids: selection.join(',') }] }, + args: ['ids'], + mapping: { + ids: { + value: (record) => { return record.id } + } + } + } + ] + }, + { + name: 'alert', + title: 'label.alerts', + icon: 'FlagOutlined', + docHelp: 'adminguide/management.html#administrator-alerts', + permission: ['listAlerts'], + columns: ['name', 'description', 'type', 'sent'], + details: ['name', 'id', 'type', 'sent', 'description'], + searchFilters: ['name', 'type'], + actions: [ + { + api: 'archiveAlerts', + icon: 'book-outlined', + label: 'label.archive.alerts', + message: 'message.confirm.archive.selected.alerts', + docHelp: 'adminguide/events.html#deleting-and-archiving-events-and-alerts', + dataView: true, + groupAction: true, + groupMap: (selection) => { return [{ ids: selection.join(',') }] }, + args: ['ids'], + mapping: { + ids: { + value: (record) => { return record.id } + } + } + }, + { + api: 'deleteAlerts', + icon: 'delete-outlined', + label: 'label.delete.alerts', + message: 'message.confirm.remove.selected.alerts', + docHelp: 'adminguide/events.html#deleting-and-archiving-events-and-alerts', + dataView: true, + groupAction: true, + groupMap: (selection) => { return [{ ids: selection.join(',') }] }, + args: ['ids'], + mapping: { + ids: { + value: (record) => { return record.id } + } + } + } + ] + } + ] +} diff --git a/ui/src/config/section/event.js b/ui/src/config/section/event.js deleted file mode 100644 index b07f3ba37c4e..000000000000 --- a/ui/src/config/section/event.js +++ /dev/null @@ -1,86 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -import store from '@/store' - -export default { - name: 'event', - title: 'label.events', - icon: 'ScheduleOutlined', - docHelp: 'adminguide/events.html', - permission: ['listEvents'], - columns: () => { - var fields = ['level', 'type', 'state', 'description', 'resource', 'username', 'account'] - if (store.getters.listAllProjects) { - fields.push('project') - } - fields.push(...['domain', 'created']) - return fields - }, - details: ['username', 'id', 'description', 'resourcetype', 'resourceid', 'state', 'level', 'type', 'account', 'domain', 'created'], - searchFilters: ['level', 'domainid', 'account', 'keyword', 'resourcetype', 'state'], - related: [{ - name: 'event', - title: 'label.event.timeline', - param: 'startid' - }], - filters: () => { - return ['active', 'archived'] - }, - actions: [ - { - api: 'archiveEvents', - icon: 'book-outlined', - label: 'label.archive.events', - message: 'message.confirm.archive.selected.events', - docHelp: 'adminguide/events.html#deleting-and-archiving-events-and-alerts', - dataView: true, - successMessage: 'label.event.archived', - groupAction: true, - groupMap: (selection) => { return [{ ids: selection.join(',') }] }, - args: ['ids'], - mapping: { - ids: { - value: (record) => { return record.id } - } - }, - show: (record) => { - return !(record.archived) - }, - groupShow: (selectedItems) => { - return selectedItems.filter(x => { return !(x.archived) }).length > 0 - } - }, - { - api: 'deleteEvents', - icon: 'delete-outlined', - label: 'label.delete.events', - message: 'message.confirm.remove.selected.events', - docHelp: 'adminguide/events.html#deleting-and-archiving-events-and-alerts', - dataView: true, - successMessage: 'label.event.deleted', - groupAction: true, - groupMap: (selection) => { return [{ ids: selection.join(',') }] }, - args: ['ids'], - mapping: { - ids: { - value: (record) => { return record.id } - } - } - } - ] -} diff --git a/ui/src/config/section/infra.js b/ui/src/config/section/infra.js index dc365b74c930..1771d8ee8afa 100644 --- a/ui/src/config/section/infra.js +++ b/ui/src/config/section/infra.js @@ -93,50 +93,6 @@ export default { docHelp: 'adminguide/management.html#metrics', permission: ['listDbMetrics', 'listUsageServerMetrics'], component: () => import('@/views/infra/Metrics.vue') - }, - { - name: 'alert', - title: 'label.alerts', - icon: 'FlagOutlined', - docHelp: 'adminguide/management.html#administrator-alerts', - permission: ['listAlerts'], - columns: ['name', 'description', 'type', 'sent'], - details: ['name', 'id', 'type', 'sent', 'description'], - searchFilters: ['name', 'type'], - actions: [ - { - api: 'archiveAlerts', - icon: 'book-outlined', - label: 'label.archive.alerts', - message: 'message.confirm.archive.selected.alerts', - docHelp: 'adminguide/events.html#deleting-and-archiving-events-and-alerts', - dataView: true, - groupAction: true, - groupMap: (selection) => { return [{ ids: selection.join(',') }] }, - args: ['ids'], - mapping: { - ids: { - value: (record) => { return record.id } - } - } - }, - { - api: 'deleteAlerts', - icon: 'delete-outlined', - label: 'label.delete.alerts', - message: 'message.confirm.remove.selected.alerts', - docHelp: 'adminguide/events.html#deleting-and-archiving-events-and-alerts', - dataView: true, - groupAction: true, - groupMap: (selection) => { return [{ ids: selection.join(',') }] }, - args: ['ids'], - mapping: { - ids: { - value: (record) => { return record.id } - } - } - } - ] } ] } diff --git a/ui/src/config/section/infra/managementServers.js b/ui/src/config/section/infra/managementServers.js index 4bf54943fd85..70d2d06b375b 100644 --- a/ui/src/config/section/infra/managementServers.js +++ b/ui/src/config/section/infra/managementServers.js @@ -48,8 +48,14 @@ export default { component: shallowRef(defineAsyncComponent(() => import('@/views/infra/ManagementServerPeerTab.vue'))) }, { - name: 'pending.jobs', - component: shallowRef(defineAsyncComponent(() => import('@/views/infra/AsyncJobsTab.vue'))) + name: 'jobs', + component: shallowRef(defineAsyncComponent(() => import('@/views/infra/AsyncJobsTab.vue'))), + show: () => { return 'listAsyncJobs' in store.getters.apis } + }, + { + name: 'usage.jobs', + component: shallowRef(defineAsyncComponent(() => import('@/views/infra/UsageJobsTab.vue'))), + show: () => { return 'listUsageJobs' in store.getters.apis } }, { name: 'connected.agents', diff --git a/ui/src/config/section/storage.js b/ui/src/config/section/storage.js index 75bdfd4d5fa6..0342fb6d8402 100644 --- a/ui/src/config/section/storage.js +++ b/ui/src/config/section/storage.js @@ -87,6 +87,12 @@ export default { component: shallowRef(defineAsyncComponent(() => import('@/components/view/EventsTab.vue'))), show: () => { return 'listEvents' in store.getters.apis } }, + { + name: 'jobs', + resourceType: 'Volume', + component: shallowRef(defineAsyncComponent(() => import('@/views/infra/AsyncJobsTab.vue'))), + show: () => { return 'listAsyncJobs' in store.getters.apis } + }, { name: 'comments', component: shallowRef(defineAsyncComponent(() => import('@/components/view/AnnotationsTab.vue'))) diff --git a/ui/src/views/activity/JobsView.vue b/ui/src/views/activity/JobsView.vue new file mode 100644 index 000000000000..29d6511f449c --- /dev/null +++ b/ui/src/views/activity/JobsView.vue @@ -0,0 +1,39 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + + + + + diff --git a/ui/src/views/compute/InstanceTab.vue b/ui/src/views/compute/InstanceTab.vue index d125995e3e1c..f840c10c1c6d 100644 --- a/ui/src/views/compute/InstanceTab.vue +++ b/ui/src/views/compute/InstanceTab.vue @@ -110,6 +110,9 @@ + + + - - - - - +
+ + + + + {{ option.label }} + + + + + + + {{ option.label }} + + + + + + + {{ $t('label.refresh') }} + + + + + + + + + + + + + + + +
+ + diff --git a/ui/src/views/infra/UsageJobsTab.vue b/ui/src/views/infra/UsageJobsTab.vue new file mode 100644 index 000000000000..84088b581198 --- /dev/null +++ b/ui/src/views/infra/UsageJobsTab.vue @@ -0,0 +1,189 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + + + diff --git a/utils/src/main/java/com/cloud/utils/SerialVersionUID.java b/utils/src/main/java/com/cloud/utils/SerialVersionUID.java index 30844953e1aa..bf689a243d13 100644 --- a/utils/src/main/java/com/cloud/utils/SerialVersionUID.java +++ b/utils/src/main/java/com/cloud/utils/SerialVersionUID.java @@ -72,4 +72,5 @@ public interface SerialVersionUID { public static final long OriginDeniedException = Base | 0x30; public static final long StorageAccessException = Base | 0x31; public static final long EncryptionException = Base | 0x32; + public static final long OperationCancelledException = Base | 0x33; } diff --git a/utils/src/main/java/com/cloud/utils/exception/CSExceptionErrorCode.java b/utils/src/main/java/com/cloud/utils/exception/CSExceptionErrorCode.java index 8b25d77e7f3e..f458f7de0738 100644 --- a/utils/src/main/java/com/cloud/utils/exception/CSExceptionErrorCode.java +++ b/utils/src/main/java/com/cloud/utils/exception/CSExceptionErrorCode.java @@ -75,6 +75,7 @@ public class CSExceptionErrorCode { ExceptionErrorCodeMap.put("com.cloud.exception.UnavailableCommandException", 4555); ExceptionErrorCodeMap.put("com.cloud.exception.OperationTimedoutException", 4560); ExceptionErrorCodeMap.put("org.apache.cloudstack.framework.kms.KMSException", 4561); + ExceptionErrorCodeMap.put("com.cloud.exception.OperationCancelledException", 4565); // Have a special error code for ServerApiException when it is // thrown in a standalone manner when failing to detect any of the above diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareClient.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareClient.java index 0e46f2e03cd6..3f47d1e44702 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareClient.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareClient.java @@ -40,6 +40,7 @@ import org.apache.cloudstack.utils.security.SSLUtils; import org.apache.cloudstack.utils.security.SecureSSLSocketFactory; +import com.cloud.utils.Pair; import com.cloud.utils.StringUtils; import org.w3c.dom.Element; @@ -148,6 +149,8 @@ private static void trustAllHttpsCertificates() throws Exception { private String serviceCookie; private final static String SVC_INST_NAME = "ServiceInstance"; private int vCenterSessionTimeout = 1200000; // Timeout in milliseconds + private static final long CANCEL_TASK_WAIT_MS = 30000; + private static final long CANCEL_TASK_POLL_MS = 500; private boolean isConnected = false; @@ -415,6 +418,12 @@ public boolean waitForTask(ManagedObjectReference task) throws InvalidPropertyFa boolean retVal = false; + // register with the executing request; a request already cancelled cancels the new task at once + if (VmwareTaskRegistry.taskStarted(task, this)) { + LOGGER.info("Request executing on this thread was already cancelled, cancelling newly created task {}", task.getValue()); + cancelTask(task); + } + try { // info has a property - state for state of the task Object[] result = waitForValues(task, new String[] { "info.state", "info.error" }, new String[] { "state" }, new Object[][] { new Object[] { @@ -460,6 +469,8 @@ public boolean waitForTask(ManagedObjectReference task) throws InvalidPropertyFa throw new RuntimeException(((LocalizedMethodFault)result[1]).getLocalizedMessage()); } } + } finally { + VmwareTaskRegistry.taskFinished(task); } return retVal; } @@ -780,11 +791,23 @@ public int getVcenterSessionTimeout() { return vCenterSessionTimeout; } - public void cancelTask(ManagedObjectReference task) throws Exception { - TaskInfo info = (TaskInfo)(getDynamicProperty(task, "info")); + public boolean isTaskCancellable(ManagedObjectReference task) throws Exception { + TaskInfo info = getDynamicProperty(task, "info"); + if (info == null || info.getState() == null) { + return false; + } + if (info.getState().equals(TaskInfoState.SUCCESS) || info.getState().equals(TaskInfoState.ERROR)) { + return false; + } + return info.isCancelable(); + } + + /** Asks vCenter to cancel and waits, bounded, for a terminal state. */ + public Pair cancelTask(ManagedObjectReference task) throws Exception { + TaskInfo info = getDynamicProperty(task, "info"); if (info == null) { LOGGER.warn("Unable to get the task info, so couldn't cancel the task"); - return; + return new Pair<>(false, "unable to get the task info"); } String taskName = StringUtils.isNotBlank(info.getName()) ? info.getName() : "Unknown"; @@ -794,40 +817,45 @@ public void cancelTask(ManagedObjectReference task) throws Exception { if (info.getState().equals(TaskInfoState.SUCCESS)) { LOGGER.debug(taskName + " task successfully completed for the entity " + entityName + ", can't cancel it"); - return; + return new Pair<>(false, "task " + taskName + " already completed"); } if (info.getState().equals(TaskInfoState.ERROR)) { LOGGER.debug(taskName + " task execution failed for the entity " + entityName + ", can't cancel it"); - return; + return new Pair<>(false, "task " + taskName + " already failed"); } LOGGER.debug(taskName + " task pending for the entity " + entityName + ", trying to cancel"); if (!info.isCancelable()) { LOGGER.warn(taskName + " task will continue to run on vCenter because it can't be cancelled"); - return; + return new Pair<>(false, "task " + taskName + " is not cancelable on vCenter"); } LOGGER.debug("Cancelling task " + taskName + " of the entity " + entityName); getService().cancelTask(task); - // Since task cancellation is asynchronous, wait for the task to be cancelled - Object[] result = waitForValues(task, new String[] {"info.state", "info.error"}, new String[] {"state"}, - new Object[][] {new Object[] {TaskInfoState.SUCCESS, TaskInfoState.ERROR}}); - - if (result != null && result.length == 2) { //result for 2 properties: info.state, info.error - if (result[0].equals(TaskInfoState.SUCCESS)) { - LOGGER.warn("Failed to cancel" + taskName + " task of the entity " + entityName + ", the task successfully completed"); + // poll rather than waitForValues: it is synchronized and the thread waiting on this task holds its monitor + final long deadline = System.currentTimeMillis() + CANCEL_TASK_WAIT_MS; + while (System.currentTimeMillis() < deadline) { + info = getDynamicProperty(task, "info"); + TaskInfoState state = info != null ? info.getState() : null; + if (TaskInfoState.SUCCESS.equals(state)) { + LOGGER.warn("Failed to cancel " + taskName + " task of the entity " + entityName + ", the task successfully completed"); + return new Pair<>(false, "task " + taskName + " completed before it could be cancelled"); } - - if (result[1] instanceof LocalizedMethodFault) { - MethodFault fault = ((LocalizedMethodFault)result[1]).getFault(); - if (fault instanceof RequestCanceled) { + if (TaskInfoState.ERROR.equals(state)) { + LocalizedMethodFault error = info.getError(); + if (error != null && error.getFault() instanceof RequestCanceled) { LOGGER.debug(taskName + " task of the entity " + entityName + " was successfully cancelled"); + return new Pair<>(true, "task " + taskName + " cancelled"); } - } else { - LOGGER.warn("Couldn't cancel " + taskName + " task of the entity " + entityName + " due to " + ((LocalizedMethodFault)result[1]).getLocalizedMessage()); + String reason = error != null ? error.getLocalizedMessage() : "unknown error"; + LOGGER.warn("Couldn't cancel " + taskName + " task of the entity " + entityName + " due to " + reason); + return new Pair<>(false, "task " + taskName + " failed: " + reason); } + Thread.sleep(CANCEL_TASK_POLL_MS); } + LOGGER.warn(taskName + " task of the entity " + entityName + " is still running after the cancel request"); + return new Pair<>(false, "task " + taskName + " did not stop within " + CANCEL_TASK_WAIT_MS / 1000 + " seconds of the cancel request"); } } diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareTaskRegistry.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareTaskRegistry.java new file mode 100644 index 000000000000..a9f25d6e891a --- /dev/null +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareTaskRegistry.java @@ -0,0 +1,160 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.vmware.util; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import com.cloud.utils.Pair; +import com.vmware.vim25.ManagedObjectReference; + +/** Per agent request sequence, the vCenter tasks a resource is waiting on; one instance per host. VmwareClient.waitForTask is the single registration point. */ +public class VmwareTaskRegistry { + private static final Logger LOGGER = LogManager.getLogger(VmwareTaskRegistry.class); + + private static final ThreadLocal CURRENT = new ThreadLocal<>(); + + private final Map requests = new ConcurrentHashMap<>(); + + private static final class ActiveTask { + final ManagedObjectReference mor; + final VmwareClient client; + + ActiveTask(final ManagedObjectReference mor, final VmwareClient client) { + this.mor = mor; + this.client = client; + } + } + + private static final class RequestState { + final Map tasks = new ConcurrentHashMap<>(); + volatile boolean cancelRequested; + } + + private static final class RequestScope { + final VmwareTaskRegistry registry; + final long sequence; + + RequestScope(final VmwareTaskRegistry registry, final long sequence) { + this.registry = registry; + this.sequence = sequence; + } + } + + /** A null sequence (not from the agent layer) leaves the thread unscoped. */ + public void beginRequest(final Long sequence) { + if (sequence == null) { + return; + } + requests.putIfAbsent(sequence, new RequestState()); + CURRENT.set(new RequestScope(this, sequence)); + } + + public void endRequest(final Long sequence) { + CURRENT.remove(); + if (sequence != null) { + requests.remove(sequence); + } + } + + public boolean wasCancelRequested(final Long sequence) { + if (sequence == null) { + return false; + } + final RequestState state = requests.get(sequence); + return state != null && state.cancelRequested; + } + + /** Returns true when the request was already cancelled, so the caller cancels the new task at once. */ + static boolean taskStarted(final ManagedObjectReference mor, final VmwareClient client) { + final RequestScope scope = CURRENT.get(); + if (scope == null || mor == null) { + return false; + } + final RequestState state = scope.registry.requests.get(scope.sequence); + if (state == null) { + return false; + } + state.tasks.put(mor.getValue(), new ActiveTask(mor, client)); + return state.cancelRequested; + } + + static void taskFinished(final ManagedObjectReference mor) { + final RequestScope scope = CURRENT.get(); + if (scope == null || mor == null) { + return; + } + final RequestState state = scope.registry.requests.get(scope.sequence); + if (state != null) { + state.tasks.remove(mor.getValue()); + } + } + + /** Unknown sequences are not cancellable; with tasks in flight, every one must be cancelable on vCenter. */ + public boolean isCancellable(final long sequence) { + final RequestState state = requests.get(sequence); + if (state == null) { + return false; + } + for (final ActiveTask task : state.tasks.values()) { + try { + if (!task.client.isTaskCancellable(task.mor)) { + LOGGER.debug("vCenter task {} of request sequence {} is not cancellable", task.mor.getValue(), sequence); + return false; + } + } catch (final Exception e) { + LOGGER.warn("Unable to check whether vCenter task {} of request sequence {} is cancellable", task.mor.getValue(), sequence, e); + return false; + } + } + return true; + } + + /** Cancels the in-flight tasks and marks the request so later tasks are cancelled on arrival. */ + public boolean cancel(final long sequence) { + final RequestState state = requests.get(sequence); + if (state == null) { + return false; + } + state.cancelRequested = true; + + boolean allCancelled = true; + for (final ActiveTask task : state.tasks.values()) { + try { + final Pair result = task.client.cancelTask(task.mor); + if (result.first()) { + LOGGER.info("Cancelled vCenter task {} of request sequence {}", task.mor.getValue(), sequence); + } else { + LOGGER.info("Could not cancel vCenter task {} of request sequence {}: {}", task.mor.getValue(), sequence, result.second()); + allCancelled = false; + } + } catch (final Exception e) { + LOGGER.warn("Failed to cancel vCenter task {} of request sequence {}", task.mor.getValue(), sequence, e); + allCancelled = false; + } + } + return allCancelled; + } + + int activeTaskCount(final long sequence) { + final RequestState state = requests.get(sequence); + return state == null ? 0 : state.tasks.size(); + } +} diff --git a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/util/VmwareTaskRegistryTest.java b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/util/VmwareTaskRegistryTest.java new file mode 100644 index 000000000000..4445dfdd8293 --- /dev/null +++ b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/util/VmwareTaskRegistryTest.java @@ -0,0 +1,161 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.hypervisor.vmware.util; + +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.utils.Pair; +import com.vmware.vim25.ManagedObjectReference; + +@RunWith(MockitoJUnitRunner.class) +public class VmwareTaskRegistryTest { + + private static final long SEQ = 42L; + + @Mock + private VmwareClient client; + + private final VmwareTaskRegistry registry = new VmwareTaskRegistry(); + + @After + public void tearDown() { + registry.endRequest(SEQ); + } + + @Test + public void unknownSequenceIsNeitherCancellableNorCancelled() { + Assert.assertFalse(registry.isCancellable(999L)); + Assert.assertFalse(registry.cancel(999L)); + Assert.assertFalse(registry.wasCancelRequested(999L)); + } + + @Test + public void requestWithNoTaskInFlightIsCancellableByInterruptAlone() throws Exception { + registry.beginRequest(SEQ); + + Assert.assertTrue(registry.isCancellable(SEQ)); + Assert.assertTrue(registry.cancel(SEQ)); + Assert.assertTrue(registry.wasCancelRequested(SEQ)); + Mockito.verify(client, Mockito.never()).cancelTask(Mockito.any()); + } + + @Test + public void taskCreatedAfterCancelIsReportedSoTheClientCancelsItImmediately() { + registry.beginRequest(SEQ); + registry.cancel(SEQ); + + Assert.assertTrue(VmwareTaskRegistry.taskStarted(mor("task-late"), client)); + } + + @Test + public void cancellableTaskIsCancelledThroughItsClient() throws Exception { + final ManagedObjectReference mor = mor("task-1"); + Mockito.when(client.isTaskCancellable(mor)).thenReturn(true); + Mockito.when(client.cancelTask(mor)).thenReturn(new Pair<>(true, "cancelled")); + + registry.beginRequest(SEQ); + Assert.assertFalse(VmwareTaskRegistry.taskStarted(mor, client)); + + Assert.assertTrue(registry.isCancellable(SEQ)); + Assert.assertTrue(registry.cancel(SEQ)); + Mockito.verify(client).cancelTask(mor); + } + + @Test + public void nonCancellableTaskRefusesAndIsLeftAlone() throws Exception { + final ManagedObjectReference mor = mor("task-2"); + Mockito.when(client.isTaskCancellable(mor)).thenReturn(false); + + registry.beginRequest(SEQ); + VmwareTaskRegistry.taskStarted(mor, client); + + Assert.assertFalse(registry.isCancellable(SEQ)); + Mockito.verify(client, Mockito.never()).cancelTask(Mockito.any()); + } + + @Test + public void cancelReportsFailureWhenVCenterDoesNotCancel() throws Exception { + final ManagedObjectReference mor = mor("task-3"); + Mockito.when(client.cancelTask(mor)).thenReturn(new Pair<>(false, "completed first")); + + registry.beginRequest(SEQ); + VmwareTaskRegistry.taskStarted(mor, client); + + Assert.assertFalse(registry.cancel(SEQ)); + } + + @Test + public void finishedTaskIsNoLongerTracked() throws Exception { + final ManagedObjectReference mor = mor("task-4"); + registry.beginRequest(SEQ); + VmwareTaskRegistry.taskStarted(mor, client); + Assert.assertEquals(1, registry.activeTaskCount(SEQ)); + + VmwareTaskRegistry.taskFinished(mor); + + Assert.assertEquals(0, registry.activeTaskCount(SEQ)); + Assert.assertTrue(registry.cancel(SEQ)); + Mockito.verify(client, Mockito.never()).cancelTask(Mockito.any()); + } + + @Test + public void endRequestForgetsTheSequence() { + registry.beginRequest(SEQ); + registry.endRequest(SEQ); + + Assert.assertFalse(registry.isCancellable(SEQ)); + Assert.assertFalse(VmwareTaskRegistry.taskStarted(mor("task-5"), client)); + Assert.assertEquals(0, registry.activeTaskCount(SEQ)); + } + + @Test + public void unscopedThreadRegistersNothing() throws Exception { + registry.beginRequest(SEQ); + final AtomicBoolean registered = new AtomicBoolean(true); + + // scope is per thread + final Thread other = new Thread(() -> registered.set(VmwareTaskRegistry.taskStarted(mor("task-6"), client))); + other.start(); + other.join(); + + Assert.assertFalse(registered.get()); + Assert.assertEquals(0, registry.activeTaskCount(SEQ)); + } + + @Test + public void nullSequenceLeavesTheThreadUnscoped() { + registry.beginRequest(null); + + Assert.assertFalse(VmwareTaskRegistry.taskStarted(mor("task-7"), client)); + registry.endRequest(null); + } + + private static ManagedObjectReference mor(final String value) { + final ManagedObjectReference mor = new ManagedObjectReference(); + mor.setType("Task"); + mor.setValue(value); + return mor; + } +}