Refactoring the LibvirtComputingResource

- Adding LibvirtOvsDestroyBridgeCommandWrapper, LibvirtOvsSetupBridgeCommandWrapper
  - 4 unit tests added
  - KVM hypervisor plugin with 13.9% coverage

More tests added to cover LibvirtPrepareForMigrationCommandWrapper
  - Coverage of this wrapper broght from 37% to 90.6%
  - 4 new tests added
This commit is contained in:
wilderrodrigues 2015-05-01 12:04:16 +02:00
parent 0d860af659
commit 4887ce7254
6 changed files with 400 additions and 42 deletions

View File

@ -108,11 +108,9 @@ import com.cloud.agent.api.NetworkUsageAnswer;
import com.cloud.agent.api.NetworkUsageCommand;
import com.cloud.agent.api.OvsCreateTunnelAnswer;
import com.cloud.agent.api.OvsCreateTunnelCommand;
import com.cloud.agent.api.OvsDestroyBridgeCommand;
import com.cloud.agent.api.OvsDestroyTunnelCommand;
import com.cloud.agent.api.OvsFetchInterfaceAnswer;
import com.cloud.agent.api.OvsFetchInterfaceCommand;
import com.cloud.agent.api.OvsSetupBridgeCommand;
import com.cloud.agent.api.OvsVpcPhysicalTopologyConfigCommand;
import com.cloud.agent.api.OvsVpcRoutingPolicyConfigCommand;
import com.cloud.agent.api.PingCommand;
@ -1342,10 +1340,6 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
return execute((CheckOnHostCommand)cmd);
} else if (cmd instanceof OvsFetchInterfaceCommand) {
return execute((OvsFetchInterfaceCommand)cmd);
} else if (cmd instanceof OvsSetupBridgeCommand) {
return execute((OvsSetupBridgeCommand)cmd);
} else if (cmd instanceof OvsDestroyBridgeCommand) {
return execute((OvsDestroyBridgeCommand)cmd);
} else if (cmd instanceof OvsCreateTunnelCommand) {
return execute((OvsCreateTunnelCommand)cmd);
} else if (cmd instanceof OvsDestroyTunnelCommand) {
@ -1381,20 +1375,6 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
}
private Answer execute(final OvsSetupBridgeCommand cmd) {
findOrCreateTunnelNetwork(cmd.getBridgeName());
configureTunnelNetwork(cmd.getNetworkId(), cmd.getHostId(),
cmd.getBridgeName());
s_logger.debug("OVS Bridge configured");
return new Answer(cmd, true, null);
}
private Answer execute(final OvsDestroyBridgeCommand cmd) {
destroyTunnelNetwork(cmd.getBridgeName());
s_logger.debug("OVS Bridge destroyed");
return new Answer(cmd, true, null);
}
public Answer execute(final OvsVpcPhysicalTopologyConfigCommand cmd) {
final String bridge = cmd.getBridgeName();
@ -1436,28 +1416,23 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
}
}
private synchronized void destroyTunnelNetwork(final String bridge) {
try {
findOrCreateTunnelNetwork(bridge);
final Script cmd = new Script(_ovsTunnelPath, _timeout, s_logger);
cmd.add("destroy_ovs_bridge");
cmd.add("--bridge", bridge);
final String result = cmd.execute();
if (result != null) {
// TODO: Should make this error not fatal?
// Can Concurrent VM shutdown/migration/reboot events can cause
// this method
// to be executed on a bridge which has already been removed?
throw new CloudRuntimeException("Unable to remove OVS bridge " + bridge);
}
return;
} catch (final Exception e) {
s_logger.warn("destroyTunnelNetwork failed:", e);
return;
public synchronized boolean destroyTunnelNetwork(final String bridge) {
findOrCreateTunnelNetwork(bridge);
final Script cmd = new Script(_ovsTunnelPath, _timeout, s_logger);
cmd.add("destroy_ovs_bridge");
cmd.add("--bridge", bridge);
final String result = cmd.execute();
if (result != null) {
s_logger.debug("OVS Bridge could not be destroyed due to error ==> " + result);
return false;
}
return true;
}
private synchronized boolean findOrCreateTunnelNetwork(final String nwName) {
public synchronized boolean findOrCreateTunnelNetwork(final String nwName) {
try {
if (checkNetwork(nwName)) {
return true;
@ -1475,7 +1450,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
return true;
}
private synchronized boolean configureTunnelNetwork(final long networkId,
public synchronized boolean configureTunnelNetwork(final long networkId,
final long hostId, final String nwName) {
try {
findOrCreateTunnelNetwork(nwName);

View File

@ -0,0 +1,43 @@
//
// 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.apache.log4j.Logger;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.OvsDestroyBridgeCommand;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.resource.CommandWrapper;
public final class LibvirtOvsDestroyBridgeCommandWrapper extends CommandWrapper<OvsDestroyBridgeCommand, Answer, LibvirtComputingResource> {
private static final Logger s_logger = Logger.getLogger(LibvirtOvsDestroyBridgeCommandWrapper.class);
@Override
public Answer execute(final OvsDestroyBridgeCommand command, final LibvirtComputingResource libvirtComputingResource) {
final boolean result = libvirtComputingResource.destroyTunnelNetwork(command.getBridgeName());
if (!result) {
s_logger.debug("Error trying to destroy OVS Bridge!");
}
return new Answer(command, result, null);
}
}

View File

@ -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 org.apache.log4j.Logger;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.OvsSetupBridgeCommand;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.resource.CommandWrapper;
public final class LibvirtOvsSetupBridgeCommandWrapper extends CommandWrapper<OvsSetupBridgeCommand, Answer, LibvirtComputingResource> {
private static final Logger s_logger = Logger.getLogger(LibvirtOvsSetupBridgeCommandWrapper.class);
@Override
public Answer execute(final OvsSetupBridgeCommand command, final LibvirtComputingResource libvirtComputingResource) {
final boolean findResult = libvirtComputingResource.findOrCreateTunnelNetwork(command.getBridgeName());
final boolean configResult = libvirtComputingResource.configureTunnelNetwork(command.getNetworkId(), command.getHostId(),
command.getBridgeName());
final boolean finalResult = findResult && configResult;
if (!finalResult) {
s_logger.debug("::FAILURE:: OVS Bridge was NOT configured properly!");
}
return new Answer(command, finalResult, null);
}
}

View File

@ -33,6 +33,7 @@ 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.hypervisor.kvm.storage.KVMStoragePoolManager;
import com.cloud.resource.CommandWrapper;
import com.cloud.storage.Volume;
@ -51,6 +52,7 @@ public final class LibvirtPrepareForMigrationCommandWrapper extends CommandWrapp
boolean skipDisconnect = false;
final KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr();
try {
final LibvirtConnectionWrapper libvirtConnectionWrapper = libvirtComputingResource.getLibvirtConnectionWrapper();
@ -69,7 +71,7 @@ public final class LibvirtPrepareForMigrationCommandWrapper extends CommandWrapp
skipDisconnect = true;
if (!libvirtComputingResource.getStoragePoolMgr().connectPhysicalDisksViaVmSpec(vm)) {
if (!storagePoolMgr.connectPhysicalDisksViaVmSpec(vm)) {
return new PrepareForMigrationAnswer(command, "failed to connect physical disks to host");
}
@ -82,7 +84,7 @@ public final class LibvirtPrepareForMigrationCommandWrapper extends CommandWrapp
return new PrepareForMigrationAnswer(command, e.toString());
} finally {
if (!skipDisconnect) {
libvirtComputingResource.getStoragePoolMgr().disconnectPhysicalDisksViaVmSpec(vm);
storagePoolMgr.disconnectPhysicalDisksViaVmSpec(vm);
}
}
}

View File

@ -35,6 +35,8 @@ import com.cloud.agent.api.GetVncPortCommand;
import com.cloud.agent.api.MaintainCommand;
import com.cloud.agent.api.MigrateCommand;
import com.cloud.agent.api.ModifySshKeysCommand;
import com.cloud.agent.api.OvsDestroyBridgeCommand;
import com.cloud.agent.api.OvsSetupBridgeCommand;
import com.cloud.agent.api.PingTestCommand;
import com.cloud.agent.api.PrepareForMigrationCommand;
import com.cloud.agent.api.ReadyCommand;
@ -94,6 +96,8 @@ public class LibvirtRequestWrapper extends RequestWrapper {
linbvirtCommands.put(GetStorageStatsCommand.class, new LibvirtGetStorageStatsCommandWrapper());
linbvirtCommands.put(UpgradeSnapshotCommand.class, new LibvirtUpgradeSnapshotCommandWrapper());
linbvirtCommands.put(DeleteStoragePoolCommand.class, new LibvirtDeleteStoragePoolCommandWrapper());
linbvirtCommands.put(OvsSetupBridgeCommand.class, new LibvirtOvsSetupBridgeCommandWrapper());
linbvirtCommands.put(OvsDestroyBridgeCommand.class, new LibvirtOvsDestroyBridgeCommandWrapper());
resources.put(LibvirtComputingResource.class, linbvirtCommands);
}

View File

@ -77,6 +77,8 @@ import com.cloud.agent.api.GetVncPortCommand;
import com.cloud.agent.api.MaintainCommand;
import com.cloud.agent.api.MigrateCommand;
import com.cloud.agent.api.ModifySshKeysCommand;
import com.cloud.agent.api.OvsDestroyBridgeCommand;
import com.cloud.agent.api.OvsSetupBridgeCommand;
import com.cloud.agent.api.PingTestCommand;
import com.cloud.agent.api.PrepareForMigrationCommand;
import com.cloud.agent.api.ReadyCommand;
@ -764,6 +766,203 @@ public class LibvirtComputingResourceTest {
verify(diskTO, times(1)).getType();
}
@Test
public void testPrepareForMigrationCommandMigration() {
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);
when(storagePoolManager.connectPhysicalDisksViaVmSpec(vm)).thenReturn(true);
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(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();
}
@SuppressWarnings("unchecked")
@Test
public void testPrepareForMigrationCommandLibvirtException() {
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 VifDriver vifDriver = Mockito.mock(VifDriver.class);
final PrepareForMigrationCommand command = new PrepareForMigrationCommand(vm);
when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper);
try {
when(libvirtConnectionWrapper.getConnectionByVmName(vm.getName())).thenThrow(LibvirtException.class);
} catch (final LibvirtException e) {
fail(e.getMessage());
}
when(vm.getNics()).thenReturn(new NicTO[]{nicTO});
when(nicTO.getType()).thenReturn(TrafficType.Guest);
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();
}
@SuppressWarnings("unchecked")
@Test
public void testPrepareForMigrationCommandURISyntaxException() {
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 volume = 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[]{volume});
when(nicTO.getType()).thenReturn(TrafficType.Guest);
when(volume.getType()).thenReturn(Volume.Type.ISO);
when(libvirtComputingResource.getVifDriver(nicTO.getType())).thenReturn(vifDriver);
when(libvirtComputingResource.getStoragePoolMgr()).thenReturn(storagePoolManager);
try {
when(libvirtComputingResource.getVolumePath(conn, volume)).thenThrow(URISyntaxException.class);
} catch (final LibvirtException e) {
fail(e.getMessage());
} catch (final URISyntaxException 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)).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(volume, times(1)).getType();
}
@SuppressWarnings("unchecked")
@Test
public void testPrepareForMigrationCommandInternalErrorException() {
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 volume = Mockito.mock(DiskTO.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(nicTO.getType()).thenReturn(TrafficType.Guest);
when(libvirtComputingResource.getVifDriver(nicTO.getType())).thenThrow(InternalErrorException.class);
when(libvirtComputingResource.getStoragePoolMgr()).thenReturn(storagePoolManager);
try {
when(libvirtComputingResource.getVolumePath(conn, volume)).thenReturn("/path");
} catch (final LibvirtException e) {
fail(e.getMessage());
} catch (final URISyntaxException 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)).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();
}
@Test(expected = UnsatisfiedLinkError.class)
public void testMigrateCommand() {
// The Connect constructor used inside the LibvirtMigrateCommandWrapper has a call to native methods, which
@ -1708,4 +1907,92 @@ public class LibvirtComputingResourceTest {
verify(libvirtComputingResource, times(1)).getStoragePoolMgr();
verify(storagePoolMgr, times(1)).deleteStoragePool(pool.getType(), pool.getUuid());
}
@Test
public void testOvsSetupBridgeCommand() {
final String name = "Test";
final Long hostId = 1l;
final Long networkId = 1l;
final OvsSetupBridgeCommand command = new OvsSetupBridgeCommand(name, hostId, networkId);
when(libvirtComputingResource.findOrCreateTunnelNetwork(command.getBridgeName())).thenReturn(true);
when(libvirtComputingResource.configureTunnelNetwork(command.getNetworkId(), command.getHostId(),
command.getBridgeName())).thenReturn(true);
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
assertNotNull(wrapper);
final Answer answer = wrapper.execute(command, libvirtComputingResource);
assertTrue(answer.getResult());
verify(libvirtComputingResource, times(1)).findOrCreateTunnelNetwork(command.getBridgeName());
verify(libvirtComputingResource, times(1)).configureTunnelNetwork(command.getNetworkId(), command.getHostId(),
command.getBridgeName());
}
@Test
public void testOvsSetupBridgeCommandFailure() {
final String name = "Test";
final Long hostId = 1l;
final Long networkId = 1l;
final OvsSetupBridgeCommand command = new OvsSetupBridgeCommand(name, hostId, networkId);
when(libvirtComputingResource.findOrCreateTunnelNetwork(command.getBridgeName())).thenReturn(true);
when(libvirtComputingResource.configureTunnelNetwork(command.getNetworkId(), command.getHostId(),
command.getBridgeName())).thenReturn(false);
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
assertNotNull(wrapper);
final Answer answer = wrapper.execute(command, libvirtComputingResource);
assertFalse(answer.getResult());
verify(libvirtComputingResource, times(1)).findOrCreateTunnelNetwork(command.getBridgeName());
verify(libvirtComputingResource, times(1)).configureTunnelNetwork(command.getNetworkId(), command.getHostId(),
command.getBridgeName());
}
@Test
public void testOvsDestroyBridgeCommand() {
final String name = "Test";
final Long hostId = 1l;
final Long networkId = 1l;
final OvsDestroyBridgeCommand command = new OvsDestroyBridgeCommand(networkId, name, hostId);
when(libvirtComputingResource.destroyTunnelNetwork(command.getBridgeName())).thenReturn(true);
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
assertNotNull(wrapper);
final Answer answer = wrapper.execute(command, libvirtComputingResource);
assertTrue(answer.getResult());
verify(libvirtComputingResource, times(1)).destroyTunnelNetwork(command.getBridgeName());
}
@Test
public void testOvsDestroyBridgeCommandFailure() {
final String name = "Test";
final Long hostId = 1l;
final Long networkId = 1l;
final OvsDestroyBridgeCommand command = new OvsDestroyBridgeCommand(networkId, name, hostId);
when(libvirtComputingResource.destroyTunnelNetwork(command.getBridgeName())).thenReturn(false);
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
assertNotNull(wrapper);
final Answer answer = wrapper.execute(command, libvirtComputingResource);
assertFalse(answer.getResult());
verify(libvirtComputingResource, times(1)).destroyTunnelNetwork(command.getBridgeName());
}
}