From 71c9c90e0c81ea9e0f90ab71d3430d62cd8fa02e Mon Sep 17 00:00:00 2001 From: Nick Livens Date: Mon, 2 May 2016 13:16:14 +0200 Subject: [PATCH 1/2] CLOUDSTACK-9365 : updateVirtualMachine with userdata should not error when a VM is attached to multiple networks from which one or more doesn't support userdata --- .../src/com/cloud/vm/UserVmManagerImpl.java | 40 ++++++++------ .../test/com/cloud/vm/UserVmManagerTest.java | 52 +++++++++++++++++++ 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index cc007d90210..83ee2489c61 100644 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -2492,24 +2492,34 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return false; } + boolean userDataApplied = false; for (Nic nic : nics) { - Network network = _networkDao.findById(nic.getNetworkId()); - NicProfile nicProfile = new NicProfile(nic, network, null, null, null, _networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag( - template.getHypervisorType(), network)); - - VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(vm); - - UserDataServiceProvider element = _networkModel.getUserDataUpdateProvider(network); - if (element == null) { - throw new CloudRuntimeException("Can't find network element for " + Service.UserData.getName() + " provider needed for UserData update"); - } - boolean result = element.saveUserData(network, nicProfile, vmProfile); - if (!result) { - s_logger.error("Failed to update userdata for vm " + vm + " and nic " + nic); - } + userDataApplied |= applyUserData(template.getHypervisorType(), vm, nic); } + return userDataApplied; + } - return true; + protected boolean applyUserData(HypervisorType hyperVisorType, UserVm vm, Nic nic) throws ResourceUnavailableException, InsufficientCapacityException { + Network network = _networkDao.findById(nic.getNetworkId()); + NicProfile nicProfile = new NicProfile(nic, network, null, null, null, _networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag( + hyperVisorType, network)); + VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(vm); + + if (_networkModel.areServicesSupportedByNetworkOffering(network.getNetworkOfferingId(), Service.UserData)) { + UserDataServiceProvider element = _networkModel.getUserDataUpdateProvider(network); + if (element == null) { + throw new CloudRuntimeException("Can't find network element for " + Service.UserData.getName() + " provider needed for UserData update"); + } + boolean result = element.saveUserData(network, nicProfile, vmProfile); + if (!result) { + s_logger.error("Failed to update userdata for vm " + vm + " and nic " + nic); + } else { + return true; + } + } else { + s_logger.debug("Not applying userdata for nic id=" + nic.getId() + " in vm id=" + vmProfile.getId() + " because it is not supported in network id=" + network.getId()); + } + return false; } @Override diff --git a/server/test/com/cloud/vm/UserVmManagerTest.java b/server/test/com/cloud/vm/UserVmManagerTest.java index 637a3092219..9df3f689714 100644 --- a/server/test/com/cloud/vm/UserVmManagerTest.java +++ b/server/test/com/cloud/vm/UserVmManagerTest.java @@ -17,6 +17,8 @@ package com.cloud.vm; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyFloat; @@ -37,9 +39,11 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; +import com.cloud.network.element.UserDataServiceProvider; import com.cloud.storage.Storage; import com.cloud.user.User; import com.cloud.event.dao.UsageEventDao; +import com.cloud.uservm.UserVm; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -929,6 +933,54 @@ public class UserVmManagerTest { } } + @Test + public void testApplyUserDataInNetworkWithoutUserDataSupport() throws Exception { + UserVm userVm = mock(UserVm.class); + when(userVm.getId()).thenReturn(1L); + + when(_nicMock.getNetworkId()).thenReturn(2L); + when(_networkMock.getNetworkOfferingId()).thenReturn(3L); + when(_networkDao.findById(2L)).thenReturn(_networkMock); + + // No userdata support + assertFalse(_userVmMgr.applyUserData(HypervisorType.KVM, userVm, _nicMock)); + } + + @Test(expected = CloudRuntimeException.class) + public void testApplyUserDataInNetworkWithoutElement() throws Exception { + UserVm userVm = mock(UserVm.class); + when(userVm.getId()).thenReturn(1L); + + when(_nicMock.getNetworkId()).thenReturn(2L); + when(_networkMock.getNetworkOfferingId()).thenReturn(3L); + when(_networkDao.findById(2L)).thenReturn(_networkMock); + + UserDataServiceProvider userDataServiceProvider = mock(UserDataServiceProvider.class); + when(userDataServiceProvider.saveUserData(any(Network.class), any(NicProfile.class), any(VirtualMachineProfile.class))).thenReturn(true); + + // Userdata support, but no implementing element + when(_networkModel.areServicesSupportedByNetworkOffering(3L, Service.UserData)).thenReturn(true); + _userVmMgr.applyUserData(HypervisorType.KVM, userVm, _nicMock); + } + + @Test + public void testApplyUserDataSuccessful() throws Exception { + UserVm userVm = mock(UserVm.class); + when(userVm.getId()).thenReturn(1L); + + when(_nicMock.getNetworkId()).thenReturn(2L); + when(_networkMock.getNetworkOfferingId()).thenReturn(3L); + when(_networkDao.findById(2L)).thenReturn(_networkMock); + + UserDataServiceProvider userDataServiceProvider = mock(UserDataServiceProvider.class); + when(userDataServiceProvider.saveUserData(any(Network.class), any(NicProfile.class), any(VirtualMachineProfile.class))).thenReturn(true); + + // Userdata support with implementing element + when(_networkModel.areServicesSupportedByNetworkOffering(3L, Service.UserData)).thenReturn(true); + when(_networkModel.getUserDataUpdateProvider(_networkMock)).thenReturn(userDataServiceProvider); + assertTrue(_userVmMgr.applyUserData(HypervisorType.KVM, userVm, _nicMock)); + } + @Test public void testPersistDeviceBusInfoWithNullController() { when(_vmMock.getDetail(any(String.class))).thenReturn(null); From 9df51faa8a2f0678e7927ac6ef23752b1eae8f71 Mon Sep 17 00:00:00 2001 From: rahul singal Date: Mon, 9 May 2016 13:24:53 +0200 Subject: [PATCH 2/2] Marvin script for cloudstack-9365 --- .../test_deploy_vm_userdata_multi_nic.py | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100755 test/integration/component/test_deploy_vm_userdata_multi_nic.py diff --git a/test/integration/component/test_deploy_vm_userdata_multi_nic.py b/test/integration/component/test_deploy_vm_userdata_multi_nic.py new file mode 100755 index 00000000000..86c0f1317bf --- /dev/null +++ b/test/integration/component/test_deploy_vm_userdata_multi_nic.py @@ -0,0 +1,188 @@ +# 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. + + +# this script will cover VMdeployment with Userdata tests for MultiNic + +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import (Account, + Network, + NetworkOffering, + ServiceOffering, + VirtualMachine) +from marvin.lib.common import (get_domain, + get_template, + get_zone, + list_virtual_machines) +from marvin.lib.utils import cleanup_resources +from nose.plugins.attrib import attr +import base64 +import random +import string + +_multiprocess_shared_ = True + + +class TestDeployVmWithUserDataMultiNic(cloudstackTestCase): + """Tests for UserData + """ + + @classmethod + def setUpClass(cls): + cls.testClient = super(TestDeployVmWithUserDataMultiNic, cls).getClsTestClient() + cls.api_client = cls.testClient.getApiClient() + cls.test_data = cls.testClient.getParsedTestDataConfig() + + # Get Domain, Zone, Template + cls.domain = get_domain(cls.api_client) + cls.zone = get_zone( + cls.api_client, + cls.testClient.getZoneForTests()) + cls.template = get_template( + cls.api_client, + cls.zone.id, + cls.test_data["ostype"] + ) + if cls.zone.localstorageenabled: + cls.storagetype = 'local' + cls.test_data["service_offerings"][ + "tiny"]["storagetype"] = 'local' + else: + cls.storagetype = 'shared' + cls.test_data["service_offerings"][ + "tiny"]["storagetype"] = 'shared' + + cls.service_offering = ServiceOffering.create( + cls.api_client, + cls.test_data["service_offerings"]["tiny"] + ) + + # Create Network offering without userdata + cls.network_offering_nouserdata = NetworkOffering.create( + cls.api_client, + cls.test_data["network_offering"] + ) + # Enable Network offering + cls.network_offering_nouserdata.update(cls.api_client, state='Enabled') + + # Create Network Offering with all the serices + cls.network_offering_all = NetworkOffering.create( + cls.api_client, + cls.test_data["isolated_network_offering"] + ) + # Enable Network offering + cls.network_offering_all.update(cls.api_client, state='Enabled') + + cls._cleanup = [ + cls.service_offering, + cls.network_offering_nouserdata, + cls.network_offering_all + ] + + # Generate userdata of 2500 bytes. This is larger than the 2048 bytes limit. + # CS however allows for upto 4K bytes in the code. So this must succeed. + # Overall, the query length must not exceed 4K, for then the json decoder + # will fail this operation at the marvin client side itcls. + + cls.userdata = ''.join(random.choice(string.ascii_uppercase + string.digits) for x in range(2500)) + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.hypervisor = self.testClient.getHypervisorInfo() + self.dbclient = self.testClient.getDbConnection() + self.account = Account.create( + self.apiclient, + self.test_data["account"], + admin=True, + domainid=self.domain.id + ) + self.cleanup = [] + return + + def tearDown(self): + try: + self.account.delete(self.apiclient) + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + @attr(tags=["simulator", "devcloud", "basic", "advanced"], required_hardware="false") + def test_deployvm_multinic(self): + """Test userdata update when non default nic is without userdata for deploy and update + """ + + self.userdata = base64.b64encode(self.userdata) + + network1 = Network.create( + self.apiclient, + self.test_data["isolated_network"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=self.network_offering_all.id, + zoneid=self.zone.id + ) + + self.test_data["network_without_acl"]["netmask"] = "255.255.255.128" + + network2 = Network.create( + self.apiclient, + self.test_data["network_without_acl"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=self.network_offering_nouserdata.id, + gateway="10.2.1.1", + zoneid=self.zone.id + ) + + deployVmResponse = VirtualMachine.create( + self.apiclient, + services=self.test_data["virtual_machine_userdata"], + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id, + networkids=[str(network1.id), str(network2.id)], + templateid=self.template.id, + zoneid=self.zone.id + ) + + vms = list_virtual_machines( + self.apiclient, + account=self.account.name, + domainid=self.account.domainid, + id=deployVmResponse.id + ) + self.assert_(len(vms) > 0, "There are no Vms deployed in the account %s" % self.account.name) + vm = vms[0] + self.assert_(vm.id == str(deployVmResponse.id), "Vm deployed is different from the test") + self.assert_(vm.state == "Running", "VM is not in Running state") + + try: + updateresponse = deployVmResponse.update(self.apiclient, userdata=self.userdata) + except Exception as e: + self.fail("Failed to update userdata: %s" % e) + + self.debug("virtual machine update response is: %s" % updateresponse) + + @classmethod + def tearDownClass(cls): + try: + # Cleanup resources used + cleanup_resources(cls.api_client, cls._cleanup) + + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e)