CLOUDSTACK-10013: Fixes based on code review and test failures

This includes test related fixes and code review fixes based on
reviews from @rafaelweingartner, @marcaurele, @wido and @DaanHoogland.

This also includes VMware disk-resize limitation bug fix based on comments
from @sateesh-chodapuneedi and @priyankparihar.

This also includes the final changes to systemvmtemplate and fixes to
code based on issues found via test failures.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

Author: yadvr

LICENSE

@@ -271,65 +271,6 @@ Within the scripts/vm/hypervisor/xenserver directory
         from OpenStack, LLC  http://www.openstack.org 
             swift 
 
-Within the tools/appliance/definitions/{devcloud,systemvmtemplate,systemvmtemplate64} directory
-    licensed under the MIT License http://www.opensource.org/licenses/mit-license.php  (as follows)
-
-            Copyright (c) 2010-2012 Patrick Debois 
-            
-            Permission is hereby granted, free  of charge, to any person obtaining
-            a  copy  of this  software  and  associated  documentation files  (the
-            "Software"), to  deal in  the Software without  restriction, including
-            without limitation  the rights to  use, copy, modify,  merge, publish,
-            distribute,  sublicense, and/or sell  copies of  the Software,  and to
-            permit persons to whom the Software  is furnished to do so, subject to
-            the following conditions:
-            
-            The  above  copyright  notice  and  this permission  notice  shall  be
-            included in all copies or substantial portions of the Software.
-            
-            THE  SOFTWARE IS  PROVIDED  "AS  IS", WITHOUT  WARRANTY  OF ANY  KIND,
-            EXPRESS OR  IMPLIED, INCLUDING  BUT NOT LIMITED  TO THE  WARRANTIES OF
-            MERCHANTABILITY,    FITNESS    FOR    A   PARTICULAR    PURPOSE    AND
-            NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
-            LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
-            OF CONTRACT, TORT OR OTHERWISE,  ARISING FROM, OUT OF OR IN CONNECTION
-            WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-                        
-        from Patrick Debois  http://www.jedi.be/blog/ 
-            base.sh  from https://github.com/jedi4ever/veewee
-            cleanup.sh  from https://github.com/jedi4ever/veewee
-            definition.rb  from https://github.com/jedi4ever/veewee
-            preseed.cfg  from https://github.com/jedi4ever/veewee
-            zerodisk.sh  from https://github.com/jedi4ever/veewee
-
-Within the tools/devcloud/src/deps/boxes/basebox-build directory
-    licensed under the MIT License http://www.opensource.org/licenses/mit-license.php  (as follows)
-
-            Copyright (c) 2010-2012 Patrick Debois 
-            
-            Permission is hereby granted, free  of charge, to any person obtaining
-            a  copy  of this  software  and  associated  documentation files  (the
-            "Software"), to  deal in  the Software without  restriction, including
-            without limitation  the rights to  use, copy, modify,  merge, publish,
-            distribute,  sublicense, and/or sell  copies of  the Software,  and to
-            permit persons to whom the Software  is furnished to do so, subject to
-            the following conditions:
-            
-            The  above  copyright  notice  and  this permission  notice  shall  be
-            included in all copies or substantial portions of the Software.
-            
-            THE  SOFTWARE IS  PROVIDED  "AS  IS", WITHOUT  WARRANTY  OF ANY  KIND,
-            EXPRESS OR  IMPLIED, INCLUDING  BUT NOT LIMITED  TO THE  WARRANTIES OF
-            MERCHANTABILITY,    FITNESS    FOR    A   PARTICULAR    PURPOSE    AND
-            NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
-            LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
-            OF CONTRACT, TORT OR OTHERWISE,  ARISING FROM, OUT OF OR IN CONNECTION
-            WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-                        
-        from Patrick Debois  http://www.jedi.be/blog/ 
-            definition.rb  from https://github.com/jedi4ever/veewee
-            preseed.cfg  from https://github.com/jedi4ever/veewee
-
 Within the ui/lib directory
     placed in the public domain
         by Eric Meyer  http://meyerweb.com/eric/ 

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

