updated ec2 to support instances without a keypair to exist and not break computeservice

aws/core/src/main/java/org/jclouds/aws/ec2/compute/domain/RegionTag.java

@@ -72,4 +72,9 @@ public class RegionTag {
       return tag;
    }
 
+   @Override
+   public String toString() {
+      return "RegionTag [region=" + region + ", tag=" + tag + "]";
+   }
+
 }

aws/core/src/main/java/org/jclouds/aws/ec2/compute/functions/RunningInstanceToNodeMetadata.java

@@ -19,6 +19,7 @@
 package org.jclouds.aws.ec2.compute.functions;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.jclouds.util.Utils.nullSafeSet;
 
 import java.net.InetAddress;
 import java.net.URI;
@@ -28,7 +29,6 @@ import java.util.Set;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 
-import com.google.common.collect.Maps;
 import org.jclouds.aws.ec2.compute.domain.RegionTag;
 import org.jclouds.aws.ec2.domain.Image;
 import org.jclouds.aws.ec2.domain.InstanceState;
@@ -46,8 +46,8 @@ import org.jclouds.domain.Credentials;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
 
 /**
  * @author Adrian Cole
@@ -76,16 +76,23 @@ public class RunningInstanceToNodeMetadata implements Function<RunningInstance,
       String id = checkNotNull(instance, "instance").getId();
       String name = null; // user doesn't determine a node name;
       URI uri = null; // no uri to get rest access to host info
+
+      Credentials credentials = null;// default if no keypair exists
+      String tag = String.format("NOTAG-%s", instance.getId());// default if no keypair exists
+
+      if (instance.getKeyName() != null) {
+         tag = instance.getKeyName().replaceAll("-[0-9]+", "");
+         credentials = new Credentials(getLoginAccountFor(instance), getPrivateKeyOrNull(instance,
+                  tag));
+      }
+
       Map<String, String> userMetadata = ImmutableMap.<String, String> of();
-      String tag = instance.getKeyName().replaceAll("-[0-9]+", "");
+
       NodeState state = instanceToNodeState.get(instance.getInstanceState());
 
       Set<InetAddress> publicAddresses = nullSafeSet(instance.getIpAddress());
       Set<InetAddress> privateAddresses = nullSafeSet(instance.getPrivateIpAddress());
 
-      Credentials credentials = new Credentials(getLoginAccountFor(instance), getPrivateKeyOrNull(
-               instance, tag));
-
       String locationId = instance.getAvailabilityZone().toString();
 
       Map<String, String> extra = getExtra(instance);
@@ -97,9 +104,10 @@ public class RunningInstanceToNodeMetadata implements Function<RunningInstance,
    /**
     * Set extras for the node.
     * 
-     * Extras are derived from either additional API calls or
+    * Extras are derived from either additional API calls or hard-coded values.
-     * hard-coded values.
+    * 
-     * @param instance instance for which the extras are retrieved
+    * @param instance
+    *           instance for which the extras are retrieved
     * @return map with extras
     */
    @VisibleForTesting
@@ -108,8 +116,7 @@ public class RunningInstanceToNodeMetadata implements Function<RunningInstance,
 
       // put storage info
       /* TODO: only valid for UNIX */
-       InstanceTypeToStorageMappingUnix instanceToStorageMapping =
+      InstanceTypeToStorageMappingUnix instanceToStorageMapping = new InstanceTypeToStorageMappingUnix();
-               new InstanceTypeToStorageMappingUnix();
       extra.putAll(instanceToStorageMapping.apply(instance.getInstanceType()));
 
       return extra;
@@ -131,11 +138,4 @@ public class RunningInstanceToNodeMetadata implements Function<RunningInstance,
                + from.getImageId()).account;
    }
 
-   Set<InetAddress> nullSafeSet(InetAddress in) {
-      if (in == null) {
-         return ImmutableSet.<InetAddress> of();
-      }
-      return ImmutableSet.<InetAddress> of(in);
-   }
-
 }

aws/core/src/main/java/org/jclouds/aws/ec2/services/AMIClient.java

