From d730efb86681b2206b82cbacc828e77e1204367b Mon Sep 17 00:00:00 2001 From: wilderrodrigues Date: Thu, 23 Apr 2015 11:36:38 +0200 Subject: [PATCH] Refactoring the LibvirtComputingResource - Adding LibvirtCheckHealthCommandWrapper and LibvirtPrepareForMigrationCommandWrapper - 2 unit tests added - KVM hypervisor plugin with 10.8% coverage --- .../resource/LibvirtComputingResource.java | 68 ++------------ .../LibvirtCheckHealthCommandWrapper.java | 34 +++++++ ...virtPrepareForMigrationCommandWrapper.java | 89 +++++++++++++++++++ .../wrapper/LibvirtRequestWrapper.java | 4 + .../LibvirtComputingResourceTest.java | 72 +++++++++++++++ 5 files changed, 206 insertions(+), 61 deletions(-) create mode 100644 plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckHealthCommandWrapper.java create mode 100644 plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index b90b28b86fc..69f291e2bc2 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -95,8 +95,6 @@ import com.cloud.agent.api.AttachVolumeAnswer; import com.cloud.agent.api.AttachVolumeCommand; import com.cloud.agent.api.BackupSnapshotAnswer; import com.cloud.agent.api.BackupSnapshotCommand; -import com.cloud.agent.api.CheckHealthAnswer; -import com.cloud.agent.api.CheckHealthCommand; import com.cloud.agent.api.CheckNetworkAnswer; import com.cloud.agent.api.CheckNetworkCommand; import com.cloud.agent.api.CheckOnHostCommand; @@ -145,8 +143,6 @@ import com.cloud.agent.api.PingRoutingWithNwGroupsCommand; import com.cloud.agent.api.PingTestCommand; import com.cloud.agent.api.PlugNicAnswer; import com.cloud.agent.api.PlugNicCommand; -import com.cloud.agent.api.PrepareForMigrationAnswer; -import com.cloud.agent.api.PrepareForMigrationCommand; import com.cloud.agent.api.PvlanSetupCommand; import com.cloud.agent.api.ReadyAnswer; import com.cloud.agent.api.ReadyCommand; @@ -397,6 +393,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return _publicBridgeName; } + public KVMStoragePoolManager getStoragePoolMgr() { + return _storagePoolMgr; + } + private static final class KeyValueInterpreter extends OutputInterpreter { private final Map map = new HashMap(); @@ -1044,7 +1044,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return vifDriver; } - protected VifDriver getVifDriver(final TrafficType trafficType) { + public VifDriver getVifDriver(final TrafficType trafficType) { VifDriver vifDriver = _trafficTypeVifDrivers.get(trafficType); if (vifDriver == null) { @@ -1299,11 +1299,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } try { - if (cmd instanceof CheckHealthCommand) { - return execute((CheckHealthCommand)cmd); - } else if (cmd instanceof PrepareForMigrationCommand) { - return execute((PrepareForMigrationCommand)cmd); - } else if (cmd instanceof MigrateCommand) { + if (cmd instanceof MigrateCommand) { return execute((MigrateCommand)cmd); } else if (cmd instanceof PingTestCommand) { return execute((PingTestCommand)cmd); @@ -3211,56 +3207,6 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } } - private synchronized Answer execute(final PrepareForMigrationCommand cmd) { - - final VirtualMachineTO vm = cmd.getVirtualMachine(); - if (s_logger.isDebugEnabled()) { - s_logger.debug("Preparing host for migrating " + vm); - } - - final NicTO[] nics = vm.getNics(); - - boolean skipDisconnect = false; - - try { - final Connect conn = LibvirtConnection.getConnectionByVmName(vm.getName()); - for (final NicTO nic : nics) { - getVifDriver(nic.getType()).plug(nic, null, ""); - } - - /* setup disks, e.g for iso */ - final DiskTO[] volumes = vm.getDisks(); - for (final DiskTO volume : volumes) { - if (volume.getType() == Volume.Type.ISO) { - getVolumePath(conn, volume); - } - } - - if (!_storagePoolMgr.connectPhysicalDisksViaVmSpec(vm)) { - skipDisconnect = true; - return new PrepareForMigrationAnswer(cmd, "failed to connect physical disks to host"); - } - - skipDisconnect = true; - - return new PrepareForMigrationAnswer(cmd); - } catch (final LibvirtException e) { - return new PrepareForMigrationAnswer(cmd, e.toString()); - } catch (final InternalErrorException e) { - return new PrepareForMigrationAnswer(cmd, e.toString()); - } catch (final URISyntaxException e) { - return new PrepareForMigrationAnswer(cmd, e.toString()); - } finally { - if (!skipDisconnect) { - _storagePoolMgr.disconnectPhysicalDisksViaVmSpec(vm); - } - } - } - - private Answer execute(final CheckHealthCommand cmd) { - return new CheckHealthAnswer(cmd, true); - } - public String networkUsage(final String privateIpAddress, final String option, final String vif) { final Script getUsage = new Script(_routerProxyPath, s_logger); getUsage.add("netusage.sh"); @@ -3740,7 +3686,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } } - private String getVolumePath(final Connect conn, final DiskTO volume) throws LibvirtException, URISyntaxException { + public String getVolumePath(final Connect conn, final DiskTO volume) throws LibvirtException, URISyntaxException { final DataTO data = volume.getData(); final DataStoreTO store = data.getDataStore(); diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckHealthCommandWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckHealthCommandWrapper.java new file mode 100644 index 00000000000..c89d031ccd7 --- /dev/null +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckHealthCommandWrapper.java @@ -0,0 +1,34 @@ +// +// 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 com.cloud.hypervisor.kvm.resource.wrapper; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CheckHealthAnswer; +import com.cloud.agent.api.CheckHealthCommand; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.resource.CommandWrapper; + +public final class LibvirtCheckHealthCommandWrapper extends CommandWrapper { + + @Override + public Answer execute(final CheckHealthCommand command, final LibvirtComputingResource libvirtComputingResource) { + return new CheckHealthAnswer(command, true); + } +} \ No newline at end of file diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java new file mode 100644 index 00000000000..921d5b8cf64 --- /dev/null +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java @@ -0,0 +1,89 @@ +// +// 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 com.cloud.hypervisor.kvm.resource.wrapper; + +import java.net.URISyntaxException; + +import org.apache.log4j.Logger; +import org.libvirt.Connect; +import org.libvirt.LibvirtException; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.PrepareForMigrationAnswer; +import com.cloud.agent.api.PrepareForMigrationCommand; +import com.cloud.agent.api.to.DiskTO; +import com.cloud.agent.api.to.NicTO; +import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.exception.InternalErrorException; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.resource.CommandWrapper; +import com.cloud.storage.Volume; + +public final class LibvirtPrepareForMigrationCommandWrapper extends CommandWrapper { + + private static final Logger s_logger = Logger.getLogger(LibvirtPrepareForMigrationCommandWrapper.class); + + @Override + public Answer execute(final PrepareForMigrationCommand command, final LibvirtComputingResource libvirtComputingResource) { + final VirtualMachineTO vm = command.getVirtualMachine(); + if (s_logger.isDebugEnabled()) { + s_logger.debug("Preparing host for migrating " + vm); + } + + final NicTO[] nics = vm.getNics(); + + boolean skipDisconnect = false; + + try { + final LibvirtConnectionWrapper libvirtConnectionWrapper = libvirtComputingResource.getLibvirtConnectionWrapper(); + + final Connect conn = libvirtConnectionWrapper.getConnectionByVmName(vm.getName()); + for (final NicTO nic : nics) { + libvirtComputingResource.getVifDriver(nic.getType()).plug(nic, null, ""); + } + + /* setup disks, e.g for iso */ + final DiskTO[] volumes = vm.getDisks(); + for (final DiskTO volume : volumes) { + if (volume.getType() == Volume.Type.ISO) { + libvirtComputingResource.getVolumePath(conn, volume); + } + } + + skipDisconnect = true; + + if (!libvirtComputingResource.getStoragePoolMgr().connectPhysicalDisksViaVmSpec(vm)) { + return new PrepareForMigrationAnswer(command, "failed to connect physical disks to host"); + } + + return new PrepareForMigrationAnswer(command); + } catch (final LibvirtException e) { + return new PrepareForMigrationAnswer(command, e.toString()); + } catch (final InternalErrorException e) { + return new PrepareForMigrationAnswer(command, e.toString()); + } catch (final URISyntaxException e) { + return new PrepareForMigrationAnswer(command, e.toString()); + } finally { + if (!skipDisconnect) { + libvirtComputingResource.getStoragePoolMgr().disconnectPhysicalDisksViaVmSpec(vm); + } + } + } +} \ No newline at end of file diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java index e76a4eb4680..d8ca27fd280 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java @@ -21,10 +21,12 @@ package com.cloud.hypervisor.kvm.resource.wrapper; import java.util.Hashtable; import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CheckHealthCommand; import com.cloud.agent.api.Command; import com.cloud.agent.api.GetHostStatsCommand; import com.cloud.agent.api.GetVmDiskStatsCommand; import com.cloud.agent.api.GetVmStatsCommand; +import com.cloud.agent.api.PrepareForMigrationCommand; import com.cloud.agent.api.RebootCommand; import com.cloud.agent.api.RebootRouterCommand; import com.cloud.agent.api.StopCommand; @@ -56,6 +58,8 @@ public class LibvirtRequestWrapper extends RequestWrapper { linbvirtCommands.put(RebootRouterCommand.class, new LibvirtRebootRouterCommandWrapper()); linbvirtCommands.put(RebootCommand.class, new LibvirtRebootCommandWrapper()); linbvirtCommands.put(GetHostStatsCommand.class, new LibvirtGetHostStatsCommandWrapper()); + linbvirtCommands.put(CheckHealthCommand.class, new LibvirtCheckHealthCommandWrapper()); + linbvirtCommands.put(PrepareForMigrationCommand.class, new LibvirtPrepareForMigrationCommandWrapper()); resources.put(LibvirtComputingResource.class, linbvirtCommands); } diff --git a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 85f2c74c0f9..9b4b4213d33 100644 --- a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -62,19 +62,26 @@ import org.w3c.dom.Document; import org.xml.sax.SAXException; import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CheckHealthCommand; import com.cloud.agent.api.GetHostStatsCommand; import com.cloud.agent.api.GetVmDiskStatsCommand; import com.cloud.agent.api.GetVmStatsCommand; +import com.cloud.agent.api.PrepareForMigrationCommand; import com.cloud.agent.api.RebootCommand; import com.cloud.agent.api.RebootRouterCommand; import com.cloud.agent.api.StopCommand; import com.cloud.agent.api.VmStatsEntry; +import com.cloud.agent.api.to.DiskTO; +import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.InterfaceDef; import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtConnectionWrapper; import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtRequestWrapper; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.network.Networks.TrafficType; +import com.cloud.storage.Volume; import com.cloud.template.VirtualMachineTemplate.BootloaderType; import com.cloud.utils.Pair; import com.cloud.vm.VirtualMachine; @@ -587,6 +594,9 @@ public class LibvirtComputingResourceTest { @Test(expected = NumberFormatException.class) public void testGetHostStatsCommand() { + // A bit difficult top test due to the logger being passed and the parser itself relying on the connection. + // Have to spend some more time afterwards in order to refactor the wrapper itself. + final String uuid = "e8d6b4d0-bc6d-4613-b8bb-cb9e0600f3c6"; final GetHostStatsCommand command = new GetHostStatsCommand(uuid, "summer", 1l); @@ -596,4 +606,66 @@ public class LibvirtComputingResourceTest { final Answer answer = wrapper.execute(command, libvirtComputingResource); assertFalse(answer.getResult()); } + + @Test + public void testCheckHealthCommand() { + // A bit difficult top test due to the logger being passed and the parser itself relying on the connection. + // Have to spend some more time afterwards in order to refactor the wrapper itself. + + final CheckHealthCommand command = new CheckHealthCommand(); + + final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); + assertNotNull(wrapper); + + final Answer answer = wrapper.execute(command, libvirtComputingResource); + assertTrue(answer.getResult()); + } + + @Test + public void testPrepareForMigrationCommand() { + final Connect conn = Mockito.mock(Connect.class); + final LibvirtConnectionWrapper libvirtConnectionWrapper = Mockito.mock(LibvirtConnectionWrapper.class); + + final VirtualMachineTO vm = Mockito.mock(VirtualMachineTO.class); + final KVMStoragePoolManager storagePoolManager = Mockito.mock(KVMStoragePoolManager.class); + final NicTO nicTO = Mockito.mock(NicTO.class); + final DiskTO diskTO = Mockito.mock(DiskTO.class); + final VifDriver vifDriver = Mockito.mock(VifDriver.class); + + final PrepareForMigrationCommand command = new PrepareForMigrationCommand(vm); + + when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper); + try { + when(libvirtConnectionWrapper.getConnectionByVmName(vm.getName())).thenReturn(conn); + } catch (final LibvirtException e) { + fail(e.getMessage()); + } + + when(vm.getNics()).thenReturn(new NicTO[]{nicTO}); + when(vm.getDisks()).thenReturn(new DiskTO[]{diskTO}); + + when(nicTO.getType()).thenReturn(TrafficType.Guest); + when(diskTO.getType()).thenReturn(Volume.Type.ISO); + + when(libvirtComputingResource.getVifDriver(nicTO.getType())).thenReturn(vifDriver); + when(libvirtComputingResource.getStoragePoolMgr()).thenReturn(storagePoolManager); + + final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); + assertNotNull(wrapper); + + final Answer answer = wrapper.execute(command, libvirtComputingResource); + assertFalse(answer.getResult()); + + verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper(); + try { + verify(libvirtConnectionWrapper, times(1)).getConnectionByVmName(vm.getName()); + } catch (final LibvirtException e) { + fail(e.getMessage()); + } + + verify(libvirtComputingResource, times(1)).getStoragePoolMgr(); + verify(vm, times(1)).getNics(); + verify(vm, times(1)).getDisks(); + verify(diskTO, times(1)).getType(); + } } \ No newline at end of file