From 240bcb8120d709897baaade2d4e2cb29c5c482f9 Mon Sep 17 00:00:00 2001 From: wilderrodrigues Date: Fri, 3 Apr 2015 11:52:53 +0200 Subject: [PATCH] Refactoring the GetGPUStatsCommand wrapper in order to cope with new design - Unit tests added: 100% coverage --- .../cloud/agent/api/GetGPUStatsAnswer.java | 12 +- .../resource/XenServer620SP1Resource.java | 26 ---- .../resource/Xenserver625Resource.java | 5 +- .../wrapper/CitrixRequestWrapper.java | 10 +- ...Server620SP1GetGPUStatsCommandWrapper.java | 51 ++++++++ .../wrapper/CitrixRequestWrapperTest.java | 1 + .../wrapper/XenServer620SP1WrapperTest.java | 115 ++++++++++++++++++ 7 files changed, 182 insertions(+), 38 deletions(-) create mode 100644 plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620SP1GetGPUStatsCommandWrapper.java create mode 100644 plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620SP1WrapperTest.java diff --git a/core/src/com/cloud/agent/api/GetGPUStatsAnswer.java b/core/src/com/cloud/agent/api/GetGPUStatsAnswer.java index dbe9ae1609f..38bb9301c78 100644 --- a/core/src/com/cloud/agent/api/GetGPUStatsAnswer.java +++ b/core/src/com/cloud/agent/api/GetGPUStatsAnswer.java @@ -28,12 +28,16 @@ public class GetGPUStatsAnswer extends Answer { private HashMap> groupDetails; - public GetGPUStatsAnswer(GetGPUStatsCommand cmd, HashMap> groupDetails) { + public GetGPUStatsAnswer(final GetGPUStatsCommand cmd, final HashMap> groupDetails) { super(cmd); this.groupDetails = groupDetails; } - public HashMap> getGroupDetails() { - return this.groupDetails; + public GetGPUStatsAnswer(final GetGPUStatsCommand cmd, final boolean success, final String details) { + super(cmd, success, details); } -} + + public HashMap> getGroupDetails() { + return groupDetails; + } +} \ No newline at end of file diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer620SP1Resource.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer620SP1Resource.java index 88e42c04923..b9022b97507 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer620SP1Resource.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer620SP1Resource.java @@ -28,10 +28,6 @@ import javax.ejb.Local; 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.GetGPUStatsAnswer; -import com.cloud.agent.api.GetGPUStatsCommand; import com.cloud.agent.api.StartCommand; import com.cloud.agent.api.StartupRoutingCommand; import com.cloud.agent.api.VgpuTypesInfo; @@ -52,28 +48,6 @@ public class XenServer620SP1Resource extends XenServer620Resource { private static final Logger s_logger = Logger.getLogger(XenServer620SP1Resource.class); - @Override - public Answer executeRequest(final Command cmd) { - final Class clazz = cmd.getClass(); - if (clazz == GetGPUStatsCommand.class) { - return execute((GetGPUStatsCommand) cmd); - } else { - return super.executeRequest(cmd); - } - } - - protected GetGPUStatsAnswer execute(final GetGPUStatsCommand cmd) { - final Connection conn = getConnection(); - HashMap> groupDetails = new HashMap>(); - try { - groupDetails = getGPUGroupDetails(conn); - } catch (final Exception e) { - final String msg = "Unable to get GPU stats" + e.toString(); - s_logger.warn(msg, e); - } - return new GetGPUStatsAnswer(cmd, groupDetails); - } - @Override protected void fillHostInfo(final Connection conn, final StartupRoutingCommand cmd) { super.fillHostInfo(conn, cmd); diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625Resource.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625Resource.java index 6a782874fa7..58d64eb6756 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625Resource.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625Resource.java @@ -41,11 +41,8 @@ import com.xensource.xenapi.VM; @Local(value=ServerResource.class) public class Xenserver625Resource extends XenServerResourceNewBase { - private static final Logger s_logger = Logger.getLogger(Xenserver625Resource.class); - public Xenserver625Resource() { - super(); - } + private static final Logger s_logger = Logger.getLogger(Xenserver625Resource.class); @Override protected List getPatchFiles() { 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 b68ef38ddc4..e7e238f9597 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 @@ -21,8 +21,6 @@ package com.cloud.hypervisor.xenserver.resource.wrapper; import java.util.Hashtable; -import org.apache.log4j.Logger; - import com.cloud.agent.api.Answer; import com.cloud.agent.api.AttachIsoCommand; import com.cloud.agent.api.AttachVolumeCommand; @@ -38,6 +36,7 @@ import com.cloud.agent.api.CreateVMSnapshotCommand; import com.cloud.agent.api.DeleteStoragePoolCommand; import com.cloud.agent.api.DeleteVMSnapshotCommand; import com.cloud.agent.api.FenceCommand; +import com.cloud.agent.api.GetGPUStatsCommand; import com.cloud.agent.api.GetHostStatsCommand; import com.cloud.agent.api.GetStorageStatsCommand; import com.cloud.agent.api.GetVmDiskStatsCommand; @@ -94,8 +93,6 @@ import com.cloud.resource.ServerResource; public class CitrixRequestWrapper extends RequestWrapper { - private static final Logger s_logger = Logger.getLogger(CitrixRequestWrapper.class); - private static CitrixRequestWrapper instance; static { @@ -186,6 +183,11 @@ public class CitrixRequestWrapper extends RequestWrapper { final Hashtable, CommandWrapper> xenServer56P1Commands = new Hashtable, CommandWrapper>(); xenServer56P1Commands.put(FenceCommand.class, new XenServer56FP1FenceCommandWrapper()); resources.put(XenServer56FP1Resource.class, xenServer56P1Commands); + + // XenServer620SP1Resource commands + final Hashtable, CommandWrapper> xenServer620SP1Commands = new Hashtable, CommandWrapper>(); + xenServer620SP1Commands.put(GetGPUStatsCommand.class, new XenServer620SP1GetGPUStatsCommandWrapper()); + resources.put(XenServer56FP1Resource.class, xenServer620SP1Commands); } public static CitrixRequestWrapper getInstance() { diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620SP1GetGPUStatsCommandWrapper.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620SP1GetGPUStatsCommandWrapper.java new file mode 100644 index 00000000000..e177256c94b --- /dev/null +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620SP1GetGPUStatsCommandWrapper.java @@ -0,0 +1,51 @@ +// +// 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 org.apache.log4j.Logger; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.GetGPUStatsAnswer; +import com.cloud.agent.api.GetGPUStatsCommand; +import com.cloud.agent.api.VgpuTypesInfo; +import com.cloud.hypervisor.xenserver.resource.XenServer620SP1Resource; +import com.cloud.resource.CommandWrapper; +import com.xensource.xenapi.Connection; + +public final class XenServer620SP1GetGPUStatsCommandWrapper extends CommandWrapper { + + private static final Logger s_logger = Logger.getLogger(XenServer620SP1GetGPUStatsCommandWrapper.class); + + @Override + public Answer execute(final GetGPUStatsCommand command, final XenServer620SP1Resource xenServer620SP1Resource) { + final Connection conn = xenServer620SP1Resource.getConnection(); + HashMap> groupDetails = new HashMap>(); + try { + groupDetails = xenServer620SP1Resource.getGPUGroupDetails(conn); + } catch (final Exception e) { + final String msg = "Unable to get GPU stats" + e.toString(); + s_logger.warn(msg, e); + return new GetGPUStatsAnswer(command, false, msg); + } + return new GetGPUStatsAnswer(command, groupDetails); + } +} \ No newline at end of file diff --git a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/CitrixRequestWrapperTest.java b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/CitrixRequestWrapperTest.java index f32e2d4becf..060d3b5d13f 100644 --- a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/CitrixRequestWrapperTest.java +++ b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/CitrixRequestWrapperTest.java @@ -589,6 +589,7 @@ public class CitrixRequestWrapperTest { when(client.execute("host.get_by_uuid", new Object[] { "befc4dcd", uuid })).thenReturn(spiedMap); PowerMockito.when(conn, "dispatch", "host.get_by_uuid", params).thenReturn(spiedMap); } catch (final Exception e) { + fail(e.getMessage()); } // try { diff --git a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620SP1WrapperTest.java b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620SP1WrapperTest.java new file mode 100644 index 00000000000..67795fe93cd --- /dev/null +++ b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620SP1WrapperTest.java @@ -0,0 +1,115 @@ +// 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 static org.junit.Assert.assertFalse; +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.util.HashMap; + +import org.apache.xmlrpc.XmlRpcException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.powermock.modules.junit4.PowerMockRunner; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.GetGPUStatsCommand; +import com.cloud.agent.api.VgpuTypesInfo; +import com.cloud.hypervisor.xenserver.resource.XenServer620SP1Resource; +import com.cloud.utils.exception.CloudRuntimeException; +import com.xensource.xenapi.Connection; +import com.xensource.xenapi.Types.XenAPIException; + +@RunWith(PowerMockRunner.class) +public class XenServer620SP1WrapperTest { + + @Mock + private XenServer620SP1Resource xenServer620SP1Resource; + + @Test + public void testGetGPUStatsCommand() { + final String guuid = "246a5b75-05ed-4bbc-a171-2d1fe94a1b0e"; + + final Connection conn = Mockito.mock(Connection.class); + + final GetGPUStatsCommand gpuStats = new GetGPUStatsCommand(guuid, "xen"); + + final CitrixRequestWrapper wrapper = CitrixRequestWrapper.getInstance(); + assertNotNull(wrapper); + + when(xenServer620SP1Resource.getConnection()).thenReturn(conn); + try { + when(xenServer620SP1Resource.getGPUGroupDetails(conn)).thenReturn(new HashMap>()); + } catch (final XenAPIException e) { + fail(e.getMessage()); + } catch (final XmlRpcException e) { + fail(e.getMessage()); + } + + final Answer answer = wrapper.execute(gpuStats, xenServer620SP1Resource); + verify(xenServer620SP1Resource, times(1)).getConnection(); + try { + verify(xenServer620SP1Resource, times(1)).getGPUGroupDetails(conn); + } catch (final XenAPIException e) { + fail(e.getMessage()); + } catch (final XmlRpcException e) { + fail(e.getMessage()); + } + + assertTrue(answer.getResult()); + } + + @Test + public void testGetGPUStatsCommandFailure() { + final String guuid = "246a5b75-05ed-4bbc-a171-2d1fe94a1b0e"; + + final Connection conn = Mockito.mock(Connection.class); + + final GetGPUStatsCommand gpuStats = new GetGPUStatsCommand(guuid, "xen"); + + final CitrixRequestWrapper wrapper = CitrixRequestWrapper.getInstance(); + assertNotNull(wrapper); + + when(xenServer620SP1Resource.getConnection()).thenReturn(conn); + try { + when(xenServer620SP1Resource.getGPUGroupDetails(conn)).thenThrow(new CloudRuntimeException("Failed!")); + } catch (final XenAPIException e) { + fail(e.getMessage()); + } catch (final XmlRpcException e) { + fail(e.getMessage()); + } + + final Answer answer = wrapper.execute(gpuStats, xenServer620SP1Resource); + verify(xenServer620SP1Resource, times(1)).getConnection(); + try { + verify(xenServer620SP1Resource, times(1)).getGPUGroupDetails(conn); + } catch (final XenAPIException e) { + fail(e.getMessage()); + } catch (final XmlRpcException e) { + fail(e.getMessage()); + } + + assertFalse(answer.getResult()); + } +} \ No newline at end of file