@@ -55,7 +55,7 @@ public interface AMIClient {
     *      />
     * @see DescribeImagesOptions
     */
-   @Timeout(duration = 180, timeUnit = TimeUnit.SECONDS)
+   @Timeout(duration = 300, timeUnit = TimeUnit.SECONDS)
    Set<Image> describeImagesInRegion(Region region, DescribeImagesOptions... options);
 
    /**

aws/core/src/test/java/org/jclouds/aws/ec2/compute/functions/ImageParserTest.java

@@ -42,7 +42,7 @@ import com.google.common.collect.Iterables;
 /**
  * @author Adrian Cole
  */
-@Test(groups = "unit", testName = "compute.PropertiesTest")
+@Test(groups = "unit", testName = "compute.ImageParserTest")
 public class ImageParserTest extends BaseHandlerTest {
 
    public void testParseAlesticCanonicalImage() {

aws/core/src/test/java/org/jclouds/aws/ec2/compute/functions/RunningInstanceToNodeMetadataTest.java

@@ -0,0 +1,150 @@
+/**
+ *
+ * Copyright (C) 2009 Cloud Conscious, LLC. <info@cloudconscious.com>
+ *
+ * ====================================================================
+ * Licensed 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.aws.ec2.compute.functions;
+
+import static org.easymock.EasyMock.expect;
+import static org.easymock.classextension.EasyMock.createMock;
+import static org.easymock.classextension.EasyMock.replay;
+import static org.easymock.classextension.EasyMock.verify;
+import static org.jclouds.aws.ec2.options.DescribeImagesOptions.Builder.imageIds;
+import static org.testng.Assert.assertEquals;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.Map;
+
+import org.jclouds.aws.domain.Region;
+import org.jclouds.aws.ec2.compute.domain.RegionTag;
+import org.jclouds.aws.ec2.domain.AvailabilityZone;
+import org.jclouds.aws.ec2.domain.Image;
+import org.jclouds.aws.ec2.domain.InstanceState;
+import org.jclouds.aws.ec2.domain.InstanceType;
+import org.jclouds.aws.ec2.domain.KeyPair;
+import org.jclouds.aws.ec2.domain.RunningInstance;
+import org.jclouds.aws.ec2.services.AMIClient;
+import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.strategy.PopulateDefaultLoginCredentialsForImageStrategy;
+import org.jclouds.domain.Credentials;
+import org.testng.annotations.Test;
+
+import com.google.common.collect.ImmutableSet;
+
+/**
+ * @author Adrian Cole
+ */
+@Test(groups = "unit", testName = "ec2.RunningInstanceToNodeMetadataTest")
+public class RunningInstanceToNodeMetadataTest {
+
+   @SuppressWarnings("unchecked")
+   @Test
+   public void testApplyWithNoKeyPairCreatesTagOfIdPrefixedByTagAndNullCredentials()
+            throws UnknownHostException {
+      AMIClient amiClient = createMock(AMIClient.class);
+      Map<RegionTag, KeyPair> credentialsMap = createMock(Map.class);
+      PopulateDefaultLoginCredentialsForImageStrategy credentialProvider = createMock(PopulateDefaultLoginCredentialsForImageStrategy.class);
+      RunningInstance instance = createMock(RunningInstance.class);
+
+      expect(instance.getId()).andReturn("id").atLeastOnce();
+      expect(instance.getKeyName()).andReturn(null).atLeastOnce();
+      expect(instance.getInstanceState()).andReturn(InstanceState.RUNNING);
+
+      expect(instance.getIpAddress()).andReturn(
+               InetAddress.getByAddress(new byte[] { 12, 10, 10, 1 }));
+      expect(instance.getPrivateIpAddress()).andReturn(
+               InetAddress.getByAddress(new byte[] { 10, 10, 10, 1 }));
+
+      expect(instance.getAvailabilityZone()).andReturn(AvailabilityZone.US_EAST_1A).atLeastOnce();
+
+      expect(instance.getInstanceType()).andReturn(InstanceType.C1_XLARGE).atLeastOnce();
+
+      replay(amiClient);
+      replay(credentialsMap);
+      replay(credentialProvider);
+      replay(instance);
+
+      RunningInstanceToNodeMetadata parser = new RunningInstanceToNodeMetadata(amiClient,
+               credentialsMap, credentialProvider);
+
+      NodeMetadata metadata = parser.apply(instance);
+
+      assertEquals(metadata.getTag(), "NOTAG-id");
+      assertEquals(metadata.getCredentials(), null);
+
+      verify(amiClient);
+      verify(credentialsMap);
+      verify(credentialProvider);
+      verify(instance);
+   }
+
+   @SuppressWarnings("unchecked")
+   @Test
+   public void testApplyWithKeyPairCreatesTagOfParsedKeyPairAndCredentialsBasedOnIt()
+            throws UnknownHostException {
+      AMIClient amiClient = createMock(AMIClient.class);
+      Map<RegionTag, KeyPair> credentialsMap = createMock(Map.class);
+      PopulateDefaultLoginCredentialsForImageStrategy credentialProvider = createMock(PopulateDefaultLoginCredentialsForImageStrategy.class);
+      RunningInstance instance = createMock(RunningInstance.class);
+      Image image = createMock(Image.class);
+
+      expect(instance.getId()).andReturn("id").atLeastOnce();
+      expect(instance.getKeyName()).andReturn("keyName-100").atLeastOnce();
+      expect(instance.getInstanceState()).andReturn(InstanceState.RUNNING);
+
+      expect(instance.getIpAddress()).andReturn(
+               InetAddress.getByAddress(new byte[] { 12, 10, 10, 1 }));
+      expect(instance.getPrivateIpAddress()).andReturn(
+               InetAddress.getByAddress(new byte[] { 10, 10, 10, 1 }));
+
+      expect(instance.getRegion()).andReturn(Region.US_EAST_1).atLeastOnce();
+
+      expect(instance.getImageId()).andReturn("imageId").atLeastOnce();
+
+      expect(amiClient.describeImagesInRegion(Region.US_EAST_1, imageIds("imageId"))).andReturn(
+               ImmutableSet.<Image> of(image));
+
+      expect(credentialProvider.execute(image)).andReturn(new Credentials("user", "pass"));
+
+      expect(credentialsMap.get(new RegionTag(Region.US_EAST_1, "keyName-100"))).andReturn(
+               new KeyPair(Region.US_EAST_1, "keyName-100", "keyFingerprint", "pass"));
+
+      expect(instance.getAvailabilityZone()).andReturn(AvailabilityZone.US_EAST_1A).atLeastOnce();
+
+      expect(instance.getInstanceType()).andReturn(InstanceType.C1_XLARGE).atLeastOnce();
+
+      replay(amiClient);
+      replay(credentialsMap);
+      replay(credentialProvider);
+      replay(instance);
+
+      RunningInstanceToNodeMetadata parser = new RunningInstanceToNodeMetadata(amiClient,
+               credentialsMap, credentialProvider);
+
+      NodeMetadata metadata = parser.apply(instance);
+
+      assertEquals(metadata.getTag(), "keyName");
+
+      assertEquals(metadata.getCredentials(), new Credentials("user", "pass"));
+
+      verify(amiClient);
+      verify(credentialsMap);
+      verify(credentialProvider);
+      verify(instance);
+   }
+
+}

compute/src/test/java/org/jclouds/compute/BaseComputeServiceLiveTest.java

@@ -304,16 +304,20 @@ public abstract class BaseComputeServiceLiveTest {
    }
 
    public void testGetNodesWithDetails() throws Exception {
-      for (Entry<String, ? extends ComputeMetadata> node : client.getNodes(new GetNodesOptions().withDetails()).entrySet()) {
+      for (Entry<String, ? extends ComputeMetadata> node : client.getNodes(
+               new GetNodesOptions().withDetails()).entrySet()) {
          assertEquals(node.getKey(), node.getValue().getId());
-         assert node.getValue().getId() != null;
+         assert node.getValue().getId() != null : node;
-         assert node.getValue().getLocationId() != null;
+         assert node.getValue().getLocationId() != null : node;
          assertEquals(node.getValue().getType(), ComputeType.NODE);
          assert node.getValue() instanceof NodeMetadata;
          NodeMetadata nodeMetadata = (NodeMetadata) node.getValue();
-         assertNotNull(nodeMetadata.getName());
+         assert nodeMetadata.getId() != null : nodeMetadata;
-         assertNotNull(nodeMetadata.getPublicAddresses());
+         // user specified name is not always supported
-         assert nodeMetadata.getPublicAddresses().size() > 1;
+         // assert nodeMetadata.getName() != null : nodeMetadata;
+         assert nodeMetadata.getPublicAddresses() != null : nodeMetadata;
+
+         assert nodeMetadata.getPublicAddresses().size() > 1 : nodeMetadata;
          assertNotNull(nodeMetadata.getPrivateAddresses());
       }
    }

core/src/main/java/org/jclouds/http/options/BaseHttpRequestOptions.java

@@ -96,4 +96,66 @@ public class BaseHttpRequestOptions implements HttpRequestOptions {
       return formParameters;
    }
 
+   @Override
+   public int hashCode() {
+      final int prime = 31;
+      int result = 1;
+      result = prime * result + ((formParameters == null) ? 0 : formParameters.hashCode());
+      result = prime * result + ((headers == null) ? 0 : headers.hashCode());
+      result = prime * result + ((matrixParameters == null) ? 0 : matrixParameters.hashCode());
+      result = prime * result + ((pathSuffix == null) ? 0 : pathSuffix.hashCode());
+      result = prime * result + ((payload == null) ? 0 : payload.hashCode());
+      result = prime * result + ((queryParameters == null) ? 0 : queryParameters.hashCode());
+      return result;
+   }
+
+   @Override
+   public boolean equals(Object obj) {
+      if (this == obj)
+         return true;
+      if (obj == null)
+         return false;
+      if (getClass() != obj.getClass())
+         return false;
+      BaseHttpRequestOptions other = (BaseHttpRequestOptions) obj;
+      if (formParameters == null) {
+         if (other.formParameters != null)
+            return false;
+      } else if (!formParameters.equals(other.formParameters))
+         return false;
+      if (headers == null) {
+         if (other.headers != null)
+            return false;
+      } else if (!headers.equals(other.headers))
+         return false;
+      if (matrixParameters == null) {
+         if (other.matrixParameters != null)
+            return false;
+      } else if (!matrixParameters.equals(other.matrixParameters))
+         return false;
+      if (pathSuffix == null) {
+         if (other.pathSuffix != null)
+            return false;
+      } else if (!pathSuffix.equals(other.pathSuffix))
+         return false;
+      if (payload == null) {
+         if (other.payload != null)
+            return false;
+      } else if (!payload.equals(other.payload))
+         return false;
+      if (queryParameters == null) {
+         if (other.queryParameters != null)
+            return false;
+      } else if (!queryParameters.equals(other.queryParameters))
+         return false;
+      return true;
+   }
+
+   @Override
+   public String toString() {
+      return "[formParameters=" + formParameters + ", headers=" + headers + ", matrixParameters="
+               + matrixParameters + ", pathSuffix=" + pathSuffix + ", payload=" + payload
+               + ", queryParameters=" + queryParameters + "]";
+   }
+
 }

core/src/main/java/org/jclouds/util/Utils.java

@@ -29,6 +29,7 @@ import java.io.OutputStream;
 import java.io.UnsupportedEncodingException;
 import java.util.Collection;
 import java.util.Map;
+import java.util.Set;
 import java.util.Map.Entry;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -40,6 +41,7 @@ import org.jclouds.logging.Logger;
 import com.google.common.base.Charsets;
 import com.google.common.base.Supplier;
 import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.io.ByteStreams;
 import com.google.common.io.Closeables;
 import com.google.common.io.OutputSupplier;
@@ -52,6 +54,13 @@ import com.google.common.io.OutputSupplier;
 public class Utils {
    public static final String UTF8_ENCODING = "UTF-8";
 
+   public static <T> Set<T> nullSafeSet(T in) {
+      if (in == null) {
+         return ImmutableSet.<T> of();
+      }
+      return ImmutableSet.<T> of(in);
+   }
+
    public static Object propagateOrNull(Exception from) {
       Throwables.propagate(from);
       assert false : "exception should have propogated";
@@ -204,6 +213,7 @@ public class Utils {
 
    /**
     * Will throw an exception if the argument is null or empty.
+    * 
     * @param nullableString
     *           string to verify. Can be null or empty.
     */
@@ -212,15 +222,14 @@ public class Utils {
    }
 
    /**
-     * Will throw an exception if the argument is null or empty. Accepts
+    * Will throw an exception if the argument is null or empty. Accepts a custom error message.
-     * a custom error message.
+    * 
     * @param nullableString
     *           string to verify. Can be null or empty.
     * @param message
     *           message to show in case of exception
     */
    public static void checkNotEmpty(String nullableString, String message) {
-        checkArgument(nullableString != null && nullableString.length() > 0,
+      checkArgument(nullableString != null && nullableString.length() > 0, message);
-                            message);
    }
 }