You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Bayer <no...@github.com> on 2013/07/20 01:32:05 UTC
[jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
No new APIs added other than Zone/GlobalOperations, which are needed for zone-scoping changes. But all existing tests pass.
You can merge this Pull Request by running:
git pull https://github.com/abayer/jclouds-labs jclouds-192-for-1.6.x
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs/pull/16
-- Commit Summary --
* JCLOUDS-192. Move GCE to API v1beta15 - no new APIs added other than Zone/GlobalOperations, which are needed for zone-scoping changes.
-- File Changes --
M google-compute-engine/pom.xml (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineApi.java (43)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineApiMetadata.java (32)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineConstants.java (5)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineService.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceAdapter.java (132)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/config/GoogleComputeEngineServiceContextModule.java (122)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/GoogleComputeEngineImageToImage.java (9)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/InstanceInZoneToNodeMetadata.java (17)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/MachineTypeInZoneToHardware.java (86)
D google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/MachineTypeToHardware.java (57)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/strategy/CreateNodesWithGroupEncodedIntoNameThenAddToSet.java (8)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/config/GoogleComputeEngineHttpApiModule.java (54)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/config/GoogleComputeEngineParserModule.java (15)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/config/OAuthModuleWithoutTypeAdapters.java (12)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/AbstractDisk.java (133)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Disk.java (65)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Firewall.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Image.java (205)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Instance.java (168)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/InstanceInZone.java (60)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/InstanceTemplate.java (66)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Kernel.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/MachineType.java (107)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/MachineTypeInZone.java (55)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Network.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Operation.java (57)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Project.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Resource.java (8)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/SlashEncodedIds.java (86)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Zone.java (4)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/DiskApi.java (74)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/FirewallApi.java (34)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/GlobalOperationApi.java (226)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ImageApi.java (78)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java (211)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/KernelApi.java (16)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/MachineTypeApi.java (43)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/NetworkApi.java (29)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ProjectApi.java (6)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ZoneApi.java (2)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ZoneOperationApi.java (79)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/BaseToPagedIterable.java (4)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/BaseWithZoneToPagedIterable.java (74)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseDisks.java (10)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseFirewalls.java (3)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseGlobalOperations.java (9)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseImages.java (3)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseInstances.java (12)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseKernels.java (3)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseMachineTypes.java (9)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseNetworks.java (3)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseZoneOperations.java (68)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/functions/internal/ParseZones.java (3)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/handlers/InstanceBinder.java (8)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/options/AttachDiskOptions.java (116)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/options/DeprecateOptions.java (132)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/options/FirewallOptions.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/options/ListOptions.java (2)
R google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/predicates/GlobalOperationDonePredicate.java (8)
A google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/predicates/ZoneOperationDonePredicate.java (72)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/PageSystemExpectTest.java (8)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceExpectTest.java (159)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceLiveTest.java (10)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/DiskApiExpectTest.java (34)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/DiskApiLiveTest.java (12)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/FirewallApiExpectTest.java (24)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/FirewallApiLiveTest.java (16)
R google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/GlobalOperationApiExpectTest.java (66)
R google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/GlobalOperationApiLiveTest.java (24)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ImageApiExpectTest.java (21)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiExpectTest.java (80)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java (59)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/KernelApiExpectTest.java (8)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/MachineTypeApiExpectTest.java (26)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/MachineTypeApiLiveTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/NetworkApiExpectTest.java (14)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/NetworkApiLiveTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ProjectApiExpectTest.java (26)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ProjectApiLiveTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ZoneApiExpectTest.java (6)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ZoneOperationApiExpectTest.java (198)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ZoneOperationApiLiveTest.java (95)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/handlers/GoogleComputeEngineErrorHandlerTest.java (2)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/internal/BaseGoogleComputeEngineApiLiveTest.java (64)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseDiskListTest.java (8)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseDiskTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseFirewallListTest.java (6)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseFirewallTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseImageListTest.java (12)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseImageTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseInstanceListTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseInstanceTest.java (14)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseKernelListTest.java (8)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseKernelTest.java (2)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseMachineTypeListTest.java (12)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseMachineTypeTest.java (7)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseNetworkListTest.java (2)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseNetworkTest.java (2)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseOperationListTest.java (6)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseOperationTest.java (7)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseProjectTest.java (2)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseZoneListTest.java (8)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseZoneTest.java (2)
M google-compute-engine/src/test/resources/disk_get.json (16)
M google-compute-engine/src/test/resources/disk_insert.json (2)
M google-compute-engine/src/test/resources/disk_list.json (30)
M google-compute-engine/src/test/resources/firewall_get.json (52)
M google-compute-engine/src/test/resources/firewall_insert.json (2)
M google-compute-engine/src/test/resources/firewall_list.json (106)
A google-compute-engine/src/test/resources/global_operation.json (15)
A google-compute-engine/src/test/resources/global_operation_list.json (22)
M google-compute-engine/src/test/resources/image_get.json (26)
M google-compute-engine/src/test/resources/image_insert.json (5)
M google-compute-engine/src/test/resources/image_list.json (44)
M google-compute-engine/src/test/resources/image_list_multiple_page_1.json (106)
M google-compute-engine/src/test/resources/image_list_multiple_page_2.json (98)
M google-compute-engine/src/test/resources/image_list_single_page.json (100)
M google-compute-engine/src/test/resources/instance_add_access_config.json (8)
M google-compute-engine/src/test/resources/instance_get.json (101)
M google-compute-engine/src/test/resources/instance_insert.json (2)
M google-compute-engine/src/test/resources/instance_insert_simple.json (2)
M google-compute-engine/src/test/resources/instance_list.json (111)
A google-compute-engine/src/test/resources/instance_list_central1b_empty.json (6)
M google-compute-engine/src/test/resources/instance_serial_port.json (4)
M google-compute-engine/src/test/resources/kernel.json (12)
M google-compute-engine/src/test/resources/kernel_list.json (42)
M google-compute-engine/src/test/resources/logback.xml (41)
M google-compute-engine/src/test/resources/machinetype.json (39)
M google-compute-engine/src/test/resources/machinetype_list.json (80)
A google-compute-engine/src/test/resources/machinetype_list_central1b.json (43)
A google-compute-engine/src/test/resources/machinetype_list_central1b_empty.json (6)
M google-compute-engine/src/test/resources/network_get.json (16)
M google-compute-engine/src/test/resources/network_list.json (30)
M google-compute-engine/src/test/resources/operation.json (10)
M google-compute-engine/src/test/resources/operation_error.json (22)
M google-compute-engine/src/test/resources/operation_list.json (42)
M google-compute-engine/src/test/resources/project.json (130)
A google-compute-engine/src/test/resources/region_operation.json (16)
A google-compute-engine/src/test/resources/region_operation_list.json (23)
A google-compute-engine/src/test/resources/snapshot_get.json (11)
A google-compute-engine/src/test/resources/tag_insert.json (1)
M google-compute-engine/src/test/resources/zone_get.json (30)
M google-compute-engine/src/test/resources/zone_list.json (76)
A google-compute-engine/src/test/resources/zone_list_short.json (24)
A google-compute-engine/src/test/resources/zone_operation.json (16)
A google-compute-engine/src/test/resources/zone_operation_error.json (25)
A google-compute-engine/src/test/resources/zone_operation_list.json (23)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/16.patch
https://github.com/jclouds/jclouds-labs/pull/16.diff
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -44,17 +43,8 @@
> })
> private Disk(String id, Date creationTimestamp, URI selfLink, String name, String description,
Annotation again out of date. Needed or can be removed?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362447
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #221](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/221/) 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/pull/16#issuecomment-21655108
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
>
> protected Operation(String id, Date creationTimestamp, URI selfLink, String name, String description,
> URI targetLink, String targetId, String clientOperationId, Status status,
> String statusMessage, String user, Integer progress, Date insertTime, Date startTime,
> Date endTime, Integer httpErrorStatusCode, String httpErrorMessage, String operationType,
> - List<Error> errors) {
> + List<Error> errors, URI region, URI zone) {
Mark as `@Nullable`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362624
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + public enum DiskType {
> + SCRATCH,
> + PERSISTENT
> + }
> +
> + public enum DiskMode {
> + READ_WRITE,
> + READ_ONLY
> + }
> +
> + private DiskType type;
> + private DiskMode mode;
> + private URI source;
> + private String deviceName;
> +
> + // TODO decide whether to do something with the index and boot options
Since I guess we're _not_ doing anything with them right now, change comment to something like "index and boot options not currently supported"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362858
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
>
> InstanceTemplate instanceTemplate = InstanceTemplate.builder()
> - .forMachineType(machineType);
> + .forMachineType(hardware.getUri());
No need for a `null` check here any more?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362005
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #22](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/22/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21450936
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import org.jclouds.collect.IterableWithMarker;
> +import org.jclouds.collect.PagedIterable;
> +import org.jclouds.collect.PagedIterables;
> +import org.jclouds.googlecomputeengine.domain.ListPage;
> +import org.jclouds.googlecomputeengine.options.ListOptions;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.rest.InvocationContext;
> +import org.jclouds.rest.internal.GeneratedHttpRequest;
> +
> +import com.google.common.annotations.Beta;
> +import com.google.common.base.Function;
> +import com.google.common.base.Optional;
> +
> +/**
> + * @author Adrian Cole
Sorry, I meant the `@author` comment ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379626
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -349,14 +356,127 @@ public Builder fromInstance(Instance in) {
> .networkInterfaces(in.getNetworkInterfaces())
> .disks(in.getDisks())
> .metadata(in.getMetadata())
> - .serviceAccoutns(in.getServiceAccounts());
> + .serviceAccounts(in.getServiceAccounts());
> + }
> + }
> +
> + /**
> + * Tags for an instance, with their fingerprint.
> + */
> + public static class Tags {
> + private final String fingerprint;
> + private final Set<String> tags;
Rename to `tagValues` or `values` or something like that? `tags.getValues()` seems a little clearer than `tags.getTags()`..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362538
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
>
> InstanceTemplate instanceTemplate = InstanceTemplate.builder()
> - .forMachineType(machineType);
> + .forMachineType(hardware.getUri());
> +
> + if (hardware.getUserMetadata().get("imageSpaceGb").equals("0")) {
> + // The machine needs a boot disk - create a 1GB drive for this purpose
> + // TODO need to delete it at end!
> + Operation operation = api.getDiskApiForProject(userProject.get()).createInZone(name + "-disk", 10,
> + template.getLocation().getId());
> + waitOperationDone(operation);
> + instanceTemplate.addDisk(InstanceTemplate.PersistentDisk.Mode.READ_WRITE, operation.getTargetLink());
> + }
No, and frankly, I'm not sure if I want to keep this here. @richardcloudsoft did something similar in his v1beta14 version, and I aped it. The reasoning is to deal with the f1-micro and g1-small machine types, which have no image space by default. Let me think about this one a bit and get back to it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377189
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -349,14 +356,127 @@ public Builder fromInstance(Instance in) {
> .networkInterfaces(in.getNetworkInterfaces())
> .disks(in.getDisks())
> .metadata(in.getMetadata())
> - .serviceAccoutns(in.getServiceAccounts());
> + .serviceAccounts(in.getServiceAccounts());
> + }
> + }
> +
> + /**
> + * Tags for an instance, with their fingerprint.
> + */
> + public static class Tags {
> + private final String fingerprint;
> + private final Set<String> tags;
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378753
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -44,17 +43,8 @@
> })
> private Disk(String id, Date creationTimestamp, URI selfLink, String name, String description,
No, that's right, so far as I can see.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378651
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Richard Downer <no...@github.com>.
>
> InstanceTemplate instanceTemplate = InstanceTemplate.builder()
> - .forMachineType(machineType);
> + .forMachineType(hardware.getUri());
> +
> + if (hardware.getUserMetadata().get("imageSpaceGb").equals("0")) {
> + // The machine needs a boot disk - create a 1GB drive for this purpose
> + // TODO need to delete it at end!
> + Operation operation = api.getDiskApiForProject(userProject.get()).createInZone(name + "-disk", 10,
> + template.getLocation().getId());
> + waitOperationDone(operation);
> + instanceTemplate.addDisk(InstanceTemplate.PersistentDisk.Mode.READ_WRITE, operation.getTargetLink());
> + }
I changed my version to do something different, as this was causing far more trouble than it was worth. I experimented with changing listHardwareProfile to filter out machine types that did not have disks, on the basis that the compute service cannot create them, but this caused problems when non-jclouds instances existed using one of the filtered-out types - jclouds threw exceptions when it couldn't find the hardware type in use by an instance.
In the end I did something with the supportImage() property of Hardware instances - I made it so the diskless Hardwares refused to support *any* image. This is true, because a diskless machine type needs to be given a disk, and when a disk is given, it's not permitted to give an image. See https://github.com/richardcloudsoft/jclouds-labs/commit/d554fd65495b5181a686b89e61fe9c483a9426c6
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5394042
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +/**
> + * Options for attaching disks to instances.
> + *
> + * @author Andrew Bayer
> + * @see <a href="https://developers.google.com/compute/docs/reference/latest/instances/attachDisk"/>
> + */
> +public class AttachDiskOptions {
> +
> + public enum DiskType {
> + SCRATCH,
> + PERSISTENT
> + }
> +
> + public enum DiskMode {
> + READ_WRITE,
> + READ_ONLY
Let's leave as-is. They both compile, after all ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380334
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by David Ribeiro Alves <no...@github.com>.
> return input.get() != null;
> }
> }, operationCompleteCheckTimeout, operationCompleteCheckInterval, MILLISECONDS).apply(instance);
>
> - return new NodeAndInitialCredentials<Instance>(instance.get(), name, credentials);
> + if (options.getTags().size() > 0) {
> + Operation tagsOperation = api.getInstanceApiForProject(userProject.get()).setTagsInZone(template.getLocation().getId(),
> + name, options.getTags(), instance.get().getTags().getFingerprint());
> +
> + waitOperationDone(tagsOperation);
> +
> + retry(new Predicate<AtomicReference<Instance>>() {
> + @Override
> + public boolean apply(AtomicReference<Instance> input) {
> + input.set(api.getInstanceApiForProject(userProject.get()).getInZone(template.getLocation().getId(),
> + name));
IMO it couldn't since the timeout check is done after the predicate returns. I.e. AFAIK retry won't timeout in the middle of a predicate execution but if the predicate execution does not complete within the timeout.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5412546
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + @Override
> + public PagedIterable<T> apply(ListPage<T> input) {
> + if (input.nextMarker() == null)
> + return PagedIterables.of(input);
> +
> + Optional<Object> project = tryFind(request.getCaller().get().getArgs(), instanceOf(String.class));
> +
> + Optional<Object> zone = tryFind(request.getCaller().get().getArgs(), instanceOf(String.class));
> +
> + Optional<Object> listOptions = tryFind(request.getInvocation().getArgs(), instanceOf(ListOptions.class));
> +
> + assert project.isPresent() : String.format("programming error, method %s should have a string param for the "
> + + "project", request.getCaller().get().getInvokable());
> +
> + assert zone.isPresent() : String.format("programming error, method %s should have a string param for the "
> + + "zone", request.getCaller().get().getInvokable());
'cos I cut and pasted it and this was what was there previously. My default is to leave as is. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379205
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> .operatingSystem(image.getOperatingSystem())
> .status(toPortableNodeStatus.get(input.getStatus()))
> - .tags(input.getTags())
> + .tags(input.getTags().getTags())
Yeah, we'll address this below.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378006
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -0,0 +1 @@
> +{"items":["aTag"],"fingerprint":"abcd"}
Format nicely like the other ones? ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5363027
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> */
> - public String getStatus() {
> - return status;
> + @Override
> + public boolean equals(Object obj) {
> + if (this == obj) return true;
> + if (obj == null || getClass() != obj.getClass()) return false;
> + Disk that = Disk.class.cast(obj);
> + return equal(this.kind, that.kind)
> + && equal(this.name, that.name)
> + && equal(this.zone, that.zone);
`status` doesn't matter?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362470
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -170,34 +223,57 @@ public Image getImage(String id) {
> }
>
> @Override
> - public Instance getNode(String name) {
> - return api.getInstanceApiForProject(userProject.get()).get(name);
> + public InstanceInZone getNode(String name) {
> + SlashEncodedIds slashEncodedIds = SlashEncodedIds.fromSlashEncoded(name);
Here, I'd actually like to keep the SlashEncodedIds.fromSlashEncoded - it reads better to me.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377537
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + @Override
> + public PagedIterable<T> apply(ListPage<T> input) {
> + if (input.nextMarker() == null)
> + return PagedIterables.of(input);
> +
> + Optional<Object> project = tryFind(request.getCaller().get().getArgs(), instanceOf(String.class));
> +
> + Optional<Object> zone = tryFind(request.getCaller().get().getArgs(), instanceOf(String.class));
> +
> + Optional<Object> listOptions = tryFind(request.getInvocation().getArgs(), instanceOf(ListOptions.class));
> +
> + assert project.isPresent() : String.format("programming error, method %s should have a string param for the "
> + + "project", request.getCaller().get().getInvokable());
> +
> + assert zone.isPresent() : String.format("programming error, method %s should have a string param for the "
> + + "zone", request.getCaller().get().getInvokable());
Any reason for `assert` here over `checkState`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362832
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -65,6 +65,13 @@
> String version = on(".").join(limit(skip(splits, 1), splits.size() - 2));
> osBuilder.version(version);
>
> + if (image.getDeprecated().isPresent()) {
> + if (image.getDeprecated().get().getState().isPresent()) {
> + builder.userMetadata(ImmutableMap.<String,String>builder()
Turns out this still needed the first isPresent check, and I changed `getState().get()` to `getState().orNull()` to be safe there too.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379741
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> }
>
> @Provides
> @Singleton
> @Memoized
> public Supplier<Map<URI, ? extends Hardware>> provideHardwaresMap(
> - final Supplier<Set<? extends Hardware>> hardwares) {
> - return new Supplier<Map<URI, ? extends Hardware>>() {
> - @Override
> - public Map<URI, ? extends Hardware> get() {
> - return uniqueIndex(hardwares.get(), new Function<Hardware, URI>() {
> - @Override
> - public URI apply(Hardware input) {
> - return input.getUri();
> - }
> - });
> - }
> - };
> + AtomicReference<AuthorizationException> authException,
> + final Supplier<Set<? extends Hardware>> hardwares,
If it's `hardwares` here (and not e.g. `hardwareSet`), how about `hardwareMap` -> `hardwares` above?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362272
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> image(template.getImage());
> - tags(template.getTags());
> +// tags(template.getTags());
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378536
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
>
> InstanceTemplate instanceTemplate = InstanceTemplate.builder()
> - .forMachineType(machineType);
> + .forMachineType(hardware.getUri());
We've got a null check on hardware above - my gut reaction was that was sufficient. But you're right. Adding a checkNotNull on hardware.getUri()
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377123
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> +
> + }
> +
> + /**
> + * Deprecation information for an image
> + */
> + public static class Deprecated {
> + private final Optional<String> state;
> + private final Optional<URI> replacement;
> + private final Optional<String> deprecated;
> + private final Optional<String> obsolete;
> + private final Optional<String> deleted;
> +
> + @ConstructorProperties({"state", "replacement", "deprecated", "obsolete", "deleted"})
> + public Deprecated(String state, URI replacement, String deprecated, String obsolete,
> + String deleted) {
Weirdly common in our code these days. Why, I do not know.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380753
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> - public Iterable<Instance> listNodes() {
> - return api.getInstanceApiForProject(userProject.get()).list().concat();
> + public Iterable<InstanceInZone> listNodes() {
> + return FluentIterable.from(zones.get().values()).transformAndConcat(new Function<Location, ImmutableSet<InstanceInZone>>() {
> + @Override
> + public ImmutableSet<InstanceInZone> apply(final Location input) {
> + return api.getInstanceApiForProject(userProject.get()).listInZone(input.getId()).concat()
> + .transform(new Function<Instance, InstanceInZone>() {
> +
> + @Override
> + public InstanceInZone apply(Instance arg0) {
> + return new InstanceInZone(arg0, input.getId());
> + }
> + }).toSet();
> + }
> + }).toSet();
I think this was another situation like the above (equality check screwup) so I'll check the imperative version later as well.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377592
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + @Path("/global/operations/{operation}")
> + @OAuthScopes(COMPUTE_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + void delete(@PathParam("operation") String operationName);
> +
> + /**
> + * @see org.jclouds.googlecomputeengine.features.GlobalOperationApi#listAtMarker(String, org.jclouds.googlecomputeengine.options.ListOptions)
> + */
> + @Named("GlobalOperations:list")
> + @GET
> + @Path("/global/operations")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @Consumes(MediaType.APPLICATION_JSON)
> + @ResponseParser(ParseGlobalOperations.class)
> + @Fallback(EmptyIterableWithMarkerOnNotFoundOr404.class)
> + ListPage<Operation> listFirstPage();
What's the difference between this and `listAtMarker` with a `null` marker (sorry, didn't look in the impl)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362737
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by David Ribeiro Alves <no...@github.com>.
>
> import javax.inject.Singleton;
>
> -import static com.google.common.base.Preconditions.checkNotNull;
> -import static java.lang.String.format;
> +import org.jclouds.compute.options.TemplateOptions;
> +
> +import com.google.common.base.Function;
> +import com.google.common.collect.ImmutableMap;
>
> /**
spurious import reorder
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5412910
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + @Override
> + public PagedIterable<T> apply(ListPage<T> input) {
> + if (input.nextMarker() == null)
> + return PagedIterables.of(input);
> +
> + Optional<Object> project = tryFind(request.getCaller().get().getArgs(), instanceOf(String.class));
> +
> + Optional<Object> zone = tryFind(request.getCaller().get().getArgs(), instanceOf(String.class));
> +
> + Optional<Object> listOptions = tryFind(request.getInvocation().getArgs(), instanceOf(ListOptions.class));
> +
> + assert project.isPresent() : String.format("programming error, method %s should have a string param for the "
> + + "project", request.getCaller().get().getInvokable());
> +
> + assert zone.isPresent() : String.format("programming error, method %s should have a string param for the "
> + + "zone", request.getCaller().get().getInvokable());
Fine
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380322
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + @Path("/global/operations/{operation}")
> + @OAuthScopes(COMPUTE_SCOPE)
> + @Fallback(NullOnNotFoundOr404.class)
> + void delete(@PathParam("operation") String operationName);
> +
> + /**
> + * @see org.jclouds.googlecomputeengine.features.GlobalOperationApi#listAtMarker(String, org.jclouds.googlecomputeengine.options.ListOptions)
> + */
> + @Named("GlobalOperations:list")
> + @GET
> + @Path("/global/operations")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @Consumes(MediaType.APPLICATION_JSON)
> + @ResponseParser(ParseGlobalOperations.class)
> + @Fallback(EmptyIterableWithMarkerOnNotFoundOr404.class)
> + ListPage<Operation> listFirstPage();
Honestly? Hell if I know. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379029
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> +import com.google.inject.Inject;
> +
> +/**
> + * Transforms a google compute domain specific machine type to a generic Hardware object.
> + *
> + * @author David Alves
> + */
> +public class MachineTypeInZoneToHardware implements Function<MachineTypeInZone, Hardware> {
> +
> + private final Supplier<Map<URI, ? extends Location>> locations;
> +
> + @Inject
> + public MachineTypeInZoneToHardware(@Memoized Supplier<Map<URI, ? extends Location>> locations) {
> + this.locations = locations;
> + }
> + @Override
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378017
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -65,10 +55,16 @@ public URI getZone() {
> }
>
> /**
> - * @return the status of disk creation.
> + * {@inheritDoc}
I have no idea. I've seen it around enough in the code that I used it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378662
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + return self();
> + }
> +
> + /**
> + * @see org.jclouds.googlecomputeengine.domain.AbstractDisk#getStatus()
> + */
> + public T status(String status) {
> + this.status = status;
> + return self();
> + }
> +
> +
> + @Override
> + protected T self() {
> + return self();
> + }
Will this not cause an endless loop?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362426
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> </logger>
>
> <logger name="jclouds.headers">
> - <level value="DEBUG"/>
> - <appender-ref ref="WIREFILE"/>
> + <level value="TRACE" />
> + <appender-ref ref="WIREFILE" />
> + </logger>
> +
> + <logger name="jclouds.compute">
> + <level value="TRACE" />
> + <appender-ref ref="COMPUTEFILE" />
> + </logger>
> +
> + <logger name="jclouds.ssh">
> + <level value="TRACE" />
> + <appender-ref ref="SSHFILE" />
`additivity="false"` for all the loggers that go to a file? Or do we want that percolating out to the standard logs at trace level too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5363019
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> return input.get() != null;
> }
> }, operationCompleteCheckTimeout, operationCompleteCheckInterval, MILLISECONDS).apply(instance);
>
> - return new NodeAndInitialCredentials<Instance>(instance.get(), name, credentials);
> + if (options.getTags().size() > 0) {
> + Operation tagsOperation = api.getInstanceApiForProject(userProject.get()).setTagsInZone(template.getLocation().getId(),
> + name, options.getTags(), instance.get().getTags().getFingerprint());
> +
> + waitOperationDone(tagsOperation);
> +
> + retry(new Predicate<AtomicReference<Instance>>() {
> + @Override
> + public boolean apply(AtomicReference<Instance> input) {
> + input.set(api.getInstanceApiForProject(userProject.get()).getInZone(template.getLocation().getId(),
> + name));
What happens if the retry is interrupted _before_ the `return` check, assuming that's even possible. Could it cause a problem that the retry will fail but that `input` _has_, in fact, been successfully set?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362101
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -349,14 +356,127 @@ public Builder fromInstance(Instance in) {
> .networkInterfaces(in.getNetworkInterfaces())
> .disks(in.getDisks())
> .metadata(in.getMetadata())
> - .serviceAccoutns(in.getServiceAccounts());
> + .serviceAccounts(in.getServiceAccounts());
> + }
> + }
> +
> + /**
> + * Tags for an instance, with their fingerprint.
> + */
> + public static class Tags {
> + private final String fingerprint;
> + private final Set<String> tags;
> +
> + @ConstructorProperties({"fingerprint", "items"})
> + public Tags(String fingerprint, Set<String> tags) {
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378884
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + *
> + * 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.googlecomputeengine.domain;
> +
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Adam Lowe
> + */
> +public class MachineTypeInZone extends SlashEncodedIds {
Yeah, it's definitely not a provider-domain class. Like I said, I'm just aping how things are done in openstack-nova and ec2 - I have no problem with rearranging it. Lemme think about it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380834
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> +/**
> + * Options for attaching disks to instances.
> + *
> + * @author Andrew Bayer
> + * @see <a href="https://developers.google.com/compute/docs/reference/latest/instances/attachDisk"/>
> + */
> +public class AttachDiskOptions {
> +
> + public enum DiskType {
> + SCRATCH,
> + PERSISTENT
> + }
> +
> + public enum DiskMode {
> + READ_WRITE,
> + READ_ONLY
I have no idea in the world. Your call. Lemme know.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379244
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
>
> InstanceTemplate instanceTemplate = InstanceTemplate.builder()
> - .forMachineType(machineType);
> + .forMachineType(hardware.getUri());
> +
> + if (hardware.getUserMetadata().get("imageSpaceGb").equals("0")) {
> + // The machine needs a boot disk - create a 1GB drive for this purpose
> + // TODO need to delete it at end!
> + Operation operation = api.getDiskApiForProject(userProject.get()).createInZone(name + "-disk", 10,
> + template.getLocation().getId());
> + waitOperationDone(operation);
> + instanceTemplate.addDisk(InstanceTemplate.PersistentDisk.Mode.READ_WRITE, operation.getTargetLink());
> + }
Is there a test covering this case (sorry, didn't scroll through the whole PR to check)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362016
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @OAuthScopes(COMPUTE_READONLY_SCOPE)
> @ResponseParser(ParseDisks.class)
> @Transform(ParseDisks.ToPagedIterable.class)
> @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> - PagedIterable<Disk> list(ListOptions options);
> + PagedIterable<Disk> listInZone(@PathParam("zone") String zone, ListOptions options);
> +
> + /**
> + * TODO live and expect tests for DiskApi#createSnapshot
TODO still valid? Does it belong in the Javadoc?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362654
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @Override
> protected GoogleComputeEngineApi create(Properties props, Iterable<Module> modules) {
> GoogleComputeEngineApi api = super.create(props, modules);
> + URI imageUri = api.getImageApiForProject("google")
> + .list(new ListOptions.Builder().filter("name eq gcel.*"))
> + .concat()
> + .filter(new Predicate<Image>() {
> + @Override
> + public boolean apply(Image input) {
> + if (input.getDeprecated().isPresent()) {
> + if (input.getDeprecated().get().getState().isPresent()) {
Turns out I didn't read your bit well enough - it excluded anything that was marked as DEPRECATED, but actually, what we want is anything that either has no deprecation info at all *or* if it does have deprecation info, it's gotta have DEPRECATED and not OBSOLETE or DELETED. Whoopsie daisy. Fixed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380952
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> return input.get() != null;
> }
> }, operationCompleteCheckTimeout, operationCompleteCheckInterval, MILLISECONDS).apply(instance);
>
> - return new NodeAndInitialCredentials<Instance>(instance.get(), name, credentials);
> + if (options.getTags().size() > 0) {
> + Operation tagsOperation = api.getInstanceApiForProject(userProject.get()).setTagsInZone(template.getLocation().getId(),
> + name, options.getTags(), instance.get().getTags().getFingerprint());
> +
> + waitOperationDone(tagsOperation);
> +
> + retry(new Predicate<AtomicReference<Instance>>() {
> + @Override
> + public boolean apply(AtomicReference<Instance> input) {
> + input.set(api.getInstanceApiForProject(userProject.get()).getInZone(template.getLocation().getId(),
> + name));
I'll be honest - no idea. I just copied this from the same logic above for the actual creation. I'm not even sure it's strictly needed, but went with a better safe than sorry approach - I'm quite open to tweaks.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377354
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
>
> /**
> * Retrieves the list of machine type resources available to the specified project.
> * By default the list as a maximum size of 100, if no options are provided or ListOptions#getMaxResults() has not
> * been set.
> *
> - *
> + * @param zone The zone to list in.
For consistency, something like `the name of the zone to list in`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362815
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +public class MachineTypeInZoneToHardware implements Function<MachineTypeInZone, Hardware> {
> +
> + private final Supplier<Map<URI, ? extends Location>> locations;
> +
> + @Inject
> + public MachineTypeInZoneToHardware(@Memoized Supplier<Map<URI, ? extends Location>> locations) {
> + this.locations = locations;
> + }
> + @Override
> + public Hardware apply(final MachineTypeInZone input) {
> + Location location = checkNotNull(getOnlyElement(filter(locations.get().values(), new Predicate<Location>() {
> + @Override
> + public boolean apply(Location l) {
> + return l.getId().equals(input.getMachineType().getZone());
> + }
> + })), "location for %s", input.getMachineType().getZone());
Break into two statements?
```
Iterable<...> locations = filter(...)
Location location = checkNotNull(getOnlyElement(locations), ...)
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362335
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @author David Alves
> + */
> +public class ZoneOperationApiLiveTest extends BaseGoogleComputeEngineApiLiveTest {
> +
> + private static final String METADATA_ITEM_KEY = "operationLiveTestTestProp";
> + private static final String METADATA_ITEM_VALUE = "operationLiveTestTestValue";
> + private Operation addOperation;
> + private Operation deleteOperation;
> +
> + private ZoneOperationApi api() {
> + return api.getZoneOperationApiForProject(userProject.get());
> + }
> +
> +
> +// @Test(groups = "live")
Use `ignored = true` (or whatever the proper TestNG attribute is)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380363
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by David Ribeiro Alves <no...@github.com>.
> }
>
> @Provides
> @Singleton
> @Memoized
> public Supplier<Map<URI, ? extends Hardware>> provideHardwaresMap(
> - final Supplier<Set<? extends Hardware>> hardwares) {
> - return new Supplier<Map<URI, ? extends Hardware>>() {
> - @Override
> - public Map<URI, ? extends Hardware> get() {
> - return uniqueIndex(hardwares.get(), new Function<Hardware, URI>() {
> - @Override
> - public URI apply(Hardware input) {
> - return input.getUri();
> - }
> - });
> - }
> - };
> + AtomicReference<AuthorizationException> authException,
> + final Supplier<Set<? extends Hardware>> hardwares,
since getUri() comes from the top level ResourceMetadata isn't is possible to consolidate these set-to-map transforms
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5412893
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #218](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/218/) 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/pull/16#issuecomment-21512396
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #23](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/23/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21512543
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
>
> InstanceTemplate instanceTemplate = InstanceTemplate.builder()
> - .forMachineType(machineType);
> + .forMachineType(hardware.getUri());
> +
> + if (hardware.getUserMetadata().get("imageSpaceGb").equals("0")) {
> + // The machine needs a boot disk - create a 1GB drive for this purpose
> + // TODO need to delete it at end!
> + Operation operation = api.getDiskApiForProject(userProject.get()).createInZone(name + "-disk", 10,
> + template.getLocation().getId());
> + waitOperationDone(operation);
> + instanceTemplate.addDisk(InstanceTemplate.PersistentDisk.Mode.READ_WRITE, operation.getTargetLink());
> + }
'k, my tweaks look good - verifying live tests now, but in expect tests, they're properly skipping the f1-micro I added to the machineType list. In the 1.7 version, I'd like to come up with a robust way of using the machineTypes with imageSpaceGb==0, but for 1.6.2, this workaround seems sufficient.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5402890
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> +
> + }
> +
> + /**
> + * Deprecation information for an image
> + */
> + public static class Deprecated {
> + private final Optional<String> state;
> + private final Optional<URI> replacement;
> + private final Optional<String> deprecated;
> + private final Optional<String> obsolete;
> + private final Optional<String> deleted;
> +
> + @ConstructorProperties({"state", "replacement", "deprecated", "obsolete", "deleted"})
> + public Deprecated(String state, URI replacement, String deprecated, String obsolete,
> + String deleted) {
Rather than go Optional everywhere? I honestly don't know what the Right Way is. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378705
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -44,17 +43,8 @@
> })
> private Disk(String id, Date creationTimestamp, URI selfLink, String name, String description,
Ah, OK, didn't see `status` but that's probably on the next line ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379531
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #28](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/28/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21655182
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
>
> InstanceTemplate instanceTemplate = InstanceTemplate.builder()
> - .forMachineType(machineType);
> + .forMachineType(hardware.getUri());
> +
> + if (hardware.getUserMetadata().get("imageSpaceGb").equals("0")) {
> + // The machine needs a boot disk - create a 1GB drive for this purpose
> + // TODO need to delete it at end!
> + Operation operation = api.getDiskApiForProject(userProject.get()).createInZone(name + "-disk", 10,
> + template.getLocation().getId());
> + waitOperationDone(operation);
> + instanceTemplate.addDisk(InstanceTemplate.PersistentDisk.Mode.READ_WRITE, operation.getTargetLink());
> + }
Oh, I like that. I'll tweak this to ditch the above and instead use the imageSpaceGb to determine what the supportsImage returns. New change incoming! =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5402481
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> instanceTemplate.serviceAccounts(options.getServiceAccounts());
> instanceTemplate.image(checkNotNull(template.getImage().getUri(), "image URI is null"));
>
> Operation operation = api.getInstanceApiForProject(userProject.get())
> - .createInZone(name, instanceTemplate, template.getLocation().getId());
> + .createInZone(template.getLocation().getId(), name, instanceTemplate);
Yeah, that was probably a cockup - I wanted to always have zone first in a *InZone method's arguments, but backwards compatibility is probably more important. Switching.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377212
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -170,34 +223,57 @@ public Image getImage(String id) {
> }
>
> @Override
> - public Instance getNode(String name) {
> - return api.getInstanceApiForProject(userProject.get()).get(name);
> + public InstanceInZone getNode(String name) {
Is this marked `@Nullable` at the API level (sorry, didn't check the rest of the PR)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362249
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + @ResponseParser(ParseGlobalOperations.class)
> + @Transform(ParseGlobalOperations.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<Operation> list(ListOptions listOptions);
> +
> + /**
> + * @see org.jclouds.googlecomputeengine.features.GlobalOperationApi#aggregatedListAtMarker(String, org.jclouds.googlecomputeengine.options.ListOptions)
> + */
> + @Named("GlobalOperations:aggregatedList")
> + @GET
> + @Path("/aggregated/operations")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @Consumes(MediaType.APPLICATION_JSON)
> + @ResponseParser(ParseGlobalOperations.class)
> + @Fallback(EmptyIterableWithMarkerOnNotFoundOr404.class)
> + ListPage<Operation> aggregatedListFirstPage();
Would you ever need to call `aggregatedListFirstPage(ListOptions)` or `listFirstPage(ListOptions)`? If so, how would you do that?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362768
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> +
> +import org.jclouds.collect.IterableWithMarker;
> +import org.jclouds.collect.PagedIterable;
> +import org.jclouds.collect.PagedIterables;
> +import org.jclouds.googlecomputeengine.domain.ListPage;
> +import org.jclouds.googlecomputeengine.options.ListOptions;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.rest.InvocationContext;
> +import org.jclouds.rest.internal.GeneratedHttpRequest;
> +
> +import com.google.common.annotations.Beta;
> +import com.google.common.base.Function;
> +import com.google.common.base.Optional;
> +
> +/**
> + * @author Adrian Cole
Had to do a variant class to deal with three arguments rather than two. But if there's a better way to do it than this, I am all ears.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379184
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> </logger>
>
> <logger name="jclouds.headers">
> - <level value="DEBUG"/>
> - <appender-ref ref="WIREFILE"/>
> + <level value="TRACE" />
> + <appender-ref ref="WIREFILE" />
> + </logger>
> +
> + <logger name="jclouds.compute">
> + <level value="TRACE" />
> + <appender-ref ref="COMPUTEFILE" />
> + </logger>
> +
> + <logger name="jclouds.ssh">
> + <level value="TRACE" />
> + <appender-ref ref="SSHFILE" />
Perhaps leave commented-out, then? For developers to uncomment when needed? Otherwise there'll be a _lot_ of logging...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380391
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + private final Supplier<Map<URI, ? extends Location>> zones;
> +
> + @Inject
> + ZoneOperationDonePredicate(GoogleComputeEngineApi api, @UserProject Supplier<String> project,
> + @Memoized Supplier<Map<URI, ? extends Location>> zones) {
> + this.api = api;
> + this.project = project;
> + this.zones = zones;
> + }
> +
> + @Override
> + public boolean apply(AtomicReference<Operation> input) {
> + checkNotNull(input, "input");
> + Operation current = api.getZoneOperationApiForProject(project.get())
> + .getInZone(zones.get().get(input.get().getZone().get()).getId(),
> + input.get().getName());
sigh, I know, right?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379380
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> .name(input.getName())
> .providerId(input.getId())
> .hostname(input.getName())
> .imageId(image.getId())
> .location(checkNotNull(locations.get().get(input.getZone()), "location for %s", input.getZone()))
> .hardware(checkNotNull(hardwares.get().get(input.getMachineType()), "hardware type for %s",
> - input.getMachineType().toString()))
> + input.getMachineType().toString()))
Fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378001
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
FYI, pushed to 1.6.x.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21736906
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + *
> + * @author David Alves
> + */
> +public class ZoneOperationApiLiveTest extends BaseGoogleComputeEngineApiLiveTest {
> +
> + private static final String METADATA_ITEM_KEY = "operationLiveTestTestProp";
> + private static final String METADATA_ITEM_VALUE = "operationLiveTestTestValue";
> + private Operation addOperation;
> + private Operation deleteOperation;
> +
> + private ZoneOperationApi api() {
> + return api.getZoneOperationApiForProject(userProject.get());
> + }
> +
> +
> +// @Test(groups = "live")
Nah, I'm gonna want to rewrite those tests completely, so I'd rather just drop them from 1.6.2.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380911
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -170,34 +223,57 @@ public Image getImage(String id) {
> }
>
> @Override
> - public Instance getNode(String name) {
> - return api.getInstanceApiForProject(userProject.get()).get(name);
> + public InstanceInZone getNode(String name) {
> + SlashEncodedIds slashEncodedIds = SlashEncodedIds.fromSlashEncoded(name);
Static import?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362245
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> - return compose(new Function<Credentials, String>() {
> - public String apply(Credentials in) {
> - checkState(in.identity.indexOf("@") != 1, "identity should be in project_id@developer.gserviceaccount.com" +
> - " format");
> - return Iterables.get(Splitter.on("@").split(in.identity), 0);
> - }
> - }, creds);
> + public Supplier<String> supplyProject(@org.jclouds.location.Provider final Supplier<Credentials> creds,
> + final GoogleComputeEngineApi api,
> + AtomicReference<AuthorizationException> authException,
> + @Named(PROPERTY_SESSION_INTERVAL) long seconds) {
> + return MemoizedRetryOnTimeOutButNotOnAuthorizationExceptionSupplier.create(authException,
> + compose(new Function<Credentials, String>() {
> + public String apply(Credentials in) {
> + checkState(in.identity.indexOf("@") != 1, "identity should be in project_id@developer.gserviceaccount.com" +
> + " format");
Why the string split across two lines? Just move the whole message onto the next line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362400
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> return input.get() != null;
> }
> }, operationCompleteCheckTimeout, operationCompleteCheckInterval, MILLISECONDS).apply(instance);
>
> - return new NodeAndInitialCredentials<Instance>(instance.get(), name, credentials);
> + if (options.getTags().size() > 0) {
> + Operation tagsOperation = api.getInstanceApiForProject(userProject.get()).setTagsInZone(template.getLocation().getId(),
> + name, options.getTags(), instance.get().getTags().getFingerprint());
> +
> + waitOperationDone(tagsOperation);
> +
> + retry(new Predicate<AtomicReference<Instance>>() {
> + @Override
> + public boolean apply(AtomicReference<Instance> input) {
> + input.set(api.getInstanceApiForProject(userProject.get()).getInZone(template.getLocation().getId(),
> + name));
Let's just leave things as they are - was more a curiosity question from my side, too.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379934
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @Override
> protected GoogleComputeEngineApi create(Properties props, Iterable<Module> modules) {
> GoogleComputeEngineApi api = super.create(props, modules);
> + URI imageUri = api.getImageApiForProject("google")
> + .list(new ListOptions.Builder().filter("name eq gcel.*"))
> + .concat()
> + .filter(new Predicate<Image>() {
> + @Override
> + public boolean apply(Image input) {
> + if (input.getDeprecated().isPresent()) {
> + if (input.getDeprecated().get().getState().isPresent()) {
Combine these two? I.e.
```
if (input.getDeprecated().isPresent() && input.getDeprecated().get().getState().isPresent()) {
...
}
```
?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362932
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + .location(location)
> + .name(input.getMachineType().getName())
> + .hypervisor("kvm")
> + .processor(new Processor(input.getMachineType().getGuestCpus(), 1.0))
> + .providerId(input.getMachineType().getId())
> + .ram(input.getMachineType().getMemoryMb())
> + .uri(input.getMachineType().getSelfLink())
> + .userMetadata(ImmutableMap.of("imageSpaceGb", Integer.toString(input.getMachineType().getImageSpaceGb())))
> + .volumes(collectVolumes(input.getMachineType()))
> + .build();
> + }
> +
> + private Iterable<Volume> collectVolumes(MachineType input) {
> + ImmutableSet.Builder<Volume> volumes = ImmutableSet.builder();
> + for (MachineType.ScratchDisk disk : input.getScratchDisks()) {
> + volumes.add(new VolumeImpl(null, Volume.Type.LOCAL, new Integer(disk.getDiskGb()).floatValue(), null, true,
Good question. That's actually lingering from the original version - switching to VolumeBuilder.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378330
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -0,0 +1 @@
> +{"items":["aTag"],"fingerprint":"abcd"}
> Something went weird and expect tests started failing with pretty-formatted json
Hm...but all the other ones still worked? Smell?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380408
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -65,6 +65,13 @@
> String version = on(".").join(limit(skip(splits, 1), splits.size() - 2));
> osBuilder.version(version);
>
> + if (image.getDeprecated().isPresent()) {
> + if (image.getDeprecated().get().getState().isPresent()) {
> + builder.userMetadata(ImmutableMap.<String,String>builder()
Works for me!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377981
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
Ok, looks like the live test issues were due to wait times being too short in a few places. Double-triple-checking, but I think we're good again.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21507213
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> super(Kind.IMAGE, id, creationTimestamp, selfLink, name, description);
> this.sourceType = checkNotNull(sourceType, "sourceType of %s", name);
> this.preferredKernel = fromNullable(preferredKernel);
> - this.rawDisk = checkNotNull(rawDisk, "rawDisk of %s", name); ;
> + this.rawDisk = checkNotNull(rawDisk, "rawDisk of %s", name);
> + this.deprecated = fromNullable(deprecated);
> + ;
Remove?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362494
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #208](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/208/) 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/pull/16#issuecomment-21283890
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @OAuthScopes({COMPUTE_SCOPE})
> @MapBinder(InstanceBinder.class)
> - Operation createInZone(@PayloadParam("name") String instanceName,
> - @PayloadParam("template") InstanceTemplate template,
> - @PayloadParam("zone") String zone);
> + Operation createInZone(@PathParam("zone") String zone,
> + @PayloadParam("name") String instanceName,
> + @PayloadParam("template") InstanceTemplate template);
See comment way above about shuffling these args around. I can see the desire for consistency, though.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362790
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +
> + }
> +
> + /**
> + * Deprecation information for an image
> + */
> + public static class Deprecated {
> + private final Optional<String> state;
> + private final Optional<URI> replacement;
> + private final Optional<String> deprecated;
> + private final Optional<String> obsolete;
> + private final Optional<String> deleted;
> +
> + @ConstructorProperties({"state", "replacement", "deprecated", "obsolete", "deleted"})
> + public Deprecated(String state, URI replacement, String deprecated, String obsolete,
> + String deleted) {
I think `Optional`s are fine, but the constructor args aren't `Optional`s and I haven't seen that in the code too often, but might be looking at a skewed sample. I'm just wondering whether a user looking at the Javadoc or an IDE here would know that all of the properties are allowed to be `null`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379602
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> return Uris.uriBuilder(endpoint.get()).appendPath("/projects/").appendPath(userProject.get()).appendPath
> - ("/machineTypes/").appendPath(input).build();
Move `appendPath` from the previous line down?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362404
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #220](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/220/) 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/pull/16#issuecomment-21638440
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> image(template.getImage());
> - tags(template.getTags());
> +// tags(template.getTags());
Remove commented out lines?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362410
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> */
> - public String getStatus() {
> - return status;
> + @Override
> + public boolean equals(Object obj) {
> + if (this == obj) return true;
> + if (obj == null || getClass() != obj.getClass()) return false;
> + Disk that = Disk.class.cast(obj);
> + return equal(this.kind, that.kind)
> + && equal(this.name, that.name)
> + && equal(this.zone, that.zone);
For equality? No, it shouldn't.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378669
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + *
> + * @author David Alves
> + */
> +public class ZoneOperationApiLiveTest extends BaseGoogleComputeEngineApiLiveTest {
> +
> + private static final String METADATA_ITEM_KEY = "operationLiveTestTestProp";
> + private static final String METADATA_ITEM_VALUE = "operationLiveTestTestValue";
> + private Operation addOperation;
> + private Operation deleteOperation;
> +
> + private ZoneOperationApi api() {
> + return api.getZoneOperationApiForProject(userProject.get());
> + }
> +
> +
> +// @Test(groups = "live")
It needs more work, basically. In a perfect world I refactor the test to actually do something that'll create zone operations - but there's nothing as trivial as setting global metadata, so I avoided it. I'll just nuke the live test for now, since in practice, it's covered quite nicely by the instance/disk/machinetype live tests. In 1.7, I'll come up with something better.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379568
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> - public Iterable<MachineType> listHardwareProfiles() {
> - return api.getMachineTypeApiForProject(userProject.get()).list().concat();
> + public Iterable<MachineTypeInZone> listHardwareProfiles() {
> + return FluentIterable.from(zones.get().values())
> + .transformAndConcat(new Function<Location, ImmutableSet<MachineType>>() {
> + @Override
> + public ImmutableSet<MachineType> apply(Location input) {
> + return api.getMachineTypeApiForProject(userProject.get()).listInZone(input.getId()).concat().toSet();
> + }
> + }).transform(new Function<MachineType, MachineTypeInZone>() {
> +
> + @Override
> + public MachineTypeInZone apply(MachineType arg0) {
> + return new MachineTypeInZone(arg0, arg0.getZone());
> + }
> + }).toSet();
Good question. In earlier iterations, pre-MachineTypeInZone (not that that particular change made a difference - just meant adding the second transform), I had:
```java
public Iterable<MachineType> listHardwareProfiles() {
ImmutableSet.Builder<MachineType> builder = ImmutableSet.builder();
for (final Location zone : zones.get().values()) {
builder.addAll(api.getMachineTypeApiForProject(userProject.get()).listInZone(zone.getId()).concat());
}
return builder.build();
}
```
...and for some reason, I kept getting just the machineTypes from the *first* zone. But now I think that was due to another problem that I caught later (equality check on MachineType didn't take zone into equation). Once I deal with the various and sundry sure things that need to be cleaned up, I'll revisit this and see how it works with the imperative approach.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377528
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -0,0 +1 @@
> +{"items":["aTag"],"fingerprint":"abcd"}
Annoyingly, no. Something went weird and expect tests started failing with pretty-formatted json. I was confused, but hey.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379604
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> +public class MachineTypeInZoneToHardware implements Function<MachineTypeInZone, Hardware> {
> +
> + private final Supplier<Map<URI, ? extends Location>> locations;
> +
> + @Inject
> + public MachineTypeInZoneToHardware(@Memoized Supplier<Map<URI, ? extends Location>> locations) {
> + this.locations = locations;
> + }
> + @Override
> + public Hardware apply(final MachineTypeInZone input) {
> + Location location = checkNotNull(getOnlyElement(filter(locations.get().values(), new Predicate<Location>() {
> + @Override
> + public boolean apply(Location l) {
> + return l.getId().equals(input.getMachineType().getZone());
> + }
> + })), "location for %s", input.getMachineType().getZone());
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378098
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -76,47 +91,54 @@ protected void bindErrorHandlers() {
> @Provides
> @Singleton
> @UserProject
> - public Supplier<String> supplyProject(@org.jclouds.location.Provider final Supplier<Credentials> creds) {
> - return compose(new Function<Credentials, String>() {
> - public String apply(Credentials in) {
> - checkState(in.identity.indexOf("@") != 1, "identity should be in project_id@developer.gserviceaccount.com" +
> - " format");
> - return Iterables.get(Splitter.on("@").split(in.identity), 0);
> - }
> - }, creds);
> + public Supplier<String> supplyProject(@org.jclouds.location.Provider final Supplier<Credentials> creds,
I'm guessing no good reason but intellij being persnickety. Fixed. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378374
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> .operatingSystem(image.getOperatingSystem())
> .status(toPortableNodeStatus.get(input.getStatus()))
> - .tags(input.getTags())
> + .tags(input.getTags().getTags())
? Certainly looks a little weird? Whence the double `getTags`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362323
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * 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.googlecomputeengine.domain;
> +
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Adam Lowe
> + */
> +public class MachineTypeInZone extends SlashEncodedIds {
`SlashEncodedIds` really sounds more like a helper class or an artifact of the representation - is it really part of the domain hierarchy?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362617
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> - public Iterable<MachineType> listHardwareProfiles() {
> - return api.getMachineTypeApiForProject(userProject.get()).list().concat();
> + public Iterable<MachineTypeInZone> listHardwareProfiles() {
> + return FluentIterable.from(zones.get().values())
> + .transformAndConcat(new Function<Location, ImmutableSet<MachineType>>() {
> + @Override
> + public ImmutableSet<MachineType> apply(Location input) {
> + return api.getMachineTypeApiForProject(userProject.get()).listInZone(input.getId()).concat().toSet();
> + }
> + }).transform(new Function<MachineType, MachineTypeInZone>() {
> +
> + @Override
> + public MachineTypeInZone apply(MachineType arg0) {
> + return new MachineTypeInZone(arg0, arg0.getZone());
> + }
> + }).toSet();
Any urgent (performance or otherwise) need for FluentIterable here? Smells a little like https://code.google.com/p/guava-libraries/wiki/FunctionalExplained - what would the imperative version look like?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362240
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by David Ribeiro Alves <no...@github.com>.
> - public Iterable<MachineType> listHardwareProfiles() {
> - return api.getMachineTypeApiForProject(userProject.get()).list().concat();
> + public Iterable<MachineTypeInZone> listHardwareProfiles() {
> + return FluentIterable.from(zones.get().values())
> + .transformAndConcat(new Function<Location, ImmutableSet<MachineType>>() {
> + @Override
> + public ImmutableSet<MachineType> apply(Location input) {
> + return api.getMachineTypeApiForProject(userProject.get()).listInZone(input.getId()).concat().toSet();
> + }
> + }).transform(new Function<MachineType, MachineTypeInZone>() {
> +
> + @Override
> + public MachineTypeInZone apply(MachineType arg0) {
> + return new MachineTypeInZone(arg0, arg0.getZone());
> + }
> + }).toSet();
+1 for the imperative version when you have a chance
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5412693
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +import com.google.inject.Inject;
> +
> +/**
> + * Transforms a google compute domain specific machine type to a generic Hardware object.
> + *
> + * @author David Alves
> + */
> +public class MachineTypeInZoneToHardware implements Function<MachineTypeInZone, Hardware> {
> +
> + private final Supplier<Map<URI, ? extends Location>> locations;
> +
> + @Inject
> + public MachineTypeInZoneToHardware(@Memoized Supplier<Map<URI, ? extends Location>> locations) {
> + this.locations = locations;
> + }
> + @Override
Add blank line
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362326
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @OAuthScopes(COMPUTE_READONLY_SCOPE)
> @ResponseParser(ParseDisks.class)
> @Transform(ParseDisks.ToPagedIterable.class)
> @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> - PagedIterable<Disk> list(ListOptions options);
> + PagedIterable<Disk> listInZone(@PathParam("zone") String zone, ListOptions options);
> +
> + /**
> + * TODO live and expect tests for DiskApi#createSnapshot
I'm actually gonna remove createSnapshot - I nuked most of the new API calls I added in the work-in-progress for 1.7 when I backported it, but some slipped through.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379012
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> </logger>
>
> <logger name="jclouds.headers">
> - <level value="DEBUG"/>
> - <appender-ref ref="WIREFILE"/>
> + <level value="TRACE" />
> + <appender-ref ref="WIREFILE" />
> + </logger>
> +
> + <logger name="jclouds.compute">
> + <level value="TRACE" />
> + <appender-ref ref="COMPUTEFILE" />
> + </logger>
> +
> + <logger name="jclouds.ssh">
> + <level value="TRACE" />
> + <appender-ref ref="SSHFILE" />
I just cut and pasted this junk from another provider to get better logging. I realy have no idea what it does. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379585
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> }
>
> @Provides
> @Singleton
> @Memoized
> public Supplier<Map<URI, ? extends Hardware>> provideHardwaresMap(
> - final Supplier<Set<? extends Hardware>> hardwares) {
> - return new Supplier<Map<URI, ? extends Hardware>>() {
> - @Override
> - public Map<URI, ? extends Hardware> get() {
> - return uniqueIndex(hardwares.get(), new Function<Hardware, URI>() {
> - @Override
> - public URI apply(Hardware input) {
> - return input.getUri();
> - }
> - });
> - }
> - };
> + AtomicReference<AuthorizationException> authException,
> + final Supplier<Set<? extends Hardware>> hardwares,
Different animals. It's a bit annoying, but we really need both the set of hardwares (which is coming from ComputeServiceAdapterContextModule's providesHardware, and below that, GoogleComputeEngineServiceAdapter's listHardwareProfiles) as in every other compute API/provider, and a mapping of URIs to hardware, specifically for GCE.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377898
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +/**
> + * Options for attaching disks to instances.
> + *
> + * @author Andrew Bayer
> + * @see <a href="https://developers.google.com/compute/docs/reference/latest/instances/attachDisk"/>
> + */
> +public class AttachDiskOptions {
> +
> + public enum DiskType {
> + SCRATCH,
> + PERSISTENT
> + }
> +
> + public enum DiskMode {
> + READ_WRITE,
> + READ_ONLY
Do we prefer enums with or without a trailing `;`? Do I recall a change earlier where we _added_ a trailing `;`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362849
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -349,14 +356,127 @@ public Builder fromInstance(Instance in) {
> .networkInterfaces(in.getNetworkInterfaces())
> .disks(in.getDisks())
> .metadata(in.getMetadata())
> - .serviceAccoutns(in.getServiceAccounts());
> + .serviceAccounts(in.getServiceAccounts());
> + }
> + }
> +
> + /**
> + * Tags for an instance, with their fingerprint.
> + */
> + public static class Tags {
> + private final String fingerprint;
> + private final Set<String> tags;
> +
> + @ConstructorProperties({"fingerprint", "items"})
> + public Tags(String fingerprint, Set<String> tags) {
Mark second argument as `@Nullable`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362543
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> - public Iterable<Instance> listNodes() {
> - return api.getInstanceApiForProject(userProject.get()).list().concat();
> + public Iterable<InstanceInZone> listNodes() {
> + return FluentIterable.from(zones.get().values()).transformAndConcat(new Function<Location, ImmutableSet<InstanceInZone>>() {
> + @Override
> + public ImmutableSet<InstanceInZone> apply(final Location input) {
> + return api.getInstanceApiForProject(userProject.get()).listInZone(input.getId()).concat()
> + .transform(new Function<Instance, InstanceInZone>() {
> +
> + @Override
> + public InstanceInZone apply(Instance arg0) {
> + return new InstanceInZone(arg0, input.getId());
> + }
> + }).toSet();
> + }
> + }).toSet();
Again, https://code.google.com/p/guava-libraries/wiki/FunctionalExplained?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362252
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + return obsolete;
> + }
> +
> + /**
> + * Optional RFC3339 timestamp for when the deprecation state was changed to DELETED.
> + *
> + * @return the timestamp
> + */
> + public Date getDeleted() {
> + return deleted;
> + }
> +
> + /**
> + * @see DeprecateOptions#getState()
> + */
> + public DeprecateOptions state(State state) {
No builder for these - builder and options class in one? Do we do that elsewhere?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362870
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + public void testDeleteOperationResponseIs4xx() throws Exception {
> + HttpRequest delete = HttpRequest
> + .builder()
> + .method("DELETE")
> + .endpoint(OPERATIONS_URL_PREFIX + "/operation-1352178598164-4cdcc9d031510-4aa46279")
> + .addHeader("Authorization", "Bearer " + TOKEN).build();
> +
> + HttpResponse operationResponse = HttpResponse.builder().statusCode(404).build();
> +
> + ZoneOperationApi zoneOperationApi = requestsSendResponses(requestForScopes(COMPUTE_SCOPE),
> + TOKEN_RESPONSE, delete, operationResponse).getZoneOperationApiForProject("myproject");
> +
> + zoneOperationApi.deleteInZone("us-central1-a", "operation-1352178598164-4cdcc9d031510-4aa46279");
> + }
> +
> + public void testLisOperationWithNoOptionsResponseIs2xx() {
`testListOp...`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362970
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
Yeah, I'm removing all the operations that don't have expect tests, since they're new operations that I'm fine with not supporting in 1.6.2. When the 1.7 PR comes in, it'll have expect/live tests for all the operations. I am running into some weirdness in live tests right now, but I'm working on those and will push an updated commit once they're resolved.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21507037
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @Override
> protected GoogleComputeEngineApi create(Properties props, Iterable<Module> modules) {
> GoogleComputeEngineApi api = super.create(props, modules);
> + URI imageUri = api.getImageApiForProject("google")
> + .list(new ListOptions.Builder().filter("name eq gcel.*"))
> + .concat()
> + .filter(new Predicate<Image>() {
> + @Override
> + public boolean apply(Image input) {
> + if (input.getDeprecated().isPresent()) {
> + if (input.getDeprecated().get().getState().isPresent()) {
Done!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379455
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> return Uris.uriBuilder(endpoint.get()).appendPath("/projects/").appendPath(userProject.get()).appendPath
> - ("/machineTypes/").appendPath(input).build();
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378531
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import org.jclouds.collect.IterableWithMarker;
> +import org.jclouds.collect.PagedIterable;
> +import org.jclouds.collect.PagedIterables;
> +import org.jclouds.googlecomputeengine.domain.ListPage;
> +import org.jclouds.googlecomputeengine.options.ListOptions;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.rest.InvocationContext;
> +import org.jclouds.rest.internal.GeneratedHttpRequest;
> +
> +import com.google.common.annotations.Beta;
> +import com.google.common.base.Function;
> +import com.google.common.base.Optional;
> +
> +/**
> + * @author Adrian Cole
?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362825
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +
> + }
> +
> + /**
> + * Deprecation information for an image
> + */
> + public static class Deprecated {
> + private final Optional<String> state;
> + private final Optional<URI> replacement;
> + private final Optional<String> deprecated;
> + private final Optional<String> obsolete;
> + private final Optional<String> deleted;
> +
> + @ConstructorProperties({"state", "replacement", "deprecated", "obsolete", "deleted"})
> + public Deprecated(String state, URI replacement, String deprecated, String obsolete,
> + String deleted) {
Mark constructor properties as `@Nullable`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362506
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #24](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/24/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21570093
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
>
> @Override
> - public boolean apply(Instance instance) {
> - return contains(ids, instance.getName());
> + public boolean apply(InstanceInZone instance) {
> + return contains(ids, instance.getInstance().getName());
Rename var to `instanceInZone`? Then `instanceInZone.getInstance().` reads a little more clearly?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362259
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + * @param fingerprint The current fingerprint for the tags
> + * @return an Operations resource. To check on the status of an operation, poll the Operations resource returned
> + * to you, and look for the status field.
> + */
> + @Named("Instances:setTags")
> + @POST
> + @Path("/zones/{zone}/instances/{instance}/setTags")
> + @OAuthScopes(COMPUTE_SCOPE)
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Fallback(NullOnNotFoundOr404.class)
> + @MapBinder(BindToJsonPayload.class)
> + @Nullable
> + Operation setTagsInZone(@PathParam("zone") String zone,
> + @PathParam("instance") String instanceName,
> + @PayloadParam("items") Set<String> items,
If their called `items` here, perhaps call the field `items` in the `Tags` class also?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362806
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
>
> @Override
> - public boolean apply(Instance instance) {
> - return contains(ids, instance.getName());
> + public boolean apply(InstanceInZone instance) {
> + return contains(ids, instance.getInstance().getName());
+1. Changing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377610
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
>
> protected Operation(String id, Date creationTimestamp, URI selfLink, String name, String description,
> URI targetLink, String targetId, String clientOperationId, Status status,
> String statusMessage, String user, Integer progress, Date insertTime, Date startTime,
> Date endTime, Integer httpErrorStatusCode, String httpErrorMessage, String operationType,
> - List<Error> errors) {
> + List<Error> errors, URI region, URI zone) {
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378974
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> super(Kind.IMAGE, id, creationTimestamp, selfLink, name, description);
> this.sourceType = checkNotNull(sourceType, "sourceType of %s", name);
> this.preferredKernel = fromNullable(preferredKernel);
> - this.rawDisk = checkNotNull(rawDisk, "rawDisk of %s", name); ;
> + this.rawDisk = checkNotNull(rawDisk, "rawDisk of %s", name);
> + this.deprecated = fromNullable(deprecated);
> + ;
That's a silly semicolon. Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378692
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
>
> /**
> * Retrieves the list of machine type resources available to the specified project.
> * By default the list as a maximum size of 100, if no options are provided or ListOptions#getMaxResults() has not
> * been set.
> *
> - *
> + * @param zone The zone to list in.
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379165
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> +/**
> + * A persistent disk resource
> + *
> + * @author David Alves
> + * @see <a href="https://developers.google.com/compute/docs/reference/v1beta15/disks"/>
> + */
> +@Beta
> +public abstract class AbstractDisk extends Resource {
> +
> + protected final Integer sizeGb;
> + protected final String status;
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "sizeGb",
> + "status"
> + })
Yeah, ConstructorProperties not needed here. Removing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378570
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -65,6 +65,13 @@
> String version = on(".").join(limit(skip(splits, 1), splits.size() - 2));
> osBuilder.version(version);
>
> + if (image.getDeprecated().isPresent()) {
> + if (image.getDeprecated().get().getState().isPresent()) {
> + builder.userMetadata(ImmutableMap.<String,String>builder()
`builder.userMetadata(ImmutableMap.of("deprecatedState", image.getDeprecated().get().getState().get()));`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362303
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -0,0 +1 @@
> +{"items":["aTag"],"fingerprint":"abcd"}
Actually, this guy isn't referenced anywhere yet, I don't think...I know I had to change a bunch of them over on the 1.7 branch. I'll nuke this guy for now and see what's what over on 1.7.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5381031
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> </logger>
>
> <logger name="jclouds.headers">
> - <level value="DEBUG"/>
> - <appender-ref ref="WIREFILE"/>
> + <level value="TRACE" />
> + <appender-ref ref="WIREFILE" />
> + </logger>
> +
> + <logger name="jclouds.compute">
> + <level value="TRACE" />
> + <appender-ref ref="COMPUTEFILE" />
> + </logger>
> +
> + <logger name="jclouds.ssh">
> + <level value="TRACE" />
> + <appender-ref ref="SSHFILE" />
Well, it's only for tests. I don't mind lots of logs for tests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5381001
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #21](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/21/) 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/pull/16#issuecomment-21283609
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> super(Kind.MACHINE_TYPE, id, creationTimestamp, selfLink, name, description);
> this.guestCpus = checkNotNull(guestCpus, "guestCpus of %s", name);
> this.memoryMb = checkNotNull(memoryMb, "memoryMb of %s", name);
> this.imageSpaceGb = checkNotNull(imageSpaceGb, "imageSpaceGb of %s", name);
> - this.ephemeralDisks = ephemeralDisks == null ? ImmutableList.<EphemeralDisk>of() : ephemeralDisks;
> + this.scratchDisks = scratchDisks == null ? ImmutableList.<ScratchDisk>of() : scratchDisks;
If `scratchDisks` is built by a builder, how would it ever be `null`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362594
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + public enum DiskType {
> + SCRATCH,
> + PERSISTENT
> + }
> +
> + public enum DiskMode {
> + READ_WRITE,
> + READ_ONLY
> + }
> +
> + private DiskType type;
> + private DiskMode mode;
> + private URI source;
> + private String deviceName;
> +
> + // TODO decide whether to do something with the index and boot options
I'll just remove that comment here and leave it over on the full work-in-progress for 1.7 where I'll actually resolve that question.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379271
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + return self();
> + }
> +
> + /**
> + * @see org.jclouds.googlecomputeengine.domain.AbstractDisk#getStatus()
> + */
> + public T status(String status) {
> + this.status = status;
> + return self();
> + }
> +
> +
> + @Override
> + protected T self() {
> + return self();
> + }
YEah, that's dumb and I'm not sure why I have it. Fixing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378612
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + * Tests that a Zone Operation is done, returning the completed Operation when it is.
> + *
> + * @author David Alves
> + */
> +public class ZoneOperationDonePredicate implements Predicate<AtomicReference<Operation>> {
> +
> + private final GoogleComputeEngineApi api;
> + private final Supplier<String> project;
> + private final Supplier<Map<URI, ? extends Location>> zones;
> +
> + @Inject
> + ZoneOperationDonePredicate(GoogleComputeEngineApi api, @UserProject Supplier<String> project,
> + @Memoized Supplier<Map<URI, ? extends Location>> zones) {
> + this.api = api;
> + this.project = project;
> + this.zones = zones;
Can never be null by this point or all kinds of other things have already blown up. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379370
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -65,10 +55,16 @@ public URI getZone() {
> }
>
> /**
> - * @return the status of disk creation.
> + * {@inheritDoc}
Do we do `{@inheritDoc}`? Isn't that IntelliJ-only stuff?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362453
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> +
> +import java.net.URI;
> +import java.util.Date;
> +
> +/**
> + * Options to set the deprecation status of a resource. Currently only for images.
> + *
> + * @author Andrew Bayer
> + * @see <a href="https://developers.google.com/compute/docs/reference/latest/images/deprecate" />
> + */
> +public class DeprecateOptions {
> +
> + public enum State {
> + DEPRECATED,
> + OBSOLETE,
> + DELETED
Same shrug as before. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379292
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
Thanks, @abayer! A couple of expect tests missing for some operations (e.g. deprecate, I think?) but you mentioned that.
Still curious about having the "SlashEncodedIds" thing as part of the domain model, but otherwise looks good to me overall!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21461596
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> - return compose(new Function<Credentials, String>() {
> - public String apply(Credentials in) {
> - checkState(in.identity.indexOf("@") != 1, "identity should be in project_id@developer.gserviceaccount.com" +
> - " format");
> - return Iterables.get(Splitter.on("@").split(in.identity), 0);
> - }
> - }, creds);
> + public Supplier<String> supplyProject(@org.jclouds.location.Provider final Supplier<Credentials> creds,
> + final GoogleComputeEngineApi api,
> + AtomicReference<AuthorizationException> authException,
> + @Named(PROPERTY_SESSION_INTERVAL) long seconds) {
> + return MemoizedRetryOnTimeOutButNotOnAuthorizationExceptionSupplier.create(authException,
> + compose(new Function<Credentials, String>() {
> + public String apply(Credentials in) {
> + checkState(in.identity.indexOf("@") != 1, "identity should be in project_id@developer.gserviceaccount.com" +
> + " format");
Legacy, I think. Fixing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378383
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @@ -170,34 +223,57 @@ public Image getImage(String id) {
> }
>
> @Override
> - public Instance getNode(String name) {
> - return api.getInstanceApiForProject(userProject.get()).get(name);
> + public InstanceInZone getNode(String name) {
InstanceApi.getInZone(...) is `@Nullable`, yes.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5377559
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #217](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/217/) 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/pull/16#issuecomment-21450762
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + * @param fingerprint The current fingerprint for the tags
> + * @return an Operations resource. To check on the status of an operation, poll the Operations resource returned
> + * to you, and look for the status field.
> + */
> + @Named("Instances:setTags")
> + @POST
> + @Path("/zones/{zone}/instances/{instance}/setTags")
> + @OAuthScopes(COMPUTE_SCOPE)
> + @Consumes(MediaType.APPLICATION_JSON)
> + @Produces(MediaType.APPLICATION_JSON)
> + @Fallback(NullOnNotFoundOr404.class)
> + @MapBinder(BindToJsonPayload.class)
> + @Nullable
> + Operation setTagsInZone(@PathParam("zone") String zone,
> + @PathParam("instance") String instanceName,
> + @PayloadParam("items") Set<String> items,
Oh. Duh. Doing so.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379074
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
checkstyle doesn't work on 1.6.x in labs. fun!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21283739
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -170,34 +223,57 @@ public Image getImage(String id) {
> }
>
> @Override
> - public Instance getNode(String name) {
> - return api.getInstanceApiForProject(userProject.get()).get(name);
> + public InstanceInZone getNode(String name) {
> + SlashEncodedIds slashEncodedIds = SlashEncodedIds.fromSlashEncoded(name);
Fine
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380040
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> @OAuthScopes({COMPUTE_SCOPE})
> @MapBinder(InstanceBinder.class)
> - Operation createInZone(@PayloadParam("name") String instanceName,
> - @PayloadParam("template") InstanceTemplate template,
> - @PayloadParam("zone") String zone);
> + Operation createInZone(@PathParam("zone") String zone,
> + @PayloadParam("name") String instanceName,
> + @PayloadParam("template") InstanceTemplate template);
Yup, switched it as discussed above.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379057
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + private final Supplier<Map<URI, ? extends Location>> zones;
> +
> + @Inject
> + ZoneOperationDonePredicate(GoogleComputeEngineApi api, @UserProject Supplier<String> project,
> + @Memoized Supplier<Map<URI, ? extends Location>> zones) {
> + this.api = api;
> + this.project = project;
> + this.zones = zones;
> + }
> +
> + @Override
> + public boolean apply(AtomicReference<Operation> input) {
> + checkNotNull(input, "input");
> + Operation current = api.getZoneOperationApiForProject(project.get())
> + .getInZone(zones.get().get(input.get().getZone().get()).getId(),
> + input.get().getName());
Aiaiaiaiaiaai....getitis! ;-) But not much you can do about that, I fear...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362887
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> instanceTemplate.serviceAccounts(options.getServiceAccounts());
> instanceTemplate.image(checkNotNull(template.getImage().getUri(), "image URI is null"));
>
> Operation operation = api.getInstanceApiForProject(userProject.get())
> - .createInZone(name, instanceTemplate, template.getLocation().getId());
> + .createInZone(template.getLocation().getId(), name, instanceTemplate);
Any specific reason for shuffling these around? Since the first and last arguments are both strings, is there a risk that users will simply swap the first and second, or second and third args when fixing their code, leading to an incorrect invocation?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362039
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> .build();
>
> ComputeService client = requestsSendResponses(requestResponseMap);
> Template template = client.templateBuilder().build();
> - Template toMatch = client.templateBuilder().imageId(template.getImage().getId()).build();
> + Hardware defaultSize = client.templateBuilder().build().getHardware();
> +
> + Hardware smallest = client.templateBuilder().smallest().build().getHardware();
> + assertEquals(defaultSize, smallest);
> +
> + Hardware fastest = client.templateBuilder().fastest().build().getHardware();
> + assertNotNull(fastest);
Honestly, it's there to make sure we can actually get it in the first place. There were problems that led to it just barfing when trying to get fastest.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379395
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> .name(input.getName())
> .providerId(input.getId())
> .hostname(input.getName())
> .imageId(image.getId())
> .location(checkNotNull(locations.get().get(input.getZone()), "location for %s", input.getZone()))
> .hardware(checkNotNull(hardwares.get().get(input.getMachineType()), "hardware type for %s",
> - input.getMachineType().toString()))
> + input.getMachineType().toString()))
Spacing?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362312
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @author David Alves
> + */
> +public class ZoneOperationApiLiveTest extends BaseGoogleComputeEngineApiLiveTest {
> +
> + private static final String METADATA_ITEM_KEY = "operationLiveTestTestProp";
> + private static final String METADATA_ITEM_VALUE = "operationLiveTestTestValue";
> + private Operation addOperation;
> + private Operation deleteOperation;
> +
> + private ZoneOperationApi api() {
> + return api.getZoneOperationApiForProject(userProject.get());
> + }
> +
> +
> +// @Test(groups = "live")
Why commented out?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362972
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> - public Iterable<MachineType> listHardwareProfiles() {
> - return api.getMachineTypeApiForProject(userProject.get()).list().concat();
> + public Iterable<MachineTypeInZone> listHardwareProfiles() {
> + return FluentIterable.from(zones.get().values())
> + .transformAndConcat(new Function<Location, ImmutableSet<MachineType>>() {
> + @Override
> + public ImmutableSet<MachineType> apply(Location input) {
> + return api.getMachineTypeApiForProject(userProject.get()).listInZone(input.getId()).concat().toSet();
> + }
> + }).transform(new Function<MachineType, MachineTypeInZone>() {
> +
> + @Override
> + public MachineTypeInZone apply(MachineType arg0) {
> + return new MachineTypeInZone(arg0, arg0.getZone());
> + }
> + }).toSet();
Yup, imperative (with a transform added at the end to get to MachineTypeInZone) does the trick. Will push new version shortly.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5427581
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * 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.googlecomputeengine.domain;
> +
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Adam Lowe
> + */
> +public class MachineTypeInZone extends SlashEncodedIds {
> should it move to org.jclouds.googlecomputeengine.compute.domain
That would still be in a domain package, and that's what doesn't feel quite right to me. `RegionAndName` may well be a couple of IDs encoded with a slash, but the name represents a _domain_ concept, not a _type of representation_.
I couldn't find any reference in the API docs to a first-class concept called "SlashEncodedIds" (but didn't look very hard), hence the feeling that this doesn't really belong. If we were doing something like Scala I'd say this looks more like a _trait_ than a _parent class_.
In Java, my gut feel would be to extract the encoding logic into a `util.SlashEncodedIds` class and simply call that, rather than making it part of the inheritance hierarchy. If necessary, this could be `implements SlashEncodedIds` if we want to impose some kind of "contract".
Does that make some kind of sense?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380268
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> .build();
>
> ComputeService client = requestsSendResponses(requestResponseMap);
> Template template = client.templateBuilder().build();
> - Template toMatch = client.templateBuilder().imageId(template.getImage().getId()).build();
> + Hardware defaultSize = client.templateBuilder().build().getHardware();
> +
> + Hardware smallest = client.templateBuilder().smallest().build().getHardware();
> + assertEquals(defaultSize, smallest);
> +
> + Hardware fastest = client.templateBuilder().fastest().build().getHardware();
> + assertNotNull(fastest);
No other checks on `fastest`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362903
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + .location(location)
> + .name(input.getMachineType().getName())
> + .hypervisor("kvm")
> + .processor(new Processor(input.getMachineType().getGuestCpus(), 1.0))
> + .providerId(input.getMachineType().getId())
> + .ram(input.getMachineType().getMemoryMb())
> + .uri(input.getMachineType().getSelfLink())
> + .userMetadata(ImmutableMap.of("imageSpaceGb", Integer.toString(input.getMachineType().getImageSpaceGb())))
> + .volumes(collectVolumes(input.getMachineType()))
> + .build();
> + }
> +
> + private Iterable<Volume> collectVolumes(MachineType input) {
> + ImmutableSet.Builder<Volume> volumes = ImmutableSet.builder();
> + for (MachineType.ScratchDisk disk : input.getScratchDisks()) {
> + volumes.add(new VolumeImpl(null, Volume.Type.LOCAL, new Integer(disk.getDiskGb()).floatValue(), null, true,
`new VolumeImpl`? No builder or other factory for this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362360
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> super(Kind.MACHINE_TYPE, id, creationTimestamp, selfLink, name, description);
> this.guestCpus = checkNotNull(guestCpus, "guestCpus of %s", name);
> this.memoryMb = checkNotNull(memoryMb, "memoryMb of %s", name);
> this.imageSpaceGb = checkNotNull(imageSpaceGb, "imageSpaceGb of %s", name);
> - this.ephemeralDisks = ephemeralDisks == null ? ImmutableList.<EphemeralDisk>of() : ephemeralDisks;
> + this.scratchDisks = scratchDisks == null ? ImmutableList.<ScratchDisk>of() : scratchDisks;
I...have no idea. I just renamed that guy, didn't change much more there.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378900
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #219](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/219/) 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/pull/16#issuecomment-21569971
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + return obsolete;
> + }
> +
> + /**
> + * Optional RFC3339 timestamp for when the deprecation state was changed to DELETED.
> + *
> + * @return the timestamp
> + */
> + public Date getDeleted() {
> + return deleted;
> + }
> +
> + /**
> + * @see DeprecateOptions#getState()
> + */
> + public DeprecateOptions state(State state) {
Actually, I'm gonna remove that here (along with the deprecate API thingie itself) since it's new APIs that I haven't tested and here I'm really just concerned with having the old stuff work.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379325
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.googlecomputeengine.domain;
> +
> +import static com.google.common.base.Preconditions.checkArgument;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +import com.google.common.base.Objects;
> +import com.google.common.base.Splitter;
> +import com.google.common.collect.Iterables;
> +
> +/**
> + * @author Adam Lowe
> + */
> +public class SlashEncodedIds {
Feels weird as part of the domain model? If the representation changed tomorrow, would you expect the domain model to be updated, too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362637
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + @ResponseParser(ParseGlobalOperations.class)
> + @Transform(ParseGlobalOperations.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<Operation> list(ListOptions listOptions);
> +
> + /**
> + * @see org.jclouds.googlecomputeengine.features.GlobalOperationApi#aggregatedListAtMarker(String, org.jclouds.googlecomputeengine.options.ListOptions)
> + */
> + @Named("GlobalOperations:aggregatedList")
> + @GET
> + @Path("/aggregated/operations")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @Consumes(MediaType.APPLICATION_JSON)
> + @ResponseParser(ParseGlobalOperations.class)
> + @Fallback(EmptyIterableWithMarkerOnNotFoundOr404.class)
> + ListPage<Operation> aggregatedListFirstPage();
Let's leave as-is, then ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380305
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-pull-requests #27](https://jclouds.ci.cloudbees.com/job/jclouds-labs-pull-requests/27/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16#issuecomment-21638494
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + *
> + * 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.googlecomputeengine.domain;
> +
> +import static com.google.common.base.Objects.equal;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +/**
> + * @author Adam Lowe
> + */
> +public class MachineTypeInZone extends SlashEncodedIds {
I'm aping things like org.jclouds.ec2.compute.domain.RegionAndName but generalizing it further - should it move to org.jclouds.googlecomputeengine.compute.domain, then?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378945
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> + * Tests that a Zone Operation is done, returning the completed Operation when it is.
> + *
> + * @author David Alves
> + */
> +public class ZoneOperationDonePredicate implements Predicate<AtomicReference<Operation>> {
> +
> + private final GoogleComputeEngineApi api;
> + private final Supplier<String> project;
> + private final Supplier<Map<URI, ? extends Location>> zones;
> +
> + @Inject
> + ZoneOperationDonePredicate(GoogleComputeEngineApi api, @UserProject Supplier<String> project,
> + @Memoized Supplier<Map<URI, ? extends Location>> zones) {
> + this.api = api;
> + this.project = project;
> + this.zones = zones;
Null checks needed or can never be null?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362879
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.googlecomputeengine.domain;
> +
> +import static com.google.common.base.Preconditions.checkArgument;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +
> +import com.google.common.base.Objects;
> +import com.google.common.base.Splitter;
> +import com.google.common.collect.Iterables;
> +
> +/**
> + * @author Adam Lowe
> + */
> +public class SlashEncodedIds {
See above.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5378982
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> +
> +import org.jclouds.collect.IterableWithMarker;
> +import org.jclouds.collect.PagedIterable;
> +import org.jclouds.collect.PagedIterables;
> +import org.jclouds.googlecomputeengine.domain.ListPage;
> +import org.jclouds.googlecomputeengine.options.ListOptions;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.rest.InvocationContext;
> +import org.jclouds.rest.internal.GeneratedHttpRequest;
> +
> +import com.google.common.annotations.Beta;
> +import com.google.common.base.Function;
> +import com.google.common.base.Optional;
> +
> +/**
> + * @author Adrian Cole
Well, 99% of it is him, I think. =) I'll add myself as well.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5380855
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +
> +import java.net.URI;
> +import java.util.Date;
> +
> +/**
> + * Options to set the deprecation status of a resource. Currently only for images.
> + *
> + * @author Andrew Bayer
> + * @see <a href="https://developers.google.com/compute/docs/reference/latest/images/deprecate" />
> + */
> +public class DeprecateOptions {
> +
> + public enum State {
> + DEPRECATED,
> + OBSOLETE,
> + DELETED
See comment on enums and trailing `;` above.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362863
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by David Ribeiro Alves <no...@github.com>.
>
> import javax.inject.Inject;
> import javax.inject.Singleton;
> -import java.util.Set;
> +
> +import org.jclouds.compute.domain.NodeMetadata;
> +
> +import com.google.common.base.Function;
> +import com.google.common.base.Predicate;
> +import com.google.common.collect.Sets;
>
same spurious import change
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5412959
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + public void testDeleteOperationResponseIs4xx() throws Exception {
> + HttpRequest delete = HttpRequest
> + .builder()
> + .method("DELETE")
> + .endpoint(OPERATIONS_URL_PREFIX + "/operation-1352178598164-4cdcc9d031510-4aa46279")
> + .addHeader("Authorization", "Bearer " + TOKEN).build();
> +
> + HttpResponse operationResponse = HttpResponse.builder().statusCode(404).build();
> +
> + ZoneOperationApi zoneOperationApi = requestsSendResponses(requestForScopes(COMPUTE_SCOPE),
> + TOKEN_RESPONSE, delete, operationResponse).getZoneOperationApiForProject("myproject");
> +
> + zoneOperationApi.deleteInZone("us-central1-a", "operation-1352178598164-4cdcc9d031510-4aa46279");
> + }
> +
> + public void testLisOperationWithNoOptionsResponseIs2xx() {
ha typo fun. Fixing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379464
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> +/**
> + * A persistent disk resource
> + *
> + * @author David Alves
> + * @see <a href="https://developers.google.com/compute/docs/reference/v1beta15/disks"/>
> + */
> +@Beta
> +public abstract class AbstractDisk extends Resource {
> +
> + protected final Integer sizeGb;
> + protected final String status;
> +
> + @ConstructorProperties({
> + "id", "creationTimestamp", "selfLink", "name", "description", "sizeGb",
> + "status"
> + })
Annotation list doesn't match (e.g. missing `kind`). Is this even needed?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362420
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Phillips <no...@github.com>.
> @@ -76,47 +91,54 @@ protected void bindErrorHandlers() {
> @Provides
> @Singleton
> @UserProject
> - public Supplier<String> supplyProject(@org.jclouds.location.Provider final Supplier<Credentials> creds) {
> - return compose(new Function<Credentials, String>() {
> - public String apply(Credentials in) {
> - checkState(in.identity.indexOf("@") != 1, "identity should be in project_id@developer.gserviceaccount.com" +
> - " format");
> - return Iterables.get(Splitter.on("@").split(in.identity), 0);
> - }
> - }, creds);
> + public Supplier<String> supplyProject(@org.jclouds.location.Provider final Supplier<Credentials> creds,
`@Provider` doesn't work?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5362378
Re: [jclouds-labs] JCLOUDS-192. Move 1.6.x GCE to API v1beta15 (#16)
Posted by Andrew Bayer <no...@github.com>.
> + @ResponseParser(ParseGlobalOperations.class)
> + @Transform(ParseGlobalOperations.ToPagedIterable.class)
> + @Fallback(EmptyPagedIterableOnNotFoundOr404.class)
> + PagedIterable<Operation> list(ListOptions listOptions);
> +
> + /**
> + * @see org.jclouds.googlecomputeengine.features.GlobalOperationApi#aggregatedListAtMarker(String, org.jclouds.googlecomputeengine.options.ListOptions)
> + */
> + @Named("GlobalOperations:aggregatedList")
> + @GET
> + @Path("/aggregated/operations")
> + @OAuthScopes(COMPUTE_READONLY_SCOPE)
> + @Consumes(MediaType.APPLICATION_JSON)
> + @ResponseParser(ParseGlobalOperations.class)
> + @Fallback(EmptyIterableWithMarkerOnNotFoundOr404.class)
> + ListPage<Operation> aggregatedListFirstPage();
Beats me. Ask @dralves. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/16/files#r5379050