You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "DaanHoogland (via GitHub)" <gi...@apache.org> on 2025/12/15 14:00:24 UTC

[PR] 4.22 merge fwd [cloudstack]

DaanHoogland opened a new pull request, #12261:
URL: https://github.com/apache/cloudstack/pull/12261

   ### Description
   
   This PR...
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ******************************************************************************* -->
   <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ******************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   - [ ] Build/CI
   - [ ] Test (unit or integration test code)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   ### Screenshots (if appropriate):
   
   ### How Has This Been Tested?
   
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   
   #### How did you try to break this feature and the system with this change?
   
   <!-- see how your change affects other areas of the code, etc. -->
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
   


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 4.22 merge fwd [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12261:
URL: https://github.com/apache/cloudstack/pull/12261#discussion_r2621850408


##########
ui/src/views/network/VpnDetails.vue:
##########
@@ -215,7 +267,11 @@ export default {
         publicipid: this.resource.id,
         domainid: this.resource.domainid,
         account: this.resource.account

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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 4.22 merge fwd [cloudstack]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12261:
URL: https://github.com/apache/cloudstack/pull/12261#issuecomment-3659087396

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12261) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [23.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12261&metric=new_coverage&view=list) (required ≥ 40%)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12261)
   
   


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 4.22 merge fwd [cloudstack]

Posted by "sudo87 (via GitHub)" <gi...@apache.org>.
sudo87 commented on code in PR #12261:
URL: https://github.com/apache/cloudstack/pull/12261#discussion_r2621804107


