Skip to content
This repository has been archived by the owner on Mar 25, 2018. It is now read-only.

Commit

Permalink
Adding listNodesByIds across the board.
Browse files Browse the repository at this point in the history
Adding to both ListNodesStrategy and ComputeServiceAdapter. When
possible, does a query explicitly for the specified IDs. When not,
falls back on either listDetailsOnNodesMatching (for ListNodesStrategy
implementations and in BaseComputeService) or filters listNodes output
itself (in ComputeServiceAdapter).
  • Loading branch information
abayer committed Apr 9, 2013
1 parent eaff39c commit c566418
Show file tree
Hide file tree
Showing 27 changed files with 516 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
package org.jclouds.byon.internal;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Predicates.in;
import static com.google.common.collect.Iterables.filter;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.collect.Iterables.transform;
import static com.google.common.collect.Maps.filterKeys;

import java.util.Set;

Expand All @@ -44,6 +49,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.ImmutableSet.Builder;
import com.google.common.collect.Maps;
import com.google.common.util.concurrent.UncheckedExecutionException;
/**
*
Expand Down Expand Up @@ -80,14 +86,19 @@ public Iterable<Image> listImages() {

@Override
public Iterable<NodeMetadata> listNodes() {
return Iterables.transform(nodes.get().asMap().values(), converter);
return transform(nodes.get().asMap().values(), converter);
}

@Override
public Iterable<NodeMetadata> listNodesByIds(Iterable<String> ids) {
return transform(filterKeys(nodes.get().asMap(), in(ImmutableSet.copyOf(ids))).values(), converter);
}

@Override
public Iterable<Location> listLocations() {
Builder<Location> locations = ImmutableSet.builder();
Location provider = Iterables.getOnlyElement(locationSupplier.get());
Set<String> zones = ImmutableSet.copyOf(Iterables.filter(Iterables.transform(nodes.get().asMap().values(),
Location provider = getOnlyElement(locationSupplier.get());
Set<String> zones = ImmutableSet.copyOf(filter(transform(nodes.get().asMap().values(),
new Function<Node, String>() {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
package org.jclouds.cloudservers.compute.strategy;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Iterables.contains;
import static com.google.common.collect.Iterables.filter;
import static org.jclouds.cloudservers.options.CreateServerOptions.Builder.withMetadata;
import static org.jclouds.cloudservers.options.ListOptions.Builder.withDetails;
import static org.jclouds.compute.util.ComputeServiceUtils.metadataAndTagsAsCommaDelimitedValue;
Expand All @@ -37,7 +39,9 @@
import org.jclouds.domain.Location;
import org.jclouds.domain.LoginCredentials;

import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;

/**
* defines the connection between the {@link CloudServersClient} implementation and the jclouds
Expand Down Expand Up @@ -83,6 +87,17 @@ public Iterable<Server> listNodes() {
return client.listServers(ListOptions.Builder.withDetails());
}

@Override
public Iterable<Server> listNodesByIds(final Iterable<String> ids) {
return filter(listNodes(), new Predicate<Server>() {

@Override
public boolean apply(Server server) {
return contains(ids, server.getId());
}
});
}

@Override
public Iterable<Location> listLocations() {
// Not using the adapter to determine locations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,13 @@ public void testListNodes() throws Exception {
super.testListNodes();
}

@Test(enabled = true, dependsOnMethods = { "testListNodes", "testGetNodesWithDetails" })
@Test(enabled = true, dependsOnMethods = "testSuspendResume")
@Override
public void testListNodesByIds() throws Exception {
super.testListNodesByIds();
}

@Test(enabled = true, dependsOnMethods = { "testListNodes", "testGetNodesWithDetails", "listNodesByIds" })
@Override
public void testDestroyNodes() {
super.testDestroyNodes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Iterables.contains;
import static com.google.common.collect.Iterables.filter;
import static org.jclouds.concurrent.FutureIterables.transformParallel;

import javax.annotation.Resource;
Expand Down Expand Up @@ -62,6 +64,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSet.Builder;
import com.google.common.collect.Iterables;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
Expand Down Expand Up @@ -197,6 +200,17 @@ public Iterable<ServerInfo> listNodes() {
return (Iterable<ServerInfo>) client.listServerInfo();
}

@Override
public Iterable<ServerInfo> listNodesByIds(final Iterable<String> ids) {
return filter(listNodes(), new Predicate<ServerInfo>() {

@Override
public boolean apply(ServerInfo server) {
return contains(ids, server.getUuid());
}
});
}

@Override
public Iterable<Location> listLocations() {
// Not using the adapter to determine locations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.Iterables.contains;
import static com.google.common.collect.Iterables.filter;
import static com.google.common.collect.Iterables.get;
import static org.jclouds.cloudstack.options.DeployVirtualMachineOptions.Builder.displayName;
Expand Down Expand Up @@ -69,6 +70,7 @@
import com.google.common.base.Throwables;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.collect.ImmutableSet.Builder;
import com.google.common.primitives.Ints;
Expand Down Expand Up @@ -234,6 +236,17 @@ public Iterable<VirtualMachine> listNodes() {
return client.getVirtualMachineClient().listVirtualMachines();
}

@Override
public Iterable<VirtualMachine> listNodesByIds(final Iterable<String> ids) {
return filter(listNodes(), new Predicate<VirtualMachine>() {

@Override
public boolean apply(VirtualMachine vm) {
return contains(ids, vm.getId());
}
});
}

@Override
public Iterable<Zone> listLocations() {
// TODO: we may need to filter these
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,15 @@

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Predicates.and;
import static com.google.common.base.Predicates.in;
import static com.google.common.base.Predicates.notNull;
import static com.google.common.collect.Iterables.concat;
import static com.google.common.collect.Iterables.filter;
import static com.google.common.collect.Iterables.toArray;
import static com.google.common.collect.Iterables.transform;
import static com.google.common.collect.Multimaps.filterKeys;
import static com.google.common.collect.Multimaps.index;
import static com.google.common.collect.Multimaps.transformValues;
import static org.jclouds.concurrent.FutureIterables.transformParallel;

import java.util.Set;
Expand All @@ -33,12 +38,13 @@
import javax.inject.Singleton;

import org.jclouds.Constants;
import org.jclouds.aws.util.AWSUtils;
import org.jclouds.compute.domain.ComputeMetadata;
import org.jclouds.compute.domain.NodeMetadata;
import org.jclouds.compute.predicates.NodePredicates;
import org.jclouds.compute.reference.ComputeServiceConstants;
import org.jclouds.compute.strategy.ListNodesStrategy;
import org.jclouds.ec2.EC2AsyncClient;
import org.jclouds.ec2.EC2Client;
import org.jclouds.ec2.domain.Reservation;
import org.jclouds.ec2.domain.RunningInstance;
import org.jclouds.location.Region;
Expand All @@ -48,6 +54,8 @@
import com.google.common.base.Predicate;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Multimap;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.inject.Inject;
Expand All @@ -67,13 +75,13 @@ public class EC2ListNodesStrategy implements ListNodesStrategy {
@Named(Constants.PROPERTY_REQUEST_TIMEOUT)
protected static Long maxTime;

protected final EC2AsyncClient client;
protected final EC2Client client;
protected final Supplier<Set<String>> regions;
protected final Function<RunningInstance, NodeMetadata> runningInstanceToNodeMetadata;
protected final ListeningExecutorService userExecutor;

@Inject
protected EC2ListNodesStrategy(EC2AsyncClient client, @Region Supplier<Set<String>> regions,
protected EC2ListNodesStrategy(EC2Client client, @Region Supplier<Set<String>> regions,
Function<RunningInstance, NodeMetadata> runningInstanceToNodeMetadata,
@Named(Constants.PROPERTY_USER_THREADS) ListeningExecutorService userExecutor) {
this.client = checkNotNull(client, "client");
Expand All @@ -87,6 +95,22 @@ public Set<? extends ComputeMetadata> listNodes() {
return listDetailsOnNodesMatching(NodePredicates.all());
}

@Override
public Set<? extends NodeMetadata> listNodesByIds(Iterable<String> ids) {
Multimap<String, String> idsByHandles = index(ids, splitHandle(1));
Multimap<String, String> idsByRegions = transformValues(idsByHandles, splitHandle(0));
Multimap<String, String> idsByConfiguredRegions = filterKeys(idsByRegions, in(regions.get()));

if (idsByConfiguredRegions.isEmpty()) {
return ImmutableSet.of();
}

Iterable<? extends RunningInstance> instances = pollRunningInstancesByRegionsAndIds(idsByConfiguredRegions);
Iterable<? extends NodeMetadata> nodes = transform(filter(instances, notNull()),
runningInstanceToNodeMetadata);
return ImmutableSet.copyOf(nodes);
}

@Override
public Set<? extends NodeMetadata> listDetailsOnNodesMatching(Predicate<ComputeMetadata> filter) {
Iterable<? extends RunningInstance> instances = pollRunningInstances();
Expand All @@ -96,15 +120,50 @@ public Set<? extends NodeMetadata> listDetailsOnNodesMatching(Predicate<ComputeM
}

protected Iterable<? extends RunningInstance> pollRunningInstances() {
Iterable<? extends Set<? extends Reservation<? extends RunningInstance>>> reservations = transformParallel(
regions.get(), new Function<String, ListenableFuture<? extends Set<? extends Reservation<? extends RunningInstance>>>>() {

@Override
public ListenableFuture<? extends Set<? extends Reservation<? extends RunningInstance>>> apply(String from) {
return client.getInstanceServices().describeInstancesInRegion(from);
}
Iterable<? extends Set<? extends Reservation<? extends RunningInstance>>> reservations
= transform(regions.get(), allInstancesInRegion());

return concat(concat(reservations));
}

}, userExecutor, maxTime, logger, "reservations");
protected Iterable<? extends RunningInstance> pollRunningInstancesByRegionsAndIds(final Multimap<String,String> idsByRegions) {
Iterable<? extends Set<? extends Reservation<? extends RunningInstance>>> reservations
= transform(idsByRegions.keySet(), instancesByIdInRegion(idsByRegions));

return concat(concat(reservations));
}

protected Function<String, String> splitHandle(final int pos) {
return new Function<String, String>() {

@Override
public String apply(String handle) {
return AWSUtils.parseHandle(handle)[pos];
}
};
}

protected Function<String, Set<? extends Reservation<? extends RunningInstance>>> allInstancesInRegion() {
return new Function<String, Set<? extends Reservation<? extends RunningInstance>>>() {

@Override
public Set<? extends Reservation<? extends RunningInstance>> apply(String from) {
return client.getInstanceServices().describeInstancesInRegion(from);
}

};
}

protected Function<String, Set<? extends Reservation<? extends RunningInstance>>>
instancesByIdInRegion(final Multimap<String,String> idsByRegions) {
return new Function<String, Set<? extends Reservation<? extends RunningInstance>>>() {

@Override
public Set<? extends Reservation<? extends RunningInstance>> apply(String from) {
return client.getInstanceServices()
.describeInstancesInRegion(from, toArray(idsByRegions.get(from), String.class));
}

};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public void testCreateNodeWithGeneratedKeyPairAndOverriddenLoginUser() throws Ex
requestResponseMap.put(authorizeSecurityGroupIngressRequestGroup, authorizeSecurityGroupIngressResponse);
requestResponseMap.put(runInstancesRequest, runInstancesResponse);
requestResponseMap.put(describeInstanceRequest, describeInstanceResponse);
requestResponseMap.put(describeInstanceMultiIdsRequest, describeInstanceMultiIdsResponse);
requestResponseMap.put(describeImageRequest, describeImagesResponse);

ComputeService apiThatCreatesNode = requestsSendResponses(requestResponseMap.build());
Expand All @@ -77,6 +78,7 @@ public void testCreateNodeWithGeneratedKeyPairAndOverriddenLoginUserWithTemplate
requestResponseMap.put(authorizeSecurityGroupIngressRequestGroup, authorizeSecurityGroupIngressResponse);
requestResponseMap.put(runInstancesRequest, runInstancesResponse);
requestResponseMap.put(describeInstanceRequest, describeInstanceResponse);
requestResponseMap.put(describeInstanceMultiIdsRequest, describeInstanceMultiIdsResponse);
requestResponseMap.put(describeImageRequest, describeImagesResponse);

ComputeService apiThatCreatesNode = requestsSendResponses(requestResponseMap.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ protected Properties setupProperties() {
protected HttpResponse runInstancesResponse;
protected HttpRequest describeInstanceRequest;
protected HttpResponse describeInstanceResponse;
protected HttpRequest describeInstanceMultiIdsRequest;
protected HttpResponse describeInstanceMultiIdsResponse;
protected HttpRequest describeImageRequest;

public BaseEC2ComputeServiceExpectTest() {
Expand Down Expand Up @@ -190,6 +192,20 @@ protected void setupDefaultRequests() {
.payload(payloadFromResourceWithContentType(
"/describe_instances_running-1.xml", MediaType.APPLICATION_XML)).build();

describeInstanceMultiIdsRequest =
formSigner.filter(HttpRequest.builder()
.method("POST")
.endpoint("https://ec2." + region + ".amazonaws.com/")
.addHeader("Host", "ec2." + region + ".amazonaws.com")
.addFormParam("Action", "DescribeInstances")
.addFormParam("InstanceId.1", "i-2baa5550")
.addFormParam("InstanceId.2", "i-abcd1234").build());

describeInstanceMultiIdsResponse =
HttpResponse.builder().statusCode(200)
.payload(payloadFromResourceWithContentType(
"/describe_instances_multiple.xml", MediaType.APPLICATION_XML)).build();

//TODO: duplicate.. shouldn't need this
describeImageRequest =
formSigner.filter(HttpRequest.builder()
Expand Down
Loading

4 comments on commit c566418

@demobox
Copy link
Member

@demobox demobox commented on c566418 Apr 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This broke virtualbox in labs, for what it's worth.

We'll need to figure out a way to handle ripple changes like this: do we care about labs problems caused by changes in "main", or not? If so, how can we verify what we're breaking (or not)?

@nacx
Copy link
Member

@nacx nacx commented on c566418 Apr 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of failures also happen in jclouds-chef and karaf/cli. Currently we are relying on cloudbees email notifications, and a best effort to fix the branches as soon as possible. If there is a way to handle this better, it would be really helpful!

@codefromthecrypt
Copy link
Contributor

@codefromthecrypt codefromthecrypt commented on c566418 Apr 9, 2013 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@demobox
Copy link
Member

@demobox demobox commented on c566418 Apr 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zzzzzzzzz ;-) (peaceful sleep...)

Please sign in to comment.