Merge remote-tracking branch 'apache/4.20' into deployvm-improvements

Author: shwstppr

.github/workflows/ci.yml

@@ -236,7 +236,7 @@ jobs:
 
       - name: Install Python dependencies
         run: |
-          python3 -m pip install --user --upgrade urllib3 lxml paramiko nose texttable ipmisim pyopenssl pycrypto mock flask netaddr pylint pycodestyle six astroid
+          python3 -m pip install --user --upgrade urllib3 lxml paramiko nose texttable ipmisim pyopenssl pycrypto mock flask netaddr pylint pycodestyle six astroid pynose
 
       - name: Install jacoco dependencies
         run: |

api/src/main/java/com/cloud/storage/VMTemplateStorageResourceAssoc.java

@@ -17,6 +17,7 @@
 package com.cloud.storage;
 
 import java.util.Date;
+import java.util.List;
 
 import org.apache.cloudstack.api.InternalIdentity;
 
@@ -25,6 +26,8 @@ public static enum Status {
         UNKNOWN, DOWNLOAD_ERROR, NOT_DOWNLOADED, DOWNLOAD_IN_PROGRESS, DOWNLOADED, ABANDONED, UPLOADED, NOT_UPLOADED, UPLOAD_ERROR, UPLOAD_IN_PROGRESS, CREATING, CREATED, BYPASSED
     }
 
+    List<Status> PENDING_DOWNLOAD_STATES = List.of(Status.NOT_DOWNLOADED, Status.DOWNLOAD_IN_PROGRESS);
+
     String getInstallPath();
 
     long getTemplateId();

api/src/main/java/org/apache/cloudstack/api/command/user/iso/ExtractIsoCmd.java

@@ -17,6 +17,7 @@
 package org.apache.cloudstack.api.command.user.iso;
 
 
+import com.cloud.dc.DataCenter;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiCommandResourceType;
 import org.apache.cloudstack.api.ApiConstants;
