From 1668a708f475b7174831aeae3f018bb524da731a Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sun, 16 Oct 2011 03:05:53 -0700 Subject: [PATCH] hardening script running --- ...nitScriptStatusIsZeroThenReturnOutput.java | 9 ++-- .../RunScriptOnNodeAsInitScriptUsingSsh.java | 5 +- ...itScriptUsingSshAndBlockUntilComplete.java | 5 +- .../ScriptStillRunningException.java | 20 +++++-- .../callables/SudoAwareInitManager.java | 34 ++++++------ .../ScriptStillRunningExceptionTest.java | 52 +++++++++++++++++++ 6 files changed, 94 insertions(+), 31 deletions(-) create mode 100644 compute/src/test/java/org/jclouds/compute/callable/ScriptStillRunningExceptionTest.java diff --git a/compute/src/main/java/org/jclouds/compute/callables/BlockUntilInitScriptStatusIsZeroThenReturnOutput.java b/compute/src/main/java/org/jclouds/compute/callables/BlockUntilInitScriptStatusIsZeroThenReturnOutput.java index eac3a80f11..e4f642f129 100644 --- a/compute/src/main/java/org/jclouds/compute/callables/BlockUntilInitScriptStatusIsZeroThenReturnOutput.java +++ b/compute/src/main/java/org/jclouds/compute/callables/BlockUntilInitScriptStatusIsZeroThenReturnOutput.java @@ -72,15 +72,14 @@ public class BlockUntilInitScriptStatusIsZeroThenReturnOutput extends AbstractFu final ScriptStatusReturnsZero stateRunning, @Assisted final SudoAwareInitManager commandRunner) { this.commandRunner = checkNotNull(commandRunner, "commandRunner"); this.userThreads = checkNotNull(userThreads, "userThreads"); - // arbitrarily high value, but Long.MAX_VALUE doesn't work! this.notRunningAnymore = new RetryablePredicate(new Predicate() { @Override public boolean apply(String arg0) { return commandRunner.runAction(arg0).getOutput().trim().equals(""); } - - }, TimeUnit.DAYS.toMillis(1)) { + // arbitrarily high value, but Long.MAX_VALUE doesn't work! + }, TimeUnit.DAYS.toMillis(365)) { /** * make sure we stop the retry loop if someone cancelled the future, this keeps threads * from being consumed on dead tasks @@ -128,7 +127,7 @@ public class BlockUntilInitScriptStatusIsZeroThenReturnOutput extends AbstractFu @Override protected void interruptTask() { logger.debug("<< cancelled(%s)", commandRunner.getStatement().getInstanceName()); - commandRunner.runAction("stop"); + commandRunner.refreshAndRunAction("stop"); shouldCancel = true; super.interruptTask(); } @@ -190,7 +189,7 @@ public class BlockUntilInitScriptStatusIsZeroThenReturnOutput extends AbstractFu try { return super.get(timeout, unit); } catch (TimeoutException e) { - throw new ScriptStillRunningException(e.getMessage(), this); + throw new ScriptStillRunningException(timeout, unit, this); } } diff --git a/compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSsh.java b/compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSsh.java index 6e63f0007e..cf834964a4 100644 --- a/compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSsh.java +++ b/compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSsh.java @@ -110,7 +110,7 @@ public class RunScriptOnNodeAsInitScriptUsingSsh extends SudoAwareInitManager im return new InitBuilder(name, path, path, Collections. emptyMap(), Collections.singleton(script)); } - public void refreshSshIfNewAdminCredentialsConfigured(AdminAccess input) { + protected void refreshSshIfNewAdminCredentialsConfigured(AdminAccess input) { if (input.getAdminCredentials() != null && input.shouldGrantSudoToAdminUser()) { ssh.disconnect(); logger.debug(">> reconnecting as %s@%s", input.getAdminCredentials().identity, ssh.getHostAddress()); @@ -121,9 +121,6 @@ public class RunScriptOnNodeAsInitScriptUsingSsh extends SudoAwareInitManager im } } - /** - * ssh client is initialized through this call. - */ protected ExecResponse doCall() { try { ssh.put(initFile, init.render(OsFamily.UNIX)); diff --git a/compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete.java b/compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete.java index 0034acf1d8..136db8e607 100644 --- a/compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete.java +++ b/compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete.java @@ -19,6 +19,7 @@ package org.jclouds.compute.callables; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import java.util.concurrent.TimeUnit; @@ -65,8 +66,8 @@ public class RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete extends Ru public BlockUntilInitScriptStatusIsZeroThenReturnOutput future() { ExecResponse returnVal = super.doCall(); - if (returnVal.getExitCode() != 0) - logger.warn("task: %s had non-zero exit status: %s", init.getInstanceName(), returnVal); + checkState(returnVal.getExitCode() == 0, String.format("task: %s had non-zero exit status: %s", init + .getInstanceName(), returnVal)); return statusFactory.create(this).init(); } diff --git a/compute/src/main/java/org/jclouds/compute/callables/ScriptStillRunningException.java b/compute/src/main/java/org/jclouds/compute/callables/ScriptStillRunningException.java index 98f014658a..b9754ce6d8 100644 --- a/compute/src/main/java/org/jclouds/compute/callables/ScriptStillRunningException.java +++ b/compute/src/main/java/org/jclouds/compute/callables/ScriptStillRunningException.java @@ -19,30 +19,40 @@ package org.jclouds.compute.callables; import static com.google.common.base.Preconditions.checkNotNull; +import static java.lang.String.format; +import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import org.jclouds.compute.domain.ExecResponse; + import com.google.common.base.Supplier; +import com.google.common.util.concurrent.ListenableFuture; /** * * @author Adrian Cole */ -public class ScriptStillRunningException extends TimeoutException implements - Supplier { +public class ScriptStillRunningException extends TimeoutException implements Supplier> { /** The serialVersionUID */ private static final long serialVersionUID = -7265376839848564663L; - private final BlockUntilInitScriptStatusIsZeroThenReturnOutput delegate; + private final ListenableFuture delegate; - public ScriptStillRunningException(String message, BlockUntilInitScriptStatusIsZeroThenReturnOutput delegate) { + public ScriptStillRunningException(long timeout, TimeUnit unit, ListenableFuture delegate) { + this(format("time up waiting %ds for %s to complete." + + " call get() on this exception to get access to the task in progress", TimeUnit.SECONDS.convert( + timeout, unit), delegate), delegate); + } + + public ScriptStillRunningException(String message, ListenableFuture delegate) { super(checkNotNull(message, "message")); this.delegate = checkNotNull(delegate, "delegate"); } @Override - public BlockUntilInitScriptStatusIsZeroThenReturnOutput get() { + public ListenableFuture get() { return delegate; } diff --git a/compute/src/main/java/org/jclouds/compute/callables/SudoAwareInitManager.java b/compute/src/main/java/org/jclouds/compute/callables/SudoAwareInitManager.java index ec1ce36bcd..daf50668b3 100644 --- a/compute/src/main/java/org/jclouds/compute/callables/SudoAwareInitManager.java +++ b/compute/src/main/java/org/jclouds/compute/callables/SudoAwareInitManager.java @@ -19,6 +19,7 @@ package org.jclouds.compute.callables; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import javax.annotation.PostConstruct; import javax.annotation.Resource; @@ -26,16 +27,16 @@ import javax.inject.Named; import org.jclouds.compute.domain.ExecResponse; import org.jclouds.compute.domain.NodeMetadata; -import org.jclouds.compute.domain.NodeMetadataBuilder; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.logging.Logger; import org.jclouds.scriptbuilder.InitBuilder; -import org.jclouds.scriptbuilder.statements.login.AdminAccess; import org.jclouds.ssh.SshClient; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Objects; +import com.google.common.base.Predicates; +import com.google.common.collect.ImmutableSet; /** * @@ -45,6 +46,7 @@ public class SudoAwareInitManager { @Resource @Named(ComputeServiceConstants.COMPUTE_LOGGER) protected Logger computeLogger = Logger.NULL; + protected Logger logger = Logger.NULL; protected NodeMetadata node; protected final InitBuilder init; protected final boolean runAsRoot; @@ -65,42 +67,44 @@ public class SudoAwareInitManager { return this; } - public void refreshSshIfNewAdminCredentialsConfigured(AdminAccess input) { - if (input.getAdminCredentials() != null && input.shouldGrantSudoToAdminUser()) { - ssh.disconnect(); - computeLogger.debug(">> reconnecting as %s@%s", input.getAdminCredentials().identity, ssh.getHostAddress()); - ssh = sshFactory.apply(node = NodeMetadataBuilder.fromNodeMetadata(node).adminPassword(null).credentials( - input.getAdminCredentials()).build()); + public ExecResponse refreshAndRunAction(String action) { + checkState(ssh != null, "please call init() before invoking call"); + try { ssh.connect(); + return runAction(action); + } finally { + if (ssh != null) + ssh.disconnect(); } } public ExecResponse runAction(String action) { ExecResponse returnVal; - String command = (runAsRoot) ? execScriptAsRoot(action) : execScriptAsDefaultUser(action); + String command = (runAsRoot && Predicates.in(ImmutableSet.of("start", "stop", "run")).apply(action)) ? execScriptAsRoot(action) + : execScriptAsDefaultUser(action); returnVal = runCommand(command); - if (computeLogger.isTraceEnabled()) + if ("status".equals(action)) + logger.trace("<< %s(%d)", action, returnVal.getExitCode()); + else if (computeLogger.isTraceEnabled()) computeLogger.trace("<< %s[%s]", action, returnVal); - else if ("status".equals(action)) - computeLogger.trace("<< %s(%d)", action, returnVal.getExitCode()); else computeLogger.debug("<< %s(%d)", action, returnVal.getExitCode()); return returnVal; } - public ExecResponse runCommand(String command) { + ExecResponse runCommand(String command) { String statement = String.format(">> running [%s] as %s@%s", command.replace( node.getAdminPassword() != null ? node.getAdminPassword() : "XXXXX", "XXXXX"), ssh.getUsername(), ssh .getHostAddress()); if (command.endsWith("status")) - computeLogger.trace(statement); + logger.trace(statement); else computeLogger.debug(statement); return ssh.exec(command); } @VisibleForTesting - public String execScriptAsRoot(String action) { + String execScriptAsRoot(String action) { String command; if (node.getCredentials().identity.equals("root")) { command = "./" + init.getInstanceName() + " " + action; diff --git a/compute/src/test/java/org/jclouds/compute/callable/ScriptStillRunningExceptionTest.java b/compute/src/test/java/org/jclouds/compute/callable/ScriptStillRunningExceptionTest.java new file mode 100644 index 0000000000..f78579c199 --- /dev/null +++ b/compute/src/test/java/org/jclouds/compute/callable/ScriptStillRunningExceptionTest.java @@ -0,0 +1,52 @@ +/** + * Licensed to jclouds, Inc. (jclouds) under one or more + * contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. jclouds 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 org.jclouds.compute.callable; + +import static org.testng.Assert.assertEquals; + +import java.util.concurrent.TimeUnit; + +import org.jclouds.compute.callables.ScriptStillRunningException; +import org.jclouds.compute.domain.ExecResponse; +import org.testng.annotations.Test; + +import com.google.common.util.concurrent.AbstractFuture; +import com.google.common.util.concurrent.ListenableFuture; + +/** + * @author Adam Lowe + */ +@Test(groups = "unit", singleThreaded = true, testName = "ScriptStillRunningExceptionTest") +public class ScriptStillRunningExceptionTest { + + public void simpleMessage() { + ListenableFuture future = new AbstractFuture() { + @Override + public String toString() { + return "task for foo"; + } + + }; + + ScriptStillRunningException testMe = new ScriptStillRunningException(1000, TimeUnit.MILLISECONDS, future); + assertEquals(testMe.getMessage(), + "time up waiting 1s for task for foo to complete. call get() on this exception to get access to the task in progress"); + + } +}