##########
ui/src/views/network/VpnDetails.vue:
##########
@@ -215,7 +267,11 @@ export default {
         publicipid: this.resource.id,
         domainid: this.resource.domainid,
         account: this.resource.account

Review Comment:
   const params = {
           publicipid: this.resource.id,
           domainid: this.resource.domainid,
           account: this.resource.account
         }



-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 4.22 merge fwd [cloudstack]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12261:
URL: https://github.com/apache/cloudstack/pull/12261#discussion_r2619642621


##########
ui/src/views/network/VpnDetails.vue:
##########
@@ -16,40 +16,86 @@
 // under the License.
 
 <template>
-  <div v-if="remoteAccessVpn">
-    <div>
-      <p>{{ $t('message.enabled.vpn') }} <strong>{{ remoteAccessVpn.publicip }}</strong></p>
-      <p>{{ $t('message.enabled.vpn.ip.sec') }} <strong>{{ remoteAccessVpn.presharedkey }}</strong></p>
-      <a-divider/>
-      <a-button
-        style="margin-left: 10px"
-        type="primary"
-        danger
-        @click="disableVpn = true"
-        :disabled="!('deleteRemoteAccessVpn' in $store.getters.apis)">
-        {{ $t('label.disable.vpn') }}
-      </a-button>
-      <a-button><router-link :to="{ path: '/vpnuser'}">{{ $t('label.manage.vpn.user') }}</router-link></a-button>
+  <div class="vpn-details">
+    <div v-if="remoteAccessVpn">
+      <div>
+        <p>{{ $t('message.enabled.vpn') }} <strong>{{ remoteAccessVpn.publicip }}</strong></p>
+        <p>{{ $t('message.enabled.vpn.ip.sec') }} <strong>{{ remoteAccessVpn.presharedkey }}</strong></p>
+        <p>{{ $t('message.enabled.vpn.ip.range') }} <strong>{{ remoteAccessVpn.iprange }}</strong></p>
+        <a-divider/>
+        <a-button><router-link :to="{ path: '/vpnuser'}">{{ $t('label.manage.vpn.user') }}</router-link></a-button>
+        <a-button
+          style="margin-left: 10px"
+          type="primary"
+          danger
+          @click="disableVpn = true"
+          :disabled="!('deleteRemoteAccessVpn' in $store.getters.apis)">
+          {{ $t('label.disable.vpn') }}
+        </a-button>
+      </div>
+
+      <a-modal
+        :visible="disableVpn"
+        :footer="null"
+        :title="$t('label.disable.vpn')"
+        :closable="true"
+        :maskClosable="false"
+        @cancel="disableVpn = false">
+        <div v-ctrl-enter="handleDisableVpn">
+          <p>{{ $t('message.disable.vpn') }}</p>
+
+          <a-divider />
+
+          <div class="actions">
+            <a-button @click="() => disableVpn = false">{{ $t('label.cancel') }}</a-button>
+            <a-button type="primary" @click="handleDisableVpn">{{ $t('label.yes') }}</a-button>
+          </div>
+        </div>
+      </a-modal>
+
     </div>
+    <div v-else>
+      <a-button :disabled="!('createRemoteAccessVpn' in $store.getters.apis)" type="primary" @click="enableVpn = true">
+        {{ $t('label.enable.vpn') }}
+      </a-button>
 
-    <a-modal
-      :visible="disableVpn"
-      :footer="null"
-      :title="$t('label.disable.vpn')"
-      :closable="true"
-      :maskClosable="false"
-      @cancel="disableVpn = false">
-      <div v-ctrl-enter="handleDisableVpn">
-        <p>{{ $t('message.disable.vpn') }}</p>
+      <a-modal
+        :visible="enableVpn"
+        :footer="null"
+        :title="$t('label.enable.vpn')"
+        :maskClosable="false"
+        :closable="true"
+        @cancel="enableVpn = false">
+        <div v-ctrl-enter="handleCreateVpn">
+          <p>{{ $t('message.enable.vpn') }}</p>
+          <a-form-item>
+            <a-checkbox v-model:checked="specifyIpRange">
+              {{ $t('label.remote.access.vpn.specify.iprange') }}
+            </a-checkbox>
+          </a-form-item>
+          <a-form-item
+            v-if="specifyIpRange"
+            name="iprange"
+            :colon="false"
+            ref="iprange">
+            <template #label>
+              <tooltip-label :title="$t('label.ip.range')" :tooltip="$t('message.remote.access.vpn.iprange.description')"/>
+            </template>
+            <a-input
+              v-model:value="vpnIpRange"
+              :placeholder="'10.1.2.1-10.1.2.8'"
+            />
+          </a-form-item>
 
-        <a-divider />
+          <a-divider />
 
-        <div class="actions">
-          <a-button @click="() => disableVpn = false">{{ $t('label.cancel') }}</a-button>
-          <a-button type="primary" @click="handleDisableVpn">{{ $t('label.yes') }}</a-button>
+          <div class="actions">
+            <a-button @click="() => enableVpn = false">{{ $t('label.cancel') }}</a-button>
+            <a-button type="primary" ref="submit" @click="handleCreateVpn">{{ $t('label.yes') }}</a-button>
+          </div>
         </div>
-      </div>
-    </a-modal>
+      </a-modal>
+    </div>
 
   </div>
   <div v-else>

Review Comment:
   There is a duplicate closing div tag followed by a `v-else` that seems to be orphaned. Line 100 has a closing `</div>` tag, and line 101 has another closing `</div>` with `v-else` attribute. This `v-else` appears misplaced as it's on a closing tag rather than an opening tag. This could cause rendering issues or break the component's template structure.



##########
ui/src/views/network/NicsTab.vue:
##########
@@ -104,14 +110,28 @@
               </span>
             </a-select-option>
           </a-select>
-          <p class="modal-form__label">{{ $t('label.publicip') }}:</p>
-          <a-input v-model:value="addNetworkData.ip"></a-input>
-          <br>
+        </a-form-item>
+        <a-form-item name="ip" ref="ip">
+          <template #label>
+            <tooltip-label :title="$t('label.ipaddress')" :tooltip="addNetworkData.apiParams.ipaddress.description"/>
+          </template>
+          <a-input
+            :placeholder="addNetworkData.apiParams.ipaddress.description"
+            v-model:value="addNetworkData.ipaddress" />

Review Comment:
   The variable name changed from `ip` to `ipaddress` but this inconsistency with the previous usage suggests this could be incomplete. The field is named `ip` in the data structure on line 364, but the v-model binds to `addNetworkData.ipaddress` on line 120. This mismatch will cause the IP address value not to be properly stored or cleared.
   ```suggestion
               v-model:value="addNetworkData.ip" />
   ```



##########
ui/src/views/infra/network/IpRangesTabPublic.vue:
##########
@@ -220,6 +220,20 @@
             <a-select-option v-for="pod in pods" :key="pod.id" :value="pod.id" :label="pod.name">{{ pod.name }}</a-select-option>
           </a-select>
         </a-form-item>
+        <a-form-item name="isolationmethod" ref="isolationmethod" class="form__item" v-if="!basicGuestNetwork">
+          <tooltip-label :title="$t('label.isolation.method')" :tooltip="$t('label.choose.isolation.method.public.ip.range')" class="tooltip-label-wrapper"/>
+          <a-select
+            v-model:value="form.isolationmethod"
+            showSearch
+            optionFilterProp="label"
+            :filterOption="(input, option) => {
+              return option.label.toLowerCase().indexOf(input.toLowerCase()) >= 0
+            }" >
+            <a-select-option value="">{{ }}</a-select-option>

Review Comment:
   The empty string value for the first select option will display as blank, making it unclear to users what this option represents. Consider adding a meaningful label like "Default" or "None" to improve user experience.