@@ -101,7 +102,15 @@ public long getEntityOwnerId() {
 
     @Override
     public String getEventDescription() {
-        return "extracting ISO: " + getId() + " from zone: " + getZoneId();
+        String isoId = this._uuidMgr.getUuid(VirtualMachineTemplate.class, getId());
+        String baseDescription = String.format("Extracting ISO: %s", isoId);
+
+        Long zoneId = getZoneId();
+        if (zoneId == null) {
+            return baseDescription;
+        }
+
+        return String.format("%s from zone: %s", baseDescription, this._uuidMgr.getUuid(DataCenter.class, zoneId));
     }
 
     @Override

api/src/main/java/org/apache/cloudstack/api/command/user/template/ExtractTemplateCmd.java

@@ -101,7 +101,15 @@ public String getEventType() {
 
     @Override
     public String getEventDescription() {
-     return "extracting template: " + this._uuidMgr.getUuid(VirtualMachineTemplate.class, getId()) + ((getZoneId() != null) ? " from zone: " + this._uuidMgr.getUuid(DataCenter.class, getZoneId()) : "");
+        String templateId = this._uuidMgr.getUuid(VirtualMachineTemplate.class, getId());
+        String baseDescription = String.format("Extracting template: %s", templateId);
+
+        Long zoneId = getZoneId();
+        if (zoneId == null) {
+            return baseDescription;
+        }
+
+        return String.format("%s from zone: %s", baseDescription, this._uuidMgr.getUuid(DataCenter.class, zoneId));
     }
 
     @Override

api/src/main/java/org/apache/cloudstack/api/command/user/volume/ExtractVolumeCmd.java

@@ -17,6 +17,7 @@
 package org.apache.cloudstack.api.command.user.volume;
 
 
+import com.cloud.dc.DataCenter;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.api.ACL;
 import org.apache.cloudstack.api.APICommand;
@@ -114,12 +115,15 @@ public String getEventType() {
 
     @Override
     public String getEventDescription() {
-        return  "Extraction job";
+        String volumeId = this._uuidMgr.getUuid(Volume.class, getId());
+        String zoneId = this._uuidMgr.getUuid(DataCenter.class, getZoneId());
+
+        return String.format("Extracting volume: %s from zone: %s", volumeId, zoneId);
     }
 
     @Override
     public void execute() {
-        CallContext.current().setEventDetails("Volume Id: " + this._uuidMgr.getUuid(Volume.class, getId()));
+        CallContext.current().setEventDetails(getEventDescription());
         String uploadUrl = _volumeService.extractVolume(this);
         if (uploadUrl != null) {
             ExtractResponse response = _responseGenerator.createVolumeExtractResponse(id, zoneId, getEntityOwnerId(), mode, uploadUrl);

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -5240,10 +5240,9 @@ public Outcome<VirtualMachine> migrateVmAwayThroughJobQueue(final String vmUuid,
             workJob = newVmWorkJobAndInfo.first();
             VmWorkMigrateAway workInfo = new VmWorkMigrateAway(newVmWorkJobAndInfo.second(), srcHostId);
 
-            workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo));
+            setCmdInfoAndSubmitAsyncJob(workJob, workInfo, vmId);
         }
 
-        _jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vmId);
 
         AsyncJobExecutionContext.getCurrentExecutionContext().joinJob(workJob.getId());
 

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java

@@ -208,9 +208,7 @@ protected List<DataObject> getAllReadyTemplates(DataStore srcDataStore, Map<Data
         List<TemplateInfo> files = new LinkedList<>();
         for (TemplateDataStoreVO template : templates) {
             VMTemplateVO templateVO = templateDao.findById(template.getTemplateId());
-            if (template.getState() == ObjectInDataStoreStateMachine.State.Ready && templateVO != null &&
-                    (!templateVO.isPublicTemplate() || (templateVO.isPublicTemplate() && templateVO.getUrl() == null)) &&
-                    templateVO.getHypervisorType() != Hypervisor.HypervisorType.Simulator && templateVO.getParentTemplateId() == null) {
+            if (shouldMigrateTemplate(template, templateVO)) {
                 files.add(templateFactory.getTemplate(template.getTemplateId(), srcDataStore));
             }
         }
@@ -231,6 +229,34 @@ protected List<DataObject> getAllReadyTemplates(DataStore srcDataStore, Map<Data
         return getAllReadyTemplates(srcDataStore, childTemplates, templates);
     }
 
+    /**
+     * Returns whether a template should be migrated. A template should be migrated if:
+     * <ol>
+     * <li>its state is ready, and</li>
+     * <li>its hypervisor type is not simulator, and</li>
+     * <li>it is not a child template.</li>
+     * </ol>
+     */
+    protected boolean shouldMigrateTemplate(TemplateDataStoreVO template, VMTemplateVO templateVO) {
+        if (template.getState() != State.Ready) {
+            logger.debug("Template [{}] should not be migrated as it is not ready.", template);
+            return false;
+        }
+
+        if (templateVO.getHypervisorType() == Hypervisor.HypervisorType.Simulator) {
+            logger.debug("Template [{}] should not be migrated as its hypervisor type is simulator.", template);
+            return false;
+        }
+
+        if (templateVO.getParentTemplateId() != null) {
+            logger.debug("Template [{}] should not be migrated as it has a parent template.", template);
+            return false;
+        }
+
+        logger.debug("Template [{}] should be migrated.", template);
+        return true;
+    }
+
     /** Returns parent snapshots and snapshots that do not have any children; snapshotChains comprises of the snapshot chain info
      * for each parent snapshot and the cumulative size of the chain - this is done to ensure that all the snapshots in a chain
      * are migrated to the same datastore

engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtilityTest.java

@@ -0,0 +1,88 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.engine.orchestration;
+
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.VMTemplateVO;
+import junit.framework.TestCase;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class DataMigrationUtilityTest extends TestCase {
+
+    @Spy
+    private DataMigrationUtility dataMigrationUtility;
+
+    @Mock
+    private VMTemplateVO templateVoMock;
+
+    @Mock
+    private TemplateDataStoreVO templateDataStoreVoMock;
+
+    private void prepareForShouldMigrateTemplateTests() {
+        Mockito.when(templateDataStoreVoMock.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Ready);
+        Mockito.when(templateVoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM);
+        Mockito.when(templateVoMock.getParentTemplateId()).thenReturn(null);
+    }
+
+    @Test
+    public void shouldMigrateTemplateTestReturnsFalseWhenTemplateIsNotReady() {
+        prepareForShouldMigrateTemplateTests();
+        Mockito.when(templateDataStoreVoMock.getState()).thenReturn(ObjectInDataStoreStateMachine.State.Migrating);
+
+        boolean result = dataMigrationUtility.shouldMigrateTemplate(templateDataStoreVoMock, templateVoMock);
+
+        Assert.assertFalse(result);
+    }
+
+    @Test
+    public void shouldMigrateTemplateTestReturnsFalseWhenHypervisorTypeIsSimulator() {
+        prepareForShouldMigrateTemplateTests();
+        Mockito.when(templateVoMock.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.Simulator);
+
+        boolean result = dataMigrationUtility.shouldMigrateTemplate(templateDataStoreVoMock, templateVoMock);
+
+        Assert.assertFalse(result);
+    }
+
+    @Test
+    public void shouldMigrateTemplateTestReturnsFalseWhenTemplateHasParentTemplate() {
+        prepareForShouldMigrateTemplateTests();
+        Mockito.when(templateVoMock.getParentTemplateId()).thenReturn(1L);
+
+        boolean result = dataMigrationUtility.shouldMigrateTemplate(templateDataStoreVoMock, templateVoMock);
+
+        Assert.assertFalse(result);
+    }
+
+    @Test
+    public void shouldMigrateTemplateTestReturnsTrueWhenTemplateIsReadyAndDoesNotHaveParentTemplateAndHypervisorTypeIsNotSimulator() {
+        prepareForShouldMigrateTemplateTests();
+
+        boolean result = dataMigrationUtility.shouldMigrateTemplate(templateDataStoreVoMock, templateVoMock);
+
+        Assert.assertTrue(result);
+    }
+}

engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java

@@ -503,7 +503,7 @@ public void setRemoved(Date removed) {
 
     @Override
     public String toString() {
-        return String.format("VM instance %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "id", "instanceName", "uuid", "type"));
+        return String.format("VM instance %s", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "id", "instanceName", "uuid", "type", "state"));
     }
 
     @Override

engine/schema/src/main/resources/META-INF/db/schema-42000to42010.sql

@@ -48,7 +48,29 @@ CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.storage_pool', 'used_iops', 'bigint
 -- Add reason column for op_ha_work
 CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.op_ha_work', 'reason', 'varchar(32) DEFAULT NULL COMMENT "Reason for the HA work"');
 
--- Add featured column for guest_os_category
+-- Grant access to 2FA APIs for the "Read-Only User - Default" role
+
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only User - Default', 'setupUserTwoFactorAuthentication', 'ALLOW');
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only User - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW');
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only User - Default', 'listUserTwoFactorAuthenticatorProviders', 'ALLOW');
+
+-- Grant access to 2FA APIs for the "Support User - Default" role
+
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support User - Default', 'setupUserTwoFactorAuthentication', 'ALLOW');
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support User - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW');
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support User - Default', 'listUserTwoFactorAuthenticatorProviders', 'ALLOW');
+
+-- Grant access to 2FA APIs for the "Read-Only Admin - Default" role
+
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only Admin - Default', 'setupUserTwoFactorAuthentication', 'ALLOW');
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Read-Only Admin - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW');
+
+-- Grant access to 2FA APIs for the "Support Admin - Default" role
+
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support Admin - Default', 'setupUserTwoFactorAuthentication', 'ALLOW');
+CALL `cloud`.`IDEMPOTENT_UPDATE_API_PERMISSION`('Support Admin - Default', 'validateUserTwoFactorAuthenticationCode', 'ALLOW');
+
+-- Add featured and sort_key column for guest_os_category
 CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.guest_os_category', 'featured', 'tinyint(1) NOT NULL DEFAULT 0 COMMENT "whether the category is featured or not" AFTER `uuid`');
 CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.guest_os_category', 'sort_key', 'int NOT NULL DEFAULT 0 COMMENT "sort key used for customising sort method" AFTER `featured`');
-UPDATE `cloud`.`guest_os_category` SET `featured` = 1 WHERE `name` NOT IN ('Novel', 'None');
+UPDATE `cloud`.`guest_os_category` SET `featured` = 1 WHERE `name` NOT IN ('Novel', 'None');