updated cloudstack impl based on advice from citrix

This commit is contained in:
Adrian Cole 2011-11-28 00:25:14 -05:00
parent d31844469b
commit 933c9102ec
6 changed files with 63 additions and 43 deletions

View File

@ -225,31 +225,53 @@ public class CloudStackComputeServiceAdapter implements
@Override
public void destroyNode(String id) {
long virtualMachineId = Long.parseLong(id);
Builder<Long> jobsToTrack = ImmutableSet.<Long> builder();
Set<Long> ipAddresses = deleteIPForwardingRulesForVMAndReturnDistinctIPs(virtualMachineId, jobsToTrack);
disableStaticNATOnIPAddresses(jobsToTrack, ipAddresses);
destroyVirtualMachine(virtualMachineId, jobsToTrack);
ImmutableSet<Long> jobs = jobsToTrack.build();
logger.debug(">> awaiting completion of jobs(%s)", jobs);
for (long job : jobsToTrack.build())
awaitCompletion(job);
logger.trace("<< completed jobs(%s)", jobs);
// There was a bug in 2.2.12 release happening when static nat IP address
// was being released, and corresponding firewall rules were left behind.
// So next time the same IP is allocated, it might be not be static nat
// enabled, but there are still rules associated with it. And when you try
// to release this IP, the release will fail.
//
// The bug was fixed in 2.2.13 release only, and the current system wasn't
// updated yet.
//
// To avoid the issue, every time you release a static nat ip address, do
// the following:
// 1) Delete IP forwarding rules associated with IP.
Set<Long> ipAddresses = deleteIPForwardingRulesForVMAndReturnDistinctIPs(virtualMachineId);
// 2) Disable static nat rule for the IP.
disableStaticNATOnIPAddresses(ipAddresses);
// 3) Only after 1 and 2 release the IP address.
disassociateIPAddresses(ipAddresses);
destroyVirtualMachine(virtualMachineId);
vmToRules.invalidate(virtualMachineId);
}
public void destroyVirtualMachine(long virtualMachineId, Builder<Long> jobsToTrack) {
Long destroyVirtualMachine = client.getVirtualMachineClient().destroyVirtualMachine(virtualMachineId);
if (destroyVirtualMachine != null) {
logger.debug(">> destroying virtualMachine(%s) job(%s)", virtualMachineId, destroyVirtualMachine);
jobsToTrack.add(destroyVirtualMachine);
} else {
logger.trace("<< virtualMachine(%s) not found", virtualMachineId);
public void disassociateIPAddresses(Set<Long> ipAddresses) {
for (long ipAddress : ipAddresses) {
logger.debug(">> disassociating IPAddress(%s)", ipAddress);
client.getAddressClient().disassociateIPAddress(ipAddress);
}
}
public void disableStaticNATOnIPAddresses(Builder<Long> jobsToTrack, Set<Long> ipAddresses) {
public void destroyVirtualMachine(long virtualMachineId) {
Long destroyVirtualMachine = client.getVirtualMachineClient().destroyVirtualMachine(virtualMachineId);
if (destroyVirtualMachine != null) {
logger.debug(">> destroying virtualMachine(%s) job(%s)", virtualMachineId, destroyVirtualMachine);
awaitCompletion(destroyVirtualMachine);
} else {
logger.trace("<< virtualMachine(%s) not found", virtualMachineId);
}
}
public void disableStaticNATOnIPAddresses(Set<Long> ipAddresses) {
Builder<Long> jobsToTrack = ImmutableSet.<Long> builder();
for (Long ipAddress : ipAddresses) {
Long disableStaticNAT = client.getNATClient().disableStaticNATOnPublicIP(ipAddress);
if (disableStaticNAT != null) {
@ -257,9 +279,12 @@ public class CloudStackComputeServiceAdapter implements
jobsToTrack.add(disableStaticNAT);
}
}
awaitCompletion(jobsToTrack.build());
}
public Set<Long> deleteIPForwardingRulesForVMAndReturnDistinctIPs(long virtualMachineId, Builder<Long> jobsToTrack) {
public Set<Long> deleteIPForwardingRulesForVMAndReturnDistinctIPs(long virtualMachineId) {
Builder<Long> jobsToTrack = ImmutableSet.<Long> builder();
// immutable doesn't permit duplicates
Set<Long> ipAddresses = Sets.newLinkedHashSet();
@ -275,9 +300,17 @@ public class CloudStackComputeServiceAdapter implements
}
}
}
awaitCompletion(jobsToTrack.build());
return ipAddresses;
}
public void awaitCompletion(Iterable<Long> jobs) {
logger.debug(">> awaiting completion of jobs(%s)", jobs);
for (long job : jobs)
awaitCompletion(job);
logger.trace("<< completed jobs(%s)", jobs);
}
public void awaitCompletion(long job) {
boolean completed = jobComplete.apply(job);
logger.trace("<< job(%s) complete(%s)", job, completed);

View File

@ -112,9 +112,8 @@ public interface NATAsyncClient {
*/
@GET
@QueryParams(keys = "command", values = "enableStaticNat")
@Unwrap
@Consumes(MediaType.APPLICATION_JSON)
ListenableFuture<AsyncCreateResponse> enableStaticNATForVirtualMachine(
ListenableFuture<Void> enableStaticNATForVirtualMachine(
@QueryParam("virtualmachineid") long virtualMachineId, @QueryParam("ipaddressid") long IPAddressId);
/**

View File

@ -99,7 +99,8 @@ public interface NATClient {
*/
Long deleteIPForwardingRule(long id);
AsyncCreateResponse enableStaticNATForVirtualMachine(long virtualMachineId, long IPAddressId);
@Timeout(duration = 120, timeUnit = TimeUnit.SECONDS)
void enableStaticNATForVirtualMachine(long virtualMachineId, long IPAddressId);
/**
* Disables static rule for given ip address

View File

@ -26,11 +26,9 @@ import javax.inject.Named;
import javax.inject.Singleton;
import org.jclouds.cloudstack.CloudStackClient;
import org.jclouds.cloudstack.domain.AsyncCreateResponse;
import org.jclouds.cloudstack.domain.Network;
import org.jclouds.cloudstack.domain.PublicIPAddress;
import org.jclouds.cloudstack.domain.VirtualMachine;
import org.jclouds.cloudstack.strategy.BlockUntilJobCompletesAndReturnResult;
import org.jclouds.compute.reference.ComputeServiceConstants;
import org.jclouds.logging.Logger;
@ -52,17 +50,13 @@ public class StaticNATVirtualMachineInNetwork implements Function<VirtualMachine
protected Logger logger = Logger.NULL;
private final CloudStackClient client;
private final BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult;
private final ReuseOrAssociateNewPublicIPAddress reuseOrAssociate;
private final Network network;
@Inject
public StaticNATVirtualMachineInNetwork(CloudStackClient client,
BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult,
ReuseOrAssociateNewPublicIPAddress reuseOrAssociate, @Assisted Network network) {
this.client = checkNotNull(client, "client");
this.blockUntilJobCompletesAndReturnResult = checkNotNull(blockUntilJobCompletesAndReturnResult,
"blockUntilJobCompletesAndReturnResult");
this.reuseOrAssociate = checkNotNull(reuseOrAssociate, "reuseOrAssociate");
this.network = checkNotNull(network, "network");
}
@ -75,16 +69,9 @@ public class StaticNATVirtualMachineInNetwork implements Function<VirtualMachine
if (ip.getVirtualMachineId() > 0 && ip.getVirtualMachineId() != vm.getId())
continue;
try {
AsyncCreateResponse response = client.getNATClient().enableStaticNATForVirtualMachine(vm.getId(),
ip.getId());
logger.debug(">> static NATing IPAddress(%s) to virtualMachine(%s); response(%s)", ip.getId(), vm.getId(),
response);
// TODO: asked citrix for clarity as to whether this is supposed to
// return an async job or not; awaiting their response.
if (AsyncCreateResponse.UNINITIALIZED.equals(response))
ip = client.getAddressClient().getPublicIPAddress(ip.getId());
else
ip = blockUntilJobCompletesAndReturnResult.<PublicIPAddress> apply(response);
logger.debug(">> static NATing IPAddress(%s) to virtualMachine(%s)", ip.getId(), vm.getId());
client.getNATClient().enableStaticNATForVirtualMachine(vm.getId(), ip.getId());
ip = client.getAddressClient().getPublicIPAddress(ip.getId());
if (ip.isStaticNAT() && ip.getVirtualMachineId() == vm.getId())
break;
} catch (IllegalStateException e) {

View File

@ -25,6 +25,7 @@ import org.jclouds.cloudstack.options.CreateIPForwardingRuleOptions;
import org.jclouds.cloudstack.options.ListIPForwardingRulesOptions;
import org.jclouds.http.HttpRequest;
import org.jclouds.http.functions.ParseFirstJsonValueNamed;
import org.jclouds.http.functions.ReleasePayloadAndReturn;
import org.jclouds.http.functions.UnwrapOnlyJsonValue;
import org.jclouds.rest.functions.MapHttp4xxCodesToExceptions;
import org.jclouds.rest.functions.ReturnEmptySetOnNotFoundOr404;
@ -144,7 +145,7 @@ public class NATAsyncClientTest extends BaseCloudStackAsyncClientTest<NATAsyncCl
assertNonPayloadHeadersEqual(httpRequest, "Accept: application/json\n");
assertPayloadEquals(httpRequest, null, null, false);
assertResponseParserClassEquals(method, httpRequest, UnwrapOnlyJsonValue.class);
assertResponseParserClassEquals(method, httpRequest, ReleasePayloadAndReturn.class);
assertSaxResponseParserClassEquals(method, null);
assertExceptionParserClassEquals(method, MapHttp4xxCodesToExceptions.class);

View File

@ -83,8 +83,7 @@ public class StaticNATVirtualMachineInNetworkLiveTest extends NATClientLiveTest
if (networksDisabled)
return;
BlockUntilJobCompletesAndReturnResult blocker = new BlockUntilJobCompletesAndReturnResult(client, jobComplete);
StaticNATVirtualMachineInNetwork fn = new StaticNATVirtualMachineInNetwork(client, blocker, reuseOrAssociate,
network);
StaticNATVirtualMachineInNetwork fn = new StaticNATVirtualMachineInNetwork(client, reuseOrAssociate, network);
CreatePortForwardingRulesForIP createPortForwardingRulesForIP = new CreatePortForwardingRulesForIP(client,
blocker, CacheBuilder.newBuilder().<Long, Set<IPForwardingRule>> build(
new GetIPForwardingRulesByVirtualMachine(client)));
@ -97,7 +96,7 @@ public class StaticNATVirtualMachineInNetworkLiveTest extends NATClientLiveTest
ip = fn.apply(vm);
createPortForwardingRulesForIP.apply(ip, ImmutableSet.of(22));
rule = getOnlyElement(filter(client.getNATClient().getIPForwardingRulesForIPAddress(ip.getId()),
new Predicate<IPForwardingRule>() {
@Override