You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrea Turli <no...@github.com> on 2015/09/22 16:59:43 UTC
[jclouds] Fix openstack nova when floating ips are not available
(#860)
- improve destroyNode that could remove keypair and/or securityGroup if required
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/860
-- Commit Summary --
* improve roll-back strategy when floating-ip are not available
-- File Changes --
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java (30)
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNode.java (12)
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/handlers/NovaErrorHandler.java (9)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/860.patch
https://github.com/jclouds/jclouds/pull/860.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Zack Shoylev <no...@github.com>.
> @@ -97,6 +97,13 @@ else if (message.indexOf("already exists") != -1)
> exception = new ResourceNotFoundException(message, exception);
> }
> break;
> + case 500:
Is this a different fix or part of the same fix?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860/files#r40214877
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Ignasi Barrera <no...@github.com>.
> import com.google.common.util.concurrent.ListeningExecutorService;
>
> @Singleton
> public class NovaComputeService extends BaseComputeService {
> - protected final NovaApi novaApi;
> - protected final LoadingCache<RegionAndName, SecurityGroupInRegion> securityGroupMap;
> - protected final LoadingCache<RegionAndName, KeyPair> keyPairCache;
> - protected final Function<Set<? extends NodeMetadata>, Multimap<String, String>> orphanedGroupsByRegionId;
> - protected final GroupNamingConvention.Factory namingConvention;
> + protected CleanupServer cleanupServer;
Make it final.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860/files#r40970865
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Ignasi Barrera <no...@github.com>.
The test failures are real test failures:
https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/2053/org.apache.jclouds.api$openstack-nova/testReport/
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#issuecomment-142526890
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Andrea Turli <no...@github.com>.
Closed #860.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#event-522786139
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Andrea Turli <no...@github.com>.
Merged at [master](http://git-wip-us.apache.org/repos/asf/jclouds/commit/a4a255fa)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#issuecomment-173874399
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Zack Shoylev <no...@github.com>.
> @@ -97,6 +97,13 @@ else if (message.indexOf("already exists") != -1)
> exception = new ResourceNotFoundException(message, exception);
> }
> break;
> + case 500:
Can you add this comment to the code. It seems to me like somewhat inconsistent openstack behavior.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860/files#r40219793
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Zack Shoylev <no...@github.com>.
Looks like good cleanup to me overall
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#issuecomment-142633794
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -81,14 +82,16 @@ public AllocateAndAddFloatingIpToNode(@Named(TIMEOUT_NODE_RUNNING) Predicate<Ato
>
> Optional<FloatingIP> ip = allocateFloatingIPForNode(floatingIpApi, poolNames, node.getId());
> if (!ip.isPresent()) {
> - throw new InsufficientResourcesException("Failed to allocate a FloatingIP for node(" + node.getId() + ")");
> + RegionAndId regionAndId = RegionAndId.fromSlashEncoded(node.getId());
> + novaApi.getServerApi(regionAndId.getRegion()).delete(regionAndId.getId());
Should we do here the same security group and key pair cleanup?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860/files#r40178912
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Andrea Turli <no...@github.com>.
thanks @zack-shoylev, I've added the comment to justify the case 500
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#issuecomment-142650280
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Andrea Turli <no...@github.com>.
@nacx still good for merging?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#issuecomment-173717563
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Ignasi Barrera <no...@github.com>.
Just a minor comment. lgtm @andreaturli. Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#issuecomment-144860760
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Ignasi Barrera <no...@github.com>.
Go ahead!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#issuecomment-173735802
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Zack Shoylev <no...@github.com>.
Reopened #860.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#event-416164317
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Andrea Turli <no...@github.com>.
> @@ -97,6 +97,13 @@ else if (message.indexOf("already exists") != -1)
> exception = new ResourceNotFoundException(message, exception);
> }
> break;
> + case 500:
it is part of the same fix, when floating IP are finished, openstack returns 500 instead of https://github.com/andreaturli/jclouds/blob/fix/openstack-nova-floating-ips/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/handlers/NovaErrorHandler.java#L84 so I added that case to trigger the rollback cleanly.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860/files#r40219422
Re: [jclouds] Fix openstack nova when floating ips are not available
(#860)
Posted by Zack Shoylev <no...@github.com>.
Closed #860.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/860#event-416164244