From af4bd3a86b964c53e31a568221cc5ad08da35615 Mon Sep 17 00:00:00 2001 From: Andrea Turli Date: Mon, 28 Aug 2017 15:35:10 +0200 Subject: [PATCH] [JCLOUDS-1332] destroyNode and destroyNodesMatchingPredicate different semantic - modify BaseComputeService to make the 2 operations more similar - remove overridden destroyNode and destroyNodesMatching from GoogleComputeEngineService --- .../compute/internal/BaseComputeService.java | 24 ++---- .../compute/GoogleComputeEngineService.java | 29 ------- .../GoogleComputeEngineServiceMockTest.java | 7 +- .../src/test/resources/logback-test.xml | 42 ++++++++++ .../src/test/resources/logback.xml | 83 ------------------- 5 files changed, 50 insertions(+), 135 deletions(-) create mode 100644 providers/google-compute-engine/src/test/resources/logback-test.xml delete mode 100644 providers/google-compute-engine/src/test/resources/logback.xml diff --git a/compute/src/main/java/org/jclouds/compute/internal/BaseComputeService.java b/compute/src/main/java/org/jclouds/compute/internal/BaseComputeService.java index 8dc5c518a9..b60592b3f8 100644 --- a/compute/src/main/java/org/jclouds/compute/internal/BaseComputeService.java +++ b/compute/src/main/java/org/jclouds/compute/internal/BaseComputeService.java @@ -23,7 +23,6 @@ import static com.google.common.base.Throwables.propagate; import static com.google.common.collect.Iterables.filter; import static com.google.common.collect.Maps.newLinkedHashMap; import static com.google.common.collect.Sets.newLinkedHashSet; -import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_RUNNING; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_SUSPENDED; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_TERMINATED; @@ -32,7 +31,6 @@ import static org.jclouds.compute.predicates.NodePredicates.all; import static org.jclouds.compute.util.ComputeServiceUtils.formatStatus; import static org.jclouds.concurrent.FutureIterables.awaitCompletion; import static org.jclouds.concurrent.FutureIterables.transformParallel; -import static org.jclouds.util.Predicates2.retry; import java.util.Map; import java.util.NoSuchElementException; @@ -291,26 +289,14 @@ public class BaseComputeService implements ComputeService { protected NodeMetadata doDestroyNode(final String id) { checkNotNull(id, "id"); logger.debug(">> destroying node(%s)", id); - final AtomicReference node = Atomics.newReference(); - Predicate tester = retry(new Predicate() { - public boolean apply(String input) { - try { - NodeMetadata md = destroyNodeStrategy.destroyNode(id); - if (md != null) - node.set(md); - return true; - } catch (IllegalStateException e) { - logger.warn("<< illegal state destroying node(%s)", id); - return false; - } - } - }, timeouts.nodeTerminated, 1000, MILLISECONDS); - - boolean successful = tester.apply(id) && (node.get() == null || nodeTerminated.apply(node)); + NodeMetadata nodeMetadata = destroyNodeStrategy.destroyNode(id); + if (nodeMetadata == null) return null; + final AtomicReference node = Atomics.newReference(nodeMetadata); + boolean successful = node.get() == null || nodeTerminated.apply(node); if (successful) credentialStore.remove("node#" + id); logger.debug("<< destroyed node(%s) success(%s)", id, successful); - return node.get(); + return nodeMetadata; } protected void cleanUpIncidentalResourcesOfDeadNodes(Set deadNodes) { diff --git a/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineService.java b/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineService.java index acf456a9dd..3a05f4f14b 100644 --- a/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineService.java +++ b/providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineService.java @@ -16,12 +16,9 @@ */ package org.jclouds.googlecomputeengine.compute; -import static com.google.common.collect.Iterables.filter; -import static com.google.common.collect.Sets.newHashSet; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_RUNNING; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_SUSPENDED; import static org.jclouds.compute.config.ComputeServiceProperties.TIMEOUT_NODE_TERMINATED; -import static org.jclouds.compute.predicates.NodePredicates.all; import static org.jclouds.googlecloud.internal.ListPages.concat; import java.util.Map; @@ -69,7 +66,6 @@ import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.base.Supplier; -import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.Atomics; import com.google.common.util.concurrent.ListeningExecutorService; @@ -124,31 +120,6 @@ public final class GoogleComputeEngineService extends BaseComputeService { this.operationDone = operationDone; } - @Override - public void destroyNode(String id) { - // GCE does not return TERMINATED nodes, so in practice no node will never reach the TERMINATED - // state, and the deleted nodes will never be returned. - // In order to be able to clean up the resources associated to the deleted nodes, we have to retrieve - // the details of the nodes before deleting them. - NodeMetadata node = getNodeMetadata(id); - super.destroyNode(id); - cleanUpIncidentalResourcesOfDeadNodes(ImmutableSet.of(node)); - } - - @Override - public Set destroyNodesMatching(Predicate filter) { - // GCE does not return TERMINATED nodes, so in practice no node will never reach the TERMINATED - // state, and the deleted nodes will never be returned. - // In order to be able to clean up the resources associated to the deleted nodes, we have to retrieve - // the details of the nodes before deleting them. - Set nodes = newHashSet(filter(listNodesDetailsMatching(all()), filter)); - super.destroyNodesMatching(filter); // This returns an empty list (a list of null elements) in GCE, as the api does not return deleted nodes - cleanUpIncidentalResourcesOfDeadNodes(nodes); - return nodes; - } - - - @Override protected synchronized void cleanUpIncidentalResourcesOfDeadNodes(Set deadNodes) { Set orphanedGroups = findOrphanedGroups.apply(deadNodes); diff --git a/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceMockTest.java b/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceMockTest.java index 38c6a4d250..e5216e52da 100644 --- a/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceMockTest.java +++ b/providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceMockTest.java @@ -25,8 +25,6 @@ import static org.testng.Assert.assertTrue; import java.io.IOException; import java.util.Set; -import com.google.common.collect.ImmutableSet; -import com.squareup.okhttp.mockwebserver.MockResponse; import org.jclouds.compute.ComputeService; import org.jclouds.compute.domain.ComputeMetadata; import org.jclouds.compute.domain.Hardware; @@ -39,6 +37,9 @@ import org.jclouds.googlecomputeengine.domain.Instance; import org.jclouds.googlecomputeengine.internal.BaseGoogleComputeEngineApiMockTest; import org.testng.annotations.Test; +import com.google.common.collect.ImmutableSet; +import com.squareup.okhttp.mockwebserver.MockResponse; + @Test(groups = "unit", testName = "GoogleComputeEngineServiceMockTest", singleThreaded = true) public class GoogleComputeEngineServiceMockTest extends BaseGoogleComputeEngineApiMockTest { @@ -76,7 +77,6 @@ public class GoogleComputeEngineServiceMockTest extends BaseGoogleComputeEngineA server.enqueue(jsonResponse("/disk_get_with_source_image.json")); server.enqueue(jsonResponse("/image_get_for_source_image.json")); server.enqueue(jsonResponse("/aggregated_machinetype_list.json")); // Why are we getting machineTypes to delete an instance? - server.enqueue(instanceWithNetworkAndStatus("test-delete-1", "default", RUNNING)); server.enqueue(jsonResponse("/operation.json")); // instance delete server.enqueue(jsonResponse("/zone_operation.json")); server.enqueue(response404()); // deleted instance no longer exists @@ -93,7 +93,6 @@ public class GoogleComputeEngineServiceMockTest extends BaseGoogleComputeEngineA assertSent(server, "GET", "/projects/party/zones/us-central1-a/disks/test"); assertSent(server, "GET", "/projects/debian-cloud/global/images/debian-7-wheezy-v20140718"); assertSent(server, "GET", "/projects/party/aggregated/machineTypes"); // Why are we getting machineTypes to delete an instance? - assertSent(server, "GET", "/jclouds/zones/us-central1-a/instances/test-delete-1"); assertSent(server, "DELETE", "/jclouds/zones/us-central1-a/instances/test-delete-1"); // instance delete assertSent(server, "GET", "/projects/party/zones/us-central1-a/operations/operation-1354084865060"); assertSent(server, "GET", "/projects/party/zones/us-central1-a/instances/test-delete-1"); // get instance diff --git a/providers/google-compute-engine/src/test/resources/logback-test.xml b/providers/google-compute-engine/src/test/resources/logback-test.xml new file mode 100644 index 0000000000..cb55d49823 --- /dev/null +++ b/providers/google-compute-engine/src/test/resources/logback-test.xml @@ -0,0 +1,42 @@ + + + + target/test-data/jclouds.log + + %d %-5p [%c] [%thread] %m%n + + + + target/test-data/jclouds-wire.log + + %d %-5p [%c] [%thread] %m%n + + + + target/jclouds-compute.log + + %d %-5p [%c] [%thread] %m%n + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/providers/google-compute-engine/src/test/resources/logback.xml b/providers/google-compute-engine/src/test/resources/logback.xml deleted file mode 100644 index c83c449005..0000000000 --- a/providers/google-compute-engine/src/test/resources/logback.xml +++ /dev/null @@ -1,83 +0,0 @@ - - - - - target/test-data/jclouds.log - - - %d %-5p [%c] [%thread] %m%n - - - - - target/test-data/jclouds-wire.log - - - %d %-5p [%c] [%thread] %m%n - - - - - target/test-data/jclouds-compute.log - - - %d %-5p [%c] [%thread] %m%n - - - - - target/test-data/jclouds-ssh.log - - - %d %-5p [%c] [%thread] %m%n - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -