mirror of https://github.com/apache/cloudstack.git
Refactoring the LibvirtComputingResource
- Adding LibvirtGetVmStatsCommandWrapper - 3 unit tests Refactored the LibvirtConnectiobn by surrounding it with an wrapper. - Make it easier to cover the static/native calls - Added better coverage to StopCommand tests
This commit is contained in:
parent
508f10527f
commit
7086d64387
|
|
@ -31,4 +31,7 @@
|
|||
<property name="name" value="KVMInvestigator" />
|
||||
</bean>
|
||||
|
||||
<bean id="libvirtConnectionWrapper"
|
||||
class="com.cloud.hypervisor.kvm.resource.wrapper.LibvirtConnectionWrapper" />
|
||||
|
||||
</beans>
|
||||
|
|
|
|||
|
|
@ -59,6 +59,7 @@ import java.util.regex.Matcher;
|
|||
import java.util.regex.Pattern;
|
||||
|
||||
import javax.ejb.Local;
|
||||
import javax.inject.Inject;
|
||||
import javax.naming.ConfigurationException;
|
||||
|
||||
import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
|
||||
|
|
@ -230,6 +231,7 @@ import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.SerialDef;
|
|||
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.TermPolicy;
|
||||
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.VideoDef;
|
||||
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.VirtioSerialDef;
|
||||
import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtConnectionWrapper;
|
||||
import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtRequestWrapper;
|
||||
import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk;
|
||||
import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
|
||||
|
|
@ -332,6 +334,9 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
|
|||
protected static final String DEFAULT_OVS_VIF_DRIVER_CLASS_NAME = "com.cloud.hypervisor.kvm.resource.OvsVifDriver";
|
||||
protected static final String DEFAULT_BRIDGE_VIF_DRIVER_CLASS_NAME = "com.cloud.hypervisor.kvm.resource.BridgeVifDriver";
|
||||
|
||||
@Inject
|
||||
private LibvirtConnectionWrapper libvirtConnectionWrapper;
|
||||
|
||||
@Override
|
||||
public ExecutionResult executeInVR(final String routerIp, final String script, final String args) {
|
||||
return executeInVR(routerIp, script, args, _timeout / 1000);
|
||||
|
|
@ -393,6 +398,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
|
|||
return new ExecutionResult(true, null);
|
||||
}
|
||||
|
||||
public LibvirtConnectionWrapper getLibvirtConnectionWrapper() {
|
||||
return libvirtConnectionWrapper;
|
||||
}
|
||||
|
||||
private static final class KeyValueInterpreter extends OutputInterpreter {
|
||||
private final Map<String, String> map = new HashMap<String, String>();
|
||||
|
||||
|
|
@ -915,7 +924,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
|
|||
}
|
||||
s_logger.debug("Found pif: " + _pifs.get("private") + " on " + _privBridgeName + ", pif: " + _pifs.get("public") + " on " + _publicBridgeName);
|
||||
|
||||
_canBridgeFirewall = can_bridge_firewall(_pifs.get("public"));
|
||||
_canBridgeFirewall = canBridgeFirewall(_pifs.get("public"));
|
||||
|
||||
_localGateway = Script.runSimpleBashScript("ip route |grep default|awk '{print $3}'");
|
||||
if (_localGateway == null) {
|
||||
|
|
@ -4917,7 +4926,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
|
|||
Calendar _timestamp;
|
||||
}
|
||||
|
||||
VmStatsEntry getVmStat(final Connect conn, final String vmName) throws LibvirtException {
|
||||
public VmStatsEntry getVmStat(final Connect conn, final String vmName) throws LibvirtException {
|
||||
Domain dm = null;
|
||||
try {
|
||||
dm = getDomain(conn, vmName);
|
||||
|
|
@ -5020,7 +5029,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
|
|||
}
|
||||
}
|
||||
|
||||
private boolean can_bridge_firewall(final String prvNic) {
|
||||
private boolean canBridgeFirewall(final String prvNic) {
|
||||
final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
|
||||
cmd.add("can_bridge_firewall");
|
||||
cmd.add(prvNic);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,33 @@
|
|||
// 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 org.libvirt.Connect;
|
||||
import org.libvirt.LibvirtException;
|
||||
|
||||
import com.cloud.hypervisor.kvm.resource.LibvirtConnection;
|
||||
|
||||
/**
|
||||
* This class is used to wrapp the calls to LibvirtConnection and ease the burden of the unit tests.
|
||||
* Please do not instantiate this class directly, but inject it using the {@code @Inject} annotation.
|
||||
*/
|
||||
public class LibvirtConnectionWrapper {
|
||||
|
||||
public Connect getConnectionByName(final String vmName) throws LibvirtException {
|
||||
return LibvirtConnection.getConnectionByVmName(vmName);
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,63 @@
|
|||
//
|
||||
// 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.util.HashMap;
|
||||
import java.util.List;
|
||||
|
||||
import org.apache.log4j.Logger;
|
||||
import org.libvirt.Connect;
|
||||
import org.libvirt.LibvirtException;
|
||||
|
||||
import com.cloud.agent.api.Answer;
|
||||
import com.cloud.agent.api.GetVmStatsAnswer;
|
||||
import com.cloud.agent.api.GetVmStatsCommand;
|
||||
import com.cloud.agent.api.VmStatsEntry;
|
||||
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
|
||||
import com.cloud.resource.CommandWrapper;
|
||||
|
||||
public final class LibvirtGetVmStatsCommandWrapper extends CommandWrapper<GetVmStatsCommand, Answer, LibvirtComputingResource> {
|
||||
|
||||
private static final Logger s_logger = Logger.getLogger(LibvirtGetVmStatsCommandWrapper.class);
|
||||
|
||||
@Override
|
||||
public Answer execute(final GetVmStatsCommand command, final LibvirtComputingResource libvirtComputingResource) {
|
||||
final List<String> vmNames = command.getVmNames();
|
||||
try {
|
||||
final HashMap<String, VmStatsEntry> vmStatsNameMap = new HashMap<String, VmStatsEntry>();
|
||||
for (final String vmName : vmNames) {
|
||||
|
||||
final LibvirtConnectionWrapper libvirtConnectionWrapper = libvirtComputingResource.getLibvirtConnectionWrapper();
|
||||
|
||||
final Connect conn = libvirtConnectionWrapper.getConnectionByName(vmName);
|
||||
final VmStatsEntry statEntry = libvirtComputingResource.getVmStat(conn, vmName);
|
||||
if (statEntry == null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
vmStatsNameMap.put(vmName, statEntry);
|
||||
}
|
||||
return new GetVmStatsAnswer(command, vmStatsNameMap);
|
||||
} catch (final LibvirtException e) {
|
||||
s_logger.debug("Can't get vm stats: " + e.toString());
|
||||
return new GetVmStatsAnswer(command, null);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -22,6 +22,7 @@ import java.util.Hashtable;
|
|||
|
||||
import com.cloud.agent.api.Answer;
|
||||
import com.cloud.agent.api.Command;
|
||||
import com.cloud.agent.api.GetVmStatsCommand;
|
||||
import com.cloud.agent.api.StopCommand;
|
||||
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
|
||||
import com.cloud.resource.CommandWrapper;
|
||||
|
|
@ -46,6 +47,7 @@ public class LibvirtRequestWrapper extends RequestWrapper {
|
|||
final Hashtable<Class<? extends Command>, CommandWrapper> linbvirtCommands = new Hashtable<Class<? extends Command>, CommandWrapper>();
|
||||
|
||||
linbvirtCommands.put(StopCommand.class, new LibvirtStopCommandWrapper());
|
||||
linbvirtCommands.put(GetVmStatsCommand.class, new LibvirtGetVmStatsCommandWrapper());
|
||||
resources.put(LibvirtComputingResource.class, linbvirtCommands);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -31,7 +31,6 @@ import com.cloud.agent.api.Answer;
|
|||
import com.cloud.agent.api.StopAnswer;
|
||||
import com.cloud.agent.api.StopCommand;
|
||||
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
|
||||
import com.cloud.hypervisor.kvm.resource.LibvirtConnection;
|
||||
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef;
|
||||
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.InterfaceDef;
|
||||
import com.cloud.hypervisor.kvm.resource.VifDriver;
|
||||
|
|
@ -42,12 +41,14 @@ public final class LibvirtStopCommandWrapper extends CommandWrapper<StopCommand,
|
|||
private static final Logger s_logger = Logger.getLogger(LibvirtStopCommandWrapper.class);
|
||||
|
||||
@Override
|
||||
public Answer execute(final StopCommand command, final LibvirtComputingResource citrixResourceBase) {
|
||||
public Answer execute(final StopCommand command, final LibvirtComputingResource libvirtComputingResource) {
|
||||
final String vmName = command.getVmName();
|
||||
|
||||
final LibvirtConnectionWrapper libvirtConnectionWrapper = libvirtComputingResource.getLibvirtConnectionWrapper();
|
||||
|
||||
if (command.checkBeforeCleanup()) {
|
||||
try {
|
||||
final Connect conn = LibvirtConnection.getConnectionByVmName(vmName);
|
||||
final Connect conn = libvirtConnectionWrapper.getConnectionByName(vmName);
|
||||
final Domain vm = conn.domainLookupByName(command.getVmName());
|
||||
if (vm != null && vm.getInfo().state == DomainState.VIR_DOMAIN_RUNNING) {
|
||||
return new StopAnswer(command, "vm is still running on host", false);
|
||||
|
|
@ -58,21 +59,21 @@ public final class LibvirtStopCommandWrapper extends CommandWrapper<StopCommand,
|
|||
}
|
||||
|
||||
try {
|
||||
final Connect conn = LibvirtConnection.getConnectionByVmName(vmName);
|
||||
final Connect conn = libvirtConnectionWrapper.getConnectionByName(vmName);
|
||||
|
||||
final List<DiskDef> disks = citrixResourceBase.getDisks(conn, vmName);
|
||||
final List<InterfaceDef> ifaces = citrixResourceBase.getInterfaces(conn, vmName);
|
||||
final List<DiskDef> disks = libvirtComputingResource.getDisks(conn, vmName);
|
||||
final List<InterfaceDef> ifaces = libvirtComputingResource.getInterfaces(conn, vmName);
|
||||
|
||||
citrixResourceBase.destroyNetworkRulesForVM(conn, vmName);
|
||||
final String result = citrixResourceBase.stopVM(conn, vmName);
|
||||
libvirtComputingResource.destroyNetworkRulesForVM(conn, vmName);
|
||||
final String result = libvirtComputingResource.stopVM(conn, vmName);
|
||||
if (result == null) {
|
||||
for (final DiskDef disk : disks) {
|
||||
citrixResourceBase.cleanupDisk(disk);
|
||||
libvirtComputingResource.cleanupDisk(disk);
|
||||
}
|
||||
for (final InterfaceDef iface : ifaces) {
|
||||
// We don't know which "traffic type" is associated with
|
||||
// each interface at this point, so inform all vif drivers
|
||||
for (final VifDriver vifDriver : citrixResourceBase.getAllVifDrivers()) {
|
||||
for (final VifDriver vifDriver : libvirtComputingResource.getAllVifDrivers()) {
|
||||
vifDriver.unplug(iface);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,9 +20,15 @@
|
|||
package com.cloud.hypervisor.kvm.resource;
|
||||
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.junit.Assert.fail;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
import java.util.Random;
|
||||
|
|
@ -54,11 +60,14 @@ import org.powermock.modules.junit4.PowerMockRunner;
|
|||
import org.w3c.dom.Document;
|
||||
import org.xml.sax.SAXException;
|
||||
|
||||
import com.cloud.agent.api.Answer;
|
||||
import com.cloud.agent.api.GetVmStatsCommand;
|
||||
import com.cloud.agent.api.StopCommand;
|
||||
import com.cloud.agent.api.VmStatsEntry;
|
||||
import com.cloud.agent.api.to.VirtualMachineTO;
|
||||
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.template.VirtualMachineTemplate.BootloaderType;
|
||||
import com.cloud.utils.Pair;
|
||||
|
|
@ -379,16 +388,107 @@ public class LibvirtComputingResourceTest {
|
|||
* New Tests
|
||||
*/
|
||||
|
||||
@Test(expected = UnsatisfiedLinkError.class)
|
||||
public void testStopCommand() {
|
||||
@Test
|
||||
public void testStopCommandNoCheck() {
|
||||
// We cannot do much here due to the Native libraries and Static methods used by the LibvirtConnection we need
|
||||
// a better way to mock stuff!
|
||||
|
||||
final Connect conn = Mockito.mock(Connect.class);
|
||||
final LibvirtConnectionWrapper libvirtConnectionWrapper = Mockito.mock(LibvirtConnectionWrapper.class);
|
||||
|
||||
final String vmName = "Test";
|
||||
final StopCommand stopCommand = new StopCommand(vmName, false, false);
|
||||
|
||||
when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper);
|
||||
try {
|
||||
when(libvirtConnectionWrapper.getConnectionByName(vmName)).thenReturn(conn);
|
||||
} catch (final LibvirtException e) {
|
||||
fail(e.getMessage());
|
||||
}
|
||||
|
||||
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
|
||||
assertNotNull(wrapper);
|
||||
|
||||
wrapper.execute(stopCommand, libvirtComputingResource);
|
||||
final Answer answer = wrapper.execute(stopCommand, libvirtComputingResource);
|
||||
|
||||
assertTrue(answer.getResult());
|
||||
|
||||
verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper();
|
||||
try {
|
||||
verify(libvirtConnectionWrapper, times(1)).getConnectionByName(vmName);
|
||||
} catch (final LibvirtException e) {
|
||||
fail(e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testStopCommandCheck() {
|
||||
// We cannot do much here due to the Native libraries and Static methods used by the LibvirtConnection we need
|
||||
// a better way to mock stuff!
|
||||
|
||||
final Connect conn = Mockito.mock(Connect.class);
|
||||
final LibvirtConnectionWrapper libvirtConnectionWrapper = Mockito.mock(LibvirtConnectionWrapper.class);
|
||||
final Domain domain = Mockito.mock(Domain.class);
|
||||
|
||||
final String vmName = "Test";
|
||||
final StopCommand stopCommand = new StopCommand(vmName, false, true);
|
||||
|
||||
when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper);
|
||||
try {
|
||||
when(libvirtConnectionWrapper.getConnectionByName(vmName)).thenReturn(conn);
|
||||
when(conn.domainLookupByName(stopCommand.getVmName())).thenReturn(domain);
|
||||
} catch (final LibvirtException e) {
|
||||
fail(e.getMessage());
|
||||
}
|
||||
|
||||
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
|
||||
assertNotNull(wrapper);
|
||||
|
||||
final Answer answer = wrapper.execute(stopCommand, libvirtComputingResource);
|
||||
|
||||
assertTrue(answer.getResult());
|
||||
|
||||
verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper();
|
||||
try {
|
||||
verify(libvirtConnectionWrapper, times(2)).getConnectionByName(vmName);
|
||||
} catch (final LibvirtException e) {
|
||||
fail(e.getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testGetVmStatsCommand() {
|
||||
// We cannot do much here due to the Native libraries and Static methods used by the LibvirtConnection we need
|
||||
// a better way to mock stuff!
|
||||
|
||||
final Connect conn = Mockito.mock(Connect.class);
|
||||
final LibvirtConnectionWrapper libvirtConnectionWrapper = Mockito.mock(LibvirtConnectionWrapper.class);
|
||||
|
||||
final String vmName = "Test";
|
||||
final String uuid = "e8d6b4d0-bc6d-4613-b8bb-cb9e0600f3c6";
|
||||
final List<String> vms = new ArrayList<String>();
|
||||
vms.add(vmName);
|
||||
|
||||
final GetVmStatsCommand stopCommand = new GetVmStatsCommand(vms, uuid, "hostname");
|
||||
|
||||
when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper);
|
||||
try {
|
||||
when(libvirtConnectionWrapper.getConnectionByName(vmName)).thenReturn(conn);
|
||||
} catch (final LibvirtException e) {
|
||||
fail(e.getMessage());
|
||||
}
|
||||
|
||||
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
|
||||
assertNotNull(wrapper);
|
||||
|
||||
final Answer answer = wrapper.execute(stopCommand, libvirtComputingResource);
|
||||
assertTrue(answer.getResult());
|
||||
|
||||
verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper();
|
||||
try {
|
||||
verify(libvirtConnectionWrapper, times(1)).getConnectionByName(vmName);
|
||||
} catch (final LibvirtException e) {
|
||||
fail(e.getMessage());
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue