You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Kris Sterckx <no...@github.com> on 2013/10/23 09:20:30 UTC
[jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Hello, I've added some Neutron v2.0 Extensions :
* Router
* FloatingIP
* LBaaS
- Pool
- PoolMember
- VIP
- HealthMonitor
I've run the live tests against a DevStack setup which is using the Grizzly version of OpenStack.
Find below the contents of my localrc file :
ADMIN_PASSWORD=password
MYSQL_PASSWORD=password
RABBIT_PASSWORD=password
SERVICE_PASSWORD=password
SERVICE_TOKEN=token
disable_service n-net
enable_service q-svc
enable_service q-agt
enable_service q-dhcp
enable_service q-meta
enable_service quantum
enable_service q-lbaas
OVS_ENABLE_TUNNELING=False
NOVA_BRANCH=stable/grizzly
CINDER_BRANCH=stable/grizzly
GLANCE_BRANCH=stable/grizzly
HORIZON_BRANCH=stable/grizzly
KEYSTONE_BRANCH=stable/grizzly
NEUTRON_BRANCH=stable/grizzly
LOGFILE=/opt/stack/logs/stack.sh.log
VERBOSE=True
LOG_COLOR=False
SCREEN_LOGDIR=/opt/stack/logs
OVS_VLAN_RANGES=RegionOne:1:4000
To remove all resources, run the following commands :
neutron lb-vip-list | awk '{print $2}' | xargs -I{} neutron lb-vip-delete "{}"
neutron lb-healthmonitor-list | awk '{print $2}' | xargs -I{} neutron lb-healthmonitor-delete "{}"
neutron lb-member-list | awk '{print $2}' | xargs -I{} neutron lb-member-delete "{}"
neutron lb-pool-list | awk '{print $2}' | xargs -I{} neutron lb-pool-delete "{}"
neutron floatingip-list | awk '{print $2}' | xargs -I{} neutron floatingip-delete "{}"
neutron router-list | awk '{print $2}' | xargs -I{} neutron router-delete "{}"
neutron port-list | awk '{print $2}' | xargs -I{} neutron port-delete "{}"
neutron subnet-list | awk '{print $2}' | xargs -I{} neutron subnet-delete "{}"
neutron net-list | awk '{print $2}' | xargs -I{} neutron net-delete "{}"
You can merge this Pull Request by running:
git pull https://github.com/KrisSterckx/jclouds-labs-openstack os-neutron-extensions
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-openstack/pull/44
-- Commit Summary --
* OS Neutron v2.0 Extensions
-- File Changes --
M openstack-neutron/pom.xml (1)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/NeutronApi.java (43)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/EmptyObject.java (32)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/ExternalGatewayInfo.java (105)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/FloatingIP.java (190)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/HealthMonitor.java (294)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/Member.java (214)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/Pool.java (315)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/Router.java (151)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/RouterInterface.java (127)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/SessionPersistence.java (141)
M openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/State.java (2)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/VIP.java (336)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/extensions/FloatingIPApi.java (157)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/extensions/HealthMonitorApi.java (161)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/extensions/MemberApi.java (159)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/extensions/PoolApi.java (186)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/extensions/RouterApi.java (209)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/extensions/VIPApi.java (162)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseFloatingIPDetails.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseFloatingIPs.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseHealthMonitorDetails.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseHealthMonitors.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseMemberDetails.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseMembers.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParsePoolDetails.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParsePools.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseRouterDetails.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseRouters.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseVIPDetails.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/functions/ParseVIPs.java (93)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/CreateFloatingIPOptions.java (140)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/CreateHealthMonitorOptions.java (198)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/CreateMemberOptions.java (148)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/CreatePoolOptions.java (198)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/CreateRouterOptions.java (168)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/CreateVIPOptions.java (246)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/EmptyOptions.java (48)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/UpdateFloatingIPOptions.java (132)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/UpdateHealthMonitorOptions.java (247)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/UpdateMemberOptions.java (156)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/UpdatePoolOptions.java (196)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/UpdateRouterOptions.java (168)
A openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/options/UpdateVIPOptions.java (228)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/FloatingIPExpectTest.java (198)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/FloatingIPLiveTest.java (154)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/HealthMonitorApiExpectTest.java (208)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/HealthMonitorApiLiveTest.java (124)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/MemberApiExpectTest.java (197)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/MemberApiLiveTest.java (112)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/PoolApiExpectTest.java (256)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/PoolApiLiveTest.java (103)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/RouterApiExpectTest.java (295)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/RouterApiLiveTest.java (192)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/VIPApiExpectTest.java (209)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/extensions/VIPApiLiveTest.java (111)
M openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/features/NetworkApiLiveTest.java (8)
M openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/features/PortApiLiveTest.java (2)
M openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/features/SubnetApiLiveTest.java (2)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/parse/ParseFloatingIPTest.java (52)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/parse/ParseHealthMonitorTest.java (54)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/parse/ParseMemberTest.java (57)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/parse/ParsePoolTest.java (55)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/parse/ParseRouterTest.java (55)
A openstack-neutron/src/test/java/org/jclouds/openstack/neutron/v2_0/parse/ParseVIPTest.java (64)
A openstack-neutron/src/test/resources/floating_ip.json (6)
A openstack-neutron/src/test/resources/health_monitor.json (8)
A openstack-neutron/src/test/resources/list_floating_ips.json (74)
A openstack-neutron/src/test/resources/list_health_monitors.json (98)
A openstack-neutron/src/test/resources/list_members.json (86)
A openstack-neutron/src/test/resources/list_pools.json (98)
A openstack-neutron/src/test/resources/list_routers.json (74)
A openstack-neutron/src/test/resources/list_vips.json (110)
A openstack-neutron/src/test/resources/member.json (10)
A openstack-neutron/src/test/resources/pool.json (8)
A openstack-neutron/src/test/resources/router.json (7)
A openstack-neutron/src/test/resources/vip.json (15)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-openstack/pull/44.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/44.diff
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2_0.domain;
> +
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Information on external gateway for the router
> + *
> + * @author Nick Livens
> + */
> +public class ExternalGatewayInfo {
> +
> + private final String networkId;
Then make the class final?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7293898
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> + * KIND, either express or implied. See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2_0.domain;
> +
> +/**
> + * This is used for empty responses
> + *
> + * @author Nick Livens
> + */
> +public class EmptyObject {
> +
> + public static void main(String[] args) {
> +
> + }
It was a return type for one method, but I'll just return Object instead of that EmptyObject then.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7304326
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Everett Toews <no...@github.com>.
Sweet. All live tests run 100% on Havana. Hopefully I'll get a chance to review the code sometime this week.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44#issuecomment-26963434
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2_0.domain;
> +
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Information on external gateway for the router
> + *
> + * @author Nick Livens
> + */
> +public class ExternalGatewayInfo {
> +
> + private final String networkId;
Will do this for the new classes and existing classes that are not meant to be subclassed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7305141
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
In general: this is quite a lot of code to review in one piece. Do all these extensions need to be committed together, or would it have been possible to have one PR for e.g. Router, onr for FloatingIP and one for LBaaS?
Just as a point for all of us to think about for the future. @everett-toews: would be interested to hear your thoughts on this one!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44#issuecomment-27153996
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> I could split this PR up in 3 different PRs if this is easier for you guys.
Perhaps for next time? But would want to hear from other reviewers whether they feel this would help first...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44#issuecomment-27341423
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @author Nick Livens
> + * @see <a href="http://docs.openstack.org/api/openstack-network/2.0/content/lbaas_ext_ops_health_monitor.html">api doc</a> *
> + */
> +public class HealthMonitor extends Reference {
> +
> + public static enum Type {
> + PING, TCP, HTTP, HTTPS;
> +
> + public static Type fromValue(String value) {
> + try {
> + return valueOf(value.toUpperCase());
> + } catch (IllegalArgumentException e) {
> + return null;
> + }
> + }
What do we need this for? Do we really expect invalid entries and, if so, is it wise to be hiding them?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7234799
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-openstack-pull-requests #82](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/82/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44#issuecomment-26885239
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> I'll split this up in 3 PRs as @demobox suggested :
Thanks, @KrisSterckx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44#issuecomment-27492984
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> @@ -126,6 +126,7 @@
> <test.openstack-neutron.credential>${test.openstack-neutron.credential}</test.openstack-neutron.credential>
> <test.jclouds.keystone.credential-type>${test.jclouds.keystone.credential-type}</test.jclouds.keystone.credential-type>
> </systemPropertyVariables>
> + <parallel>classes</parallel>
Ah, like that. Thanks for explaining! Do we know which tests are causing the problems, by the way?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7342869
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Information on external gateway for the router
> + *
> + * @author Nick Livens
> + */
> +public class ExternalGatewayInfo {
> +
> + private final String networkId;
> +
> + @ConstructorProperties({"network_id"})
> + protected ExternalGatewayInfo(String networkId) {
> + this.networkId = networkId;
Do we need null checks here and for similar non-nullable properties of the other domain objects, or are we assuming that we'll always be constructing these correctly from internal code?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7234789
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Information on external gateway for the router
> + *
> + * @author Nick Livens
> + */
> +public class ExternalGatewayInfo {
> +
> + private final String networkId;
> +
> + @ConstructorProperties({"network_id"})
> + protected ExternalGatewayInfo(String networkId) {
> + this.networkId = networkId;
I didn't add any null checks since the most of these properties can effectively be null. For those properties that are required (non-nullable), I didn't add a null check since they're always filled in, whether it's due to creating this object through the interface or creating this object through receiving a response from OpenStack.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7242452
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> @@ -126,6 +126,7 @@
> <test.openstack-neutron.credential>${test.openstack-neutron.credential}</test.openstack-neutron.credential>
> <test.jclouds.keystone.credential-type>${test.jclouds.keystone.credential-type}</test.jclouds.keystone.credential-type>
> </systemPropertyVariables>
> + <parallel>classes</parallel>
I've added this to make sure the live tests run sequential instead of parallel. Running these live tests parallel can cause some issues sometimes due to different tests creating / listing the same entity at the same time.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7242230
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> @@ -126,6 +126,7 @@
> <test.openstack-neutron.credential>${test.openstack-neutron.credential}</test.openstack-neutron.credential>
> <test.jclouds.keystone.credential-type>${test.jclouds.keystone.credential-type}</test.jclouds.keystone.credential-type>
> </systemPropertyVariables>
> + <parallel>classes</parallel>
> I've added this to make sure the live tests run sequential instead of parallel.
I think elsewhere we do this by adding `singleThreaded = true` to the `@Test` annotation in the class - could you do that here too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7293802
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Everett Toews <no...@github.com>.
@KrisSterckx My apologies but I leave for Hong Kong tomorrow and I won't be able to review this personally. I see @demobox got a start on it though.
And he's right. This is a lot of code to review at once. Splitting it up into 3 PRs would definitely help. I should have suggested that as soon as I saw this PR. It's *my* mistake.
Maybe @zack-shoylev can chip in on the review of the new PRs too.
BTW, you'll need to rebase both jclouds/jclouds and jclouds/jclouds-labs-openstack before submitting the new PRs. There were some changes to the pagination stuff. I don't think it will cause any conflicts for you.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44#issuecomment-27361442
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2_0.domain;
> +
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Information on external gateway for the router
> + *
> + * @author Nick Livens
> + */
> +public class ExternalGatewayInfo {
> +
> + private final String networkId;
Exactly, these are classes that are not meant to be subclassed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7242167
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> + * Provides synchronous access to LBaaS Member features
> + */
> + @Delegate
> + Optional<? extends MemberApi> getMemberExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> + /**
> + * Provides synchronous access to LBaaS HealthMonitor features
> + */
> + @Delegate
> + Optional<? extends HealthMonitorApi> getHealthMonitorExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> + /**
> + * Provides synchronous access to LBaaS VIP features
> + */
> + @Delegate
> + Optional<? extends VIPApi> getVIPExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
Looking at the documentation of rackspace regarding these, I can say that they're talking about the same concepts.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7242299
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> + * Provides synchronous access to LBaaS Member features
> + */
> + @Delegate
> + Optional<? extends MemberApi> getMemberExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> + /**
> + * Provides synchronous access to LBaaS HealthMonitor features
> + */
> + @Delegate
> + Optional<? extends HealthMonitorApi> getHealthMonitorExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> + /**
> + * Provides synchronous access to LBaaS VIP features
> + */
> + @Delegate
> + Optional<? extends VIPApi> getVIPExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
If so, should they both be named the same (not "VIP" vs. "VirtualIP")?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7293819
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
I'll split this up in 3 PRs as @demobox suggested :
* Router
* FloatingIP
* LBaaS
I'll fix the given comments in those PRs too.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44#issuecomment-27370387
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> + * Provides synchronous access to LBaaS Member features
> + */
> + @Delegate
> + Optional<? extends MemberApi> getMemberExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> + /**
> + * Provides synchronous access to LBaaS HealthMonitor features
> + */
> + @Delegate
> + Optional<? extends HealthMonitorApi> getHealthMonitorExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> + /**
> + * Provides synchronous access to LBaaS VIP features
> + */
> + @Delegate
> + Optional<? extends VIPApi> getVIPExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
Out of curiosity: how, if at all, does this relate to [VirtualIPApi](https://github.com/jclouds/jclouds/blob/master/apis/rackspace-cloudloadbalancers/src/main/java/org/jclouds/rackspace/cloudloadbalancers/v1/features/VirtualIPApi.java) in cloudloadbalancers?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7234781
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> + * KIND, either express or implied. See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2_0.domain;
> +
> +/**
> + * This is used for empty responses
> + *
> + * @author Nick Livens
> + */
> +public class EmptyObject {
> +
> + public static void main(String[] args) {
> +
> + }
Forgot to remove the main method, I'll remove it. Where do you suggest we make it a singleton object?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7242188
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> + * KIND, either express or implied. See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2_0.domain;
> +
> +/**
> + * This is used for empty responses
> + *
> + * @author Nick Livens
> + */
> +public class EmptyObject {
> +
> + public static void main(String[] args) {
> +
> + }
Remove the main method? And could this be a singleton object elsewhere?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7234785
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> + *
> + * @author Nick Livens
> + * @see <a href="http://docs.openstack.org/api/openstack-network/2.0/content/lbaas_ext_ops_health_monitor.html">api doc</a> *
> + */
> +public class HealthMonitor extends Reference {
> +
> + public static enum Type {
> + PING, TCP, HTTP, HTTPS;
> +
> + public static Type fromValue(String value) {
> + try {
> + return valueOf(value.toUpperCase());
> + } catch (IllegalArgumentException e) {
> + return null;
> + }
> + }
I've done this almost the same way as in [NetworkType](https://github.com/jclouds/jclouds-labs-openstack/blob/master/openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2_0/domain/NetworkType.java)
Perhaps we should indeed remove these methods since we don't expect invalid entries.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7242835
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> + * KIND, either express or implied. See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2_0.domain;
> +
> +/**
> + * This is used for empty responses
> + *
> + * @author Nick Livens
> + */
> +public class EmptyObject {
> +
> + public static void main(String[] args) {
> +
> + }
If we only need it in one class, make a static final object?
```
public static final Object EMPTY_OBJECT = new Object();
```
?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7293856
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
@demobox: It could split this PR up in 3 different PRs if this is easier for you guys.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44#issuecomment-27197875
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> @@ -126,6 +126,7 @@
> <test.openstack-neutron.credential>${test.openstack-neutron.credential}</test.openstack-neutron.credential>
> <test.jclouds.keystone.credential-type>${test.jclouds.keystone.credential-type}</test.jclouds.keystone.credential-type>
> </systemPropertyVariables>
> + <parallel>classes</parallel>
Does this also work if the tests are run from an IDE? Just out of curiosity, because I recall some issues with TestNG in that regard...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7234767
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @author Nick Livens
> + * @see <a href="http://docs.openstack.org/api/openstack-network/2.0/content/lbaas_ext_ops_health_monitor.html">api doc</a> *
> + */
> +public class HealthMonitor extends Reference {
> +
> + public static enum Type {
> + PING, TCP, HTTP, HTTPS;
> +
> + public static Type fromValue(String value) {
> + try {
> + return valueOf(value.toUpperCase());
> + } catch (IllegalArgumentException e) {
> + return null;
> + }
> + }
> Perhaps we should indeed remove these methods since we don't expect invalid entries.
+1
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7293904
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Information on external gateway for the router
> + *
> + * @author Nick Livens
> + */
> +public class ExternalGatewayInfo {
> +
> + private final String networkId;
> +
> + @ConstructorProperties({"network_id"})
> + protected ExternalGatewayInfo(String networkId) {
> + this.networkId = networkId;
That seems sensible enough, but it differs from what we do elsewhere in OpenStack, I think, where null checks are indeed added even though (I guess) the values "should never" be null.
@everett-toews: thoughts on this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7293892
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> @@ -126,6 +126,7 @@
> <test.openstack-neutron.credential>${test.openstack-neutron.credential}</test.openstack-neutron.credential>
> <test.jclouds.keystone.credential-type>${test.jclouds.keystone.credential-type}</test.jclouds.keystone.credential-type>
> </systemPropertyVariables>
> + <parallel>classes</parallel>
Well, I've tried that, but the only thing that does is make the methods run sequential. What I need is to run all test classes sequential, which this attribute of the annotation does not do.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7304301
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Kris Sterckx <no...@github.com>.
> + * Provides synchronous access to LBaaS Member features
> + */
> + @Delegate
> + Optional<? extends MemberApi> getMemberExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> + /**
> + * Provides synchronous access to LBaaS HealthMonitor features
> + */
> + @Delegate
> + Optional<? extends HealthMonitorApi> getHealthMonitorExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
> +
> + /**
> + * Provides synchronous access to LBaaS VIP features
> + */
> + @Delegate
> + Optional<? extends VIPApi> getVIPExtensionForZone(@EndpointParam(parser = ZoneToEndpoint.class) @Nullable String zone);
I'll rename it to VirtualIP to be consequent with the other code
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7304304
Re: [jclouds-labs-openstack] OS Neutron v2.0 Extensions (#44)
Posted by Andrew Phillips <no...@github.com>.
> + * under the License.
> + */
> +package org.jclouds.openstack.neutron.v2_0.domain;
> +
> +import com.google.common.base.Objects;
> +
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Information on external gateway for the router
> + *
> + * @author Nick Livens
> + */
> +public class ExternalGatewayInfo {
> +
> + private final String networkId;
Here and for other domain objects: why not `protected`? Is this class never intended to be subclassed?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/44/files#r7234794