[JCLOUDS-1332] destroyNode and destroyNodesMatchingPredicate different semantic

- modify BaseComputeService to make the 2 operations more similar
- remove overridden destroyNode and destroyNodesMatching from GoogleComputeEngineService
This commit is contained in:
Andrea Turli 2017-08-28 15:35:10 +02:00
parent 1937e200e3
commit af4bd3a86b
5 changed files with 50 additions and 135 deletions

View File

@ -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<NodeMetadata> node = Atomics.newReference();
Predicate<String> tester = retry(new Predicate<String>() {
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<NodeMetadata> 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<? extends NodeMetadata> deadNodes) {

View File

@ -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<? extends NodeMetadata> destroyNodesMatching(Predicate<? super NodeMetadata> 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<? extends NodeMetadata> 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<? extends NodeMetadata> deadNodes) {
Set<String> orphanedGroups = findOrphanedGroups.apply(deadNodes);

View File

@ -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

View File

@ -0,0 +1,42 @@
<?xml version="1.0"?>
<configuration scan="false">
<appender name="FILE" class="ch.qos.logback.core.FileAppender">
<file>target/test-data/jclouds.log</file>
<encoder>
<Pattern>%d %-5p [%c] [%thread] %m%n</Pattern>
</encoder>
</appender>
<appender name="WIREFILE" class="ch.qos.logback.core.FileAppender">
<file>target/test-data/jclouds-wire.log</file>
<encoder>
<Pattern>%d %-5p [%c] [%thread] %m%n</Pattern>
</encoder>
</appender>
<appender name="COMPUTEFILE" class="ch.qos.logback.core.FileAppender">
<file>target/jclouds-compute.log</file>
<encoder>
<Pattern>%d %-5p [%c] [%thread] %m%n</Pattern>
</encoder>
</appender>
<logger name="org.jclouds">
<level value="DEBUG" />
<appender-ref ref="FILE" />
</logger>
<logger name="jclouds.compute">
<level value="DEBUG" />
<appender-ref ref="COMPUTEFILE" />
</logger>
<logger name="jclouds.wire">
<level value="DEBUG" />
<appender-ref ref="WIREFILE" />
</logger>
<logger name="jclouds.headers">
<level value="DEBUG" />
<appender-ref ref="WIREFILE" />
</logger>
<root>
<level value="INFO" />
</root>
</configuration>

View File

@ -1,83 +0,0 @@
<?xml version="1.0"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF 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.
-->
<configuration scan="false">
<appender name="FILE" class="ch.qos.logback.core.FileAppender">
<file>target/test-data/jclouds.log</file>
<encoder>
<Pattern>%d %-5p [%c] [%thread] %m%n</Pattern>
</encoder>
</appender>
<appender name="WIREFILE" class="ch.qos.logback.core.FileAppender">
<file>target/test-data/jclouds-wire.log</file>
<encoder>
<Pattern>%d %-5p [%c] [%thread] %m%n</Pattern>
</encoder>
</appender>
<appender name="COMPUTEFILE" class="ch.qos.logback.core.FileAppender">
<file>target/test-data/jclouds-compute.log</file>
<encoder>
<Pattern>%d %-5p [%c] [%thread] %m%n</Pattern>
</encoder>
</appender>
<appender name="SSHFILE" class="ch.qos.logback.core.FileAppender">
<file>target/test-data/jclouds-ssh.log</file>
<encoder>
<Pattern>%d %-5p [%c] [%thread] %m%n</Pattern>
</encoder>
</appender>
<root>
<level value="warn" />
</root>
<logger name="org.jclouds">
<level value="TRACE" />
<appender-ref ref="FILE" />
</logger>
<logger name="jclouds.wire">
<level value="TRACE" />
<appender-ref ref="WIREFILE" />
</logger>
<logger name="jclouds.headers">
<level value="TRACE" />
<appender-ref ref="WIREFILE" />
</logger>
<logger name="jclouds.compute">
<level value="TRACE" />
<appender-ref ref="COMPUTEFILE" />
</logger>
<logger name="jclouds.ssh">
<level value="TRACE" />
<appender-ref ref="SSHFILE" />
</logger>
</configuration>