From c889077f50aa2d36f0aa5eed2162047e3790d8e4 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sat, 29 Oct 2011 12:30:24 +0200 Subject: [PATCH] cleaning up virtualbox constructors, etc --- .../virtualbox/functions/IsoToIMachine.java | 31 +++++++----- .../admin/StartJettyIfNotAlreadyRunning.java | 27 +++++++---- .../admin/StartVBoxIfNotAlreadyRunning.java | 47 +++++++++++-------- ...rtualBoxComputeServiceAdapterLiveTest.java | 2 +- .../virtualbox/experiment/TestUtils.java | 2 +- 5 files changed, 65 insertions(+), 44 deletions(-) diff --git a/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/IsoToIMachine.java b/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/IsoToIMachine.java index 932a9a22b6..9cd2458158 100644 --- a/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/IsoToIMachine.java +++ b/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/IsoToIMachine.java @@ -35,7 +35,6 @@ import java.io.File; import javax.annotation.Resource; import javax.inject.Named; -import org.eclipse.jetty.server.Server; import org.jclouds.compute.ComputeServiceContext; import org.jclouds.compute.domain.ExecResponse; import org.jclouds.compute.options.RunScriptOptions; @@ -44,9 +43,7 @@ import org.jclouds.domain.Credentials; import org.jclouds.javax.annotation.Nullable; import org.jclouds.logging.Logger; import org.jclouds.ssh.SshException; -import org.jclouds.virtualbox.config.VirtualBoxConstants; import org.jclouds.virtualbox.domain.ExecutionType; -import org.jclouds.virtualbox.functions.admin.StartJettyIfNotAlreadyRunning; import org.jclouds.virtualbox.settings.KeyboardScancodes; import org.virtualbox_4_1.AccessMode; import org.virtualbox_4_1.DeviceType; @@ -104,9 +101,12 @@ public class IsoToIMachine implements Function { @Override public IMachine apply(@Nullable String isoName) { - String port = System.getProperty(VirtualBoxConstants.VIRTUALBOX_JETTY_PORT, "8080"); - String baseResource = "."; - Server server = new StartJettyIfNotAlreadyRunning(port).apply(baseResource); + // TODO: WTF :) this is a prerequisite, so check state as opposed to + // starting. + // ex checkState(endpoint accessible, "please start jetty on %s", + // endpoint) + // Server server = new + // StartJettyIfNotAlreadyRunning(port).apply(baseResource); IMachine vm = new CreateAndRegisterMachineFromIsoIfNotAlreadyExists(settingsFile, osTypeId, vmId, forceOverwrite, manager).apply(vmName); @@ -174,13 +174,18 @@ public class IsoToIMachine implements Function { } }); - try { - logger.debug("Stopping Jetty server..."); - server.stop(); - logger.debug("Jetty server stopped."); - } catch (Exception e) { - logger.error(e, "Could not stop Jetty server."); - } + // TODO: See above. + // if you want to manage jetty, do it outside this class as it + // has too many responsibilities otherwise. Allow this class to focus + // solely on making an IMachine + // + // try { + // logger.debug("Stopping Jetty server..."); + // server.stop(); + // logger.debug("Jetty server stopped."); + // } catch (Exception e) { + // logger.error(e, "Could not stop Jetty server."); + // } return vm; } diff --git a/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartJettyIfNotAlreadyRunning.java b/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartJettyIfNotAlreadyRunning.java index 96d5e4c21f..9ca1b1799d 100644 --- a/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartJettyIfNotAlreadyRunning.java +++ b/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartJettyIfNotAlreadyRunning.java @@ -19,6 +19,8 @@ package org.jclouds.virtualbox.functions.admin; +import static com.google.common.base.Preconditions.checkNotNull; + import javax.annotation.Resource; import javax.inject.Inject; import javax.inject.Named; @@ -47,18 +49,25 @@ public class StartJettyIfNotAlreadyRunning implements Function { @Named(ComputeServiceConstants.COMPUTE_LOGGER) protected Logger logger = Logger.NULL; + private final Server jetty; private final int port; + public StartJettyIfNotAlreadyRunning(Server jetty, @Named(VirtualBoxConstants.VIRTUALBOX_JETTY_PORT) int port) { + this.jetty = checkNotNull(jetty, "jetty"); + this.port = port; + } + + // TODO: getting an instance of the Server object should really be done in + // Guice, so inside a *Module class, perhaps as a @Provides method @Inject - public StartJettyIfNotAlreadyRunning(@Named(VirtualBoxConstants.VIRTUALBOX_JETTY_PORT) final String port) { - this.port = Integer.parseInt(port); + public StartJettyIfNotAlreadyRunning(@Named(VirtualBoxConstants.VIRTUALBOX_JETTY_PORT) int port) { + this(ServerJetty.getInstance().getServer(), port); } @Override public Server apply(@Nullable String baseResource) { - final Server server = ServerJetty.getInstance().getServer(); - - if (!server.getState().equals(Server.STARTED) + if (!jetty.getState().equals(Server.STARTED) + // TODO code smell = hard coding addresses or ports!! && !new InetSocketAddressConnect().apply(new IPSocket("localhost", port))) { ResourceHandler resource_handler = new ResourceHandler(); resource_handler.setDirectoriesListed(true); @@ -69,17 +78,17 @@ public class StartJettyIfNotAlreadyRunning implements Function { HandlerList handlers = new HandlerList(); handlers.setHandlers(new Handler[] { resource_handler, new DefaultHandler() }); - server.setHandler(handlers); + jetty.setHandler(handlers); try { - server.start(); + jetty.start(); } catch (Exception e) { logger.error(e, "Server jetty could not be started at this %s", baseResource); } - return server; + return jetty; } else { logger.debug("Server jetty serving %s already running. Skipping start", baseResource); - return server; + return jetty; } } diff --git a/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartVBoxIfNotAlreadyRunning.java b/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartVBoxIfNotAlreadyRunning.java index 9da826f9f1..59c4898800 100644 --- a/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartVBoxIfNotAlreadyRunning.java +++ b/sandbox-apis/virtualbox/src/main/java/org/jclouds/virtualbox/functions/admin/StartVBoxIfNotAlreadyRunning.java @@ -19,67 +19,74 @@ package org.jclouds.virtualbox.functions.admin; - +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static org.jclouds.compute.options.RunScriptOptions.Builder.runAsRoot; import java.net.URI; -import javax.annotation.Nullable; import javax.annotation.Resource; import javax.inject.Named; -import org.jclouds.compute.ComputeServiceContext; +import org.jclouds.compute.ComputeService; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.domain.Credentials; import org.jclouds.logging.Logger; import org.jclouds.net.IPSocket; -import org.jclouds.predicates.InetSocketAddressConnect; import org.virtualbox_4_1.VirtualBoxManager; import com.google.common.base.Function; +import com.google.common.base.Predicate; public class StartVBoxIfNotAlreadyRunning implements Function { @Resource @Named(ComputeServiceConstants.COMPUTE_LOGGER) protected Logger logger = Logger.NULL; - private ComputeServiceContext context; - private String hostId; - private Credentials credentials; - public StartVBoxIfNotAlreadyRunning(ComputeServiceContext context, String hostId, Credentials credentials) { - this.context = context; - this.hostId = hostId; - this.credentials = credentials; + private final ComputeService compute; + private final VirtualBoxManager manager; + private final Predicate socketTester; + private final String hostId; + private final Credentials credentials; + + public StartVBoxIfNotAlreadyRunning(ComputeService compute, VirtualBoxManager manager, + Predicate socketTester, String hostId, Credentials credentials) { + this.compute = checkNotNull(compute, "compute"); + this.manager = checkNotNull(manager, "manager"); + this.socketTester = checkNotNull(socketTester, "socketTester"); + this.hostId = checkNotNull(hostId, "hostId"); + this.credentials = checkNotNull(credentials, "credentials"); } @Override - public VirtualBoxManager apply(@Nullable URI endpoint) { + public VirtualBoxManager apply(URI endpoint) { + checkState(compute.getNodeMetadata(hostId) != null, "compute service %s cannot locate node with id %s", compute, + hostId); + checkNotNull(endpoint, "endpoint to virtualbox websrvd is needed"); - // TODO Really create new object here? Should we cache these instead? - VirtualBoxManager manager = VirtualBoxManager.createInstance(hostId); - - if (new InetSocketAddressConnect().apply(new IPSocket(endpoint.getHost(), endpoint.getPort()))) { + if (socketTester.apply(new IPSocket(endpoint.getHost(), endpoint.getPort()))) { manager.connect(endpoint.toASCIIString(), credentials.identity, credentials.credential); return manager; } logger.debug("disabling password access"); - context.getComputeService().runScriptOnNode(hostId, "VBoxManage setproperty websrvauthlibrary null", runAsRoot(false).wrapInInitScript(false)); + compute.runScriptOnNode(hostId, "VBoxManage setproperty websrvauthlibrary null", runAsRoot(false) + .wrapInInitScript(false)); logger.debug("starting vboxwebsrv"); String vboxwebsrv = "vboxwebsrv -t 10000 -v -b"; if (isOSX(hostId)) vboxwebsrv = "cd /Applications/VirtualBox.app/Contents/MacOS/ && " + vboxwebsrv; - context.getComputeService().runScriptOnNode(hostId, vboxwebsrv, runAsRoot(false).wrapInInitScript(false).blockOnComplete(false).nameTask("vboxwebsrv")); + compute.runScriptOnNode(hostId, vboxwebsrv, runAsRoot(false).wrapInInitScript(false).blockOnComplete(false) + .nameTask("vboxwebsrv")); manager.connect(endpoint.toASCIIString(), credentials.identity, credentials.credential); return manager; } private boolean isOSX(String hostId) { - return context.getComputeService().getNodeMetadata(hostId).getOperatingSystem().getDescription().equals( - "Mac OS X"); + return compute.getNodeMetadata(hostId).getOperatingSystem().getDescription().equals("Mac OS X"); } } diff --git a/sandbox-apis/virtualbox/src/test/java/org/jclouds/virtualbox/compute/VirtualBoxComputeServiceAdapterLiveTest.java b/sandbox-apis/virtualbox/src/test/java/org/jclouds/virtualbox/compute/VirtualBoxComputeServiceAdapterLiveTest.java index 893eb6a392..cb600efbd1 100644 --- a/sandbox-apis/virtualbox/src/test/java/org/jclouds/virtualbox/compute/VirtualBoxComputeServiceAdapterLiveTest.java +++ b/sandbox-apis/virtualbox/src/test/java/org/jclouds/virtualbox/compute/VirtualBoxComputeServiceAdapterLiveTest.java @@ -130,7 +130,7 @@ public class VirtualBoxComputeServiceAdapterLiveTest extends BaseVirtualBoxClien } @AfterGroups(groups = "live") - protected void tearDown() { + protected void tearDown() throws Exception { if (machine != null) adapter.destroyNode(machine.getId() + ""); super.tearDown(); diff --git a/sandbox-apis/virtualbox/src/test/java/org/jclouds/virtualbox/experiment/TestUtils.java b/sandbox-apis/virtualbox/src/test/java/org/jclouds/virtualbox/experiment/TestUtils.java index 38def58dac..05b133a19d 100644 --- a/sandbox-apis/virtualbox/src/test/java/org/jclouds/virtualbox/experiment/TestUtils.java +++ b/sandbox-apis/virtualbox/src/test/java/org/jclouds/virtualbox/experiment/TestUtils.java @@ -58,7 +58,7 @@ public class TestUtils { } public static ComputeServiceContext computeServiceForLocalhostAndGuest(String hostId, String hostname, - String guestId, String guestHostname, Credentials guestLogin) throws IOException { + String guestId, String guestHostname, Credentials guestLogin) { Node host = Node.builder().id(hostId).name("host installing virtualbox").hostname(hostname) .osFamily(OsFamily.LINUX.toString()).osDescription(System.getProperty("os.name"))