@@ -839,6 +839,21 @@ public void advanceStart(final String vmUuid, final Map<VirtualMachineProfile.Pa
         }
     }
 
+    private void setupAgentSecurity(final Host vmHost, final Map<String, String> sshAccessDetails, final VirtualMachine vm) throws AgentUnavailableException, OperationTimedoutException {
+        final String csr = caManager.generateKeyStoreAndCsr(vmHost, sshAccessDetails);
+        if (!Strings.isNullOrEmpty(csr)) {
+            final Map<String, String> ipAddressDetails = new HashMap<>(sshAccessDetails);
+            ipAddressDetails.remove(NetworkElementCommand.ROUTER_NAME);
+            final Certificate certificate = caManager.issueCertificate(csr, Arrays.asList(vm.getHostName(), vm.getInstanceName()),
+                    new ArrayList<>(ipAddressDetails.values()), CAManager.CertValidityPeriod.value(), null);
+            final boolean result = caManager.deployCertificate(vmHost, certificate, false, sshAccessDetails);
+            if (!result) {
+                s_logger.error("Failed to setup certificate for system vm: " + vm.getInstanceName());
+            }
+        } else {
+            s_logger.error("Failed to setup keystore and generate CSR for system vm: " + vm.getInstanceName());
+        }
+    }
 
     @Override
     public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfile.Param, Object> params, final DeploymentPlan planToDeploy, final DeploymentPlanner planner)
