Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
@fbrouille Personally, I close them once the work is pushed. I just resolved the issue in JIRA, so not to worry. We really appreciate the contribution, thanks! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57810495
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
Who is supposed to close the issue in JIRA? (https://issues.apache.org/jira/browse/JCLOUDS-611) me or someone else? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57797789
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
Closed #146. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#event-173498545
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
Merged to master and backported to 1.8.x with compatibility changes for java 1.6 --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57703775
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
About to merge and backport --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57696129
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
sounds good, zach. I have noticed that folks are using jackson w/auto, so I think it is possible w/gson. Since we have our own instance creators, this may require work, but who knows.. may end up worth it. Will catch up later and yeah I agree labs providers shift anyway, so this sort of thing can be done in follow-up. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57678731
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
@adriancole there is also a thread about potentially using Optional instead of all the boxed types in this API on the dev list, if you want to chime in there? I am also very much in favor of any way to eliminate more boilerplate - but we will have to address that in separate PRs. Neutron is currently very much beta so we expect it to change. Some side notes: we should only have builders where users would use them, and yes, if there are any builders for say objects returned by the service, those would be removed. Autovalue: Let's talk about that some more on the dev list. I am unsure it will work with GSON (specifically serialization), but would like to test it some. Also glad to see you are back! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57678465
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
ps in case it doesn't go without saying, don't consider my commentary binding. I've been awol and don't expect I have full enough understanding of how things are. In other words, take my notes as advise, not anything blocking. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57663053
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
general commentary. I wouldn't want to use this api. It is laden with danger as everything is marked nullable, even booleans. If in the last couple years we've gotten away from making null-safe objects, forgive the late commentary, but yeah, this codebase is a NPE waiting to happen, and I'd have no idea which fields can be relied on at all. I'd suggest we start decrufting with google auto value as well, since many things here are laborious boilerplates. Moreover, in many cases builders are only used in tests. Using tools like assertj, we should be able to guarantee tests without builders. If there's no non-internal use case for constructing an object, we should probably spare the pain of writing builders everywhere. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57659517
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + // Optional attributes > + @Named("status_description") > + protected final String statusDescription; > + > + @ConstructorProperties({ "monitor_id", "status", "status_description" }) > + protected HealthMonitorStatus(String id, LBaaSStatus status, String > statusDescription) { > + this.id = id; > + this.status = status; > + this.statusDescription = statusDescription; > + } > + > + /** > +* @return the id of the HealthMonitorStatus. > +*/ > + @Nullable > + public String getId() { really? Are all of the contents including the id nullable? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18350860
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + > + /** > +* Copy constructor. > +* > +* @param member the Member to copy from. > +*/ > + private Member(Member member) { > + this(member.id, member.tenantId, member.poolId, member.address, > member.protocolPort, member.weight, > +member.adminStateUp, member.status, member.statusDescription); > + } > + > + /** > +* @return the id of the Member. > +*/ > + @Nullable > + public String getId() { hmm are we saying every part of this object is nullable? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18350877
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + return expectedCodes; > + } > + > + /** > +* @return the pools for this HealthMonitor. > +*/ > + @Nullable > + public ImmutableList getPools() { > + return pools; > + } > + > + /** > +* @return the administrative state for this HealthMonitor. > +*/ > + @Nullable > + public Boolean getAdminStateUp() { specs or no specs using Boolean or `Optional` is pretty awful experience. I'd try to find a default and leave primitive. isX gives most cognitive sense immediately, especially if no chance of NPE --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18350820
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
Alright anything else? If not, let's merge and backport. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57656568
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + /** > + * Provides the administrative state for this HealthMonitor's Builder. > + * > + * @return the Builder. > + * @see HealthMonitor#getAdminStateUp() > + */ > + public ParameterizedBuilderType adminStateUp(Boolean adminStateUp) { > + healthMonitor.adminStateUp = adminStateUp; > + return self(); > + } > + } > + > + /** > +* Create builder (inheriting from Builder). > +*/ > + public static class CreateBuilder extends Builder { Ok, in code they don't have a default value specified, so they are probably required. Thanks for clarifying! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18349831
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + /** > + * Provides the administrative state for this HealthMonitor's Builder. > + * > + * @return the Builder. > + * @see HealthMonitor#getAdminStateUp() > + */ > + public ParameterizedBuilderType adminStateUp(Boolean adminStateUp) { > + healthMonitor.adminStateUp = adminStateUp; > + return self(); > + } > + } > + > + /** > +* Create builder (inheriting from Builder). > +*/ > + public static class CreateBuilder extends Builder { Ok for a single constructor as it is now. For me, ProbeType type, Integer delay, Integer timeout, Integer maxRetries are required. See http://developer.openstack.org/api-ref-networking-v2.html and https://github.com/openstack/neutron/blob/master/neutron/extensions/loadbalancer.py --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18325968
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + /** > + * Provides the administrative state for this HealthMonitor's Builder. > + * > + * @return the Builder. > + * @see HealthMonitor#getAdminStateUp() > + */ > + public ParameterizedBuilderType adminStateUp(Boolean adminStateUp) { > + healthMonitor.adminStateUp = adminStateUp; > + return self(); > + } > + } > + > + /** > +* Create builder (inheriting from Builder). > +*/ > + public static class CreateBuilder extends Builder { Always use a single builder constructor. The constructor should only take in parameters that are required every time. All other attributes should be set by the setters. In this case it seems CreateBuilder takes in ProbeType type, Integer delay, Integer timeout, Integer maxRetries. Are all of these required when creating a HealthMonitor? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18289995
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
[jclouds-labs-openstack-pull-requests #440](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/440/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57445612
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
[jclouds » jclouds-labs-openstack #1762](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1762/) SUCCESS This pull request looks good [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57445508
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> Here is a somewhat different suggestion: keep v1 directory structures as they > are right now. When v2 > comes around, implement it without using lbaas.v2 in package and class names. > Then deprecate v1. Makes sense to me. Assuming we don't expect a `v3` to come along, that is ;-) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-57418757
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
Version in packages: > maybe it is preferable to simply ignore it and remove version in package > names and in classnames It is difficult to say. Here is something to consider: right now, neutron is marked as Beta, which means we can still modify the API and introduce breaking changes. However, eventually it will not be - so it makes sense to future-proof it. Here is a somewhat different suggestion: keep v1 directory structures as they are right now. When v2 comes around, implement it without using lbaas.v2 in package and class names. Then deprecate v1. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56851650
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> @@ -103,4 +104,14 @@ > */ > @Delegate > Optional getSecurityGroupApi(@EndpointParam(parser = > RegionToEndpoint.class) String region); > + > + /** > +* Provides access to LBaaS version 1 features. > +* > +* NOTE > +* This API is an extension that may or may not be present in your > OpenStack cloud. Use the Optional return type > +* to determine if it is present. > +*/ > + @Delegate > + Optional getLBaaSv1Api(@EndpointParam(parser = > RegionToEndpoint.class) String region); You are absolutely right! So I would rename method to Optional getLBaaSApi(@EndpointParam(parser = RegionToEndpoint.class) String region); --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18032497
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> @@ -103,4 +104,14 @@ > */ > @Delegate > Optional getSecurityGroupApi(@EndpointParam(parser = > RegionToEndpoint.class) String region); > + > + /** > +* Provides access to LBaaS version 1 features. > +* > +* NOTE > +* This API is an extension that may or may not be present in your > OpenStack cloud. Use the Optional return type > +* to determine if it is present. > +*/ > + @Delegate > + Optional getLBaaSv1Api(@EndpointParam(parser = > RegionToEndpoint.class) String region); The v2 support is on the [`feature/lbaasv2`](https://github.com/openstack/neutron/tree/feature/lbaasv2/neutron/extensions) branch in [`loadbalancersv2.py`](https://github.com/openstack/neutron/blob/feature/lbaasv2/neutron/extensions/loadbalancerv2.py) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18030800
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
Dear all, first of all, thanks for your review everybody! I have changed the PR to fix some issues you have pointed. Before merge, I think there is still some points to clarify: - Version in method name: Because in future, 2 versions of LBaaS API will coexist (v1 and v2), I simply decided to put the version in method name (without thinking too much): NeutronApi.getLBaaSv1Api (and NeutronApi.getLBaaSv2Api in another future PR for LBaaS v2). If the rule for extension APIs is to use the alias name in the method signature, I have to rename the method to NeutronApi.getLBaaSApi (and the method name for v2 will be NeutronApi.getLBaaSv2Api), as @jdaggett has suggested it. Let me know if I have to remove the version in method name. - Version in packages: For the same reason, I simply decide to put all classes about LBaaS in a 'v1' packages (domain, extensions, fallbacks, functions). I thought this was the simplest solution to prepare future integration of LBaaS v2. However, it seems more and more unlikely that LBaaS v2 will be part of the next release of OpenStack (Juno due date is october 16 and LBaaS improvements have not yet been reviewed). So maybe it is to early to talk about LBaaS v2 and maybe it is preferable to simply ignore it and remove version in package names and in classnames. What do you think? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56814435
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported Neutron LBaaS v1 resources status. > + */ > +public enum LBaaSStatus { > Question: At some point in the future, would it make sense to drop v1 and > just have v2 as the LBaaS? I read on some forums (I don't remember where) that LBaaS v1 API will be deprecated when LBaaS v2 will be released. Moreover LBaaS v1 will be removed in far future. I think that jclouds should deprecate the LBaaS v1 when LBaaS v2 is released, but not remove it in far future because someone might still need to use it. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18027504
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported Neutron LBaaS v1 resources status. > + */ > +public enum LBaaSStatus { Done --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18024252
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > + > +/** > + * Enumerates supported HTTP methods. > + */ > +public enum HttpMethod { > + GET("GET"), Done --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18024094
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + public String getName() { > + return name; > + } > + > + /** > +* @return the description of the Pool. > +*/ > + @Nullable > + public String getDescription() { > + return description; > + } > + > + /** > +* @return the subnet id for this Pool. > +*/ > + public String getSubnetId() { Done --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18024058
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/health_monitors/{id}") > + @Fallback(FalseOnNotFoundOr404.class) > + boolean deleteHealthMonitor(@PathParam("id") String id); > + > + /** > +* Associate a HealthMonitor to a Pool. > +* > +* @param poolId the id of the Pool to associate. > +* @param healthMonitor the HealthMonitor to associate. > +* @return the newly associated HealthMonitor. > +*/ > + @Named("pool:associate_health_monitor") > + @POST > + @Path("/pools/{pool-id}/health_monitors") > + @SelectJson("health_monitor") > + HealthMonitor associateHealthMonitor(@PathParam("pool-id") String poolId, Good! I tried with the @PayloadParam annotation some days ago, but it did not work. But it was because I had missed the @Payload annotation. So now the new associate method signature is: // Associate a HealthMonitor to a Pool. @Named("pool:associate_health_monitor") @POST @Path("/pools/{pool-id}/health_monitors") @SelectJson("health_monitor") @Payload("%7B\"health_monitor\":%7B\"id\":\"{healthMonitorId}\"%7D%7D") @Produces(MediaType.APPLICATION_JSON) HealthMonitor associateHealthMonitor(@PathParam("pool-id") String poolId, @PayloadParam("healthMonitorId") String healthMonitorId); ... and the AssociateBuilder is removed. Thank you for your help --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18023998
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> @@ -103,4 +104,14 @@ > */ > @Delegate > Optional getSecurityGroupApi(@EndpointParam(parser = > RegionToEndpoint.class) String region); > + > + /** > +* Provides access to LBaaS version 1 features. > +* > +* NOTE > +* This API is an extension that may or may not be present in your > OpenStack cloud. Use the Optional return type > +* to determine if it is present. > +*/ > + @Delegate > + Optional getLBaaSv1Api(@EndpointParam(parser = > RegionToEndpoint.class) String region); Sorry, but I don't see in the sources where v2 will be lbaasv2. However, if the rule is to use the alias name in the method signature, I'll do it. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18023746
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
[jclouds-labs-openstack-pull-requests #436](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/436/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56797586
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
[jclouds » jclouds-labs-openstack #1754](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1754/) SUCCESS This pull request looks good [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56797193
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +import java.beans.ConstructorProperties; > + > +import javax.inject.Named; > + > +import org.jclouds.javax.annotation.Nullable; > + > +import com.google.common.base.MoreObjects; > +import com.google.common.base.Objects; > +import com.google.common.collect.ImmutableList; > + > +/** > + * A Neutron LBaaS v1 HealthMonitor. > + * > + * @see href="http://docs.openstack.org/api/openstack-network/2.0/content/lbaas_ext_ops_health_monitor.html";>api Done on all domain objects --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18023382
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported SessionPersistence types. > + */ > +public enum SessionPersistenceType { Done --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18023342
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
Looks almost ready to merge! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-5674
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > + > +/** > + * Enumerates supported HTTP methods. > + */ > +public enum HttpMethod { > + GET("GET"), JavaDocs? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18005267
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported Neutron LBaaS v1 resources status. > + */ > +public enum LBaaSStatus { JavaDocs and fromValue --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18005275
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported types of probe send by by load balancer to verify > member state. > + */ > +public enum ProbeType { Ha, didn't see that you have. Thanks! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18005130
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported types of probe send by by load balancer to verify > member state. > + */ > +public enum ProbeType { Also, if you could add some JavaDoc to the enumerables, that would be great. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18004213
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> I would like to do it, but this will be a problem when support of LBaaS v2 > will be added. As mentioned previously, LBaaS v1 and LBaaS v2 differ on > domain objects and on API. I see. I had a look at the v1 vs v2 issues. I am fine with keeping the structure as it is for now. Question: At some point in the future, would it make sense to drop v1 and just have v2 as the LBaaS? Because then we could get rid of the extra directories (and multiple version strings in the paths). --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56747192
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + public String getName() { > + return name; > + } > + > + /** > +* @return the description of the Pool. > +*/ > + @Nullable > + public String getDescription() { > + return description; > + } > + > + /** > +* @return the subnet id for this Pool. > +*/ > + public String getSubnetId() { UpdatePool updatePool = ... [make an update pool] My point was that an updatePool.getSubnetId() will return null (because you cannot send a subnet id for update), so the getter should be marked as nullable? Somewhat of an edge case though. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18003662
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/health_monitors/{id}") > + @Fallback(FalseOnNotFoundOr404.class) > + boolean deleteHealthMonitor(@PathParam("id") String id); > + > + /** > +* Associate a HealthMonitor to a Pool. > +* > +* @param poolId the id of the Pool to associate. > +* @param healthMonitor the HealthMonitor to associate. > +* @return the newly associated HealthMonitor. > +*/ > + @Named("pool:associate_health_monitor") > + @POST > + @Path("/pools/{pool-id}/health_monitors") > + @SelectJson("health_monitor") > + HealthMonitor associateHealthMonitor(@PathParam("pool-id") String poolId, > how to simplify the method signature (without AssociateBuilder) and generate > a json payload with health_monitor object in the same time: I see. Question: Will using something similar to https://github.com/jclouds/jclouds-labs-openstack/blob/master/openstack-neutron/src/main/java/org/jclouds/openstack/neutron/v2/extensions/RouterApi.java#L153 work? If not, there are ways to either specify the json in the annotations or use a more complex MapBinder. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18003448
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/vips") > + @SelectJson("vip") > + VIP createVIP(@WrapWith("vip") VIP.CreateVIP vip); > + > + /** > +* Update a VIP. > +* > +* @param id the id of the VIP to update. > +* @param vip the VIP's attributes to update. > +* @return a reference of the updated VIP. > +*/ > + @Named("vip:update") > + @PUT > + @Path("/vips/{id}") > + @SelectJson("vip") > + @Fallback(NullOnNotFoundOr404.class) Yep, the 404 fallbacks will have to be addressed separately. Leave as is for now. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18003225
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> @@ -103,4 +104,14 @@ > */ > @Delegate > Optional getSecurityGroupApi(@EndpointParam(parser = > RegionToEndpoint.class) String region); > + > + /** > +* Provides access to LBaaS version 1 features. > +* > +* NOTE > +* This API is an extension that may or may not be present in your > OpenStack cloud. Use the Optional return type > +* to determine if it is present. > +*/ > + @Delegate > + Optional getLBaaSv1Api(@EndpointParam(parser = > RegionToEndpoint.class) String region); I am not completely sure, but is this what you are asking for? https://github.com/openstack/neutron/tree/master/neutron/services/loadbalancer --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r18003146
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + return expectedCodes; > + } > + > + /** > +* @return the pools for this HealthMonitor. > +*/ > + @Nullable > + public ImmutableList getPools() { > + return pools; > + } > + > + /** > +* @return the administrative state for this HealthMonitor. > +*/ > + @Nullable > + public Boolean getAdminStateUp() { Please disregard this comment. You are correct, the spec says to use `getXXX()` for types. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17979286
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + return expectedCodes; > + } > + > + /** > +* @return the pools for this HealthMonitor. > +*/ > + @Nullable > + public ImmutableList getPools() { > + return pools; > + } > + > + /** > +* @return the administrative state for this HealthMonitor. > +*/ > + @Nullable > + public Boolean getAdminStateUp() { I think isXXX() is only for boolean properties, getXXX() should be used for Boolean. This is because getAdminStateUp() can return 3 values: null, Boolean.TRUE or Boolean.FALSE. http://download.oracle.com/otndocs/jcp/7224-javabeans-1.01-fr-spec-oth-JSpec/ http://blog.950buy.com/article/javabean-specification-on-a-few-you-should-know/ --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17962110
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> @@ -103,4 +104,14 @@ > */ > @Delegate > Optional getSecurityGroupApi(@EndpointParam(parser = > RegionToEndpoint.class) String region); > + > + /** > +* Provides access to LBaaS version 1 features. > +* > +* NOTE > +* This API is an extension that may or may not be present in your > OpenStack cloud. Use the Optional return type > +* to determine if it is present. > +*/ > + @Delegate > + Optional getLBaaSv1Api(@EndpointParam(parser = > RegionToEndpoint.class) String region); Please, can you tell me where lbaas and lbaasv2 are defined in the OpenStack sources? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17959167
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + /** > + * Provides the administrative state for this HealthMonitor's Builder. > + * > + * @return the Builder. > + * @see HealthMonitor#getAdminStateUp() > + */ > + public ParameterizedBuilderType adminStateUp(Boolean adminStateUp) { > + healthMonitor.adminStateUp = adminStateUp; > + return self(); > + } > + } > + > + /** > +* Create builder (inheriting from Builder). > +*/ > + public static class CreateBuilder extends Builder { Those fields are required only if type is HTTP/HTTPS and to overwrite the default values. CreateBuilder inherits from Builder that exposes the fields. HealthMonitor.CreateBuilder builder = HealthMonitor.createBuilder(ProbeType.HTTP, 10, 5, 2).httpMethod(HttpMethod.POST).urlPath("/path").expectedCodes("201").build(); I could add additional CreateBuilder constructors dedicated to HTTP/HTTPS and with those parameters, but I'm not sure this is a good idea: HealthMonitor.CreateBuilder httpBuilder = HealthMonitor.createHttpBuilder(10, 5, 2, HttpMethod.POST, "/path", "201").build(); HealthMonitor.CreateBuilder httpsBuilder = HealthMonitor.createHttpsBuilder(10, 5, 2, HttpMethod.POST, "/path", "201").build(); Let me know --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17957618
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/vips") > + @SelectJson("vip") > + VIP createVIP(@WrapWith("vip") VIP.CreateVIP vip); > + > + /** > +* Update a VIP. > +* > +* @param id the id of the VIP to update. > +* @param vip the VIP's attributes to update. > +* @return a reference of the updated VIP. > +*/ > + @Named("vip:update") > + @PUT > + @Path("/vips/{id}") > + @SelectJson("vip") > + @Fallback(NullOnNotFoundOr404.class) Let's keep things simple and leave it as-is :) I'll open a PR to address all POST and PUT methods in all apis to fix the mentioned issue once this one has been merged. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17933842
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + /** > + * Provides the administrative state for this HealthMonitor's Builder. > + * > + * @return the Builder. > + * @see HealthMonitor#getAdminStateUp() > + */ > + public ParameterizedBuilderType adminStateUp(Boolean adminStateUp) { > + healthMonitor.adminStateUp = adminStateUp; > + return self(); > + } > + } > + > + /** > +* Create builder (inheriting from Builder). > +*/ > + public static class CreateBuilder extends Builder { This `CreateBuilder` does not expose the fields for creating an HTTP/HTTPS Health Monitor: - `http_method` - `url_path` - `expected_codes` --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17931256
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +import java.beans.ConstructorProperties; > + > +import javax.inject.Named; > + > +import org.jclouds.javax.annotation.Nullable; > + > +import com.google.common.base.MoreObjects; > +import com.google.common.base.Objects; > +import com.google.common.collect.ImmutableList; > + > +/** > + * A Neutron LBaaS v1 HealthMonitor. > + * > + * @see href="http://docs.openstack.org/api/openstack-network/2.0/content/lbaas_ext_ops_health_monitor.html";>api Yes, please remove all external links from the sources. Thx! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17930455
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported SessionPersistence types. > + */ > +public enum SessionPersistenceType { This enum should be moved to `SessionPersistence.Type` and also needs the `fromValue` method added. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17928260
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +"admin_state_up": false, > +"subnet_id": "8032909d-47a1-4715-90af-5153ffe39861", > +"tenant_id": "83657cfcdfe44cd5920adaf26c48ceea", > +"connection_limit": 50, > +"pool_id": "61b1f87a-7a21-4ad3-9dda-7f81d249944f", > +"session_persistence": { > +"cookie_name": "MyNewAppCookie", > +"type": "APP_COOKIE" > +}, > +"address": "10.0.0.11", > +"protocol_port": 80, > +"port_id": "f7e6fe6a-b8b5-43a8-8215-73456b32e0f5", > +"id": "c987d2be-9a3c-4ac9-a046-e8716b1350e2", > +"name": "new-name" > +} > +} Newline after `}` --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17920633
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> @@ -0,0 +1,13 @@ > +{ > +"vip": { > +"name": "new-name", > +"description": "new description", > +"pool_id": "61b1f87a-7a21-4ad3-9dda-7f81d249944f", > +"session_persistence": { > +"cookie_name": "MyNewAppCookie", > +"type": "APP_COOKIE" > +}, > +"connection_limit": 50, > +"admin_state_up": false > +} > +} Newline after `}` --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17920632
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + return expectedCodes; > + } > + > + /** > +* @return the pools for this HealthMonitor. > +*/ > + @Nullable > + public ImmutableList getPools() { > + return pools; > + } > + > + /** > +* @return the administrative state for this HealthMonitor. > +*/ > + @Nullable > + public Boolean getAdminStateUp() { `public Boolean isAdminStateUp()` --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17917756
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> @@ -67,6 +67,8 @@ protected void configure() { > > URI.create("http://docs.openstack.org/ext/neutron/router/api/v1.0";)) >.put(URI.create(ExtensionNamespaces.SECURITY_GROUPS), > > URI.create("http://docs.openstack.org/ext/securitygroups/api/v2.0";)) > + .put(URI.create(ExtensionNamespaces.LBAAS_V1), `ExtensionNamespaces.LBAAS` to be consistent with the alias name. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17917571
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> @@ -103,4 +104,14 @@ > */ > @Delegate > Optional getSecurityGroupApi(@EndpointParam(parser = > RegionToEndpoint.class) String region); > + > + /** > +* Provides access to LBaaS version 1 features. > +* > +* NOTE > +* This API is an extension that may or may not be present in your > OpenStack cloud. Use the Optional return type > +* to determine if it is present. > +*/ > + @Delegate > + Optional getLBaaSv1Api(@EndpointParam(parser = > RegionToEndpoint.class) String region); No. Extension APIs use the alias name in the method signature. If you refer to the OpenStack sources, v1 is `lbaas` and v2 will be `lbaasv2`. Given this fact, this method should read: `Optional getLBaaSApi(@EndpointParam(parser = RegionToEndpoint.class) String region);` When v2 is supported, the method would be: `Optional getLBaaSv2Api(@EndpointParam(parser = RegionToEndpoint.class) String region);` --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17917264
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + > + /* > +* Methods to get the builder follow. > +*/ > + > + /** > +* @return the Builder for SessionPersistence. > +*/ > + public static Builder builder() { > + return new Builder(); > + } > + > + /** > +* Gets a Builder configured as this object. > +*/ > + public Builder toBuilder() { I removed the to and from builder methods. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17914083
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +*/ > + public static Builder builder() { > + return new Builder(); > + } > + > + /** > +* Gets a Builder configured as this object. > +*/ > + public Builder toBuilder() { > + return new Builder().fromPoolStatus(this); > + } > + > + /** > +* Builder. > +*/ > + public static class Builder { I removed the builder --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17914014
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +{ > +"health_monitor": { > +"status": "PENDING_CREATE", > +"admin_state_up": true, > +"tenant_id": "4fd44f30292945e481c7b8a0c8908869", > +"delay": 1, > +"expected_codes": "200", > +"max_retries": 1, > +"http_method": "GET", > +"timeout": 1, > +"pools": [], > +"url_path": "/", > +"type": "HTTP", > +"id": "b624decf-d5d3-4c66-9a3d-f047e7786181" > +} > +} Done --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17914138
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
[jclouds » jclouds-labs-openstack #1748](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1748/) SUCCESS This pull request looks good [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56534317
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +/** > + * Enumerates supported SessionPersistence types. > + */ > +public enum SessionPersistenceType { > + /** > +* All connections that originate from the same source IP address are > handled by the same member of the pool. > +*/ > + SOURCE_IP, > + /** > +* The load balancing function creates a cookie on the first request from > a client. Subsequent requests that > +* contain the same cookie value are handled by the same member of the > pool. > +*/ > + HTTP_COOKIE, > + /** > +* The load balancing function relies on a cookie established by the > back-end application. All requests with the > +* same cookie value are handled by the same member of the pool. I added the fromValue method --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17914125
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +*/ > + public static class CreateBuilder extends Builder { > + /** > + * Supply required properties for creating a HealthMonitor's > CreateBuilder. > + * > + * @param type the probe type. > + * @param delay the delay. > + * @param timeout the timeout. > + * @param maxRetries the max retries. > + */ > + private CreateBuilder(ProbeType type, Integer delay, Integer timeout, > Integer maxRetries) { > + type(type).delay(delay).timeout(timeout).maxRetries(maxRetries); > + } > + > + /** > + * Provide the tenantId for this HealthMonitor's CreateBuilder. > Admin-only. Done --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17913799
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported HTTP methods. > + */ > +public enum HttpMethod { Done --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17913906
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + private Integer timeout; > + @Named("max_retries") > + private Integer maxRetries; > + // Optional attributes that can be updated > + @Named("http_method") > + private HttpMethod httpMethod; > + @Named("url_path") > + private String urlPath; > + @Named("expected_codes") > + private String expectedCodes; > + @Named("admin_state_up") > + private Boolean adminStateUp; > + // Read-only attributes > + @Named("id") > + private String id; > + @Named("pools") I have removed @Named where it is not needed --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17913785
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Named("address") > + private String address; > + @Named("protocol_port") > + private Integer protocolPort; > + // Mandatory attributes that can be updated > + @Named("pool_id") > + private String poolId; > + // Optional attributes that can be updated > + @Named("weight") > + private Integer weight; > + @Named("admin_state_up") > + private Boolean adminStateUp; > + // Read-only attributes > + private String id; > + @Named("status") > + private Status status; Status renamed to LBaaSStatus --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17913946
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +*/ > + public static Builder builder() { > + return new Builder(); > + } > + > + /** > +* Gets a Builder configured as this object. > +*/ > + public Builder toBuilder() { > + return new Builder().fromHealthMonitorStatus(this); > + } > + > + /** > +* Builder. > +*/ > + public static class Builder { You are right. I removed the builder. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17913875
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
[jclouds-labs-openstack-pull-requests #433](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/433/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56533447
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +import java.beans.ConstructorProperties; > + > +import javax.inject.Named; > + > +import org.jclouds.javax.annotation.Nullable; > + > +import com.google.common.base.MoreObjects; > +import com.google.common.base.Objects; > +import com.google.common.collect.ImmutableList; > + > +/** > + * A Neutron LBaaS v1 HealthMonitor. > + * > + * @see href="http://docs.openstack.org/api/openstack-network/2.0/content/lbaas_ext_ops_health_monitor.html";>api Done --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17913732
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
[jclouds » jclouds-labs-openstack #1747](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/1747/) UNSTABLE Looks like there's a problem with this pull request [(what's this?)](https://www.cloudbees.com/what-is-buildhive) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56529361
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
[jclouds-labs-openstack-pull-requests #432](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/432/) UNSTABLE Looks like there's a problem with this pull request --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56528410
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
I don't agree with you about putiing LBaaSApi on the same directory level as RouterApi. I would like to do it, but this will be a problem when support of LBaaS v2 will be added. As mentioned previously, LBaaS v1 and LBaaS v2 differ on domain objects and on API. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56525829
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/health_monitors/{id}") > + @Fallback(FalseOnNotFoundOr404.class) > + boolean deleteHealthMonitor(@PathParam("id") String id); > + > + /** > +* Associate a HealthMonitor to a Pool. > +* > +* @param poolId the id of the Pool to associate. > +* @param healthMonitor the HealthMonitor to associate. > +* @return the newly associated HealthMonitor. > +*/ > + @Named("pool:associate_health_monitor") > + @POST > + @Path("/pools/{pool-id}/health_monitors") > + @SelectJson("health_monitor") > + HealthMonitor associateHealthMonitor(@PathParam("pool-id") String poolId, Here are 2 examples to use LBaaS v1 or LBaaS v2. as you can see, it would be difficult to have a common API compatible with LBaaS v1 and LBaaS v2. // Get org.jclouds.openstack.neutron.v2.extensions.lbaas.v1.LBaaSApi extension Optional lbaasV1ApiExtension = neutronApi.getLBaaSv1Api(region); if (lbaasV1ApiExtension.isPresent()) { LBaaSApi lbaasV1Api = lbaasV1ApiExtension.get(); // Create org.jclouds.openstack.neutron.v2.domain.lbaas.v1.Pool Pool.CreatePool createPool = Pool.createBuilder(subnetId, Protocol.HTTP, Pool.ROUND_ROBIN) .name("mypool").provider("HAPROXY").build(); Pool poolV1 = lbaasV1Api.createPool(createPool); // Create org.jclouds.openstack.neutron.v2.domain.lbaas.v1.Member Member.CreateMember createMember = Member.createBuilder(poolV1.getId(), privateIPAddressOfNode, 80).build(); Member memberV1 = lbaasV1Api.createMember(createMember); // Create 2 org.jclouds.openstack.neutron.v2.domain.lbaas.v1.HealthMonitor HealthMonitor.CreateHealthMonitor createHealthMonitor1 = HealthMonitor.createBuilder(ProbeType.HTTP, 60, 5, 2).build(); HealthMonitor firstHealthMonitorV1 = lbaasV1Api.createHealthMonitor(createHealthMonitor1); HealthMonitor.CreateHealthMonitor createHealthMonitor2 = HealthMonitor.createBuilder(ProbeType.PING, 60, 5, 2).build(); HealthMonitor secondHealthMonitorV1 = lbaasV1Api.createHealthMonitor(createHealthMonitor2); // Associate Health Monitors to Pool HealthMonitor.AssociateHealthMonitor associateHealthMonitor1 = HealthMonitor.associateBuilder(firstHealthMonitorV1.getId()).build(); HealthMonitor healthMonitorAssociated1 = lbaasV1Api.associateHealthMonitor(poolV1.getId(), associateHealthMonitor1); HealthMonitor.AssociateHealthMonitor associateHealthMonitor2 = HealthMonitor.associateBuilder(firstHealthMonitorV1.getId()).build(); HealthMonitor healthMonitorAssociated2 = lbaasV1Api.associateHealthMonitor(poolV1.getId(), associateHealthMonitor2); // Create org.jclouds.openstack.neutron.v2.domain.lbaas.v1.VIP VIP.CreateVIP createVIP = VIP.createBuilder(subnetId, Protocol.HTTP, 80, poolV1.getId()) .name("myvip").build(); VIP vipV1 = lbaasV1Api.createVIP(createVIP); } else { logger.info("LBaaS API Version 1 is unavailable"); } // Note: this is my understand of the LBaaS API improvement // see https://blueprints.launchpad.net/neutron/+spec/lbaas-api-and-objmodel-improvement // Note: the OpenStack Networking API V2.0 document (September 9, 2014) describesLBaaS v2, but // it has been reviewed September 23, 2014 and finally, ONLY the LBaaS v1 API is described. // Does it mean next OpenStack release (Juno) will not provide LBaaS v2? I think so. // Get org.jclouds.openstack.neutron.v2.extensions.lbaas.v2.LBaaSApi extension Optional lbaasV2ApiExtension = neutronApi.getLBaaSv2Api(region); if (lbaasV2ApiExtension.isPresent()) { LBaaSApi lbaasV2Api = lbaasV2ApiExtension.get(); // Create org.jclouds.openstack.neutron.v2.domain.lbaas.v2.Pool // Note: vipId, subnetId, protocol and provider are not attributes of Pool anymore in LBaaS v2 Pool.CreatePool createPool = Pool.createBuilder(Pool.ROUND_ROBIN) .name("mypool").build(); Pool poolV2 = lbaasV2Api.createPool(createPool); // Create org.jclouds.openstack.neutron.v2.domain.lbaas.v2.Member // Note: subnetId is new but optional. I think moving subnetId from pool to member allows load balancing between nodes located on different subnets. Member.CreateMember createMember = Member.createBuilder(poolV2.getId(), privateIPAddressOfNode, 80) .subnetId(subnetId).build(); Member memberV2 = lbaasV2Api.createMember(createMember); // Create org.jclouds.openstack.neutron.v2.domain.lbaas.v2.HealthMonitor HealthMonitor.CreateHealthMonitor createHealthMonitor = HealthMonitor.createBuilder(ProbeType.HTTP, 60, 5, 2).build(); HealthMonitor healthMonitorV2 = lbaasV2Api.createHealthMonitor(createHealthMonitor); // Associate Health Monitor to Pool // Note: Only one health monitor can be associated to a
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +public class Pool { > + > + // Load balancing methods that must be supported by all providers. > + // Not an enum type because any provider may support additional balancing > methods. > + public static String ROUND_ROBIN = "ROUND_ROBIN"; > + public static String LEAST_CONNECTIONS = "LEAST_CONNECTIONS"; > + public static String SOURCE_IP = "SOURCE_IP"; > + > + // Mandatory attributes when creating > + @Named("tenant_id") > + private String tenantId; > + @Named("subnet_id") > + private String subnetId; > + @Named("protocol") > + private Protocol protocol; > + // Mandatory attributes that can be updated No "Mandatory attributes when creating" section is for attributes that are mandatory for creation and that cannot be modified after creation. Maybe I have to review the comments? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17899421
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + public String getName() { > + return name; > + } > + > + /** > +* @return the description of the Pool. > +*/ > + @Nullable > + public String getDescription() { > + return description; > + } > + > + /** > +* @return the subnet id for this Pool. > +*/ > + public String getSubnetId() { - Create: CreateBuilder contructor constrains the developper to specify the subnetId. Do I have to test parameter is not null in CreateBuilder.subnetId()? I prefer let OpenStack reply with the appropriate error message. - Update: subnetId cannot be modified after creation, so UpdateBuilder does not provide method to modify it. The UpdatePool.subnetId is always null, and so subnet_id does not appear in the request's payload (and Neutron does not expect subnet_id in PUT operation). --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17899346
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/health_monitors/{id}") > + @Fallback(FalseOnNotFoundOr404.class) > + boolean deleteHealthMonitor(@PathParam("id") String id); > + > + /** > +* Associate a HealthMonitor to a Pool. > +* > +* @param poolId the id of the Pool to associate. > +* @param healthMonitor the HealthMonitor to associate. > +* @return the newly associated HealthMonitor. > +*/ > + @Named("pool:associate_health_monitor") > + @POST > + @Path("/pools/{pool-id}/health_monitors") > + @SelectJson("health_monitor") > + HealthMonitor associateHealthMonitor(@PathParam("pool-id") String poolId, As I explained to @demobox , LBaaS v1 association method takes a health_monitor as parameter, not a String. However, only the health monitor's id must be specified. I thought that AssociateBuilder was the more elegant way to map the "associate health monitor" call. However, let me know how to simplify the method signature (without AssociateBuilder) and generate a json payload with health_monitor object in the same time: associateHealthMonitor(String poolId, String healthMonitorId) -> { "health_monitor": { "id":"5d4b5228-33b0-4e60-b225-9b727c1a20e7" } } --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17898166
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/vips") > + @SelectJson("vip") > + VIP createVIP(@WrapWith("vip") VIP.CreateVIP vip); > + > + /** > +* Update a VIP. > +* > +* @param id the id of the VIP to update. > +* @param vip the VIP's attributes to update. > +* @return a reference of the updated VIP. > +*/ > + @Named("vip:update") > + @PUT > + @Path("/vips/{id}") > + @SelectJson("vip") > + @Fallback(NullOnNotFoundOr404.class) POST operations don't use 404 fallbacks. Should I have to remove 404 fallbacks on PUT operations? Or is it going to be the subject of a future PR? I mean, a new PR to remove 404 fallbacks on all service operations (RouterApi, NetworkApi, etc.) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17897680
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
@fbrouille Thanks for this PR! Overall it looks very good to me. I have a few comments, but they all seem pretty minor. It would also be nice if we can add some more docs to some of these enums and getters and builder setters for users. Some notes on structure and style differences: most of these seem like improvements to me, and we might have to apply them to the rest of the neutron code (in a different PR). I do like that the new stuff is in its own lbaas/v1 directories (esp the domain classes, something we should consider doing). However, this is probably not needed under /extensions - just keep LBaaSApi on the same directory level as say RouterApi. I am not so sure if we should do the same for the fallbacks and functions or not: thoughts? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56415849
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +{ > +"health_monitor": { > +"status": "PENDING_CREATE", > +"admin_state_up": true, > +"tenant_id": "4fd44f30292945e481c7b8a0c8908869", > +"delay": 1, > +"expected_codes": "200", > +"max_retries": 1, > +"http_method": "GET", > +"timeout": 1, > +"pools": [], > +"url_path": "/", > +"type": "HTTP", > +"id": "b624decf-d5d3-4c66-9a3d-f047e7786181" > +} > +} new line --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17866171
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +/** > + * Enumerates supported SessionPersistence types. > + */ > +public enum SessionPersistenceType { > + /** > +* All connections that originate from the same source IP address are > handled by the same member of the pool. > +*/ > + SOURCE_IP, > + /** > +* The load balancing function creates a cookie on the first request from > a client. Subsequent requests that > +* contain the same cookie value are handled by the same member of the > pool. > +*/ > + HTTP_COOKIE, > + /** > +* The load balancing function relies on a cookie established by the > back-end application. All requests with the > +* same cookie value are handled by the same member of the pool. Docs are always great! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17865484
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + > + /* > +* Methods to get the builder follow. > +*/ > + > + /** > +* @return the Builder for SessionPersistence. > +*/ > + public static Builder builder() { > + return new Builder(); > + } > + > + /** > +* Gets a Builder configured as this object. > +*/ > + public Builder toBuilder() { I usually don't see the to and from builder methods used much, but eh. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17865416
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported types of probe send by by load balancer to verify > member state. > + */ > +public enum ProbeType { fromValue We have this documented here: https://wiki.apache.org/jclouds/Enums Though it can always be documented a bit better! :) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17865305
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +*/ > + public static Builder builder() { > + return new Builder(); > + } > + > + /** > +* Gets a Builder configured as this object. > +*/ > + public Builder toBuilder() { > + return new Builder().fromPoolStatus(this); > + } > + > + /** > +* Builder. > +*/ > + public static class Builder { Same as (someplace) above, it seems status is not something the user will pass to the service. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17865210
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +public class Pool { > + > + // Load balancing methods that must be supported by all providers. > + // Not an enum type because any provider may support additional balancing > methods. > + public static String ROUND_ROBIN = "ROUND_ROBIN"; > + public static String LEAST_CONNECTIONS = "LEAST_CONNECTIONS"; > + public static String SOURCE_IP = "SOURCE_IP"; > + > + // Mandatory attributes when creating > + @Named("tenant_id") > + private String tenantId; > + @Named("subnet_id") > + private String subnetId; > + @Named("protocol") > + private Protocol protocol; > + // Mandatory attributes that can be updated Are these required to be present when updating? If so, they need to be included in the updateBuilder params, etc. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17864827
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + public String getName() { > + return name; > + } > + > + /** > +* @return the description of the Pool. > +*/ > + @Nullable > + public String getDescription() { > + return description; > + } > + > + /** > +* @return the subnet id for this Pool. > +*/ > + public String getSubnetId() { Is this guaranteed not null in the update/create instances as well? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17864745
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +import org.jclouds.javax.annotation.Nullable; > + > +import com.google.common.base.MoreObjects; > +import com.google.common.base.Objects; > +import com.google.common.collect.ImmutableList; > +import com.google.common.collect.ImmutableSet; > + > +/** > + * A Neutron LBaaS v1 Pool. > + * > + * @see href="http://docs.openstack.org/api/openstack-network/2.0/content/lbaas_ext_ops_pool.html";>api > doc > + */ > +public class Pool { > + > + // Load balancing methods that must be supported by all providers. > + // Not an enum type because any provider may support additional balancing > methods. Good comment! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17864305
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Named("address") > + private String address; > + @Named("protocol_port") > + private Integer protocolPort; > + // Mandatory attributes that can be updated > + @Named("pool_id") > + private String poolId; > + // Optional attributes that can be updated > + @Named("weight") > + private Integer weight; > + @Named("admin_state_up") > + private Boolean adminStateUp; > + // Read-only attributes > + private String id; > + @Named("status") > + private Status status; Status or LoadBalancerStatus? I am thinking Status might be too generic, thoughts? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17863840
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + * > + * 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. > + */ > + > +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +/** > + * Enumerates supported HTTP methods. > + */ > +public enum HttpMethod { You need to implement fromValue for this enum to work properly with jclouds --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17863735
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +*/ > + public static Builder builder() { > + return new Builder(); > + } > + > + /** > +* Gets a Builder configured as this object. > +*/ > + public Builder toBuilder() { > + return new Builder().fromHealthMonitorStatus(this); > + } > + > + /** > +* Builder. > +*/ > + public static class Builder { This builder is probably not needed, unless a jclouds user has to be able to build a Status as part of a request object. If this is something that you get from the service, but don't pass to the service, the builder is not needed (and might be confusing to have it). --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17863685
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/health_monitors/{id}") > + @Fallback(FalseOnNotFoundOr404.class) > + boolean deleteHealthMonitor(@PathParam("id") String id); > + > + /** > +* Associate a HealthMonitor to a Pool. > +* > +* @param poolId the id of the Pool to associate. > +* @param healthMonitor the HealthMonitor to associate. > +* @return the newly associated HealthMonitor. > +*/ > + @Named("pool:associate_health_monitor") > + @POST > + @Path("/pools/{pool-id}/health_monitors") > + @SelectJson("health_monitor") > + HealthMonitor associateHealthMonitor(@PathParam("pool-id") String poolId, I have not looked at the extension docs much, but isn't it enough to send a health monitor id instead of a AssociateHealthMonitor JSON object? Similar to addInterfaceForSubnet and addInterfaceForPort in the Router extension? https://wiki.openstack.org/wiki/Neutron/LBaaS/API_1.0#Associate_Health_Monitors_with_a_Pool --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17860140
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
The way the versioning was handled was the first thing I looked at and was comfortable with the approach. But thinking about it a bit more I could use an example. @fbrouille Can you provide us with an example here in the PR comments that shows how a user would write some code against v1 LBaaS *and* the v2 LBaaS? This will give us a much better idea of how these different versions will co-exist. I'm thinking...example code that creates an LB, attaches a node, and creates a health monitor check. I know there isn't any v2 LBaaS so that code would be "hypothetical". --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56398355
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/vips") > + @SelectJson("vip") > + VIP createVIP(@WrapWith("vip") VIP.CreateVIP vip); > + > + /** > +* Update a VIP. > +* > +* @param id the id of the VIP to update. > +* @param vip the VIP's attributes to update. > +* @return a reference of the updated VIP. > +*/ > + @Named("vip:update") > + @PUT > + @Path("/vips/{id}") > + @SelectJson("vip") > + @Fallback(NullOnNotFoundOr404.class) So far: create operations should not use it, but update operations still use the 404 (for now, but going away in the future, I believe). --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17857909
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + private Integer timeout; > + @Named("max_retries") > + private Integer maxRetries; > + // Optional attributes that can be updated > + @Named("http_method") > + private HttpMethod httpMethod; > + @Named("url_path") > + private String urlPath; > + @Named("expected_codes") > + private String expectedCodes; > + @Named("admin_state_up") > + private Boolean adminStateUp; > + // Read-only attributes > + @Named("id") > + private String id; > + @Named("pools") @Named here is not needed. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17857030
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + @Path("/vips") > + @SelectJson("vip") > + VIP createVIP(@WrapWith("vip") VIP.CreateVIP vip); > + > + /** > +* Update a VIP. > +* > +* @param id the id of the VIP to update. > +* @param vip the VIP's attributes to update. > +* @return a reference of the updated VIP. > +*/ > + @Named("vip:update") > + @PUT > + @Path("/vips/{id}") > + @SelectJson("vip") > + @Fallback(NullOnNotFoundOr404.class) POST and PUT operations should not use 404 fallbacks as they can produce misleading results. [JCLOUDS-691](https://issues.apache.org/jira/browse/JCLOUDS-691) is already reported to remove the current uses of this practice, but new APIs/methods shouldn't use it. (@zack-shoylev @everett-toews do you agree?) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17828087
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> es, LBaaS v1 association method takes a health_monitor as parameter (even if > only the id must be specified) OK, then the AssociateBuilder is consistent with the pattern. As regards the naming of the API and how to represent the version, I'll defer to @jdaggett @zack-shoylev @everett-toews ;-) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56268955
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
Thanks for your review @demobox, - should domain objects have private or protected constructors and fields? I wrote the domain objects in order to be coherent with the rest of the code. Others domain objects were written in the same way: e.g. Pool has private fileds and constructors as Subnet and HealthMonitorStatus has protected fiields and constructors as AllocationPool. However I agree to use private visibility for all domain objects. - should we have version numbers in method names and constants (LBaaSv1)? Next version of OpenStack (Juno) will improve LBaaS API and object model (https://blueprints.launchpad.net/neutron/+spec/lbaas-api-and-objmodel-improvement). The new LBaaS will expose a new API (v2) but will also be compatible with the legacy API (v1). This means that a cloud-platform built with OpenStack may have (or not) LBaaS extension, may expose LBaaS API v1 (if Havana or IceHouse is used), or may expose LBaaS API v1 and LBaaS API v2 (if Juno is used). My point of view is that jclouds must support both versions to be able to address all cloud-platforms built with OpenStack (Havana, IceHouse, Juno, etc.). I have tried to define "generic" API and domain objects that would be compatible with LBaaS v1 and LBaaS v2. However, even if LBaaS improvements do not change all, it is not so simple: e.g. VIP is replaced by LoadBalancer and Listener; attribute subnet_id moves from Pool to Member, I think that "generic" domain objects would be confused for developpers that use jclouds: e.g. as a developper, do I have to use the Pool.subnetId or the Member.subnetId? Do I have to use VIP or LoadBalancer? Finally, I decided to explicitly seperate LBaaS v1 and LBaaS v2. I think it is more simple for everyone (contributors an users jclouds) to use either LBaaSv1 API or LBaaSv2 API. This implies having NeutronAPI.getLBaaSv1Api and NeutronAPI.getLBaaSv2Api. And I think that it is the role of the jclouds LoadBlancer abstraction to use either LBaaS v1 or v2 depending on the API exposed by the cloud-platform. However, if you have better idea to create a generic API independant of the version, let's discuss about it. - does the "associate health monitor" call require a domain/parameter object too? Yes, LBaaS v1 association method takes a health_monitor as parameter (even if only the id must be specified), not a String. I thought that AssociateBuilder was the more elegant way to map the "associate health monitor" call. However, if you have a better idea, let me know. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56264339
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
Only just started, but some general comments/questions to check: * should domain objects have `private` or `protected` constructors and fields? I think the PR includes a mix of both * stupid question: do we need `@Named` even if the field name matches the value of the annotation? * should we have version numbers in method names and constants (LBaaSv1)? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146#issuecomment-56254916
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +*/ > + public static class CreateBuilder extends Builder { > + /** > + * Supply required properties for creating a HealthMonitor's > CreateBuilder. > + * > + * @param type the probe type. > + * @param delay the delay. > + * @param timeout the timeout. > + * @param maxRetries the max retries. > + */ > + private CreateBuilder(ProbeType type, Integer delay, Integer timeout, > Integer maxRetries) { > + type(type).delay(delay).timeout(timeout).maxRetries(maxRetries); > + } > + > + /** > + * Provide the tenantId for this HealthMonitor's CreateBuilder. > Admin-only. [minor] "Provides" --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17817364
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +*/ > + public static CreateBuilder createBuilder(ProbeType type, Integer delay, > Integer timeout, Integer maxRetries) { > + return new CreateBuilder(type, delay, timeout, maxRetries); > + } > + > + /** > +* @return the Builder for updating a HealthMonitor. > +*/ > + public static UpdateBuilder updateBuilder() { > + return new UpdateBuilder(); > + } > + > + /** > +* @return the Builder for associating a HealthMonitor to a Pool. > +*/ > + public static AssociateBuilder associateBuilder(String id) { Do we need a "parameter object" for the "associate" call? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17817360
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> +package org.jclouds.openstack.neutron.v2.domain.lbaas.v1; > + > +import java.beans.ConstructorProperties; > + > +import javax.inject.Named; > + > +import org.jclouds.javax.annotation.Nullable; > + > +import com.google.common.base.MoreObjects; > +import com.google.common.base.Objects; > +import com.google.common.collect.ImmutableList; > + > +/** > + * A Neutron LBaaS v1 HealthMonitor. > + * > + * @see href="http://docs.openstack.org/api/openstack-network/2.0/content/lbaas_ext_ops_health_monitor.html";>api @jdaggett We were removing external URLs from Javadoc, no? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17817349
Re: [jclouds-labs-openstack] JCLOUDS-611: Neutron LBaaS (version 1) extension request (#146)
> + private Integer timeout; > + @Named("max_retries") > + private Integer maxRetries; > + // Optional attributes that can be updated > + @Named("http_method") > + private HttpMethod httpMethod; > + @Named("url_path") > + private String urlPath; > + @Named("expected_codes") > + private String expectedCodes; > + @Named("admin_state_up") > + private Boolean adminStateUp; > + // Read-only attributes > + @Named("id") > + private String id; > + @Named("pools") Stupid question...do we need `@Named` if the field name is the same? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/146/files#r17817353