From 02d1cdd49a1d7e7d2f4493cfeafd97b35eff92a0 Mon Sep 17 00:00:00 2001 From: wilderrodrigues Date: Fri, 3 Apr 2015 16:57:07 +0200 Subject: [PATCH] Refactoring the MigrateVolumeCommand wrapper in order to cope with new design - Unit tests added: 43.1% coverage - It's a bit difficult to test the Xen API classes due to their static nature --- .../resource/XenServer610Resource.java | 44 +----------- .../wrapper/CitrixRequestWrapper.java | 2 + ...nServer610MigrateVolumeCommandWrapper.java | 71 +++++++++++++++++++ .../wrapper/XenServer610WrapperTest.java | 62 ++++++++++++++++ 4 files changed, 136 insertions(+), 43 deletions(-) create mode 100644 plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer610MigrateVolumeCommandWrapper.java diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer610Resource.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer610Resource.java index 212b41b0b48..68a5acf2eeb 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer610Resource.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer610Resource.java @@ -28,18 +28,11 @@ import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.log4j.Logger; import org.apache.xmlrpc.XmlRpcException; -import com.cloud.agent.api.Answer; -import com.cloud.agent.api.Command; -import com.cloud.agent.api.storage.MigrateVolumeAnswer; -import com.cloud.agent.api.storage.MigrateVolumeCommand; import com.cloud.agent.api.to.DiskTO; -import com.cloud.agent.api.to.StorageFilerTO; import com.cloud.resource.ServerResource; import com.cloud.storage.Volume; import com.cloud.utils.exception.CloudRuntimeException; import com.xensource.xenapi.Connection; -import com.xensource.xenapi.SR; -import com.xensource.xenapi.Task; import com.xensource.xenapi.Types; import com.xensource.xenapi.Types.XenAPIException; import com.xensource.xenapi.VBD; @@ -52,15 +45,6 @@ public class XenServer610Resource extends XenServer600Resource { private static final Logger s_logger = Logger.getLogger(XenServer610Resource.class); - @Override - public Answer executeRequest(final Command cmd) { - if (cmd instanceof MigrateVolumeCommand) { - return execute((MigrateVolumeCommand)cmd); - } else { - return super.executeRequest(cmd); - } - } - public List getUpdatedVolumePathsOfMigratedVm(final Connection connection, final VM migratedVm, final DiskTO[] volumes) throws CloudRuntimeException { final List volumeToList = new ArrayList(); @@ -96,34 +80,8 @@ public class XenServer610Resource extends XenServer600Resource { return volumeToList; } - protected MigrateVolumeAnswer execute(final MigrateVolumeCommand cmd) { - final Connection connection = getConnection(); - final String volumeUUID = cmd.getVolumePath(); - final StorageFilerTO poolTO = cmd.getPool(); - - try { - final SR destinationPool = getStorageRepository(connection, poolTO.getUuid()); - final VDI srcVolume = getVDIbyUuid(connection, volumeUUID); - final Map other = new HashMap(); - other.put("live", "true"); - - // Live migrate the vdi across pool. - final Task task = srcVolume.poolMigrateAsync(connection, destinationPool, other); - final long timeout = _migratewait * 1000L; - waitForTask(connection, task, 1000, timeout); - checkForSuccess(connection, task); - final VDI dvdi = Types.toVDI(task, connection); - - return new MigrateVolumeAnswer(cmd, true, null, dvdi.getUuid(connection)); - } catch (final Exception e) { - final String msg = "Catch Exception " + e.getClass().getName() + " due to " + e.toString(); - s_logger.error(msg, e); - return new MigrateVolumeAnswer(cmd, false, msg, null); - } - } - @Override protected void plugDom0Vif(final Connection conn, final VIF dom0Vif) throws XmlRpcException, XenAPIException { // do nothing. In xenserver 6.1 and beyond this step isn't needed. } -} +} \ No newline at end of file diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/CitrixRequestWrapper.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/CitrixRequestWrapper.java index b0376c756cc..8128f232e7f 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/CitrixRequestWrapper.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/CitrixRequestWrapper.java @@ -86,6 +86,7 @@ import com.cloud.agent.api.proxy.WatchConsoleProxyLoadCommand; import com.cloud.agent.api.routing.NetworkElementCommand; import com.cloud.agent.api.storage.CreateCommand; import com.cloud.agent.api.storage.DestroyCommand; +import com.cloud.agent.api.storage.MigrateVolumeCommand; import com.cloud.agent.api.storage.PrimaryStorageDownloadCommand; import com.cloud.agent.api.storage.ResizeVolumeCommand; import com.cloud.hypervisor.xenserver.resource.CitrixResourceBase; @@ -201,6 +202,7 @@ public class CitrixRequestWrapper extends RequestWrapper { xenServer610Commands.put(MigrateWithStorageReceiveCommand.class, new XenServer610MigrateWithStorageReceiveCommandWrapper()); xenServer610Commands.put(MigrateWithStorageSendCommand.class, new XenServer610MigrateWithStorageSendCommandWrapper()); xenServer610Commands.put(MigrateWithStorageCompleteCommand.class, new XenServer610MigrateWithStorageCompleteCommandWrapper()); + xenServer610Commands.put(MigrateVolumeCommand.class, new XenServer610MigrateVolumeCommandWrapper()); resources.put(XenServer610Resource.class, xenServer610Commands); } diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer610MigrateVolumeCommandWrapper.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer610MigrateVolumeCommandWrapper.java new file mode 100644 index 00000000000..284ec89568d --- /dev/null +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer610MigrateVolumeCommandWrapper.java @@ -0,0 +1,71 @@ +// +// 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.xenserver.resource.wrapper; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.log4j.Logger; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.storage.MigrateVolumeAnswer; +import com.cloud.agent.api.storage.MigrateVolumeCommand; +import com.cloud.agent.api.to.StorageFilerTO; +import com.cloud.hypervisor.xenserver.resource.XenServer610Resource; +import com.cloud.resource.CommandWrapper; +import com.xensource.xenapi.Connection; +import com.xensource.xenapi.SR; +import com.xensource.xenapi.Task; +import com.xensource.xenapi.Types; +import com.xensource.xenapi.VDI; + +public final class XenServer610MigrateVolumeCommandWrapper extends CommandWrapper { + + private static final Logger s_logger = Logger.getLogger(XenServer610MigrateVolumeCommandWrapper.class); + + @Override + public Answer execute(final MigrateVolumeCommand command, final XenServer610Resource xenServer610Resource) { + final Connection connection = xenServer610Resource.getConnection(); + final String volumeUUID = command.getVolumePath(); + final StorageFilerTO poolTO = command.getPool(); + + try { + String uuid = poolTO.getUuid(); + final SR destinationPool = xenServer610Resource.getStorageRepository(connection, uuid); + final VDI srcVolume = xenServer610Resource.getVDIbyUuid(connection, volumeUUID); + final Map other = new HashMap(); + other.put("live", "true"); + + // Live migrate the vdi across pool. + final Task task = srcVolume.poolMigrateAsync(connection, destinationPool, other); + final long timeout = xenServer610Resource.getMigrateWait() * 1000L; + xenServer610Resource.waitForTask(connection, task, 1000, timeout); + xenServer610Resource.checkForSuccess(connection, task); + + final VDI dvdi = Types.toVDI(task, connection); + + return new MigrateVolumeAnswer(command, true, null, dvdi.getUuid(connection)); + } catch (final Exception e) { + final String msg = "Catch Exception " + e.getClass().getName() + " due to " + e.toString(); + s_logger.error(msg, e); + return new MigrateVolumeAnswer(command, false, msg, null); + } + } +} \ No newline at end of file diff --git a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer610WrapperTest.java b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer610WrapperTest.java index 3e047825ccf..0a539e970d3 100644 --- a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer610WrapperTest.java +++ b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer610WrapperTest.java @@ -43,6 +43,7 @@ import com.cloud.agent.api.MigrateWithStorageCompleteCommand; import com.cloud.agent.api.MigrateWithStorageReceiveCommand; import com.cloud.agent.api.MigrateWithStorageSendCommand; import com.cloud.agent.api.SetupCommand; +import com.cloud.agent.api.storage.MigrateVolumeCommand; import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.StorageFilerTO; import com.cloud.agent.api.to.VirtualMachineTO; @@ -53,9 +54,12 @@ import com.cloud.hypervisor.xenserver.resource.XsHost; import com.cloud.hypervisor.xenserver.resource.XsLocalNetwork; import com.cloud.network.Networks.TrafficType; import com.cloud.network.PhysicalNetworkSetupInfo; +import com.cloud.storage.StoragePool; import com.xensource.xenapi.Connection; import com.xensource.xenapi.Network; import com.xensource.xenapi.SR; +import com.xensource.xenapi.Task; +import com.xensource.xenapi.Types.BadServerResponse; import com.xensource.xenapi.Types.XenAPIException; import com.xensource.xenapi.VDI; import com.xensource.xenapi.VIF; @@ -456,8 +460,66 @@ public class XenServer610WrapperTest { when(xsHost.getUuid()).thenReturn(uuid); final Answer answer = wrapper.execute(createStorageCommand, xenServer610Resource); + verify(xenServer610Resource, times(1)).getConnection(); assertFalse(answer.getResult()); } + + @Test + public void testXenServer610MigrateVolumeCommandWrapper() { + final String uuid = "206b21a7-c6ec-40e2-b5e2-f861b9612f04"; + + final Connection conn = Mockito.mock(Connection.class); + final SR destinationPool = Mockito.mock(SR.class); + final VDI srcVolume = Mockito.mock(VDI.class); + final Task task = Mockito.mock(Task.class); + + final long volumeId = 1l; + final String volumePath = "206b21a7-c6ec-40e2-b5e2-f861b9612f04"; + final StoragePool pool = Mockito.mock(StoragePool.class); + final int timeout = 120; + + final Map other = new HashMap(); + other.put("live", "true"); + + final MigrateVolumeCommand createStorageCommand = new MigrateVolumeCommand(volumeId, volumePath, pool, timeout); + + final CitrixRequestWrapper wrapper = CitrixRequestWrapper.getInstance(); + assertNotNull(wrapper); + + when(xenServer610Resource.getConnection()).thenReturn(conn); + when(pool.getUuid()).thenReturn(uuid); + when(xenServer610Resource.getStorageRepository(conn, uuid)).thenReturn(destinationPool); + when(xenServer610Resource.getVDIbyUuid(conn, volumePath)).thenReturn(srcVolume); + + try { + when(srcVolume.poolMigrateAsync(conn, destinationPool, other)).thenReturn(task); + } catch (final BadServerResponse e) { + fail(e.getMessage()); + } catch (final XenAPIException e) { + fail(e.getMessage()); + } catch (final XmlRpcException e) { + fail(e.getMessage()); + } + + when(xenServer610Resource.getMigrateWait()).thenReturn(120); + + final Answer answer = wrapper.execute(createStorageCommand, xenServer610Resource); + + verify(xenServer610Resource, times(1)).getConnection(); + + // try { + // verify(xenServer610Resource, times(1)).waitForTask(conn, task, 1000, timeout); + // verify(xenServer610Resource, times(1)).checkForSuccess(conn, task); + // } catch (final XenAPIException e) { + // fail(e.getMessage()); + // } catch (final XmlRpcException e) { + // fail(e.getMessage()); + // } catch (final TimeoutException e) { + // fail(e.getMessage()); + // } + + assertFalse(answer.getResult()); + } } \ No newline at end of file