Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2176025245 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2176021787 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2126279705 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9685 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2126213483 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2126212617 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1609654096
##
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##
@@ -1147,6 +1155,41 @@ public ListResponse
searchForUserVMs(ListVMsCmd cmd) {
return response;
}
+@Override
+public ListResponse
findAffectedVmsForStorageScopeChange(FindAffectedVmsForStorageScopeChangeCmd
cmd) {
+Long poolId = cmd.getStorageId();
+StoragePoolVO pool = storagePoolDao.findById(poolId);
+if (pool == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + poolId);
+}
+if (!pool.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope change of Storage
pool is only allowed in Disabled state");
+}
Review Comment:
Good point. Will remove the disabled check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1609378387
##
engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java:
##
@@ -1041,6 +1042,8 @@ public Pair, Integer>
listByVmsNotInClusterUsingPool(long clu
sc.setParameters("vmStates", State.Starting, State.Running,
State.Stopping, State.Migrating, State.Restoring);
sc.setJoinParameters("volumeSearch", "poolId", poolId);
sc.setJoinParameters("hostSearch2", "clusterId", clusterId);
-return searchAndCount(sc, null, false);
+List vms = search(sc, null);
+List uniqueVms =
vms.stream().distinct().collect(Collectors.toList());
+return new Pair<>(uniqueVms, uniqueVms.size());
Review Comment:
good finding !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1609368832
##
engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java:
##
@@ -1041,6 +1042,8 @@ public Pair, Integer>
listByVmsNotInClusterUsingPool(long clu
sc.setParameters("vmStates", State.Starting, State.Running,
State.Stopping, State.Migrating, State.Restoring);
sc.setJoinParameters("volumeSearch", "poolId", poolId);
sc.setJoinParameters("hostSearch2", "clusterId", clusterId);
-return searchAndCount(sc, null, false);
+List vms = search(sc, null);
+List uniqueVms =
vms.stream().distinct().collect(Collectors.toList());
+return new Pair<>(uniqueVms, uniqueVms.size());
Review Comment:
Yes @weizhouapache, before this change it was returning multiple entries for
the same VM like this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1609368832
##
engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java:
##
@@ -1041,6 +1042,8 @@ public Pair, Integer>
listByVmsNotInClusterUsingPool(long clu
sc.setParameters("vmStates", State.Starting, State.Running,
State.Stopping, State.Migrating, State.Restoring);
sc.setJoinParameters("volumeSearch", "poolId", poolId);
sc.setJoinParameters("hostSearch2", "clusterId", clusterId);
-return searchAndCount(sc, null, false);
+List vms = search(sc, null);
+List uniqueVms =
vms.stream().distinct().collect(Collectors.toList());
+return new Pair<>(uniqueVms, uniqueVms.size());
Review Comment:
Yes, before this change it was returning multiple entries for the same VM
like this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1609368832
##
engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java:
##
@@ -1041,6 +1042,8 @@ public Pair, Integer>
listByVmsNotInClusterUsingPool(long clu
sc.setParameters("vmStates", State.Starting, State.Running,
State.Stopping, State.Migrating, State.Restoring);
sc.setJoinParameters("volumeSearch", "poolId", poolId);
sc.setJoinParameters("hostSearch2", "clusterId", clusterId);
-return searchAndCount(sc, null, false);
+List vms = search(sc, null);
+List uniqueVms =
vms.stream().distinct().collect(Collectors.toList());
+return new Pair<>(uniqueVms, uniqueVms.size());
Review Comment:
Yes, it was returning multiple entries for the same VM like this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1609350275
##
engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java:
##
@@ -1041,6 +1042,8 @@ public Pair, Integer>
listByVmsNotInClusterUsingPool(long clu
sc.setParameters("vmStates", State.Starting, State.Running,
State.Stopping, State.Migrating, State.Restoring);
sc.setJoinParameters("volumeSearch", "poolId", poolId);
sc.setJoinParameters("hostSearch2", "clusterId", clusterId);
-return searchAndCount(sc, null, false);
+List vms = search(sc, null);
+List uniqueVms =
vms.stream().distinct().collect(Collectors.toList());
+return new Pair<>(uniqueVms, uniqueVms.size());
Review Comment:
make sense :+1:
@abh1sar
code looks good. have you tested it ? (vm has multiple disks on a pool)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1609267083
##
api/src/main/java/org/apache/cloudstack/api/response/VirtualMachineResponse.java:
##
@@ -0,0 +1,116 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.response;
+
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.EntityReference;
+
+import com.cloud.serializer.Param;
+import com.cloud.vm.VirtualMachine;
+import com.google.gson.annotations.SerializedName;
+
+@EntityReference(value = VirtualMachine.class)
+public class VirtualMachineResponse extends BaseResponse {
+@SerializedName("id")
+@Param(description = "the ID of the VM")
+private String id;
+
+@SerializedName("type")
+@Param(description = "the type of VM")
+private String type;
+
+@SerializedName("name")
+@Param(description = "the name of the VM")
+private String name;
+
+@SerializedName("clusterid")
+@Param(description = "the Pod ID for the system VM")
+private String clusterId;
+
+@SerializedName("clustername")
+@Param(description = "the Pod name for the system VM", since = "4.13.2")
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1609266983
##
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/FindAffectedVmsForStorageScopeChangeCmd.java:
##
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.api.command.admin.vm;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseListCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.VirtualMachineResponse;
+
+import com.cloud.vm.VirtualMachine;
+
+@APICommand(name = "findAffectedVmsForStorageScopeChange",
+description = "List user and system VMs that need to be stopped and
destroyed respectively for changing the scope of the storage pool from Zone to
Cluster.",
+responseObject = VirtualMachineResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.19.1",
+authorized = {RoleType.Admin})
+public class FindAffectedVmsForStorageScopeChangeCmd extends BaseListCmd {
Review Comment:
Changed to list
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1609266798
##
api/src/main/java/org/apache/cloudstack/api/response/VirtualMachineResponse.java:
##
@@ -0,0 +1,116 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.response;
+
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.EntityReference;
+
+import com.cloud.serializer.Param;
+import com.cloud.vm.VirtualMachine;
+import com.google.gson.annotations.SerializedName;
+
+@EntityReference(value = VirtualMachine.class)
+public class VirtualMachineResponse extends BaseResponse {
+@SerializedName("id")
+@Param(description = "the ID of the VM")
+private String id;
+
+@SerializedName("type")
+@Param(description = "the type of VM")
+private String type;
+
+@SerializedName("name")
+@Param(description = "the name of the VM")
+private String name;
+
+@SerializedName("clusterid")
+@Param(description = "the Pod ID for the system VM")
+private String clusterId;
+
+@SerializedName("clustername")
+@Param(description = "the Pod name for the system VM", since = "4.13.2")
Review Comment:
Done
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1211,10 +1211,13 @@ private void
changeStoragePoolScopeToCluster(StoragePoolVO primaryStorage, Long
List states = Arrays.asList(State.Starting,
State.Running, State.Stopping, State.Migrating, State.Restoring);
Long id = primaryStorage.getId();
-List volumesInOtherClusters =
volumeDao.listByPoolIdVMStatesNotInCluster(clusterId, states, id);
-if (volumesInOtherClusters.size() > 0) {
-throw new CloudRuntimeException(String.format("Cannot change scope
of the pool %s to cluster %s as there are VMs running on other clusters which
are using volumes on this storage pool",
-primaryStorage.getName(), clusterVO.getName()));
+Pair, Integer> vmsNotInClusterUsingPool =
_vmInstanceDao.listByVmsNotInClusterUsingPool(clusterId, id);
+if (vmsNotInClusterUsingPool.second() != 0) {
+throw new CloudRuntimeException(String.format("Cannot change scope
of the storage pool [%s] to cluster [%s] %s %s %s",
+primaryStorage.getName(), clusterVO.getName(),
+"as there are VMs with volumes in this pool that are
running on other clusters.",
+"All such User VMs must be stopped and System VMs must be
destroyed before proceeding.",
+"Please use the API findAffectedVmsForStorageScopeChange
to get the list."));
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608519272
##
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##
@@ -1147,6 +1155,41 @@ public ListResponse
searchForUserVMs(ListVMsCmd cmd) {
return response;
}
+@Override
+public ListResponse
findAffectedVmsForStorageScopeChange(FindAffectedVmsForStorageScopeChangeCmd
cmd) {
+Long poolId = cmd.getStorageId();
+StoragePoolVO pool = storagePoolDao.findById(poolId);
+if (pool == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + poolId);
+}
+if (!pool.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope change of Storage
pool is only allowed in Disabled state");
+}
+ListResponse response = new ListResponse<>();
+if (pool.getScope() != ScopeType.ZONE) {
+response.setResponses(null, 0);
+return response;
+}
+
+Pair, Integer> vms =
_vmInstanceDao.listByVmsNotInClusterUsingPool(cmd.getClusterIdForScopeChange(),
poolId);
+List responsesList = new ArrayList<>();
+for (VMInstanceVO vm : vms.first()) {
+VirtualMachineResponse resp = new VirtualMachineResponse();
+
resp.setObjectName(VirtualMachine.class.getSimpleName().toLowerCase());
+resp.setId(vm.getUuid());
+resp.setVmType(vm.getType().toString());
+HostVO host = hostDao.findById(vm.getHostId());
+resp.setHostId(host.getUuid());
Review Comment:
if the vm state is State.Starting, State.Running, State.Stopping,
State.Migrating, State.Restoring,
the host_id should not be null, and host should not be removed
but... I do not know if there is any edge cases (or by manual db changes,
etc)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608511027
##
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##
@@ -1147,6 +1155,41 @@ public ListResponse
searchForUserVMs(ListVMsCmd cmd) {
return response;
}
+@Override
+public ListResponse
findAffectedVmsForStorageScopeChange(FindAffectedVmsForStorageScopeChangeCmd
cmd) {
+Long poolId = cmd.getStorageId();
+StoragePoolVO pool = storagePoolDao.findById(poolId);
+if (pool == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + poolId);
+}
+if (!pool.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope change of Storage
pool is only allowed in Disabled state");
+}
Review Comment:
I think this API could be useful in two scenarios
- users fail to change the scope of storage pool, and then use this API to
get the list of vm instances are affected.
- users use this API to check if a storage pool can be re-scoped, before
performing the actions (disable pool, change pool scope)
if the check on pool status is present (only available for Disabled pools),
users have to (1) disable pool; (2) list affected vms; (3) enable pool if the
pool cannot be re-scoped.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608507465
##
api/src/main/java/org/apache/cloudstack/api/response/VirtualMachineResponse.java:
##
@@ -0,0 +1,116 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.response;
+
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.EntityReference;
+
+import com.cloud.serializer.Param;
+import com.cloud.vm.VirtualMachine;
+import com.google.gson.annotations.SerializedName;
+
+@EntityReference(value = VirtualMachine.class)
+public class VirtualMachineResponse extends BaseResponse {
+@SerializedName("id")
+@Param(description = "the ID of the VM")
+private String id;
+
+@SerializedName("type")
+@Param(description = "the type of VM")
+private String type;
+
+@SerializedName("name")
+@Param(description = "the name of the VM")
+private String name;
+
+@SerializedName("clusterid")
+@Param(description = "the Pod ID for the system VM")
+private String clusterId;
+
+@SerializedName("clustername")
+@Param(description = "the Pod name for the system VM", since = "4.13.2")
+private String clusterName;
+
+@SerializedName("hostid")
+@Param(description = "the host ID for the system VM")
+private String hostId;
+
+@SerializedName("hostname")
+@Param(description = "the hostname for the system VM")
+private String hostName;
+
Review Comment:
Yes, I agree to keep it simple
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608502249
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1211,10 +1211,13 @@ private void
changeStoragePoolScopeToCluster(StoragePoolVO primaryStorage, Long
List states = Arrays.asList(State.Starting,
State.Running, State.Stopping, State.Migrating, State.Restoring);
Long id = primaryStorage.getId();
-List volumesInOtherClusters =
volumeDao.listByPoolIdVMStatesNotInCluster(clusterId, states, id);
-if (volumesInOtherClusters.size() > 0) {
-throw new CloudRuntimeException(String.format("Cannot change scope
of the pool %s to cluster %s as there are VMs running on other clusters which
are using volumes on this storage pool",
-primaryStorage.getName(), clusterVO.getName()));
+Pair, Integer> vmsNotInClusterUsingPool =
_vmInstanceDao.listByVmsNotInClusterUsingPool(clusterId, id);
+if (vmsNotInClusterUsingPool.second() != 0) {
+throw new CloudRuntimeException(String.format("Cannot change scope
of the storage pool [%s] to cluster [%s] %s %s %s",
+primaryStorage.getName(), clusterVO.getName(),
+"as there are VMs with volumes in this pool that are
running on other clusters.",
+"All such User VMs must be stopped and System VMs must be
destroyed before proceeding.",
+"Please use the API findAffectedVmsForStorageScopeChange
to get the list."));
Review Comment:
no hard reason. But I see how this could look weird. Will update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608498589
##
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##
@@ -1147,6 +1155,41 @@ public ListResponse
searchForUserVMs(ListVMsCmd cmd) {
return response;
}
+@Override
+public ListResponse
findAffectedVmsForStorageScopeChange(FindAffectedVmsForStorageScopeChangeCmd
cmd) {
+Long poolId = cmd.getStorageId();
+StoragePoolVO pool = storagePoolDao.findById(poolId);
+if (pool == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + poolId);
+}
+if (!pool.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope change of Storage
pool is only allowed in Disabled state");
+}
+ListResponse response = new ListResponse<>();
+if (pool.getScope() != ScopeType.ZONE) {
+response.setResponses(null, 0);
+return response;
+}
+
+Pair, Integer> vms =
_vmInstanceDao.listByVmsNotInClusterUsingPool(cmd.getClusterIdForScopeChange(),
poolId);
+List responsesList = new ArrayList<>();
+for (VMInstanceVO vm : vms.first()) {
+VirtualMachineResponse resp = new VirtualMachineResponse();
+
resp.setObjectName(VirtualMachine.class.getSimpleName().toLowerCase());
+resp.setId(vm.getUuid());
+resp.setVmType(vm.getType().toString());
+HostVO host = hostDao.findById(vm.getHostId());
+resp.setHostId(host.getUuid());
Review Comment:
Have added the check but just curious if there could be any special
condition when the vm_instance won't contain the host_id?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608479716
##
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##
@@ -1147,6 +1155,41 @@ public ListResponse
searchForUserVMs(ListVMsCmd cmd) {
return response;
}
+@Override
+public ListResponse
findAffectedVmsForStorageScopeChange(FindAffectedVmsForStorageScopeChangeCmd
cmd) {
+Long poolId = cmd.getStorageId();
+StoragePoolVO pool = storagePoolDao.findById(poolId);
+if (pool == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + poolId);
+}
+if (!pool.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope change of Storage
pool is only allowed in Disabled state");
+}
Review Comment:
Since scope change is only allowed when storage pool is disabled, the idea
behind doing this was to not let admin use this api too unless the storage pool
is disabled.
Do you think operator would want to use this api to get the list of vms even
when storage pool state is up?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608446424 ## api/src/main/java/org/apache/cloudstack/api/response/VirtualMachineResponse.java: ## @@ -0,0 +1,116 @@ +// Licensed to the Apache Software Foundation (ASF) under one Review Comment: yes, the response can return system VMs as well. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608270565 ## api/src/main/java/org/apache/cloudstack/api/response/VirtualMachineResponse.java: ## @@ -0,0 +1,116 @@ +// Licensed to the Apache Software Foundation (ASF) under one Review Comment: if storage scope is changed from Zone to Cluster, the hosts in other cluster will disconnect from the storage. the running VMs (user vms, vr, system vms) on the hosts will be affected. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608252968
##
api/src/main/java/org/apache/cloudstack/api/response/VirtualMachineResponse.java:
##
@@ -0,0 +1,116 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.response;
+
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.EntityReference;
+
+import com.cloud.serializer.Param;
+import com.cloud.vm.VirtualMachine;
+import com.google.gson.annotations.SerializedName;
+
+@EntityReference(value = VirtualMachine.class)
+public class VirtualMachineResponse extends BaseResponse {
+@SerializedName("id")
+@Param(description = "the ID of the VM")
+private String id;
+
+@SerializedName("type")
+@Param(description = "the type of VM")
+private String type;
+
+@SerializedName("name")
+@Param(description = "the name of the VM")
+private String name;
+
+@SerializedName("clusterid")
+@Param(description = "the Pod ID for the system VM")
Review Comment:
```
@Param(description = "the Cluster ID for the VM instance")
```
##
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/FindAffectedVmsForStorageScopeChangeCmd.java:
##
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.api.command.admin.vm;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseListCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.VirtualMachineResponse;
+
+import com.cloud.vm.VirtualMachine;
+
+@APICommand(name = "findAffectedVmsForStorageScopeChange",
+description = "List user and system VMs that need to be stopped and
destroyed respectively for changing the scope of the storage pool from Zone to
Cluster.",
+responseObject = VirtualMachineResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false,
since = "4.19.1",
+authorized = {RoleType.Admin})
+public class FindAffectedVmsForStorageScopeChangeCmd extends BaseListCmd {
Review Comment:
normally the APIs to list resources start with `list`, except
`FindHostsForMigrationCmd` and `FindStoragePoolsForMigrationCmd`
it may be better to use "List" instead of "Find" (most cases `find` returns
only 0 or 1 record).
it is a trivial issue if use 'find' (ok to me)
##
api/src/main/java/org/apache/cloudstack/api/response/VirtualMachineResponse.java:
##
@@ -0,0 +1,116 @@
+// 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
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
shwstppr commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1608235310
##
api/src/main/java/org/apache/cloudstack/api/response/VirtualMachineResponse.java:
##
@@ -0,0 +1,116 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.response;
+
+import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.EntityReference;
+
+import com.cloud.serializer.Param;
+import com.cloud.vm.VirtualMachine;
+import com.google.gson.annotations.SerializedName;
+
+@EntityReference(value = VirtualMachine.class)
+public class VirtualMachineResponse extends BaseResponse {
+@SerializedName("id")
+@Param(description = "the ID of the VM")
+private String id;
+
+@SerializedName("type")
+@Param(description = "the type of VM")
+private String type;
+
+@SerializedName("name")
+@Param(description = "the name of the VM")
+private String name;
+
+@SerializedName("clusterid")
+@Param(description = "the Pod ID for the system VM")
+private String clusterId;
+
+@SerializedName("clustername")
+@Param(description = "the Pod name for the system VM", since = "4.13.2")
Review Comment:
```suggestion
@Param(description = "the Pod name for the system VM")
```
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1211,10 +1211,13 @@ private void
changeStoragePoolScopeToCluster(StoragePoolVO primaryStorage, Long
List states = Arrays.asList(State.Starting,
State.Running, State.Stopping, State.Migrating, State.Restoring);
Long id = primaryStorage.getId();
-List volumesInOtherClusters =
volumeDao.listByPoolIdVMStatesNotInCluster(clusterId, states, id);
-if (volumesInOtherClusters.size() > 0) {
-throw new CloudRuntimeException(String.format("Cannot change scope
of the pool %s to cluster %s as there are VMs running on other clusters which
are using volumes on this storage pool",
-primaryStorage.getName(), clusterVO.getName()));
+Pair, Integer> vmsNotInClusterUsingPool =
_vmInstanceDao.listByVmsNotInClusterUsingPool(clusterId, id);
+if (vmsNotInClusterUsingPool.second() != 0) {
+throw new CloudRuntimeException(String.format("Cannot change scope
of the storage pool [%s] to cluster [%s] %s %s %s",
+primaryStorage.getName(), clusterVO.getName(),
+"as there are VMs with volumes in this pool that are
running on other clusters.",
+"All such User VMs must be stopped and System VMs must be
destroyed before proceeding.",
+"Please use the API findAffectedVmsForStorageScopeChange
to get the list."));
Review Comment:
curious - why are we adding constant strings with format?
##
api/src/main/java/org/apache/cloudstack/api/response/VirtualMachineResponse.java:
##
@@ -0,0 +1,116 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
Why do we need this class? Is it possible to use UserVmResponse or do we
also have affected VR and system VMs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2088182906 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9499 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2088115311 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2088113808 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2084751548 @blueorangutan test rocky8 kvm-rocky8 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2084501990 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9474 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2084422951 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2084421819 @blueorangutan pakage -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2082322949 @shwstppr @weizhouapache I have added some UT to increase coverage. Please check. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2076469304 Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 9420 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2076429330 @abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2076427276 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar closed pull request #8875: Change storage pool scope from Cluster to Zone and vise versa URL: https://github.com/apache/cloudstack/pull/8875 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1577718662
##
test/integration/smoke/test_primary_storage_scope.py:
##
@@ -0,0 +1,170 @@
+# 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.
+""" BVT tests for Primary Storage
+"""
+
+# Import System modules
+# Import Local Modules
+from marvin.cloudstackTestCase import *
+from marvin.lib.base import (Host, StoragePool, Cluster, updateStoragePool,
changeStoragePoolScope)
+from marvin.lib.common import (get_zone, get_pod, list_clusters)
+from nose.plugins.attrib import attr
+
+class TestPrimaryStorageScope(cloudstackTestCase):
+
+def setUp(self):
+
+self.apiclient = self.testClient.getApiClient()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.debug("here")
+self.debug(self.services)
+self.cluster1 = list_clusters(self.apiclient)[0]
+self.debug("here1")
+self.debug(self.cluster1)
+self.cluster = {
+'clustername': 'C0_testScope',
+'clustertype': 'CloudManaged'
+}
+self.cluster2 = Cluster.create(self.apiclient,
+ self.cluster,
+ zoneid=self.zone.id,
+ podid=self.pod.id,
+ hypervisor=self.cluster1.hypervisortype
+ )
+self.storage = StoragePool.create(self.apiclient,
+ self.services["nfs"],
+ scope = 'ZONE',
+ zoneid=self.zone.id,
+
hypervisor=self.cluster1.hypervisortype
+ )
+self.debug("Created storage pool %s in zone scope", self.storage.id)
+return
+
+def tearDown(self):
+StoragePool.delete(self.storage, self.apiclient)
+Cluster.delete(self.cluster2, self.apiclient)
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
shwstppr commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1577642488 ## test/integration/smoke/test_primary_storage.py: ## @@ -372,8 +372,6 @@ def __init__(self): "a": "NFS-A", "b": "NFS-B" } - - Review Comment: Maybe remove these non-required changes -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
DaanHoogland commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1576366408
##
test/integration/smoke/test_primary_storage_scope.py:
##
@@ -0,0 +1,170 @@
+# 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.
+""" BVT tests for Primary Storage
+"""
+
+# Import System modules
+# Import Local Modules
+from marvin.cloudstackTestCase import *
+from marvin.lib.base import (Host, StoragePool, Cluster, updateStoragePool,
changeStoragePoolScope)
+from marvin.lib.common import (get_zone, get_pod, list_clusters)
+from nose.plugins.attrib import attr
+
+class TestPrimaryStorageScope(cloudstackTestCase):
+
+def setUp(self):
+
+self.apiclient = self.testClient.getApiClient()
+self.dbclient = self.testClient.getDbConnection()
+self.services = self.testClient.getParsedTestDataConfig()
+self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests())
+self.pod = get_pod(self.apiclient, self.zone.id)
+self.debug("here")
+self.debug(self.services)
+self.cluster1 = list_clusters(self.apiclient)[0]
+self.debug("here1")
+self.debug(self.cluster1)
+self.cluster = {
+'clustername': 'C0_testScope',
+'clustertype': 'CloudManaged'
+}
+self.cluster2 = Cluster.create(self.apiclient,
+ self.cluster,
+ zoneid=self.zone.id,
+ podid=self.pod.id,
+ hypervisor=self.cluster1.hypervisortype
+ )
+self.storage = StoragePool.create(self.apiclient,
+ self.services["nfs"],
+ scope = 'ZONE',
+ zoneid=self.zone.id,
+
hypervisor=self.cluster1.hypervisortype
+ )
+self.debug("Created storage pool %s in zone scope", self.storage.id)
+return
+
+def tearDown(self):
+StoragePool.delete(self.storage, self.apiclient)
+Cluster.delete(self.cluster2, self.apiclient)
Review Comment:
please look at the convention (a `cleanup` array) and implement thus.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1576328651
##
ui/src/config/section/infra/primaryStorages.js:
##
@@ -133,6 +133,24 @@ export default {
dataView: true,
show: (record) => { return ['Maintenance', 'PrepareForMaintenance',
'ErrorInMaintenance'].includes(record.state) }
},
+{
+ api: 'changeStoragePoolScope',
+ icon: 'arrow-right-outlined',
+ label: 'label.action.change.primary.storage.scope',
+ dataView: true,
+ popup: true,
+ show: (record) => {
+return (record.state === 'Disabled' &&
+ (record.hypervisor === 'KVM' ||
+ record.hypervisor === 'VMware' ||
+ record.hypervisor === 'HyperV' ||
+ record.hypervisor === 'LXC' ||
+ record.hypervisor === 'Any' ||
+ record.hypervisor === 'Simulator')
Review Comment:
Only two hypervisors are tested.
But we will allow root admin the ability to use the feature for other
hypervisors as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
shwstppr commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1576111447
##
ui/src/config/section/infra/primaryStorages.js:
##
@@ -133,6 +133,24 @@ export default {
dataView: true,
show: (record) => { return ['Maintenance', 'PrepareForMaintenance',
'ErrorInMaintenance'].includes(record.state) }
},
+{
+ api: 'changeStoragePoolScope',
+ icon: 'arrow-right-outlined',
+ label: 'label.action.change.primary.storage.scope',
+ dataView: true,
+ popup: true,
+ show: (record) => {
+return (record.state === 'Disabled' &&
+ (record.hypervisor === 'KVM' ||
+ record.hypervisor === 'VMware' ||
+ record.hypervisor === 'HyperV' ||
+ record.hypervisor === 'LXC' ||
+ record.hypervisor === 'Any' ||
+ record.hypervisor === 'Simulator')
Review Comment:
Then shouldn't we show the action only for the supported hypervisors? More
hypervisors can be added once we have support for them?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2071932628 @shwstppr @weizhouapache @DaanHoogland I have addressed the review comments. Please check. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1576002264
##
ui/src/config/section/infra/primaryStorages.js:
##
@@ -133,6 +133,24 @@ export default {
dataView: true,
show: (record) => { return ['Maintenance', 'PrepareForMaintenance',
'ErrorInMaintenance'].includes(record.state) }
},
+{
+ api: 'changeStoragePoolScope',
+ icon: 'arrow-right-outlined',
+ label: 'label.action.change.primary.storage.scope',
+ dataView: true,
+ popup: true,
+ show: (record) => {
+return (record.state === 'Disabled' &&
+ (record.hypervisor === 'KVM' ||
+ record.hypervisor === 'VMware' ||
+ record.hypervisor === 'HyperV' ||
+ record.hypervisor === 'LXC' ||
+ record.hypervisor === 'Any' ||
+ record.hypervisor === 'Simulator')
Review Comment:
I have updated the PR description.
This feature is tested and supported for a limited no. of storage-hypervisor
configurations.
But we are not gating this functionality for other combinations.
Hypervisors that are given here is same as the hypervisors which support
zone scope primary storage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1575990957 ## test/integration/smoke/test_primary_storage.py: ## @@ -363,6 +363,151 @@ def test_01_add_primary_storage_disabled_host(self): return +class TestPrimaryStorageScope(cloudstackTestCase): Review Comment: Moved the testcase to a new file. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1575990408
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AbstractPrimaryDataStoreLifeCycleImpl.java:
##
@@ -0,0 +1,125 @@
+// 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.storage.datastore.lifecycle;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper;
+import org.apache.log4j.Logger;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.DeleteStoragePoolCommand;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.utils.Pair;
+
+public class AbstractPrimaryDataStoreLifeCycleImpl {
+private static final Logger s_logger =
Logger.getLogger(AbstractPrimaryDataStoreLifeCycleImpl.class);
+@Inject
+AgentManager agentMgr;
+@Inject
+protected ResourceManager resourceMgr;
+@Inject
+StorageManager storageMgr;
+@Inject
+PrimaryDataStoreHelper dataStoreHelper;
+@Inject
+protected HostDao hostDao;
+@Inject
+protected StoragePoolHostDao storagePoolHostDao;
+
+private HypervisorType getHypervisorType(long hostId) {
+HostVO host = hostDao.findById(hostId);
+if (host != null)
+return host.getHypervisorType();
+return HypervisorType.None;
+}
+
+private List getPoolHostsList(ClusterScope clusterScope,
HypervisorType hypervisorType) {
+
+List hosts = new ArrayList();
+
+if (hypervisorType != null) {
+ hosts = resourceMgr
+
.listAllHostsInOneZoneNotInClusterByHypervisor(hypervisorType,
clusterScope.getZoneId(), clusterScope.getScopeId());
+} else {
+List xenServerHosts = resourceMgr
Review Comment:
No, Xenserver is not supported for zone scope primary storage. Removed it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1575988623 ## ui/src/views/infra/changeStoragePoolScope.vue: ## @@ -0,0 +1,226 @@ +// Licensed to the Apache Software Foundation (ASF) under one Review Comment: Renamed the file -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1575990408
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AbstractPrimaryDataStoreLifeCycleImpl.java:
##
@@ -0,0 +1,125 @@
+// 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.storage.datastore.lifecycle;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper;
+import org.apache.log4j.Logger;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.DeleteStoragePoolCommand;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.utils.Pair;
+
+public class AbstractPrimaryDataStoreLifeCycleImpl {
+private static final Logger s_logger =
Logger.getLogger(AbstractPrimaryDataStoreLifeCycleImpl.class);
+@Inject
+AgentManager agentMgr;
+@Inject
+protected ResourceManager resourceMgr;
+@Inject
+StorageManager storageMgr;
+@Inject
+PrimaryDataStoreHelper dataStoreHelper;
+@Inject
+protected HostDao hostDao;
+@Inject
+protected StoragePoolHostDao storagePoolHostDao;
+
+private HypervisorType getHypervisorType(long hostId) {
+HostVO host = hostDao.findById(hostId);
+if (host != null)
+return host.getHypervisorType();
+return HypervisorType.None;
+}
+
+private List getPoolHostsList(ClusterScope clusterScope,
HypervisorType hypervisorType) {
+
+List hosts = new ArrayList();
+
+if (hypervisorType != null) {
+ hosts = resourceMgr
+
.listAllHostsInOneZoneNotInClusterByHypervisor(hypervisorType,
clusterScope.getZoneId(), clusterScope.getScopeId());
+} else {
+List xenServerHosts = resourceMgr
Review Comment:
No, Xenserver is not supported for zone scope primary storage
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1575987724
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AbstractPrimaryDataStoreLifeCycleImpl.java:
##
@@ -0,0 +1,125 @@
+// 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.storage.datastore.lifecycle;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper;
+import org.apache.log4j.Logger;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.DeleteStoragePoolCommand;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.utils.Pair;
+
+public class AbstractPrimaryDataStoreLifeCycleImpl {
Review Comment:
yes, renamed the class
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1575988321
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AbstractPrimaryDataStoreLifeCycleImpl.java:
##
@@ -0,0 +1,125 @@
+// 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.storage.datastore.lifecycle;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper;
+import org.apache.log4j.Logger;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.DeleteStoragePoolCommand;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.utils.Pair;
+
+public class AbstractPrimaryDataStoreLifeCycleImpl {
+private static final Logger s_logger =
Logger.getLogger(AbstractPrimaryDataStoreLifeCycleImpl.class);
+@Inject
+AgentManager agentMgr;
+@Inject
+protected ResourceManager resourceMgr;
+@Inject
+StorageManager storageMgr;
+@Inject
+PrimaryDataStoreHelper dataStoreHelper;
+@Inject
+protected HostDao hostDao;
+@Inject
+protected StoragePoolHostDao storagePoolHostDao;
+
+private HypervisorType getHypervisorType(long hostId) {
+HostVO host = hostDao.findById(hostId);
+if (host != null)
+return host.getHypervisorType();
+return HypervisorType.None;
+}
+
+private List getPoolHostsList(ClusterScope clusterScope,
HypervisorType hypervisorType) {
+
+List hosts = new ArrayList();
+
+if (hypervisorType != null) {
+ hosts = resourceMgr
+
.listAllHostsInOneZoneNotInClusterByHypervisor(hypervisorType,
clusterScope.getZoneId(), clusterScope.getScopeId());
+} else {
+List xenServerHosts = resourceMgr
+
.listAllHostsInOneZoneNotInClusterByHypervisor(HypervisorType.XenServer,
clusterScope.getZoneId(), clusterScope.getScopeId());
+List vmWareServerHosts = resourceMgr
+
.listAllHostsInOneZoneNotInClusterByHypervisor(HypervisorType.VMware,
clusterScope.getZoneId(), clusterScope.getScopeId());
+List kvmHosts = resourceMgr.
+
listAllHostsInOneZoneNotInClusterByHypervisor(HypervisorType.KVM,
clusterScope.getZoneId(), clusterScope.getScopeId());
Review Comment:
Added new method
##
plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java:
##
@@ -146,4 +146,14 @@ public void enableStoragePool(DataStore store) {
@Override
public void disableStoragePool(DataStore store) {
}
+
+@Override
+public void changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+return;
Review Comment:
removed the return statement
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1575987312
##
engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java:
##
@@ -841,4 +866,13 @@ public List listByIds(List ids) {
sc.setParameters("idIN", ids.toArray());
return listBy(sc, null);
}
+
+@Override
+public List listByPoolIdVMStatesNotInCluster(long clusterId,
List states, long poolId) {
+SearchCriteria sc = volumePoolNotInClusterSearch.create();
+sc.setParameters("poolId", poolId);
+sc.setParameters("vmStates", states);
Review Comment:
Yes, made the change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1574654750 ## ui/src/views/infra/changeStoragePoolScope.vue: ## @@ -0,0 +1,226 @@ +// 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. + + + + + + + + + + + + + + + + Review Comment: Its working :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
github-actions[bot] commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2069250234 This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
shwstppr commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1574592360 ## ui/src/views/infra/changeStoragePoolScope.vue: ## @@ -0,0 +1,226 @@ +// 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. + + + + + + + + + + + + + + + + Review Comment: Not sure but can it be something like, ``` ``` Need to check this actually works :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
DaanHoogland commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1574579125 ## test/integration/smoke/test_primary_storage.py: ## @@ -363,6 +363,151 @@ def test_01_add_primary_storage_disabled_host(self): return +class TestPrimaryStorageScope(cloudstackTestCase): Review Comment: I see your point and want to add that for maintainability it is preferable to have a single test class per file. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1574530881 ## ui/src/views/infra/changeStoragePoolScope.vue: ## @@ -0,0 +1,226 @@ +// 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. + + + + + + + + + + + + + + + + Review Comment: message.action.primary.storage.scope.cluster is specific to cluster. message.warn.change.primary.storage.scope is a generic msg about changing the storage pool scope (zone and cluster). Is there a way we can combine these msgs into a single warning box? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
rohityadavcloud commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2065938481 @abh1sar can you - pull or merge the latest 4.19 on your PR branch - resolve all review comments you've addressed - address any other outstanding remark(s) - ask for a re-review from reviewers whose comments you've addressed thnx -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
shwstppr commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1565422034
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AbstractPrimaryDataStoreLifeCycleImpl.java:
##
@@ -0,0 +1,125 @@
+// 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.storage.datastore.lifecycle;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper;
+import org.apache.log4j.Logger;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.DeleteStoragePoolCommand;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.utils.Pair;
+
+public class AbstractPrimaryDataStoreLifeCycleImpl {
Review Comment:
minor: would it make sense to name it BasePrimaryDataStoreLifeCycleImpl
instead?
##
plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java:
##
@@ -146,4 +146,14 @@ public void enableStoragePool(DataStore store) {
@Override
public void disableStoragePool(DataStore store) {
}
+
+@Override
+public void changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+return;
Review Comment:
Not sure if these return statement add much value
##
engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java:
##
@@ -841,4 +866,13 @@ public List listByIds(List ids) {
sc.setParameters("idIN", ids.toArray());
return listBy(sc, null);
}
+
+@Override
+public List listByPoolIdVMStatesNotInCluster(long clusterId,
List states, long poolId) {
+SearchCriteria sc = volumePoolNotInClusterSearch.create();
+sc.setParameters("poolId", poolId);
+sc.setParameters("vmStates", states);
Review Comment:
Should this use `setJoinParameters`?
##
ui/src/config/section/infra/primaryStorages.js:
##
@@ -133,6 +133,24 @@ export default {
dataView: true,
show: (record) => { return ['Maintenance', 'PrepareForMaintenance',
'ErrorInMaintenance'].includes(record.state) }
},
+{
+ api: 'changeStoragePoolScope',
+ icon: 'arrow-right-outlined',
+ label: 'label.action.change.primary.storage.scope',
+ dataView: true,
+ popup: true,
+ show: (record) => {
+return (record.state === 'Disabled' &&
+ (record.hypervisor === 'KVM' ||
+ record.hypervisor === 'VMware' ||
+ record.hypervisor === 'HyperV' ||
+ record.hypervisor === 'LXC' ||
+ record.hypervisor === 'Any' ||
+ record.hypervisor === 'Simulator')
Review Comment:
PR description says feature is supported for KVM, VMware and Simulator only?
##
ui/src/views/infra/changeStoragePoolScope.vue:
##
@@ -0,0 +1,226 @@
+// 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
+//
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1562038355
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1151,93 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public void changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd) throws
IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType newScope = EnumUtils.getEnumIgnoreCase(ScopeType.class,
cmd.getScope());
+if (newScope != ScopeType.ZONE && newScope != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
newScope.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if (!primaryStorage.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope of the Primary
storage with id "
++ primaryStorage.getUuid() +
+" cannot be changed, as it is not in the Disabled state");
+}
+
+ScopeType currentScope = primaryStorage.getScope();
+if (currentScope.equals(newScope)) {
+throw new InvalidParameterValueException("New scope must be
different than the current scope");
+}
+
+HypervisorType hypervisorType;
+if (currentScope.equals(ScopeType.CLUSTER)) {
+/*
+ * For cluster wide primary storage the hypervisor type might not
be set.
+ * So, get it from the clusterVO.
+ */
+Long clusterId = primaryStorage.getClusterId();
+ClusterVO clusterVO = _clusterDao.findById(clusterId);
+hypervisorType = clusterVO.getHypervisorType();
+} else {
+hypervisorType = primaryStorage.getHypervisor();
+}
+if (!zoneWidePoolSupportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+String providerName = primaryStorage.getStorageProviderName();
+DataStoreProvider storeProvider =
_dataStoreProviderMgr.getDataStoreProvider(providerName);
+PrimaryDataStoreLifeCycle lifeCycle = (PrimaryDataStoreLifeCycle)
storeProvider.getDataStoreLifeCycle();
+DataStore primaryStore = _dataStoreMgr.getPrimaryDataStore(id);
+
+Long zoneId = primaryStorage.getDataCenterId();
+DataCenterVO zone = _dcDao.findById(zoneId);
+if (zone == null) {
+throw new InvalidParameterValueException("Unable to find zone by
id " + zoneId);
+}
+if
(zone.getAllocationState().equals(Grouping.AllocationState.Disabled)) {
+throw new PermissionDeniedException("Cannot perform this
operation, Zone is currently disabled: " + zoneId);
+}
+
+if (newScope.equals(ScopeType.ZONE)) {
+ClusterScope clusterScope = new
ClusterScope(primaryStorage.getClusterId(), null, zoneId);
+lifeCycle.changeStoragePoolScopeToZone(primaryStore, clusterScope,
hypervisorType);
+
+} else {
+Long clusterId = cmd.getClusterId();
+if (clusterId == null) {
+throw new IllegalArgumentException("Unable to find cluster
with ID: " + clusterId);
+}
+ClusterVO clusterVO = _clusterDao.findById(clusterId);
+if (clusterVO == null) {
+throw new InvalidParameterValueException("Unable to find
cluster by id " + clusterId);
+}
+if
(clusterVO.getAllocationState().equals(Grouping.AllocationState.Disabled)) {
+throw new PermissionDeniedException("Cannot perform this
operation, Cluster is currently disabled: " + zoneId);
+}
+List states = Arrays.asList(State.Starting,
State.Running, State.Stopping, State.Migrating, State.Restoring);
+List volumesInOtherClusters =
volumeDao.listByPoolIdVMStatesNotInCluster(clusterId, states, id);
+if (volumesInOtherClusters.size() > 0) {
+throw new CloudRuntimeException("Cannot change scope of the
pool " + primaryStorage.getName() + " to cluster " + clusterVO.getName() + " as
there are associated volumes present for other clusters");
Review Comment:
Done
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1561803442
##
plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/DateraPrimaryDataStoreLifeCycle.java:
##
@@ -395,6 +395,15 @@ public void disableStoragePool(DataStore dataStore) {
dataStoreHelper.disable(dataStore);
}
+@Override
+public void changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+/*
+ * We need to attach all VMware, Xenserver and KVM hosts in the zone.
+ * So pass hypervisorTpe as null.
+ */
+super.changeStoragePoolScopeToZone(store, clusterScope, null);
Review Comment:
> Feature is not tested and supported on Datera. But if a root admin wishes,
he can use it. We are not preventing any kind of storage from using this
feature. The feature will make the DB updates and connect/disconnect the hosts
to the storage pool as required. It is not ensured that it will work with
configurations other than NFS/Ceph, KVM/VMware and Default Primary storage
provider.
> Have mentioned this as a warning in UI and Api docs.
Fair enough
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1561357090
##
plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/DateraPrimaryDataStoreLifeCycle.java:
##
@@ -395,6 +395,15 @@ public void disableStoragePool(DataStore dataStore) {
dataStoreHelper.disable(dataStore);
}
+@Override
+public void changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+/*
+ * We need to attach all VMware, Xenserver and KVM hosts in the zone.
+ * So pass hypervisorTpe as null.
+ */
+super.changeStoragePoolScopeToZone(store, clusterScope, null);
Review Comment:
Feature is not tested and supported on Datera. But if a root admin wishes,
he can use it. We are not preventing any kind of storage from using this
feature. The feature will make the DB updates and connect/disconnect the hosts
to the storage pool as required. It is not ensured that it will work with
configurations other than NFS/Ceph, KVM/VMware and Default Primary storage
provider.
Have mentioned this as a warning in UI and Api docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1560975816
##
plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/DateraPrimaryDataStoreLifeCycle.java:
##
@@ -395,6 +395,15 @@ public void disableStoragePool(DataStore dataStore) {
dataStoreHelper.disable(dataStore);
}
+@Override
+public void changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+/*
+ * We need to attach all VMware, Xenserver and KVM hosts in the zone.
+ * So pass hypervisorTpe as null.
+ */
+super.changeStoragePoolScopeToZone(store, clusterScope, null);
Review Comment:
is this feature enabled for Detera storage ?
or this will throw an exception ?
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1151,93 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public void changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd) throws
IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType newScope = EnumUtils.getEnumIgnoreCase(ScopeType.class,
cmd.getScope());
+if (newScope != ScopeType.ZONE && newScope != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
newScope.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if (!primaryStorage.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope of the Primary
storage with id "
++ primaryStorage.getUuid() +
+" cannot be changed, as it is not in the Disabled state");
+}
+
+ScopeType currentScope = primaryStorage.getScope();
+if (currentScope.equals(newScope)) {
+throw new InvalidParameterValueException("New scope must be
different than the current scope");
+}
+
+HypervisorType hypervisorType;
+if (currentScope.equals(ScopeType.CLUSTER)) {
+/*
+ * For cluster wide primary storage the hypervisor type might not
be set.
+ * So, get it from the clusterVO.
+ */
+Long clusterId = primaryStorage.getClusterId();
+ClusterVO clusterVO = _clusterDao.findById(clusterId);
+hypervisorType = clusterVO.getHypervisorType();
+} else {
+hypervisorType = primaryStorage.getHypervisor();
+}
+if (!zoneWidePoolSupportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+String providerName = primaryStorage.getStorageProviderName();
+DataStoreProvider storeProvider =
_dataStoreProviderMgr.getDataStoreProvider(providerName);
+PrimaryDataStoreLifeCycle lifeCycle = (PrimaryDataStoreLifeCycle)
storeProvider.getDataStoreLifeCycle();
+DataStore primaryStore = _dataStoreMgr.getPrimaryDataStore(id);
+
+Long zoneId = primaryStorage.getDataCenterId();
+DataCenterVO zone = _dcDao.findById(zoneId);
+if (zone == null) {
+throw new InvalidParameterValueException("Unable to find zone by
id " + zoneId);
+}
+if
(zone.getAllocationState().equals(Grouping.AllocationState.Disabled)) {
+throw new PermissionDeniedException("Cannot perform this
operation, Zone is currently disabled: " + zoneId);
+}
+
+if (newScope.equals(ScopeType.ZONE)) {
+ClusterScope clusterScope = new
ClusterScope(primaryStorage.getClusterId(), null, zoneId);
+lifeCycle.changeStoragePoolScopeToZone(primaryStore, clusterScope,
hypervisorType);
+
+} else {
+Long clusterId = cmd.getClusterId();
+if (clusterId == null) {
+throw new IllegalArgumentException("Unable to find cluster
with ID: " + clusterId);
+}
+ClusterVO clusterVO = _clusterDao.findById(clusterId);
+if (clusterVO == null) {
+throw new InvalidParameterValueException("Unable to find
cluster by id " + clusterId);
+}
+if
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2048957224 @rohityadavcloud Gentle reminder for review. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2046936550 [SF] Trillian test result (tid-9766) Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8 Total time taken: 51815 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8875-t9766-kvm-rocky8.zip Smoke tests completed. 129 look OK, 0 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2045787244 @weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2045782845 @blueorangutan test rocky8 kvm-rocky8 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2045324068 [SF] Trillian Build Failed (tid-9765) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2045277341 @weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2045274696 @blueorangutan test rocky8 kvm-rocky8 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2045196429 Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9203 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
weizhouapache commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2045073954 @blueorangutan package -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2045074757 @weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1557451128
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1151,95 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
Review Comment:
made it void
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1557450366
##
engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java:
##
@@ -72,8 +79,14 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol
protected GenericSearchBuilder primaryStorageSearch2;
protected GenericSearchBuilder secondaryStorageSearch;
private final SearchBuilder poolAndPathSearch;
+protected SearchBuilder volumePoolNotInClusterSearch;
+
@Inject
ResourceTagDao _tagsDao;
+@Inject
+HostDao _hostDao;
+@Inject
+VMInstanceDao _vmDao;
Review Comment:
Done
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AbstractPrimaryDataStoreLifeCycleImpl.java:
##
@@ -0,0 +1,127 @@
+// 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.storage.datastore.lifecycle;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper;
+import org.apache.log4j.Logger;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.DeleteStoragePoolCommand;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.utils.Pair;
+
+public class AbstractPrimaryDataStoreLifeCycleImpl {
+private static final Logger s_logger =
Logger.getLogger(AbstractPrimaryDataStoreLifeCycleImpl.class);
+@Inject
+AgentManager agentMgr;
+@Inject
+protected ResourceManager _resourceMgr;
+@Inject
+StorageManager storageMgr;
+@Inject
+PrimaryDataStoreHelper dataStoreHelper;
+@Inject
+protected HostDao _hostDao;
+@Inject
+protected StoragePoolHostDao _storagePoolHostDao;
Review Comment:
Done
##
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AbstractPrimaryDataStoreLifeCycleImpl.java:
##
@@ -0,0 +1,127 @@
+// 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.storage.datastore.lifecycle;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper;
+import org.apache.log4j.Logger;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.DeleteStoragePoolCommand;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.StorageManager;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolHostVO;
+import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.utils.Pair;
+
+public class
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1557452273
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1151,95 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType newScope = EnumUtils.getEnumIgnoreCase(ScopeType.class,
cmd.getScope());
+if (newScope != ScopeType.ZONE && newScope != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
newScope.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if (!primaryStorage.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope of the Primary
storage with id "
++ primaryStorage.getUuid() +
+" cannot be changed, as it is not in the Disabled state");
+}
+
+ScopeType currentScope = primaryStorage.getScope();
+if (currentScope.equals(newScope)) {
+throw new InvalidParameterValueException("New scope must be
different than the current scope");
+}
+
+HypervisorType hypervisorType;
+if (currentScope.equals(ScopeType.CLUSTER)) {
+/*
+ * For cluster wide primary storage the hypervisor type might not
be set.
+ * So, get it from the clusterVO.
+ */
+Long clusterId = primaryStorage.getClusterId();
+ClusterVO clusterVO = _clusterDao.findById(clusterId);
+hypervisorType = clusterVO.getHypervisorType();
+} else {
+hypervisorType = primaryStorage.getHypervisor();
+}
+if (!zoneWidePoolSupportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+String providerName = primaryStorage.getStorageProviderName();
+DataStoreProvider storeProvider =
_dataStoreProviderMgr.getDataStoreProvider(providerName);
+PrimaryDataStoreLifeCycle lifeCycle = (PrimaryDataStoreLifeCycle)
storeProvider.getDataStoreLifeCycle();
+DataStore primaryStore = _dataStoreMgr.getPrimaryDataStore(id);
+
+Long zoneId = primaryStorage.getDataCenterId();
+DataCenterVO zone = _dcDao.findById(zoneId);
+if (zone == null) {
+throw new InvalidParameterValueException("Unable to find zone by
id " + zoneId);
+}
+if (Grouping.AllocationState.Disabled == zone.getAllocationState()) {
+throw new PermissionDeniedException("Cannot perform this
operation, Zone is currently disabled: " + zoneId);
+}
+
+if (newScope.equals(ScopeType.ZONE)) {
+ClusterScope clusterScope = new
ClusterScope(primaryStorage.getClusterId(), null, zoneId);
+lifeCycle.changeStoragePoolScopeToZone(primaryStore, clusterScope,
hypervisorType);
+
+} else {
+Long clusterId = cmd.getClusterId();
+if (clusterId == null) {
+throw new IllegalArgumentException("Unable to find cluster
with ID: " + clusterId);
+}
+ClusterVO clusterVO = _clusterDao.findById(clusterId);
+if (clusterVO == null) {
+throw new InvalidParameterValueException("Unable to find
cluster by id " + clusterId);
+}
+if (Grouping.AllocationState.Disabled ==
clusterVO.getAllocationState()) {
+throw new PermissionDeniedException("Cannot perform this
operation, Cluster is currently disabled: " + zoneId);
+}
+List states = Arrays.asList(State.Starting,
State.Running, State.Stopping, State.Migrating, State.Restoring);
+List volumesInOtherClusters =
volumeDao.listByPoolIdVMStatesNotInCluster(clusterId, states, id);
+if (volumesInOtherClusters.size() > 0) {
+throw new CloudRuntimeException("Cannot change scope of the
pool " + primaryStorage.getName() + " to cluster " + clusterVO.getName() + " as
there are associated volumes present for other clusters");
+}
+
+
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1557451734
##
test/integration/smoke/test_primary_storage.py:
##
@@ -711,4 +854,4 @@ def test_03_migration_options_storage_tags(self):
self.debug("Suitable storage pools found: %s" % len(pools_suitable))
self.assertEqual(0, len(pools_suitable), "Check that there is no
migration option for volume")
-return
+return
Review Comment:
Thanks for pointing it. Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
BryanMLima commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1555813585
##
engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java:
##
@@ -72,8 +79,14 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol
protected GenericSearchBuilder primaryStorageSearch2;
protected GenericSearchBuilder secondaryStorageSearch;
private final SearchBuilder poolAndPathSearch;
+protected SearchBuilder volumePoolNotInClusterSearch;
+
@Inject
ResourceTagDao _tagsDao;
+@Inject
+HostDao _hostDao;
+@Inject
+VMInstanceDao _vmDao;
Review Comment:
I would encourage to not use the old pattern of using `_` for private class
variables, as it is not part of the Java naming conventions. TBH, we should
probably update the CloudStack [coding
conventions](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions)
to reflect this as well.
##
test/integration/smoke/test_primary_storage.py:
##
@@ -711,4 +854,4 @@ def test_03_migration_options_storage_tags(self):
self.debug("Suitable storage pools found: %s" % len(pools_suitable))
self.assertEqual(0, len(pools_suitable), "Check that there is no
migration option for volume")
-return
+return
Review Comment:
The pre-commit lint is failing because of the removed line at the EOF.
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1151,95 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
Review Comment:
Same here for this method, it only returns true or throws an exception.
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1151,95 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType newScope = EnumUtils.getEnumIgnoreCase(ScopeType.class,
cmd.getScope());
+if (newScope != ScopeType.ZONE && newScope != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
newScope.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if (!primaryStorage.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope of the Primary
storage with id "
++ primaryStorage.getUuid() +
+" cannot be changed, as it is not in the Disabled state");
+}
+
+ScopeType currentScope = primaryStorage.getScope();
+if (currentScope.equals(newScope)) {
+throw new InvalidParameterValueException("New scope must be
different than the current scope");
+}
+
+HypervisorType hypervisorType;
+if (currentScope.equals(ScopeType.CLUSTER)) {
+/*
+ * For cluster wide primary storage the hypervisor type might not
be set.
+ * So, get it from the clusterVO.
+ */
+Long clusterId = primaryStorage.getClusterId();
+ClusterVO clusterVO = _clusterDao.findById(clusterId);
+hypervisorType = clusterVO.getHypervisorType();
+} else {
+hypervisorType = primaryStorage.getHypervisor();
+}
+if (!zoneWidePoolSupportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+String providerName = primaryStorage.getStorageProviderName();
+DataStoreProvider storeProvider =
_dataStoreProviderMgr.getDataStoreProvider(providerName);
+PrimaryDataStoreLifeCycle lifeCycle = (PrimaryDataStoreLifeCycle)
storeProvider.getDataStoreLifeCycle();
+DataStore primaryStore = _dataStoreMgr.getPrimaryDataStore(id);
+
+Long zoneId =
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2040987249 In a discussion with @rohityadavcloud and @andrijapanicsb it was decided to not gate this feature for other hypervisor and storage types. We will give the root admin the option to change the scope, but by giving enough warning that officially this feature is only tested for a limited set of hypervisor-storage configuration. And other configurations might not work and need some extra steps. We will give this information in the UI prompt as well as Api docs. Apache CloudStack documentation will also get a mention on what hypervisors/storage is supported. So, I have removed the checks in UI and `StorageManagerImpl`, and added proper warning. Only check present is for hypervisorType which is same as the hypervisor types currently supported for Zone wide primary storage. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554533244
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
Review Comment:
Now we are using the same hypervisorTypes as in createPool(). Have extracted
that as a class attribute.
##
ui/public/locales/en.json:
##
@@ -2447,6 +2448,8 @@
"message.action.patch.router": "Please confirm that you want to live patch the
router. This operation is equivalent updating the router packages and
restarting the Network without cleanup.",
"message.action.patch.systemvm": "Please confirm that you want to patch the
System VM.",
"message.action.primarystorage.enable.maintenance.mode": "Warning: placing the
primary storage into maintenance mode will cause all Instances using volumes
from it to be stopped. Do you want to continue?",
+"message.action.primary.storage.scope.cluster": "Change the primary storage
scope from zone to cluster",
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554533138
##
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java:
##
@@ -543,4 +544,45 @@ public void enableStoragePool(DataStore dataStore) {
public void disableStoragePool(DataStore dataStore) {
dataStoreHelper.disable(dataStore);
}
-}
+
+public boolean changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+List hosts =
_resourceMgr.listAllHostsInOneZoneNotInClusterByHypervisor(hypervisorType,
clusterScope.getZoneId(), clusterScope.getScopeId());
+s_logger.debug("In createPool. Attaching the pool to each of the
hosts.");
+List poolHosts = new ArrayList();
Review Comment:
Removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554533111
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
Review Comment:
Done
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
+if (!supportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+if (StoragePoolStatus.Disabled != primaryStorage.getStatus()) {
+throw new InvalidParameterValueException("Primary storage should
be disabled for this operation");
+}
+
+if (scopeType == primaryStorage.getScope()) {
+throw new InvalidParameterValueException("Invalid scope change");
+}
+
+if (!primaryStorage.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope of the Primary
storage with id "
++ primaryStorage.getUuid() +
+" cannot be changed, as it is not in the Disabled state");
+}
+
+String providerName = primaryStorage.getStorageProviderName();
+DataStoreProvider storeProvider =
_dataStoreProviderMgr.getDataStoreProvider(providerName);
+PrimaryDataStoreLifeCycle lifeCycle = (PrimaryDataStoreLifeCycle)
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554533096
##
engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java:
##
@@ -841,4 +866,13 @@ public List listByIds(List ids) {
sc.setParameters("idIN", ids.toArray());
return listBy(sc, null);
}
+
+@Override
+public List listByPoolIdVMStatesNotInCluster(long clusterId,
List states, long poolId) {
+SearchCriteria sc = volumePoolNotInClusterSearch.create();
+sc.setParameters("poolId", poolId);
+sc.setParameters("states", states);
Review Comment:
There was a bug here in the way I was using the conditionName.
Have fixed and tested. It seems to be working. Please look
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554532786
##
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java:
##
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.storage;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.event.EventTypes;
+import com.cloud.storage.StorageService;
+
+@APICommand(name = "changeStoragePoolScope", description = "Changes the scope
of a storage pool.", responseObject = SuccessResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class ChangeStoragePoolScopeCmd extends BaseAsyncCmd {
+
+@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType =
StoragePoolResponse.class, required = true, description = "the Id of the
storage pool")
+private Long id;
+
+@Parameter(name = ApiConstants.SCOPE, type = CommandType.STRING, required
= true, description = "the scope of the storage: cluster or zone")
+private String scope;
+
+@Parameter(name = ApiConstants.CLUSTER_ID, type = CommandType.UUID,
entityType = ClusterResponse.class, description = "the CLuster ID to use for
the storage pool's scope")
+private Long clusterId;
+
+@Inject
+public StorageService _storageService;
+
+@Override
+public String getEventType() {
+return EventTypes.EVENT_CHANGE_STORAGE_POOL_SCOPE;
+}
+
+@Override
+public String getEventDescription() {
+return "Change Storage Pool Scope";
Review Comment:
Added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554532751
##
engine/storage/src/main/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java:
##
@@ -265,4 +268,45 @@ public boolean deletePrimaryDataStore(DataStore store) {
return true;
}
-}
+public void switchToZone(DataStore store) {
+StoragePoolVO pool = dataStoreDao.findById(store.getId());
+CapacityVO capacity = _capacityDao.findByHostIdType(store.getId(),
Capacity.CAPACITY_TYPE_STORAGE_ALLOCATED);
+Transaction.execute(new TransactionCallbackNoReturn() {
+public void doInTransactionWithoutResult(TransactionStatus status)
{
+pool.setScope(ScopeType.ZONE);
+pool.setPodId(null);
+pool.setClusterId(null);
+dataStoreDao.update(pool.getId(), pool);
Review Comment:
I realized was getting the hypervisor type for cluster scope primary storage
from the storage_pool table, which might not have the hypervisor field set.
So have changed that code too. Now I am getting the hypervisor type from
clusterVO.
##
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java:
##
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.storage;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.event.EventTypes;
+import com.cloud.storage.StorageService;
+
+@APICommand(name = "changeStoragePoolScope", description = "Changes the scope
of a storage pool.", responseObject = SuccessResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
Review Comment:
Done
##
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java:
##
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.storage;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.event.EventTypes;
+import com.cloud.storage.StorageService;
+
+@APICommand(name = "changeStoragePoolScope", description = "Changes the scope
of a storage pool.", responseObject = SuccessResponse.class,
Review Comment:
Added
--
This is an automated message from the Apache Git Service.
To respond to the
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554532435
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
+if (!supportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+if (StoragePoolStatus.Disabled != primaryStorage.getStatus()) {
+throw new InvalidParameterValueException("Primary storage should
be disabled for this operation");
+}
+
+if (scopeType == primaryStorage.getScope()) {
Review Comment:
Done
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
+if (!supportedHypervisorTypes.contains(hypervisorType)) {
+throw
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554532474
##
plugins/storage/volume/adaptive/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/AdaptiveDataStoreLifeCycleImpl.java:
##
@@ -404,4 +404,14 @@ public void disableStoragePool(DataStore store) {
s_logger.info("Disabling storage pool [" + store.getName() + "]");
_dataStoreHelper.disable(store);
}
+
+@Override
+public boolean changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
Review Comment:
Thanks for the idea. I have implemented it this way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554532347
##
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java:
##
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.storage;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.event.EventTypes;
+import com.cloud.storage.StorageService;
+
+@APICommand(name = "changeStoragePoolScope", description = "Changes the scope
of a storage pool.", responseObject = SuccessResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class ChangeStoragePoolScopeCmd extends BaseAsyncCmd {
+
+@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType =
StoragePoolResponse.class, required = true, description = "the Id of the
storage pool")
+private Long id;
+
+@Parameter(name = ApiConstants.SCOPE, type = CommandType.STRING, required
= true, description = "the scope of the storage: cluster or zone")
+private String scope;
+
+@Parameter(name = ApiConstants.CLUSTER_ID, type = CommandType.UUID,
entityType = ClusterResponse.class, description = "the CLuster ID to use for
the storage pool's scope")
+private Long clusterId;
+
+@Inject
+public StorageService _storageService;
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554532368
##
api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ChangeStoragePoolScopeCmd.java:
##
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.api.command.admin.storage;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ClusterResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.event.EventTypes;
+import com.cloud.storage.StorageService;
+
+@APICommand(name = "changeStoragePoolScope", description = "Changes the scope
of a storage pool.", responseObject = SuccessResponse.class,
+requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class ChangeStoragePoolScopeCmd extends BaseAsyncCmd {
+
+@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType =
StoragePoolResponse.class, required = true, description = "the Id of the
storage pool")
+private Long id;
+
+@Parameter(name = ApiConstants.SCOPE, type = CommandType.STRING, required
= true, description = "the scope of the storage: cluster or zone")
+private String scope;
+
+@Parameter(name = ApiConstants.CLUSTER_ID, type = CommandType.UUID,
entityType = ClusterResponse.class, description = "the CLuster ID to use for
the storage pool's scope")
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1554532232
##
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java:
##
@@ -543,4 +544,45 @@ public void enableStoragePool(DataStore dataStore) {
public void disableStoragePool(DataStore dataStore) {
dataStoreHelper.disable(dataStore);
}
-}
+
+public boolean changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+List hosts =
_resourceMgr.listAllHostsInOneZoneNotInClusterByHypervisor(hypervisorType,
clusterScope.getZoneId(), clusterScope.getScopeId());
+s_logger.debug("In createPool. Attaching the pool to each of the
hosts.");
+List poolHosts = new ArrayList();
+for (HostVO host : hosts) {
+try {
+storageMgr.connectHostToSharedPool(host.getId(),
store.getId());
+} catch (Exception e) {
+s_logger.warn("Unable to establish a connection between " +
host + " and " + store, e);
Review Comment:
The Host might be in switched off state. In that case when it comes up, the
agent will get the storage pool information from the db and mount the storage
pool according to the new zone scope. So, we don't need to fail the operation.
This behaviour is similar to when we are creating a new storage pool and
connecting it to each host (`CloudStackPrimaryDataStoreLifeCycleImpl ->
attachZone)`
##
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java:
##
@@ -543,4 +544,45 @@ public void enableStoragePool(DataStore dataStore) {
public void disableStoragePool(DataStore dataStore) {
dataStoreHelper.disable(dataStore);
}
-}
+
+public boolean changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+List hosts =
_resourceMgr.listAllHostsInOneZoneNotInClusterByHypervisor(hypervisorType,
clusterScope.getZoneId(), clusterScope.getScopeId());
+s_logger.debug("In createPool. Attaching the pool to each of the
hosts.");
+List poolHosts = new ArrayList();
+for (HostVO host : hosts) {
+try {
+storageMgr.connectHostToSharedPool(host.getId(),
store.getId());
+} catch (Exception e) {
+s_logger.warn("Unable to establish a connection between " +
host + " and " + store, e);
+}
+}
+dataStoreHelper.switchToZone(store);
+return true;
+}
+
+public boolean changeStoragePoolScopeToCluster(DataStore store,
ClusterScope clusterScope, HypervisorType hypervisorType) {
+Pair, Integer> hostPoolRecords =
_storagePoolHostDao.listByPoolIdNotInCluster(clusterScope.getScopeId(),
store.getId());
+HypervisorType hType = null;
+if (hostPoolRecords.second() > 0) {
+hType =
getHypervisorType(hostPoolRecords.first().get(0).getHostId());
+}
+
+StoragePool pool = (StoragePool) store;
+for (StoragePoolHostVO host : hostPoolRecords.first()) {
+DeleteStoragePoolCommand deleteCmd = new
DeleteStoragePoolCommand(pool);
+final Answer answer = agentMgr.easySend(host.getHostId(),
deleteCmd);
+
+if (answer != null && answer.getResult()) {
+if (HypervisorType.KVM != hType) {
+break;
+}
+} else {
+if (answer != null) {
+s_logger.debug("Failed to delete storage pool: " +
answer.getResult());
Review Comment:
Similar to the above comment. We shouldn't fail the operation similar to
`CloudStackPrimaryDataStoreLifeCycleImpl -> deleteDataStore`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
blueorangutan commented on PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#issuecomment-2035817174 [SF] Trillian test result (tid-9682) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 52569 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8875-t9682-kvm-centos7.zip Smoke tests completed. 127 look OK, 2 have errors, 0 did not run Only failed and skipped tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_01_primary_storage_scope_change | `Error` | 0.06 | test_primary_storage.py test_02_trigger_shutdown | `Failure` | 341.66 | test_safe_shutdown.py -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1550306580
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
+if (!supportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+if (StoragePoolStatus.Disabled != primaryStorage.getStatus()) {
+throw new InvalidParameterValueException("Primary storage should
be disabled for this operation");
+}
+
+if (scopeType == primaryStorage.getScope()) {
+throw new InvalidParameterValueException("Invalid scope change");
+}
+
+if (!primaryStorage.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope of the Primary
storage with id "
++ primaryStorage.getUuid() +
+" cannot be changed, as it is not in the Disabled state");
+}
+
+String providerName = primaryStorage.getStorageProviderName();
+DataStoreProvider storeProvider =
_dataStoreProviderMgr.getDataStoreProvider(providerName);
+PrimaryDataStoreLifeCycle lifeCycle = (PrimaryDataStoreLifeCycle)
storeProvider.getDataStoreLifeCycle();
+DataStore primaryStore = _dataStoreMgr.getPrimaryDataStore(id);
+
+Long zoneId = primaryStorage.getDataCenterId();
+DataCenterVO zone = _dcDao.findById(zoneId);
+if (zone == null) {
+throw new InvalidParameterValueException("unable to find zone by
id " + zoneId);
+}
+if (Grouping.AllocationState.Disabled == zone.getAllocationState()) {
+throw new PermissionDeniedException("Cannot perform this
operation, Zone is currently disabled: " + zoneId);
+}
+
+if (scopeType == ScopeType.ZONE) {
+ClusterScope clusterScope = new
ClusterScope(primaryStorage.getClusterId(), null, zoneId);
+lifeCycle.changeStoragePoolScopeToZone(primaryStore, clusterScope,
hypervisorType);
+
+} else {
+
+Long clusterId = cmd.getClusterId();
+ClusterVO clusterVO = _clusterDao.findById(clusterId);
Review Comment:
no, will add some checks. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1550305738
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
+if (!supportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+if (StoragePoolStatus.Disabled != primaryStorage.getStatus()) {
+throw new InvalidParameterValueException("Primary storage should
be disabled for this operation");
+}
+
+if (scopeType == primaryStorage.getScope()) {
+throw new InvalidParameterValueException("Invalid scope change");
Review Comment:
Will do, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1550305428
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
+if (!supportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+if (StoragePoolStatus.Disabled != primaryStorage.getStatus()) {
Review Comment:
Will remove one of them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1550306216
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
+if (!supportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+if (StoragePoolStatus.Disabled != primaryStorage.getStatus()) {
+throw new InvalidParameterValueException("Primary storage should
be disabled for this operation");
+}
+
+if (scopeType == primaryStorage.getScope()) {
+throw new InvalidParameterValueException("Invalid scope change");
+}
+
+if (!primaryStorage.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope of the Primary
storage with id "
++ primaryStorage.getUuid() +
+" cannot be changed, as it is not in the Disabled state");
+}
+
+String providerName = primaryStorage.getStorageProviderName();
+DataStoreProvider storeProvider =
_dataStoreProviderMgr.getDataStoreProvider(providerName);
+PrimaryDataStoreLifeCycle lifeCycle = (PrimaryDataStoreLifeCycle)
storeProvider.getDataStoreLifeCycle();
+DataStore primaryStore = _dataStoreMgr.getPrimaryDataStore(id);
+
+Long zoneId = primaryStorage.getDataCenterId();
+DataCenterVO zone = _dcDao.findById(zoneId);
+if (zone == null) {
+throw new InvalidParameterValueException("unable to find zone by
id " + zoneId);
+}
+if (Grouping.AllocationState.Disabled == zone.getAllocationState()) {
+throw new PermissionDeniedException("Cannot perform this
operation, Zone is currently disabled: " + zoneId);
+}
+
+if (scopeType == ScopeType.ZONE) {
+ClusterScope clusterScope = new
ClusterScope(primaryStorage.getClusterId(), null, zoneId);
+lifeCycle.changeStoragePoolScopeToZone(primaryStore, clusterScope,
hypervisorType);
+
+} else {
+
+Long clusterId = cmd.getClusterId();
Review Comment:
Correct, will add validation. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1550300021
##
engine/storage/src/main/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java:
##
@@ -265,4 +268,45 @@ public boolean deletePrimaryDataStore(DataStore store) {
return true;
}
-}
+public void switchToZone(DataStore store) {
+StoragePoolVO pool = dataStoreDao.findById(store.getId());
+CapacityVO capacity = _capacityDao.findByHostIdType(store.getId(),
Capacity.CAPACITY_TYPE_STORAGE_ALLOCATED);
+Transaction.execute(new TransactionCallbackNoReturn() {
+public void doInTransactionWithoutResult(TransactionStatus status)
{
+pool.setScope(ScopeType.ZONE);
+pool.setPodId(null);
+pool.setClusterId(null);
+dataStoreDao.update(pool.getId(), pool);
Review Comment:
Will add
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1550285999
##
ui/src/config/section/infra/primaryStorages.js:
##
@@ -133,12 +133,27 @@ export default {
dataView: true,
show: (record) => { return ['Maintenance', 'PrepareForMaintenance',
'ErrorInMaintenance'].includes(record.state) }
},
+{
+ api: 'changeStoragePoolScope',
+ icon: 'arrow-right-outlined',
+ label: 'label.action.change.primary.storage.scope',
+ dataView: true,
+ popup: true,
+ show: (record) => {
+return (
+ record.state === 'Disabled' &&
+ (record.hypervisor === 'Simulator' || record.hypervisor === 'KVM' ||
record.hypervisor === 'VMware') &&
+ (record.type === 'NetworkFilesystem' || record.type === 'RBD') &&
+ record.provider === 'DefaultPrimary'
+)
+ },
+ component: shallowRef(defineAsyncComponent(() =>
import('@/views/infra/changeStoragePoolScope.vue')))
+},
{
api: 'deleteStoragePool',
icon: 'delete-outlined',
label: 'label.action.delete.primary.storage',
dataView: true,
- args: ['forced'],
Review Comment:
No, got removed by mistake
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
abh1sar commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1549932174
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
Review Comment:
No, it can't be.
I can see how usage of validateAndGetScopeType can cause confusion about the
assumption.
Have changed the code to use EnumUtils.getEnumIgnoreCase to get the enum
from string.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
GaOrtiga commented on code in PR #8875: URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1549676821 ## ui/public/locales/en.json: ## @@ -2447,6 +2448,8 @@ "message.action.patch.router": "Please confirm that you want to live patch the router. This operation is equivalent updating the router packages and restarting the Network without cleanup.", "message.action.patch.systemvm": "Please confirm that you want to patch the System VM.", "message.action.primarystorage.enable.maintenance.mode": "Warning: placing the primary storage into maintenance mode will cause all Instances using volumes from it to be stopped. Do you want to continue?", +"message.action.primary.storage.scope.cluster": "Change the primary storage scope from zone to cluster", Review Comment: These should be before `message.action.primarystorage.enable.maintenance.mode` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
JoaoJandre commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1549638108
##
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java:
##
@@ -543,4 +544,45 @@ public void enableStoragePool(DataStore dataStore) {
public void disableStoragePool(DataStore dataStore) {
dataStoreHelper.disable(dataStore);
}
-}
+
+public boolean changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+List hosts =
_resourceMgr.listAllHostsInOneZoneNotInClusterByHypervisor(hypervisorType,
clusterScope.getZoneId(), clusterScope.getScopeId());
+s_logger.debug("In createPool. Attaching the pool to each of the
hosts.");
+List poolHosts = new ArrayList();
+for (HostVO host : hosts) {
+try {
+storageMgr.connectHostToSharedPool(host.getId(),
store.getId());
+} catch (Exception e) {
+s_logger.warn("Unable to establish a connection between " +
host + " and " + store, e);
Review Comment:
If you fail to establish the connection between a host and the storage,
shouldn't the operation fail?
##
plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java:
##
@@ -543,4 +544,45 @@ public void enableStoragePool(DataStore dataStore) {
public void disableStoragePool(DataStore dataStore) {
dataStoreHelper.disable(dataStore);
}
-}
+
+public boolean changeStoragePoolScopeToZone(DataStore store, ClusterScope
clusterScope, HypervisorType hypervisorType) {
+List hosts =
_resourceMgr.listAllHostsInOneZoneNotInClusterByHypervisor(hypervisorType,
clusterScope.getZoneId(), clusterScope.getScopeId());
+s_logger.debug("In createPool. Attaching the pool to each of the
hosts.");
+List poolHosts = new ArrayList();
+for (HostVO host : hosts) {
+try {
+storageMgr.connectHostToSharedPool(host.getId(),
store.getId());
+} catch (Exception e) {
+s_logger.warn("Unable to establish a connection between " +
host + " and " + store, e);
+}
+}
+dataStoreHelper.switchToZone(store);
+return true;
+}
+
+public boolean changeStoragePoolScopeToCluster(DataStore store,
ClusterScope clusterScope, HypervisorType hypervisorType) {
+Pair, Integer> hostPoolRecords =
_storagePoolHostDao.listByPoolIdNotInCluster(clusterScope.getScopeId(),
store.getId());
+HypervisorType hType = null;
+if (hostPoolRecords.second() > 0) {
+hType =
getHypervisorType(hostPoolRecords.first().get(0).getHostId());
+}
+
+StoragePool pool = (StoragePool) store;
+for (StoragePoolHostVO host : hostPoolRecords.first()) {
+DeleteStoragePoolCommand deleteCmd = new
DeleteStoragePoolCommand(pool);
+final Answer answer = agentMgr.easySend(host.getHostId(),
deleteCmd);
+
+if (answer != null && answer.getResult()) {
+if (HypervisorType.KVM != hType) {
+break;
+}
+} else {
+if (answer != null) {
+s_logger.debug("Failed to delete storage pool: " +
answer.getResult());
Review Comment:
same logic here, if you fail to delete the storage pool, the operation
should fail.
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
GaOrtiga commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1549620636
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
+if (!supportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+if (StoragePoolStatus.Disabled != primaryStorage.getStatus()) {
+throw new InvalidParameterValueException("Primary storage should
be disabled for this operation");
+}
+
+if (scopeType == primaryStorage.getScope()) {
+throw new InvalidParameterValueException("Invalid scope change");
Review Comment:
This exception is kind of vague, could you add the reason for the exception
in the message? (Its current scope being the same as the chosen)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Change storage pool scope from Cluster to Zone and vise versa [cloudstack]
GaOrtiga commented on code in PR #8875:
URL: https://github.com/apache/cloudstack/pull/8875#discussion_r1549633449
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this operation");
+}
+
+ScopeType scopeType =
ScopeType.validateAndGetScopeType(cmd.getScope());
+if (scopeType != ScopeType.ZONE && scopeType != ScopeType.CLUSTER) {
+throw new InvalidParameterValueException("Invalid scope " +
scopeType.toString() + "for Primary storage");
+}
+
+StoragePoolVO primaryStorage = _storagePoolDao.findById(id);
+if (primaryStorage == null) {
+throw new IllegalArgumentException("Unable to find storage pool
with ID: " + id);
+}
+
+if
(!primaryStorage.getStorageProviderName().equals(DataStoreProvider.DEFAULT_PRIMARY))
{
+throw new InvalidParameterValueException("Primary storage scope
change is only supported with "
++ DataStoreProvider.DEFAULT_PRIMARY.toString() + " data
store provider");
+}
+
+if
(!primaryStorage.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)
&&
+!primaryStorage.getPoolType().equals(Storage.StoragePoolType.RBD))
{
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for protocol "
++ primaryStorage.getPoolType().toString());
+}
+
+HypervisorType hypervisorType = primaryStorage.getHypervisor();
+Set supportedHypervisorTypes =
Sets.newHashSet(HypervisorType.KVM, HypervisorType.VMware,
HypervisorType.Simulator);
+if (!supportedHypervisorTypes.contains(hypervisorType)) {
+throw new InvalidParameterValueException("Primary storage scope
change is not supported for hypervisor type " + hypervisorType);
+}
+
+if (StoragePoolStatus.Disabled != primaryStorage.getStatus()) {
+throw new InvalidParameterValueException("Primary storage should
be disabled for this operation");
+}
+
+if (scopeType == primaryStorage.getScope()) {
+throw new InvalidParameterValueException("Invalid scope change");
+}
+
+if (!primaryStorage.getStatus().equals(StoragePoolStatus.Disabled)) {
+throw new InvalidParameterValueException("Scope of the Primary
storage with id "
++ primaryStorage.getUuid() +
+" cannot be changed, as it is not in the Disabled state");
+}
+
+String providerName = primaryStorage.getStorageProviderName();
+DataStoreProvider storeProvider =
_dataStoreProviderMgr.getDataStoreProvider(providerName);
+PrimaryDataStoreLifeCycle lifeCycle = (PrimaryDataStoreLifeCycle)
storeProvider.getDataStoreLifeCycle();
+DataStore primaryStore = _dataStoreMgr.getPrimaryDataStore(id);
+
+Long zoneId = primaryStorage.getDataCenterId();
+DataCenterVO zone = _dcDao.findById(zoneId);
+if (zone == null) {
+throw new InvalidParameterValueException("unable to find zone by
id " + zoneId);
+}
+if (Grouping.AllocationState.Disabled == zone.getAllocationState()) {
+throw new PermissionDeniedException("Cannot perform this
operation, Zone is currently disabled: " + zoneId);
+}
+
+if (scopeType == ScopeType.ZONE) {
+ClusterScope clusterScope = new
ClusterScope(primaryStorage.getClusterId(), null, zoneId);
+lifeCycle.changeStoragePoolScopeToZone(primaryStore, clusterScope,
hypervisorType);
+
+} else {
+
Review Comment:
```suggestion
```
##
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##
@@ -1148,6 +1150,91 @@ public PrimaryDataStoreInfo
updateStoragePool(UpdateStoragePoolCmd cmd) throws I
return (PrimaryDataStoreInfo)_dataStoreMgr.getDataStore(pool.getId(),
DataStoreRole.Primary);
}
+@Override
+public boolean changeStoragePoolScope(ChangeStoragePoolScopeCmd cmd)
throws IllegalArgumentException, InvalidParameterValueException,
PermissionDeniedException {
+Long id = cmd.getId();
+
+Long accountId = cmd.getEntityOwnerId();
+if (!_accountMgr.isRootAdmin(accountId)) {
+throw new PermissionDeniedException("Only root admin can perform
this
