From 814c8b6a12137556ce082a13c8b2a31f021dfc8a Mon Sep 17 00:00:00 2001 From: GaOrtiga <49285692+GaOrtiga@users.noreply.github.com> Date: Fri, 28 Jun 2024 12:31:22 -0300 Subject: [PATCH 1/4] differentiate between instalation ISO and attached ISO (#9146) Co-authored-by: Gabriel --- ui/src/components/view/InfoCard.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/src/components/view/InfoCard.vue b/ui/src/components/view/InfoCard.vue index 53a1f7675eb..6a0329ada8f 100644 --- a/ui/src/components/view/InfoCard.vue +++ b/ui/src/components/view/InfoCard.vue @@ -521,7 +521,7 @@
-
{{ $t('label.templatename') }}
+
{{ resource.templateformat === 'ISO'? $t('label.iso') : $t('label.templatename') }}
@@ -529,7 +529,7 @@
-
{{ $t('label.iso') }}
+
{{ $t('label.isoname') }}
From 2ca1b474bd6ccaafb91c90cf91dc7d3e71519148 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Sat, 29 Jun 2024 10:01:50 +0530 Subject: [PATCH 2/4] PowerFlex/ScaleIO SDC client connection improvements (#9268) * Mitigation for non-scalable Powerflex/ScaleIO clients - Added ScaleIOSDCManager to manage SDC connections, checks clients limit, prepare and unprepare SDC on the hosts. - Added commands for prepare and unprepare storage clients to prepare/start and stop SDC service respectively on the hosts. - Introduced config 'storage.pool.connected.clients.limit' at storage level for client limits, currently support for Powerflex only. * tests issue fixed * refactor / improvements * lock with powerflex systemid while checking connections limit * updated powerflex systemid lock to hold till sdc preparation * Added custom stats support for storage pool, through listStoragePools API * code improvements, and unit tests * unit tests fixes * Update config 'storage.pool.connected.clients.limit' to dynamic, and some improvements * Stop SDC on host after migration if no volumes mapped to host * Wait for SDC to connect after scini service start, and some log improvements * Do not throw exception (log it) when SDC is not connected while revoking access for the powerflex volume * some log improvements --- .../apache/cloudstack/api/ApiConstants.java | 1 + .../admin/storage/ListStoragePoolsCmd.java | 7 +- .../api/response/StoragePoolResponse.java | 12 + .../agent/api/PrepareStorageClientAnswer.java | 43 +++ .../api/PrepareStorageClientCommand.java | 56 +++ .../api/UnprepareStorageClientAnswer.java | 34 ++ .../api/UnprepareStorageClientCommand.java | 48 +++ .../api/storage/PrimaryDataStoreDriver.java | 26 ++ .../com/cloud/storage/StorageManager.java | 21 +- ...irtPrepareStorageClientCommandWrapper.java | 52 +++ ...tUnprepareStorageClientCommandWrapper.java | 49 +++ .../kvm/storage/KVMStoragePoolManager.java | 11 + .../kvm/storage/ScaleIOStorageAdaptor.java | 64 ++++ .../kvm/storage/StorageAdaptor.java | 24 ++ ...repareStorageClientCommandWrapperTest.java | 87 +++++ ...repareStorageClientCommandWrapperTest.java | 73 ++++ .../storage/ScaleIOStorageAdaptorTest.java | 191 ++++++++++ .../client/ScaleIOGatewayClient.java | 2 + .../client/ScaleIOGatewayClientImpl.java | 26 ++ .../driver/ScaleIOPrimaryDataStoreDriver.java | 102 ++++-- .../ScaleIOPrimaryDataStoreLifeCycle.java | 30 +- .../datastore/manager/ScaleIOSDCManager.java | 47 +++ .../manager/ScaleIOSDCManagerImpl.java | 346 ++++++++++++++++++ .../provider/ScaleIOHostListener.java | 66 ++-- .../storage/datastore/util/ScaleIOUtil.java | 45 +++ .../spring-storage-volume-scaleio-context.xml | 2 + .../ScaleIOPrimaryDataStoreLifeCycleTest.java | 5 +- .../main/java/com/cloud/api/ApiDBUtils.java | 4 +- .../java/com/cloud/api/ApiResponseHelper.java | 2 +- .../com/cloud/api/query/QueryManagerImpl.java | 6 +- .../cloud/api/query/ViewResponseHelper.java | 4 +- .../api/query/dao/StoragePoolJoinDao.java | 2 +- .../api/query/dao/StoragePoolJoinDaoImpl.java | 11 +- .../deploy/DeploymentPlanningManagerImpl.java | 9 + .../com/cloud/storage/StorageManagerImpl.java | 41 +++ 35 files changed, 1431 insertions(+), 118 deletions(-) create mode 100644 core/src/main/java/com/cloud/agent/api/PrepareStorageClientAnswer.java create mode 100644 core/src/main/java/com/cloud/agent/api/PrepareStorageClientCommand.java create mode 100644 core/src/main/java/com/cloud/agent/api/UnprepareStorageClientAnswer.java create mode 100644 core/src/main/java/com/cloud/agent/api/UnprepareStorageClientCommand.java create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareStorageClientCommandWrapper.java create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnprepareStorageClientCommandWrapper.java create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareStorageClientCommandWrapperTest.java create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnprepareStorageClientCommandWrapperTest.java create mode 100644 plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManager.java create mode 100644 plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManagerImpl.java diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 050464a13a6..2324b861830 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -439,6 +439,7 @@ public class ApiConstants { public static final String STORAGE_POLICY = "storagepolicy"; public static final String STORAGE_MOTION_ENABLED = "storagemotionenabled"; public static final String STORAGE_CAPABILITIES = "storagecapabilities"; + public static final String STORAGE_CUSTOM_STATS = "storagecustomstats"; public static final String SUBNET = "subnet"; public static final String OWNER = "owner"; public static final String SWAP_OWNER = "swapowner"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java index 6923353b3bf..3da99de050b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java @@ -74,7 +74,8 @@ public class ListStoragePoolsCmd extends BaseListCmd { @Parameter(name = ApiConstants.HOST_ID, type = CommandType.UUID, entityType = HostResponse.class, description = "host ID of the storage pools") private Long hostId; - + @Parameter(name = ApiConstants.STORAGE_CUSTOM_STATS, type = CommandType.BOOLEAN, description = "If true, lists the custom stats of the storage pool", since = "4.18.1") + private Boolean customStats; ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -131,6 +132,10 @@ public class ListStoragePoolsCmd extends BaseListCmd { this.scope = scope; } + public Boolean getCustomStats() { + return customStats != null && customStats; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java index f514c8167ac..9e7f5159e0e 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/StoragePoolResponse.java @@ -97,6 +97,10 @@ public class StoragePoolResponse extends BaseResponseWithAnnotations { @Param(description = "total min IOPS currently in use by volumes") private Long allocatedIops; + @SerializedName(ApiConstants.STORAGE_CUSTOM_STATS) + @Param(description = "the storage pool custom stats", since = "4.18.1") + private Map customStats; + @SerializedName("tags") @Param(description = "the tags for the storage pool") private String tags; @@ -304,6 +308,14 @@ public class StoragePoolResponse extends BaseResponseWithAnnotations { this.allocatedIops = allocatedIops; } + public Map getCustomStats() { + return customStats; + } + + public void setCustomStats(Map customStats) { + this.customStats = customStats; + } + public String getTags() { return tags; } diff --git a/core/src/main/java/com/cloud/agent/api/PrepareStorageClientAnswer.java b/core/src/main/java/com/cloud/agent/api/PrepareStorageClientAnswer.java new file mode 100644 index 00000000000..85afb925646 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/PrepareStorageClientAnswer.java @@ -0,0 +1,43 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.agent.api; + +import java.util.Map; + +public class PrepareStorageClientAnswer extends Answer { + Map detailsMap; + + public PrepareStorageClientAnswer() { + super(); + } + + public PrepareStorageClientAnswer(Command command, boolean success, Map detailsMap) { + super(command, success, ""); + this.detailsMap = detailsMap; + } + + public PrepareStorageClientAnswer(Command command, boolean success, String details) { + super(command, success, details); + } + + public Map getDetailsMap() { + return detailsMap; + } +} diff --git a/core/src/main/java/com/cloud/agent/api/PrepareStorageClientCommand.java b/core/src/main/java/com/cloud/agent/api/PrepareStorageClientCommand.java new file mode 100644 index 00000000000..8dea9c11c53 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/PrepareStorageClientCommand.java @@ -0,0 +1,56 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.agent.api; + +import java.util.Map; + +import com.cloud.storage.Storage.StoragePoolType; + +public class PrepareStorageClientCommand extends Command { + private StoragePoolType poolType; + private String poolUuid; + private Map details; + + public PrepareStorageClientCommand() { + } + + public PrepareStorageClientCommand(StoragePoolType poolType, String poolUuid, Map details) { + this.poolType = poolType; + this.poolUuid = poolUuid; + this.details = details; + } + + @Override + public boolean executeInSequence() { + return false; + } + + public StoragePoolType getPoolType() { + return poolType; + } + + public String getPoolUuid() { + return poolUuid; + } + + public Map getDetails() { + return details; + } +} diff --git a/core/src/main/java/com/cloud/agent/api/UnprepareStorageClientAnswer.java b/core/src/main/java/com/cloud/agent/api/UnprepareStorageClientAnswer.java new file mode 100644 index 00000000000..1280293db0d --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/UnprepareStorageClientAnswer.java @@ -0,0 +1,34 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.agent.api; + +public class UnprepareStorageClientAnswer extends Answer { + public UnprepareStorageClientAnswer() { + super(); + } + + public UnprepareStorageClientAnswer(Command command, boolean success) { + super(command, success, ""); + } + + public UnprepareStorageClientAnswer(Command command, boolean success, String details) { + super(command, success, details); + } +} diff --git a/core/src/main/java/com/cloud/agent/api/UnprepareStorageClientCommand.java b/core/src/main/java/com/cloud/agent/api/UnprepareStorageClientCommand.java new file mode 100644 index 00000000000..bebd30ca519 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/UnprepareStorageClientCommand.java @@ -0,0 +1,48 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.agent.api; + +import com.cloud.storage.Storage.StoragePoolType; + +public class UnprepareStorageClientCommand extends Command { + private StoragePoolType poolType; + private String poolUuid; + + public UnprepareStorageClientCommand() { + } + + public UnprepareStorageClientCommand(StoragePoolType poolType, String poolUuid) { + this.poolType = poolType; + this.poolUuid = poolUuid; + } + + @Override + public boolean executeInSequence() { + return false; + } + + public StoragePoolType getPoolType() { + return poolType; + } + + public String getPoolUuid() { + return poolUuid; + } +} diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java index 2c7d3c60278..0e70c7b528d 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java @@ -18,6 +18,8 @@ */ package org.apache.cloudstack.engine.subsystem.api.storage; +import java.util.Map; + import org.apache.cloudstack.framework.async.AsyncCompletionCallback; import org.apache.cloudstack.storage.command.CommandResult; @@ -86,6 +88,22 @@ public interface PrimaryDataStoreDriver extends DataStoreDriver { */ boolean canProvideStorageStats(); + /** + * intended for managed storage + * returns true if the storage can provide its custom stats + */ + default boolean poolProvidesCustomStorageStats() { + return false; + } + + /** + * intended for managed storage + * returns the custom stats if the storage can provide them + */ + default Map getCustomStorageStats(StoragePool pool) { + return null; + } + /** * intended for managed storage * returns the total capacity and used size in bytes @@ -110,6 +128,14 @@ public interface PrimaryDataStoreDriver extends DataStoreDriver { */ boolean canHostAccessStoragePool(Host host, StoragePool pool); + /** + * intended for managed storage + * returns true if the host can prepare storage client to provide access the storage pool + */ + default boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool) { + return false; + } + /** * Used by storage pools which want to keep VMs' information * @return true if additional VM info is needed (intended for storage pools). diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index 5e97cc9edfe..86ef02bb9bc 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -118,7 +118,7 @@ public interface StorageManager extends StorageService { "storage.pool.disk.wait", "Storage", "60", - "Timeout (in secs) for the storage pool disk (of managed pool) to become available in the host. Currently only supported for PowerFlex.", + "Timeout (in secs) for the storage pool disk (of managed pool) to become available in the host. Currently supported for PowerFlex only.", true, ConfigKey.Scope.StoragePool, null); @@ -127,7 +127,7 @@ public interface StorageManager extends StorageService { "storage.pool.client.timeout", "Storage", "60", - "Timeout (in secs) for the storage pool client connection timeout (for managed pools). Currently only supported for PowerFlex.", + "Timeout (in secs) for the API client connection timeout of storage pool (for managed pools). Currently supported for PowerFlex only.", false, ConfigKey.Scope.StoragePool, null); @@ -136,11 +136,20 @@ public interface StorageManager extends StorageService { "storage.pool.client.max.connections", "Storage", "100", - "Maximum connections for the storage pool client (for managed pools). Currently only supported for PowerFlex.", + "Maximum connections for the API client of storage pool (for managed pools). Currently supported for PowerFlex only.", false, ConfigKey.Scope.StoragePool, null); + ConfigKey STORAGE_POOL_CONNECTED_CLIENTS_LIMIT = new ConfigKey<>(Integer.class, + "storage.pool.connected.clients.limit", + "Storage", + "-1", + "Maximum connected storage pool clients supported for the storage (for managed pools), <= 0 for unlimited (default: -1). Currently supported for PowerFlex only.", + true, + ConfigKey.Scope.StoragePool, + null); + ConfigKey STORAGE_POOL_IO_POLICY = new ConfigKey<>(String.class, "kvm.storage.pool.io.policy", "Storage", @@ -252,6 +261,10 @@ public interface StorageManager extends StorageService { boolean canPoolProvideStorageStats(StoragePool pool); + boolean poolProvidesCustomStorageStats(StoragePool pool); + + Map getCustomStorageStats(StoragePool pool); + /** * Checks if a host has running VMs that are using its local storage pool. * @return true if local storage is active on the host @@ -286,6 +299,8 @@ public interface StorageManager extends StorageService { boolean canHostAccessStoragePool(Host host, StoragePool pool); + boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool); + Host getHost(long hostId); Host updateSecondaryStorage(long secStorageId, String newUrl); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareStorageClientCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareStorageClientCommandWrapper.java new file mode 100644 index 00000000000..79afd4696b0 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareStorageClientCommandWrapper.java @@ -0,0 +1,52 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import java.util.Map; + +import org.apache.log4j.Logger; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.PrepareStorageClientAnswer; +import com.cloud.agent.api.PrepareStorageClientCommand; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.Ternary; + +@ResourceWrapper(handles = PrepareStorageClientCommand.class) +public class LibvirtPrepareStorageClientCommandWrapper extends CommandWrapper { + + private static final Logger s_logger = Logger.getLogger(LibvirtPrepareStorageClientCommandWrapper.class); + + @Override + public Answer execute(PrepareStorageClientCommand cmd, LibvirtComputingResource libvirtComputingResource) { + final KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr(); + Ternary, String> prepareStorageClientResult = storagePoolMgr.prepareStorageClient(cmd.getPoolType(), cmd.getPoolUuid(), cmd.getDetails()); + if (!prepareStorageClientResult.first()) { + String msg = prepareStorageClientResult.third(); + s_logger.debug("Unable to prepare storage client, due to: " + msg); + return new PrepareStorageClientAnswer(cmd, false, msg); + } + Map details = prepareStorageClientResult.second(); + return new PrepareStorageClientAnswer(cmd, true, details); + } +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnprepareStorageClientCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnprepareStorageClientCommandWrapper.java new file mode 100644 index 00000000000..f98782fe748 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnprepareStorageClientCommandWrapper.java @@ -0,0 +1,49 @@ +// +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import org.apache.log4j.Logger; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.UnprepareStorageClientAnswer; +import com.cloud.agent.api.UnprepareStorageClientCommand; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import com.cloud.utils.Pair; + +@ResourceWrapper(handles = UnprepareStorageClientCommand.class) +public class LibvirtUnprepareStorageClientCommandWrapper extends CommandWrapper { + + private static final Logger s_logger = Logger.getLogger(LibvirtUnprepareStorageClientCommandWrapper.class); + + @Override + public Answer execute(UnprepareStorageClientCommand cmd, LibvirtComputingResource libvirtComputingResource) { + final KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr(); + Pair unprepareStorageClientResult = storagePoolMgr.unprepareStorageClient(cmd.getPoolType(), cmd.getPoolUuid()); + if (!unprepareStorageClientResult.first()) { + String msg = unprepareStorageClientResult.second(); + s_logger.debug("Couldn't unprepare storage client, due to: " + msg); + return new UnprepareStorageClientAnswer(cmd, false, msg); + } + return new UnprepareStorageClientAnswer(cmd, true); + } +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java index b1842f38da2..4f25cfa08d5 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java @@ -42,6 +42,8 @@ import com.cloud.storage.Storage; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageLayer; import com.cloud.storage.Volume; +import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachine; @@ -447,4 +449,13 @@ public class KVMStoragePoolManager { return adaptor.createTemplateFromDirectDownloadFile(templateFilePath, destTemplatePath, destPool, format, timeout); } + public Ternary, String> prepareStorageClient(StoragePoolType type, String uuid, Map details) { + StorageAdaptor adaptor = getStorageAdaptor(type); + return adaptor.prepareStorageClient(type, uuid, details); + } + + public Pair unprepareStorageClient(StoragePoolType type, String uuid) { + StorageAdaptor adaptor = getStorageAdaptor(type); + return adaptor.unprepareStorageClient(type, uuid); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java index 7a98e3fb11f..60986f198a8 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import org.apache.cloudstack.storage.datastore.client.ScaleIOGatewayClient; import org.apache.cloudstack.storage.datastore.util.ScaleIOUtil; import org.apache.cloudstack.utils.cryptsetup.CryptSetup; import org.apache.cloudstack.utils.cryptsetup.CryptSetupException; @@ -43,6 +44,8 @@ import org.libvirt.LibvirtException; import com.cloud.storage.Storage; import com.cloud.storage.StorageLayer; import com.cloud.storage.StorageManager; +import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.OutputInterpreter; import com.cloud.utils.script.Script; @@ -561,6 +564,67 @@ public class ScaleIOStorageAdaptor implements StorageAdaptor { qemu.resize(options, objects, usableSizeBytes); } + public Ternary, String> prepareStorageClient(Storage.StoragePoolType type, String uuid, Map details) { + if (!ScaleIOUtil.isSDCServiceInstalled()) { + LOGGER.debug("SDC service not installed on host, preparing the SDC client not possible"); + return new Ternary<>(false, null, "SDC service not installed on host"); + } + + if (!ScaleIOUtil.isSDCServiceEnabled()) { + LOGGER.debug("SDC service not enabled on host, enabling it"); + if (!ScaleIOUtil.enableSDCService()) { + return new Ternary<>(false, null, "SDC service not enabled on host"); + } + } + + if (!ScaleIOUtil.isSDCServiceActive()) { + if (!ScaleIOUtil.startSDCService()) { + return new Ternary<>(false, null, "Couldn't start SDC service on host"); + } + } else if (!ScaleIOUtil.restartSDCService()) { + return new Ternary<>(false, null, "Couldn't restart SDC service on host"); + } + + return new Ternary<>( true, getSDCDetails(details), "Prepared client successfully"); + } + + public Pair unprepareStorageClient(Storage.StoragePoolType type, String uuid) { + if (!ScaleIOUtil.isSDCServiceInstalled()) { + LOGGER.debug("SDC service not installed on host, no need to unprepare the SDC client"); + return new Pair<>(true, "SDC service not installed on host, no need to unprepare the SDC client"); + } + + if (!ScaleIOUtil.isSDCServiceEnabled()) { + LOGGER.debug("SDC service not enabled on host, no need to unprepare the SDC client"); + return new Pair<>(true, "SDC service not enabled on host, no need to unprepare the SDC client"); + } + + if (!ScaleIOUtil.stopSDCService()) { + return new Pair<>(false, "Couldn't stop SDC service on host"); + } + + return new Pair<>(true, "Unprepared SDC client successfully"); + } + + private Map getSDCDetails(Map details) { + Map sdcDetails = new HashMap(); + if (details == null || !details.containsKey(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID)) { + return sdcDetails; + } + + String storageSystemId = details.get(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID); + String sdcId = ScaleIOUtil.getSdcId(storageSystemId); + if (sdcId != null) { + sdcDetails.put(ScaleIOGatewayClient.SDC_ID, sdcId); + } else { + String sdcGuId = ScaleIOUtil.getSdcGuid(); + if (sdcGuId != null) { + sdcDetails.put(ScaleIOGatewayClient.SDC_GUID, sdcGuId); + } + } + return sdcDetails; + } + /** * Calculates usable size from raw size, assuming qcow2 requires 192k/1GB for metadata * We also remove 128MiB for encryption/fragmentation/safety factor. diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java index 5cfdf6d1a8b..80e73e01a86 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.hypervisor.kvm.storage; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -23,6 +24,8 @@ import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; import com.cloud.storage.Storage; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; public interface StorageAdaptor { @@ -105,4 +108,25 @@ public interface StorageAdaptor { * @param timeout */ KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, String destTemplatePath, KVMStoragePool destPool, Storage.ImageFormat format, int timeout); + + /** + * Prepares the storage client. + * @param type type of the storage pool + * @param uuid uuid of the storage pool + * @param details any details of the storage pool that are required for client preparation + * @return status, client details, & message in case failed + */ + default Ternary, String> prepareStorageClient(StoragePoolType type, String uuid, Map details) { + return new Ternary<>(true, new HashMap<>(), ""); + } + + /** + * Unprepares the storage client. + * @param type type of the storage pool + * @param uuid uuid of the storage pool + * @return status, & message in case failed + */ + default Pair unprepareStorageClient(StoragePoolType type, String uuid) { + return new Pair<>(true, ""); + } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareStorageClientCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareStorageClientCommandWrapperTest.java new file mode 100644 index 00000000000..e7dffeece71 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareStorageClientCommandWrapperTest.java @@ -0,0 +1,87 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.cloudstack.storage.datastore.client.ScaleIOGatewayClient; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.agent.api.PrepareStorageClientAnswer; +import com.cloud.agent.api.PrepareStorageClientCommand; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.storage.Storage; +import com.cloud.utils.Ternary; + +@RunWith(MockitoJUnitRunner.class) +public class LibvirtPrepareStorageClientCommandWrapperTest { + + @Spy + LibvirtPrepareStorageClientCommandWrapper libvirtPrepareStorageClientCommandWrapperSpy = Mockito.spy(LibvirtPrepareStorageClientCommandWrapper.class); + + @Mock + LibvirtComputingResource libvirtComputingResourceMock; + + private final static String poolUuid = "345fc603-2d7e-47d2-b719-a0110b3732e6"; + private final static String systemId = "218ce1797566a00f"; + private final static String sdcId = "301b852c00000003"; + + @Test + public void testPrepareStorageClientSuccess() { + Map details = new HashMap<>(); + details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId); + PrepareStorageClientCommand cmd = Mockito.mock(PrepareStorageClientCommand.class); + Mockito.when(cmd.getPoolType()).thenReturn(Storage.StoragePoolType.PowerFlex); + Mockito.when(cmd.getPoolUuid()).thenReturn(poolUuid); + Mockito.when(cmd.getDetails()).thenReturn(details); + + KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + Mockito.when(libvirtComputingResourceMock.getStoragePoolMgr()).thenReturn(storagePoolMgr); + details.put(ScaleIOGatewayClient.SDC_ID, sdcId); + Mockito.when(storagePoolMgr.prepareStorageClient(cmd.getPoolType(), cmd.getPoolUuid(), cmd.getDetails())).thenReturn(new Ternary<>(true, details, "")); + + PrepareStorageClientAnswer result = (PrepareStorageClientAnswer) libvirtPrepareStorageClientCommandWrapperSpy.execute(cmd, libvirtComputingResourceMock); + + Assert.assertTrue(result.getResult()); + Assert.assertEquals(sdcId, result.getDetailsMap().get(ScaleIOGatewayClient.SDC_ID)); + } + + @Test + public void testPrepareStorageClientFailure() { + Map details = new HashMap<>(); + details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId); + PrepareStorageClientCommand cmd = Mockito.mock(PrepareStorageClientCommand.class); + Mockito.when(cmd.getPoolType()).thenReturn(Storage.StoragePoolType.PowerFlex); + Mockito.when(cmd.getPoolUuid()).thenReturn(poolUuid); + Mockito.when(cmd.getDetails()).thenReturn(details); + + KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + Mockito.when(libvirtComputingResourceMock.getStoragePoolMgr()).thenReturn(storagePoolMgr); + Mockito.when(storagePoolMgr.prepareStorageClient(cmd.getPoolType(), cmd.getPoolUuid(), cmd.getDetails())).thenReturn(new Ternary<>(false, new HashMap<>() , "Prepare storage client failed")); + + PrepareStorageClientAnswer result = (PrepareStorageClientAnswer) libvirtPrepareStorageClientCommandWrapperSpy.execute(cmd, libvirtComputingResourceMock); + + Assert.assertFalse(result.getResult()); + Assert.assertEquals("Prepare storage client failed", result.getDetails()); + } +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnprepareStorageClientCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnprepareStorageClientCommandWrapperTest.java new file mode 100644 index 00000000000..7409b286f32 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnprepareStorageClientCommandWrapperTest.java @@ -0,0 +1,73 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.cloud.hypervisor.kvm.resource.wrapper; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.agent.api.UnprepareStorageClientAnswer; +import com.cloud.agent.api.UnprepareStorageClientCommand; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.storage.Storage; +import com.cloud.utils.Pair; + +@RunWith(MockitoJUnitRunner.class) +public class LibvirtUnprepareStorageClientCommandWrapperTest { + + @Spy + LibvirtUnprepareStorageClientCommandWrapper libvirtUnprepareStorageClientCommandWrapperSpy = Mockito.spy(LibvirtUnprepareStorageClientCommandWrapper.class); + + @Mock + LibvirtComputingResource libvirtComputingResourceMock; + + private final static String poolUuid = "345fc603-2d7e-47d2-b719-a0110b3732e6"; + + @Test + public void testUnprepareStorageClientSuccess() { + UnprepareStorageClientCommand cmd = Mockito.mock(UnprepareStorageClientCommand.class); + Mockito.when(cmd.getPoolType()).thenReturn(Storage.StoragePoolType.PowerFlex); + Mockito.when(cmd.getPoolUuid()).thenReturn(poolUuid); + + KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + Mockito.when(libvirtComputingResourceMock.getStoragePoolMgr()).thenReturn(storagePoolMgr); + Mockito.when(storagePoolMgr.unprepareStorageClient(cmd.getPoolType(), cmd.getPoolUuid())).thenReturn(new Pair<>(true, "")); + + UnprepareStorageClientAnswer result = (UnprepareStorageClientAnswer) libvirtUnprepareStorageClientCommandWrapperSpy.execute(cmd, libvirtComputingResourceMock); + + Assert.assertTrue(result.getResult()); + } + + @Test + public void testUnprepareStorageClientFailure() { + UnprepareStorageClientCommand cmd = Mockito.mock(UnprepareStorageClientCommand.class); + Mockito.when(cmd.getPoolType()).thenReturn(Storage.StoragePoolType.PowerFlex); + Mockito.when(cmd.getPoolUuid()).thenReturn(poolUuid); + + KVMStoragePoolManager storagePoolMgr = Mockito.mock(KVMStoragePoolManager.class); + Mockito.when(libvirtComputingResourceMock.getStoragePoolMgr()).thenReturn(storagePoolMgr); + Mockito.when(storagePoolMgr.unprepareStorageClient(cmd.getPoolType(), cmd.getPoolUuid())).thenReturn(new Pair<>(false, "Unprepare storage client failed")); + + UnprepareStorageClientAnswer result = (UnprepareStorageClientAnswer) libvirtUnprepareStorageClientCommandWrapperSpy.execute(cmd, libvirtComputingResourceMock); + + Assert.assertFalse(result.getResult()); + Assert.assertEquals("Unprepare storage client failed", result.getDetails()); + } +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptorTest.java index 25fab1a6ff8..7db4f114e8c 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptorTest.java @@ -17,13 +17,50 @@ package com.cloud.hypervisor.kvm.storage; +import static org.mockito.Mockito.when; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.cloudstack.storage.datastore.client.ScaleIOGatewayClient; +import org.apache.cloudstack.storage.datastore.util.ScaleIOUtil; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import com.cloud.storage.Storage; +import com.cloud.storage.StorageLayer; +import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; +import com.cloud.utils.script.Script; + @RunWith(MockitoJUnitRunner.class) public class ScaleIOStorageAdaptorTest { + + @Mock + StorageLayer storageLayer; + ScaleIOStorageAdaptor scaleIOStorageAdaptor; + + private final static String poolUuid = "345fc603-2d7e-47d2-b719-a0110b3732e6"; + private static MockedStatic + + From d79735606bbe5e80ee1642bdb234028f9e7b5dcd Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Sat, 29 Jun 2024 01:58:01 -0300 Subject: [PATCH 4/4] Handle public IP race conditions (#9234) * Lock public IP * Release IP if ID is not null * Fix NPEs Co-authored-by: Henrique Sato --- .../cloud/network/IpAddressManagerImpl.java | 77 ++--- .../com/cloud/network/NetworkModelImpl.java | 4 + .../network/firewall/FirewallManagerImpl.java | 95 +++--- .../lb/LoadBalancingRulesManagerImpl.java | 94 +++--- .../cloud/network/rules/RulesManagerImpl.java | 270 +++++++++--------- .../vpn/RemoteAccessVpnManagerImpl.java | 130 +++++---- 6 files changed, 340 insertions(+), 330 deletions(-) diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index 0178236a21a..c8ac0c1016b 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -725,50 +725,59 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage @Override @DB public boolean disassociatePublicIpAddress(long addrId, long userId, Account caller) { - boolean success = true; - IPAddressVO ipToBeDisassociated = _ipAddressDao.findById(addrId); - PublicIpQuarantine publicIpQuarantine = null; - // Cleanup all ip address resources - PF/LB/Static nat rules - if (!cleanupIpResources(addrId, userId, caller)) { - success = false; - s_logger.warn("Failed to release resources for ip address id=" + addrId); - } + try { + IPAddressVO ipToBeDisassociated = _ipAddressDao.acquireInLockTable(addrId); - IPAddressVO ip = markIpAsUnavailable(addrId); - if (ip == null) { - return true; - } + if (ipToBeDisassociated == null) { + s_logger.error(String.format("Unable to acquire lock on public IP %s.", addrId)); + throw new CloudRuntimeException("Unable to acquire lock on public IP."); + } - if (s_logger.isDebugEnabled()) { - s_logger.debug("Releasing ip id=" + addrId + "; sourceNat = " + ip.isSourceNat()); - } + PublicIpQuarantine publicIpQuarantine = null; + // Cleanup all ip address resources - PF/LB/Static nat rules + if (!cleanupIpResources(addrId, userId, caller)) { + success = false; + s_logger.warn("Failed to release resources for ip address id=" + addrId); + } - if (ip.getAssociatedWithNetworkId() != null) { - Network network = _networksDao.findById(ip.getAssociatedWithNetworkId()); - try { - if (!applyIpAssociations(network, rulesContinueOnErrFlag)) { - s_logger.warn("Unable to apply ip address associations for " + network); - success = false; + IPAddressVO ip = markIpAsUnavailable(addrId); + if (ip == null) { + return true; + } + + if (s_logger.isDebugEnabled()) { + s_logger.debug("Releasing ip id=" + addrId + "; sourceNat = " + ip.isSourceNat()); + } + + if (ip.getAssociatedWithNetworkId() != null) { + Network network = _networksDao.findById(ip.getAssociatedWithNetworkId()); + try { + if (!applyIpAssociations(network, rulesContinueOnErrFlag)) { + s_logger.warn("Unable to apply ip address associations for " + network); + success = false; + } + } catch (ResourceUnavailableException e) { + throw new CloudRuntimeException("We should never get to here because we used true when applyIpAssociations", e); } - } catch (ResourceUnavailableException e) { - throw new CloudRuntimeException("We should never get to here because we used true when applyIpAssociations", e); + } else if (ip.getState() == State.Releasing) { + publicIpQuarantine = addPublicIpAddressToQuarantine(ipToBeDisassociated, caller.getDomainId()); + _ipAddressDao.unassignIpAddress(ip.getId()); } - } else if (ip.getState() == State.Releasing) { - publicIpQuarantine = addPublicIpAddressToQuarantine(ipToBeDisassociated, caller.getDomainId()); - _ipAddressDao.unassignIpAddress(ip.getId()); - } - annotationDao.removeByEntityType(AnnotationService.EntityType.PUBLIC_IP_ADDRESS.name(), ip.getUuid()); + annotationDao.removeByEntityType(AnnotationService.EntityType.PUBLIC_IP_ADDRESS.name(), ip.getUuid()); - if (success) { - if (ip.isPortable()) { - releasePortableIpAddress(addrId); + if (success) { + if (ip.isPortable()) { + releasePortableIpAddress(addrId); + } + s_logger.debug("Released a public ip id=" + addrId); + } else if (publicIpQuarantine != null) { + removePublicIpAddressFromQuarantine(publicIpQuarantine.getId(), "Public IP address removed from quarantine as there was an error while disassociating it."); } - s_logger.debug("Released a public ip id=" + addrId); - } else if (publicIpQuarantine != null) { - removePublicIpAddressFromQuarantine(publicIpQuarantine.getId(), "Public IP address removed from quarantine as there was an error while disassociating it."); + } finally { + _ipAddressDao.releaseFromLockTable(addrId); } return success; diff --git a/server/src/main/java/com/cloud/network/NetworkModelImpl.java b/server/src/main/java/com/cloud/network/NetworkModelImpl.java index 2a604796d6e..4088e9539ea 100644 --- a/server/src/main/java/com/cloud/network/NetworkModelImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkModelImpl.java @@ -1612,6 +1612,10 @@ public class NetworkModelImpl extends ManagerBase implements NetworkModel, Confi } NetworkVO network = _networksDao.findById(networkId); + if (network == null) { + throw new CloudRuntimeException("Could not find network associated with public IP."); + } + NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); if (offering.getGuestType() != GuestType.Isolated) { return true; diff --git a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java index b08df5a3d1b..4ed480ea68b 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -194,57 +194,54 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, return createFirewallRule(sourceIpAddressId, caller, rule.getXid(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), rule.getSourceCidrList(), null, rule.getIcmpCode(), rule.getIcmpType(), null, rule.getType(), rule.getNetworkId(), rule.getTrafficType(), rule.isDisplay()); } + //Destination CIDR capability is currently implemented for egress rules only. For others, the field is passed as null. @DB - protected FirewallRule createFirewallRule(final Long ipAddrId, Account caller, final String xId, final Integer portStart, final Integer portEnd, - final String protocol, final List sourceCidrList, final List destCidrList, final Integer icmpCode, final Integer icmpType, final Long relatedRuleId, - final FirewallRule.FirewallRuleType type, - final Long networkId, final FirewallRule.TrafficType trafficType, final Boolean forDisplay) throws NetworkRuleConflictException { - + protected FirewallRule createFirewallRule(final Long ipAddrId, Account caller, final String xId, final Integer portStart, final Integer portEnd, final String protocol, + final List sourceCidrList, final List destCidrList, final Integer icmpCode, final Integer icmpType, final Long relatedRuleId, + final FirewallRule.FirewallRuleType type, final Long networkId, final FirewallRule.TrafficType trafficType, final Boolean forDisplay) throws NetworkRuleConflictException { IPAddressVO ipAddress = null; - if (ipAddrId != null) { - // this for ingress firewall rule, for egress id is null - ipAddress = _ipAddressDao.findById(ipAddrId); - // Validate ip address - if (ipAddress == null && type == FirewallRule.FirewallRuleType.User) { - throw new InvalidParameterValueException("Unable to create firewall rule; " + "couldn't locate IP address by id in the system"); - } - _networkModel.checkIpForService(ipAddress, Service.Firewall, null); - } + try { + // Validate ip address + if (ipAddrId != null) { + // this for ingress firewall rule, for egress id is null + ipAddress = _ipAddressDao.acquireInLockTable(ipAddrId); + if (ipAddress == null) { + throw new InvalidParameterValueException("Unable to create firewall rule; " + "couldn't locate IP address by id in the system"); + } + _networkModel.checkIpForService(ipAddress, Service.Firewall, null); + } - validateFirewallRule(caller, ipAddress, portStart, portEnd, protocol, Purpose.Firewall, type, networkId, trafficType); + validateFirewallRule(caller, ipAddress, portStart, portEnd, protocol, Purpose.Firewall, type, networkId, trafficType); - // icmp code and icmp type can't be passed in for any other protocol rather than icmp - if (!protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (icmpCode != null || icmpType != null)) { - throw new InvalidParameterValueException("Can specify icmpCode and icmpType for ICMP protocol only"); - } + // icmp code and icmp type can't be passed in for any other protocol rather than icmp + if (!protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (icmpCode != null || icmpType != null)) { + throw new InvalidParameterValueException("Can specify icmpCode and icmpType for ICMP protocol only"); + } - if (protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (portStart != null || portEnd != null)) { - throw new InvalidParameterValueException("Can't specify start/end port when protocol is ICMP"); - } + if (protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (portStart != null || portEnd != null)) { + throw new InvalidParameterValueException("Can't specify start/end port when protocol is ICMP"); + } - Long accountId = null; - Long domainId = null; + Long accountId = null; + Long domainId = null; - if (ipAddress != null) { - //Ingress firewall rule - accountId = ipAddress.getAllocatedToAccountId(); - domainId = ipAddress.getAllocatedInDomainId(); - } else if (networkId != null) { - //egress firewall rule + if (ipAddress != null) { + //Ingress firewall rule + accountId = ipAddress.getAllocatedToAccountId(); + domainId = ipAddress.getAllocatedInDomainId(); + } else if (networkId != null) { + //egress firewall rule Network network = _networkModel.getNetwork(networkId); accountId = network.getAccountId(); domainId = network.getDomainId(); - } + } - final Long accountIdFinal = accountId; - final Long domainIdFinal = domainId; - return Transaction.execute(new TransactionCallbackWithException() { - @Override - public FirewallRuleVO doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { - FirewallRuleVO newRule = - new FirewallRuleVO(xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountIdFinal, domainIdFinal, Purpose.Firewall, - sourceCidrList, destCidrList, icmpCode, icmpType, relatedRuleId, trafficType); + final Long accountIdFinal = accountId; + final Long domainIdFinal = domainId; + return Transaction.execute((TransactionCallbackWithException) status -> { + FirewallRuleVO newRule = new FirewallRuleVO(xId, ipAddrId, portStart, portEnd, protocol.toLowerCase(), networkId, accountIdFinal, domainIdFinal, Purpose.Firewall, + sourceCidrList, destCidrList, icmpCode, icmpType, relatedRuleId, trafficType); newRule.setType(type); if (forDisplay != null) { newRule.setDisplay(forDisplay); @@ -261,8 +258,12 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, CallContext.current().putContextParameter(FirewallRule.class, newRule.getId()); return newRule; + }); + } finally { + if (ipAddrId != null) { + _ipAddressDao.releaseFromLockTable(ipAddrId); } - }); + } } @Override @@ -668,9 +669,19 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, } @Override + @DB public boolean applyIngressFirewallRules(long ipId, Account caller) throws ResourceUnavailableException { - List rules = _firewallDao.listByIpAndPurpose(ipId, Purpose.Firewall); - return applyFirewallRules(rules, false, caller); + try { + IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(ipId); + if (ipAddress == null) { + s_logger.error(String.format("Unable to acquire lock for public IP [%s].", ipId)); + throw new CloudRuntimeException("Unable to acquire lock for public IP."); + } + List rules = _firewallDao.listByIpAndPurpose(ipId, Purpose.Firewall); + return applyFirewallRules(rules, false, caller); + } finally { + _ipAddressDao.releaseFromLockTable(ipId); + } } @Override diff --git a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java index 8cb8972e295..e0d8082c0d9 100644 --- a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java +++ b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java @@ -1814,13 +1814,12 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements } return cidr; } + @DB @Override - public LoadBalancer createPublicLoadBalancer(final String xId, final String name, final String description, final int srcPort, final int destPort, - final long sourceIpId, - final String protocol, final String algorithm, final boolean openFirewall, final CallContext caller, final String lbProtocol, final Boolean forDisplay, String cidrList) - throws NetworkRuleConflictException { - + public LoadBalancer createPublicLoadBalancer(final String xId, final String name, final String description, final int srcPort, final int destPort, final long sourceIpId, + final String protocol, final String algorithm, final boolean openFirewall, final CallContext caller, final String lbProtocol, + final Boolean forDisplay, String cidrList) throws NetworkRuleConflictException { if (!NetUtils.isValidPort(destPort)) { throw new InvalidParameterValueException("privatePort is an invalid value: " + destPort); } @@ -1829,55 +1828,41 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements throw new InvalidParameterValueException("Invalid algorithm: " + algorithm); } - final IPAddressVO ipAddr = _ipAddressDao.findById(sourceIpId); - // make sure ip address exists - if (ipAddr == null || !ipAddr.readyToUse()) { - InvalidParameterValueException ex = new InvalidParameterValueException("Unable to create load balancer rule, invalid IP address id specified"); - if (ipAddr == null) { - ex.addProxyObject(String.valueOf(sourceIpId), "sourceIpId"); - } else { + try { + final IPAddressVO ipAddr = _ipAddressDao.acquireInLockTable(sourceIpId); + + // make sure ip address exists + if (ipAddr == null || !ipAddr.readyToUse()) { + InvalidParameterValueException ex = new InvalidParameterValueException("Unable to create load balancer rule, invalid IP address id specified"); + if (ipAddr == null) { + ex.addProxyObject(String.valueOf(sourceIpId), "sourceIpId"); + } else { + ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); + } + throw ex; + } else if (ipAddr.isOneToOneNat()) { + InvalidParameterValueException ex = new InvalidParameterValueException("Unable to create load balancer rule; specified sourceip id has static nat enabled"); ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); + throw ex; } - throw ex; - } else if (ipAddr.isOneToOneNat()) { - InvalidParameterValueException ex = new InvalidParameterValueException("Unable to create load balancer rule; specified sourceip id has static nat enabled"); - ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); - throw ex; - } - _accountMgr.checkAccess(caller.getCallingAccount(), null, true, ipAddr); + _accountMgr.checkAccess(caller.getCallingAccount(), null, true, ipAddr); - final Long networkId = ipAddr.getAssociatedWithNetworkId(); - if (networkId == null) { - InvalidParameterValueException ex = - new InvalidParameterValueException("Unable to create load balancer rule ; specified sourceip id is not associated with any network"); - ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); - throw ex; - } + final Long networkId = ipAddr.getAssociatedWithNetworkId(); + if (networkId == null) { + InvalidParameterValueException ex = + new InvalidParameterValueException("Unable to create load balancer rule ; specified sourceip id is not associated with any network"); + ex.addProxyObject(ipAddr.getUuid(), "sourceIpId"); + throw ex; + } - // verify that lb service is supported by the network - isLbServiceSupportedInNetwork(networkId, Scheme.Public); + // verify that lb service is supported by the network + isLbServiceSupportedInNetwork(networkId, Scheme.Public); - _firewallMgr.validateFirewallRule(caller.getCallingAccount(), ipAddr, srcPort, srcPort, protocol, Purpose.LoadBalancing, FirewallRuleType.User, networkId, null); + _firewallMgr.validateFirewallRule(caller.getCallingAccount(), ipAddr, srcPort, srcPort, protocol, Purpose.LoadBalancing, FirewallRuleType.User, networkId, null); - LoadBalancerVO newRule = - new LoadBalancerVO(xId, name, description, sourceIpId, srcPort, destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(), - ipAddr.getAllocatedInDomainId(), lbProtocol, cidrList); - - // verify rule is supported by Lb provider of the network - Ip sourceIp = getSourceIp(newRule); - LoadBalancingRule loadBalancing = - new LoadBalancingRule(newRule, new ArrayList(), new ArrayList(), new ArrayList(), sourceIp, null, - lbProtocol); - if (!validateLbRule(loadBalancing)) { - throw new InvalidParameterValueException("LB service provider cannot support this rule"); - } - - return Transaction.execute(new TransactionCallbackWithException() { - @Override - public LoadBalancerVO doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { - LoadBalancerVO newRule = - new LoadBalancerVO(xId, name, description, sourceIpId, srcPort, destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(), + return Transaction.execute((TransactionCallbackWithException) status -> { + LoadBalancerVO newRule = new LoadBalancerVO(xId, name, description, sourceIpId, srcPort, destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(), ipAddr.getAllocatedInDomainId(), lbProtocol, cidrList); if (forDisplay != null) { @@ -1886,9 +1871,7 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements // verify rule is supported by Lb provider of the network Ip sourceIp = getSourceIp(newRule); - LoadBalancingRule loadBalancing = - new LoadBalancingRule(newRule, new ArrayList(), new ArrayList(), new ArrayList(), sourceIp, - null, lbProtocol); + LoadBalancingRule loadBalancing = new LoadBalancingRule(newRule, new ArrayList<>(), new ArrayList<>(), new ArrayList<>(), sourceIp, null, lbProtocol); if (!validateLbRule(loadBalancing)) { throw new InvalidParameterValueException("LB service provider cannot support this rule"); } @@ -1908,10 +1891,10 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements throw new CloudRuntimeException("Unable to update the state to add for " + newRule); } s_logger.debug("Load balancer " + newRule.getId() + " for Ip address id=" + sourceIpId + ", public port " + srcPort + ", private port " + destPort + - " is added successfully."); + " is added successfully."); CallContext.current().setEventDetails("Load balancer Id: " + newRule.getId()); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_LOAD_BALANCER_CREATE, ipAddr.getAllocatedToAccountId(), ipAddr.getDataCenterId(), newRule.getId(), - null, LoadBalancingRule.class.getName(), newRule.getUuid()); + null, LoadBalancingRule.class.getName(), newRule.getUuid()); return newRule; } catch (Exception e) { @@ -1926,9 +1909,10 @@ public class LoadBalancingRulesManagerImpl extends ManagerBase implements removeLBRule(newRule); } } - } - }); - + }); + } finally { + _ipAddressDao.releaseFromLockTable(sourceIpId); + } } @Override diff --git a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java index 624fbfb9d24..7686a8d7887 100644 --- a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java @@ -207,124 +207,122 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules final Long ipAddrId = rule.getSourceIpAddressId(); - IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId); - - // Validate ip address - if (ipAddress == null) { - throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " doesn't exist in the system"); - } else if (ipAddress.isOneToOneNat()) { - throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " has static nat enabled"); - } - - final Long networkId = rule.getNetworkId(); - Network network = _networkModel.getNetwork(networkId); - //associate ip address to network (if needed) - boolean performedIpAssoc = false; - Nic guestNic; - if (ipAddress.getAssociatedWithNetworkId() == null) { - boolean assignToVpcNtwk = network.getVpcId() != null && ipAddress.getVpcId() != null && ipAddress.getVpcId().longValue() == network.getVpcId(); - if (assignToVpcNtwk) { - _networkModel.checkIpForService(ipAddress, Service.PortForwarding, networkId); - - s_logger.debug("The ip is not associated with the VPC network id=" + networkId + ", so assigning"); - try { - ipAddress = _ipAddrMgr.associateIPToGuestNetwork(ipAddrId, networkId, false); - performedIpAssoc = true; - } catch (Exception ex) { - throw new CloudRuntimeException("Failed to associate ip to VPC network as " + "a part of port forwarding rule creation"); - } - } - } else { - _networkModel.checkIpForService(ipAddress, Service.PortForwarding, null); - } - - if (ipAddress.getAssociatedWithNetworkId() == null) { - throw new InvalidParameterValueException("Ip address " + ipAddress + " is not assigned to the network " + network); - } - try { - _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.PortForwarding, - FirewallRuleType.User, networkId, rule.getTrafficType()); + IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(ipAddrId); - final Long accountId = ipAddress.getAllocatedToAccountId(); - final Long domainId = ipAddress.getAllocatedInDomainId(); - - // start port can't be bigger than end port - if (rule.getDestinationPortStart() > rule.getDestinationPortEnd()) { - throw new InvalidParameterValueException("Start port can't be bigger than end port"); + // Validate ip address + if (ipAddress == null) { + throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " doesn't exist in the system"); + } else if (ipAddress.isOneToOneNat()) { + throw new InvalidParameterValueException("Unable to create port forwarding rule; ip id=" + ipAddrId + " has static nat enabled"); } - // check that the port ranges are of equal size - if ((rule.getDestinationPortEnd() - rule.getDestinationPortStart()) != (rule.getSourcePortEnd() - rule.getSourcePortStart())) { - throw new InvalidParameterValueException("Source port and destination port ranges should be of equal sizes."); - } + final Long networkId = rule.getNetworkId(); + Network network = _networkModel.getNetwork(networkId); + //associate ip address to network (if needed) + boolean performedIpAssoc = false; + Nic guestNic; + if (ipAddress.getAssociatedWithNetworkId() == null) { + boolean assignToVpcNtwk = network.getVpcId() != null && ipAddress.getVpcId() != null && ipAddress.getVpcId().longValue() == network.getVpcId(); + if (assignToVpcNtwk) { + _networkModel.checkIpForService(ipAddress, Service.PortForwarding, networkId); - // validate user VM exists - UserVm vm = _vmDao.findById(vmId); - if (vm == null) { - throw new InvalidParameterValueException("Unable to create port forwarding rule on address " + ipAddress + ", invalid virtual machine id specified (" + - vmId + ")."); - } else if (vm.getState() == VirtualMachine.State.Destroyed || vm.getState() == VirtualMachine.State.Expunging) { - throw new InvalidParameterValueException("Invalid user vm: " + vm.getId()); - } - - // Verify that vm has nic in the network - Ip dstIp = rule.getDestinationIpAddress(); - guestNic = _networkModel.getNicInNetwork(vmId, networkId); - if (guestNic == null || guestNic.getIPv4Address() == null) { - throw new InvalidParameterValueException("Vm doesn't belong to network associated with ipAddress"); - } else { - dstIp = new Ip(guestNic.getIPv4Address()); - } - - if (vmIp != null) { - //vm ip is passed so it can be primary or secondary ip addreess. - if (!dstIp.equals(vmIp)) { - //the vm ip is secondary ip to the nic. - // is vmIp is secondary ip or not - NicSecondaryIp secondaryIp = _nicSecondaryDao.findByIp4AddressAndNicId(vmIp.toString(), guestNic.getId()); - if (secondaryIp == null) { - throw new InvalidParameterValueException("IP Address is not in the VM nic's network "); + s_logger.debug("The ip is not associated with the VPC network id=" + networkId + ", so assigning"); + try { + ipAddress = _ipAddrMgr.associateIPToGuestNetwork(ipAddrId, networkId, false); + performedIpAssoc = true; + } catch (Exception ex) { + throw new CloudRuntimeException("Failed to associate ip to VPC network as " + "a part of port forwarding rule creation"); } - dstIp = vmIp; } + } else { + _networkModel.checkIpForService(ipAddress, Service.PortForwarding, null); } - //if start port and end port are passed in, and they are not equal to each other, perform the validation - boolean validatePortRange = false; - if (rule.getSourcePortStart().intValue() != rule.getSourcePortEnd().intValue() || rule.getDestinationPortStart() != rule.getDestinationPortEnd()) { - validatePortRange = true; + if (ipAddress.getAssociatedWithNetworkId() == null) { + throw new InvalidParameterValueException("Ip address " + ipAddress + " is not assigned to the network " + network); } - if (validatePortRange) { - //source start port and source dest port should be the same. The same applies to dest ports - if (rule.getSourcePortStart().intValue() != rule.getDestinationPortStart()) { - throw new InvalidParameterValueException("Private port start should be equal to public port start"); + try { + _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.PortForwarding, + FirewallRuleType.User, networkId, rule.getTrafficType()); + + final Long accountId = ipAddress.getAllocatedToAccountId(); + final Long domainId = ipAddress.getAllocatedInDomainId(); + + // start port can't be bigger than end port + if (rule.getDestinationPortStart() > rule.getDestinationPortEnd()) { + throw new InvalidParameterValueException("Start port can't be bigger than end port"); } - if (rule.getSourcePortEnd().intValue() != rule.getDestinationPortEnd()) { - throw new InvalidParameterValueException("Private port end should be equal to public port end"); + // check that the port ranges are of equal size + if ((rule.getDestinationPortEnd() - rule.getDestinationPortStart()) != (rule.getSourcePortEnd() - rule.getSourcePortStart())) { + throw new InvalidParameterValueException("Source port and destination port ranges should be of equal sizes."); } - } - final Ip dstIpFinal = dstIp; - final IPAddressVO ipAddressFinal = ipAddress; - return Transaction.execute(new TransactionCallbackWithException() { - @Override - public PortForwardingRuleVO doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { + // validate user VM exists + UserVm vm = _vmDao.findById(vmId); + if (vm == null) { + throw new InvalidParameterValueException("Unable to create port forwarding rule on address " + ipAddress + ", invalid virtual machine id specified (" + + vmId + ")."); + } else if (vm.getState() == VirtualMachine.State.Destroyed || vm.getState() == VirtualMachine.State.Expunging) { + throw new InvalidParameterValueException("Invalid user vm: " + vm.getId()); + } + + // Verify that vm has nic in the network + Ip dstIp = rule.getDestinationIpAddress(); + guestNic = _networkModel.getNicInNetwork(vmId, networkId); + if (guestNic == null || guestNic.getIPv4Address() == null) { + throw new InvalidParameterValueException("Vm doesn't belong to network associated with ipAddress"); + } else { + dstIp = new Ip(guestNic.getIPv4Address()); + } + + if (vmIp != null) { + //vm ip is passed so it can be primary or secondary ip addreess. + if (!dstIp.equals(vmIp)) { + //the vm ip is secondary ip to the nic. + // is vmIp is secondary ip or not + NicSecondaryIp secondaryIp = _nicSecondaryDao.findByIp4AddressAndNicId(vmIp.toString(), guestNic.getId()); + if (secondaryIp == null) { + throw new InvalidParameterValueException("IP Address is not in the VM nic's network "); + } + dstIp = vmIp; + } + } + + //if start port and end port are passed in, and they are not equal to each other, perform the validation + boolean validatePortRange = false; + if (rule.getSourcePortStart().intValue() != rule.getSourcePortEnd().intValue() || rule.getDestinationPortStart() != rule.getDestinationPortEnd()) { + validatePortRange = true; + } + + if (validatePortRange) { + //source start port and source dest port should be the same. The same applies to dest ports + if (rule.getSourcePortStart().intValue() != rule.getDestinationPortStart()) { + throw new InvalidParameterValueException("Private port start should be equal to public port start"); + } + + if (rule.getSourcePortEnd().intValue() != rule.getDestinationPortEnd()) { + throw new InvalidParameterValueException("Private port end should be equal to public port end"); + } + } + + final Ip dstIpFinal = dstIp; + final IPAddressVO ipAddressFinal = ipAddress; + return Transaction.execute((TransactionCallbackWithException) status -> { PortForwardingRuleVO newRule = - new PortForwardingRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), dstIpFinal, - rule.getDestinationPortStart(), rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, accountId, domainId, vmId); + new PortForwardingRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), dstIpFinal, + rule.getDestinationPortStart(), rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId, accountId, domainId, vmId); if (forDisplay != null) { newRule.setDisplay(forDisplay); } newRule = _portForwardingDao.persist(newRule); - // create firewallRule for 0.0.0.0/0 cidr if (openFirewall) { _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, null, - newRule.getId(), networkId); + newRule.getId(), networkId); } try { @@ -334,7 +332,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules } CallContext.current().setEventDetails("Rule Id: " + newRule.getId()); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_RULE_ADD, newRule.getAccountId(), ipAddressFinal.getDataCenterId(), newRule.getId(), null, - PortForwardingRule.class.getName(), newRule.getUuid()); + PortForwardingRule.class.getName(), newRule.getUuid()); return newRule; } catch (Exception e) { if (newRule != null) { @@ -349,16 +347,17 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules throw new CloudRuntimeException("Unable to add rule for the ip id=" + ipAddrId, e); } + }); + } finally { + // release ip address if ipassoc was perfored + if (performedIpAssoc) { + //if the rule is the last one for the ip address assigned to VPC, unassign it from the network + IpAddress ip = _ipAddressDao.findById(ipAddress.getId()); + _vpcMgr.unassignIPFromVpcNetwork(ip.getId(), networkId); } - }); - - } finally { - // release ip address if ipassoc was perfored - if (performedIpAssoc) { - //if the rule is the last one for the ip address assigned to VPC, unassign it from the network - IpAddress ip = _ipAddressDao.findById(ipAddress.getId()); - _vpcMgr.unassignIPFromVpcNetwork(ip.getId(), networkId); } + } finally { + _ipAddressDao.releaseFromLockTable(ipAddrId); } } @@ -370,46 +369,44 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules final Long ipAddrId = rule.getSourceIpAddressId(); - IPAddressVO ipAddress = _ipAddressDao.findById(ipAddrId); + try { + IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(ipAddrId); - // Validate ip address - if (ipAddress == null) { - throw new InvalidParameterValueException("Unable to create static nat rule; ip id=" + ipAddrId + " doesn't exist in the system"); - } else if (ipAddress.isSourceNat() || !ipAddress.isOneToOneNat() || ipAddress.getAssociatedWithVmId() == null) { - throw new NetworkRuleConflictException("Can't do static nat on ip address: " + ipAddress.getAddress()); - } + // Validate ip address + if (ipAddress == null) { + throw new InvalidParameterValueException("Unable to create static nat rule; ip id=" + ipAddrId + " doesn't exist in the system"); + } else if (ipAddress.isSourceNat() || !ipAddress.isOneToOneNat() || ipAddress.getAssociatedWithVmId() == null) { + throw new NetworkRuleConflictException("Can't do static nat on ip address: " + ipAddress.getAddress()); + } - _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.StaticNat, - FirewallRuleType.User, null, rule.getTrafficType()); + _firewallMgr.validateFirewallRule(caller, ipAddress, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), Purpose.StaticNat, + FirewallRuleType.User, null, rule.getTrafficType()); - final Long networkId = ipAddress.getAssociatedWithNetworkId(); - final Long accountId = ipAddress.getAllocatedToAccountId(); - final Long domainId = ipAddress.getAllocatedInDomainId(); + final Long networkId = ipAddress.getAssociatedWithNetworkId(); + final Long accountId = ipAddress.getAllocatedToAccountId(); + final Long domainId = ipAddress.getAllocatedInDomainId(); - _networkModel.checkIpForService(ipAddress, Service.StaticNat, null); + _networkModel.checkIpForService(ipAddress, Service.StaticNat, null); - Network network = _networkModel.getNetwork(networkId); - NetworkOffering off = _entityMgr.findById(NetworkOffering.class, network.getNetworkOfferingId()); - if (off.isElasticIp()) { - throw new InvalidParameterValueException("Can't create ip forwarding rules for the network where elasticIP service is enabled"); - } - - //String dstIp = _networkModel.getIpInNetwork(ipAddress.getAssociatedWithVmId(), networkId); - final String dstIp = ipAddress.getVmIp(); - return Transaction.execute(new TransactionCallbackWithException() { - @Override - public StaticNatRule doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { + Network network = _networkModel.getNetwork(networkId); + NetworkOffering off = _entityMgr.findById(NetworkOffering.class, network.getNetworkOfferingId()); + if (off.isElasticIp()) { + throw new InvalidParameterValueException("Can't create ip forwarding rules for the network where elasticIP service is enabled"); + } + //String dstIp = _networkModel.getIpInNetwork(ipAddress.getAssociatedWithVmId(), networkId); + final String dstIp = ipAddress.getVmIp(); + return Transaction.execute((TransactionCallbackWithException) status -> { FirewallRuleVO newRule = - new FirewallRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol().toLowerCase(), - networkId, accountId, domainId, rule.getPurpose(), null, null, null, null, null); + new FirewallRuleVO(rule.getXid(), rule.getSourceIpAddressId(), rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol().toLowerCase(), + networkId, accountId, domainId, rule.getPurpose(), null, null, null, null, null); newRule = _firewallDao.persist(newRule); // create firewallRule for 0.0.0.0/0 cidr if (openFirewall) { _firewallMgr.createRuleForAllCidrs(ipAddrId, caller, rule.getSourcePortStart(), rule.getSourcePortEnd(), rule.getProtocol(), null, null, - newRule.getId(), networkId); + newRule.getId(), networkId); } try { @@ -419,11 +416,9 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules } CallContext.current().setEventDetails("Rule Id: " + newRule.getId()); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_RULE_ADD, newRule.getAccountId(), 0, newRule.getId(), null, FirewallRule.class.getName(), - newRule.getUuid()); + newRule.getUuid()); - StaticNatRule staticNatRule = new StaticNatRuleImpl(newRule, dstIp); - - return staticNatRule; + return new StaticNatRuleImpl(newRule, dstIp); } catch (Exception e) { if (newRule != null) { // no need to apply the rule as it wasn't programmed on the backend yet @@ -436,9 +431,10 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules } throw new CloudRuntimeException("Unable to add static nat rule for the ip id=" + newRule.getSourceIpAddressId(), e); } - } - }); - + }); + } finally { + _ipAddressDao.releaseFromLockTable(ipAddrId); + } } @Override diff --git a/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java b/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java index 61d247d7b8a..9ff599b47c6 100644 --- a/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpn/RemoteAccessVpnManagerImpl.java @@ -155,6 +155,7 @@ public class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc return vpns; } + @Override @DB public RemoteAccessVpn createRemoteAccessVpn(final long publicIpId, String ipRange, boolean openFirewall, final Boolean forDisplay) throws NetworkRuleConflictException { @@ -172,92 +173,97 @@ public class RemoteAccessVpnManagerImpl extends ManagerBase implements RemoteAcc throw new InvalidParameterValueException("The Ip address is not ready to be used yet: " + ipAddr.getAddress()); } - IPAddressVO ipAddress = _ipAddressDao.findById(publicIpId); + try { + IPAddressVO ipAddress = _ipAddressDao.acquireInLockTable(publicIpId); - Long networkId = ipAddress.getAssociatedWithNetworkId(); - if (networkId != null) { - _networkMgr.checkIpForService(ipAddress, Service.Vpn, null); - } - - final Long vpcId = ipAddress.getVpcId(); - if (vpcId != null && ipAddress.isSourceNat()) { - assert networkId == null; - openFirewall = false; - } - - final boolean openFirewallFinal = openFirewall; - - if (networkId == null && vpcId == null) { - throw new InvalidParameterValueException("Unable to create remote access vpn for the ipAddress: " + ipAddr.getAddress().addr() + - " as ip is not associated with any network or VPC"); - } - - RemoteAccessVpnVO vpnVO = _remoteAccessVpnDao.findByPublicIpAddress(publicIpId); - - if (vpnVO != null) { - if (vpnVO.getState() == RemoteAccessVpn.State.Added) { - return vpnVO; + if (ipAddress == null) { + s_logger.error(String.format("Unable to acquire lock on public IP %s.", publicIpId)); + throw new CloudRuntimeException("Unable to acquire lock on public IP."); } - throw new InvalidParameterValueException(String.format("A remote Access VPN already exists for the public IP address [%s].", ipAddr.getAddress().toString())); - } + Long networkId = ipAddress.getAssociatedWithNetworkId(); + if (networkId != null) { + _networkMgr.checkIpForService(ipAddress, Service.Vpn, null); + } - if (ipRange == null) { - ipRange = RemoteAccessVpnClientIpRange.valueIn(ipAddr.getAccountId()); - } + final Long vpcId = ipAddress.getVpcId(); + if (vpcId != null && ipAddress.isSourceNat()) { + assert networkId == null; + openFirewall = false; + } - validateIpRange(ipRange, InvalidParameterValueException.class); + final boolean openFirewallFinal = openFirewall; - String[] range = ipRange.split("-"); + if (networkId == null && vpcId == null) { + throw new InvalidParameterValueException("Unable to create remote access vpn for the ipAddress: " + ipAddr.getAddress().addr() + + " as ip is not associated with any network or VPC"); + } - Pair cidr = null; + RemoteAccessVpnVO vpnVO = _remoteAccessVpnDao.findByPublicIpAddress(publicIpId); - if (networkId != null) { - long ipAddressOwner = ipAddr.getAccountId(); - vpnVO = _remoteAccessVpnDao.findByAccountAndNetwork(ipAddressOwner, networkId); if (vpnVO != null) { if (vpnVO.getState() == RemoteAccessVpn.State.Added) { return vpnVO; } - throw new InvalidParameterValueException(String.format("A remote access VPN already exists for the account [%s].", ipAddressOwner)); + throw new InvalidParameterValueException(String.format("A remote Access VPN already exists for the public IP address [%s].", ipAddr.getAddress().toString())); } - Network network = _networkMgr.getNetwork(networkId); - if (!_networkMgr.areServicesSupportedInNetwork(network.getId(), Service.Vpn)) { - throw new InvalidParameterValueException("Vpn service is not supported in network id=" + ipAddr.getAssociatedWithNetworkId()); + + if (ipRange == null) { + ipRange = RemoteAccessVpnClientIpRange.valueIn(ipAddr.getAccountId()); } - cidr = NetUtils.getCidr(network.getCidr()); - } else { - Vpc vpc = _vpcDao.findById(vpcId); - cidr = NetUtils.getCidr(vpc.getCidr()); - } - String[] guestIpRange = NetUtils.getIpRangeFromCidr(cidr.first(), cidr.second()); - if (NetUtils.ipRangesOverlap(range[0], range[1], guestIpRange[0], guestIpRange[1])) { - throw new InvalidParameterValueException("Invalid ip range: " + ipRange + " overlaps with guest ip range " + guestIpRange[0] + "-" + guestIpRange[1]); - } + validateIpRange(ipRange, InvalidParameterValueException.class); - long startIp = NetUtils.ip2Long(range[0]); - final String newIpRange = NetUtils.long2Ip(++startIp) + "-" + range[1]; - final String sharedSecret = PasswordGenerator.generatePresharedKey(_pskLength); + String[] range = ipRange.split("-"); - return Transaction.execute(new TransactionCallbackWithException() { - @Override - public RemoteAccessVpn doInTransaction(TransactionStatus status) throws NetworkRuleConflictException { + Pair cidr = null; + + if (networkId != null) { + long ipAddressOwner = ipAddr.getAccountId(); + vpnVO = _remoteAccessVpnDao.findByAccountAndNetwork(ipAddressOwner, networkId); + if (vpnVO != null) { + if (vpnVO.getState() == RemoteAccessVpn.State.Added) { + return vpnVO; + } + + throw new InvalidParameterValueException(String.format("A remote access VPN already exists for the account [%s].", ipAddressOwner)); + } + Network network = _networkMgr.getNetwork(networkId); + if (!_networkMgr.areServicesSupportedInNetwork(network.getId(), Service.Vpn)) { + throw new InvalidParameterValueException("Vpn service is not supported in network id=" + ipAddr.getAssociatedWithNetworkId()); + } + cidr = NetUtils.getCidr(network.getCidr()); + } else { + Vpc vpc = _vpcDao.findById(vpcId); + cidr = NetUtils.getCidr(vpc.getCidr()); + } + + String[] guestIpRange = NetUtils.getIpRangeFromCidr(cidr.first(), cidr.second()); + if (NetUtils.ipRangesOverlap(range[0], range[1], guestIpRange[0], guestIpRange[1])) { + throw new InvalidParameterValueException("Invalid ip range: " + ipRange + " overlaps with guest ip range " + guestIpRange[0] + "-" + guestIpRange[1]); + } + + long startIp = NetUtils.ip2Long(range[0]); + final String newIpRange = NetUtils.long2Ip(++startIp) + "-" + range[1]; + final String sharedSecret = PasswordGenerator.generatePresharedKey(_pskLength); + + return Transaction.execute((TransactionCallbackWithException) status -> { if (vpcId == null) { _rulesMgr.reservePorts(ipAddr, NetUtils.UDP_PROTO, Purpose.Vpn, openFirewallFinal, caller, NetUtils.VPN_PORT, NetUtils.VPN_L2TP_PORT, - NetUtils.VPN_NATT_PORT); + NetUtils.VPN_NATT_PORT); } - RemoteAccessVpnVO vpnVO = - new RemoteAccessVpnVO(ipAddr.getAccountId(), ipAddr.getDomainId(), ipAddr.getAssociatedWithNetworkId(), publicIpId, vpcId, range[0], newIpRange, - sharedSecret); + RemoteAccessVpnVO remoteAccessVpnVO = new RemoteAccessVpnVO(ipAddr.getAccountId(), ipAddr.getDomainId(), ipAddr.getAssociatedWithNetworkId(), + publicIpId, vpcId, range[0], newIpRange, sharedSecret); if (forDisplay != null) { - vpnVO.setDisplay(forDisplay); + remoteAccessVpnVO.setDisplay(forDisplay); } - return _remoteAccessVpnDao.persist(vpnVO); - } - }); + return _remoteAccessVpnDao.persist(remoteAccessVpnVO); + }); + } finally { + _ipAddressDao.releaseFromLockTable(publicIpId); + } } private void validateRemoteAccessVpnConfiguration() throws ConfigurationException {