@@ -1088,18 +1103,15 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
                             if (vmHost != null && (VirtualMachine.Type.ConsoleProxy.equals(vm.getType()) ||
                                     VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())) && caManager.canProvisionCertificates()) {
                                 final Map<String, String> sshAccessDetails = _networkMgr.getSystemVMAccessDetails(vm);
-                                final String csr = caManager.generateKeyStoreAndCsr(vmHost, sshAccessDetails);
-                                if (!Strings.isNullOrEmpty(csr)) {
-                                    final Map<String, String> ipAddressDetails = new HashMap<>(sshAccessDetails);
-                                    ipAddressDetails.remove(NetworkElementCommand.ROUTER_NAME);
-                                    final Certificate certificate = caManager.issueCertificate(csr, Arrays.asList(vm.getHostName(), vm.getInstanceName()), new ArrayList<>(ipAddressDetails.values()), CAManager.CertValidityPeriod.value(), null);
-                                    final boolean result = caManager.deployCertificate(vmHost, certificate, false, sshAccessDetails);
-                                    if (!result) {
-                                        s_logger.error("Failed to setup certificate for system vm: " + vm.getInstanceName());
+                                for (int retries = 3; retries > 0; retries--) {
+                                    try {
+                                        setupAgentSecurity(vmHost, sshAccessDetails, vm);
+                                        return;
+                                    } catch (final Exception e) {
+                                        s_logger.error("Retrying after catching exception while trying to secure agent for systemvm id=" + vm.getId(), e);
                                     }
-                                } else {
-                                    s_logger.error("Failed to setup keystore and generate CSR for system vm: " + vm.getInstanceName());
                                 }
+                                throw new CloudRuntimeException("Failed to setup and secure agent for systemvm id=" + vm.getId());
                             }
                             return;
                         } else {

engine/schema/resources/META-INF/db/schema-41000to41100.sql

@@ -493,9 +493,6 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis
 -- Change monitor patch for apache2 in systemvm
 UPDATE `cloud`.`monitoring_services` SET pidfile="/var/run/apache2/apache2.pid" WHERE process_name="apache2" AND service_name="apache2";
 
--- Boost secondary storage systemvm
-UPDATE `cloud`.`service_offering` SET ram_size=1024, cpu=2 WHERE vm_type="secondarystoragevm" and cpu=1 and ram_size=512;
-
 -- Use 'Other Linux 64-bit' as guest os for the default systemvmtemplate for VMware
 -- This fixes a memory allocation issue to systemvms on VMware/ESXi
 UPDATE `cloud`.`vm_template` SET guest_os_id=99 WHERE id=8;

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java

@@ -102,6 +102,7 @@
 import com.vmware.vim25.VirtualMachineVideoCard;
 import com.vmware.vim25.VmwareDistributedVirtualSwitchVlanIdSpec;
 
+import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.storage.command.CopyCommand;
 import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
 import org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource;
@@ -2178,8 +2179,9 @@ protected StartAnswer execute(StartCommand cmd) {
                 hyperHost.setRestartPriorityForVM(vmMo, DasVmPriority.HIGH.value());
             }
 
-            // For resizing root disk.
-            if (rootDiskTO != null && !hasSnapshot) {
+            // Resizing root disk only when explicit requested by user
+            final Map<String, String> vmDetails = cmd.getVirtualMachine().getDetails();
+            if (rootDiskTO != null && !hasSnapshot && (vmDetails != null && vmDetails.containsKey(ApiConstants.ROOT_DISK_SIZE))) {
                 resizeRootDiskOnVMStart(vmMo, rootDiskTO, hyperHost, context);
             }
 
@@ -2254,20 +2256,24 @@ private void resizeRootDiskOnVMStart(VirtualMachineMO vmMo, DiskTO rootDiskTO, V
         final Pair<VirtualDisk, String> vdisk = getVirtualDiskInfo(vmMo, appendFileType(rootDiskTO.getPath(), ".vmdk"));
         assert(vdisk != null);
 
-        final Long reqSize = ((VolumeObjectTO)rootDiskTO.getData()).getSize() / 1024;
+        Long reqSize = 0L;
+        final VolumeObjectTO volumeTO = ((VolumeObjectTO)rootDiskTO.getData());
+        if (volumeTO != null) {
+            reqSize = volumeTO.getSize() / 1024;
+        }
         final VirtualDisk disk = vdisk.first();
         if (reqSize > disk.getCapacityInKB()) {
             final VirtualMachineDiskInfo diskInfo = getMatchingExistingDisk(vmMo.getDiskInfoBuilder(), rootDiskTO, hyperHost, context);
             assert (diskInfo != null);
             final String[] diskChain = diskInfo.getDiskChain();
 
             if (diskChain != null && diskChain.length > 1) {
-                s_logger.warn("Disk chain length for the VM is greater than one, skipping resizing of root disk.");
-                return;
+                s_logger.warn("Disk chain length for the VM is greater than one, this is not supported");
+                throw new CloudRuntimeException("Unsupported VM disk chain length: "+ diskChain.length);
             }
             if (diskInfo.getDiskDeviceBusName() == null || !diskInfo.getDiskDeviceBusName().toLowerCase().startsWith("scsi")) {
-                s_logger.warn("Resizing of root disk is only support for scsi device/bus, the provide disk's device bus name is " + diskInfo.getDiskDeviceBusName());
-                return;
+                s_logger.warn("Resizing of root disk is only support for scsi device/bus, the provide VM's disk device bus name is " + diskInfo.getDiskDeviceBusName());
+                throw new CloudRuntimeException("Unsupported VM root disk device bus: "+ diskInfo.getDiskDeviceBusName());
             }
 
             disk.setCapacityInKB(reqSize);

pom.xml

@@ -883,8 +883,6 @@
               <exclude>tools/devcloud/basebuild/puppet-devcloudinitial/files/network.conf</exclude>
               <exclude>tools/appliance/*/template.json</exclude>
               <exclude>tools/cli/cloudmonkey.egg-info/*</exclude>
-              <exclude>tools/devcloud/src/deps/boxes/basebox-build/definition.rb</exclude>
-              <exclude>tools/devcloud/src/deps/boxes/basebox-build/preseed.cfg</exclude>
               <exclude>tools/marvin/Marvin.egg-info/*</exclude>
               <exclude>ui/css/token-input-facebook.css</exclude>
               <exclude>ui/l10n/*</exclude>

python/lib/cloudutils/utilities.py

@@ -217,10 +217,7 @@ class serviceOpsRedhat7(serviceOps):
     def isServiceRunning(self, servicename):
         try:
             o = bash("systemctl is-active " + servicename)
-            if "inactive" not in o.getStdout():
-                return True
-            else:
-                return False
+            return "inactive" not in o.getStdout()
         except:
             return False
 

scripts/util/keystore-cert-import

@@ -89,9 +89,7 @@ fi
 
 # Restart cloud service if we're in systemvm
 if [ "$MODE" == "ssh" ] && [ -f $SYSTEM_FILE ]; then
-    /etc/init.d/cloud stop > /dev/null 2>&1
-    sleep 2
-    /etc/init.d/cloud start > /dev/null 2>&1
+    systemctl restart cloud > /dev/null 2>&1
 fi
 
 # Fix file permission

scripts/util/keystore-setup

@@ -38,11 +38,11 @@ fi
 # Generate keystore
 rm -f "$KS_FILE"
 CN=$(hostname --fqdn)
-keytool -genkey -storepass "$KS_PASS" -keypass "$KS_PASS" -alias "$ALIAS" -keyalg RSA -validity "$KS_VALIDITY" -dname cn="$CN",ou="cloudstack",o="cloudstack",c="cloudstack" -keystore "$KS_FILE"
+keytool -genkey -storepass "$KS_PASS" -keypass "$KS_PASS" -alias "$ALIAS" -keyalg RSA -validity "$KS_VALIDITY" -dname cn="$CN",ou="cloudstack",o="cloudstack",c="cloudstack" -keystore "$KS_FILE" > /dev/null 2>&1
 
 # Generate CSR
 rm -f "$CSR_FILE"
-keytool -certreq -storepass "$KS_PASS" -alias "$ALIAS" -file $CSR_FILE -keystore "$KS_FILE"
+keytool -certreq -storepass "$KS_PASS" -alias "$ALIAS" -file $CSR_FILE -keystore "$KS_FILE" > /dev/null 2>&1
 cat "$CSR_FILE"
 
 # Fix file permissions

server/src/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java

@@ -29,6 +29,7 @@
 
 import org.apache.cloudstack.ca.CAManager;
 import org.apache.cloudstack.ca.SetupCertificateCommand;
+import org.apache.cloudstack.config.ApiServiceConfiguration;
 import org.apache.cloudstack.framework.ca.Certificate;
 import org.apache.cloudstack.utils.security.KeyStoreUtils;
 import org.apache.log4j.Logger;
@@ -66,7 +67,6 @@
 
 public abstract class LibvirtServerDiscoverer extends DiscovererBase implements Discoverer, Listener, ResourceStateAdapter {
     private static final Logger s_logger = Logger.getLogger(LibvirtServerDiscoverer.class);
-    private String _hostIp;
     private final int _waitTime = 5; /* wait for 5 minutes */
     private String _kvmPrivateNic;
     private String _kvmPublicNic;
@@ -291,7 +291,7 @@ private void setupAgentSecurity(final Connection sshConnection, final String age
 
             setupAgentSecurity(sshConnection, agentIp, hostname);
 
-            String parameters = " -m " + StringUtils.shuffleCSVList(_hostIp) + " -z " + dcId + " -p " + podId + " -c " + clusterId + " -g " + guid + " -a";
+            String parameters = " -m " + StringUtils.shuffleCSVList(ApiServiceConfiguration.ManagementHostIPAdr.value()) + " -z " + dcId + " -p " + podId + " -c " + clusterId + " -g " + guid + " -a";
 
             parameters += " --pubNic=" + kvmPublicNic;
             parameters += " --prvNic=" + kvmPrivateNic;
@@ -395,10 +395,6 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
             _kvmGuestNic = _kvmPrivateNic;
         }
 
-        _hostIp = _configDao.getValue("host");
-        if (_hostIp == null) {
-            throw new ConfigurationException("Can't get host IP");
-        }
         _resourceMgr.registerResourceStateAdapter(this.getClass().getSimpleName(), this);
         return true;
     }

server/src/com/cloud/network/IpAddressManagerImpl.java

@@ -824,7 +824,7 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
                         if (userIp.getState() == IpAddress.State.Free) {
                             addr.setState(IpAddress.State.Allocating);
                             if (_ipAddressDao.update(addr.getId(), addr)) {
-                                finalAddr = _ipAddressDao.findById(addr.getId());
+                                finalAddr = addr;
                                 break;
                             }
                         }