##########
server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java:
##########
@@ -372,8 +411,6 @@ List<Ternary<VirtualMachine, Host, Host>> getDrsPlan(Cluster cluster,
             ServiceOffering serviceOffering = vmIdServiceOfferingMap.get(vm.getId());
             migrationPlan.add(new Ternary<>(vm, hostMap.get(vm.getHostId()), hostMap.get(destHost.getId())));
 
-            hostVmMap.get(vm.getHostId()).remove(vm);
-            hostVmMap.get(destHost.getId()).add(vm);
             hostVmMap.get(vm.getHostId()).remove(vm);
             hostVmMap.get(destHost.getId()).add(vm);

Review Comment:
   The duplicate lines 414-415 suggest an incomplete merge. Line 414 and 415 both perform the same operation: removing vm from source host and adding to destination host. One of these duplicate blocks should be removed.



##########
server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java:
##########
@@ -118,240 +113,283 @@ public boolean maintain(DataStore store) {
 
     @Override
     public boolean maintain(DataStore store, Map<String,String> details) {
-        Long userId = CallContext.current().getCallingUserId();
-        User user = _userDao.findById(userId);
-        Account account = CallContext.current().getCallingAccount();
         StoragePoolVO pool = primaryDataStoreDao.findById(store.getId());
         try {
-            List<StoragePoolVO> spes = null;
-            // Handling Zone and Cluster wide storage scopes.
-            // if the storage is ZONE wide then we pass podid and cluster id as null as they will be empty for ZWPS
-            if (pool.getScope() == ScopeType.ZONE) {
-                spes = primaryDataStoreDao.listBy(pool.getDataCenterId(), null, null, ScopeType.ZONE);
-            } else {
-                spes = primaryDataStoreDao.listBy(pool.getDataCenterId(), pool.getPodId(), pool.getClusterId(), ScopeType.CLUSTER);
-            }
-            for (StoragePoolVO sp : spes) {
-                if (sp.getParent() != pool.getParent() && sp.getId() != pool.getParent()) { // If Datastore cluster is tried to prepare for maintenance then child storage pools are also kept in PrepareForMaintenance mode
-                    if (sp.getStatus() == StoragePoolStatus.PrepareForMaintenance) {
-                        throw new CloudRuntimeException(String.format("Only one storage pool in a cluster can be in PrepareForMaintenance mode, %s is already in  PrepareForMaintenance mode ", sp));
-                    }
-                }
-            }
-            StoragePool storagePool = (StoragePool)store;
-
-            //Handeling the Zone wide and cluster wide primay storage
-            List<HostVO> hosts = new ArrayList<HostVO>();
-            // if the storage scope is ZONE wide, then get all the hosts for which hypervisor ZWSP created to send Modifystoragepoolcommand
-            //TODO: if it's zone wide, this code will list a lot of hosts in the zone, which may cause performance/OOM issue.
-            if (pool.getScope().equals(ScopeType.ZONE)) {
-                if (HypervisorType.Any.equals(pool.getHypervisor())) {
-                    hosts = _resourceMgr.listAllUpAndEnabledHostsInOneZone(pool.getDataCenterId());
-                }
-                else {
-                    hosts = _resourceMgr.listAllUpAndEnabledHostsInOneZoneByHypervisor(pool.getHypervisor(), pool.getDataCenterId());
-                }
-            } else {
-                hosts = _resourceMgr.listHostsInClusterByStatus(pool.getClusterId(), Status.Up);
+            getStoragePoolForSpecification(pool);
+
+            List<HostVO> hosts = getHostsForStoragePool(pool);
+
+            if (setNextStateForMaintenance(hosts, pool) == StoragePoolStatus.PrepareForMaintenance) {
+                removeHeartbeatForHostsFromPool(hosts, pool);
+                // check to see if other ps exist
+                // if they do, then we can migrate over the system vms to them
+                // if they don't, then just stop all vms on this one
+                List<StoragePoolVO> upPools = primaryDataStoreDao.listByStatusInZone(pool.getDataCenterId(), StoragePoolStatus.Up);
+                boolean restart = !CollectionUtils.isEmpty(upPools);
+
+                // 2. Get a list of all the ROOT volumes within this storage pool
+                List<VolumeVO> allVolumes = volumeDao.findByPoolId(pool.getId());
+                // 3. Enqueue to the work queue
+                enqueueMigrationsForVolumes(allVolumes, pool);
+                // 4. Process the queue
+                processMigrationWorkloads(pool, restart);
             }
+        } catch (Exception e) {
+            logger.error("Exception in enabling primary storage maintenance:", e);
+            pool.setStatus(StoragePoolStatus.ErrorInMaintenance);
+            primaryDataStoreDao.update(pool.getId(), pool);
+            // TODO decide on what recovery is possible
+            throw new CloudRuntimeException(e.getMessage());
+        }
+        return true;
+    }
 
-            if (hosts == null || hosts.size() == 0) {
-                pool.setStatus(StoragePoolStatus.Maintenance);
-                primaryDataStoreDao.update(pool.getId(), pool);
-                return true;
-            } else {
-                // set the pool state to prepare for maintenance
-                pool.setStatus(StoragePoolStatus.PrepareForMaintenance);
-                primaryDataStoreDao.update(pool.getId(), pool);
-            }
-            // remove heartbeat
-            for (HostVO host : hosts) {
-                ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(false, storagePool);
-                if (MapUtils.isNotEmpty(details) && storageManager.canDisconnectHostFromStoragePool(host, storagePool)) {
-                    cmd.setDetails(details);
-                }
-                final Answer answer = agentMgr.easySend(host.getId(), cmd);
-                if (answer == null || !answer.getResult()) {
-                    if (logger.isDebugEnabled()) {
-                        logger.debug("ModifyStoragePool false failed due to " + ((answer == null) ? "answer null" : answer.getDetails()));
-                    }
-                } else {
-                    if (logger.isDebugEnabled()) {
-                        logger.debug("ModifyStoragePool false succeeded");
-                    }
-                    if (pool.getPoolType() == Storage.StoragePoolType.DatastoreCluster) {
-                        logger.debug("Started synchronising datastore cluster storage pool {} with vCenter", pool);
-                        storageManager.syncDatastoreClusterStoragePool(pool.getId(), ((ModifyStoragePoolAnswer) answer).getDatastoreClusterChildren(), host.getId());
-                    }
-                }
-            }
-            // check to see if other ps exist
-            // if they do, then we can migrate over the system vms to them
-            // if they don't, then just stop all vms on this one
-            List<StoragePoolVO> upPools = primaryDataStoreDao.listByStatusInZone(pool.getDataCenterId(), StoragePoolStatus.Up);
-            boolean restart = true;
-            if (upPools == null || upPools.size() == 0) {
-                restart = false;
-            }
+    @Override
+    public boolean cancelMaintain(DataStore store) {
+        return cancelMaintain(store, null);
+    }
 
-            // 2. Get a list of all the ROOT volumes within this storage pool
-            List<VolumeVO> allVolumes = volumeDao.findByPoolId(pool.getId());
+    @Override
+    public boolean cancelMaintain(DataStore store, Map<String,String> details) {
+        // Change the storage state back to up
+        StoragePoolVO poolVO = primaryDataStoreDao.findById(store.getId());
+        StoragePool pool = (StoragePool)store;
 
-            // 3. Enqueue to the work queue
-            for (VolumeVO volume : allVolumes) {
-                VMInstanceVO vmInstance = vmDao.findById(volume.getInstanceId());
+        List<HostVO> hosts = getHostsForStoragePool(poolVO);
 
-                if (vmInstance == null) {
-                    continue;
-                }
+        if (CollectionUtils.isEmpty(hosts)) {
+            return true;
+        }
 
-                // enqueue sp work
-                if (vmInstance.getState().equals(State.Running) || vmInstance.getState().equals(State.Starting) || vmInstance.getState().equals(State.Stopping)) {
-
-                    try {
-                        StoragePoolWorkVO work = new StoragePoolWorkVO(vmInstance.getId(), pool.getId(), false, false, server.getId());
-                        _storagePoolWorkDao.persist(work);
-                    } catch (Exception e) {
-                        if (logger.isDebugEnabled()) {
-                            logger.debug("Work record already exists, re-using by re-setting values");
-                        }
-                        StoragePoolWorkVO work = _storagePoolWorkDao.findByPoolIdAndVmId(pool.getId(), vmInstance.getId());
-                        work.setStartedAfterMaintenance(false);
-                        work.setStoppedForMaintenance(false);
-                        work.setManagementServerId(server.getId());
-                        _storagePoolWorkDao.update(work.getId(), work);
-                    }
-                }
+        Pair<Map<String, String>, Boolean> nfsMountOpts = storageManager.getStoragePoolNFSMountOpts(pool, null);
+        addHeartbeatToHostsInPool(hosts, pool, nfsMountOpts);
+
+        // 2. Get a list of pending work for this queue
+        List<StoragePoolWorkVO> pendingWork = _storagePoolWorkDao.listPendingWorkForCancelMaintenanceByPoolId(poolVO.getId());
+
+        // 3. work through the queue
+        cancelMigrationWorkloads(pendingWork);
+        return false;
+    }
+
+    private StoragePoolStatus setNextStateForMaintenance(List<HostVO> hosts, StoragePoolVO pool) {
+        if (CollectionUtils.isEmpty(hosts)) {
+            pool.setStatus(StoragePoolStatus.Maintenance);
+            primaryDataStoreDao.update(pool.getId(), pool);
+            return StoragePoolStatus.Maintenance;
+        } else {
+            // set the pool state to prepare for maintenance
+            pool.setStatus(StoragePoolStatus.PrepareForMaintenance);
+            primaryDataStoreDao.update(pool.getId(), pool);
+            return StoragePoolStatus.PrepareForMaintenance;
+        }
+    }
+
+    private void processMigrationWorkloads(StoragePoolVO pool, boolean restart) throws ResourceUnavailableException, OperationTimedoutException, InsufficientCapacityException {
+        List<StoragePoolWorkVO> pendingWork = _storagePoolWorkDao.listPendingWorkForPrepareForMaintenanceByPoolId(pool.getId());
+
+        for (StoragePoolWorkVO work : pendingWork) {
+            // shut down the running vms
+            VMInstanceVO vmInstance = vmDao.findById(work.getVmId());
+
+            if (vmInstance == null) {
+                continue;
             }
 
-            // 4. Process the queue
-            List<StoragePoolWorkVO> pendingWork = _storagePoolWorkDao.listPendingWorkForPrepareForMaintenanceByPoolId(pool.getId());
+            switch (vmInstance.getType()) {
+                case ConsoleProxy:
+                case SecondaryStorageVm:
+                case DomainRouter:
+                    handleVmMigration(restart, work, vmInstance);
+                    break;
+                case User:
+                    handleStopVmForMigration(work, vmInstance);
+                    break;
+            }
+        }
+    }
 
-            for (StoragePoolWorkVO work : pendingWork) {
-                // shut down the running vms
+    private void cancelMigrationWorkloads(List<StoragePoolWorkVO> pendingWork) {
+        for (StoragePoolWorkVO work : pendingWork) {
+            try {
                 VMInstanceVO vmInstance = vmDao.findById(work.getVmId());
 
                 if (vmInstance == null) {
                     continue;
                 }
 
-                // if the instance is of type consoleproxy, call the console
-                // proxy
-                if (vmInstance.getType().equals(VirtualMachine.Type.ConsoleProxy)) {
-                    // call the consoleproxymanager
-                    ConsoleProxyVO consoleProxy = _consoleProxyDao.findById(vmInstance.getId());
-                    vmMgr.advanceStop(consoleProxy.getUuid(), false);
-                    // update work status
-                    work.setStoppedForMaintenance(true);
-                    _storagePoolWorkDao.update(work.getId(), work);
+                switch (vmInstance.getType()) {
+                    case ConsoleProxy:
+                    case SecondaryStorageVm:
+                    case DomainRouter:
+                        handleVmStart(work, vmInstance);
+                        break;
+                    case User:
+                        handleUserVmStart(work, vmInstance);
+                        break;
+                }
+            } catch (Exception e) {
+                logger.debug("Failed start vm", e);
+                throw new CloudRuntimeException(e.toString());
+            }
+        }
+    }
 
-                    if (restart) {
+    private void handleStopVmForMigration(StoragePoolWorkVO work, VMInstanceVO vmInstance) throws ResourceUnavailableException, OperationTimedoutException {
+        vmMgr.advanceStop(vmInstance.getUuid(), false);
+        // update work status
+        work.setStoppedForMaintenance(true);
+        _storagePoolWorkDao.update(work.getId(), work);
+    }
 
-                        vmMgr.advanceStart(consoleProxy.getUuid(), null, null);
-                        // update work status
-                        work.setStartedAfterMaintenance(true);
-                        _storagePoolWorkDao.update(work.getId(), work);
-                    }
-                }
+    private void handleVmMigration(boolean restart, StoragePoolWorkVO work, VMInstanceVO vmInstance) throws ResourceUnavailableException, OperationTimedoutException, InsufficientCapacityException {
+        handleStopVmForMigration(work, vmInstance);
 
-                // if the instance is of type uservm, call the user vm manager
-                if (vmInstance.getType() == VirtualMachine.Type.User) {
-                    UserVmVO userVm = userVmDao.findById(vmInstance.getId());
-                    vmMgr.advanceStop(userVm.getUuid(), false);
-                    // update work status
-                    work.setStoppedForMaintenance(true);
-                    _storagePoolWorkDao.update(work.getId(), work);
-                }
+        if (restart) {
+            handleVmStart(work, vmInstance);
+        }
+    }
 
-                // if the instance is of type secondary storage vm, call the
-                // secondary storage vm manager
-                if (vmInstance.getType().equals(VirtualMachine.Type.SecondaryStorageVm)) {
-                    SecondaryStorageVmVO secStrgVm = _secStrgDao.findById(vmInstance.getId());
-                    vmMgr.advanceStop(secStrgVm.getUuid(), false);
-                    // update work status
-                    work.setStoppedForMaintenance(true);
-                    _storagePoolWorkDao.update(work.getId(), work);
+    private void handleVmStart(StoragePoolWorkVO work, VMInstanceVO vmInstance) throws InsufficientCapacityException, ResourceUnavailableException, OperationTimedoutException {
+        vmMgr.advanceStart(vmInstance.getUuid(), null, null);
+        // update work queue
+        work.setStartedAfterMaintenance(true);
+        _storagePoolWorkDao.update(work.getId(), work);
+    }
 
-                    if (restart) {
-                        vmMgr.advanceStart(secStrgVm.getUuid(), null, null);
-                        // update work status
-                        work.setStartedAfterMaintenance(true);
-                        _storagePoolWorkDao.update(work.getId(), work);
-                    }
-                }
+    private void enqueueMigrationsForVolumes(List<VolumeVO> allVolumes, StoragePoolVO pool) {
+        for (VolumeVO volume : allVolumes) {
+            VMInstanceVO vmInstance = vmDao.findById(volume.getInstanceId());
 
-                // if the instance is of type domain router vm, call the network
-                // manager
-                if (vmInstance.getType().equals(VirtualMachine.Type.DomainRouter)) {
-                    DomainRouterVO domR = _domrDao.findById(vmInstance.getId());
-                    vmMgr.advanceStop(domR.getUuid(), false);
-                    // update work status
-                    work.setStoppedForMaintenance(true);
-                    _storagePoolWorkDao.update(work.getId(), work);
+            if (vmInstance == null) {
+                continue;
+            }
 
-                    if (restart) {
-                        vmMgr.advanceStart(domR.getUuid(), null, null);
-                        // update work status
-                        work.setStartedAfterMaintenance(true);
-                        _storagePoolWorkDao.update(work.getId(), work);
+            // enqueue sp work
+            if (vmInstance.getState().equals(State.Running) || vmInstance.getState().equals(State.Starting) || vmInstance.getState().equals(State.Stopping)) {
+
+                try {
+                    StoragePoolWorkVO work = new StoragePoolWorkVO(vmInstance.getId(), pool.getId(), false, false, server.getId());
+                    _storagePoolWorkDao.persist(work);
+                } catch (Exception e) {
+                    if (logger.isDebugEnabled()) {
+                        logger.debug("Work record already exists, re-using by re-setting values");
                     }
+                    StoragePoolWorkVO work = _storagePoolWorkDao.findByPoolIdAndVmId(pool.getId(), vmInstance.getId());
+                    work.setStartedAfterMaintenance(false);
+                    work.setStoppedForMaintenance(false);
+                    work.setManagementServerId(server.getId());
+                    _storagePoolWorkDao.update(work.getId(), work);
                 }
             }
-        } catch (Exception e) {
-            logger.error("Exception in enabling primary storage maintenance:", e);
-            pool.setStatus(StoragePoolStatus.ErrorInMaintenance);
-            primaryDataStoreDao.update(pool.getId(), pool);
-            throw new CloudRuntimeException(e.getMessage());
         }
-        return true;
     }
 
-    @Override
-    public boolean cancelMaintain(DataStore store) {
-        return cancelMaintain(store, null);
+    private void removeHeartbeatForHostsFromPool(List<HostVO> hosts, StoragePool storagePool) {
+        // remove heartbeat
+        for (HostVO host : hosts) {
+            ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(false, storagePool);
+            final Answer answer = agentMgr.easySend(host.getId(), cmd);
+            if (answer == null || !answer.getResult()) {
+                if (logger.isDebugEnabled()) {
+                    logger.debug("ModifyStoragePool false failed due to {}", ((answer == null) ? "answer null" : answer.getDetails()));
+                }
+            } else {
+                reportSucceededModifyStorePool(storagePool, (ModifyStoragePoolAnswer) answer, host, false);
+            }
+        }
     }
 
-    @Override
-    public boolean cancelMaintain(DataStore store, Map<String,String> details) {
-        // Change the storage state back to up
-        Long userId = CallContext.current().getCallingUserId();
-        User user = _userDao.findById(userId);
-        Account account = CallContext.current().getCallingAccount();
-        StoragePoolVO poolVO = primaryDataStoreDao.findById(store.getId());
-        StoragePool pool = (StoragePool)store;
+    private void reportSucceededModifyStorePool(StoragePool storagePool, ModifyStoragePoolAnswer answer, HostVO host, boolean add) {
+        if (logger.isDebugEnabled()) {
+            logger.debug("ModifyStoragePool succeeded for {}", add ? "adding" : "removing");
+        }
+        if (storagePool.getPoolType() == Storage.StoragePoolType.DatastoreCluster) {
+            logger.debug("Started synchronising datastore cluster storage pool {} with vCenter", storagePool);
+            storageManager.syncDatastoreClusterStoragePool(storagePool.getId(), answer.getDatastoreClusterChildren(), host.getId());
+        }
+    }
 
-        //Handeling the Zone wide and cluster wide primay storage
-        List<HostVO> hosts = new ArrayList<HostVO>();
-        // if the storage scope is ZONE wide, then get all the hosts for which hypervisor ZWSP created to send Modifystoragepoolcommand
-        if (poolVO.getScope().equals(ScopeType.ZONE)) {
+    /**
+     * Handling the Zone wide and cluster wide primary storage
+     * if the storage scope is ZONE wide, then get all the hosts for which hypervisor ZoneWideStoragePools created to send ModifyStoragePoolCommand
+     * TODO: if it's zone wide, this code will list a lot of hosts in the zone, which may cause performance/OOM issue.
+     * @param pool pool to check for connected hosts
+     * @return a list of connected hosts
+     */
+    private List<HostVO> getHostsForStoragePool(StoragePoolVO pool) {
+        List<HostVO> hosts;
+        if (pool.getScope().equals(ScopeType.ZONE)) {
             if (HypervisorType.Any.equals(pool.getHypervisor())) {
                 hosts = _resourceMgr.listAllUpAndEnabledHostsInOneZone(pool.getDataCenterId());
             }
             else {
-                hosts = _resourceMgr.listAllUpAndEnabledHostsInOneZoneByHypervisor(poolVO.getHypervisor(), pool.getDataCenterId());
+                hosts = _resourceMgr.listAllUpAndEnabledHostsInOneZoneByHypervisor(pool.getHypervisor(), pool.getDataCenterId());
             }
         } else {
             hosts = _resourceMgr.listHostsInClusterByStatus(pool.getClusterId(), Status.Up);
         }
+        return hosts;
+    }
 
-        if (hosts == null || hosts.size() == 0) {
-            return true;
+    /**
+     * Handling Zone and Cluster wide storage scopes. Depending on the scope of the pool, check for other storage pools in the same scope
+     * If the storage is ZONE wide then we pass podId and cluster id as null as they will be empty for Zone wide storage
+     *
+     * @param pool pool to check for other pools in the same scope
+     */
+    private void getStoragePoolForSpecification(StoragePoolVO pool) {
+        List<StoragePoolVO> storagePools;
+        if (pool.getScope() == ScopeType.ZONE) {
+            storagePools = primaryDataStoreDao.listBy(pool.getDataCenterId(), null, null, ScopeType.ZONE);
+        } else {
+            storagePools = primaryDataStoreDao.listBy(pool.getDataCenterId(), pool.getPodId(), pool.getClusterId(), ScopeType.CLUSTER);
         }
+        checkHierarchyForPreparingForMaintenance(pool, storagePools);
+    }
 
-        Pair<Map<String, String>, Boolean> nfsMountOpts = storageManager.getStoragePoolNFSMountOpts(pool, null);
-        // add heartbeat
-        for (HostVO host : hosts) {
-            ModifyStoragePoolCommand msPoolCmd = new ModifyStoragePoolCommand(true, pool);
-            if (MapUtils.isNotEmpty(details)) {
-                msPoolCmd.setDetails(details);
+    /**
+     * If Datastore cluster is tried to prepare for maintenance then child storage pools are also kept in PrepareForMaintenance mode
+     * @param pool target to put in maintenance
+     * @param storagePools list of possible peers/parents/children
+     */
+    private static void checkHierarchyForPreparingForMaintenance(StoragePoolVO pool, List<StoragePoolVO> storagePools) {
+        for (StoragePoolVO storagePool : storagePools) {
+            if (!(storagePool.getParent().equals(pool.getParent()) || !pool.getParent().equals(storagePool.getId())) &&

Review Comment:
   The logic for checking hierarchy in PrepareForMaintenance mode appears inverted. The condition `!(storagePool.getParent().equals(pool.getParent()) || !pool.getParent().equals(storagePool.getId()))` uses double negation which is confusing. The expression `!(A || !B)` is equivalent to `!A && B` by De Morgan's law, but this doesn't clearly express the intended logic. This should be simplified for clarity and correctness.
   ```suggestion
               if (!storagePool.getParent().equals(pool.getParent()) && pool.getParent().equals(storagePool.getId()) &&
   ```



-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 4.22 merge fwd [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland closed pull request #12261: 4.22 merge fwd
URL: https://github.com/apache/cloudstack/pull/12261


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 4.22 merge fwd [cloudstack]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #12261:
URL: https://github.com/apache/cloudstack/pull/12261#issuecomment-3657455681

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/12261?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `37.03704%` with `289 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 18.65%. Comparing base ([`0a13fb2`](https://app.codecov.io/gh/apache/cloudstack/commit/0a13fb27f58f29ef1d5da46dae2d4f91d253d8ed?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`c4b9f7a`](https://app.codecov.io/gh/apache/cloudstack/commit/c4b9f7abc940e4af3e91a995ca7b9fc9f20bb0d6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/cloudstack/pull/12261?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...a/com/cloud/storage/StoragePoolAutomationImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fstorage%2FStoragePoolAutomationImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3N0b3JhZ2UvU3RvcmFnZVBvb2xBdXRvbWF0aW9uSW1wbC5qYXZh) | 0.00% | [155 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ache/cloudstack/cluster/ClusterDrsServiceImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fcluster%2FClusterDrsServiceImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2NsdXN0ZXIvQ2x1c3RlckRyc1NlcnZpY2VJbXBsLmphdmE=) | 70.58% | [29 Missing and 11 partials :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...in/java/com/cloud/server/ManagementServerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fserver%2FManagementServerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3NlcnZlci9NYW5hZ2VtZW50U2VydmVySW1wbC5qYXZh) | 0.00% | [39 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...java/com/cloud/upgrade/DatabaseUpgradeChecker.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=engine%2Fschema%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fupgrade%2FDatabaseUpgradeChecker.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC91cGdyYWRlL0RhdGFiYXNlVXBncmFkZUNoZWNrZXIuamF2YQ==) | 56.14% | [22 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...apache/cloudstack/cluster/ClusterDrsAlgorithm.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fcluster%2FClusterDrsAlgorithm.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2NsdXN0ZXIvQ2x1c3RlckRyc0FsZ29yaXRobS5qYXZh) | 58.82% | [12 Missing and 9 partials :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [.../java/org/apache/cloudstack/cluster/Condensed.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=plugins%2Fdrs%2Fcluster%2Fcondensed%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fcluster%2FCondensed.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGx1Z2lucy9kcnMvY2x1c3Rlci9jb25kZW5zZWQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Nsb3Vkc3RhY2svY2x1c3Rlci9Db25kZW5zZWQuamF2YQ==) | 62.50% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...n/java/org/apache/cloudstack/cluster/Balanced.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=plugins%2Fdrs%2Fcluster%2Fbalanced%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fcluster%2FBalanced.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGx1Z2lucy9kcnMvY2x1c3Rlci9iYWxhbmNlZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY2xvdWRzdGFjay9jbHVzdGVyL0JhbGFuY2VkLmphdmE=) | 71.42% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...c/main/java/com/cloud/user/AccountManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fuser%2FAccountManagerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3VzZXIvQWNjb3VudE1hbmFnZXJJbXBsLmphdmE=) | 50.00% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...min/network/CreateManagementNetworkIpRangeCmd.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fapi%2Fcommand%2Fadmin%2Fnetwork%2FCreateManagementNetworkIpRangeCmd.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2FwaS9jb21tYW5kL2FkbWluL25ldHdvcmsvQ3JlYXRlTWFuYWdlbWVudE5ldHdvcmtJcFJhbmdlQ21kLmphdmE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...k/api/command/admin/vlan/CreateVlanIpRangeCmd.java](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&filepath=api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fapi%2Fcommand%2Fadmin%2Fvlan%2FCreateVlanIpRangeCmd.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2FwaS9jb21tYW5kL2FkbWluL3ZsYW4vQ3JlYXRlVmxhbklwUmFuZ2VDbWQuamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12261?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##               4.22   #12261      +/-   ##
   ============================================
   + Coverage     17.55%   18.65%   +1.09%     
   - Complexity    15544    15575      +31     
   ============================================
     Files          5910     5465     -445     
     Lines        529162   491818   -37344     
     Branches      64641    57756    -6885     
   ============================================
   - Hits          92918    91746    -1172     
   + Misses       425786   389754   -36032     
   + Partials      10458    10318     -140     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/12261/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/12261/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/12261/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `18.65% <37.03%> (+0.02%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/12261?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 4.22 merge fwd [cloudstack]

Posted by "sudo87 (via GitHub)" <gi...@apache.org>.
sudo87 commented on code in PR #12261:
URL: https://github.com/apache/cloudstack/pull/12261#discussion_r2621804107


##########
ui/src/views/network/VpnDetails.vue:
##########
@@ -215,7 +267,11 @@ export default {
         publicipid: this.resource.id,
         domainid: this.resource.domainid,
         account: this.resource.account

Review Comment:
   Need to define params:
   ```
   const params = {
           publicipid: this.resource.id,
           domainid: this.resource.domainid,
           account: this.resource.account
         }



-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 4.22 merge fwd [cloudstack]

Posted by "sudo87 (via GitHub)" <gi...@apache.org>.
sudo87 commented on code in PR #12261:
URL: https://github.com/apache/cloudstack/pull/12261#discussion_r2621804107


##########
ui/src/views/network/VpnDetails.vue:
##########
@@ -215,7 +267,11 @@ export default {
         publicipid: this.resource.id,
         domainid: this.resource.domainid,
         account: this.resource.account

Review Comment:
   We need to define params:
   ```
   const params = {
           publicipid: this.resource.id,
           domainid: this.resource.domainid,
           account: this.resource.account
         }



-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] 4.22 merge fwd [cloudstack]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12261:
URL: https://github.com/apache/cloudstack/pull/12261#issuecomment-3657467030

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12261) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [23.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=12261&metric=new_coverage&view=list) (required ≥ 40%)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=12261)
   
   


-- 
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-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org