You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrea Turli <no...@github.com> on 2014/03/29 11:37:47 UTC
[jclouds-labs-google] initial commit to support GCE LB (#22)
add support targetPools, forwardingRules and httpHealthChecks API
add expectedTests and LiveTests for the above API
You can merge this Pull Request by running:
git pull https://github.com/andreaturli/jclouds-labs-google loadbalancer
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/22
-- Commit Summary --
* initial commit to support GCE LB
-- File Changes --
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineApi.java (40)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/config/GoogleComputeEngineParserModule.java (48)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Address.java (12)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/ForwardingRule.java (193)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/HttpHealthCheck.java (232)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Resource.java (27)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/TargetPool.java (234)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ForwardingRuleApi.java (259)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/HttpHealthCheckApi.java (214)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/TargetPoolApi.java (346)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseForwardingRules.java (64)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseHttpHealthChecks.java (64)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseTargetPools.java (64)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ForwardingRuleApiExpectTest.java (165)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ForwardingRuleApiLiveTest.java (87)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/HttpHealthCheckApiLiveTest.java (62)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/TargetPoolApiExpectTest.java (196)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/TargetPoolApiLiveTest.java (63)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/internal/BaseGoogleComputeEngineApiLiveTest.java (8)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseForwardingRuleListTest.java (62)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseForwardingRuleTest.java (51)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseRegionOperationTest.java (52)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseTargetPoolListTest.java (55)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseTargetPoolTest.java (47)
A google-compute-engine/src/test/resources/forwardingrule_get.json (12)
A google-compute-engine/src/test/resources/forwardingrule_insert.json (4)
A google-compute-engine/src/test/resources/forwardingrule_list.json (19)
A google-compute-engine/src/test/resources/forwardingrule_region_operation.json (14)
A google-compute-engine/src/test/resources/targetpool_addinstance.json (7)
A google-compute-engine/src/test/resources/targetpool_get.json (9)
A google-compute-engine/src/test/resources/targetpool_insert.json (3)
A google-compute-engine/src/test/resources/targetpool_list.json (17)
A google-compute-engine/src/test/resources/targetpool_region_operation.json (14)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/22.patch
https://github.com/jclouds/jclouds-labs-google/pull/22.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
> + *
> + * @param region the region to search in
> + * @param marker marks the beginning of the next list page
> + * @param listOptions listing options
> + * @return a page of the listPage
> + * @see org.jclouds.googlecomputeengine.options.ListOptions
> + * @see org.jclouds.googlecomputeengine.domain.ListPage
> + */
> + @Named("ForwardingRules:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseForwardingRules.class)
> + @Fallback(EmptyIterableWithMarkerOnNotFoundOr404.class)
> + ListPage<ForwardingRule> listAtMarkerInRegion(@PathParam("region") String region, @QueryParam("pageToken") @Nullable String marker, ListOptions listOptions);
Do we really need three methods to get a concrete page? Could you refactor all three into a single one that accepts the ListOptions object? The "pageToken" parameter could be moved to that options object.
Also rename the method to the same name than the one that lists all records. In general prefer method overloading for this kind of list methods.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r15394268
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + * @see <a href="https://developers.google.com/compute/docs/reference/latest/httpHealthChecks"/>
> + */
> +@SkipEncoding({'/', '='})
> +@RequestFilters(OAuthAuthenticator.class)
> +public interface HttpHealthCheckApi {
> +
> + /**
> + * Returns the specified HttpHealthCheck resource.
> + *
> + * @param httpHealthCheck the name of the HttpHealthCheck resource to return.
> + * @return a HttpHealthCheck resource.
> + */
> + @Named("HttpHealthChecks:get")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/global/httpHealthChecks/{httpHealthCheck}")
`/global/httpHealthChecks` seems to be a common prefix to all paths. Can that be moved to the interface?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015482
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> +
> + @Named("HttpHealthChecks:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/global/httpHealthChecks")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseHttpHealthChecks.class)
> + @Transform(ParseHttpHealthChecks.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<HttpHealthCheck> list(ListOptions options);
> +
> + /**
> + * Changes target url for forwarding rule.
> + *
> + * @param httpHealthCheck the name of the HttpHealthCheck resource to update. .
> + * @param httpHealthCheck The URL of the target resource to receive traffic from this forwarding rule.
Confusing/incorrect param names?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015553
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -0,0 +1,193 @@
> +/*
> + * Licensed to jclouds, Inc. (jclouds) under one or more contributor license agreements.
> + * See the NOTICE file distributed with this work for additional information regarding
> + * copyright ownership.
> + * jclouds licenses this file to you under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License. You may obtain a copy of the Licens at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
> + */
Format the header properly?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r15394028
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
> + * @see <a href="https://developers.google.com/compute/docs/reference/latest/forwardingRules"/>
> + */
> +@SkipEncoding({'/', '='})
> +@RequestFilters(OAuthAuthenticator.class)
> +public interface ForwardingRuleApi {
> +
> + /**
> + * Returns the specified ForwardingRule resource.
> + *
> + * @param region the name of the region scoping this request.
> + * @param forwardingRule the name of the ForwardingRule resource to return.
> + * @return a ForwardingRule resource.
> + */
> + @Named("ForwardingRules:get")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
Move the consumes annotations to the interface declaration and remove them from each method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r15394119
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> +
> + /**
> + * Creates a ForwardingRule resource in the specified project and region using the data included in the request.
> + *
> + * @param forwardingRuleName the name of the forwarding rule.
> + * @param targetSelfLink the URL of the target resource to receive the matched traffic. The target resource must live
> + * in the same region as this forwarding rule.
> + * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to
> + * you, and look for the status field.
> + */
> + @Named("ForwardingRules:insert")
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules")
> + @OAuthScopes({COMPUTE_SCOPE})
Sorry - wasn't clear: `@OAuthScopes(COMPUTE_SCOPE)` rather than `@OAuthScopes({COMPUTE_SCOPE})`
Does that make more sense?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18217516
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by ashmrtnz <no...@github.com>.
No problem! I didn't want to have to re-implement what has already been done but I also wanted to make sure that the L7 features would be usable when I finish them up.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-50678727
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> +
> + /**
> + * Creates a HttpHealthCheck resource in the specified project and region using the data included in the request.
> + *
> + * @param httpHealthCheckName the name of the forwarding rule.
> + * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to
> + * you, and look for the status field.
> + */
> + @Named("HttpHealthChecks:insert")
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/global/httpHealthChecks")
> + @OAuthScopes({COMPUTE_SCOPE})
> + @MapBinder(BindToJsonPayload.class)
> + Operation create(@PayloadParam("name") String httpHealthCheckName);
Align with the param name of other methods?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015512
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + private HttpHealthCheckApi api() {
> + return api.getHttpHealthCheckApiForProject(userProject.get());
> + }
> +
> + @Test(groups = "live")
> + public void testInsertHttpHealthCheck() {
> + assertGlobalOperationDoneSucessfully(api().create(HTTP_HEALTH_CHECK_NAME), TIME_WAIT);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testInsertHttpHealthCheck")
> + public void testGetHttpHealthCheck() {
> + HttpHealthCheck httpHealthCheck = api().get(HTTP_HEALTH_CHECK_NAME);
> + assertNotNull(httpHealthCheck);
> + assertEquals(httpHealthCheck.getName(), HTTP_HEALTH_CHECK_NAME);
> + }
> +
Does this actually depend on `get` or on `insert`? Could `testGet` and `testList` be run in parallel?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015780
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrea Turli <no...@github.com>.
> +
> + /**
> + * Returns the specified ForwardingRule resource.
> + *
> + * @param region the name of the region scoping this request.
> + * @param forwardingRule the name of the ForwardingRule resource to return.
> + * @return a ForwardingRule resource.
> + */
> + @Named("ForwardingRules:get")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules/{forwardingRule}")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + ForwardingRule getInRegion(@PathParam("region") String region, @PathParam("forwardingRule") String forwardingRule);
could you please point me to an existing API? Is [NovaApi]( https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/NovaApi.java) a good example? If yes, I'm not sure about the usage of @EndpointParam
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r16079041
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +@Beta
> +public class ForwardingRule extends Resource {
> +
> + private final URI region;
> + private final Optional<String> ipAddress;
> + private final Optional<String> ipProtocol;
> + private final Optional<String> portRange;
> + private final URI target;
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "region", "IPAddress", "IPProtocol",
> + "portRange", "target"
> + })
> + private ForwardingRule(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + URI region, String ipAddress, String ipProtocol, String portRange, URI target) {
Mark nullable fields as `@Nullable`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r15394066
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #51](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/51/) 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-google/pull/22#issuecomment-38992176
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
>
> import java.beans.ConstructorProperties;
> import java.net.URI;
> import java.util.Date;
>
> -import com.google.common.annotations.Beta;
> -import com.google.common.base.Objects;
> -import com.google.common.base.Optional;
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Optional.fromNullable;
> +import static com.google.common.base.Preconditions.checkNotNull;
If possible, please remove import reordering changes such as this one
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015172
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrea Turli <no...@github.com>.
thanks @danbroudy for your help. I've pushed my latest changes which address most of the comments but probably need some more polish. Feel free to continue from here, Happy to review your changes once finished with @ccustine
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-60166842
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by danbroudy <no...@github.com>.
Sounds like a plan. Happy to help!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-60166999
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #786](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/786/) 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-google/pull/22#issuecomment-39005694
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1515](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1515/) 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-google/pull/22#issuecomment-60168270
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @andreaturli! Nice work (and nice documentation!). A few general comments:
* [ ] Remove the `author` tags from the javadoc comments.
* [ ] Some license headers are not properly formatted.
* [ ] Review the domain model classes: add the serialization annotations to the class member variables to match the ones defined in the `@ConstructorProeprties` annotation.
* [ ] In the domain object constructors, add the `@Nullable` annotation to all nullable fields.
* [ ] The APIs seem to always consume json. Move the `@Consumes` annotations at class level and remove them from the methods.
* [ ] There are many methods to list paginated collections. For each one, there should be only two:
* One without parameters that lists all records and returns a `PagedIterable`.
* One with a single `Options` parameter that lists the records with the given criteria (including the page number, if provided), and returns an `IterableWithMarker` (a single page, but getting the `PagedIterable` from it is trivial).
* [ ] There are some APIs that provide many `create` methods. Consider creating the corresponding `CreateOptions` objects and leave only two methods: one without parameters, and one with the options object.
* [ ] Add the expect tests for the `HealthCheckApi`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-50132223
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrea Turli <no...@github.com>.
> +
> + /**
> + * Creates a ForwardingRule resource in the specified project and region using the data included in the request.
> + *
> + * @param forwardingRuleName the name of the forwarding rule.
> + * @param targetSelfLink the URL of the target resource to receive the matched traffic. The target resource must live
> + * in the same region as this forwarding rule.
> + * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to
> + * you, and look for the status field.
> + */
> + @Named("ForwardingRules:insert")
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules")
> + @OAuthScopes({COMPUTE_SCOPE})
sorry @demobox where do you mean?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18215125
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #52](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/52/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-39005673
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + return api.getTargetPoolApiForProject(userProject.get());
> + }
> +
> + @Test(groups = "live")
> + public void testInsertTargetPool() {
> + assertRegionOperationDoneSucessfully(api().createInRegion(DEFAULT_REGION_NAME, TARGETPOOL_NAME), TIME_WAIT);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testInsertTargetPool")
> + public void testGetTargetPool() {
> + TargetPool targetPool = api().getInRegion(DEFAULT_REGION_NAME, TARGETPOOL_NAME);
> + assertNotNull(targetPool);
> + assertEquals(targetPool.getName(), TARGETPOOL_NAME);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testGetTargetPool")
Does this actually depend on `get` or on `insert`? Could `testGet` and `testList` be run in parallel?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015776
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #172](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/172/) 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-google/pull/22#issuecomment-60167924
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by danbroudy <no...@github.com>.
Just wanted to ping this, I would like to cleanup #38 and it would be helpful for this to get merged.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-59810943
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by danbroudy <no...@github.com>.
> + private HttpHealthCheckApi api() {
> + return api.getHttpHealthCheckApiForProject(userProject.get());
> + }
> +
> + @Test(groups = "live")
> + public void testInsertHttpHealthCheck() {
> + assertGlobalOperationDoneSucessfully(api().create(HTTP_HEALTH_CHECK_NAME), TIME_WAIT);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testInsertHttpHealthCheck")
> + public void testGetHttpHealthCheck() {
> + HttpHealthCheck httpHealthCheck = api().get(HTTP_HEALTH_CHECK_NAME);
> + assertNotNull(httpHealthCheck);
> + assertEquals(httpHealthCheck.getName(), HTTP_HEALTH_CHECK_NAME);
> + }
> +
nice catch, updating
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r19366520
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + @Transform(ParseTargetPools.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<TargetPool> listInRegion(@PathParam("region") String region);
> +
> + @Named("TargetPools:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/targetPools")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseTargetPools.class)
> + @Transform(ParseTargetPools.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<TargetPool> listInRegion(@PathParam("region") String region, ListOptions options);
> +
> + /**
> + * Adds instance url to targetPool.
> this is interesting as it is how Google describe it
Thanks for the link. From the Google doc, it seems as though the thing that you pass as a parameter is **not** a name, it's the actual instance URL. So our parameter name should not be `instanceName`, in that case?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18227937
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> @@ -0,0 +1,12 @@
> +{
> +"kind": "compute#forwardingRule",
> +"id": "6732523704970219884",
> +"creationTimestamp": "2014-01-08T06:51:10.809-08:00",
> +"name": "test-forwarding-rule",
> +"region": "https://www.googleapis.com/compute/v1/projects/myproject/regions/europe-west1",
> +"IPAddress": "23.251.129.77",
> +"IPProtocol": "TCP",
> +"portRange": "1-65535",
> +"target": "https://www.googleapis.com/compute/v1/projects/myproject/regions/europe-west1/targetPools/test-target-pool",
> +"selfLink": "https://www.googleapis.com/compute/v1/projects/myproject/regions/europe-west1/forwardingRules/test-forwarding-rule"
> +}
[minor] Here and below: indent JSON?
@zack-shoylev What were the settings you were using again..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015817
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import javax.ws.rs.core.MediaType;
> +
> +import java.net.URI;
> +
> +import static org.jclouds.googlecomputeengine.GoogleComputeEngineConstants.COMPUTE_READONLY_SCOPE;
> +import static org.jclouds.googlecomputeengine.GoogleComputeEngineConstants.COMPUTE_SCOPE;
> +import static org.testng.Assert.assertEquals;
> +import static org.testng.Assert.assertTrue;
> +import static org.testng.AssertJUnit.assertNull;
> +
> +/**
> + * @author Andrea Turli
> + */
> +@Test(groups = "unit")
> +public class ForwardingRuleApiExpectTest extends BaseGoogleComputeEngineApiExpectTest {
Still using expect tests? Would it make sense to start with MWS at some point here..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015704
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + /**
> + * @see org.jclouds.googlecomputeengine.domain.ForwardingRule#getTarget()
> + */
> + public Builder target(URI target) {
> + this.target = target;
> + return this;
> + }
> +
> + @Override
> + protected Builder self() {
> + return this;
> + }
> +
> + public ForwardingRule build() {
> + return new ForwardingRule(super.id, super.creationTimestamp, super.selfLink, super.name,
> + super.description, region, ipAddress, ipProtocol,portRange, target);
[minor] Space before `portRange`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015207
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by ashmrtnz <no...@github.com>.
Is there any intention on merging this into master? All the code looks good and the tests passed when I pulled this branch and ran them. I was also working on creating L7 load balancing features to add to jclouds.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-50046343
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> +import com.google.common.collect.ImmutableSet;
> +
> +import java.beans.ConstructorProperties;
> +import java.net.URI;
> +import java.util.Date;
> +import java.util.Set;
> +
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Optional.fromNullable;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * Represents an TargetPool resource.
> + *
> + * @author Andrea Turli
> + * @see <a href="https://developers.google.com/compute/docs/reference/latest/targetPools#resource"/>
Remove external URLs from Javadoc
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015239
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @param region the region to search in
> + * @param marker marks the beginning of the next list page
> + * @param listOptions listing options
> + * @return a page of the listPage
> + * @see org.jclouds.googlecomputeengine.options.ListOptions
> + * @see org.jclouds.googlecomputeengine.domain.ListPage
> + */
> + @Named("ForwardingRules:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseForwardingRules.class)
> + @Fallback(EmptyIterableWithMarkerOnNotFoundOr404.class)
> + ListPage<ForwardingRule> listAtMarkerInRegion(@PathParam("region") String region, @QueryParam("pageToken") @Nullable String marker, ListOptions listOptions);
+1
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015415
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
>
> /**
> - * @author David Alves
> + * @author David Alves, Andrea Turli
Remove the author tags from all classes.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r15394018
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrea Turli <no...@github.com>.
> +
> + /**
> + * Creates a ForwardingRule resource in the specified project and region using the data included in the request.
> + *
> + * @param forwardingRuleName the name of the forwarding rule.
> + * @param targetSelfLink the URL of the target resource to receive the matched traffic. The target resource must live
> + * in the same region as this forwarding rule.
> + * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to
> + * you, and look for the status field.
> + */
> + @Named("ForwardingRules:insert")
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules")
> + @OAuthScopes({COMPUTE_SCOPE})
Completely, thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18217736
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + * region as this pool.
> + */
> + public Set<URI> getInstances() {
> + return instances;
> + }
> +
> + /**
> + * @return Defines the session affinity option, determines the hash method that Google Compute Engine uses to
> + * distribute traffic.
> + */
> + public Optional<String> getSessionAffinity() {
> + return sessionAffinity;
> + }
> +
> + /**
> + * @return This field is applicable only when the target pool is serving a forwarding rule as the primary pool.
Make the description not be the `@return` comment? I.e. just:
```
This field...
@return the failover ratio
```
or so?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015303
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -88,6 +98,15 @@
> GlobalOperationApi getGlobalOperationApiForProject(@PathParam("project") String projectName);
We also nuked all of the `*ForBlahAndBlah` methods for further simplification. This would become `getGlobalOperationApi(@PathParam("project") String projectName);`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r16143900
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + public void testGetForwardingRuleResponseIs2xx() throws Exception {
> + HttpRequest get = HttpRequest
> + .builder()
> + .method("GET")
> + .endpoint("https://www.googleapis" +
> + ".com/compute/v1/projects/myproject/regions/us-central1/forwardingRules/test-forwarding-rule")
> + .addHeader("Accept", "application/json")
> + .addHeader("Authorization", "Bearer " + TOKEN).build();
> +
> + HttpResponse operationResponse = HttpResponse.builder().statusCode(200)
> + .payload(payloadFromResource("/forwardingrule_get.json")).build();
> +
> + ForwardingRuleApi api = requestsSendResponses(requestForScopes(COMPUTE_READONLY_SCOPE),
> + TOKEN_RESPONSE, get, operationResponse).getForwardingRuleApiForProject("myproject");
> +
> + assertEquals(api.getInRegion("us-central1", "test-forwarding-rule"),
We have `assertj` in jclouds-labs-google now - use that style instead (here and below)? @andrewgaul can probably help if there are questions.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015731
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Adrian Cole <no...@github.com>.
> .build();
> }
>
> @Override
> public Image getImage(String id) {
> - return MoreObjects.firstNonNull(api.getImageApiForProject(userProject.get()).get(id),
> - MoreObjects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> - api.getImageApiForProject(CENTOS_PROJECT).get(id)));
> + return MoreObjects.firstNonNull(api.getImageApi(userProject.get()).get(id),
Careful. You'll need to do a s/MoreObjects/Objects/ scrub, since we reverted guava 18!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r19370153
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by danbroudy <no...@github.com>.
> + return api.getTargetPoolApiForProject(userProject.get());
> + }
> +
> + @Test(groups = "live")
> + public void testInsertTargetPool() {
> + assertRegionOperationDoneSucessfully(api().createInRegion(DEFAULT_REGION_NAME, TARGETPOOL_NAME), TIME_WAIT);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testInsertTargetPool")
> + public void testGetTargetPool() {
> + TargetPool targetPool = api().getInRegion(DEFAULT_REGION_NAME, TARGETPOOL_NAME);
> + assertNotNull(targetPool);
> + assertEquals(targetPool.getName(), TARGETPOOL_NAME);
> + }
> +
> + @Test(groups = "live", dependsOnMethods = "testGetTargetPool")
updating
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r19366512
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + @Transform(ParseTargetPools.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<TargetPool> listInRegion(@PathParam("region") String region);
> +
> + @Named("TargetPools:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/targetPools")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseTargetPools.class)
> + @Transform(ParseTargetPools.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<TargetPool> listInRegion(@PathParam("region") String region, ListOptions options);
> +
> + /**
> + * Adds instance url to targetPool.
The parameters only allow you to add a _name_, not a URL?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015591
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
Haven't reviewed it all, but some recurring comments (mainly from @nacx's review - thanks ;-)) so far:
* properly format the license header
* remove `@author` tags and external URLs from Javadoc
* move common `@Produces` and `@Consumes` annotations to the interface
* move shared `@Path` prefixes to the interface, if that actually work (@nacx: ?)
* see if the multiple `list` variants in many of the interface can be consolidated, as per @nacx's suggestion
* consistent use of parameter naming: some methods have `foo` and others have `fooName`, but in both cases the Javadoc suggests it's actually the name we're talking about
* Javadoc for some methods suggests that the value being passed is a URL (e.g. for health checks), but the parameter type is `String`. Use URI or so instead?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-56775894
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + @Transform(ParseForwardingRules.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<ForwardingRule> listInRegion(@PathParam("region") String region);
> +
> + @Named("ForwardingRules:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseForwardingRules.class)
> + @Transform(ParseForwardingRules.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<ForwardingRule> listInRegion(@PathParam("region") String region, ListOptions options);
> +
> + /**
> + * Changes target url for forwarding rule.
"Changes the target URL for a forwarding rule"
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015441
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "host", "requestPath", "port",
> + "checkIntervalSec", "timeoutSec", "unhealthyThreshold", "healthyThreshold"
> + })
> + private HttpHealthCheck(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + String host, String requestPath, int port, int checkIntervalSec,
> + int timeoutSec, int unhealthyThreshold, int healthyThreshold) {
> + super(Kind.HTTP_HEALTH_CHECK, id, creationTimestamp, selfLink, name, description);
> + this.host = fromNullable(host);
> + this.requestPath = fromNullable(requestPath);
> + this.port = fromNullable(port);
> + this.checkIntervalSec = fromNullable(checkIntervalSec);
> + this.timeoutSec = fromNullable(timeoutSec);
> + this.unhealthyThreshold = fromNullable(unhealthyThreshold);
> + this.healthyThreshold = fromNullable(healthyThreshold);
See @nacx's comments from above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015220
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + /**
> + * Returns the specified ForwardingRule resource.
> + *
> + * @param region the name of the region scoping this request.
> + * @param forwardingRule the name of the ForwardingRule resource to return.
> + * @return a ForwardingRule resource.
> + */
> + @Named("ForwardingRules:get")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules/{forwardingRule}")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + ForwardingRule getInRegion(@PathParam("region") String region, @PathParam("forwardingRule") String forwardingRule);
A general comment: the approach that is used in other providers is to put the "region" parameter at "delegate" level. You often see `getInstanceApiForRegion`, etc. Could you move the region parameter to the delegate to follow that approach?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r15394157
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + @MapBinder(BindToJsonPayload.class)
> + Operation create(@PayloadParam("name") String httpHealthCheckName);
> +
> + /**
> + * Creates a HttpHealthCheck resource in the specified project and region using the data included in the request.
> + *
> + * @param httpHealthCheckName the name of the forwarding rule.
> + * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to
> + * you, and look for the status field.
> + */
> + @Named("HttpHealthChecks:insert")
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/global/httpHealthChecks")
> + @OAuthScopes({COMPUTE_SCOPE})
Here and elsewhere: can just be `@OAuthScopes(COMPUTE_SCOPE)`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015528
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Adrian Cole <no...@github.com>.
> .build();
> }
>
> @Override
> public Image getImage(String id) {
> - return MoreObjects.firstNonNull(api.getImageApiForProject(userProject.get()).get(id),
> - MoreObjects.firstNonNull(api.getImageApiForProject(DEBIAN_PROJECT).get(id),
> - api.getImageApiForProject(CENTOS_PROJECT).get(id)));
> + return MoreObjects.firstNonNull(api.getImageApi(userProject.get()).get(id),
though maybe just a rebase will take care of it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r19370179
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by ashmrtnz <no...@github.com>.
@andreaturli have you had a chance to review the suggestions nacx put up? As I mentioned I'm working on implementing L7 load balancing features and some of those features require http health checks when they are created.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-50677721
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by danbroudy <no...@github.com>.
I think it would make sense to do the api renames (ie. from getZoneApiForProject to getZoneApi) in a separate pull request because it significantly increases the number of changed files and makes it difficult to track the changes that really matter in this already large PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-60420116
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Chris Custine <no...@github.com>.
I spoke to @andreaturli today and he is hoping to work on finalizing this next week so that we can merge this and #38 before graduating to jclouds/jclouds.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-56577803
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -79,6 +80,15 @@
> FirewallApi getFirewallApiForProject(@PathParam("project") String projectName);
>
> /**
> + * Provides access to ForwardingRule features
> + *
> + * @param projectName the name of the project
> + */
> + @Delegate
> + @Path("/projects/{project}")
@andreaturli Oh you are right, I missed that method!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r16185211
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Optional.fromNullable;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Andrea Turli
> + */
> +@Beta
> +public class ForwardingRule extends Resource {
> +
> + private final URI region;
> + private final Optional<String> ipAddress;
> + private final Optional<String> ipProtocol;
> + private final Optional<String> portRange;
> + private final URI target;
Add the serialization annotations here, so each field matches the name in the `@ConstructorProperties` annotation.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r15394045
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + private final Set<URI> healthChecks;
> + private final Set<URI> instances;
> + private final Optional<String> sessionAffinity;
> + private final float failoverRatio;
> + private final Optional<String> backupPool;
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "region", "healthChecks", "instances",
> + "sessionAffinity", "failoverRatio", "backupPool"
> + })
> + private TargetPool(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + URI region, Set<URI> healthChecks, Set<URI> instances, String sessionAffinity, float failoverRatio, String backupPool) {
> + super(Kind.TARGET_POOL, id, creationTimestamp, selfLink, name, description);
> + this.region = checkNotNull(region, "region of %s", name);
> + this.healthChecks = healthChecks == null ? ImmutableSet.<URI>of() : healthChecks;
> + this.instances = instances == null ? ImmutableSet.<URI>of() : instances;
Should we be making immutable copies of the input sets here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015255
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -79,6 +80,15 @@
> FirewallApi getFirewallApiForProject(@PathParam("project") String projectName);
>
> /**
> + * Provides access to ForwardingRule features
> + *
> + * @param projectName the name of the project
> + */
> + @Delegate
> + @Path("/projects/{project}")
After going through all of the OpenStack APIs recently, you could move this `@Path` annotation up to the interface level.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r16143726
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrea Turli <no...@github.com>.
Sorry @ashmrtnz I still need to address them. I should have time next week, though, if it is fine for you.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-50678291
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrea Turli <no...@github.com>.
> + @Transform(ParseTargetPools.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<TargetPool> listInRegion(@PathParam("region") String region);
> +
> + @Named("TargetPools:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/targetPools")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseTargetPools.class)
> + @Transform(ParseTargetPools.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<TargetPool> listInRegion(@PathParam("region") String region, ListOptions options);
> +
> + /**
> + * Adds instance url to targetPool.
thanks @demobox this is interesting as it is how Google describe it
https://cloud.google.com/compute/docs/reference/latest/targetPools/addInstance
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18215781
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> +
> + /**
> + * @return This field is applicable only when the target pool is serving a forwarding rule as the primary pool.
> + * The value of the a float between [0, 1]. If set, backupPool must also be set. Together,
> + * they define the fallback behavior of the primary target pool. If the ratio of the healthy VMs in the primary
> + * pool is at or below this number, traffic arriving at the load-balanced IP will be directed to the backup pool.
> + * In case where failoverRatio is not set or all the VMs in the backup pool are unhealthy,
> + * the traffic will be directed back to the primary pool in the force mode, where traffic will be spread to the
> + * healthy VMs with the best effort, or to all VMs when no VM is healthy.
> + */
> + public float getFailoverRatio() {
> + return failoverRatio;
> + }
> +
> + /**
> + * @return This field is applicable only when the target pool is serving a forwarding rule as the primary pool.
See previous comment
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015308
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + * @author Andrea Turli
> + * @see <a href="https://developers.google.com/compute/docs/reference/latest/httpHealthChecks"/>
> + */
> +@SkipEncoding({'/', '='})
> +@RequestFilters(OAuthAuthenticator.class)
> +public interface HttpHealthCheckApi {
> +
> + /**
> + * Returns the specified HttpHealthCheck resource.
> + *
> + * @param httpHealthCheck the name of the HttpHealthCheck resource to return.
> + * @return a HttpHealthCheck resource.
> + */
> + @Named("HttpHealthChecks:get")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
This can be moved to the interface
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015457
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Adrian Cole <no...@github.com>.
closed via #69
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-61203817
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + * not been set.
> + *
> + * @param marker marks the beginning of the next list page
> + * @param listOptions listing options
> + * @return a page of the listPage
> + * @see org.jclouds.googlecomputeengine.options.ListOptions
> + * @see org.jclouds.googlecomputeengine.domain.ListPage
> + */
> + @Named("HttpHealthChecks:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/global/httpHealthChecks")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseHttpHealthChecks.class)
> + @Fallback(EmptyIterableWithMarkerOnNotFoundOr404.class)
> + ListPage<HttpHealthCheck> listAtMarker(@QueryParam("pageToken") @Nullable String marker, ListOptions listOptions);
See @nacx's comment about merging these three methods in the previous interface
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015538
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> I spoke to @andreaturli today and he is hoping to work on finalizing this next week
Thanks, @ccustine and @andreaturli! Looks like this needs a rebase first of all ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22#issuecomment-56586080
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + private final Set<URI> healthChecks;
> + private final Set<URI> instances;
> + private final Optional<String> sessionAffinity;
> + private final float failoverRatio;
> + private final Optional<String> backupPool;
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "region", "healthChecks", "instances",
> + "sessionAffinity", "failoverRatio", "backupPool"
> + })
> + private TargetPool(String id, Date creationTimestamp, URI selfLink, String name, String description,
> + URI region, Set<URI> healthChecks, Set<URI> instances, String sessionAffinity, float failoverRatio, String backupPool) {
> + super(Kind.TARGET_POOL, id, creationTimestamp, selfLink, name, description);
> + this.region = checkNotNull(region, "region of %s", name);
> + this.healthChecks = healthChecks == null ? ImmutableSet.<URI>of() : healthChecks;
> + this.instances = instances == null ? ImmutableSet.<URI>of() : instances;
Can these two ever be `null`? The builder will always call them with a non-null value...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015350
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrea Turli <no...@github.com>.
> @@ -79,6 +80,15 @@
> FirewallApi getFirewallApiForProject(@PathParam("project") String projectName);
>
> /**
> + * Provides access to ForwardingRule features
> + *
> + * @param projectName the name of the project
> + */
> + @Delegate
> + @Path("/projects/{project}")
@jdaggett unfortunately I think I can't as
```
@Delegate
ProjectApi getProjectApi();
```
doesn't need @Path annotation :(
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r16160343
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1514](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1514/) FAILURE
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-google/pull/22#issuecomment-60166809
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @param region the region to search in
> + * @param marker marks the beginning of the next list page
> + * @param listOptions listing options
> + * @return a page of the listPage
> + * @see org.jclouds.googlecomputeengine.options.ListOptions
> + * @see org.jclouds.googlecomputeengine.domain.ListPage
> + */
> + @Named("TargetPools:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/targetPools")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseTargetPools.class)
> + @Fallback(EmptyIterableWithMarkerOnNotFoundOr404.class)
> + ListPage<TargetPool> listAtMarkerInRegion(@PathParam("region") String region, @QueryParam("pageToken") @Nullable String marker, ListOptions listOptions);
See @nacx's comment from previous
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015573
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> +
> + /**
> + * Creates a ForwardingRule resource in the specified project and region using the data included in the request.
> + *
> + * @param forwardingRuleName the name of the forwarding rule.
> + * @param targetSelfLink the URL of the target resource to receive the matched traffic. The target resource must live
> + * in the same region as this forwarding rule.
> + * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to
> + * you, and look for the status field.
> + */
> + @Named("ForwardingRules:insert")
> + @POST
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules")
> + @OAuthScopes({COMPUTE_SCOPE})
Don't need `{` and `}` here and below?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015494
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
> + @Path("/regions/{region}/forwardingRules")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseForwardingRules.class)
> + @Transform(ParseForwardingRules.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<ForwardingRule> listInRegion(@PathParam("region") String region);
> +
> + @Named("ForwardingRules:list")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @ResponseParser(ParseForwardingRules.class)
> + @Transform(ParseForwardingRules.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<ForwardingRule> listInRegion(@PathParam("region") String region, ListOptions options);
This method shouldn't be needed. Let's have a method that returns all records without parameters, and one that returns the iterable with marker, and gets the ListOptions object as a parameter.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r15394295
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrew Phillips <no...@github.com>.
> + * @param httpHealthCheck The URL of the target resource to receive traffic from this forwarding rule.
> + * It must live in the same region as this forwarding rule.
> + *
> + * @return an Operation resource. To check on the status of an operation, poll the Operations resource returned to
> + * you, and look for the status field.
> + */
> + @Named("HttpHealthChecks:patch")
> + @PATCH
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/global/httpHealthChecks/{httpHealthCheck}")
> + @OAuthScopes(COMPUTE_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @MapBinder(BindToJsonPayload.class)
> + @Nullable
> + Operation patch(@PathParam("httpHealthCheck") String httpHealthCheck, @PayloadParam("httpHealthCheck")
> + HttpHealthCheck httpHealthChecks);
[minor] Indent
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18015556
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by danbroudy <no...@github.com>.
> @@ -0,0 +1,12 @@
> +{
> +"kind": "compute#forwardingRule",
> +"id": "6732523704970219884",
> +"creationTimestamp": "2014-01-08T06:51:10.809-08:00",
> +"name": "test-forwarding-rule",
> +"region": "https://www.googleapis.com/compute/v1/projects/myproject/regions/europe-west1",
> +"IPAddress": "23.251.129.77",
> +"IPProtocol": "TCP",
> +"portRange": "1-65535",
> +"target": "https://www.googleapis.com/compute/v1/projects/myproject/regions/europe-west1/targetPools/test-target-pool",
> +"selfLink": "https://www.googleapis.com/compute/v1/projects/myproject/regions/europe-west1/forwardingRules/test-forwarding-rule"
> +}
updating
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r19430028
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + /**
> + * Returns the specified ForwardingRule resource.
> + *
> + * @param region the name of the region scoping this request.
> + * @param forwardingRule the name of the ForwardingRule resource to return.
> + * @return a ForwardingRule resource.
> + */
> + @Named("ForwardingRules:get")
> + @GET
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Path("/regions/{region}/forwardingRules/{forwardingRule}")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + @Nullable
> + ForwardingRule getInRegion(@PathParam("region") String region, @PathParam("forwardingRule") String forwardingRule);
The endpoint param provides a function that returns a URI given a value. In the nova example, it returns the endpoint given the region. jclouds has a set of default functions to do that, that delegate to different region/zone suppliers. You can have a look at the [RegionToEndpoint](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/location/functions/RegionToEndpoint.java) function to see how it works and to the [keystone suppliers package](https://github.com/jclouds/jclouds/tree/master/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/suppliers) to see how the suppliers where the function delegates are implemented in openstack.
This is needed, in general, when you need to perform an API call to get the endpoint (such as getting them from the keystone service catalog, or some complex work. In this case, if I'm not wrong, you can just move the `@Path` annotation to the "delegating" interface and all the methods in this one will already be scoped to that path.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r16397997
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #171](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/171/) FAILURE
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-google/pull/22#issuecomment-60166693
Re: [jclouds-labs-google] initial commit to support GCE LB (#22)
Posted by Andrea Turli <no...@github.com>.
> +
> +import javax.ws.rs.core.MediaType;
> +
> +import java.net.URI;
> +
> +import static org.jclouds.googlecomputeengine.GoogleComputeEngineConstants.COMPUTE_READONLY_SCOPE;
> +import static org.jclouds.googlecomputeengine.GoogleComputeEngineConstants.COMPUTE_SCOPE;
> +import static org.testng.Assert.assertEquals;
> +import static org.testng.Assert.assertTrue;
> +import static org.testng.AssertJUnit.assertNull;
> +
> +/**
> + * @author Andrea Turli
> + */
> +@Test(groups = "unit")
> +public class ForwardingRuleApiExpectTest extends BaseGoogleComputeEngineApiExpectTest {
it is probably a good idea, but not for this PR :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/22/files#r18215853