diff --git a/api/src/com/cloud/agent/api/Answer.java b/api/src/com/cloud/agent/api/Answer.java index 9d106115d91..fd6a0d1b46a 100644 --- a/api/src/com/cloud/agent/api/Answer.java +++ b/api/src/com/cloud/agent/api/Answer.java @@ -26,16 +26,16 @@ public class Answer extends Command { this(null); } - public Answer(Command command) { + public Answer(final Command command) { this(command, true, null); } - public Answer(Command command, boolean success, String details) { + public Answer(final Command command, final boolean success, final String details) { result = success; this.details = details; } - public Answer(Command command, Exception e) { + public Answer(final Command command, final Exception e) { this(command, false, ExceptionUtil.toString(e)); } @@ -52,11 +52,11 @@ public class Answer extends Command { return false; } - public static UnsupportedAnswer createUnsupportedCommandAnswer(Command cmd) { - return new UnsupportedAnswer(cmd, "Unsupported command issued:" + cmd.toString() + ". Are you sure you got the right type of server?"); + public static UnsupportedAnswer createUnsupportedCommandAnswer(final Command cmd) { + return new UnsupportedAnswer(cmd, "Unsupported command issued: " + cmd.toString() + ". Are you sure you got the right type of server?"); } - public static UnsupportedAnswer createUnsupportedVersionAnswer(Command cmd) { + public static UnsupportedAnswer createUnsupportedVersionAnswer(final Command cmd) { return new UnsupportedAnswer(cmd, "Unsuppored Version."); } -} +} \ No newline at end of file diff --git a/core/test/org/apache/cloudstack/api/agent/test/AnswerTest.java b/core/test/org/apache/cloudstack/api/agent/test/AnswerTest.java index c685d682975..608791736ac 100644 --- a/core/test/org/apache/cloudstack/api/agent/test/AnswerTest.java +++ b/core/test/org/apache/cloudstack/api/agent/test/AnswerTest.java @@ -34,19 +34,19 @@ public class AnswerTest { @Test public void testExecuteInSequence() { - boolean b = a.executeInSequence(); + final boolean b = a.executeInSequence(); assertFalse(b); } @Test public void testGetResult() { - boolean b = a.getResult(); + final boolean b = a.getResult(); assertTrue(b); } @Test public void testGetDetails() { - String d = a.getDetails(); + final String d = a.getDetails(); assertTrue(d.equals("details")); } @@ -60,7 +60,7 @@ public class AnswerTest { assertFalse(b); String d = usa.getDetails(); - assertTrue(d.equals("Unsupported command issued:" + acc.toString() + ". Are you sure you got the right type of server?")); + assertTrue(d.contains("Unsupported command issued: " + acc.toString() + ". Are you sure you got the right type of server?")); usa = Answer.createUnsupportedVersionAnswer(acc); b = usa.executeInSequence(); diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XcpServerResource.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XcpServerResource.java index 63cdf114bf2..4b2936eb720 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XcpServerResource.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XcpServerResource.java @@ -41,7 +41,6 @@ import com.xensource.xenapi.VM; public class XcpServerResource extends CitrixResourceBase { private final static Logger s_logger = Logger.getLogger(XcpServerResource.class); private static final long mem_32m = 33554432L; - private String version; public XcpServerResource() { super(); diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer56SP2Resource.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer56SP2Resource.java index c0d6f004e3b..37748178fbb 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer56SP2Resource.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer56SP2Resource.java @@ -22,15 +22,12 @@ import java.util.List; import javax.ejb.Local; -import org.apache.log4j.Logger; - import com.cloud.resource.ServerResource; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; @Local(value = ServerResource.class) public class XenServer56SP2Resource extends XenServer56FP1Resource { - private static final Logger s_logger = Logger.getLogger(XenServer56SP2Resource.class); public XenServer56SP2Resource() { super(); @@ -40,13 +37,13 @@ public class XenServer56SP2Resource extends XenServer56FP1Resource { @Override protected List getPatchFiles() { - List files = new ArrayList(); - String patch = "scripts/vm/hypervisor/xenserver/xenserver56fp1/patch"; - String patchfilePath = Script.findScript("", patch); + final List files = new ArrayList(); + final String patch = "scripts/vm/hypervisor/xenserver/xenserver56fp1/patch"; + final String patchfilePath = Script.findScript("", patch); if (patchfilePath == null) { throw new CloudRuntimeException("Unable to find patch file " + patch); } - File file = new File(patchfilePath); + final File file = new File(patchfilePath); files.add(file); return files; } diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer600Resource.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer600Resource.java index 037ded32321..c6e5d82af06 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer600Resource.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer600Resource.java @@ -22,29 +22,22 @@ import java.util.List; import javax.ejb.Local; -import org.apache.log4j.Logger; - import com.cloud.resource.ServerResource; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; @Local(value = ServerResource.class) public class XenServer600Resource extends XenServer56SP2Resource { - private static final Logger s_logger = Logger.getLogger(XenServer600Resource.class); - - public XenServer600Resource() { - super(); - } @Override protected List getPatchFiles() { - List files = new ArrayList(); - String patch = "scripts/vm/hypervisor/xenserver/xenserver60/patch"; - String patchfilePath = Script.findScript("", patch); + final List files = new ArrayList(); + final String patch = "scripts/vm/hypervisor/xenserver/xenserver60/patch"; + final String patchfilePath = Script.findScript("", patch); if (patchfilePath == null) { throw new CloudRuntimeException("Unable to find patch file " + patch); } - File file = new File(patchfilePath); + final File file = new File(patchfilePath); files.add(file); return files; } 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 3c5caf03ce3..e71a97db2b6 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 @@ -239,7 +239,6 @@ public class XenServer610Resource extends XenServer600Resource { final Map token = cmd.getToken(); final String vmName = vmSpec.getName(); final Set volumeToSet = null; - boolean migrated = false; Task task = null; try { final Set vms = VM.getByNameLabel(connection, vmSpec.getName()); @@ -295,7 +294,6 @@ public class XenServer610Resource extends XenServer600Resource { throw new CloudRuntimeException("Error while migrating vm " + vmName, e); } - migrated = true; return new MigrateWithStorageSendAnswer(cmd, volumeToSet); } catch (final CloudRuntimeException e) { s_logger.error("Migration of vm " + vmName + " with storage failed due to " + e.toString(), e); diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer650Resource.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer650Resource.java index 4fa82a80680..98796eb0435 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer650Resource.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer650Resource.java @@ -18,32 +18,28 @@ */ package com.cloud.hypervisor.xenserver.resource; -import com.cloud.resource.ServerResource; -import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.script.Script; -import org.apache.log4j.Logger; - -import javax.ejb.Local; import java.io.File; import java.util.ArrayList; import java.util.List; +import javax.ejb.Local; + +import com.cloud.resource.ServerResource; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; + @Local(value=ServerResource.class) public class XenServer650Resource extends Xenserver625Resource { - private static final Logger s_logger = Logger.getLogger(XenServer650Resource.class); - - public XenServer650Resource() { - super(); - } + @Override protected List getPatchFiles() { - List files = new ArrayList(); - String patch = "scripts/vm/hypervisor/xenserver/xenserver65/patch"; - String patchfilePath = Script.findScript("", patch); + final List files = new ArrayList(); + final String patch = "scripts/vm/hypervisor/xenserver/xenserver65/patch"; + final String patchfilePath = Script.findScript("", patch); if (patchfilePath == null) { throw new CloudRuntimeException("Unable to find patch file " + patch); } - File file = new File(patchfilePath); + final File file = new File(patchfilePath); files.add(file); return files; } 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 8b0043f6437..b68ef38ddc4 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,6 +21,8 @@ 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; @@ -92,14 +94,14 @@ 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 { instance = new CitrixRequestWrapper(); } - private boolean initialised; - @SuppressWarnings("rawtypes") private final Hashtable, Hashtable, CommandWrapper>> resources; @@ -184,40 +186,95 @@ public class CitrixRequestWrapper extends RequestWrapper { final Hashtable, CommandWrapper> xenServer56P1Commands = new Hashtable, CommandWrapper>(); xenServer56P1Commands.put(FenceCommand.class, new XenServer56FP1FenceCommandWrapper()); resources.put(XenServer56FP1Resource.class, xenServer56P1Commands); - - initialised = true; } public static CitrixRequestWrapper getInstance() { return instance; } - boolean isInitialised() { - return initialised; - } - - @SuppressWarnings({ "unchecked", "rawtypes" }) + @SuppressWarnings({"rawtypes" }) @Override public Answer execute(final Command command, final ServerResource serverResource) { - Hashtable, CommandWrapper> commands = resources.get(serverResource.getClass()); + final Class resourceClass = serverResource.getClass(); - // Can't instantiate the CitrixResourceBase because it's abstract. In order to reuse the command with subclasses - // I need to do this check here. - if (commands == null) { - commands = resources.get(serverResource.getClass().getSuperclass()); - } + final Hashtable, CommandWrapper> resourceCommands = retrieveResource(command, resourceClass); - CommandWrapper commandWrapper = commands.get(command.getClass()); + CommandWrapper commandWrapper = retrieveCommands(command.getClass(), resourceCommands); - // This is temporary. We have to map the classes with several sub-classes better. - if (commandWrapper == null && command instanceof NetworkElementCommand) { - commandWrapper = commands.get(NetworkElementCommand.class); - } - - if (commandWrapper == null) { - throw new NullPointerException("No key found for '" + command.getClass() + "' in the Map!"); + while (commandWrapper == null) { + //Could not find the command in the given resource, will traverse the family tree. + commandWrapper = retryWhenAllFails(command, resourceClass, resourceCommands); } return commandWrapper.execute(command, serverResource); } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + protected Hashtable, CommandWrapper> retrieveResource(final Command command, final Class resourceClass) { + Class keepResourceClass = resourceClass; + Hashtable, CommandWrapper> resource = resources.get(keepResourceClass); + while (resource == null) { + try { + final Class keepResourceClass2 = (Class) keepResourceClass.getSuperclass(); + resource = resources.get(keepResourceClass2); + + keepResourceClass = keepResourceClass2; + } catch (final ClassCastException e) { + throw new NullPointerException("No key found for '" + command.getClass() + "' in the Map!"); + } + } + return resource; + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + protected CommandWrapper retrieveCommands(final Class commandClass, + final Hashtable, CommandWrapper> resourceCommands) { + + Class keepCommandClass = commandClass; + CommandWrapper commandWrapper = resourceCommands.get(keepCommandClass); + while (commandWrapper == null) { + try { + final Class commandClass2 = (Class) keepCommandClass.getSuperclass(); + + if (commandClass2 == null) { + throw new NullPointerException("All the COMMAND hierarchy tree has been visited but no compliant key has been found for '" + commandClass +"'."); + } + + commandWrapper = resourceCommands.get(commandClass2); + + keepCommandClass = commandClass2; + } catch (final NullPointerException e) { + // Will now traverse all the resource hierarchy. Returning null is not a problem. + // It is all being nicely checked and in case we do not have a resource, an Unsupported answer will be thrown by the base class. + return null; + } + } + return commandWrapper; + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + protected CommandWrapper retryWhenAllFails(final Command command, final Class resourceClass, + final Hashtable, CommandWrapper> resourceCommands) { + + Class keepResourceClass = resourceClass; + CommandWrapper commandWrapper = resourceCommands.get(command.getClass()); + while (commandWrapper == null) { + //Could not find the command in the given resource, will traverse the family tree. + try { + final Class resourceClass2 = (Class) keepResourceClass.getSuperclass(); + + if (resourceClass2 == null) { + throw new NullPointerException("All the SERVER-RESOURCE hierarchy tree has been visited but no compliant key has been found for '" + command.getClass() +"'."); + } + + final Hashtable, CommandWrapper> resourceCommands2 = retrieveResource(command, (Class) keepResourceClass.getSuperclass()); + keepResourceClass = resourceClass2; + + commandWrapper = retrieveCommands(command.getClass(), resourceCommands2); + } catch (final NullPointerException e) { + throw e; + } + } + return commandWrapper; + } } \ 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 286708a6e8f..1a02af4b57c 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 @@ -86,6 +86,7 @@ import com.cloud.agent.api.VMSnapshotTO; import com.cloud.agent.api.check.CheckSshCommand; import com.cloud.agent.api.proxy.CheckConsoleProxyLoadCommand; import com.cloud.agent.api.proxy.WatchConsoleProxyLoadCommand; +import com.cloud.agent.api.routing.IpAssocCommand; import com.cloud.agent.api.routing.IpAssocVpcCommand; import com.cloud.agent.api.storage.CreateAnswer; import com.cloud.agent.api.storage.CreateCommand; @@ -1753,7 +1754,7 @@ public class CitrixRequestWrapperTest { } @Test - public void testNetworkElementCommand() { + public void testIpAssocVpcCommand() { final VirtualRoutingResource routingResource = Mockito.mock(VirtualRoutingResource.class); final IpAddressTO [] ips = new IpAddressTO[0]; @@ -1771,6 +1772,26 @@ public class CitrixRequestWrapperTest { // Requires more testing, but the VirtualResourceRouting is quite big. assertNull(answer); } + + @Test + public void testIpAssocCommand() { + final VirtualRoutingResource routingResource = Mockito.mock(VirtualRoutingResource.class); + final IpAddressTO [] ips = new IpAddressTO[0]; + + final IpAssocCommand ipAssociation = new IpAssocCommand(ips); + + final CitrixRequestWrapper wrapper = CitrixRequestWrapper.getInstance(); + assertNotNull(wrapper); + + when(citrixResourceBase.getVirtualRoutingResource()).thenReturn(routingResource); + + final Answer answer = wrapper.execute(ipAssociation, citrixResourceBase); + + verify(routingResource, times(1)).executeRequest(ipAssociation); + + // Requires more testing, but the VirtualResourceRouting is quite big. + assertNull(answer); + } } class NotAValidCommand extends Command { diff --git a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer56WrapperTest.java b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer56WrapperTest.java index 2035c521b71..a004344cfa8 100644 --- a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer56WrapperTest.java +++ b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer56WrapperTest.java @@ -17,8 +17,11 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.CheckOnHostCommand; import com.cloud.agent.api.FenceCommand; import com.cloud.agent.api.NetworkUsageCommand; +import com.cloud.agent.api.SetupCommand; import com.cloud.host.Host; +import com.cloud.host.HostEnvironment; import com.cloud.hypervisor.xenserver.resource.XenServer56Resource; +import com.cloud.hypervisor.xenserver.resource.XsHost; import com.cloud.utils.ExecutionResult; import com.cloud.vm.VMInstanceVO; import com.xensource.xenapi.Connection; @@ -137,4 +140,22 @@ public class XenServer56WrapperTest { assertFalse(answer.getResult()); } + + @Test + public void testSetupCommand() { + final XsHost xsHost = Mockito.mock(XsHost.class); + final HostEnvironment env = Mockito.mock(HostEnvironment.class); + + final SetupCommand setupCommand = new SetupCommand(env); + + final CitrixRequestWrapper wrapper = CitrixRequestWrapper.getInstance(); + assertNotNull(wrapper); + + when(xenServer56Resource.getHost()).thenReturn(xsHost); + + final Answer answer = wrapper.execute(setupCommand, xenServer56Resource); + verify(xenServer56Resource, times(1)).getConnection(); + + assertFalse(answer.getResult()); + } } \ 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 new file mode 100644 index 00000000000..edc4b6d93d0 --- /dev/null +++ b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer610WrapperTest.java @@ -0,0 +1,56 @@ +package com.cloud.hypervisor.xenserver.resource.wrapper; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.List; + +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.CheckNetworkCommand; +import com.cloud.agent.api.SetupCommand; +import com.cloud.host.HostEnvironment; +import com.cloud.hypervisor.xenserver.resource.XenServer610Resource; +import com.cloud.network.PhysicalNetworkSetupInfo; + +@RunWith(PowerMockRunner.class) +public class XenServer610WrapperTest { + + @Mock + protected XenServer610Resource xenServer610Resource; + + @Test + public void testCheckNetworkCommandFailure() { + final XenServer610Resource xenServer610Resource = new XenServer610Resource(); + + final PhysicalNetworkSetupInfo info = new PhysicalNetworkSetupInfo(); + + final List setupInfos = new ArrayList(); + setupInfos.add(info); + + final CheckNetworkCommand checkNet = new CheckNetworkCommand(setupInfos); + + final Answer answer = xenServer610Resource.executeRequest(checkNet); + + assertTrue(answer.getResult()); + } + + @Test + public void testSetupCommand() { + final XenServer610Resource xenServer610Resource = new XenServer610Resource(); + + final HostEnvironment env = Mockito.mock(HostEnvironment.class); + + final SetupCommand setupCommand = new SetupCommand(env); + + final Answer answer = xenServer610Resource.executeRequest(setupCommand); + + assertFalse(answer.getResult()); + } +} \ No newline at end of file diff --git a/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620WrapperTest.java b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620WrapperTest.java new file mode 100644 index 00000000000..74fc9cd04c7 --- /dev/null +++ b/plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/wrapper/XenServer620WrapperTest.java @@ -0,0 +1,35 @@ +package com.cloud.hypervisor.xenserver.resource.wrapper; + +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.powermock.modules.junit4.PowerMockRunner; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CheckNetworkCommand; +import com.cloud.hypervisor.xenserver.resource.XenServer620Resource; +import com.cloud.network.PhysicalNetworkSetupInfo; + +@RunWith(PowerMockRunner.class) +public class XenServer620WrapperTest { + + @Test + public void testCheckNetworkCommandFailure() { + final XenServer620Resource xenServer620Resource = new XenServer620Resource(); + + final PhysicalNetworkSetupInfo info = new PhysicalNetworkSetupInfo(); + + final List setupInfos = new ArrayList(); + setupInfos.add(info); + + final CheckNetworkCommand checkNet = new CheckNetworkCommand(setupInfos); + + final Answer answer = xenServer620Resource.executeRequest(checkNet); + + assertTrue(answer.getResult()); + } +} \ No newline at end of file