From fc06c9652dae1bc980456d593bb99f99ec0c7b81 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 28 Oct 2026 13:46:00 +0530 Subject: [PATCH 1/9] veeam-control-service: fix issues around missing apis, parameter handling, responses --- .../command/user/vm/RemoveNicFromVMCmd.java | 8 + .../apache/cloudstack/veeam/RouteHandler.java | 48 ++- .../cloudstack/veeam/VeeamControlService.java | 6 + .../veeam/VeeamControlServiceImpl.java | 1 + .../adapter/AsyncJobFailedException.java | 38 ++ .../veeam/adapter/ServerAdapter.java | 185 ++++++++-- .../veeam/api/DataCentersRouteHandler.java | 4 +- .../veeam/api/DisksRouteHandler.java | 14 +- .../veeam/api/StorageDomainsRouteHandler.java | 110 ++++++ .../cloudstack/veeam/api/VmsRouteHandler.java | 132 +++++-- .../AsyncJobJoinVOToJobConverter.java | 16 + .../cloudstack/veeam/api/dto/Fault.java | 3 + .../veeam/api/dto/ResourceAction.java | 9 + .../veeam/api/request/ListQuery.java | 26 +- .../spring-veeam-control-service-context.xml | 1 + .../cloudstack/veeam/RouteHandlerTest.java | 72 ++++ .../veeam/VeeamControlServiceImplTest.java | 2 +- .../veeam/adapter/ServerAdapterTest.java | 303 ++++++++++++++-- .../api/DataCentersRouteHandlerTest.java | 2 +- .../veeam/api/DisksRouteHandlerTest.java | 54 ++- .../api/StorageDomainsRouteHandlerTest.java | 88 +++++ .../veeam/api/VmsRouteHandlerTest.java | 325 +++++++++++++++++- .../veeam/api/request/ListQueryTest.java | 42 +++ .../api/query/dao/StoragePoolJoinDao.java | 6 +- .../api/query/dao/StoragePoolJoinDaoImpl.java | 34 +- .../cloud/api/query/dao/UserVmJoinDao.java | 3 + .../api/query/dao/UserVmJoinDaoImpl.java | 17 +- .../query/dao/StoragePoolJoinDaoImplTest.java | 71 ++-- .../api/query/dao/UserVmJoinDaoImplTest.java | 30 ++ 29 files changed, 1533 insertions(+), 117 deletions(-) create mode 100644 plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/AsyncJobFailedException.java create mode 100644 plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/StorageDomainsRouteHandler.java create mode 100644 plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/RouteHandlerTest.java create mode 100644 plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/StorageDomainsRouteHandlerTest.java diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/RemoveNicFromVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/RemoveNicFromVMCmd.java index cfbc64339909..356293859646 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/RemoveNicFromVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/RemoveNicFromVMCmd.java @@ -67,6 +67,14 @@ public Long getNicId() { return nicId; } + public void setVmId(Long vmId) { + this.vmId = vmId; + } + + public void setNicId(Long nicId) { + this.nicId = nicId; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/RouteHandler.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/RouteHandler.java index 375222f47680..7908ee24d14c 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/RouteHandler.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/RouteHandler.java @@ -23,6 +23,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.cloudstack.veeam.api.dto.ResourceAction; import org.apache.cloudstack.veeam.utils.Negotiation; import org.apache.logging.log4j.Logger; @@ -74,8 +75,51 @@ static String getRequestData(HttpServletRequest req) { } } + /** + * Interprets true/false (case-insensitive) and 1/0 as boolean. + * Returns defaultValue when the value is null, empty or unrecognized. + */ + static boolean parseBoolean(String value, boolean defaultValue) { + if (value == null) { + return defaultValue; + } + String trimmed = value.trim(); + if ("1".equals(trimmed) || Boolean.TRUE.toString().equalsIgnoreCase(trimmed)) { + return true; + } + if ("0".equals(trimmed) || Boolean.FALSE.toString().equalsIgnoreCase(trimmed)) { + return false; + } + return defaultValue; + } + + /** + * Returns whether the request asks for async processing. Mimics oVirt: the async parameter present + * without a value (e.g. ?async) means true, while a missing or unrecognized value means false. + */ static boolean isRequestAsync(HttpServletRequest req) { - String asyncStr = req.getParameter("async"); - return Boolean.TRUE.toString().equals(asyncStr); + String async = req.getParameter("async"); + if (async != null && async.isEmpty()) { + return true; + } + return parseBoolean(async, false); + } + + /** + * Writes the response for a successful DELETE the way oVirt does: 202 Accepted without a body for + * async requests, otherwise 200 OK with a completed action, linking the job when there is one. + */ + static void writeDeleteResponse(final HttpServletResponse resp, final boolean async, final ResourceAction action, + final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (async) { + io.getWriter().write(resp, HttpServletResponse.SC_ACCEPTED, null, outFormat); + return; + } + ResourceAction response = new ResourceAction(); + if (action != null) { + response.setJob(action.getJob()); + } + response.setStatus("complete"); + io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); } } diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/VeeamControlService.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/VeeamControlService.java index 9255ab34aed2..d92d1e5927ac 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/VeeamControlService.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/VeeamControlService.java @@ -61,6 +61,12 @@ public interface VeeamControlService extends PluggableService, Configurable { "", "Comma-separated list of CIDR blocks representing clients allowed to access the API. " + "If empty, all clients will be allowed. Example: '192.168.1.1/24,192.168.2.100/32", true); + ConfigKey JobWaitTimeout = new ConfigKey<>("Advanced", Integer.class, + "integration.veeam.control.job.wait.timeout", + "3600", "Time in seconds that synchronous (non-async) requests wait for the underlying CloudStack " + + "job to complete. When it elapses the request fails, but the job keeps running in the background and " + + "may still complete. Values of 0 or less fall back to the default.", true); + ConfigKey DeveloperLogs = new ConfigKey<>("Developer", Boolean.class, "integration.veeam.control.developer.logs", "false", "Enable verbose logging for development and troubleshooting purposes. " + "Logs will include detailed information about API requests, responses, and internal operations.", false); diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/VeeamControlServiceImpl.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/VeeamControlServiceImpl.java index 3ad5d2390978..19105bed803b 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/VeeamControlServiceImpl.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/VeeamControlServiceImpl.java @@ -151,6 +151,7 @@ public ConfigKey[] getConfigKeys() { InstanceRestoreAssignOwner, InstanceEncryptVolumes, AllowedClientCidrs, + JobWaitTimeout, DeveloperLogs }; } diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/AsyncJobFailedException.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/AsyncJobFailedException.java new file mode 100644 index 000000000000..9ddadcea6eff --- /dev/null +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/AsyncJobFailedException.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 org.apache.cloudstack.veeam.adapter; + +import com.cloud.utils.exception.CloudRuntimeException; + +/** + * Thrown when a synchronous request did not get a successful result from its async job, either because + * the job failed or was cancelled, or because waiting for it timed out or was interrupted. In the latter + * case the job may still be running in the background. + */ +public class AsyncJobFailedException extends CloudRuntimeException { + private final String jobUuid; + + public AsyncJobFailedException(String jobUuid, String message) { + super(message); + this.jobUuid = jobUuid; + } + + public String getJobUuid() { + return jobUuid; + } +} diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java index 8297c74864c3..d99aa53996c1 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java @@ -56,6 +56,7 @@ import org.apache.cloudstack.api.command.admin.vm.AssignVMCmd; import org.apache.cloudstack.api.command.admin.vm.DeployVMCmdByAdmin; import org.apache.cloudstack.api.command.admin.vm.DestroyVMCmdByAdmin; +import org.apache.cloudstack.api.command.admin.volume.DestroyVolumeCmdByAdmin; import org.apache.cloudstack.api.command.user.backup.ListBackupsCmd; import org.apache.cloudstack.api.command.user.job.ListAsyncJobsCmd; import org.apache.cloudstack.api.command.user.network.ListNetworksCmd; @@ -67,6 +68,7 @@ import org.apache.cloudstack.api.command.user.vm.DestroyVMCmd; import org.apache.cloudstack.api.command.user.vm.ListNicsCmd; import org.apache.cloudstack.api.command.user.vm.ListVMsCmd; +import org.apache.cloudstack.api.command.user.vm.RemoveNicFromVMCmd; import org.apache.cloudstack.api.command.user.vm.StartVMCmd; import org.apache.cloudstack.api.command.user.vm.StopVMCmd; import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd; @@ -82,6 +84,7 @@ import org.apache.cloudstack.api.command.user.volume.ListVolumesCmd; import org.apache.cloudstack.api.command.user.volume.UpdateVolumeCmd; import org.apache.cloudstack.api.command.user.zone.ListZonesCmd; +import org.apache.cloudstack.api.response.ExceptionResponse; import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.api.response.ServiceOfferingResponse; import org.apache.cloudstack.backup.BackupVO; @@ -94,6 +97,7 @@ import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.jobs.dao.AsyncJobDao; import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; +import org.apache.cloudstack.jobs.JobInfo; import org.apache.cloudstack.query.QueryService; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; @@ -142,6 +146,7 @@ import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; +import com.cloud.api.ApiSerializerHelper; import com.cloud.api.query.dao.AsyncJobJoinDao; import com.cloud.api.query.dao.DataCenterJoinDao; import com.cloud.api.query.dao.HostJoinDao; @@ -374,21 +379,29 @@ protected static Map getDummyTags() { return tags; } - protected void waitForJobCompletion(long jobId) { - long timeoutNanos = TimeUnit.MINUTES.toNanos(5); - final long deadline = System.nanoTime() + timeoutNanos; + /** + * Waits for the async job to reach a final state and returns it, or null if the job is not found. + * Throws if the wait times out or is interrupted. Only the wait is abandoned in that case, the job + * itself keeps running in the background and may still complete. + */ + protected AsyncJobVO waitForJobCompletion(long jobId, long timeoutMillis) { + final long start = System.nanoTime(); + final long deadline = start + TimeUnit.MILLISECONDS.toNanos(timeoutMillis); long sleepMillis = 500; while (true) { AsyncJobVO job = asyncJobDao.findByIdIncludingRemoved(jobId); if (job == null) { logger.warn("Async job with ID {} not found", jobId); - return; + return null; } - if (job.getStatus() == AsyncJobVO.Status.SUCCEEDED || job.getStatus() == AsyncJobVO.Status.FAILED) { - return; + if (job.getStatus().done()) { + return job; } if (System.nanoTime() > deadline) { - logger.warn("Timed out waiting for {} completion", job); + logger.warn("Timed out waiting for {} completion after {}s", job, getElapsedSeconds(start)); + throw new AsyncJobFailedException(job.getUuid(), String.format( + "Timed out waiting for job %s to complete after %ds, the job may still be running in the background", + job.getUuid(), getElapsedSeconds(start))); } try { Thread.sleep(sleepMillis); @@ -396,21 +409,65 @@ protected void waitForJobCompletion(long jobId) { sleepMillis = Math.min(5000, sleepMillis + 500); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - logger.warn("Interrupted while waiting for async job completion"); + throw new AsyncJobFailedException(job.getUuid(), String.format( + "Interrupted while waiting for job %s to complete after %ds, the job may still be running in the background", + job.getUuid(), getElapsedSeconds(start))); } } } - protected void waitForJobCompletion(AsyncJobJoinVO job) { - if (job == null) { + /** + * Waits for the async job to complete, up to the configured job wait timeout, and throws + * {@link AsyncJobFailedException} if it did not succeed. + * See {@link #waitForJobCompletion(long, long)} for timeout behaviour. + */ + protected void waitForJobCompletion(AsyncJobJoinVO jobVo) { + if (jobVo == null) { logger.warn("Async job not found"); return; } - if (job.getStatus() == AsyncJobVO.Status.SUCCEEDED.ordinal() || - job.getStatus() == AsyncJobVO.Status.FAILED.ordinal()) { - logger.warn("Async job with ID {} already completed with status {}", job.getId(), job.getStatus()); + final long start = System.nanoTime(); + AsyncJobVO job = waitForJobCompletion(jobVo.getId(), + getJobWaitTimeoutMillis(VeeamControlService.JobWaitTimeout.value())); + if (job == null || job.getStatus() == JobInfo.Status.SUCCEEDED) { + return; + } + throw new AsyncJobFailedException(jobVo.getUuid(), String.format("Job %s %s after %ds: %s", jobVo.getUuid(), + job.getStatus().name().toLowerCase(), getElapsedSeconds(start), getJobErrorText(job))); + } + + /** + * Converts the configured job wait timeout to milliseconds, falling back to the config's default + * when the configured value is not positive. + */ + protected long getJobWaitTimeoutMillis(Integer configuredSeconds) { + if (configuredSeconds == null || configuredSeconds <= 0) { + final int defaultSeconds = Integer.parseInt(VeeamControlService.JobWaitTimeout.defaultValue()); + logger.warn("Invalid value {} for {}, using default of {}s", configuredSeconds, + VeeamControlService.JobWaitTimeout.key(), defaultSeconds); + return TimeUnit.SECONDS.toMillis(defaultSeconds); + } + return TimeUnit.SECONDS.toMillis(configuredSeconds); + } + + protected static long getElapsedSeconds(long startNanos) { + return TimeUnit.NANOSECONDS.toSeconds(System.nanoTime() - startNanos); + } + + protected static String getJobErrorText(AsyncJobVO job) { + String result = job.getResult(); + if (StringUtils.isBlank(result)) { + return "no error details available"; + } + try { + Object resultObject = ApiSerializerHelper.fromSerializedString(result); + if (resultObject instanceof ExceptionResponse) { + return ((ExceptionResponse) resultObject).getErrorText(); + } + } catch (Exception e) { + // fall back to the raw result } - waitForJobCompletion(job.getId()); + return result; } protected ApiServerService.AsyncCmdResult processAsyncCmdWithContext(BaseAsyncCmd cmd, Map params) @@ -1178,17 +1235,41 @@ public DataCenter getDataCenter(String uuid) { } @ApiAccess(command = ListStoragePoolsCmd.class) - public List listStorageDomainsByDcId(final String uuid, final Long offset, final Long limit) { + public List listStorageDomainsByDcId(final String uuid, final String nameLikePattern, final Long offset, + final Long limit) { final DataCenterVO dataCenterVO = dataCenterDao.findByUuid(uuid); if (dataCenterVO == null) { throw new InvalidParameterValueException("DataCenter with ID " + uuid + " not found"); } + return listStorageDomainsByDcIds(List.of(dataCenterVO.getId()), nameLikePattern, offset, limit); + } + + @ApiAccess(command = ListStoragePoolsCmd.class) + public List listAllStorageDomains(final String nameLikePattern, final Long offset, final Long limit) { + return listStorageDomainsByDcIds(kvmBackupExportService.listCompatibleDataCenterIds(), nameLikePattern, offset, limit); + } + + protected List listStorageDomainsByDcIds(final List dcIds, final String nameLikePattern, + final Long offset, final Long limit) { Filter filter = new Filter(StoragePoolJoinVO.class, "id", true, offset, limit); - List storagePoolVOS = storagePoolJoinDao.listAvailableByZoneHypervisorAndType(dataCenterVO.getId(), - Hypervisor.HypervisorType.KVM, SUPPORTED_STORAGE_TYPES, filter); + List storagePoolVOS = storagePoolJoinDao.listAvailableByZonesHypervisorAndType(dcIds, + Hypervisor.HypervisorType.KVM, SUPPORTED_STORAGE_TYPES, nameLikePattern, filter); return StoreVOToStorageDomainConverter.toStorageDomainListFromPools(storagePoolVOS); } + @ApiAccess(command = ListStoragePoolsCmd.class) + public StorageDomain getStorageDomain(final String uuid) { + final StoragePoolVO pool = primaryDataStoreDao.findByUuid(uuid); + if (pool == null) { + throw new InvalidParameterValueException("Storage domain with ID " + uuid + " not found"); + } + final List poolViews = storagePoolJoinDao.newStoragePoolView(pool); + if (CollectionUtils.isEmpty(poolViews)) { + throw new InvalidParameterValueException("Storage domain with ID " + uuid + " not found"); + } + return StoreVOToStorageDomainConverter.toStorageDomain(poolViews.get(0)); + } + @ApiAccess(command = ListNetworksCmd.class) public List listNetworksByDcId(final String uuid, final Long offset, final Long limit) { final DataCenterJoinVO dataCenterVO = dataCenterJoinDao.findByUuid(uuid); @@ -1687,16 +1768,31 @@ public Disk createDisk(Disk request) { } @ApiAccess(command = DestroyVolumeCmd.class) - public void deleteDisk(String uuid) { + public ResourceAction deleteDisk(String uuid, boolean async) { VolumeVO vo = volumeDao.findByUuid(uuid); if (vo == null) { throw new InvalidParameterValueException("Disk with ID " + uuid + " not found"); } - CallContext context = CallContext.current(); - context.setEventDetails("Volume Id: " + vo.getUuid()); - context.setEventResourceType(ApiCommandResourceType.Volume); - context.setEventResourceId(vo.getId()); - volumeApiService.destroyVolume(vo.getId(), CallContext.current().getCallingAccount(), true, false, null); + boolean isAdmin = accountService.isRootAdmin(CallContext.current().getCallingAccountId()); + try { + DestroyVolumeCmd cmd = isAdmin ? new DestroyVolumeCmdByAdmin() : new DestroyVolumeCmd(); + cmd.setHttpMethod(BaseCmd.HTTPMethod.POST.name()); + ComponentContext.inject(cmd); + Map params = new HashMap<>(); + params.put(ApiConstants.ID, vo.getUuid()); + params.put(ApiConstants.EXPUNGE, Boolean.TRUE.toString()); + ApiServerService.AsyncCmdResult result = processAsyncCmdWithContext(cmd, params); + AsyncJobJoinVO jobVo = asyncJobJoinDao.findById(result.jobId); + if (jobVo == null) { + throw new CloudRuntimeException("Failed to find job for disk deletion"); + } + if (!async) { + waitForJobCompletion(jobVo); + } + return AsyncJobJoinVOToJobConverter.toAction(jobVo); + } catch (Exception e) { + throw new CloudRuntimeException("Failed to delete disk: " + e.getMessage(), e); + } } @ApiAccess(command = UpdateVolumeCmd.class) @@ -1783,6 +1879,49 @@ public Nic attachInstanceNic(final String vmUuid, final Nic request) { return NicVOToNicConverter.toNic(nic, vmUuid, this::getNetworkById); } + @ApiAccess(command = ListNicsCmd.class) + public Nic getInstanceNic(final String vmUuid, final String nicUuid) { + UserVmVO vmVo = userVmDao.findByUuid(vmUuid); + if (vmVo == null) { + throw new InvalidParameterValueException("VM with ID " + vmUuid + " not found"); + } + accountService.checkAccess(CallContext.current().getCallingAccount(), null, false, vmVo); + NicVO nic = getInstanceNicVO(vmVo, nicUuid); + return NicVOToNicConverter.toNic(nic, vmUuid, this::getNetworkById); + } + + @ApiAccess(command = RemoveNicFromVMCmd.class) + public void detachInstanceNic(final String vmUuid, final String nicUuid) { + UserVmVO vmVo = userVmDao.findByUuid(vmUuid); + if (vmVo == null) { + throw new InvalidParameterValueException("VM with ID " + vmUuid + " not found"); + } + accountService.checkAccess(CallContext.current().getCallingAccount(), SecurityChecker.AccessType.OperateEntry, + false, vmVo); + NicVO nic = getInstanceNicVO(vmVo, nicUuid); + CallContext context = CallContext.current(); + context.setEventResourceType(ApiCommandResourceType.VirtualMachine); + context.setEventResourceId(vmVo.getId()); + context.setEventDetails("Vm Id: " + vmVo.getUuid() + " Nic Id: " + nic.getUuid()); + RemoveNicFromVMCmd cmd = new RemoveNicFromVMCmd(); + ComponentContext.inject(cmd); + cmd.setVmId(vmVo.getId()); + cmd.setNicId(nic.getId()); + try { + userVmManager.removeNicFromVirtualMachine(cmd); + } catch (Exception e) { + throw new CloudRuntimeException("Failed to remove NIC: " + e.getMessage(), e); + } + } + + protected NicVO getInstanceNicVO(final UserVmVO vmVo, final String nicUuid) { + NicVO nic = nicDao.findByUuid(nicUuid); + if (nic == null || nic.getInstanceId() != vmVo.getId()) { + throw new InvalidParameterValueException("NIC with ID " + nicUuid + " not found for VM with ID " + vmVo.getUuid()); + } + return nic; + } + @ApiAccess(command = ListImageTransfersCmd.class) public List listAllImageTransfers(Long offset, Long limit) { Filter filter = new Filter(ImageTransferVO.class, "id", true, offset, limit); diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/DataCentersRouteHandler.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/DataCentersRouteHandler.java index a06af4f24429..2735c66660ae 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/DataCentersRouteHandler.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/DataCentersRouteHandler.java @@ -122,8 +122,8 @@ protected void handleGetStorageDomainsByDcId(final String id, final HttpServletR throws IOException { try { ListQuery query = ListQuery.fromRequest(req); - List storageDomains = serverAdapter.listStorageDomainsByDcId(id, query.getOffset(), - query.getMax()); + List storageDomains = serverAdapter.listStorageDomainsByDcId(id, query.getSearchNameLikePattern(), + query.getOffset(), query.getMax()); NamedList response = NamedList.of("storage_domain", storageDomains); io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); } catch (InvalidParameterValueException e) { diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/DisksRouteHandler.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/DisksRouteHandler.java index d12745769e1b..f5dc632fde7c 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/DisksRouteHandler.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/DisksRouteHandler.java @@ -29,6 +29,7 @@ import org.apache.cloudstack.veeam.adapter.ServerAdapter; import org.apache.cloudstack.veeam.api.dto.Disk; import org.apache.cloudstack.veeam.api.dto.NamedList; +import org.apache.cloudstack.veeam.api.dto.ResourceAction; import org.apache.cloudstack.veeam.api.request.ListQuery; import org.apache.cloudstack.veeam.utils.Negotiation; import org.apache.cloudstack.veeam.utils.PathUtil; @@ -89,7 +90,7 @@ public void handle(HttpServletRequest req, HttpServletResponse resp, String path return; } if ("DELETE".equalsIgnoreCase(method)) { - handleDeleteById(id, resp, outFormat, io); + handleDeleteById(id, req, resp, outFormat, io); return; } if ("PUT".equalsIgnoreCase(method)) { @@ -155,13 +156,14 @@ protected void handleGetById(final String id, final HttpServletResponse resp, fi } } - protected void handleDeleteById(final String id, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, - final VeeamControlServlet io) throws IOException { + protected void handleDeleteById(final String id, final HttpServletRequest req, final HttpServletResponse resp, + final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + boolean async = RouteHandler.isRequestAsync(req); try { - serverAdapter.deleteDisk(id); - io.getWriter().write(resp, HttpServletResponse.SC_OK, "Deleted disk ID: " + id, outFormat); + ResourceAction action = serverAdapter.deleteDisk(id, async); + RouteHandler.writeDeleteResponse(resp, async, action, outFormat, io); } catch (CloudRuntimeException e) { - io.badRequest(resp, e.getMessage(), outFormat); + VmsRouteHandler.handleDeleteError(resp, e, outFormat, io); } } diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/StorageDomainsRouteHandler.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/StorageDomainsRouteHandler.java new file mode 100644 index 000000000000..dfcaac10acf0 --- /dev/null +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/StorageDomainsRouteHandler.java @@ -0,0 +1,110 @@ +// 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.veeam.api; + +import java.io.IOException; +import java.util.List; + +import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.cloudstack.veeam.RouteHandler; +import org.apache.cloudstack.veeam.VeeamControlServlet; +import org.apache.cloudstack.veeam.adapter.ServerAdapter; +import org.apache.cloudstack.veeam.api.dto.NamedList; +import org.apache.cloudstack.veeam.api.dto.StorageDomain; +import org.apache.cloudstack.veeam.api.request.ListQuery; +import org.apache.cloudstack.veeam.utils.Negotiation; +import org.apache.cloudstack.veeam.utils.PathUtil; +import org.apache.commons.collections.CollectionUtils; + +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.component.ManagerBase; +import com.cloud.utils.exception.CloudRuntimeException; + +public class StorageDomainsRouteHandler extends ManagerBase implements RouteHandler { + public static final String BASE_ROUTE = "/api/storagedomains"; + + @Inject + ServerAdapter serverAdapter; + + @Override + public boolean start() { + return true; + } + + @Override + public int priority() { + return 5; + } + + @Override + public boolean canHandle(String method, String path) { + return getSanitizedPath(path).startsWith(BASE_ROUTE); + } + + @Override + public void handle(HttpServletRequest req, HttpServletResponse resp, String path, Negotiation.OutFormat outFormat, VeeamControlServlet io) throws IOException { + final String method = req.getMethod(); + if (!"GET".equalsIgnoreCase(method)) { + io.methodNotAllowed(resp, "GET", outFormat); + return; + } + final String sanitizedPath = getSanitizedPath(path); + if (sanitizedPath.equals(BASE_ROUTE)) { + handleGet(req, resp, outFormat, io); + return; + } + + List idAndSubPath = PathUtil.extractIdAndSubPath(sanitizedPath, BASE_ROUTE); + if (CollectionUtils.isNotEmpty(idAndSubPath)) { + String id = idAndSubPath.get(0); + if (idAndSubPath.size() == 1) { + handleGetById(id, resp, outFormat, io); + return; + } + } + + io.notFound(resp, null, outFormat); + } + + protected void handleGet(final HttpServletRequest req, final HttpServletResponse resp, + Negotiation.OutFormat outFormat, VeeamControlServlet io) throws IOException { + try { + ListQuery query = ListQuery.fromRequest(req); + final List result = serverAdapter.listAllStorageDomains(query.getSearchNameLikePattern(), + query.getOffset(), query.getLimit()); + NamedList response = NamedList.of("storage_domain", result); + io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); + } catch (CloudRuntimeException e) { + io.badRequest(resp, e.getMessage(), outFormat); + } + } + + protected void handleGetById(final String id, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, + final VeeamControlServlet io) throws IOException { + try { + StorageDomain response = serverAdapter.getStorageDomain(id); + io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); + } catch (InvalidParameterValueException e) { + io.notFound(resp, e.getMessage(), outFormat); + } catch (CloudRuntimeException e) { + io.badRequest(resp, e.getMessage(), outFormat); + } + } +} diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/VmsRouteHandler.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/VmsRouteHandler.java index 10ca7f80f04e..34b3b0253e5e 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/VmsRouteHandler.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/VmsRouteHandler.java @@ -19,7 +19,6 @@ import java.io.IOException; import java.util.List; -import java.util.Map; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; @@ -27,11 +26,14 @@ import org.apache.cloudstack.veeam.RouteHandler; import org.apache.cloudstack.veeam.VeeamControlServlet; +import org.apache.cloudstack.veeam.adapter.AsyncJobFailedException; import org.apache.cloudstack.veeam.adapter.ServerAdapter; +import org.apache.cloudstack.veeam.api.converter.AsyncJobJoinVOToJobConverter; import org.apache.cloudstack.veeam.api.dto.Backup; import org.apache.cloudstack.veeam.api.dto.Checkpoint; import org.apache.cloudstack.veeam.api.dto.Disk; import org.apache.cloudstack.veeam.api.dto.DiskAttachment; +import org.apache.cloudstack.veeam.api.dto.Fault; import org.apache.cloudstack.veeam.api.dto.NamedList; import org.apache.cloudstack.veeam.api.dto.Nic; import org.apache.cloudstack.veeam.api.dto.ResourceAction; @@ -182,6 +184,15 @@ public void handle(HttpServletRequest req, HttpServletResponse resp, String path handleDeleteDiskAttachmentForVmId(id, subId, req, resp, outFormat, io); } return; + } else if ("nics".equals(subPath)) { + if ("GET".equalsIgnoreCase(method)) { + handleGetNicById(id, subId, resp, outFormat, io); + } else if ("DELETE".equalsIgnoreCase(method)) { + handleDeleteNicById(id, subId, req, resp, outFormat, io); + } else { + io.methodNotAllowed(resp, "GET, DELETE", outFormat); + } + return; } else if ("snapshots".equals(subPath)) { if (!"GET".equalsIgnoreCase(method) && !"DELETE".equalsIgnoreCase(method)) { io.methodNotAllowed(resp, "GET, DELETE", outFormat); @@ -200,7 +211,7 @@ public void handle(HttpServletRequest req, HttpServletResponse resp, String path return; } else if ("checkpoints".equals(subPath)) { if ("DELETE".equalsIgnoreCase(method)) { - handleDeleteCheckpoint(id, subId, resp, outFormat, io); + handleDeleteCheckpoint(id, subId, req, resp, outFormat, io); } else { io.methodNotAllowed(resp, "DELETE", outFormat); } @@ -299,12 +310,12 @@ protected void handleUpdateById(final String id, final HttpServletRequest req, f protected void handleDeleteById(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { boolean async = RouteHandler.isRequestAsync(req); - boolean deleteVolumes = Boolean.FALSE.toString().equals(req.getParameter("detach_only")); + boolean deleteVolumes = !isRequestDetachOnly(req, false); try { VmAction vm = serverAdapter.deleteInstance(id, deleteVolumes, async); - io.getWriter().write(resp, HttpServletResponse.SC_OK, vm, outFormat); + RouteHandler.writeDeleteResponse(resp, async, vm, outFormat, io); } catch (CloudRuntimeException e) { - io.badRequest(resp, e.getMessage(), outFormat); + handleDeleteError(resp, e, outFormat, io); } } @@ -315,7 +326,7 @@ protected void handleStartVmById(final String id, final HttpServletRequest req, VmAction vm = serverAdapter.startInstance(id, async); io.getWriter().write(resp, HttpServletResponse.SC_ACCEPTED, vm, outFormat); } catch (CloudRuntimeException e) { - io.badRequest(resp, e.getMessage(), outFormat); + handleActionError(resp, e, outFormat, io); } } @@ -326,7 +337,7 @@ protected void handleStopVmById(final String id, final HttpServletRequest req, f VmAction vm = serverAdapter.stopInstance(id, async); io.getWriter().write(resp, HttpServletResponse.SC_ACCEPTED, vm, outFormat); } catch (CloudRuntimeException e) { - io.badRequest(resp, e.getMessage(), outFormat); + handleActionError(resp, e, outFormat, io); } } @@ -337,7 +348,7 @@ protected void handleShutdownVmById(final String id, final HttpServletRequest re VmAction vm = serverAdapter.shutdownInstance(id, async); io.getWriter().write(resp, HttpServletResponse.SC_ACCEPTED, vm, outFormat); } catch (CloudRuntimeException e) { - io.badRequest(resp, e.getMessage(), outFormat); + handleActionError(resp, e, outFormat, io); } } @@ -393,6 +404,32 @@ protected void handlePostNicForVmId(final String id, final HttpServletRequest re } } + protected void handleGetNicById(final String vmId, final String nicId, final HttpServletResponse resp, + final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + try { + Nic response = serverAdapter.getInstanceNic(vmId, nicId); + io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); + } catch (InvalidParameterValueException e) { + io.notFound(resp, e.getMessage(), outFormat); + } catch (CloudRuntimeException e) { + io.badRequest(resp, e.getMessage(), outFormat); + } + } + + protected void handleDeleteNicById(final String vmId, final String nicId, final HttpServletRequest req, + final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) + throws IOException { + boolean async = RouteHandler.isRequestAsync(req); + try { + serverAdapter.detachInstanceNic(vmId, nicId); + RouteHandler.writeDeleteResponse(resp, async, null, outFormat, io); + } catch (InvalidParameterValueException e) { + io.notFound(resp, e.getMessage(), outFormat); + } catch (CloudRuntimeException e) { + io.badRequest(resp, e.getMessage(), outFormat); + } + } + protected void handleGetSnapshotsByVmId(final String id, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { try { @@ -422,18 +459,19 @@ protected void handlePostSnapshotForVmId(final String id, final HttpServletReque protected void handleDeleteDiskAttachmentForVmId(final String vmId, final String diskId, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { - boolean delete = Boolean.FALSE.toString().equals(req.getParameter("detach_only")); + boolean async = RouteHandler.isRequestAsync(req); + boolean delete = !isRequestDetachOnly(req, true); try { serverAdapter.detachInstanceDisk(vmId, diskId); + ResourceAction action = null; if (delete) { - serverAdapter.deleteDisk(diskId); + action = serverAdapter.deleteDisk(diskId, async); } - Map response = Map.of("status", "complete"); - io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); + RouteHandler.writeDeleteResponse(resp, async, action, outFormat, io); } catch (InvalidParameterValueException e) { io.notFound(resp, e.getMessage(), outFormat); } catch (CloudRuntimeException e) { - io.badRequest(resp, e.getMessage(), outFormat); + handleDeleteError(resp, e, outFormat, io); } } @@ -455,13 +493,9 @@ protected void handleDeleteSnapshotById(final String id, final HttpServletReques boolean async = RouteHandler.isRequestAsync(req); try { ResourceAction action = serverAdapter.deleteSnapshot(id, async); - if (action != null) { - io.getWriter().write(resp, HttpServletResponse.SC_ACCEPTED, action, outFormat); - } else { - io.getWriter().write(resp, HttpServletResponse.SC_OK, null, outFormat); - } + RouteHandler.writeDeleteResponse(resp, async, action, outFormat, io); } catch (CloudRuntimeException e) { - io.badRequest(resp, e.getMessage(), outFormat); + handleDeleteError(resp, e, outFormat, io); } } @@ -474,7 +508,7 @@ protected void handleRestoreSnapshotById(final String id, final HttpServletReque ResourceAction response = serverAdapter.revertInstanceToSnapshot(id, async); io.getWriter().write(resp, HttpServletResponse.SC_ACCEPTED, response, outFormat); } catch (CloudRuntimeException e) { - io.badRequest(resp, e.getMessage(), outFormat); + handleActionError(resp, e, outFormat, io); } } @@ -554,12 +588,13 @@ protected void handleGetCheckpointsByVmId(final String id, final HttpServletResp } } - protected void handleDeleteCheckpoint(final String vmId, final String checkpointId, + protected void handleDeleteCheckpoint(final String vmId, final String checkpointId, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + boolean async = RouteHandler.isRequestAsync(req); try { serverAdapter.deleteCheckpoint(vmId, checkpointId); - io.getWriter().write(resp, HttpServletResponse.SC_OK, null, outFormat); + RouteHandler.writeDeleteResponse(resp, async, null, outFormat, io); } catch (CloudRuntimeException e) { io.badRequest(resp, e.getMessage(), outFormat); } @@ -577,4 +612,57 @@ protected void handlePostTagForVmId(final String id, final HttpServletRequest re io.badRequest(resp, e.getMessage(), outFormat); } } + + /** + * Writes the error for a VM or snapshot action. When the action's async job failed, or waiting for it + * timed out, responds like oVirt does for a failed blocking action: the action with a "failed" status, + * a fault and the job link. Other errors are written as a plain bad request. + */ + protected static void handleActionError(final HttpServletResponse resp, final CloudRuntimeException e, + final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + final AsyncJobFailedException jobFailure = findAsyncJobFailure(e); + if (jobFailure != null) { + io.getWriter().write(resp, HttpServletResponse.SC_BAD_REQUEST, + AsyncJobJoinVOToJobConverter.toFailedAction(jobFailure.getJobUuid(), e.getMessage()), outFormat); + return; + } + io.badRequest(resp, e.getMessage(), outFormat); + } + + /** + * Writes the error for a DELETE. When its async job failed, or waiting for it timed out, responds like + * oVirt does for a failed DELETE: a plain "Operation Failed" fault, without an action. Other errors are + * written as a plain bad request. + */ + protected static void handleDeleteError(final HttpServletResponse resp, final CloudRuntimeException e, + final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (findAsyncJobFailure(e) != null) { + io.getWriter().writeFault(resp, HttpServletResponse.SC_BAD_REQUEST, Fault.OPERATION_FAILED, e.getMessage(), outFormat); + return; + } + io.badRequest(resp, e.getMessage(), outFormat); + } + + protected static AsyncJobFailedException findAsyncJobFailure(final Throwable e) { + for (Throwable t = e; t != null; t = t.getCause()) { + if (t instanceof AsyncJobFailedException) { + return (AsyncJobFailedException) t; + } + } + return null; + } + + /** + * Returns whether disks should only be detached and preserved, as per the detach_only request parameter. + * Mimics oVirt: a parameter present without a value (e.g. ?detach_only) means true, while a missing or + * unrecognized value falls back to valueIfMissing, which differs per endpoint (false for VM removal, + * true for disk attachment removal). + */ + protected static boolean isRequestDetachOnly(final HttpServletRequest req, final boolean valueIfMissing) { + String detachOnly = req.getParameter("detach_only"); + if (detachOnly != null && detachOnly.isEmpty()) { + return true; + } + return RouteHandler.parseBoolean(detachOnly, valueIfMissing); + } } diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/converter/AsyncJobJoinVOToJobConverter.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/converter/AsyncJobJoinVOToJobConverter.java index e1fbfc82e7aa..84b3d9647aff 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/converter/AsyncJobJoinVOToJobConverter.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/converter/AsyncJobJoinVOToJobConverter.java @@ -24,6 +24,7 @@ import org.apache.cloudstack.jobs.JobInfo; import org.apache.cloudstack.veeam.VeeamControlService; import org.apache.cloudstack.veeam.api.JobsRouteHandler; +import org.apache.cloudstack.veeam.api.dto.Fault; import org.apache.cloudstack.veeam.api.dto.Job; import org.apache.cloudstack.veeam.api.dto.Ref; import org.apache.cloudstack.veeam.api.dto.ResourceAction; @@ -82,6 +83,21 @@ public static VmAction toVmAction(final AsyncJobJoinVO vo, final UserVmJoinVO vm return action; } + /** + * Builds a failed action, similar to oVirt's response for a blocking action that did not complete, + * still linking the job so that it can be tracked. + */ + public static ResourceAction toFailedAction(final String jobUuid, final String detail) { + ResourceAction action = new ResourceAction(); + if (jobUuid != null) { + final String basePath = VeeamControlService.ContextPath.value(); + action.setJob(Ref.of(basePath + JobsRouteHandler.BASE_ROUTE + "/" + jobUuid, jobUuid)); + } + action.setStatus("failed"); + action.setFault(new Fault(Fault.OPERATION_FAILED, detail)); + return action; + } + public static ResourceAction toAction(final AsyncJobJoinVO vo) { VmAction action = new VmAction(); fillAction(action, vo); diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Fault.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Fault.java index 20989d8cbd70..03837b2e8d8d 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Fault.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Fault.java @@ -21,6 +21,9 @@ @JsonInclude(JsonInclude.Include.NON_NULL) public final class Fault { + // oVirt's fault reason for a failed backend operation + public static final String OPERATION_FAILED = "Operation Failed"; + private String reason; // "Not Found", "Bad Request", "Unauthorized" private String detail; // full message diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/ResourceAction.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/ResourceAction.java index ed6c39240369..f4fa0346952b 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/ResourceAction.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/ResourceAction.java @@ -20,6 +20,7 @@ public class ResourceAction extends BaseDto { private Ref job; private String status; + private Fault fault; public Ref getJob() { return job; @@ -36,4 +37,12 @@ public String getStatus() { public void setStatus(String status) { this.status = status; } + + public Fault getFault() { + return fault; + } + + public void setFault(Fault fault) { + this.fault = fault; + } } diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/request/ListQuery.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/request/ListQuery.java index 1cb560a3d897..1c75311e35df 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/request/ListQuery.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/request/ListQuery.java @@ -34,6 +34,7 @@ public class ListQuery { private static final Pattern PAGE_CLAUSE_PATTERN = Pattern.compile("(?i)\\bpage\\s+(\\d+)\\b"); + private static final Pattern NAME_CLAUSE_PATTERN = Pattern.compile("(?i)(? {page=3} // sortby name page 2 --> {page=2} + // name=data* page 2 --> {page=2, name=data*} @NotNull private static Map getSearchMap(String searchClause) { Map searchItems = new LinkedHashMap<>(); @@ -148,6 +168,10 @@ private static Map getSearchMap(String searchClause) { if (matcher.find()) { searchItems.put("page", matcher.group(1)); } + matcher = NAME_CLAUSE_PATTERN.matcher(searchClause); + if (matcher.find()) { + searchItems.put("name", matcher.group(1) != null ? matcher.group(1) : matcher.group(2)); + } return searchItems; } } diff --git a/plugins/integrations/veeam-control-service/src/main/resources/META-INF/cloudstack/veeam-control-service/spring-veeam-control-service-context.xml b/plugins/integrations/veeam-control-service/src/main/resources/META-INF/cloudstack/veeam-control-service/spring-veeam-control-service-context.xml index 83d100ec76e8..f7b238515be8 100644 --- a/plugins/integrations/veeam-control-service/src/main/resources/META-INF/cloudstack/veeam-control-service/spring-veeam-control-service-context.xml +++ b/plugins/integrations/veeam-control-service/src/main/resources/META-INF/cloudstack/veeam-control-service/spring-veeam-control-service-context.xml @@ -41,6 +41,7 @@ + diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/RouteHandlerTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/RouteHandlerTest.java new file mode 100644 index 000000000000..2e2c36bd6ae3 --- /dev/null +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/RouteHandlerTest.java @@ -0,0 +1,72 @@ +// 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.veeam; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import javax.servlet.http.HttpServletRequest; + +import org.junit.Test; + +public class RouteHandlerTest { + + @Test + public void testParseBoolean_TrueValues() { + for (String value : new String[] {"true", "True", "TRUE", "tRuE", "1", " true ", " 1 "}) { + assertTrue("Expected true for '" + value + "'", RouteHandler.parseBoolean(value, false)); + } + } + + @Test + public void testParseBoolean_FalseValues() { + for (String value : new String[] {"false", "False", "FALSE", "fAlSe", "0", " false ", " 0 "}) { + assertFalse("Expected false for '" + value + "'", RouteHandler.parseBoolean(value, true)); + } + } + + @Test + public void testParseBoolean_NullEmptyOrUnrecognizedReturnsDefault() { + for (String value : new String[] {null, "", " ", "yes", "no", "2", "-1", "truee", "on"}) { + assertTrue("Expected default for '" + value + "'", RouteHandler.parseBoolean(value, true)); + assertFalse("Expected default for '" + value + "'", RouteHandler.parseBoolean(value, false)); + } + } + + @Test + public void testIsRequestAsync_TrueWhenAsyncParamIsTruthyOrEmpty() { + for (String value : new String[] {"", "true", "TRUE", "1"}) { + assertTrue("Expected async for '" + value + "'", RouteHandler.isRequestAsync(requestWithAsync(value))); + } + } + + @Test + public void testIsRequestAsync_FalseWhenAsyncParamIsMissingFalsyOrInvalid() { + for (String value : new String[] {null, " ", "false", "0", "yes"}) { + assertFalse("Expected sync for '" + value + "'", RouteHandler.isRequestAsync(requestWithAsync(value))); + } + } + + private static HttpServletRequest requestWithAsync(String value) { + final HttpServletRequest req = mock(HttpServletRequest.class); + when(req.getParameter("async")).thenReturn(value); + return req; + } +} diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/VeeamControlServiceImplTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/VeeamControlServiceImplTest.java index a4dd970c9cfb..49d84ed4d475 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/VeeamControlServiceImplTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/VeeamControlServiceImplTest.java @@ -161,7 +161,7 @@ public void testGetConfigKeysContainsExpectedEntries() { final ConfigKey[] keys = service.getConfigKeys(); - assertEquals(11, keys.length); + assertEquals(12, keys.length); assertEquals(KEY_ENABLED, keys[0].key()); } diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java index b105e7306e5f..c6e7cfc04bda 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java @@ -21,10 +21,12 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; @@ -38,6 +40,12 @@ import java.util.List; import java.util.Map; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiServerService; +import org.apache.cloudstack.api.BaseAsyncCmd; +import org.apache.cloudstack.api.command.user.vm.RemoveNicFromVMCmd; +import org.apache.cloudstack.api.command.user.volume.DestroyVolumeCmd; +import org.apache.cloudstack.api.response.ExceptionResponse; import org.apache.cloudstack.backup.BackupVO; import org.apache.cloudstack.backup.KVMBackupExportService; import org.apache.cloudstack.backup.dao.BackupDao; @@ -49,7 +57,10 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.veeam.api.dto.DataCenter; import org.apache.cloudstack.veeam.api.dto.Disk; +import org.apache.cloudstack.veeam.api.dto.Nic; import org.apache.cloudstack.veeam.api.dto.OvfXmlUtil; +import org.apache.cloudstack.veeam.api.dto.ResourceAction; +import org.apache.cloudstack.veeam.api.dto.StorageDomain; import org.apache.cloudstack.veeam.api.dto.Tag; import org.apache.cloudstack.veeam.api.dto.Vm; import org.apache.logging.log4j.Logger; @@ -57,12 +68,14 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import com.cloud.api.ApiSerializerHelper; import com.cloud.api.query.dao.AsyncJobJoinDao; import com.cloud.api.query.dao.DataCenterJoinDao; import com.cloud.api.query.dao.HostJoinDao; @@ -99,6 +112,7 @@ import com.cloud.user.User; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; +import com.cloud.utils.component.ComponentContext; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.NicVO; import com.cloud.vm.UserVmManager; @@ -515,39 +529,73 @@ public void testGetNetworkById_ReturnsVoFromDao() { @Test - public void testWaitForJobCompletion_JobNotFound_Returns() { + public void testWaitForJobCompletion_JobNotFound_ReturnsNull() { when(asyncJobDao.findByIdIncludingRemoved(99L)).thenReturn(null); - serverAdapter.waitForJobCompletion(99L); + assertNull(serverAdapter.waitForJobCompletion(99L, 60000L)); verify(asyncJobDao).findByIdIncludingRemoved(99L); } @Test - public void testWaitForJobCompletion_JobAlreadySucceeded_Returns() { + public void testWaitForJobCompletion_JobDone_ReturnsJob() { + for (AsyncJobVO.Status status : new AsyncJobVO.Status[] {AsyncJobVO.Status.SUCCEEDED, + AsyncJobVO.Status.FAILED, AsyncJobVO.Status.CANCELLED}) { + AsyncJobVO job = mock(AsyncJobVO.class); + when(job.getStatus()).thenReturn(status); + when(asyncJobDao.findByIdIncludingRemoved(1L)).thenReturn(job); + assertEquals(job, serverAdapter.waitForJobCompletion(1L, 60000L)); + } + } + + @Test + public void testWaitForJobCompletion_DeadlinePassed_ThrowsWithBackgroundCaveat() { AsyncJobVO job = mock(AsyncJobVO.class); - when(job.getStatus()).thenReturn(AsyncJobVO.Status.SUCCEEDED); - when(asyncJobDao.findByIdIncludingRemoved(1L)).thenReturn(job); - serverAdapter.waitForJobCompletion(1L); + when(job.getStatus()).thenReturn(AsyncJobVO.Status.IN_PROGRESS); + when(job.getUuid()).thenReturn("job-uuid"); + when(asyncJobDao.findByIdIncludingRemoved(3L)).thenReturn(job); + + AsyncJobFailedException e = assertThrows(AsyncJobFailedException.class, () -> serverAdapter.waitForJobCompletion(3L, 0L)); + assertEquals("job-uuid", e.getJobUuid()); + assertTrue(e.getMessage().contains("Timed out waiting for job job-uuid to complete after 0s")); + assertTrue(e.getMessage().contains("may still be running in the background")); } @Test - public void testWaitForJobCompletion_JobAlreadyFailed_Returns() { + public void testWaitForJobCompletion_Interrupted_ThrowsAndKeepsInterruptFlag() { AsyncJobVO job = mock(AsyncJobVO.class); - when(job.getStatus()).thenReturn(AsyncJobVO.Status.FAILED); - when(asyncJobDao.findByIdIncludingRemoved(2L)).thenReturn(job); - serverAdapter.waitForJobCompletion(2L); + when(job.getStatus()).thenReturn(AsyncJobVO.Status.IN_PROGRESS); + when(job.getUuid()).thenReturn("job-uuid"); + when(asyncJobDao.findByIdIncludingRemoved(4L)).thenReturn(job); + + Thread.currentThread().interrupt(); + try { + AsyncJobFailedException e = assertThrows(AsyncJobFailedException.class, () -> serverAdapter.waitForJobCompletion(4L, 60000L)); + assertTrue(e.getMessage().contains("Interrupted while waiting for job job-uuid")); + assertTrue(Thread.currentThread().isInterrupted()); + } finally { + Thread.interrupted(); + } } + @Test + public void testGetJobWaitTimeoutMillis_PositiveValueIsUsed() { + assertEquals(120000L, serverAdapter.getJobWaitTimeoutMillis(120)); + } @Test - public void testWaitForJobCompletion_NullJobJoinVO_Returns() { - AsyncJobJoinVO job = null; - serverAdapter.waitForJobCompletion(job); + public void testGetJobWaitTimeoutMillis_ZeroNegativeOrNullFallsBackToDefault() { + for (Integer value : new Integer[] {0, -1, null}) { + assertEquals("Expected default for " + value, 3600000L, serverAdapter.getJobWaitTimeoutMillis(value)); + } } @Test - public void testWaitForJobCompletion_CompletedJobJoinVO_DelegatesById() { + public void testWaitForJobSuccess_NullJobJoinVO_Returns() { + serverAdapter.waitForJobCompletion(null); + } + + @Test + public void testWaitForJobSuccess_JobSucceeded_Returns() { AsyncJobJoinVO jobVO = mock(AsyncJobJoinVO.class); - when(jobVO.getStatus()).thenReturn(AsyncJobVO.Status.SUCCEEDED.ordinal()); when(jobVO.getId()).thenReturn(5L); AsyncJobVO job = mock(AsyncJobVO.class); @@ -559,6 +607,46 @@ public void testWaitForJobCompletion_CompletedJobJoinVO_DelegatesById() { verify(asyncJobDao).findByIdIncludingRemoved(5L); } + @Test + public void testWaitForJobSuccess_JobFailed_ThrowsWithElapsedAndErrorText() { + AsyncJobJoinVO jobVO = mock(AsyncJobJoinVO.class); + when(jobVO.getId()).thenReturn(6L); + when(jobVO.getUuid()).thenReturn("job-uuid"); + + AsyncJobVO job = mock(AsyncJobVO.class); + when(job.getStatus()).thenReturn(AsyncJobVO.Status.FAILED); + when(job.getResult()).thenReturn("Unable to destroy VM"); + when(asyncJobDao.findByIdIncludingRemoved(6L)).thenReturn(job); + + AsyncJobFailedException e = assertThrows(AsyncJobFailedException.class, () -> serverAdapter.waitForJobCompletion(jobVO)); + assertEquals("job-uuid", e.getJobUuid()); + assertEquals("Job job-uuid failed after 0s: Unable to destroy VM", e.getMessage()); + } + + @Test + public void testWaitForJobSuccess_JobCancelled_Throws() { + AsyncJobJoinVO jobVO = mock(AsyncJobJoinVO.class); + when(jobVO.getId()).thenReturn(7L); + when(jobVO.getUuid()).thenReturn("job-uuid"); + + AsyncJobVO job = mock(AsyncJobVO.class); + when(job.getStatus()).thenReturn(AsyncJobVO.Status.CANCELLED); + when(asyncJobDao.findByIdIncludingRemoved(7L)).thenReturn(job); + + AsyncJobFailedException e = assertThrows(AsyncJobFailedException.class, () -> serverAdapter.waitForJobCompletion(jobVO)); + assertEquals("Job job-uuid cancelled after 0s: no error details available", e.getMessage()); + } + + @Test + public void testGetJobErrorText_ExtractsErrorTextFromSerializedExceptionResponse() { + ExceptionResponse response = new ExceptionResponse(); + response.setErrorText("Host is unreachable"); + AsyncJobVO job = mock(AsyncJobVO.class); + when(job.getResult()).thenReturn(ApiSerializerHelper.toSerializedString(response)); + + assertEquals("Host is unreachable", ServerAdapter.getJobErrorText(job)); + } + @Test public void testGetOwnerDetailsForInstanceCreation_NullAccount_ReturnsAllNulls() { @@ -836,7 +924,7 @@ public void testListAllDataCenters_EmptyList_ReturnsEmpty() { @Test(expected = InvalidParameterValueException.class) public void testListStorageDomainsByDcId_DataCenterNotFound_Throws() { when(dataCenterDao.findByUuid("dc-uuid")).thenReturn(null); - serverAdapter.listStorageDomainsByDcId("dc-uuid", 0L, 10L); + serverAdapter.listStorageDomainsByDcId("dc-uuid", null, 0L, 10L); } @Test @@ -845,9 +933,39 @@ public void testListStorageDomainsByDcId_Found_ReturnsList() { when(dcVO.getId()).thenReturn(1L); when(dataCenterDao.findByUuid("dc-uuid")).thenReturn(dcVO); StoragePoolJoinVO poolVO = mock(StoragePoolJoinVO.class); - when(storagePoolJoinDao.listAvailableByZoneHypervisorAndType(eq(1L), eq(Hypervisor.HypervisorType.KVM), any(), any())).thenReturn(List.of(poolVO)); + when(storagePoolJoinDao.listAvailableByZonesHypervisorAndType(eq(List.of(1L)), eq(Hypervisor.HypervisorType.KVM), any(), eq("data%"), any())).thenReturn(List.of(poolVO)); - assertNotNull(serverAdapter.listStorageDomainsByDcId("dc-uuid", 0L, 10L)); + assertEquals(1, serverAdapter.listStorageDomainsByDcId("dc-uuid", "data%", 0L, 10L).size()); + } + + @Test + public void testListAllStorageDomains_UsesCompatibleDataCenters() { + when(kvmBackupExportService.listCompatibleDataCenterIds()).thenReturn(List.of(1L, 2L)); + StoragePoolJoinVO poolVO = mock(StoragePoolJoinVO.class); + when(poolVO.getUuid()).thenReturn("pool-uuid"); + when(storagePoolJoinDao.listAvailableByZonesHypervisorAndType(eq(List.of(1L, 2L)), eq(Hypervisor.HypervisorType.KVM), any(), isNull(), any())).thenReturn(List.of(poolVO)); + + List result = serverAdapter.listAllStorageDomains(null, 0L, 10L); + + assertEquals(1, result.size()); + assertEquals("pool-uuid", result.get(0).getId()); + } + + @Test(expected = InvalidParameterValueException.class) + public void testGetStorageDomain_NotFound_Throws() { + when(primaryDataStoreDao.findByUuid("pool-uuid")).thenReturn(null); + serverAdapter.getStorageDomain("pool-uuid"); + } + + @Test + public void testGetStorageDomain_Found_ReturnsDomain() { + StoragePoolVO pool = mock(StoragePoolVO.class); + when(primaryDataStoreDao.findByUuid("pool-uuid")).thenReturn(pool); + StoragePoolJoinVO poolVO = mock(StoragePoolJoinVO.class); + when(poolVO.getUuid()).thenReturn("pool-uuid"); + when(storagePoolJoinDao.newStoragePoolView(pool)).thenReturn(List.of(poolVO)); + + assertEquals("pool-uuid", serverAdapter.getStorageDomain("pool-uuid").getId()); } @@ -958,19 +1076,68 @@ public void testGetDisk_NotFound_Throws() { @Test(expected = InvalidParameterValueException.class) public void testDeleteDisk_NotFound_Throws() { when(volumeDao.findByUuid("vol-uuid")).thenReturn(null); - serverAdapter.deleteDisk("vol-uuid"); + serverAdapter.deleteDisk("vol-uuid", false); } - @Test - public void testDeleteDisk_Found_DeletesVolume() { + private ServerAdapter spyAdapterForDiskDeletion(AsyncJobJoinVO jobVo) throws Exception { VolumeVO vo = mock(VolumeVO.class); - when(vo.getId()).thenReturn(10L); + when(vo.getUuid()).thenReturn("vol-uuid"); when(volumeDao.findByUuid("vol-uuid")).thenReturn(vo); - Account account = CallContext.current().getCallingAccount(); + when(asyncJobJoinDao.findById(7L)).thenReturn(jobVo); + ServerAdapter adapter = Mockito.spy(serverAdapter); + doReturn(new ApiServerService.AsyncCmdResult(null, null, null, 7L)) + .when(adapter).processAsyncCmdWithContext(any(), any()); + return adapter; + } + + @Test + @SuppressWarnings("unchecked") + public void testDeleteDisk_Sync_WaitsForJobAndReturnsAction() throws Exception { + AsyncJobJoinVO jobVo = mock(AsyncJobJoinVO.class); + when(jobVo.getUuid()).thenReturn("job-uuid"); + ServerAdapter adapter = spyAdapterForDiskDeletion(jobVo); + doNothing().when(adapter).waitForJobCompletion(any(AsyncJobJoinVO.class)); + + ResourceAction action; + try (MockedStatic ignored = Mockito.mockStatic(ComponentContext.class)) { + action = adapter.deleteDisk("vol-uuid", false); + } + + ArgumentCaptor cmdCaptor = ArgumentCaptor.forClass(BaseAsyncCmd.class); + ArgumentCaptor> paramsCaptor = ArgumentCaptor.forClass(Map.class); + verify(adapter).processAsyncCmdWithContext(cmdCaptor.capture(), paramsCaptor.capture()); + assertEquals(DestroyVolumeCmd.class, cmdCaptor.getValue().getClass()); + assertEquals("vol-uuid", paramsCaptor.getValue().get(ApiConstants.ID)); + assertEquals("true", paramsCaptor.getValue().get(ApiConstants.EXPUNGE)); + verify(adapter).waitForJobCompletion(jobVo); + assertEquals("complete", action.getStatus()); + assertEquals("job-uuid", action.getJob().getId()); + } + + @Test + public void testDeleteDisk_Async_DoesNotWaitForJob() throws Exception { + AsyncJobJoinVO jobVo = mock(AsyncJobJoinVO.class); + when(jobVo.getUuid()).thenReturn("job-uuid"); + ServerAdapter adapter = spyAdapterForDiskDeletion(jobVo); + + try (MockedStatic ignored = Mockito.mockStatic(ComponentContext.class)) { + assertEquals("job-uuid", adapter.deleteDisk("vol-uuid", true).getJob().getId()); + } + + verify(adapter, Mockito.never()).waitForJobCompletion(any(AsyncJobJoinVO.class)); + } - serverAdapter.deleteDisk("vol-uuid"); + @Test + public void testDeleteDisk_JobFailure_ThrowsWithJobFailureCause() throws Exception { + AsyncJobJoinVO jobVo = mock(AsyncJobJoinVO.class); + ServerAdapter adapter = spyAdapterForDiskDeletion(jobVo); + Mockito.doThrow(new AsyncJobFailedException("job-uuid", "volume in use")) + .when(adapter).waitForJobCompletion(any(AsyncJobJoinVO.class)); - verify(volumeApiService).destroyVolume(10L, account, true, false, null); + try (MockedStatic ignored = Mockito.mockStatic(ComponentContext.class)) { + CloudRuntimeException e = assertThrows(CloudRuntimeException.class, () -> adapter.deleteDisk("vol-uuid", false)); + assertTrue(e.getCause() instanceof AsyncJobFailedException); + } } @@ -1137,4 +1304,90 @@ public void testUpdateDisk_NotFound_Throws() { public void testListDisksByBackupUuid_AlwaysThrows() { serverAdapter.listDisksByBackupUuid("backup-uuid"); } + + @Test(expected = InvalidParameterValueException.class) + public void testGetInstanceNic_VmNotFound_Throws() { + when(userVmDao.findByUuid("vm-uuid")).thenReturn(null); + serverAdapter.getInstanceNic("vm-uuid", "nic-uuid"); + } + + @Test + public void testGetInstanceNic_NicNotFoundOrOfAnotherVm_Throws() { + UserVmVO vm = mock(UserVmVO.class); + when(vm.getId()).thenReturn(10L); + when(vm.getUuid()).thenReturn("vm-uuid"); + when(userVmDao.findByUuid("vm-uuid")).thenReturn(vm); + NicVO otherVmNic = mock(NicVO.class); + when(otherVmNic.getInstanceId()).thenReturn(11L); + when(nicDao.findByUuid("other-nic")).thenReturn(otherVmNic); + + InvalidParameterValueException e = assertThrows(InvalidParameterValueException.class, + () -> serverAdapter.getInstanceNic("vm-uuid", "missing-nic")); + assertEquals("NIC with ID missing-nic not found for VM with ID vm-uuid", e.getMessage()); + assertThrows(InvalidParameterValueException.class, () -> serverAdapter.getInstanceNic("vm-uuid", "other-nic")); + } + + @Test + public void testGetInstanceNic_ReturnsNic() { + UserVmVO vm = mock(UserVmVO.class); + when(vm.getId()).thenReturn(10L); + when(userVmDao.findByUuid("vm-uuid")).thenReturn(vm); + NicVO nic = mock(NicVO.class); + when(nic.getInstanceId()).thenReturn(10L); + when(nic.getUuid()).thenReturn("nic-uuid"); + when(nicDao.findByUuid("nic-uuid")).thenReturn(nic); + + Nic result = serverAdapter.getInstanceNic("vm-uuid", "nic-uuid"); + + assertEquals("nic-uuid", result.getId()); + assertTrue(result.getHref().endsWith("/api/vms/vm-uuid/nics/nic-uuid")); + } + + @Test + public void testDetachInstanceNic_RemovesNicFromVm() { + UserVmVO vm = mock(UserVmVO.class); + when(vm.getId()).thenReturn(10L); + when(userVmDao.findByUuid("vm-uuid")).thenReturn(vm); + NicVO nic = mock(NicVO.class); + when(nic.getId()).thenReturn(20L); + when(nic.getInstanceId()).thenReturn(10L); + when(nicDao.findByUuid("nic-uuid")).thenReturn(nic); + + try (MockedStatic ignored = Mockito.mockStatic(ComponentContext.class)) { + serverAdapter.detachInstanceNic("vm-uuid", "nic-uuid"); + } + + ArgumentCaptor captor = ArgumentCaptor.forClass(RemoveNicFromVMCmd.class); + verify(userVmManager).removeNicFromVirtualMachine(captor.capture()); + assertEquals(Long.valueOf(10L), captor.getValue().getVmId()); + assertEquals(Long.valueOf(20L), captor.getValue().getNicId()); + } + + @Test + public void testDetachInstanceNic_NicNotFound_ThrowsWithoutRemoving() { + UserVmVO vm = mock(UserVmVO.class); + when(userVmDao.findByUuid("vm-uuid")).thenReturn(vm); + + assertThrows(InvalidParameterValueException.class, () -> serverAdapter.detachInstanceNic("vm-uuid", "nic-uuid")); + verify(userVmManager, Mockito.never()).removeNicFromVirtualMachine(any()); + } + + @Test + public void testDetachInstanceNic_RemoveFailure_ThrowsCloudRuntimeException() { + UserVmVO vm = mock(UserVmVO.class); + when(vm.getId()).thenReturn(10L); + when(userVmDao.findByUuid("vm-uuid")).thenReturn(vm); + NicVO nic = mock(NicVO.class); + when(nic.getInstanceId()).thenReturn(10L); + when(nicDao.findByUuid("nic-uuid")).thenReturn(nic); + when(userVmManager.removeNicFromVirtualMachine(any())) + .thenThrow(new InvalidParameterValueException("Unable to remove nic from VM, it is the default nic")); + + try (MockedStatic ignored = Mockito.mockStatic(ComponentContext.class)) { + CloudRuntimeException e = assertThrows(CloudRuntimeException.class, + () -> serverAdapter.detachInstanceNic("vm-uuid", "nic-uuid")); + assertEquals(CloudRuntimeException.class, e.getClass()); + assertEquals("Failed to remove NIC: Unable to remove nic from VM, it is the default nic", e.getMessage()); + } + } } diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/DataCentersRouteHandlerTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/DataCentersRouteHandlerTest.java index 38ddbfad37b9..d7b358b1f113 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/DataCentersRouteHandlerTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/DataCentersRouteHandlerTest.java @@ -56,7 +56,7 @@ public void testHandleGetListAndById() throws Exception { public void testHandleGetStorageDomainsAndNetworksByDataCenterId() throws Exception { final DataCentersRouteHandler handler = new DataCentersRouteHandler(); handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); - when(handler.serverAdapter.listStorageDomainsByDcId("dc-1", null, 15L)) + when(handler.serverAdapter.listStorageDomainsByDcId("dc-1", null, null, 15L)) .thenReturn(List.of(withId(new StorageDomain(), "sd-1"))); when(handler.serverAdapter.listNetworksByDcId("dc-1", null, 15L)) .thenReturn(List.of(withId(new Network(), "net-1"))); diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/DisksRouteHandlerTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/DisksRouteHandlerTest.java index f3872d0cd89d..860660affed8 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/DisksRouteHandlerTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/DisksRouteHandlerTest.java @@ -25,11 +25,16 @@ import java.util.List; import java.util.Map; +import org.apache.cloudstack.veeam.adapter.AsyncJobFailedException; import org.apache.cloudstack.veeam.api.dto.Disk; +import org.apache.cloudstack.veeam.api.dto.Ref; +import org.apache.cloudstack.veeam.api.dto.ResourceAction; import org.apache.cloudstack.veeam.utils.Negotiation; import org.junit.Test; import org.mockito.ArgumentCaptor; +import com.cloud.utils.exception.CloudRuntimeException; + public class DisksRouteHandlerTest extends RouteHandlerTestSupport { @Test @@ -90,9 +95,9 @@ public void testHandleDeleteCopyAndReduceRoutes() throws Exception { final ResponseCapture delete = newResponse(); handler.handle(newRequest("DELETE"), delete.response, "/api/disks/disk-1", Negotiation.OutFormat.JSON, newServlet()); - verify(handler.serverAdapter).deleteDisk("disk-1"); + verify(handler.serverAdapter).deleteDisk("disk-1", false); verify(delete.response).setStatus(200); - assertContains(delete.body(), "Deleted disk ID: disk-1"); + assertContains(delete.body(), "\"status\":\"complete\""); final ResponseCapture copy = newResponse(); handler.handle(newRequest("POST", Map.of(), "application/json", "{}"), copy.response, @@ -119,4 +124,49 @@ public void testHandleCopyRejectsUnsupportedMethod() throws Exception { verify(response.response).setStatus(405); assertContains(response.body(), "Method Not Allowed"); } + + @Test + public void testHandleDelete_AsyncReturnsAcceptedWithoutBody() throws Exception { + final DisksRouteHandler handler = new DisksRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE", Map.of("async", "true"), null, null), delete.response, "/api/disks/disk-1", + Negotiation.OutFormat.JSON, newServlet()); + + verify(handler.serverAdapter).deleteDisk("disk-1", true); + verify(delete.response).setStatus(202); + assertEquals("", delete.body()); + } + + @Test + public void testHandleDelete_SyncReturnsActionWithJob() throws Exception { + final DisksRouteHandler handler = new DisksRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + final ResourceAction action = new ResourceAction(); + action.setJob(Ref.of("/ovirt-engine/api/jobs/job-1", "job-1")); + action.setStatus("complete"); + when(handler.serverAdapter.deleteDisk("disk-1", false)).thenReturn(action); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, "/api/disks/disk-1", Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(200); + assertContains(delete.body(), "\"status\":\"complete\""); + assertContains(delete.body(), "\"id\":\"job-1\""); + } + + @Test + public void testHandleDelete_JobFailureReturnsOperationFailedFault() throws Exception { + final DisksRouteHandler handler = new DisksRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.deleteDisk("disk-1", false)).thenThrow(new CloudRuntimeException("Failed to delete disk", + new AsyncJobFailedException("job-1", "volume in use"))); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, "/api/disks/disk-1", Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(400); + assertContains(delete.body(), "Operation Failed"); + } } diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/StorageDomainsRouteHandlerTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/StorageDomainsRouteHandlerTest.java new file mode 100644 index 000000000000..4484ba02a9a6 --- /dev/null +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/StorageDomainsRouteHandlerTest.java @@ -0,0 +1,88 @@ +// 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.veeam.api; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; + +import com.cloud.exception.InvalidParameterValueException; +import org.apache.cloudstack.veeam.api.dto.StorageDomain; +import org.apache.cloudstack.veeam.utils.Negotiation; +import org.junit.Test; + +public class StorageDomainsRouteHandlerTest extends RouteHandlerTestSupport { + + @Test + public void testHandleGetListAndById() throws Exception { + final StorageDomainsRouteHandler handler = new StorageDomainsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.listAllStorageDomains(null, null, null)).thenReturn(List.of(withId(new StorageDomain(), "sd-1"))); + when(handler.serverAdapter.getStorageDomain("sd-1")).thenReturn(withId(new StorageDomain(), "sd-1")); + + final ResponseCapture list = newResponse(); + handler.handle(newRequest("GET"), list.response, "/api/storagedomains", Negotiation.OutFormat.JSON, newServlet()); + verify(list.response).setStatus(200); + assertContains(list.body(), "\"storage_domain\":["); + + final ResponseCapture item = newResponse(); + handler.handle(newRequest("GET"), item.response, "/api/storagedomains/sd-1", Negotiation.OutFormat.JSON, newServlet()); + verify(item.response).setStatus(200); + assertContains(item.body(), "\"id\":\"sd-1\""); + } + + @Test + public void testHandleGetListPassesSearchNamePattern() throws Exception { + final StorageDomainsRouteHandler handler = new StorageDomainsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.listAllStorageDomains("data%", null, null)).thenReturn(List.of(withId(new StorageDomain(), "sd-1"))); + + final ResponseCapture list = newResponse(); + handler.handle(newRequest("GET", Map.of("search", "name=data*"), null, null), list.response, + "/api/storagedomains", Negotiation.OutFormat.JSON, newServlet()); + + verify(list.response).setStatus(200); + assertContains(list.body(), "sd-1"); + } + + @Test + public void testHandleMissingStorageDomainReturnsNotFound() throws Exception { + final StorageDomainsRouteHandler handler = new StorageDomainsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.getStorageDomain("missing")).thenThrow(new InvalidParameterValueException("missing storage domain")); + + final ResponseCapture response = newResponse(); + handler.handle(newRequest("GET"), response.response, "/api/storagedomains/missing", Negotiation.OutFormat.JSON, newServlet()); + + verify(response.response).setStatus(404); + assertContains(response.body(), "missing storage domain"); + } + + @Test + public void testHandleNonGetReturnsMethodNotAllowed() throws Exception { + final StorageDomainsRouteHandler handler = new StorageDomainsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + + final ResponseCapture response = newResponse(); + handler.handle(newRequest("POST"), response.response, "/api/storagedomains", Negotiation.OutFormat.JSON, newServlet()); + + verify(response.response).setStatus(405); + } +} diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/VmsRouteHandlerTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/VmsRouteHandlerTest.java index 281254fd6864..a5f4c0718335 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/VmsRouteHandlerTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/VmsRouteHandlerTest.java @@ -18,19 +18,27 @@ package org.apache.cloudstack.veeam.api; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.List; import java.util.Map; +import org.apache.cloudstack.veeam.adapter.AsyncJobFailedException; import org.apache.cloudstack.veeam.api.dto.Backup; import org.apache.cloudstack.veeam.api.dto.Checkpoint; import org.apache.cloudstack.veeam.api.dto.Disk; import org.apache.cloudstack.veeam.api.dto.DiskAttachment; import org.apache.cloudstack.veeam.api.dto.Nic; +import org.apache.cloudstack.veeam.api.dto.Ref; import org.apache.cloudstack.veeam.api.dto.ResourceAction; import org.apache.cloudstack.veeam.api.dto.Snapshot; import org.apache.cloudstack.veeam.api.dto.Vm; @@ -39,6 +47,9 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.exception.CloudRuntimeException; + public class VmsRouteHandlerTest extends RouteHandlerTestSupport { @Test @@ -122,8 +133,8 @@ public void testHandleGetByIdDeleteAndPowerActions() throws Exception { handler.handle(newRequest("DELETE", Map.of("async", "true", "detach_only", "false"), null, null), delete.response, "/api/vms/vm-1", Negotiation.OutFormat.JSON, newServlet()); verify(handler.serverAdapter).deleteInstance("vm-1", true, true); - verify(delete.response).setStatus(200); - assertContains(delete.body(), "deleted"); + verify(delete.response).setStatus(202); + assertEquals("", delete.body()); final ResponseCapture start = newResponse(); handler.handle(newRequest("POST"), start.response, "/api/vms/vm-1/start", Negotiation.OutFormat.JSON, newServlet()); @@ -225,7 +236,7 @@ public void testHandleSnapshotRoutes() throws Exception { handler.handle(newRequest("DELETE", Map.of("async", "true"), null, null), delete.response, "/api/vms/vm-1/snapshots/snap-1", Negotiation.OutFormat.JSON, newServlet()); verify(delete.response).setStatus(202); - assertContains(delete.body(), "deleted"); + assertEquals("", delete.body()); final ResponseCapture restore = newResponse(); handler.handle(newRequest("POST"), restore.response, "/api/vms/vm-1/snapshots/snap-1/restore", Negotiation.OutFormat.JSON, newServlet()); @@ -294,4 +305,312 @@ public void testHandleCheckpointRoutes() throws Exception { verify(handler.serverAdapter).deleteCheckpoint("vm-1", "chk-1"); verify(delete.response).setStatus(200); } + + @Test + public void testIsRequestDetachOnly_EmptyValueIsTrueRegardlessOfDefault() throws Exception { + assertTrue(VmsRouteHandler.isRequestDetachOnly(newRequest("DELETE", Map.of("detach_only", ""), null, null), false)); + assertTrue(VmsRouteHandler.isRequestDetachOnly(newRequest("DELETE", Map.of("detach_only", ""), null, null), true)); + } + + @Test + public void testIsRequestDetachOnly_MissingOrInvalidValueReturnsValueIfMissing() throws Exception { + assertFalse(VmsRouteHandler.isRequestDetachOnly(newRequest("DELETE"), false)); + assertTrue(VmsRouteHandler.isRequestDetachOnly(newRequest("DELETE"), true)); + assertFalse(VmsRouteHandler.isRequestDetachOnly(newRequest("DELETE", Map.of("detach_only", "invalid"), null, null), false)); + assertTrue(VmsRouteHandler.isRequestDetachOnly(newRequest("DELETE", Map.of("detach_only", "invalid"), null, null), true)); + } + + @Test + public void testIsRequestDetachOnly_ExplicitValueOverridesValueIfMissing() throws Exception { + for (String value : new String[] {"true", "TRUE", "1"}) { + assertTrue(VmsRouteHandler.isRequestDetachOnly(newRequest("DELETE", Map.of("detach_only", value), null, null), false)); + } + for (String value : new String[] {"false", "False", "0"}) { + assertFalse(VmsRouteHandler.isRequestDetachOnly(newRequest("DELETE", Map.of("detach_only", value), null, null), true)); + } + } + + @Test + public void testHandleDeleteById_DeletesVolumesWhenDetachOnlyMissing() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.deleteInstance("vm-1", true, false)).thenReturn(new VmAction()); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, "/api/vms/vm-1", Negotiation.OutFormat.JSON, newServlet()); + verify(handler.serverAdapter).deleteInstance("vm-1", true, false); + verify(delete.response).setStatus(200); + } + + @Test + public void testHandleDeleteById_KeepsVolumesWhenDetachOnlyEmptyOrTrue() throws Exception { + for (String value : new String[] {"", "true", "1"}) { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.deleteInstance("vm-1", false, false)).thenReturn(new VmAction()); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE", Map.of("detach_only", value), null, null), delete.response, + "/api/vms/vm-1", Negotiation.OutFormat.JSON, newServlet()); + verify(handler.serverAdapter).deleteInstance("vm-1", false, false); + verify(delete.response).setStatus(200); + } + } + + @Test + public void testHandleDeleteDiskAttachment_OnlyDetachesWhenDetachOnlyMissing() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, "/api/vms/vm-1/diskattachments/disk-1", + Negotiation.OutFormat.JSON, newServlet()); + verify(handler.serverAdapter).detachInstanceDisk("vm-1", "disk-1"); + verify(handler.serverAdapter, never()).deleteDisk(anyString(), anyBoolean()); + verify(delete.response).setStatus(200); + } + + @Test + public void testHandleDeleteDiskAttachment_DeletesDiskWhenDetachOnlyFalse() throws Exception { + for (String value : new String[] {"false", "0"}) { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE", Map.of("detach_only", value), null, null), delete.response, + "/api/vms/vm-1/diskattachments/disk-1", Negotiation.OutFormat.JSON, newServlet()); + verify(handler.serverAdapter).detachInstanceDisk("vm-1", "disk-1"); + verify(handler.serverAdapter).deleteDisk("disk-1", false); + verify(delete.response).setStatus(200); + } + } + + @Test + public void testHandleDeleteDiskAttachment_DeleteReturnsDiskDeletionJob() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + final ResourceAction action = new ResourceAction(); + action.setJob(Ref.of("/ovirt-engine/api/jobs/job-1", "job-1")); + when(handler.serverAdapter.deleteDisk("disk-1", false)).thenReturn(action); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE", Map.of("detach_only", "false"), null, null), delete.response, + "/api/vms/vm-1/diskattachments/disk-1", Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(200); + assertContains(delete.body(), "\"status\":\"complete\""); + assertContains(delete.body(), "\"id\":\"job-1\""); + } + + @Test + public void testHandleActions_AsyncJobFailureReturnsFailedActionWithFaultAndJob() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + final CloudRuntimeException failure = new CloudRuntimeException("Failed to start VM: Timed out waiting for job job-1", + new AsyncJobFailedException("job-1", "Timed out waiting for job job-1")); + when(handler.serverAdapter.startInstance("vm-1", false)).thenThrow(failure); + + final ResponseCapture start = newResponse(); + handler.handle(newRequest("POST"), start.response, "/api/vms/vm-1/start", Negotiation.OutFormat.JSON, newServlet()); + + verify(start.response).setStatus(400); + assertContains(start.body(), "\"status\":\"failed\""); + assertContains(start.body(), "\"reason\":\"Operation Failed\""); + assertContains(start.body(), "Failed to start VM: Timed out waiting for job job-1"); + assertContains(start.body(), "/api/jobs/job-1"); + } + + @Test + public void testHandleActions_AsyncJobFailureXmlContainsFaultAndJob() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.stopInstance("vm-1", false)).thenThrow(new CloudRuntimeException( + "Failed to stop VM: Timed out waiting for job job-1", + new AsyncJobFailedException("job-1", "Timed out waiting for job job-1"))); + + final ResponseCapture stop = newResponse(); + handler.handle(newRequest("POST"), stop.response, "/api/vms/vm-1/stop", Negotiation.OutFormat.XML, newServlet()); + + verify(stop.response).setStatus(400); + assertContains(stop.body(), "failed"); + assertContains(stop.body(), "Operation Failed"); + assertContains(stop.body(), "Failed to stop VM: Timed out waiting for job job-1"); + assertContains(stop.body(), "job-1"); + } + + @Test + public void testHandleDeleteById_AsyncJobFailureReturnsPlainFault() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + final CloudRuntimeException failure = new CloudRuntimeException("Failed to delete VM: Timed out waiting for job job-1", + new AsyncJobFailedException("job-1", "Timed out waiting for job job-1")); + when(handler.serverAdapter.deleteInstance("vm-1", true, false)).thenThrow(failure); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, "/api/vms/vm-1", Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(400); + assertContains(delete.body(), "\"reason\":\"Operation Failed\""); + assertContains(delete.body(), "Failed to delete VM: Timed out waiting for job job-1"); + assertFalse(delete.body().contains("\"status\"")); + assertFalse(delete.body().contains("\"job\"")); + } + + @Test + public void testHandleDeleteSnapshot_AsyncJobFailureReturnsOperationFailedFault() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + final CloudRuntimeException failure = new CloudRuntimeException("Failed to delete snapshot: Job job-2 failed after 3s: boom", + new AsyncJobFailedException("job-2", "Job job-2 failed after 3s: boom")); + when(handler.serverAdapter.deleteSnapshot("snap-1", false)).thenThrow(failure); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, "/api/vms/vm-1/snapshots/snap-1", + Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(400); + assertContains(delete.body(), "\"reason\":\"Operation Failed\""); + assertContains(delete.body(), "Failed to delete snapshot: Job job-2 failed after 3s: boom"); + assertFalse(delete.body().contains("\"status\"")); + } + + @Test + public void testHandleDeleteById_OtherErrorReturnsBadRequestFault() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.deleteInstance("vm-1", true, false)) + .thenThrow(new CloudRuntimeException("Failed to delete VM: boom")); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, "/api/vms/vm-1", Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(400); + assertContains(delete.body(), "Failed to delete VM: boom"); + assertFalse(delete.body().contains("Operation Failed")); + } + + @Test + public void testHandleDeleteById_SyncReturnsCompletedActionWithJob() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + final VmAction action = new VmAction(); + action.setJob(Ref.of("/ovirt-engine/api/jobs/job-1", "job-1")); + action.setVm(withId(new Vm(), "vm-1")); + action.setStatus("in_progress"); + when(handler.serverAdapter.deleteInstance("vm-1", true, false)).thenReturn(action); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, "/api/vms/vm-1", Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(200); + assertContains(delete.body(), "\"status\":\"complete\""); + assertContains(delete.body(), "/api/jobs/job-1"); + assertFalse(delete.body().contains("\"vm\"")); + } + + @Test + public void testHandleDelete_AsyncReturnsAcceptedWithoutBody() throws Exception { + for (String async : new String[] {"true", "1"}) { + for (String path : new String[] {"/api/vms/vm-1", "/api/vms/vm-1/diskattachments/disk-1", + "/api/vms/vm-1/snapshots/snap-1", "/api/vms/vm-1/checkpoints/chk-1", "/api/vms/vm-1/nics/nic-1"}) { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.deleteInstance("vm-1", true, true)).thenReturn(new VmAction()); + when(handler.serverAdapter.deleteSnapshot("snap-1", true)).thenReturn(new ResourceAction()); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE", Map.of("async", async), null, null), delete.response, path, + Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(202); + assertEquals("Expected empty body for " + path, "", delete.body()); + } + } + } + + @Test + public void testHandleDelete_SyncWithoutJobReturnsCompletedAction() throws Exception { + for (String path : new String[] {"/api/vms/vm-1/diskattachments/disk-1", "/api/vms/vm-1/checkpoints/chk-1", + "/api/vms/vm-1/nics/nic-1"}) { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, path, Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(200); + assertEquals("{\"status\":\"complete\"}", delete.body()); + } + } + + @Test + public void testHandleActions_OtherErrorReturnsPlainFault() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.startInstance("vm-1", false)).thenThrow(new CloudRuntimeException("Failed to start VM: boom")); + + final ResponseCapture start = newResponse(); + handler.handle(newRequest("POST"), start.response, "/api/vms/vm-1/start", Negotiation.OutFormat.JSON, newServlet()); + + verify(start.response).setStatus(400); + assertContains(start.body(), "Failed to start VM: boom"); + assertFalse(start.body().contains("\"status\":\"failed\"")); + } + + @Test + public void testHandleGetNicById() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.getInstanceNic("vm-1", "nic-1")).thenReturn(withId(new Nic(), "nic-1")); + + final ResponseCapture get = newResponse(); + handler.handle(newRequest("GET"), get.response, "/api/vms/vm-1/nics/nic-1", Negotiation.OutFormat.JSON, newServlet()); + + verify(handler.serverAdapter).getInstanceNic("vm-1", "nic-1"); + verify(get.response).setStatus(200); + assertContains(get.body(), "\"id\":\"nic-1\""); + } + + @Test + public void testHandleNicById_NotFoundReturns404() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.getInstanceNic("vm-1", "nic-x")) + .thenThrow(new InvalidParameterValueException("NIC with ID nic-x not found for VM with ID vm-1")); + doThrow(new InvalidParameterValueException("NIC with ID nic-x not found for VM with ID vm-1")) + .when(handler.serverAdapter).detachInstanceNic("vm-1", "nic-x"); + + for (String method : new String[] {"GET", "DELETE"}) { + final ResponseCapture response = newResponse(); + handler.handle(newRequest(method), response.response, "/api/vms/vm-1/nics/nic-x", Negotiation.OutFormat.JSON, newServlet()); + verify(response.response).setStatus(404); + assertContains(response.body(), "NIC with ID nic-x not found for VM with ID vm-1"); + } + } + + @Test + public void testHandleDeleteNicById_FailureReturns400() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + doThrow(new CloudRuntimeException("Failed to remove NIC: it is the default nic")) + .when(handler.serverAdapter).detachInstanceNic("vm-1", "nic-1"); + + final ResponseCapture delete = newResponse(); + handler.handle(newRequest("DELETE"), delete.response, "/api/vms/vm-1/nics/nic-1", Negotiation.OutFormat.JSON, newServlet()); + + verify(delete.response).setStatus(400); + assertContains(delete.body(), "Failed to remove NIC: it is the default nic"); + } + + @Test + public void testHandleNicById_UnsupportedMethodReturns405() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + + final ResponseCapture put = newResponse(); + handler.handle(newRequest("PUT"), put.response, "/api/vms/vm-1/nics/nic-1", Negotiation.OutFormat.JSON, newServlet()); + + verify(put.response).setStatus(405); + verify(put.response).setHeader("Allow", "GET, DELETE"); + } } diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/request/ListQueryTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/request/ListQueryTest.java index a6d8bfef1d4b..8ea78b21d305 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/request/ListQueryTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/request/ListQueryTest.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.veeam.api.request; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -81,6 +82,47 @@ public void testFromRequest_SearchParserExtractsPageOnly() { org.junit.Assert.assertEquals(Long.valueOf(3L), query.getPage()); } + private ListQuery queryWithSearch(String search) { + final HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getParameterMap()).thenReturn(Map.of("search", new String[]{search})); + when(request.getParameter("search")).thenReturn(search); + return ListQuery.fromRequest(request); + } + + @Test + public void testFromRequest_SearchParserExtractsNameAndPage() { + final ListQuery query = queryWithSearch("name=data* page 2"); + + assertEquals("data*", query.getSearchName()); + assertEquals("data%", query.getSearchNameLikePattern()); + assertEquals(Long.valueOf(2L), query.getPage()); + } + + @Test + public void testFromRequest_SearchParserExtractsQuotedName() { + assertEquals("my pool", queryWithSearch("name = \"my pool\"").getSearchName()); + } + + @Test + public void testFromRequest_SortByNameIsNotANameClause() { + assertNull(queryWithSearch("sortby name page 3").getSearchName()); + } + + @Test + public void testFromRequest_IgnoresQualifiedNameClause() { + assertNull(queryWithSearch("vms.name=vm1").getSearchName()); + } + + @Test + public void testGetSearchNameLikePattern_MatchAllWildcardReturnsNull() { + assertNull(queryWithSearch("name=*").getSearchNameLikePattern()); + } + + @Test + public void testGetSearchNameLikePattern_EscapesLikeSpecialChars() { + assertEquals("pool\\_1\\%%", queryWithSearch("name=pool_1%*").getSearchNameLikePattern()); + } + @Test public void testGetOffset_UsesPageAndMax() { final ListQuery query = new ListQuery(); diff --git a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java index ed381743c354..1a258da1f0a9 100644 --- a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDao.java @@ -47,6 +47,10 @@ public interface StoragePoolJoinDao extends GenericDao List findStoragePoolByScopeAndRuleTags(Long datacenterId, Long podId, Long clusterId, ScopeType scopeType, List tags); - List listAvailableByZoneHypervisorAndType(long zoneId, Hypervisor.HypervisorType hypervisorType, List types, Filter filter); + /** + * Lists available pools matching the criteria, one entry per pool. The filter's paging applies to pools, not to view rows. + * A null nameLikePattern skips the name filter. + */ + List listAvailableByZonesHypervisorAndType(List zoneIds, Hypervisor.HypervisorType hypervisorType, List types, String nameLikePattern, Filter filter); } diff --git a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java index 946e3df4e90d..1ee027502e7d 100644 --- a/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java @@ -18,8 +18,12 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; import javax.inject.Inject; @@ -423,18 +427,25 @@ public List findStoragePoolByScopeAndRuleTags(Long datacenterId, return filteredPools; } - public List listAvailableByZoneHypervisorAndType(long zoneId, Hypervisor.HypervisorType hypervisorType, List types, Filter filter) { + @Override + public List listAvailableByZonesHypervisorAndType(List zoneIds, Hypervisor.HypervisorType hypervisorType, List types, String nameLikePattern, Filter filter) { + if (CollectionUtils.isEmpty(zoneIds)) { + return Collections.emptyList(); + } List availableStatus = Arrays.asList( StoragePoolStatus.Up, StoragePoolStatus.Disabled ); SearchBuilder sb = createSearchBuilder(); - sb.and("zoneId", sb.entity().getZoneId(), SearchCriteria.Op.EQ); + // The view has a row per pool tag, access group, etc., so select distinct pool ids to page over pools + sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); + sb.and("zoneIds", sb.entity().getZoneId(), SearchCriteria.Op.IN); sb.and("hypervisors", sb.entity().getHypervisor(), SearchCriteria.Op.IN); sb.and("types", sb.entity().getPoolType(), SearchCriteria.Op.IN); sb.and("status", sb.entity().getStatus(), SearchCriteria.Op.IN); + sb.and("name", sb.entity().getName(), SearchCriteria.Op.LIKE); sb.done(); SearchCriteria sc = sb.create(); - sc.setParameters("zoneId", zoneId); + sc.setParameters("zoneIds", zoneIds.toArray()); List hypervisors = new ArrayList<>(); hypervisors.add(Hypervisor.HypervisorType.Any); hypervisors.add(hypervisorType); @@ -443,6 +454,21 @@ public List listAvailableByZoneHypervisorAndType(long zoneId, sc.setParameters("types", types.toArray()); } sc.setParameters("status", availableStatus.toArray()); - return listBy(sc, filter); + if (nameLikePattern != null) { + sc.setParameters("name", nameLikePattern); + } + List poolIds = listBy(sc, filter).stream().map(StoragePoolJoinVO::getId).collect(Collectors.toList()); + return listOneRowPerPool(poolIds); + } + + protected List listOneRowPerPool(List poolIds) { + if (CollectionUtils.isEmpty(poolIds)) { + return Collections.emptyList(); + } + Map poolsById = new HashMap<>(); + for (StoragePoolJoinVO pool : searchByIds(poolIds.toArray(new Long[0]))) { + poolsById.putIfAbsent(pool.getId(), pool); + } + return poolIds.stream().map(poolsById::get).filter(Objects::nonNull).collect(Collectors.toList()); } } diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDao.java index ae62f301f466..9cb51fe86c6b 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDao.java @@ -52,6 +52,9 @@ List listByAccountServiceOfferingTemplateAndNotInState(long accoun List listLeaseInstancesExpiringInDays(int days); + /** + * Lists VMs matching the criteria, one entry per VM. The filter's paging applies to VMs, not to view rows. + */ List listByZonesHypervisorNotTypesAndOwners(List zoneIds, Hypervisor.HypervisorType hypervisorType, List excludeTypes, List accountIds, String domainPath, Filter filter); diff --git a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java index ebc2326a72d5..ddc6d9797d15 100644 --- a/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java @@ -28,6 +28,7 @@ import java.util.Hashtable; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -935,6 +936,8 @@ public List listByZonesHypervisorNotTypesAndOwners(List zone return Collections.emptyList(); } SearchBuilder sb = createSearchBuilder(); + // The view has a row per VM volume, NIC, tag, etc., so select distinct VM ids to page over VMs + sb.select(null, SearchCriteria.Func.DISTINCT, sb.entity().getId()); sb.and("dataCenterId", sb.entity().getDataCenterId(), SearchCriteria.Op.IN); sb.and("hypervisorType", sb.entity().getHypervisorType(), Op.EQ); if (CollectionUtils.isNotEmpty(excludeTypes)) { @@ -962,6 +965,18 @@ public List listByZonesHypervisorNotTypesAndOwners(List zone if (domainPathNotBlank) { sc.setParameters("domainPath", domainPath + "%"); } - return listBy(sc, filter); + List vmIds = listBy(sc, filter).stream().map(UserVmJoinVO::getId).collect(Collectors.toList()); + return listOneRowPerVm(vmIds); + } + + protected List listOneRowPerVm(List vmIds) { + if (CollectionUtils.isEmpty(vmIds)) { + return Collections.emptyList(); + } + Map vmsById = new HashMap<>(); + for (UserVmJoinVO vm : searchByIds(vmIds.toArray(new Long[0]))) { + vmsById.putIfAbsent(vm.getId(), vm); + } + return vmIds.stream().map(vmsById::get).filter(Objects::nonNull).collect(Collectors.toList()); } } diff --git a/server/src/test/java/com/cloud/api/query/dao/StoragePoolJoinDaoImplTest.java b/server/src/test/java/com/cloud/api/query/dao/StoragePoolJoinDaoImplTest.java index 64da9abff1ef..263aca9361f9 100644 --- a/server/src/test/java/com/cloud/api/query/dao/StoragePoolJoinDaoImplTest.java +++ b/server/src/test/java/com/cloud/api/query/dao/StoragePoolJoinDaoImplTest.java @@ -16,7 +16,8 @@ // under the License. package com.cloud.api.query.dao; -import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -65,57 +66,81 @@ public void setUp() { doReturn(searchBuilder).when(storagePoolJoinDao).createSearchBuilder(); } + private StoragePoolJoinVO mockPoolRow(long id) { + StoragePoolJoinVO row = mock(StoragePoolJoinVO.class); + when(row.getId()).thenReturn(id); + return row; + } + @Test - public void listAvailableByZoneHypervisorAndTypeReturnsMatchingPoolsWhenTypesAreProvided() { - long zoneId = 42L; + public void listAvailableByZonesHypervisorAndTypeReturnsMatchingPoolsWhenTypesAreProvided() { + List zoneIds = Arrays.asList(42L, 43L); Hypervisor.HypervisorType hypervisorType = Hypervisor.HypervisorType.KVM; List types = Arrays.asList(Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.Filesystem); Filter filter = mock(Filter.class); - List expectedPools = Collections.singletonList(mock(StoragePoolJoinVO.class)); + StoragePoolJoinVO pool = mockPoolRow(5L); - doReturn(expectedPools).when(storagePoolJoinDao).listBy(searchCriteria, filter); + doReturn(Collections.singletonList(mockPoolRow(5L))).when(storagePoolJoinDao).listBy(searchCriteria, filter); + doReturn(Collections.singletonList(pool)).when(storagePoolJoinDao).searchByIds(5L); - List result = storagePoolJoinDao.listAvailableByZoneHypervisorAndType(zoneId, hypervisorType, types, filter); + List result = storagePoolJoinDao.listAvailableByZonesHypervisorAndType(zoneIds, hypervisorType, types, "data%", filter); - assertSame(expectedPools, result); - verify(searchCriteria).setParameters("zoneId", zoneId); + assertEquals(Collections.singletonList(pool), result); + verify(searchCriteria).setParameters(eq("zoneIds"), aryEq(zoneIds.toArray())); verify(searchCriteria).setParameters(eq("hypervisors"), aryEq(new Object[]{Hypervisor.HypervisorType.Any, hypervisorType})); verify(searchCriteria).setParameters(eq("types"), aryEq(types.toArray())); + verify(searchCriteria).setParameters("name", "data%"); verify(storagePoolJoinDao).listBy(searchCriteria, filter); } @Test - public void listAvailableByZoneHypervisorAndTypeSkipsTypeFilterWhenTypesAreNull() { - long zoneId = 7L; + public void listAvailableByZonesHypervisorAndTypeSkipsTypeFilterWhenTypesAreNull() { Hypervisor.HypervisorType hypervisorType = Hypervisor.HypervisorType.VMware; Filter filter = mock(Filter.class); - List expectedPools = Collections.emptyList(); - doReturn(expectedPools).when(storagePoolJoinDao).listBy(searchCriteria, filter); + doReturn(Collections.emptyList()).when(storagePoolJoinDao).listBy(searchCriteria, filter); - List result = storagePoolJoinDao.listAvailableByZoneHypervisorAndType(zoneId, hypervisorType, null, filter); + List result = storagePoolJoinDao.listAvailableByZonesHypervisorAndType(Collections.singletonList(7L), hypervisorType, null, null, filter); - assertSame(expectedPools, result); - verify(searchCriteria).setParameters("zoneId", zoneId); + assertTrue(result.isEmpty()); + verify(searchCriteria).setParameters(eq("zoneIds"), aryEq(new Object[]{7L})); verify(searchCriteria).setParameters(eq("hypervisors"), aryEq(new Object[]{Hypervisor.HypervisorType.Any, hypervisorType})); verify(searchCriteria, never()).setParameters(eq("types"), any(Object[].class)); + verify(searchCriteria, never()).setParameters(eq("name"), any()); verify(storagePoolJoinDao).listBy(searchCriteria, filter); + verify(storagePoolJoinDao, never()).searchByIds(any()); } @Test - public void listAvailableByZoneHypervisorAndTypeSkipsTypeFilterForEmptyTypesAndPassesNullFilter() { - long zoneId = 9L; + public void listAvailableByZonesHypervisorAndTypeSkipsTypeFilterForEmptyTypesAndPassesNullFilter() { Hypervisor.HypervisorType hypervisorType = Hypervisor.HypervisorType.XenServer; - List expectedPools = Collections.singletonList(mock(StoragePoolJoinVO.class)); - doReturn(expectedPools).when(storagePoolJoinDao).listBy(searchCriteria, null); + doReturn(Collections.emptyList()).when(storagePoolJoinDao).listBy(searchCriteria, null); - List result = storagePoolJoinDao.listAvailableByZoneHypervisorAndType(zoneId, hypervisorType, Collections.emptyList(), null); + storagePoolJoinDao.listAvailableByZonesHypervisorAndType(Collections.singletonList(9L), hypervisorType, Collections.emptyList(), null, null); - assertSame(expectedPools, result); - verify(searchCriteria).setParameters("zoneId", zoneId); - verify(searchCriteria).setParameters(eq("hypervisors"), aryEq(new Object[]{Hypervisor.HypervisorType.Any, hypervisorType})); verify(searchCriteria, never()).setParameters(eq("types"), any(Object[].class)); verify(storagePoolJoinDao).listBy(searchCriteria, null); } + + @Test + public void listAvailableByZonesHypervisorAndTypeReturnsEmptyListForNoZones() { + assertTrue(storagePoolJoinDao.listAvailableByZonesHypervisorAndType(Collections.emptyList(), + Hypervisor.HypervisorType.KVM, null, null, null).isEmpty()); + verify(storagePoolJoinDao, never()).createSearchBuilder(); + } + + @Test + public void listOneRowPerPoolReturnsSingleRowPerPoolInIdOrder() { + StoragePoolJoinVO pool1Tag1 = mockPoolRow(1L); + StoragePoolJoinVO pool1Tag2 = mockPoolRow(1L); + StoragePoolJoinVO pool2 = mockPoolRow(2L); + StoragePoolJoinVO pool3Group1 = mockPoolRow(3L); + StoragePoolJoinVO pool3Group2 = mockPoolRow(3L); + doReturn(Arrays.asList(pool1Tag1, pool3Group1, pool1Tag2, pool2, pool3Group2)).when(storagePoolJoinDao).searchByIds(3L, 1L, 2L, 4L); + + List result = storagePoolJoinDao.listOneRowPerPool(Arrays.asList(3L, 1L, 2L, 4L)); + + assertEquals(Arrays.asList(pool3Group1, pool1Tag1, pool2), result); + } } diff --git a/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java b/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java index f657a8bbf045..682c6565925c 100755 --- a/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java +++ b/server/src/test/java/com/cloud/api/query/dao/UserVmJoinDaoImplTest.java @@ -23,6 +23,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; +import java.util.List; import com.cloud.storage.dao.VMTemplateDao; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -212,4 +213,33 @@ public void effectiveCdromMaxCountClampsToHypervisorCap() { // Configured cap defaults to 1 (no cluster override mocked); host advertises 2; clamps to 1. Assert.assertEquals(1, _userVmJoinDaoImpl.effectiveCdromMaxCount(userVm)); } + + private UserVmJoinVO mockVmRow(long id) { + UserVmJoinVO row = Mockito.mock(UserVmJoinVO.class); + Mockito.when(row.getId()).thenReturn(id); + return row; + } + + @Test + public void testListOneRowPerVmReturnsSingleRowPerVmInIdOrder() { + UserVmJoinDaoImpl dao = Mockito.spy(_userVmJoinDaoImpl); + UserVmJoinVO vm1Disk1 = mockVmRow(1L); + UserVmJoinVO vm1Disk2 = mockVmRow(1L); + UserVmJoinVO vm2 = mockVmRow(2L); + UserVmJoinVO vm3Nic1 = mockVmRow(3L); + UserVmJoinVO vm3Nic2 = mockVmRow(3L); + Mockito.doReturn(Arrays.asList(vm1Disk1, vm3Nic1, vm1Disk2, vm2, vm3Nic2)).when(dao).searchByIds(3L, 1L, 2L, 4L); + + List result = dao.listOneRowPerVm(Arrays.asList(3L, 1L, 2L, 4L)); + + Assert.assertEquals(Arrays.asList(vm3Nic1, vm1Disk1, vm2), result); + } + + @Test + public void testListOneRowPerVmEmptyIdsReturnsEmptyList() { + UserVmJoinDaoImpl dao = Mockito.spy(_userVmJoinDaoImpl); + + Assert.assertTrue(dao.listOneRowPerVm(Collections.emptyList()).isEmpty()); + Mockito.verify(dao, Mockito.never()).searchByIds(Mockito.any()); + } } From b34f75465e4b97a967bcb9b6df54fcdca59b1d32 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 15 Oct 2026 13:23:01 +0530 Subject: [PATCH 2/9] veeam-control-service,server: fix non-existent template restore --- .../veeam/adapter/ServerAdapter.java | 47 +++++++- .../veeam/adapter/ServerAdapterTest.java | 106 +++++++++++++++++- .../java/com/cloud/vm/UserVmManagerImpl.java | 4 + .../java/com/cloud/vm/UserVmManagerTest.java | 15 +++ 4 files changed, 163 insertions(+), 9 deletions(-) diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java index d99aa53996c1..106e0c91b1ac 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java @@ -199,10 +199,12 @@ import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDetailsDao; import com.cloud.tags.ResourceTagVO; import com.cloud.tags.dao.ResourceTagDao; +import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.DomainService; @@ -306,6 +308,9 @@ public class ServerAdapter extends ManagerBase { @Inject VMTemplateDao templateDao; + @Inject + VMTemplateZoneDao templateZoneDao; + @Inject UserVmManager userVmManager; @@ -655,19 +660,53 @@ protected GuestOS getGuestOsForInstance(Vm request) { return os; } - protected VMTemplateVO getTemplateForInstanceCreation(String templateUuid, GuestOS guestOs) { + /** + * Returns the template to create the Instance with, or null when the template is missing or not usable, + * in which case the Instance is created with the blank KVM template + * ({@link com.cloud.vm.VirtualMachineManager#KVM_BLANK_VM_TEMPLATE_NAME}). + */ + protected VMTemplateVO getTemplateForInstanceCreation(String templateUuid, GuestOS guestOs, + com.cloud.dc.DataCenter zone, Account owner) { if (StringUtils.isBlank(templateUuid)) { return null; } VMTemplateVO template = templateDao.findByUuid(templateUuid); if (template == null) { - logger.warn("Template with ID {} not found, guest OS: {}, VM will be created with default template", - guestOs, templateUuid); + logger.warn("Template with ID {} not found, VM will be created with the blank template and guest OS: {}", + templateUuid, guestOs); + return null; + } + if (!isTemplateUsableForInstanceCreation(template, zone, owner)) { + logger.warn("{} is not usable, VM will be created with the blank template and guest OS: {}", + template, guestOs); return null; } return template; } + /** + * Checks whether the template can be used to create an Instance for the owner in the zone: the template must + * be active, available in the zone and the owner must have permission to use it. + */ + protected boolean isTemplateUsableForInstanceCreation(VMTemplateVO template, com.cloud.dc.DataCenter zone, + Account owner) { + if (!VirtualMachineTemplate.State.Active.equals(template.getState())) { + logger.warn("{} is not active, current state: {}", template, template.getState()); + return false; + } + if (CollectionUtils.isEmpty(templateZoneDao.listByZoneTemplate(zone.getId(), template.getId()))) { + logger.warn("{} is not available in zone {}", template, zone); + return false; + } + try { + accountService.checkAccess(owner, SecurityChecker.AccessType.UseEntry, false, template); + } catch (PermissionDeniedException e) { + logger.warn("{} is not accessible for the account {}: {}", template, owner, e.getMessage()); + return false; + } + return true; + } + protected List getValidatedSshKeyPairNames(String sshKeyPairNames, Account owner) { if (StringUtils.isBlank(sshKeyPairNames)) { return null; @@ -852,7 +891,7 @@ protected Pair createInstance(com.cloud.dc.DataCenter zone, Long clu if (bootMode != null) { cmd.setBootMode(bootMode.toString()); } - VMTemplateVO template = getTemplateForInstanceCreation(templateUuid, guestOs); + VMTemplateVO template = getTemplateForInstanceCreation(templateUuid, guestOs, zone, account); if (template != null) { cmd.setTemplateId(template.getId()); } else if (guestOs != null) { diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java index c6e7cfc04bda..42d0e40218f3 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java @@ -31,6 +31,7 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -46,6 +47,7 @@ import org.apache.cloudstack.api.command.user.vm.RemoveNicFromVMCmd; import org.apache.cloudstack.api.command.user.volume.DestroyVolumeCmd; import org.apache.cloudstack.api.response.ExceptionResponse; +import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.backup.BackupVO; import org.apache.cloudstack.backup.KVMBackupExportService; import org.apache.cloudstack.backup.dao.BackupDao; @@ -104,9 +106,12 @@ import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.VMTemplateZoneVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.storage.dao.VolumeDao; +import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; @@ -147,6 +152,7 @@ public class ServerAdapterTest { @Mock ImageTransferDao imageTransferDao; @Mock ServiceOfferingDao serviceOfferingDao; @Mock VMTemplateDao templateDao; + @Mock VMTemplateZoneDao templateZoneDao; @Mock UserVmManager userVmManager; @Mock NicDao nicDao; @Mock AsyncJobDao asyncJobDao; @@ -457,27 +463,117 @@ public void testGetDummyTags_ContainsRootTag() { @Test public void testGetTemplateForInstanceCreation_NullUuid_ReturnsNull() { - assertNull(serverAdapter.getTemplateForInstanceCreation(null, null)); + assertNull(serverAdapter.getTemplateForInstanceCreation(null, null, mock(DataCenterVO.class), mock(Account.class))); + verify(templateDao, never()).findByUuid(any()); } @Test public void testGetTemplateForInstanceCreation_BlankUuid_ReturnsNull() { - assertNull(serverAdapter.getTemplateForInstanceCreation(" ", null)); + assertNull(serverAdapter.getTemplateForInstanceCreation(" ", null, mock(DataCenterVO.class), mock(Account.class))); + verify(templateDao, never()).findByUuid(any()); } @Test public void testGetTemplateForInstanceCreation_TemplateNotFound_ReturnsNull() { + ServerAdapter spyAdapter = spy(serverAdapter); when(templateDao.findByUuid("missing-uuid")).thenReturn(null); - assertNull(serverAdapter.getTemplateForInstanceCreation("missing-uuid", null)); + assertNull(spyAdapter.getTemplateForInstanceCreation("missing-uuid", null, mock(DataCenterVO.class), + mock(Account.class))); + verify(spyAdapter, never()).isTemplateUsableForInstanceCreation(any(), any(), any()); } @Test - public void testGetTemplateForInstanceCreation_TemplateFound_ReturnsTemplate() { + public void testGetTemplateForInstanceCreation_TemplateNotUsable_ReturnsNull() { + ServerAdapter spyAdapter = spy(serverAdapter); + VMTemplateVO template = mock(VMTemplateVO.class); + DataCenterVO zone = mock(DataCenterVO.class); + Account owner = mock(Account.class); + when(templateDao.findByUuid("valid-uuid")).thenReturn(template); + doReturn(false).when(spyAdapter).isTemplateUsableForInstanceCreation(template, zone, owner); + assertNull(spyAdapter.getTemplateForInstanceCreation("valid-uuid", null, zone, owner)); + } + + @Test + public void testGetTemplateForInstanceCreation_TemplateUsable_ReturnsTemplate() { + ServerAdapter spyAdapter = spy(serverAdapter); VMTemplateVO template = mock(VMTemplateVO.class); + DataCenterVO zone = mock(DataCenterVO.class); + Account owner = mock(Account.class); when(templateDao.findByUuid("valid-uuid")).thenReturn(template); - assertEquals(template, serverAdapter.getTemplateForInstanceCreation("valid-uuid", null)); + doReturn(true).when(spyAdapter).isTemplateUsableForInstanceCreation(template, zone, owner); + assertEquals(template, spyAdapter.getTemplateForInstanceCreation("valid-uuid", null, zone, owner)); } + private VMTemplateVO mockTemplateForUsabilityCheck(VirtualMachineTemplate.State state) { + VMTemplateVO template = mock(VMTemplateVO.class); + when(template.getState()).thenReturn(state); + Mockito.lenient().when(template.getId()).thenReturn(5L); + return template; + } + + private DataCenterVO mockZoneForUsabilityCheck() { + DataCenterVO zone = mock(DataCenterVO.class); + Mockito.lenient().when(zone.getId()).thenReturn(1L); + return zone; + } + + @Test + public void testIsTemplateUsableForInstanceCreation_ActiveInZoneAndAccessible_ReturnsTrue() { + VMTemplateVO template = mockTemplateForUsabilityCheck(VirtualMachineTemplate.State.Active); + DataCenterVO zone = mockZoneForUsabilityCheck(); + Account owner = mock(Account.class); + when(templateZoneDao.listByZoneTemplate(1L, 5L)).thenReturn(List.of(mock(VMTemplateZoneVO.class))); + + assertTrue(serverAdapter.isTemplateUsableForInstanceCreation(template, zone, owner)); + verify(accountService).checkAccess(owner, SecurityChecker.AccessType.UseEntry, false, template); + } + + @Test + public void testIsTemplateUsableForInstanceCreation_NotActive_ReturnsFalse() { + for (VirtualMachineTemplate.State state : new VirtualMachineTemplate.State[] { + VirtualMachineTemplate.State.Inactive, VirtualMachineTemplate.State.NotUploaded, + VirtualMachineTemplate.State.UploadInProgress, VirtualMachineTemplate.State.UploadError, + VirtualMachineTemplate.State.UploadAbandoned}) { + VMTemplateVO template = mockTemplateForUsabilityCheck(state); + assertFalse("Expected not usable for state " + state, serverAdapter.isTemplateUsableForInstanceCreation( + template, mockZoneForUsabilityCheck(), mock(Account.class))); + } + verify(templateZoneDao, never()).listByZoneTemplate(any(), Mockito.anyLong()); + verify(accountService, never()).checkAccess(any(Account.class), any(SecurityChecker.AccessType.class), + Mockito.anyBoolean(), any(VMTemplateVO.class)); + } + + @Test + public void testIsTemplateUsableForInstanceCreation_NotInZone_ReturnsFalse() { + VMTemplateVO template = mockTemplateForUsabilityCheck(VirtualMachineTemplate.State.Active); + DataCenterVO zone = mockZoneForUsabilityCheck(); + when(templateZoneDao.listByZoneTemplate(1L, 5L)).thenReturn(Collections.emptyList()); + + assertFalse(serverAdapter.isTemplateUsableForInstanceCreation(template, zone, mock(Account.class))); + verify(accountService, never()).checkAccess(any(Account.class), any(SecurityChecker.AccessType.class), + Mockito.anyBoolean(), any(VMTemplateVO.class)); + } + + @Test + public void testIsTemplateUsableForInstanceCreation_NullZoneRefs_ReturnsFalse() { + VMTemplateVO template = mockTemplateForUsabilityCheck(VirtualMachineTemplate.State.Active); + DataCenterVO zone = mockZoneForUsabilityCheck(); + when(templateZoneDao.listByZoneTemplate(1L, 5L)).thenReturn(null); + + assertFalse(serverAdapter.isTemplateUsableForInstanceCreation(template, zone, mock(Account.class))); + } + + @Test + public void testIsTemplateUsableForInstanceCreation_NoPermission_ReturnsFalse() { + VMTemplateVO template = mockTemplateForUsabilityCheck(VirtualMachineTemplate.State.Active); + DataCenterVO zone = mockZoneForUsabilityCheck(); + Account owner = mock(Account.class); + when(templateZoneDao.listByZoneTemplate(1L, 5L)).thenReturn(List.of(mock(VMTemplateZoneVO.class))); + doThrow(new PermissionDeniedException("denied")).when(accountService) + .checkAccess(owner, SecurityChecker.AccessType.UseEntry, false, template); + + assertFalse(serverAdapter.isTemplateUsableForInstanceCreation(template, zone, owner)); + } @Test public void testGetZoneById_NullId_ReturnsNull() { diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index d93dae9ebdec..53a2de3e6a5a 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -5375,6 +5375,10 @@ protected Long getGuestOsIdIfNeeded(VirtualMachineTemplate template) { public void validateRootDiskResize(final HypervisorType hypervisorType, Long rootDiskSize, VMTemplateVO templateVO, UserVmVO vm, final Map customParameters) throws InvalidParameterValueException { + if (_itMgr.isBlankInstanceDefaultTemplate(templateVO)) { + logger.debug("{} is the default blank Instance template, skipping root disk size validation", templateVO); + return; + } // rootdisksize must be larger than template. boolean isIso = ImageFormat.ISO == templateVO.getFormat(); if ((rootDiskSize << 30) < templateVO.getSize()) { diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java index 2a7d35ed399f..9c488b53baf4 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java @@ -98,6 +98,7 @@ import java.util.UUID; import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -276,6 +277,20 @@ public void testValidateRootDiskResize() { _userVmMgr.validateRootDiskResize(hypervisorType, rootDiskSize, templateVO, vm, customParameters); } + @Test + public void testValidateRootDiskResizeBlankInstanceDefaultTemplate() { + UserVmVO vm = Mockito.mock(UserVmVO.class); + VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class); + Map customParameters = new HashMap<>(); + customParameters.put("rootdisksize", "10"); + when(_itMgr.isBlankInstanceDefaultTemplate(templateVO)).thenReturn(true); + + _userVmMgr.validateRootDiskResize(HypervisorType.KVM, 10L, templateVO, vm, customParameters); + + assertEquals("10", customParameters.get("rootdisksize")); + Mockito.verify(templateVO, Mockito.never()).getSize(); + } + // Test scaleVm on incompatible HV. @Test(expected = InvalidParameterValueException.class) public void testScaleVMF1() throws Exception { From cc729a5d7df2e3b57ebfc7dbe6ee73ebb293521c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 21 Oct 2026 14:42:43 +0530 Subject: [PATCH 3/9] veeam-control-service: non-existent vm returns 404 --- .../veeam/adapter/ServerAdapter.java | 4 + .../cloudstack/veeam/api/VmsRouteHandler.java | 111 +++++++++++++++--- .../veeam/adapter/ServerAdapterTest.java | 8 ++ .../veeam/api/VmsRouteHandlerTest.java | 68 +++++++++++ 4 files changed, 176 insertions(+), 15 deletions(-) diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java index 106e0c91b1ac..f0bc99b56291 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java @@ -1412,6 +1412,10 @@ public List listAllInstances(boolean includeTags, boolean includeDisks, bool allContent); } + public boolean instanceExists(String uuid) { + return userVmDao.findByUuid(uuid) != null; + } + @ApiAccess(command = ListVMsCmd.class) public Vm getInstance(String uuid, boolean includeTags, boolean includeDisks, boolean includeNics, boolean allContent) { diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/VmsRouteHandler.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/VmsRouteHandler.java index 34b3b0253e5e..5502473394a9 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/VmsRouteHandler.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/VmsRouteHandler.java @@ -197,14 +197,14 @@ public void handle(HttpServletRequest req, HttpServletResponse resp, String path if (!"GET".equalsIgnoreCase(method) && !"DELETE".equalsIgnoreCase(method)) { io.methodNotAllowed(resp, "GET, DELETE", outFormat); } else if ("GET".equalsIgnoreCase(method)) { - handleGetSnapshotById(subId, resp, outFormat, io); + handleGetSnapshotById(id, subId, resp, outFormat, io); } else if ("DELETE".equalsIgnoreCase(method)) { - handleDeleteSnapshotById(subId, req, resp, outFormat, io); + handleDeleteSnapshotById(id, subId, req, resp, outFormat, io); } return; } else if ("backups".equals(subPath)) { if ("GET".equalsIgnoreCase(method)) { - handleGetBackupById(subId, resp, outFormat, io); + handleGetBackupById(id, subId, resp, outFormat, io); } else { io.methodNotAllowed(resp, "GET", outFormat); } @@ -223,14 +223,14 @@ public void handle(HttpServletRequest req, HttpServletResponse resp, String path String action = idAndSubPath.get(3); if ("snapshots".equals(subPath) && "restore".equals(action)) { if ("POST".equalsIgnoreCase(method)) { - handleRestoreSnapshotById(subId, req, resp, outFormat, io); + handleRestoreSnapshotById(id, subId, req, resp, outFormat, io); } else { io.methodNotAllowed(resp, "POST", outFormat); } return; } else if ("backups".equals(subPath) && "disks".equals(action)) { if ("GET".equalsIgnoreCase(method)) { - handleGetBackupDisksById(subId, req, resp, outFormat, io); + handleGetBackupDisksById(id, subId, req, resp, outFormat, io); } else { io.methodNotAllowed(resp, "GET", outFormat); } @@ -278,8 +278,20 @@ protected void handlePost(final HttpServletRequest req, final HttpServletRespons } } + protected boolean checkInstanceExists(final String id, final HttpServletResponse resp, + final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (serverAdapter.instanceExists(id)) { + return true; + } + io.notFound(resp, "VM with ID " + id + " not found", outFormat); + return false; + } + protected void handleGetById(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } try { ListQuery query = ListQuery.fromRequest(req); Vm response = serverAdapter.getInstance(id, @@ -297,6 +309,9 @@ protected void handleGetById(final String id, final HttpServletRequest req, fina protected void handleUpdateById(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } String data = RouteHandler.getRequestData(req, logger); try { Vm request = io.getMapper().jsonMapper().readValue(data, Vm.class); @@ -309,6 +324,9 @@ protected void handleUpdateById(final String id, final HttpServletRequest req, f protected void handleDeleteById(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } boolean async = RouteHandler.isRequestAsync(req); boolean deleteVolumes = !isRequestDetachOnly(req, false); try { @@ -321,6 +339,9 @@ protected void handleDeleteById(final String id, final HttpServletRequest req, f protected void handleStartVmById(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } boolean async = RouteHandler.isRequestAsync(req); try { VmAction vm = serverAdapter.startInstance(id, async); @@ -332,6 +353,9 @@ protected void handleStartVmById(final String id, final HttpServletRequest req, protected void handleStopVmById(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } boolean async = RouteHandler.isRequestAsync(req); try { VmAction vm = serverAdapter.stopInstance(id, async); @@ -343,6 +367,9 @@ protected void handleStopVmById(final String id, final HttpServletRequest req, f protected void handleShutdownVmById(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } boolean async = RouteHandler.isRequestAsync(req); try { VmAction vm = serverAdapter.shutdownInstance(id, async); @@ -354,6 +381,9 @@ protected void handleShutdownVmById(final String id, final HttpServletRequest re protected void handleGetDiskAttachmentsByVmId(final String id, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } try { List disks = serverAdapter.listDiskAttachmentsByInstanceUuid(id); NamedList response = NamedList.of("disk_attachment", disks); @@ -368,6 +398,9 @@ protected void handleGetDiskAttachmentsByVmId(final String id, final HttpServlet protected void handlePostDiskAttachmentForVmId(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } String data = RouteHandler.getRequestData(req, logger); try { DiskAttachment request = io.getMapper().jsonMapper().readValue(data, DiskAttachment.class); @@ -380,6 +413,9 @@ protected void handlePostDiskAttachmentForVmId(final String id, final HttpServle protected void handleGetNicsByVmId(final String id, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } try { List nics = serverAdapter.listNicsByInstanceUuid(id); NamedList response = NamedList.of("nic", nics); @@ -394,6 +430,9 @@ protected void handleGetNicsByVmId(final String id, final HttpServletResponse re protected void handlePostNicForVmId(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } String data = RouteHandler.getRequestData(req, logger); try { Nic request = io.getMapper().jsonMapper().readValue(data, Nic.class); @@ -432,6 +471,9 @@ protected void handleDeleteNicById(final String vmId, final String nicId, final protected void handleGetSnapshotsByVmId(final String id, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } try { List snapshots = serverAdapter.listSnapshotsByInstanceUuid(id); NamedList response = NamedList.of("snapshot", snapshots); @@ -446,6 +488,9 @@ protected void handleGetSnapshotsByVmId(final String id, final HttpServletRespon protected void handlePostSnapshotForVmId(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } String data = RouteHandler.getRequestData(req, logger); try { Snapshot request = io.getMapper().jsonMapper().readValue(data, Snapshot.class); @@ -461,6 +506,9 @@ protected void handleDeleteDiskAttachmentForVmId(final String vmId, final String final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { boolean async = RouteHandler.isRequestAsync(req); boolean delete = !isRequestDetachOnly(req, true); + if (!checkInstanceExists(vmId, resp, outFormat, io)) { + return; + } try { serverAdapter.detachInstanceDisk(vmId, diskId); ResourceAction action = null; @@ -475,10 +523,13 @@ protected void handleDeleteDiskAttachmentForVmId(final String vmId, final String } } - protected void handleGetSnapshotById(final String id, final HttpServletResponse resp, + protected void handleGetSnapshotById(final String vmId, final String snapshotId, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(vmId, resp, outFormat, io)) { + return; + } try { - Snapshot response = serverAdapter.getSnapshot(id); + Snapshot response = serverAdapter.getSnapshot(snapshotId); io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); } catch (InvalidParameterValueException e) { io.notFound(resp, e.getMessage(), outFormat); @@ -487,25 +538,31 @@ protected void handleGetSnapshotById(final String id, final HttpServletResponse } } - protected void handleDeleteSnapshotById(final String id, final HttpServletRequest req, + protected void handleDeleteSnapshotById(final String vmId, final String snapshotId, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(vmId, resp, outFormat, io)) { + return; + } boolean async = RouteHandler.isRequestAsync(req); try { - ResourceAction action = serverAdapter.deleteSnapshot(id, async); + ResourceAction action = serverAdapter.deleteSnapshot(snapshotId, async); RouteHandler.writeDeleteResponse(resp, async, action, outFormat, io); } catch (CloudRuntimeException e) { handleDeleteError(resp, e, outFormat, io); } } - protected void handleRestoreSnapshotById(final String id, final HttpServletRequest req, + protected void handleRestoreSnapshotById(final String vmId, final String snapshotId, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(vmId, resp, outFormat, io)) { + return; + } boolean async = RouteHandler.isRequestAsync(req); String data = RouteHandler.getRequestData(req, logger); try { - ResourceAction response = serverAdapter.revertInstanceToSnapshot(id, async); + ResourceAction response = serverAdapter.revertInstanceToSnapshot(snapshotId, async); io.getWriter().write(resp, HttpServletResponse.SC_ACCEPTED, response, outFormat); } catch (CloudRuntimeException e) { handleActionError(resp, e, outFormat, io); @@ -514,6 +571,9 @@ protected void handleRestoreSnapshotById(final String id, final HttpServletReque protected void handleGetBackupsByVmId(final String id, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } try { List backups = serverAdapter.listBackupsByInstanceUuid(id); NamedList response = NamedList.of("backup", backups); @@ -528,6 +588,9 @@ protected void handleGetBackupsByVmId(final String id, final HttpServletResponse protected void handlePostBackupForVmId(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } String data = RouteHandler.getRequestData(req, logger); try { Backup request = io.getMapper().jsonMapper().readValue(data, Backup.class); @@ -538,10 +601,13 @@ protected void handlePostBackupForVmId(final String id, final HttpServletRequest } } - protected void handleGetBackupById(final String id, final HttpServletResponse resp, + protected void handleGetBackupById(final String vmId, final String backupId, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(vmId, resp, outFormat, io)) { + return; + } try { - Backup response = serverAdapter.getBackup(id); + Backup response = serverAdapter.getBackup(backupId); io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); } catch (InvalidParameterValueException e) { io.notFound(resp, e.getMessage(), outFormat); @@ -550,11 +616,14 @@ protected void handleGetBackupById(final String id, final HttpServletResponse re } } - protected void handleGetBackupDisksById(final String id, final HttpServletRequest req, + protected void handleGetBackupDisksById(final String vmId, final String backupId, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(vmId, resp, outFormat, io)) { + return; + } try { - List disks = serverAdapter.listDisksByBackupUuid(id); + List disks = serverAdapter.listDisksByBackupUuid(backupId); NamedList response = NamedList.of("disk", disks); io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); } catch (InvalidParameterValueException e) { @@ -567,6 +636,9 @@ protected void handleGetBackupDisksById(final String id, final HttpServletReques protected void handleFinalizeBackupById(final String vmId, final String backupId, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(vmId, resp, outFormat, io)) { + return; + } try { Backup backup = serverAdapter.finalizeBackup(vmId, backupId); io.getWriter().write(resp, HttpServletResponse.SC_OK, backup, outFormat); @@ -577,6 +649,9 @@ protected void handleFinalizeBackupById(final String vmId, final String backupId protected void handleGetCheckpointsByVmId(final String id, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } try { List checkpoints = serverAdapter.listCheckpointsByInstanceUuid(id); NamedList response = NamedList.of("checkpoints", checkpoints); @@ -592,6 +667,9 @@ protected void handleDeleteCheckpoint(final String vmId, final String checkpoint final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { boolean async = RouteHandler.isRequestAsync(req); + if (!checkInstanceExists(vmId, resp, outFormat, io)) { + return; + } try { serverAdapter.deleteCheckpoint(vmId, checkpointId); RouteHandler.writeDeleteResponse(resp, async, null, outFormat, io); @@ -603,6 +681,9 @@ protected void handleDeleteCheckpoint(final String vmId, final String checkpoint protected void handlePostTagForVmId(final String id, final HttpServletRequest req, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + if (!checkInstanceExists(id, resp, outFormat, io)) { + return; + } String data = RouteHandler.getRequestData(req, logger); try { Tag request = io.getMapper().jsonMapper().readValue(data, Tag.class); diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java index 42d0e40218f3..e1007d673fdd 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java @@ -1134,6 +1134,14 @@ public void testGetInstance_NotFound_Throws() { } + @Test + public void testInstanceExists() { + when(userVmDao.findByUuid("vm-uuid")).thenReturn(mock(UserVmVO.class)); + when(userVmDao.findByUuid("vm-missing")).thenReturn(null); + assertTrue(serverAdapter.instanceExists("vm-uuid")); + assertFalse(serverAdapter.instanceExists("vm-missing")); + } + @Test(expected = InvalidParameterValueException.class) public void testDeleteInstance_VmNotFound_Throws() { when(userVmDao.findByUuid("vm-uuid")).thenReturn(null); diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/VmsRouteHandlerTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/VmsRouteHandlerTest.java index a5f4c0718335..910c625391f2 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/VmsRouteHandlerTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/VmsRouteHandlerTest.java @@ -26,7 +26,9 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import java.util.List; @@ -94,6 +96,7 @@ public void testHandlePostAndUpdateParseVmJson() throws Exception { updated.setName("vm-updated"); when(handler.serverAdapter.updateInstance(eq("vm-1"), updateCaptor.capture())).thenReturn(updated); + when(handler.serverAdapter.instanceExists("vm-1")).thenReturn(true); final ResponseCapture put = newResponse(); handler.handle(newRequest("PUT", Map.of(), "application/json", "{\"name\":\"vm-updated\"}"), put.response, "/api/vms/vm-1", Negotiation.OutFormat.JSON, newServlet()); @@ -106,6 +109,7 @@ public void testHandlePostAndUpdateParseVmJson() throws Exception { public void testHandleGetByIdDeleteAndPowerActions() throws Exception { final VmsRouteHandler handler = new VmsRouteHandler(); handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.instanceExists("vm-1")).thenReturn(true); when(handler.serverAdapter.getInstance("vm-1", true, true, true, true)).thenReturn(withId(new Vm(), "vm-1")); final VmAction deleteAction = new VmAction(); @@ -152,10 +156,71 @@ public void testHandleGetByIdDeleteAndPowerActions() throws Exception { assertContains(shutdown.body(), "shutting_down"); } + @Test + public void testHandleVmRoutesReturnNotFoundForMissingVm() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.instanceExists("vm-missing")).thenReturn(false); + + final String[][] routes = { + {"GET", "/api/vms/vm-missing"}, + {"PUT", "/api/vms/vm-missing"}, + {"DELETE", "/api/vms/vm-missing"}, + {"POST", "/api/vms/vm-missing/start"}, + {"POST", "/api/vms/vm-missing/stop"}, + {"POST", "/api/vms/vm-missing/shutdown"}, + {"GET", "/api/vms/vm-missing/diskattachments"}, + {"POST", "/api/vms/vm-missing/diskattachments"}, + {"DELETE", "/api/vms/vm-missing/diskattachments/disk-1"}, + {"GET", "/api/vms/vm-missing/nics"}, + {"POST", "/api/vms/vm-missing/nics"}, + {"GET", "/api/vms/vm-missing/snapshots"}, + {"POST", "/api/vms/vm-missing/snapshots"}, + {"GET", "/api/vms/vm-missing/snapshots/snap-1"}, + {"DELETE", "/api/vms/vm-missing/snapshots/snap-1"}, + {"POST", "/api/vms/vm-missing/snapshots/snap-1/restore"}, + {"GET", "/api/vms/vm-missing/backups"}, + {"POST", "/api/vms/vm-missing/backups"}, + {"GET", "/api/vms/vm-missing/backups/backup-1"}, + {"GET", "/api/vms/vm-missing/backups/backup-1/disks"}, + {"POST", "/api/vms/vm-missing/backups/backup-1/finalize"}, + {"GET", "/api/vms/vm-missing/checkpoints"}, + {"DELETE", "/api/vms/vm-missing/checkpoints/chk-1"}, + {"POST", "/api/vms/vm-missing/tags"} + }; + for (String[] route : routes) { + final ResponseCapture response = newResponse(); + handler.handle(newRequest(route[0], Map.of(), "application/json", "{}"), response.response, route[1], + Negotiation.OutFormat.JSON, newServlet()); + verify(response.response).setStatus(404); + assertContains(response.body(), "VM with ID vm-missing not found"); + } + + verify(handler.serverAdapter, times(routes.length)).instanceExists("vm-missing"); + verifyNoMoreInteractions(handler.serverAdapter); + } + + @Test + public void testHandleVmRouteWrongMethodSkipsExistenceCheck() throws Exception { + final VmsRouteHandler handler = new VmsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + + final ResponseCapture response = newResponse(); + handler.handle(newRequest("PATCH"), response.response, "/api/vms/vm-missing", Negotiation.OutFormat.JSON, newServlet()); + verify(response.response).setStatus(405); + + final ResponseCapture start = newResponse(); + handler.handle(newRequest("GET"), start.response, "/api/vms/vm-missing/start", Negotiation.OutFormat.JSON, newServlet()); + verify(start.response).setStatus(405); + + verify(handler.serverAdapter, never()).instanceExists(anyString()); + } + @Test public void testHandleDiskAttachmentAndNicRoutes() throws Exception { final VmsRouteHandler handler = new VmsRouteHandler(); handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.instanceExists("vm-1")).thenReturn(true); when(handler.serverAdapter.listDiskAttachmentsByInstanceUuid("vm-1")) .thenReturn(List.of(withId(new DiskAttachment(), "attach-1"))); when(handler.serverAdapter.listNicsByInstanceUuid("vm-1")) @@ -200,6 +265,7 @@ public void testHandleDiskAttachmentAndNicRoutes() throws Exception { public void testHandleSnapshotRoutes() throws Exception { final VmsRouteHandler handler = new VmsRouteHandler(); handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.instanceExists("vm-1")).thenReturn(true); when(handler.serverAdapter.listSnapshotsByInstanceUuid("vm-1")).thenReturn(List.of(withId(new Snapshot(), "snap-1"))); when(handler.serverAdapter.getSnapshot("snap-1")).thenReturn(withId(new Snapshot(), "snap-1")); @@ -248,6 +314,7 @@ public void testHandleSnapshotRoutes() throws Exception { public void testHandleBackupRoutes() throws Exception { final VmsRouteHandler handler = new VmsRouteHandler(); handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.instanceExists("vm-1")).thenReturn(true); when(handler.serverAdapter.listBackupsByInstanceUuid("vm-1")).thenReturn(List.of(withId(new Backup(), "backup-1"))); when(handler.serverAdapter.getBackup("backup-1")).thenReturn(withId(new Backup(), "backup-1")); when(handler.serverAdapter.listDisksByBackupUuid("backup-1")).thenReturn(List.of(withId(new Disk(), "disk-1"))); @@ -293,6 +360,7 @@ public void testHandleBackupRoutes() throws Exception { public void testHandleCheckpointRoutes() throws Exception { final VmsRouteHandler handler = new VmsRouteHandler(); handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.instanceExists("vm-1")).thenReturn(true); when(handler.serverAdapter.listCheckpointsByInstanceUuid("vm-1")).thenReturn(List.of(withId(new Checkpoint(), "chk-1"))); final ResponseCapture list = newResponse(); From f6a5f07124df0e9bff791c7e783cb555246b9abb Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 21 Oct 2026 16:59:38 +0530 Subject: [PATCH 4/9] veeam-control-service: hosts statistics --- .../veeam/adapter/ServerAdapter.java | 29 +++- .../veeam/api/HostsRouteHandler.java | 56 ++++++- .../converter/HostJoinVOToHostConverter.java | 84 ++++++++++- .../apache/cloudstack/veeam/api/dto/Host.java | 9 ++ .../cloudstack/veeam/api/dto/Statistic.java | 138 ++++++++++++++++++ .../veeam/adapter/ServerAdapterTest.java | 59 +++++++- .../veeam/api/HostsRouteHandlerTest.java | 69 ++++++++- .../HostJoinVOToHostConverterTest.java | 111 ++++++++++++++ .../veeam/api/dto/StatisticTest.java | 85 +++++++++++ 9 files changed, 621 insertions(+), 19 deletions(-) create mode 100644 plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Statistic.java create mode 100644 plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/dto/StatisticTest.java diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java index f0bc99b56291..f310c49ea577 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/adapter/ServerAdapter.java @@ -134,6 +134,7 @@ import org.apache.cloudstack.veeam.api.dto.OvfXmlUtil; import org.apache.cloudstack.veeam.api.dto.ResourceAction; import org.apache.cloudstack.veeam.api.dto.Snapshot; +import org.apache.cloudstack.veeam.api.dto.Statistic; import org.apache.cloudstack.veeam.api.dto.StorageDomain; import org.apache.cloudstack.veeam.api.dto.Tag; import org.apache.cloudstack.veeam.api.dto.Vm; @@ -170,6 +171,7 @@ import com.cloud.exception.PermissionDeniedException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; +import com.cloud.host.HostStats; import com.cloud.hypervisor.Hypervisor; import com.cloud.network.NetworkModel; import com.cloud.network.Networks; @@ -186,6 +188,7 @@ import com.cloud.projects.Project; import com.cloud.projects.ProjectManager; import com.cloud.server.ResourceTag; +import com.cloud.server.StatsCollector; import com.cloud.server.TaggedResourceService; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; @@ -266,6 +269,9 @@ public class ServerAdapter extends ManagerBase { @Inject HostJoinDao hostJoinDao; + @Inject + StatsCollector statsCollector; + @Inject NetworkDao networkDao; @@ -1338,20 +1344,35 @@ public Cluster getCluster(String uuid) { } @ApiAccess(command = ListHostsCmd.class) - public List listAllHosts(Long offset, Long limit) { + public List listAllHosts(boolean includeStatistics, Long offset, Long limit) { Filter filter = new Filter(HostJoinVO.class, "id", true, offset, limit); final List hosts = hostJoinDao.listAvailableRoutingByZonesAndHypervisor( kvmBackupExportService.listCompatibleDataCenterIds(), Hypervisor.HypervisorType.KVM, filter); - return HostJoinVOToHostConverter.toHostList(hosts); + return HostJoinVOToHostConverter.toHostList(hosts, includeStatistics ? this::getHostStats : null); + } + + @ApiAccess(command = ListHostsCmd.class) + public Host getHost(String uuid, boolean includeStatistics) { + final HostJoinVO vo = getHostJoinVOByUuid(uuid); + return HostJoinVOToHostConverter.toHost(vo, includeStatistics ? this::getHostStats : null); } @ApiAccess(command = ListHostsCmd.class) - public Host getHost(String uuid) { + public List listHostStatistics(String uuid) { + final HostJoinVO vo = getHostJoinVOByUuid(uuid); + return HostJoinVOToHostConverter.toStatistics(vo, getHostStats(vo.getId())); + } + + protected HostJoinVO getHostJoinVOByUuid(String uuid) { final HostJoinVO vo = hostJoinDao.findByUuid(uuid); if (vo == null) { throw new InvalidParameterValueException("Host with ID " + uuid + " not found"); } - return HostJoinVOToHostConverter.toHost(vo); + return vo; + } + + protected HostStats getHostStats(Long hostId) { + return statsCollector.getHostStats(hostId); } @ApiAccess(command = ListNetworksCmd.class) diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/HostsRouteHandler.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/HostsRouteHandler.java index 931291714c6c..06558edf4417 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/HostsRouteHandler.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/HostsRouteHandler.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.List; +import java.util.Optional; import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; @@ -27,8 +28,10 @@ import org.apache.cloudstack.veeam.RouteHandler; import org.apache.cloudstack.veeam.VeeamControlServlet; import org.apache.cloudstack.veeam.adapter.ServerAdapter; +import org.apache.cloudstack.veeam.api.converter.HostJoinVOToHostConverter; import org.apache.cloudstack.veeam.api.dto.Host; import org.apache.cloudstack.veeam.api.dto.NamedList; +import org.apache.cloudstack.veeam.api.dto.Statistic; import org.apache.cloudstack.veeam.api.request.ListQuery; import org.apache.cloudstack.veeam.utils.Negotiation; import org.apache.cloudstack.veeam.utils.PathUtil; @@ -76,9 +79,19 @@ public void handle(HttpServletRequest req, HttpServletResponse resp, String path if (CollectionUtils.isNotEmpty(idAndSubPath)) { String id = idAndSubPath.get(0); if (idAndSubPath.size() == 1) { - handleGetById(id, resp, outFormat, io); + handleGetById(id, req, resp, outFormat, io); return; } + if (HostJoinVOToHostConverter.STATISTICS.equals(idAndSubPath.get(1))) { + if (idAndSubPath.size() == 2) { + handleGetStatisticsById(id, resp, outFormat, io); + return; + } + if (idAndSubPath.size() == 3) { + handleGetStatisticById(id, idAndSubPath.get(2), resp, outFormat, io); + return; + } + } } io.notFound(resp, null, outFormat); @@ -88,7 +101,8 @@ protected void handleGet(final HttpServletRequest req, final HttpServletResponse Negotiation.OutFormat outFormat, VeeamControlServlet io) throws IOException { try { ListQuery query = ListQuery.fromRequest(req); - final List result = serverAdapter.listAllHosts(query.getOffset(), query.getLimit()); + final List result = serverAdapter.listAllHosts(query.followContains(HostJoinVOToHostConverter.STATISTICS), + query.getOffset(), query.getLimit()); NamedList response = NamedList.of("host", result); io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); } catch (PermissionDeniedException e) { @@ -96,10 +110,24 @@ protected void handleGet(final HttpServletRequest req, final HttpServletResponse } } - protected void handleGetById(final String id, final HttpServletResponse resp, final Negotiation.OutFormat outFormat, - final VeeamControlServlet io) throws IOException { + protected void handleGetById(final String id, final HttpServletRequest req, final HttpServletResponse resp, + final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + try { + ListQuery query = ListQuery.fromRequest(req); + Host response = serverAdapter.getHost(id, query.followContains(HostJoinVOToHostConverter.STATISTICS)); + io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); + } catch (InvalidParameterValueException e) { + io.notFound(resp, e.getMessage(), outFormat); + } catch (PermissionDeniedException e) { + io.badRequest(resp, e.getMessage(), outFormat); + } + } + + protected void handleGetStatisticsById(final String id, final HttpServletResponse resp, + final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { try { - Host response = serverAdapter.getHost(id); + List result = serverAdapter.listHostStatistics(id); + NamedList response = NamedList.of("statistic", result); io.getWriter().write(resp, HttpServletResponse.SC_OK, response, outFormat); } catch (InvalidParameterValueException e) { io.notFound(resp, e.getMessage(), outFormat); @@ -107,4 +135,22 @@ protected void handleGetById(final String id, final HttpServletResponse resp, fi io.badRequest(resp, e.getMessage(), outFormat); } } + + protected void handleGetStatisticById(final String id, final String statisticId, final HttpServletResponse resp, + final Negotiation.OutFormat outFormat, final VeeamControlServlet io) throws IOException { + try { + Optional response = serverAdapter.listHostStatistics(id).stream() + .filter(statistic -> statisticId.equals(statistic.getId())) + .findFirst(); + if (response.isEmpty()) { + io.notFound(resp, "Statistic with ID " + statisticId + " not found", outFormat); + return; + } + io.getWriter().write(resp, HttpServletResponse.SC_OK, response.get(), outFormat); + } catch (InvalidParameterValueException e) { + io.notFound(resp, e.getMessage(), outFormat); + } catch (PermissionDeniedException e) { + io.badRequest(resp, e.getMessage(), outFormat); + } + } } diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/converter/HostJoinVOToHostConverter.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/converter/HostJoinVOToHostConverter.java index da5aadac5f0e..38023c7a2b17 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/converter/HostJoinVOToHostConverter.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/converter/HostJoinVOToHostConverter.java @@ -17,8 +17,12 @@ package org.apache.cloudstack.veeam.api.converter; +import java.math.BigDecimal; +import java.math.RoundingMode; +import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.function.Function; import java.util.stream.Collectors; import org.apache.cloudstack.veeam.VeeamControlService; @@ -26,27 +30,44 @@ import org.apache.cloudstack.veeam.api.HostsRouteHandler; import org.apache.cloudstack.veeam.api.dto.Cpu; import org.apache.cloudstack.veeam.api.dto.Host; +import org.apache.cloudstack.veeam.api.dto.Link; +import org.apache.cloudstack.veeam.api.dto.NamedList; import org.apache.cloudstack.veeam.api.dto.Ref; +import org.apache.cloudstack.veeam.api.dto.Statistic; import org.apache.cloudstack.veeam.api.dto.Topology; import com.cloud.api.query.vo.HostJoinVO; +import com.cloud.host.HostStats; import com.cloud.host.Status; import com.cloud.resource.ResourceState; public class HostJoinVOToHostConverter { + public static final String STATISTICS = "statistics"; + + private static final BigDecimal HUNDRED = BigDecimal.valueOf(100); /** * Convert CloudStack HostJoinVO -> oVirt-like Host. * */ public static Host toHost(final HostJoinVO vo) { + return toHost(vo, null); + } + + /** + * Convert CloudStack HostJoinVO -> oVirt-like Host. + * + * @param statsResolver when non-null, the host's statistics are inlined (oVirt follow=statistics) + */ + public static Host toHost(final HostJoinVO vo, final Function statsResolver) { final Host h = new Host(); final String hostUuid = vo.getUuid(); h.setId(hostUuid); final String basePath = VeeamControlService.ContextPath.value(); - h.setHref(basePath + HostsRouteHandler.BASE_ROUTE + "/" + hostUuid); + final String href = getHref(hostUuid); + h.setHref(href); final String name = vo.getName() != null ? vo.getName() : ("host-" + hostUuid); h.setName(name); @@ -81,13 +102,70 @@ public static Host toHost(final HostJoinVO vo) { h.setActions(null); - h.setLink(Collections.emptyList()); + h.setLink(Collections.singletonList(Link.of(STATISTICS, href + "/" + STATISTICS))); + + if (statsResolver != null) { + h.setStatistics(NamedList.of("statistic", toStatistics(vo, statsResolver.apply(vo.getId())))); + } return h; } public static List toHostList(final List vos) { - return vos.stream().map(HostJoinVOToHostConverter::toHost).collect(Collectors.toList()); + return toHostList(vos, null); + } + + public static List toHostList(final List vos, final Function statsResolver) { + return vos.stream().map(vo -> toHost(vo, statsResolver)).collect(Collectors.toList()); + } + + /** + * Maps the collected host stats to oVirt host statistics. Physical memory is used for memory.total (not the + * overprovisioned value reported by listHosts) so that it is consistent with memory.used. + * Live values are only reported for hosts that are Up, as the stats cache keeps the last entry of a host that went down. + */ + public static List toStatistics(final HostJoinVO vo, final HostStats hostStats) { + final Ref hostRef = Ref.of(getHref(vo.getUuid()), vo.getUuid()); + final List statistics = new ArrayList<>(); + final boolean live = hostStats != null && Status.Up.equals(vo.getStatus()); + + long totalMemory = vo.getTotalMemory(); + if (live && hostStats.getTotalMemoryKBs() > 0) { + totalMemory = kbToBytes(hostStats.getTotalMemoryKBs()); + } + statistics.add(Statistic.hostGauge(hostRef, "memory.total", "Total memory", + Statistic.TYPE_INTEGER, Statistic.UNIT_BYTES, BigDecimal.valueOf(totalMemory))); + if (!live) { + return statistics; + } + + statistics.add(Statistic.hostGauge(hostRef, "memory.used", "Used memory", + Statistic.TYPE_INTEGER, Statistic.UNIT_BYTES, BigDecimal.valueOf(Math.max(0L, (long) hostStats.getUsedMemory())))); + statistics.add(Statistic.hostGauge(hostRef, "memory.free", "Free memory", + Statistic.TYPE_INTEGER, Statistic.UNIT_BYTES, BigDecimal.valueOf(kbToBytes(hostStats.getFreeMemoryKBs())))); + if (Double.isFinite(hostStats.getCpuUtilization())) { + statistics.add(Statistic.hostGauge(hostRef, "cpu.current.idle", "Idle CPU usage", + Statistic.TYPE_DECIMAL, Statistic.UNIT_PERCENT, toIdlePercent(hostStats.getCpuUtilization()))); + } + return statistics; + } + + private static String getHref(final String hostUuid) { + return VeeamControlService.ContextPath.value() + HostsRouteHandler.BASE_ROUTE + "/" + hostUuid; + } + + private static long kbToBytes(final double kb) { + return Math.max(0L, (long) (kb * 1024)); + } + + private static BigDecimal toIdlePercent(final double cpuUtilization) { + BigDecimal idle = HUNDRED.subtract(BigDecimal.valueOf(cpuUtilization)); + if (idle.signum() < 0) { + idle = BigDecimal.ZERO; + } else if (idle.compareTo(HUNDRED) > 0) { + idle = HUNDRED; + } + return idle.setScale(2, RoundingMode.HALF_UP); } private static String mapStatus(final HostJoinVO vo) { diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Host.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Host.java index 73efba5eeb89..275e746d8ec6 100644 --- a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Host.java +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Host.java @@ -49,6 +49,7 @@ public class Host extends BaseDto { private NamedList actions; private String name; private String comment; + private NamedList statistics; private List link; // getters/setters (generate via IDE) @@ -252,6 +253,14 @@ public void setComment(String comment) { this.comment = comment; } + public NamedList getStatistics() { + return statistics; + } + + public void setStatistics(NamedList statistics) { + this.statistics = statistics; + } + public List getLink() { return link; } diff --git a/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Statistic.java b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Statistic.java new file mode 100644 index 000000000000..602f65c15ad3 --- /dev/null +++ b/plugins/integrations/veeam-control-service/src/main/java/org/apache/cloudstack/veeam/api/dto/Statistic.java @@ -0,0 +1,138 @@ +// 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.veeam.api.dto; + +import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.UUID; + +import com.fasterxml.jackson.annotation.JsonInclude; + +@JsonInclude(JsonInclude.Include.NON_NULL) +public class Statistic extends BaseDto { + public static final String KIND_GAUGE = "gauge"; + public static final String TYPE_INTEGER = "integer"; + public static final String TYPE_DECIMAL = "decimal"; + public static final String UNIT_BYTES = "bytes"; + public static final String UNIT_PERCENT = "percent"; + + private String name; + private String description; + private String kind; + private String type; + private String unit; + private NamedList values; + private Ref host; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public String getKind() { + return kind; + } + + public void setKind(String kind) { + this.kind = kind; + } + + public String getType() { + return type; + } + + public void setType(String type) { + this.type = type; + } + + public String getUnit() { + return unit; + } + + public void setUnit(String unit) { + this.unit = unit; + } + + public NamedList getValues() { + return values; + } + + public void setValues(NamedList values) { + this.values = values; + } + + public Ref getHost() { + return host; + } + + public void setHost(Ref host) { + this.host = host; + } + + /** + * oVirt derives the statistic id from its name only, so the same statistic has the same id on every host. + */ + public static String idForName(final String name) { + return UUID.nameUUIDFromBytes(name.getBytes(StandardCharsets.UTF_8)).toString(); + } + + public static Statistic hostGauge(final Ref host, final String name, final String description, final String type, + final String unit, final BigDecimal datum) { + final Statistic statistic = withHrefAndId(new Statistic(), null, idForName(name)); + statistic.setHref(host.getHref() + "/statistics/" + statistic.getId()); + statistic.setName(name); + statistic.setDescription(description); + statistic.setKind(KIND_GAUGE); + statistic.setType(type); + statistic.setUnit(unit); + statistic.setValues(NamedList.of("value", Collections.singletonList(Value.of(datum)))); + statistic.setHost(host); + return statistic; + } + + @JsonInclude(JsonInclude.Include.NON_NULL) + public static class Value { + private BigDecimal datum; + + public static Value of(final BigDecimal datum) { + Value value = new Value(); + value.setDatum(datum); + return value; + } + + public BigDecimal getDatum() { + return datum; + } + + public void setDatum(BigDecimal datum) { + this.datum = datum; + } + } +} diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java index e1007d673fdd..6ef86bc77b9e 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/adapter/ServerAdapterTest.java @@ -25,6 +25,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doNothing; @@ -60,9 +61,11 @@ import org.apache.cloudstack.veeam.api.dto.DataCenter; import org.apache.cloudstack.veeam.api.dto.Disk; import org.apache.cloudstack.veeam.api.dto.Nic; -import org.apache.cloudstack.veeam.api.dto.OvfXmlUtil; import org.apache.cloudstack.veeam.api.dto.ResourceAction; import org.apache.cloudstack.veeam.api.dto.StorageDomain; +import org.apache.cloudstack.veeam.api.dto.Host; +import org.apache.cloudstack.veeam.api.dto.OvfXmlUtil; +import org.apache.cloudstack.veeam.api.dto.Statistic; import org.apache.cloudstack.veeam.api.dto.Tag; import org.apache.cloudstack.veeam.api.dto.Vm; import org.apache.logging.log4j.Logger; @@ -95,6 +98,8 @@ import com.cloud.domain.dao.DomainDao; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; +import com.cloud.host.HostStats; +import com.cloud.host.Status; import com.cloud.hypervisor.Hypervisor; import com.cloud.network.NetworkModel; import com.cloud.network.Networks; @@ -102,6 +107,7 @@ import com.cloud.network.dao.NetworkVO; import com.cloud.projects.Project; import com.cloud.projects.ProjectManager; +import com.cloud.server.StatsCollector; import com.cloud.service.ServiceOfferingVO; import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.Storage; @@ -140,6 +146,7 @@ public class ServerAdapterTest { @Mock StoragePoolJoinDao storagePoolJoinDao; @Mock ClusterDao clusterDao; @Mock HostJoinDao hostJoinDao; + @Mock StatsCollector statsCollector; @Mock NetworkDao networkDao; @Mock UserVmDao userVmDao; @Mock UserVmJoinDao userVmJoinDao; @@ -1101,14 +1108,35 @@ public void testListAllHosts_ReturnsList() { HostJoinVO hostVO = mock(HostJoinVO.class); when(hostJoinDao.listAvailableRoutingByZonesAndHypervisor(anyList(), any(), any())).thenReturn(List.of(hostVO)); - assertEquals(1, serverAdapter.listAllHosts(0L, 10L).size()); + List hosts = serverAdapter.listAllHosts(false, 0L, 10L); + + assertEquals(1, hosts.size()); + assertNull(hosts.get(0).getStatistics()); + verify(statsCollector, never()).getHostStats(anyLong()); + } + + @Test + public void testListAllHosts_WithStatistics_IncludesStatistics() { + HostJoinVO hostVO = mock(HostJoinVO.class); + when(hostVO.getId()).thenReturn(7L); + when(hostVO.getStatus()).thenReturn(Status.Up); + HostStats hostStats = mock(HostStats.class); + when(hostStats.getTotalMemoryKBs()).thenReturn(1024D); + when(statsCollector.getHostStats(7L)).thenReturn(hostStats); + when(hostJoinDao.listAvailableRoutingByZonesAndHypervisor(anyList(), any(), any())).thenReturn(List.of(hostVO)); + + List hosts = serverAdapter.listAllHosts(true, 0L, 10L); + + assertEquals(1, hosts.size()); + assertEquals(4, hosts.get(0).getStatistics().getItems().size()); + verify(statsCollector).getHostStats(7L); } @Test(expected = InvalidParameterValueException.class) public void testGetHost_NotFound_Throws() { when(hostJoinDao.findByUuid("host-uuid")).thenReturn(null); - serverAdapter.getHost("host-uuid"); + serverAdapter.getHost("host-uuid", false); } @Test @@ -1116,7 +1144,30 @@ public void testGetHost_Found_ReturnsHost() { HostJoinVO vo = mock(HostJoinVO.class); when(hostJoinDao.findByUuid("host-uuid")).thenReturn(vo); - assertNotNull(serverAdapter.getHost("host-uuid")); + Host host = serverAdapter.getHost("host-uuid", false); + + assertNotNull(host); + assertNull(host.getStatistics()); + } + + @Test(expected = InvalidParameterValueException.class) + public void testListHostStatistics_NotFound_Throws() { + when(hostJoinDao.findByUuid("host-uuid")).thenReturn(null); + serverAdapter.listHostStatistics("host-uuid"); + } + + @Test + public void testListHostStatistics_NoStats_ReturnsTotalMemoryOnly() { + HostJoinVO vo = mock(HostJoinVO.class); + when(vo.getId()).thenReturn(7L); + when(vo.getTotalMemory()).thenReturn(2048L); + when(hostJoinDao.findByUuid("host-uuid")).thenReturn(vo); + when(statsCollector.getHostStats(7L)).thenReturn(null); + + List statistics = serverAdapter.listHostStatistics("host-uuid"); + + assertEquals(1, statistics.size()); + assertEquals("memory.total", statistics.get(0).getName()); } diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/HostsRouteHandlerTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/HostsRouteHandlerTest.java index ef9370374726..0a7ff5533006 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/HostsRouteHandlerTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/HostsRouteHandlerTest.java @@ -21,10 +21,14 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.math.BigDecimal; import java.util.List; +import java.util.Map; import com.cloud.exception.InvalidParameterValueException; import org.apache.cloudstack.veeam.api.dto.Host; +import org.apache.cloudstack.veeam.api.dto.Ref; +import org.apache.cloudstack.veeam.api.dto.Statistic; import org.apache.cloudstack.veeam.utils.Negotiation; import org.junit.Test; @@ -34,8 +38,8 @@ public class HostsRouteHandlerTest extends RouteHandlerTestSupport { public void testHandleGetListAndGetById() throws Exception { final HostsRouteHandler handler = new HostsRouteHandler(); handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); - when(handler.serverAdapter.listAllHosts(null, null)).thenReturn(List.of(withId(new Host(), "host-1"))); - when(handler.serverAdapter.getHost("host-1")).thenReturn(withId(new Host(), "host-1")); + when(handler.serverAdapter.listAllHosts(false, null, null)).thenReturn(List.of(withId(new Host(), "host-1"))); + when(handler.serverAdapter.getHost("host-1", false)).thenReturn(withId(new Host(), "host-1")); final ResponseCapture list = newResponse(); handler.handle(newRequest("GET"), list.response, "/api/hosts", Negotiation.OutFormat.JSON, newServlet()); @@ -53,7 +57,7 @@ public void testHandleGetListAndGetById() throws Exception { public void testHandleGetByIdNotFound() throws Exception { final HostsRouteHandler handler = new HostsRouteHandler(); handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); - when(handler.serverAdapter.getHost("missing")).thenThrow(new InvalidParameterValueException("missing host")); + when(handler.serverAdapter.getHost("missing", false)).thenThrow(new InvalidParameterValueException("missing host")); final ResponseCapture response = newResponse(); handler.handle(newRequest("GET"), response.response, "/api/hosts/missing", Negotiation.OutFormat.JSON, newServlet()); @@ -61,4 +65,63 @@ public void testHandleGetByIdNotFound() throws Exception { verify(response.response).setStatus(404); assertContains(response.body(), "missing host"); } + + @Test + public void testHandleGetListAndGetById_FollowStatistics() throws Exception { + final HostsRouteHandler handler = new HostsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.listAllHosts(true, 0L, 100L)).thenReturn(List.of(withId(new Host(), "host-1"))); + when(handler.serverAdapter.getHost("host-1", true)).thenReturn(withId(new Host(), "host-1")); + + final ResponseCapture list = newResponse(); + handler.handle(newRequest("GET", Map.of("follow", "statistics", "max", "100", "search", "page 1"), null, null), + list.response, "/api/hosts", Negotiation.OutFormat.JSON, newServlet()); + verify(list.response).setStatus(200); + verify(handler.serverAdapter).listAllHosts(true, 0L, 100L); + + final ResponseCapture item = newResponse(); + handler.handle(newRequest("GET", Map.of("follow", "statistics"), null, null), + item.response, "/api/hosts/host-1", Negotiation.OutFormat.JSON, newServlet()); + verify(item.response).setStatus(200); + verify(handler.serverAdapter).getHost("host-1", true); + } + + @Test + public void testHandleGetStatistics() throws Exception { + final HostsRouteHandler handler = new HostsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + final Statistic statistic = Statistic.hostGauge(Ref.of("/api/hosts/host-1", "host-1"), "memory.total", "Total memory", + Statistic.TYPE_INTEGER, Statistic.UNIT_BYTES, BigDecimal.valueOf(1024)); + when(handler.serverAdapter.listHostStatistics("host-1")).thenReturn(List.of(statistic)); + + final ResponseCapture list = newResponse(); + handler.handle(newRequest("GET"), list.response, "/api/hosts/host-1/statistics", Negotiation.OutFormat.JSON, newServlet()); + verify(list.response).setStatus(200); + assertContains(list.body(), "\"statistic\":["); + assertContains(list.body(), "\"datum\":1024"); + + final ResponseCapture item = newResponse(); + handler.handle(newRequest("GET"), item.response, "/api/hosts/host-1/statistics/" + statistic.getId(), + Negotiation.OutFormat.JSON, newServlet()); + verify(item.response).setStatus(200); + assertContains(item.body(), "\"name\":\"memory.total\""); + + final ResponseCapture missing = newResponse(); + handler.handle(newRequest("GET"), missing.response, "/api/hosts/host-1/statistics/unknown", + Negotiation.OutFormat.JSON, newServlet()); + verify(missing.response).setStatus(404); + } + + @Test + public void testHandleGetStatisticsHostNotFound() throws Exception { + final HostsRouteHandler handler = new HostsRouteHandler(); + handler.serverAdapter = mock(org.apache.cloudstack.veeam.adapter.ServerAdapter.class); + when(handler.serverAdapter.listHostStatistics("missing")).thenThrow(new InvalidParameterValueException("missing host")); + + final ResponseCapture response = newResponse(); + handler.handle(newRequest("GET"), response.response, "/api/hosts/missing/statistics", Negotiation.OutFormat.JSON, newServlet()); + + verify(response.response).setStatus(404); + assertContains(response.body(), "missing host"); + } } diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/converter/HostJoinVOToHostConverterTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/converter/HostJoinVOToHostConverterTest.java index e5ab87b40c55..26ac801553a0 100644 --- a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/converter/HostJoinVOToHostConverterTest.java +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/converter/HostJoinVOToHostConverterTest.java @@ -18,16 +18,21 @@ package org.apache.cloudstack.veeam.api.converter; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.math.BigDecimal; import java.util.List; import org.apache.cloudstack.veeam.api.dto.Host; +import org.apache.cloudstack.veeam.api.dto.Statistic; import org.junit.Test; import com.cloud.api.query.vo.HostJoinVO; +import com.cloud.host.HostStats; import com.cloud.host.Status; import com.cloud.resource.ResourceState; @@ -120,4 +125,110 @@ public void testToHostList_ConvertsAllEntries() { assertEquals("h1", hosts.get(0).getId()); assertEquals("h2", hosts.get(1).getId()); } + + private static HostJoinVO mockHostVO(final Status status) { + final HostJoinVO vo = mock(HostJoinVO.class); + when(vo.getId()).thenReturn(11L); + when(vo.getUuid()).thenReturn("host-1"); + when(vo.getStatus()).thenReturn(status); + when(vo.getResourceState()).thenReturn(ResourceState.Enabled); + when(vo.getTotalMemory()).thenReturn(537986068480L); + return vo; + } + + private static HostStats mockHostStats(final double cpuUtilization) { + final HostStats stats = mock(HostStats.class); + when(stats.getCpuUtilization()).thenReturn(cpuUtilization); + when(stats.getTotalMemoryKBs()).thenReturn(525376042D); + when(stats.getFreeMemoryKBs()).thenReturn(511901000D); + when(stats.getUsedMemory()).thenReturn((525376042D - 511901000D) * 1024); + return stats; + } + + private static Statistic findStatistic(final List statistics, final String name) { + return statistics.stream().filter(s -> name.equals(s.getName())).findFirst().orElse(null); + } + + private static BigDecimal datumOf(final Statistic statistic) { + return statistic.getValues().getItems().get(0).getDatum(); + } + + @Test + public void testToHost_AddsStatisticsLink() { + final Host host = HostJoinVOToHostConverter.toHost(mockHostVO(Status.Up)); + + assertEquals(1, host.getLink().size()); + assertEquals("statistics", host.getLink().get(0).getRel()); + assertEquals(host.getHref() + "/statistics", host.getLink().get(0).getHref()); + assertNull(host.getStatistics()); + } + + @Test + public void testToHost_WithStatsResolver_InlinesStatistics() { + final HostStats stats = mockHostStats(3.49); + + final Host host = HostJoinVOToHostConverter.toHost(mockHostVO(Status.Up), id -> id == 11L ? stats : null); + + assertNotNull(host.getStatistics()); + assertEquals(4, host.getStatistics().getItems().size()); + } + + @Test + public void testToStatistics_UpHostWithStats_MapsVeeamStatistics() { + final List statistics = HostJoinVOToHostConverter.toStatistics(mockHostVO(Status.Up), mockHostStats(3.49)); + + assertEquals(4, statistics.size()); + + final Statistic total = findStatistic(statistics, "memory.total"); + assertEquals(BigDecimal.valueOf(525376042L * 1024), datumOf(total)); + assertEquals("integer", total.getType()); + assertEquals("bytes", total.getUnit()); + assertEquals("gauge", total.getKind()); + assertEquals("Total memory", total.getDescription()); + // Same id as oVirt/OLVM reports for memory.total + assertEquals("7816602b-c05c-3db7-a4da-3769f7ad8896", total.getId()); + assertEquals(total.getHost().getHref() + "/statistics/" + total.getId(), total.getHref()); + assertEquals("host-1", total.getHost().getId()); + + assertEquals(BigDecimal.valueOf((525376042L - 511901000L) * 1024), datumOf(findStatistic(statistics, "memory.used"))); + assertEquals(BigDecimal.valueOf(511901000L * 1024), datumOf(findStatistic(statistics, "memory.free"))); + + final Statistic idle = findStatistic(statistics, "cpu.current.idle"); + assertEquals(new BigDecimal("96.51"), datumOf(idle)); + assertEquals("decimal", idle.getType()); + assertEquals("percent", idle.getUnit()); + } + + @Test + public void testToStatistics_ClampsIdleCpu() { + assertEquals(new BigDecimal("0.00"), datumOf(findStatistic( + HostJoinVOToHostConverter.toStatistics(mockHostVO(Status.Up), mockHostStats(120)), "cpu.current.idle"))); + assertEquals(new BigDecimal("100.00"), datumOf(findStatistic( + HostJoinVOToHostConverter.toStatistics(mockHostVO(Status.Up), mockHostStats(-5)), "cpu.current.idle"))); + } + + @Test + public void testToStatistics_NonFiniteCpu_SkipsIdleCpu() { + final List statistics = HostJoinVOToHostConverter.toStatistics(mockHostVO(Status.Up), mockHostStats(Double.NaN)); + + assertEquals(3, statistics.size()); + assertNull(findStatistic(statistics, "cpu.current.idle")); + } + + @Test + public void testToStatistics_NoStats_ReturnsOnlyTotalMemoryFromHost() { + final List statistics = HostJoinVOToHostConverter.toStatistics(mockHostVO(Status.Up), null); + + assertEquals(1, statistics.size()); + assertEquals("memory.total", statistics.get(0).getName()); + assertEquals(BigDecimal.valueOf(537986068480L), datumOf(statistics.get(0))); + } + + @Test + public void testToStatistics_HostNotUp_IgnoresCachedStats() { + final List statistics = HostJoinVOToHostConverter.toStatistics(mockHostVO(Status.Down), mock(HostStats.class)); + + assertEquals(1, statistics.size()); + assertEquals(BigDecimal.valueOf(537986068480L), datumOf(statistics.get(0))); + } } diff --git a/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/dto/StatisticTest.java b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/dto/StatisticTest.java new file mode 100644 index 000000000000..12cbeb3aaec6 --- /dev/null +++ b/plugins/integrations/veeam-control-service/src/test/java/org/apache/cloudstack/veeam/api/dto/StatisticTest.java @@ -0,0 +1,85 @@ +// 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.veeam.api.dto; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.math.BigDecimal; +import java.util.List; + +import org.apache.cloudstack.veeam.utils.Mapper; +import org.junit.Test; + +import com.fasterxml.jackson.databind.JsonNode; + +public class StatisticTest { + + private static Statistic memoryTotal() { + return Statistic.hostGauge(Ref.of("/ovirt-engine/api/hosts/h1", "h1"), "memory.total", "Total memory", + Statistic.TYPE_INTEGER, Statistic.UNIT_BYTES, BigDecimal.valueOf(540171829248L)); + } + + @Test + public void idForName_MatchesOvirt() { + assertEquals("7816602b-c05c-3db7-a4da-3769f7ad8896", Statistic.idForName("memory.total")); + } + + @Test + public void hostGauge_SetsFields() { + Statistic statistic = memoryTotal(); + assertEquals("memory.total", statistic.getName()); + assertEquals("gauge", statistic.getKind()); + assertEquals("/ovirt-engine/api/hosts/h1/statistics/7816602b-c05c-3db7-a4da-3769f7ad8896", statistic.getHref()); + assertEquals("h1", statistic.getHost().getId()); + assertEquals(BigDecimal.valueOf(540171829248L), statistic.getValues().getItems().get(0).getDatum()); + } + + @Test + public void hostJson_FollowStatistics_MatchesOvirtShape() throws Exception { + Mapper mapper = new Mapper(); + Host host = new Host(); + host.setStatistics(NamedList.of("statistic", List.of(memoryTotal(), + Statistic.hostGauge(Ref.of("/ovirt-engine/api/hosts/h1", "h1"), "cpu.current.idle", "Idle CPU usage", + Statistic.TYPE_DECIMAL, Statistic.UNIT_PERCENT, new BigDecimal("96.51"))))); + + JsonNode statistics = mapper.jsonMapper().readTree(mapper.toJson(host)).path("statistics").path("statistic"); + + assertEquals(2, statistics.size()); + JsonNode total = statistics.get(0); + assertEquals("memory.total", total.path("name").asText()); + assertEquals("integer", total.path("type").asText()); + assertEquals("bytes", total.path("unit").asText()); + JsonNode totalDatum = total.path("values").path("value").get(0).path("datum"); + assertTrue(totalDatum.isNumber()); + assertEquals(540171829248L, totalDatum.asLong()); + assertEquals("h1", total.path("host").path("id").asText()); + + JsonNode idleDatum = statistics.get(1).path("values").path("value").get(0).path("datum"); + assertTrue(idleDatum.isNumber()); + assertEquals(96.51, idleDatum.asDouble(), 0.0001); + } + + @Test + public void xml_ContainsDatum() throws Exception { + Mapper mapper = new Mapper(); + String xml = mapper.toXml(NamedList.of("statistic", List.of(memoryTotal()))); + assertTrue(xml, xml.contains("memory.total")); + assertTrue(xml, xml.contains("540171829248")); + } +} From 682413d78c1b8995f0bb9ec81ab2aabb1d0a6a63 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Fri, 23 Oct 2026 12:16:21 +0530 Subject: [PATCH 5/9] imageserver: linear and streaming dirty-extents merge --- .../kvm/imageserver/backends/nbd.py | 17 ++- .../kvm/imageserver/tests/test_nbd_extents.py | 124 ++++++++++++++++++ .../kvm/imageserver/tests/test_util.py | 66 ++++++++++ scripts/vm/hypervisor/kvm/imageserver/util.py | 103 ++++++++++----- 4 files changed, 278 insertions(+), 32 deletions(-) create mode 100644 scripts/vm/hypervisor/kvm/imageserver/tests/test_nbd_extents.py diff --git a/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py b/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py index 002d777080e3..1c06157ebe12 100644 --- a/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py +++ b/scripts/vm/hypervisor/kvm/imageserver/backends/nbd.py @@ -28,7 +28,7 @@ NBD_STATE_HOLE, NBD_STATE_ZERO, ) -from ..util import coalesce_allocation_extents, merge_dirty_zero_extents +from ..util import append_coalesced, coalesce_allocation_extents, merge_dirty_zero_extents from .base import BackendSession, ImageBackend @@ -246,6 +246,10 @@ def get_extents_dirty_and_zero( """ Query block status for base:allocation and a dirty bitmap context, merge boundaries, and return extents with dirty and zero flags. + + Each block status step is merged as soon as it is received and only the + merged, coalesced result is kept, so the raw extents of a very + fragmented image are never all in memory at once. """ size = self.size() if size == 0: @@ -296,18 +300,27 @@ def extent_cb(*args: Any, **kwargs: Any) -> int: ) if block_status_fn is None: return self._fallback_dirty_zero_extents(size) + result: List[Dict[str, Any]] = [] try: while offset < size: count = min(chunk, size - offset) + allocation_extents.clear() + dirty_extents.clear() try: block_status_fn(count, offset, extent_cb) except TypeError: block_status_fn(offset, count, extent_cb) + append_coalesced( + result, + merge_dirty_zero_extents( + allocation_extents, dirty_extents, offset + count, start=offset + ), + ) offset += count except Exception as e: logging.warning("get_extents_dirty_and_zero block_status failed: %r", e) return self._fallback_dirty_zero_extents(size) - return merge_dirty_zero_extents(allocation_extents, dirty_extents, size) + return result @staticmethod def _fallback_dirty_zero_extents(size: int) -> List[Dict[str, Any]]: diff --git a/scripts/vm/hypervisor/kvm/imageserver/tests/test_nbd_extents.py b/scripts/vm/hypervisor/kvm/imageserver/tests/test_nbd_extents.py new file mode 100644 index 000000000000..11959696a892 --- /dev/null +++ b/scripts/vm/hypervisor/kvm/imageserver/tests/test_nbd_extents.py @@ -0,0 +1,124 @@ +# 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. +""" +Unit tests for building dirty/zero extents from NBD block status, using a fake +libnbd handle (no qemu-nbd needed). +""" + +import random +import unittest +from unittest import mock + +from imageserver.backends import nbd as nbd_backend +from imageserver.backends.nbd import NbdConnection +from imageserver.util import merge_dirty_zero_extents + +KiB = 1024 +CTX = "qemu:dirty-bitmap:ckp-1" + + +class FakeNbd: + """ + Minimal stand-in for an nbd.NBD handle. *layout* maps a context name to + (length, flags) runs that tile the image; replies are cut at the request. + """ + + def __init__(self, layout): + self.layout = layout + self.size = sum(length for length, _ in layout["base:allocation"]) + self.calls = 0 + + def get_size(self): + return self.size + + def can_meta_context(self, ctx): + return ctx in self.layout + + def block_status_64(self, count, offset, cb): + self.calls += 1 + end = offset + count + for ctx, runs in self.layout.items(): + entries = [] + pos = 0 + for length, flags in runs: + run_end = pos + length + if run_end > offset and pos < end: + entries.append((min(run_end, end) - max(pos, offset), flags)) + pos = run_end + cb(ctx, offset, entries, 0) + + +def make_conn(fake): + conn = NbdConnection.__new__(NbdConnection) + conn._nbd = fake + return conn + + +def whole_disk_merge(layout, size): + """The extents as one merge over the whole image computes them.""" + def tiles(ctx, bit): + out, pos = [], 0 + for length, flags in layout[ctx]: + out.append((pos, length, (flags & bit) != 0)) + pos += length + return out + return merge_dirty_zero_extents(tiles("base:allocation", 3), tiles(CTX, 1), size) + + +def random_runs(rng, size, unit, flag_values): + runs, pos = [], 0 + while pos < size: + length = min(rng.randint(1, 8) * unit, size - pos) + runs.append((length, rng.choice(flag_values))) + pos += length + return runs + + +class TestDirtyExtentsPerStep(unittest.TestCase): + def test_same_extents_as_one_merge_over_the_whole_image(self): + rng = random.Random(4321) + size = 1024 * 64 * KiB + layout = { + "base:allocation": random_runs(rng, size, 64 * KiB, [0, 3]), + CTX: random_runs(rng, size, 32 * KiB, [0, 1]), + } + fake = FakeNbd(layout) + with mock.patch.object(nbd_backend, "NBD_BLOCK_STATUS_CHUNK", 5 * 1024 * KiB): + extents = make_conn(fake).get_extents_dirty_and_zero(CTX) + + self.assertGreater(fake.calls, 10) + self.assertEqual(extents, whole_disk_merge(layout, size)) + + def test_extent_across_steps_is_joined(self): + layout = {"base:allocation": [(4 * 1024 * KiB, 0)], CTX: [(1024 * KiB, 0), (3 * 1024 * KiB, 1)]} + with mock.patch.object(nbd_backend, "NBD_BLOCK_STATUS_CHUNK", 1024 * KiB): + extents = make_conn(FakeNbd(layout)).get_extents_dirty_and_zero(CTX) + + self.assertEqual(extents, [ + {"start": 0, "length": 1024 * KiB, "dirty": False, "zero": False}, + {"start": 1024 * KiB, "length": 3 * 1024 * KiB, "dirty": True, "zero": False}, + ]) + + +class TestMergeWindow(unittest.TestCase): + def test_merge_covers_only_the_window(self): + extents = merge_dirty_zero_extents([(100, 50, False)], [(100, 50, True)], 150, start=100) + self.assertEqual(extents, [{"start": 100, "length": 50, "dirty": True, "zero": False}]) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/vm/hypervisor/kvm/imageserver/tests/test_util.py b/scripts/vm/hypervisor/kvm/imageserver/tests/test_util.py index 159dff30a929..18c9cb1f02b3 100644 --- a/scripts/vm/hypervisor/kvm/imageserver/tests/test_util.py +++ b/scripts/vm/hypervisor/kvm/imageserver/tests/test_util.py @@ -17,6 +17,7 @@ """Unit tests for imageserver.util extent coalescing helpers.""" +import time import unittest from imageserver.util import ( @@ -118,5 +119,70 @@ def test_coalesces_adjacent_identical_flags_after_boundary_merge(self): ) +class TestMergeDirtyZeroExtents(unittest.TestCase): + def test_unified_boundaries(self): + allocation = [(0, 100, False), (100, 100, True)] + dirty = [(0, 50, False), (50, 100, True), (150, 50, False)] + self.assertEqual( + merge_dirty_zero_extents(allocation, dirty, 200), + [ + {"start": 0, "length": 50, "dirty": False, "zero": False}, + {"start": 50, "length": 50, "dirty": True, "zero": False}, + {"start": 100, "length": 50, "dirty": True, "zero": True}, + {"start": 150, "length": 50, "dirty": False, "zero": True}, + ], + ) + + def test_range_without_dirty_extent_is_reported_dirty(self): + """A short dirty-bitmap reply must never hide changed data.""" + allocation = [(0, 128, False)] + dirty = [(0, 64, False)] + self.assertEqual( + merge_dirty_zero_extents(allocation, dirty, 128), + [ + {"start": 0, "length": 64, "dirty": False, "zero": False}, + {"start": 64, "length": 64, "dirty": True, "zero": False}, + ], + ) + + def test_range_without_allocation_extent_is_reported_as_data(self): + allocation = [(0, 64, True)] + dirty = [(0, 128, True)] + self.assertEqual( + merge_dirty_zero_extents(allocation, dirty, 128), + [ + {"start": 0, "length": 64, "dirty": True, "zero": True}, + {"start": 64, "length": 64, "dirty": True, "zero": False}, + ], + ) + + def test_unsorted_overlapping_and_overshooting_extents(self): + allocation = [(64, 100, True), (0, 80, False)] + dirty = [(0, 300, False)] + self.assertEqual( + merge_dirty_zero_extents(allocation, dirty, 128), + [ + {"start": 0, "length": 80, "dirty": False, "zero": False}, + {"start": 80, "length": 48, "dirty": False, "zero": True}, + ], + ) + + def test_unchanged_fully_allocated_disk(self): + self.assertEqual( + merge_dirty_zero_extents([(0, 4096, False)], [(0, 4096, False)], 4096), + [{"start": 0, "length": 4096, "dirty": False, "zero": False}], + ) + + def test_many_extents_are_merged_in_linear_time(self): + n = 200000 + allocation = [(i * 64, 64, i % 2 == 0) for i in range(n)] + dirty = [(i * 64, 64, i % 3 == 0) for i in range(n)] + start = time.monotonic() + merged = merge_dirty_zero_extents(allocation, dirty, n * 64) + elapsed = time.monotonic() - start + self.assertEqual(sum(e["length"] for e in merged), n * 64) + self.assertLess(elapsed, 10) + + if __name__ == "__main__": unittest.main() diff --git a/scripts/vm/hypervisor/kvm/imageserver/util.py b/scripts/vm/hypervisor/kvm/imageserver/util.py index 473f58a50c07..01eb14bceb10 100644 --- a/scripts/vm/hypervisor/kvm/imageserver/util.py +++ b/scripts/vm/hypervisor/kvm/imageserver/util.py @@ -17,7 +17,7 @@ import json import time -from typing import Any, Dict, List, Set, Tuple +from typing import Any, Dict, List, Tuple def coalesce_allocation_extents( @@ -70,45 +70,88 @@ def json_bytes(obj: Any) -> bytes: return json.dumps(obj, separators=(",", ":"), ensure_ascii=False).encode("utf-8") +def _normalize_extents( + extents: List[Tuple[int, int, bool]], size: int +) -> List[Tuple[int, int, bool]]: + """ + Sort (start, length, flag) extents, clip them to [0, size) and drop + overlaps (the earliest extent covering a byte wins). Returns + (start, end_exclusive, flag) tuples; gaps are left uncovered. + """ + out: List[Tuple[int, int, bool]] = [] + pos = 0 + for start, length, flag in sorted(extents, key=lambda e: e[0]): + begin = max(start, pos) + end = min(start + length, size) + if begin >= end: + continue + out.append((begin, end, flag)) + pos = end + return out + + +def append_coalesced( + result: List[Dict[str, Any]], extents: List[Dict[str, Any]] +) -> None: + """ + Append dirty/zero *extents* to *result*, extending the last extent of + *result* when the first new one continues it with the same flags. + """ + for e in extents: + if result: + prev = result[-1] + if ( + prev["start"] + prev["length"] == e["start"] + and prev["dirty"] == e["dirty"] + and prev["zero"] == e["zero"] + ): + prev["length"] += e["length"] + continue + result.append(e) + + def merge_dirty_zero_extents( allocation_extents: List[Tuple[int, int, bool]], dirty_extents: List[Tuple[int, int, bool]], size: int, + start: int = 0, ) -> List[Dict[str, Any]]: """ Merge allocation (start, length, zero) and dirty (start, length, dirty) extents - into a single list of {start, length, dirty, zero} with unified boundaries. + into a single list of {start, length, dirty, zero} with unified boundaries, + covering [start, size). + + Ranges not covered by any dirty extent are reported dirty, and ranges not + covered by any allocation extent are reported as data (zero=False), so a + missing or short block status reply can never hide changed data. + + Runs in O(n log n + m log m) (sorting) plus a single linear sweep. """ - boundaries: Set[int] = {0, size} - for start, length, _ in allocation_extents: - boundaries.add(start) - boundaries.add(start + length) - for start, length, _ in dirty_extents: - boundaries.add(start) - boundaries.add(start + length) - sorted_boundaries = sorted(boundaries) - - def lookup( - extents: List[Tuple[int, int, bool]], offset: int, default: bool - ) -> bool: - for start, length, flag in extents: - if start <= offset < start + length: - return flag - return default + alloc = _normalize_extents(allocation_extents, size) + dirty = _normalize_extents(dirty_extents, size) + + def flag_at( + intervals: List[Tuple[int, int, bool]], idx: int, pos: int, default: bool + ) -> Tuple[int, bool, int]: + """Return (new_idx, flag at pos, end of that flag run).""" + while idx < len(intervals) and intervals[idx][1] <= pos: + idx += 1 + if idx < len(intervals): + begin, end, flag = intervals[idx] + if begin <= pos: + return idx, flag, end + return idx, default, begin + return idx, default, size result: List[Dict[str, Any]] = [] - for i in range(len(sorted_boundaries) - 1): - a, b = sorted_boundaries[i], sorted_boundaries[i + 1] - if a >= b: - continue - result.append( - { - "start": a, - "length": b - a, - "dirty": lookup(dirty_extents, a, False), - "zero": lookup(allocation_extents, a, False), - } - ) + ai = di = 0 + pos = start + while pos < size: + ai, zero, alloc_end = flag_at(alloc, ai, pos, False) + di, is_dirty, dirty_end = flag_at(dirty, di, pos, True) + end = min(alloc_end, dirty_end) + result.append({"start": pos, "length": end - pos, "dirty": is_dirty, "zero": zero}) + pos = end return coalesce_dirty_zero_extents(result) From e82ab3106e4a61a29954ac7a14a7276d3cde2052 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Wed, 21 Oct 2026 16:38:22 +0530 Subject: [PATCH 6/9] kvm: keep backup scratch files on primary storage and check their space --- .../LibvirtStartBackupCommandWrapper.java | 94 +++++++++++- .../LibvirtStartBackupCommandWrapperTest.java | 138 +++++++++++++++++- 2 files changed, 226 insertions(+), 6 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartBackupCommandWrapper.java index 629fd179afb2..9835786296eb 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartBackupCommandWrapper.java @@ -20,6 +20,8 @@ import java.io.File; import java.io.FileWriter; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.HashMap; import java.util.Map; @@ -32,6 +34,7 @@ import org.json.JSONArray; import org.json.JSONObject; import org.libvirt.Domain; +import org.libvirt.DomainJobInfo; import org.libvirt.LibvirtException; import com.cloud.agent.api.Answer; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; @@ -44,6 +47,9 @@ public class LibvirtStartBackupCommandWrapper extends CommandWrapper { protected Logger logger = LogManager.getLogger(getClass()); + /** virDomainJobType VIR_DOMAIN_JOB_NONE. */ + static final int NO_JOB = 0; + @Override public Answer execute(StartBackupCommand cmd, LibvirtComputingResource resource) { if (cmd.isStoppedVM()) { @@ -72,8 +78,15 @@ public Answer handleRunningVmBackup(StartBackupCommand cmd, LibvirtComputingReso dir.mkdirs(); } + Map diskPathLabelMap = resource.getDiskPathLabelMap(vmName); + Map diskPathScratchFileMap = getScratchFiles(cmd.getDiskPathUuidMap(), diskPathLabelMap); + String scratchError = prepareScratchFiles(vmName, diskPathScratchFileMap, resource); + if (scratchError != null) { + return new StartBackupAnswer(cmd, false, scratchError); + } + // Create backup XML - String backupXml = createBackupXml(cmd, fromCheckpointId, socket, resource); + String backupXml = createBackupXml(cmd, fromCheckpointId, socket, diskPathLabelMap, diskPathScratchFileMap, resource); String checkpointXml = createCheckpointXml(toCheckpointId); // Write XMLs to temp files @@ -158,7 +171,8 @@ private String createCheckpointXmlForRedefine(String checkpointName, Long create return xml.toString(); } - private String createBackupXml(StartBackupCommand cmd, String fromCheckpointId, String socket, LibvirtComputingResource resource) throws LibvirtException { + private String createBackupXml(StartBackupCommand cmd, String fromCheckpointId, String socket, Map diskPathLabelMap, + Map diskPathScratchFileMap, LibvirtComputingResource resource) throws LibvirtException { StringBuilder xml = new StringBuilder(); xml.append("\n"); @@ -167,7 +181,6 @@ private String createBackupXml(StartBackupCommand cmd, String fromCheckpointId, xml.append(" \n"); Map diskPathUuidMap = cmd.getDiskPathUuidMap(); - Map diskPathLabelMap = resource.getDiskPathLabelMap(cmd.getVmName()); Map diskPathHasFromCheckpointMap = new HashMap<>(); if (StringUtils.isNotBlank(fromCheckpointId)) { Domain vm = null; @@ -193,7 +206,7 @@ private String createBackupXml(StartBackupCommand cmd, String fromCheckpointId, } String diskName = entry.getValue(); String export = diskPathUuidMap.get(diskPath); - String scratchFile = "/var/tmp/scratch-" + export + ".qcow2"; + String scratchFile = diskPathScratchFileMap.get(diskPath); xml.append(" getScratchFiles(Map diskPathUuidMap, Map diskPathLabelMap) { + Map scratchFiles = new HashMap<>(); + for (Map.Entry entry : diskPathUuidMap.entrySet()) { + if (!diskPathLabelMap.containsKey(entry.getKey())) { + continue; + } + String directory = new File(entry.getKey()).getParent(); + scratchFiles.put(entry.getKey(), new File(directory, "backup-scratch-" + entry.getValue() + ".qcow2").getPath()); + } + return scratchFiles; + } + + String prepareScratchFiles(String vmName, Map diskPathScratchFileMap, LibvirtComputingResource resource) { + Domain dm = null; + try { + dm = resource.getDomain(resource.getLibvirtUtilitiesHelper().getConnection(), vmName); + return prepareScratchFiles(dm, diskPathScratchFileMap); + } catch (LibvirtException e) { + return "Failed to prepare the backup scratch files: " + e.getMessage(); + } finally { + if (dm != null) { + try { + dm.free(); + } catch (LibvirtException e) { + logger.debug("Failed to free domain {}", vmName, e); + } + } + } + } + + String prepareScratchFiles(Domain dm, Map diskPathScratchFileMap) throws LibvirtException { + DomainJobInfo job = dm.getJobInfo(); + if (job != null && job.getType() != NO_JOB) { + // libvirt refuses a backup anyway; never touch the scratch files of a running one. + return "The VM has an active job, a backup cannot start"; + } + Map requiredSpace = new HashMap<>(); + for (Map.Entry entry : diskPathScratchFileMap.entrySet()) { + String scratchFile = entry.getValue(); + if (!deleteStaleScratchFile(scratchFile)) { + return "Failed to remove the stale backup scratch file " + scratchFile; + } + long capacity = dm.blockInfo(entry.getKey()).getCapacity(); + // 1% on top of the virtual size covers the qcow2 metadata of the scratch file. + requiredSpace.merge(new File(scratchFile).getParent(), capacity + capacity / 100, Long::sum); + } + for (Map.Entry entry : requiredSpace.entrySet()) { + long usable = getUsableSpace(entry.getKey()); + if (usable < entry.getValue()) { + return String.format("Not enough free space for the backup scratch files in %s: %d bytes needed, %d available", + entry.getKey(), entry.getValue(), usable); + } + } + return null; + } + + boolean deleteStaleScratchFile(String scratchFile) { + try { + if (Files.deleteIfExists(Paths.get(scratchFile))) { + logger.warn("Removed the stale backup scratch file {}", scratchFile); + } + return true; + } catch (IOException e) { + logger.error("Failed to remove the stale backup scratch file {}", scratchFile, e); + return false; + } + } + + long getUsableSpace(String directory) { + return new File(directory).getUsableSpace(); + } + private String createCheckpointXml(String checkpointId) { return "\n" + " " + checkpointId + "\n" + diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartBackupCommandWrapperTest.java index 7932e90f47a0..c9df5665448f 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartBackupCommandWrapperTest.java @@ -16,7 +16,14 @@ // under the License. package com.cloud.hypervisor.kvm.resource.wrapper; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Comparator; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.cloudstack.backup.StartBackupAnswer; import org.apache.cloudstack.backup.StartBackupCommand; @@ -24,6 +31,10 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.libvirt.Domain; +import org.libvirt.DomainBlockInfo; +import org.libvirt.DomainJobInfo; +import org.libvirt.LibvirtException; import org.mockito.MockedConstruction; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; @@ -41,7 +52,8 @@ public class LibvirtStartBackupCommandWrapperTest { @Before public void setUp() { - wrapper = new LibvirtStartBackupCommandWrapper(); + wrapper = Mockito.spy(new LibvirtStartBackupCommandWrapper()); + Mockito.lenient().doReturn(null).when(wrapper).prepareScratchFiles(Mockito.any(), Mockito.any(), Mockito.any(LibvirtComputingResource.class)); command = Mockito.mock(StartBackupCommand.class); resource = Mockito.mock(LibvirtComputingResource.class); } @@ -171,7 +183,6 @@ public void testExecuteRunningVmCreateBackupXmlExceptionReturnsFailure() { Mockito.when(command.getToCheckpointId()).thenReturn("cp-running-5"); Mockito.when(command.getFromCheckpointId()).thenReturn(null); Mockito.when(command.getSocket()).thenReturn("sock-5"); - Mockito.when(command.getDiskPathUuidMap()).thenReturn(Map.of("/path/disk1.qcow2", "vol-1")); Mockito.when(resource.getDiskPathLabelMap("i-2-3-VM")).thenThrow(new RuntimeException("disk labels unavailable")); Answer answer = wrapper.execute(command, resource); @@ -179,4 +190,127 @@ public void testExecuteRunningVmCreateBackupXmlExceptionReturnsFailure() { Assert.assertFalse(answer.getResult()); Assert.assertTrue(answer.getDetails().contains("Error starting backup: disk labels unavailable")); } + + @Test + public void testScratchFilesGoNextToTheDisk() { + Map scratchFiles = LibvirtStartBackupCommandWrapper.getScratchFiles( + Map.of("/mnt/pool-1/vol-path-1", "vol-uuid-1", "/var/lib/libvirt/images/vol-path-2", "vol-uuid-2"), + Map.of("/mnt/pool-1/vol-path-1", "vda", "/var/lib/libvirt/images/vol-path-2", "vdb")); + + Assert.assertEquals("/mnt/pool-1/backup-scratch-vol-uuid-1.qcow2", scratchFiles.get("/mnt/pool-1/vol-path-1")); + Assert.assertEquals("/var/lib/libvirt/images/backup-scratch-vol-uuid-2.qcow2", scratchFiles.get("/var/lib/libvirt/images/vol-path-2")); + } + + @Test + public void testScratchFilesOnlyForDisksInTheBackup() { + Map scratchFiles = LibvirtStartBackupCommandWrapper.getScratchFiles( + Map.of("/mnt/pool-1/vol-path-1", "vol-uuid-1", "/mnt/pool-1/vol-path-2", "vol-uuid-2"), + Map.of("/mnt/pool-1/vol-path-1", "vda")); + + Assert.assertEquals(List.of("/mnt/pool-1/vol-path-1"), List.copyOf(scratchFiles.keySet())); + } + + private static Domain domainWithDisks(int jobType, Map capacities) throws LibvirtException { + Domain dm = Mockito.mock(Domain.class); + DomainJobInfo job = Mockito.mock(DomainJobInfo.class); + Mockito.when(job.getType()).thenReturn(jobType); + Mockito.when(dm.getJobInfo()).thenReturn(job); + for (Map.Entry entry : capacities.entrySet()) { + DomainBlockInfo info = Mockito.mock(DomainBlockInfo.class); + Mockito.lenient().when(info.getCapacity()).thenReturn(entry.getValue()); + Mockito.lenient().when(dm.blockInfo(entry.getKey())).thenReturn(info); + } + return dm; + } + + private static Map scratchFilesFor(Map diskPathUuidMap) { + return LibvirtStartBackupCommandWrapper.getScratchFiles(diskPathUuidMap, + diskPathUuidMap.keySet().stream().collect(Collectors.toMap(path -> path, path -> "vd"))); + } + + @Test + public void testPrepareScratchFilesRemovesStaleFile() throws Exception { + Path pool = Files.createTempDirectory("pool"); + try { + String disk = pool.resolve("vol-path-1").toString(); + Map scratchFiles = scratchFilesFor(Map.of(disk, "vol-uuid-1")); + Path stale = Path.of(scratchFiles.get(disk)); + Files.writeString(stale, "left by a crashed backup"); + Mockito.doReturn(10L * 1024 * 1024 * 1024).when(wrapper).getUsableSpace(pool.toString()); + + String error = wrapper.prepareScratchFiles(domainWithDisks(LibvirtStartBackupCommandWrapper.NO_JOB, Map.of(disk, 1024L * 1024 * 1024)), scratchFiles); + + Assert.assertNull(error); + Assert.assertFalse(Files.exists(stale)); + } finally { + deleteRecursively(pool); + } + } + + @Test + public void testPrepareScratchFilesNeedsRoomForAllDisksInTheDirectory() throws Exception { + Path pool = Files.createTempDirectory("pool"); + try { + String disk1 = pool.resolve("vol-path-1").toString(); + String disk2 = pool.resolve("vol-path-2").toString(); + long tenGiB = 10L * 1024 * 1024 * 1024; + Map scratchFiles = scratchFilesFor(Map.of(disk1, "vol-uuid-1", disk2, "vol-uuid-2")); + // Room for one disk's worst case, not for both. + Mockito.doReturn(15L * 1024 * 1024 * 1024).when(wrapper).getUsableSpace(pool.toString()); + + String error = wrapper.prepareScratchFiles(domainWithDisks(LibvirtStartBackupCommandWrapper.NO_JOB, Map.of(disk1, tenGiB, disk2, tenGiB)), scratchFiles); + + Assert.assertNotNull(error); + Assert.assertTrue(error.contains("Not enough free space")); + Assert.assertTrue(error.contains(pool.toString())); + } finally { + deleteRecursively(pool); + } + } + + @Test + public void testPrepareScratchFilesLeavesFilesOfAnActiveJobAlone() throws Exception { + Path pool = Files.createTempDirectory("pool"); + try { + String disk = pool.resolve("vol-path-1").toString(); + Map scratchFiles = scratchFilesFor(Map.of(disk, "vol-uuid-1")); + Path inUse = Path.of(scratchFiles.get(disk)); + Files.writeString(inUse, "scratch of a running backup"); + int unboundedJob = 2; + + String error = wrapper.prepareScratchFiles(domainWithDisks(unboundedJob, Map.of(disk, 1024L)), scratchFiles); + + Assert.assertNotNull(error); + Assert.assertTrue(Files.exists(inUse)); + } finally { + deleteRecursively(pool); + } + } + + @Test + public void testExecuteRunningVmScratchFailureDoesNotBeginBackup() { + Mockito.when(command.isStoppedVM()).thenReturn(false); + Mockito.when(command.getVmName()).thenReturn("i-2-3-VM"); + Mockito.when(command.getFromCheckpointId()).thenReturn(null); + Mockito.when(command.getDiskPathUuidMap()).thenReturn(Map.of("/path/disk1.qcow2", "vol-1")); + Mockito.when(resource.getDiskPathLabelMap("i-2-3-VM")).thenReturn(Map.of("/path/disk1.qcow2", "vda")); + Mockito.doReturn("Not enough free space for the backup scratch files in /path").when(wrapper) + .prepareScratchFiles(Mockito.eq("i-2-3-VM"), Mockito.any(), Mockito.any(LibvirtComputingResource.class)); + + try (MockedConstruction