From 7a90c018fbba0419c55c245aa1cb3488472d2a62 Mon Sep 17 00:00:00 2001 From: wilderrodrigues Date: Wed, 22 Apr 2015 16:06:43 +0200 Subject: [PATCH] Refactoring the LibvirtComputingResource - Adding LibvirtRebootCommandWrapper and LibVirtRebootRouterCommandWrapper - 2 unit tests added - KVM hypervisor with 10.5% coverage --- .../resource/LibvirtComputingResource.java | 55 ++-------- .../wrapper/LibvirtConnectionWrapper.java | 2 +- .../LibvirtGetVmStatsCommandWrapper.java | 2 +- .../wrapper/LibvirtRebootCommandWrapper.java | 59 ++++++++++ .../LibvirtRebootRouterCommandWrapper.java | 47 ++++++++ .../wrapper/LibvirtRequestWrapper.java | 4 + .../wrapper/LibvirtStopCommandWrapper.java | 4 +- .../LibvirtComputingResourceTest.java | 102 +++++++++++++++--- 8 files changed, 211 insertions(+), 64 deletions(-) create mode 100644 plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRebootCommandWrapper.java create mode 100644 plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRebootRouterCommandWrapper.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 82ea791333d..a559e9965c7 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 @@ -154,9 +154,6 @@ import com.cloud.agent.api.PrepareForMigrationCommand; import com.cloud.agent.api.PvlanSetupCommand; import com.cloud.agent.api.ReadyAnswer; import com.cloud.agent.api.ReadyCommand; -import com.cloud.agent.api.RebootAnswer; -import com.cloud.agent.api.RebootCommand; -import com.cloud.agent.api.RebootRouterCommand; import com.cloud.agent.api.SecurityGroupRuleAnswer; import com.cloud.agent.api.SecurityGroupRulesCmd; import com.cloud.agent.api.SetupGuestNetworkCommand; @@ -396,6 +393,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return libvirtConnectionWrapper; } + public VirtualRoutingResource getVirtRouterResource() { + return _virtRouterResource; + } + private static final class KeyValueInterpreter extends OutputInterpreter { private final Map map = new HashMap(); @@ -1298,11 +1299,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } try { - if (cmd instanceof RebootRouterCommand) { - return execute((RebootRouterCommand)cmd); - } else if (cmd instanceof RebootCommand) { - return execute((RebootCommand)cmd); - } else if (cmd instanceof GetHostStatsCommand) { + if (cmd instanceof GetHostStatsCommand) { return execute((GetHostStatsCommand)cmd); } else if (cmd instanceof CheckStateCommand) { return executeRequest(cmd); @@ -3310,7 +3307,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return new GetHostStatsAnswer(cmd, hostStats); } - protected String networkUsage(final String privateIpAddress, final String option, final String vif) { + public String networkUsage(final String privateIpAddress, final String option, final String vif) { final Script getUsage = new Script(_routerProxyPath, s_logger); getUsage.add("netusage.sh"); getUsage.add(privateIpAddress); @@ -3418,38 +3415,6 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } } - private Answer execute(final RebootCommand cmd) { - - try { - final Connect conn = LibvirtConnection.getConnectionByVmName(cmd.getVmName()); - final String result = rebootVM(conn, cmd.getVmName()); - if (result == null) { - Integer vncPort = null; - try { - vncPort = getVncPort(conn, cmd.getVmName()); - } catch (final LibvirtException e) { - s_logger.trace("Ignoring libvirt error.", e); - } - get_rule_logs_for_vms(); - return new RebootAnswer(cmd, null, vncPort); - } else { - return new RebootAnswer(cmd, result, false); - } - } catch (final LibvirtException e) { - return new RebootAnswer(cmd, e.getMessage(), false); - } - } - - protected Answer execute(final RebootRouterCommand cmd) { - final RebootAnswer answer = (RebootAnswer)execute((RebootCommand)cmd); - if (_virtRouterResource.connect(cmd.getPrivateIpAddress())) { - networkUsage(cmd.getPrivateIpAddress(), "create", null); - return answer; - } else { - return new Answer(cmd, false, "Failed to connect to virtual router " + cmd.getVmName()); - } - } - protected Answer execute(final ModifySshKeysCommand cmd) { final File sshKeysDir = new File(SSHKEYSPATH); String result = null; @@ -4466,7 +4431,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } } - protected String rebootVM(final Connect conn, final String vmName) { + public String rebootVM(final Connect conn, final String vmName) { Domain dm = null; String msg = null; try { @@ -4625,7 +4590,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return null; } - protected Integer getVncPort(final Connect conn, final String vmName) throws LibvirtException { + public Integer getVncPort(final Connect conn, final String vmName) throws LibvirtException { final LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser(); Domain dm = null; try { @@ -5114,7 +5079,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return true; } - private String get_rule_logs_for_vms() { + public String getRuleLogsForVms() { final Script cmd = new Script(_securityGroupPath, _timeout, s_logger); cmd.add("get_rule_logs_for_vms"); final OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser(); @@ -5128,7 +5093,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv private HashMap> syncNetworkGroups(final long id) { final HashMap> states = new HashMap>(); - final String result = get_rule_logs_for_vms(); + final String result = getRuleLogsForVms(); s_logger.trace("syncNetworkGroups: id=" + id + " got: " + result); final String[] rulelogs = result != null ? result.split(";") : new String[0]; for (final String rulesforvm : rulelogs) { diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConnectionWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConnectionWrapper.java index 0a6b966d61a..e6743ace118 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConnectionWrapper.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConnectionWrapper.java @@ -27,7 +27,7 @@ import com.cloud.hypervisor.kvm.resource.LibvirtConnection; */ public class LibvirtConnectionWrapper { - public Connect getConnectionByName(final String vmName) throws LibvirtException { + public Connect getConnectionByVmName(final String vmName) throws LibvirtException { return LibvirtConnection.getConnectionByVmName(vmName); } diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmStatsCommandWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmStatsCommandWrapper.java index d894243fa8a..e0649dae837 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmStatsCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVmStatsCommandWrapper.java @@ -46,7 +46,7 @@ public final class LibvirtGetVmStatsCommandWrapper extends CommandWrapper { + + private static final Logger s_logger = Logger.getLogger(LibvirtRebootCommandWrapper.class); + + @Override + public Answer execute(final RebootCommand command, final LibvirtComputingResource libvirtComputingResource) { + final LibvirtConnectionWrapper libvirtConnectionWrapper = libvirtComputingResource.getLibvirtConnectionWrapper(); + + try { + final Connect conn = libvirtConnectionWrapper.getConnectionByVmName(command.getVmName()); + final String result = libvirtComputingResource.rebootVM(conn, command.getVmName()); + if (result == null) { + Integer vncPort = null; + try { + vncPort = libvirtComputingResource.getVncPort(conn, command.getVmName()); + } catch (final LibvirtException e) { + s_logger.trace("Ignoring libvirt error.", e); + } + libvirtComputingResource.getRuleLogsForVms(); + return new RebootAnswer(command, null, vncPort); + } else { + return new RebootAnswer(command, result, false); + } + } catch (final LibvirtException e) { + return new RebootAnswer(command, e.getMessage(), false); + } + } +} \ No newline at end of file diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRebootRouterCommandWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRebootRouterCommandWrapper.java new file mode 100644 index 00000000000..671b8c800c4 --- /dev/null +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRebootRouterCommandWrapper.java @@ -0,0 +1,47 @@ +// +// 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.RebootCommand; +import com.cloud.agent.api.RebootRouterCommand; +import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.resource.CommandWrapper; + +public final class LibvirtRebootRouterCommandWrapper extends CommandWrapper { + + @Override + public Answer execute(final RebootRouterCommand command, final LibvirtComputingResource libvirtComputingResource) { + final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); + + final RebootCommand rebootCommand = new RebootCommand(command.getVmName()); + final Answer answer = wrapper.execute(rebootCommand, libvirtComputingResource); + + final VirtualRoutingResource virtualRouterResource = libvirtComputingResource.getVirtRouterResource(); + if (virtualRouterResource.connect(command.getPrivateIpAddress())) { + libvirtComputingResource.networkUsage(command.getPrivateIpAddress(), "create", null); + + return answer; + } else { + return new Answer(command, false, "Failed to connect to virtual router " + command.getVmName()); + } + } +} \ 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 c74a09797fe..c2c3d8b5d28 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 @@ -24,6 +24,8 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; import com.cloud.agent.api.GetVmDiskStatsCommand; import com.cloud.agent.api.GetVmStatsCommand; +import com.cloud.agent.api.RebootCommand; +import com.cloud.agent.api.RebootRouterCommand; import com.cloud.agent.api.StopCommand; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import com.cloud.resource.CommandWrapper; @@ -50,6 +52,8 @@ public class LibvirtRequestWrapper extends RequestWrapper { linbvirtCommands.put(StopCommand.class, new LibvirtStopCommandWrapper()); linbvirtCommands.put(GetVmStatsCommand.class, new LibvirtGetVmStatsCommandWrapper()); linbvirtCommands.put(GetVmDiskStatsCommand.class, new LibvirtGetVmDiskStatsCommandWrapper()); + linbvirtCommands.put(RebootRouterCommand.class, new LibvirtRebootRouterCommandWrapper()); + linbvirtCommands.put(RebootCommand.class, new LibvirtRebootCommandWrapper()); resources.put(LibvirtComputingResource.class, linbvirtCommands); } diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java index c5d6a5bd56e..280c7f1e7de 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java @@ -48,7 +48,7 @@ public final class LibvirtStopCommandWrapper extends CommandWrapper disks = libvirtComputingResource.getDisks(conn, vmName); final List ifaces = libvirtComputingResource.getInterfaces(conn, vmName); 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 cd1255631ce..8457e23b768 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 @@ -19,6 +19,7 @@ package com.cloud.hypervisor.kvm.resource; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -63,9 +64,12 @@ import org.xml.sax.SAXException; import com.cloud.agent.api.Answer; import com.cloud.agent.api.GetVmDiskStatsCommand; import com.cloud.agent.api.GetVmStatsCommand; +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.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; @@ -398,11 +402,11 @@ public class LibvirtComputingResourceTest { final LibvirtConnectionWrapper libvirtConnectionWrapper = Mockito.mock(LibvirtConnectionWrapper.class); final String vmName = "Test"; - final StopCommand stopCommand = new StopCommand(vmName, false, false); + final StopCommand command = new StopCommand(vmName, false, false); when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper); try { - when(libvirtConnectionWrapper.getConnectionByName(vmName)).thenReturn(conn); + when(libvirtConnectionWrapper.getConnectionByVmName(vmName)).thenReturn(conn); } catch (final LibvirtException e) { fail(e.getMessage()); } @@ -410,13 +414,13 @@ public class LibvirtComputingResourceTest { final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); - final Answer answer = wrapper.execute(stopCommand, libvirtComputingResource); + final Answer answer = wrapper.execute(command, libvirtComputingResource); assertTrue(answer.getResult()); verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper(); try { - verify(libvirtConnectionWrapper, times(1)).getConnectionByName(vmName); + verify(libvirtConnectionWrapper, times(1)).getConnectionByVmName(vmName); } catch (final LibvirtException e) { fail(e.getMessage()); } @@ -432,12 +436,12 @@ public class LibvirtComputingResourceTest { final Domain domain = Mockito.mock(Domain.class); final String vmName = "Test"; - final StopCommand stopCommand = new StopCommand(vmName, false, true); + final StopCommand command = new StopCommand(vmName, false, true); when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper); try { - when(libvirtConnectionWrapper.getConnectionByName(vmName)).thenReturn(conn); - when(conn.domainLookupByName(stopCommand.getVmName())).thenReturn(domain); + when(libvirtConnectionWrapper.getConnectionByVmName(vmName)).thenReturn(conn); + when(conn.domainLookupByName(command.getVmName())).thenReturn(domain); } catch (final LibvirtException e) { fail(e.getMessage()); } @@ -445,13 +449,13 @@ public class LibvirtComputingResourceTest { final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); - final Answer answer = wrapper.execute(stopCommand, libvirtComputingResource); + final Answer answer = wrapper.execute(command, libvirtComputingResource); assertTrue(answer.getResult()); verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper(); try { - verify(libvirtConnectionWrapper, times(2)).getConnectionByName(vmName); + verify(libvirtConnectionWrapper, times(2)).getConnectionByVmName(vmName); } catch (final LibvirtException e) { fail(e.getMessage()); } @@ -470,11 +474,11 @@ public class LibvirtComputingResourceTest { final List vms = new ArrayList(); vms.add(vmName); - final GetVmStatsCommand stopCommand = new GetVmStatsCommand(vms, uuid, "hostname"); + final GetVmStatsCommand command = new GetVmStatsCommand(vms, uuid, "hostname"); when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper); try { - when(libvirtConnectionWrapper.getConnectionByName(vmName)).thenReturn(conn); + when(libvirtConnectionWrapper.getConnectionByVmName(vmName)).thenReturn(conn); } catch (final LibvirtException e) { fail(e.getMessage()); } @@ -482,12 +486,12 @@ public class LibvirtComputingResourceTest { final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); - final Answer answer = wrapper.execute(stopCommand, libvirtComputingResource); + final Answer answer = wrapper.execute(command, libvirtComputingResource); assertTrue(answer.getResult()); verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper(); try { - verify(libvirtConnectionWrapper, times(1)).getConnectionByName(vmName); + verify(libvirtConnectionWrapper, times(1)).getConnectionByVmName(vmName); } catch (final LibvirtException e) { fail(e.getMessage()); } @@ -506,7 +510,7 @@ public class LibvirtComputingResourceTest { final List vms = new ArrayList(); vms.add(vmName); - final GetVmDiskStatsCommand stopCommand = new GetVmDiskStatsCommand(vms, uuid, "hostname"); + final GetVmDiskStatsCommand command = new GetVmDiskStatsCommand(vms, uuid, "hostname"); when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper); try { @@ -518,7 +522,7 @@ public class LibvirtComputingResourceTest { final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); - final Answer answer = wrapper.execute(stopCommand, libvirtComputingResource); + final Answer answer = wrapper.execute(command, libvirtComputingResource); assertTrue(answer.getResult()); verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper(); @@ -528,4 +532,72 @@ public class LibvirtComputingResourceTest { fail(e.getMessage()); } } + + @Test + public void testRebootCommand() { + // 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 RebootCommand command = new RebootCommand(vmName); + + when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper); + try { + when(libvirtConnectionWrapper.getConnectionByVmName(vmName)).thenReturn(conn); + } catch (final LibvirtException e) { + fail(e.getMessage()); + } + + final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); + assertNotNull(wrapper); + + final Answer answer = wrapper.execute(command, libvirtComputingResource); + assertTrue(answer.getResult()); + + verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper(); + try { + verify(libvirtConnectionWrapper, times(1)).getConnectionByVmName(vmName); + } catch (final LibvirtException e) { + fail(e.getMessage()); + } + } + + @Test + public void testRebootRouterCommand() { + // 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 VirtualRoutingResource routingResource = Mockito.mock(VirtualRoutingResource.class); + final Connect conn = Mockito.mock(Connect.class); + final LibvirtConnectionWrapper libvirtConnectionWrapper = Mockito.mock(LibvirtConnectionWrapper.class); + + final String vmName = "Test"; + final RebootRouterCommand command = new RebootRouterCommand(vmName, "192.168.0.10"); + + when(libvirtComputingResource.getVirtRouterResource()).thenReturn(routingResource); + when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper); + try { + when(libvirtConnectionWrapper.getConnectionByVmName(vmName)).thenReturn(conn); + } catch (final LibvirtException e) { + fail(e.getMessage()); + } + + final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); + assertNotNull(wrapper); + + final Answer answer = wrapper.execute(command, libvirtComputingResource); + assertFalse(answer.getResult()); + + verify(libvirtComputingResource, times(1)).getVirtRouterResource(); + + verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper(); + try { + verify(libvirtConnectionWrapper, times(1)).getConnectionByVmName(vmName); + } catch (final LibvirtException e) { + fail(e.getMessage()); + } + } } \ No newline at end of file