Skip to content

Commit d19629a

Browse files
yadvryadvr
committed
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>
1 parent 4338e0f commit d19629a

File tree

51 files changed

+191
-480
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+191
-480
lines changed

‎LICENSE‎

Lines changed: 0 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -271,65 +271,6 @@ Within the scripts/vm/hypervisor/xenserver directory
271271
from OpenStack, LLC http://www.openstack.org
272272
swift
273273

274-
Within the tools/appliance/definitions/{devcloud,systemvmtemplate,systemvmtemplate64} directory
275-
licensed under the MIT License http://www.opensource.org/licenses/mit-license.php (as follows)
276-
277-
Copyright (c) 2010-2012 Patrick Debois
278-
279-
Permission is hereby granted, free of charge, to any person obtaining
280-
a copy of this software and associated documentation files (the
281-
"Software"), to deal in the Software without restriction, including
282-
without limitation the rights to use, copy, modify, merge, publish,
283-
distribute, sublicense, and/or sell copies of the Software, and to
284-
permit persons to whom the Software is furnished to do so, subject to
285-
the following conditions:
286-
287-
The above copyright notice and this permission notice shall be
288-
included in all copies or substantial portions of the Software.
289-
290-
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
291-
EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
292-
MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
293-
NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
294-
LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
295-
OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
296-
WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
297-
298-
from Patrick Debois http://www.jedi.be/blog/
299-
base.sh from https://github.com/jedi4ever/veewee
300-
cleanup.sh from https://github.com/jedi4ever/veewee
301-
definition.rb from https://github.com/jedi4ever/veewee
302-
preseed.cfg from https://github.com/jedi4ever/veewee
303-
zerodisk.sh from https://github.com/jedi4ever/veewee
304-
305-
Within the tools/devcloud/src/deps/boxes/basebox-build directory
306-
licensed under the MIT License http://www.opensource.org/licenses/mit-license.php (as follows)
307-
308-
Copyright (c) 2010-2012 Patrick Debois
309-
310-
Permission is hereby granted, free of charge, to any person obtaining
311-
a copy of this software and associated documentation files (the
312-
"Software"), to deal in the Software without restriction, including
313-
without limitation the rights to use, copy, modify, merge, publish,
314-
distribute, sublicense, and/or sell copies of the Software, and to
315-
permit persons to whom the Software is furnished to do so, subject to
316-
the following conditions:
317-
318-
The above copyright notice and this permission notice shall be
319-
included in all copies or substantial portions of the Software.
320-
321-
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
322-
EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
323-
MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
324-
NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
325-
LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
326-
OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
327-
WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
328-
329-
from Patrick Debois http://www.jedi.be/blog/
330-
definition.rb from https://github.com/jedi4ever/veewee
331-
preseed.cfg from https://github.com/jedi4ever/veewee
332-
333274
Within the ui/lib directory
334275
placed in the public domain
335276
by Eric Meyer http://meyerweb.com/eric/

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,21 @@ public void advanceStart(final String vmUuid, final Map<VirtualMachineProfile.Pa
839839
}
840840
}
841841

842+
private void setupAgentSecurity(final Host vmHost, final Map<String, String> sshAccessDetails, final VirtualMachine vm) throws AgentUnavailableException, OperationTimedoutException {
843+
final String csr = caManager.generateKeyStoreAndCsr(vmHost, sshAccessDetails);
844+
if (!Strings.isNullOrEmpty(csr)) {
845+
final Map<String, String> ipAddressDetails = new HashMap<>(sshAccessDetails);
846+
ipAddressDetails.remove(NetworkElementCommand.ROUTER_NAME);
847+
final Certificate certificate = caManager.issueCertificate(csr, Arrays.asList(vm.getHostName(), vm.getInstanceName()),
848+
new ArrayList<>(ipAddressDetails.values()), CAManager.CertValidityPeriod.value(), null);
849+
final boolean result = caManager.deployCertificate(vmHost, certificate, false, sshAccessDetails);
850+
if (!result) {
851+
s_logger.error("Failed to setup certificate for system vm: " + vm.getInstanceName());
852+
}
853+
} else {
854+
s_logger.error("Failed to setup keystore and generate CSR for system vm: " + vm.getInstanceName());
855+
}
856+
}
842857

843858
@Override
844859
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
10881103
if (vmHost != null && (VirtualMachine.Type.ConsoleProxy.equals(vm.getType()) ||
10891104
VirtualMachine.Type.SecondaryStorageVm.equals(vm.getType())) && caManager.canProvisionCertificates()) {
10901105
final Map<String, String> sshAccessDetails = _networkMgr.getSystemVMAccessDetails(vm);
1091-
final String csr = caManager.generateKeyStoreAndCsr(vmHost, sshAccessDetails);
1092-
if (!Strings.isNullOrEmpty(csr)) {
1093-
final Map<String, String> ipAddressDetails = new HashMap<>(sshAccessDetails);
1094-
ipAddressDetails.remove(NetworkElementCommand.ROUTER_NAME);
1095-
final Certificate certificate = caManager.issueCertificate(csr, Arrays.asList(vm.getHostName(), vm.getInstanceName()), new ArrayList<>(ipAddressDetails.values()), CAManager.CertValidityPeriod.value(), null);
1096-
final boolean result = caManager.deployCertificate(vmHost, certificate, false, sshAccessDetails);
1097-
if (!result) {
1098-
s_logger.error("Failed to setup certificate for system vm: " + vm.getInstanceName());
1106+
for (int retries = 3; retries > 0; retries--) {
1107+
try {
1108+
setupAgentSecurity(vmHost, sshAccessDetails, vm);
1109+
return;
1110+
} catch (final Exception e) {
1111+
s_logger.error("Retrying after catching exception while trying to secure agent for systemvm id=" + vm.getId(), e);
10991112
}
1100-
} else {
1101-
s_logger.error("Failed to setup keystore and generate CSR for system vm: " + vm.getInstanceName());
11021113
}
1114+
throw new CloudRuntimeException("Failed to setup and secure agent for systemvm id=" + vm.getId());
11031115
}
11041116
return;
11051117
} else {

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -493,9 +493,6 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis
493493
-- Change monitor patch for apache2 in systemvm
494494
UPDATE `cloud`.`monitoring_services` SET pidfile="/var/run/apache2/apache2.pid" WHERE process_name="apache2" AND service_name="apache2";
495495

496-
-- Boost secondary storage systemvm
497-
UPDATE `cloud`.`service_offering` SET ram_size=1024, cpu=2 WHERE vm_type="secondarystoragevm" and cpu=1 and ram_size=512;
498-
499496
-- Use 'Other Linux 64-bit' as guest os for the default systemvmtemplate for VMware
500497
-- This fixes a memory allocation issue to systemvms on VMware/ESXi
501498
UPDATE `cloud`.`vm_template` SET guest_os_id=99 WHERE id=8;

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@
102102
import com.vmware.vim25.VirtualMachineVideoCard;
103103
import com.vmware.vim25.VmwareDistributedVirtualSwitchVlanIdSpec;
104104

105+
import org.apache.cloudstack.api.ApiConstants;
105106
import org.apache.cloudstack.storage.command.CopyCommand;
106107
import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
107108
import org.apache.cloudstack.storage.resource.NfsSecondaryStorageResource;
@@ -2178,8 +2179,9 @@ protected StartAnswer execute(StartCommand cmd) {
21782179
hyperHost.setRestartPriorityForVM(vmMo, DasVmPriority.HIGH.value());
21792180
}
21802181

2181-
// For resizing root disk.
2182-
if (rootDiskTO != null && !hasSnapshot) {
2182+
// Resizing root disk only when explicit requested by user
2183+
final Map<String, String> vmDetails = cmd.getVirtualMachine().getDetails();
2184+
if (rootDiskTO != null && !hasSnapshot && (vmDetails != null && vmDetails.containsKey(ApiConstants.ROOT_DISK_SIZE))) {
21832185
resizeRootDiskOnVMStart(vmMo, rootDiskTO, hyperHost, context);
21842186
}
21852187

@@ -2254,20 +2256,24 @@ private void resizeRootDiskOnVMStart(VirtualMachineMO vmMo, DiskTO rootDiskTO, V
22542256
final Pair<VirtualDisk, String> vdisk = getVirtualDiskInfo(vmMo, appendFileType(rootDiskTO.getPath(), ".vmdk"));
22552257
assert(vdisk != null);
22562258

2257-
final Long reqSize = ((VolumeObjectTO)rootDiskTO.getData()).getSize() / 1024;
2259+
Long reqSize = 0L;
2260+
final VolumeObjectTO volumeTO = ((VolumeObjectTO)rootDiskTO.getData());
2261+
if (volumeTO != null) {
2262+
reqSize = volumeTO.getSize() / 1024;
2263+
}
22582264
final VirtualDisk disk = vdisk.first();
22592265
if (reqSize > disk.getCapacityInKB()) {
22602266
final VirtualMachineDiskInfo diskInfo = getMatchingExistingDisk(vmMo.getDiskInfoBuilder(), rootDiskTO, hyperHost, context);
22612267
assert (diskInfo != null);
22622268
final String[] diskChain = diskInfo.getDiskChain();
22632269

22642270
if (diskChain != null && diskChain.length > 1) {
2265-
s_logger.warn("Disk chain length for the VM is greater than one, skipping resizing of root disk.");
2266-
return;
2271+
s_logger.warn("Disk chain length for the VM is greater than one, this is not supported");
2272+
throw new CloudRuntimeException("Unsupported VM disk chain length: "+ diskChain.length);
22672273
}
22682274
if (diskInfo.getDiskDeviceBusName() == null || !diskInfo.getDiskDeviceBusName().toLowerCase().startsWith("scsi")) {
2269-
s_logger.warn("Resizing of root disk is only support for scsi device/bus, the provide disk's device bus name is " + diskInfo.getDiskDeviceBusName());
2270-
return;
2275+
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());
2276+
throw new CloudRuntimeException("Unsupported VM root disk device bus: "+ diskInfo.getDiskDeviceBusName());
22712277
}
22722278

22732279
disk.setCapacityInKB(reqSize);

‎pom.xml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -883,8 +883,6 @@
883883
<exclude>tools/devcloud/basebuild/puppet-devcloudinitial/files/network.conf</exclude>
884884
<exclude>tools/appliance/*/template.json</exclude>
885885
<exclude>tools/cli/cloudmonkey.egg-info/*</exclude>
886-
<exclude>tools/devcloud/src/deps/boxes/basebox-build/definition.rb</exclude>
887-
<exclude>tools/devcloud/src/deps/boxes/basebox-build/preseed.cfg</exclude>
888886
<exclude>tools/marvin/Marvin.egg-info/*</exclude>
889887
<exclude>ui/css/token-input-facebook.css</exclude>
890888
<exclude>ui/l10n/*</exclude>

‎python/lib/cloudutils/utilities.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,10 +217,7 @@ class serviceOpsRedhat7(serviceOps):
217217
def isServiceRunning(self, servicename):
218218
try:
219219
o = bash("systemctl is-active " + servicename)
220-
if "inactive" not in o.getStdout():
221-
return True
222-
else:
223-
return False
220+
return "inactive" not in o.getStdout()
224221
except:
225222
return False
226223

‎scripts/util/keystore-cert-import‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,7 @@ fi
8989

9090
# Restart cloud service if we're in systemvm
9191
if [ "$MODE" == "ssh" ] && [ -f $SYSTEM_FILE ]; then
92-
/etc/init.d/cloud stop > /dev/null 2>&1
93-
sleep 2
94-
/etc/init.d/cloud start > /dev/null 2>&1
92+
systemctl restart cloud > /dev/null 2>&1
9593
fi
9694

9795
# Fix file permission

‎scripts/util/keystore-setup‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ fi
3838
# Generate keystore
3939
rm -f "$KS_FILE"
4040
CN=$(hostname --fqdn)
41-
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"
41+
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
4242

4343
# Generate CSR
4444
rm -f "$CSR_FILE"
45-
keytool -certreq -storepass "$KS_PASS" -alias "$ALIAS" -file $CSR_FILE -keystore "$KS_FILE"
45+
keytool -certreq -storepass "$KS_PASS" -alias "$ALIAS" -file $CSR_FILE -keystore "$KS_FILE" > /dev/null 2>&1
4646
cat "$CSR_FILE"
4747

4848
# Fix file permissions

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
import org.apache.cloudstack.ca.CAManager;
3131
import org.apache.cloudstack.ca.SetupCertificateCommand;
32+
import org.apache.cloudstack.config.ApiServiceConfiguration;
3233
import org.apache.cloudstack.framework.ca.Certificate;
3334
import org.apache.cloudstack.utils.security.KeyStoreUtils;
3435
import org.apache.log4j.Logger;
@@ -66,7 +67,6 @@
6667

6768
public abstract class LibvirtServerDiscoverer extends DiscovererBase implements Discoverer, Listener, ResourceStateAdapter {
6869
private static final Logger s_logger = Logger.getLogger(LibvirtServerDiscoverer.class);
69-
private String _hostIp;
7070
private final int _waitTime = 5; /* wait for 5 minutes */
7171
private String _kvmPrivateNic;
7272
private String _kvmPublicNic;
@@ -291,7 +291,7 @@ private void setupAgentSecurity(final Connection sshConnection, final String age
291291

292292
setupAgentSecurity(sshConnection, agentIp, hostname);
293293

294-
String parameters = " -m " + StringUtils.shuffleCSVList(_hostIp) + " -z " + dcId + " -p " + podId + " -c " + clusterId + " -g " + guid + " -a";
294+
String parameters = " -m " + StringUtils.shuffleCSVList(ApiServiceConfiguration.ManagementHostIPAdr.value()) + " -z " + dcId + " -p " + podId + " -c " + clusterId + " -g " + guid + " -a";
295295

296296
parameters += " --pubNic=" + kvmPublicNic;
297297
parameters += " --prvNic=" + kvmPrivateNic;
@@ -395,10 +395,6 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
395395
_kvmGuestNic = _kvmPrivateNic;
396396
}
397397

398-
_hostIp = _configDao.getValue("host");
399-
if (_hostIp == null) {
400-
throw new ConfigurationException("Can't get host IP");
401-
}
402398
_resourceMgr.registerResourceStateAdapter(this.getClass().getSimpleName(), this);
403399
return true;
404400
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,7 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
824824
if (userIp.getState() == IpAddress.State.Free) {
825825
addr.setState(IpAddress.State.Allocating);
826826
if (_ipAddressDao.update(addr.getId(), addr)) {
827-
finalAddr = _ipAddressDao.findById(addr.getId());
827+
finalAddr = addr;
828828
break;
829829
}
830830
}

0 commit comments

Comments
 (0)