Re: [jclouds] Take into account that hardware profiles may not have scratch disks (#854)
LGTM pending passing tests. Thanks for doing this! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/854#issuecomment-141230817
Re: [jclouds] Take into account that hardware profiles may not have scratch disks (#854)
I would lean towards overriding the test and removing the disks size assertion. It seems potentially confusing to me that we would add a dummy volume. What are hardware profile volumes used for in practice? Why does a Jclouds user need these values? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/854#issuecomment-140286588
Re: [jclouds] JCLOUDS-172: Promote Google Compute Engine (#787)
Thanks for all the work that went into this! I'm excited to see this go through! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/787#issuecomment-115777086
Re: [jclouds-labs-google] add rewrite to ObjectApi (#159)
Thanks! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/159#issuecomment-114679311
Re: [jclouds-labs-google] add rewrite to ObjectApi (#159)
My mistake, I guess thats what I get for removing newlines after doing checkstyle. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/159#issuecomment-114675527
[jclouds-labs-google] add rewrite to ObjectApi (#159)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/159 -- Commit Summary -- * add rewrite to ObjectApi -- File Changes -- A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/RewriteResponse.java (41) M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ObjectApi.java (53) A google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/options/RewriteObjectOptions.java (155) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/ObjectApiLiveTest.java (17) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/ObjectApiMockTest.java (21) A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/ParseObjectRewriteResponse.java (72) A google-cloud-storage/src/test/resources/object_rewrite.json (28) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/159.patch https://github.com/jclouds/jclouds-labs-google/pull/159.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/159
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update MockTests (#150)
>return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder() {
Sounds good to me, updated.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32907151
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update MockTests (#150)
>return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
I don't doubt there is a better way. Im not sure what you are envisioning, are
you suggesting I put a static `builder` method be in the ComposeObjectTemplate
class and not ComposeObjectTemplate.Builder class and make the current `public
Builder()` method package protected?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880446
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update MockTests (#150)
>return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
> +
> + public Builder
> fromGoogleCloudStorageObject(List objects){
> + ArrayList sourceObjects = new
> ArrayList();
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880374
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update MockTests (#150)
> + }
> + this.sourceObjects = ImmutableList.copyOf(sourceObjects);
> + return this;
> + }
> +
> + public Builder fromNames(List SourceObjectNames){
> + ArrayList sourceObjects = new
> ArrayList();
> + for (String name : SourceObjectNames){
> +sourceObjects.add(SourceObject.nameOnly(name));
> + }
> + this.sourceObjects = ImmutableList.copyOf(sourceObjects);
> + return this;
> + }
> +
> + public Builder destination(ObjectTemplate destination){
> + this.destination = destination;
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880367
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update MockTests (#150)
>return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
> +
> + public Builder
> fromGoogleCloudStorageObject(List objects){
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880378
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update MockTests (#150)
>return new AutoValue_ComposeObjectTemplate(sourceObjects, destination);
> }
> +
> + ComposeObjectTemplate() {
> + }
> +
> + public static class Builder {
> + private ImmutableList sourceObjects;
> + private ObjectTemplate destination;
> +
> + public Builder(){
> + }
> +
> + public Builder
> fromGoogleCloudStorageObject(List objects){
> + ArrayList sourceObjects = new
> ArrayList();
> + for (GoogleCloudStorageObject obj : objects){
Done.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32880366
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update MockTests (#150)
> @@ -44,7 +44,7 @@
> public class BaseGoogleComputeEngineApiLiveTest extends
> BaseApiLiveTest {
>
> protected static final String ZONE_API_URL_SUFFIX = "/zones/";
> - protected static final String DEFAULT_ZONE_NAME = "us-central1-a";
> + protected static final String DEFAULT_ZONE_NAME = "us-central1-f";
Yes, us-central1-a gets a lot of attention, its nice to mix it up.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/150/files#r32879925
Re: [jclouds-labs-google] Update ComposeObjectTemplate and update MockTests (#150)
ping --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/150#issuecomment-112969739
[jclouds-labs-google] Add running GoogleComputeEngineTemplateBuilderLiveTest with Json key (#156)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/156 -- Commit Summary -- * Add running GoogleComputeEngineTemplateBuilderLiveTest with Json key -- File Changes -- M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineTemplateBuilderLiveTest.java (13) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/156.patch https://github.com/jclouds/jclouds-labs-google/pull/156.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/156
Re: [jclouds-labs-google] Update ObjectApi and getBlob, fix testGetRangeOutOfRange (#151)
> @@ -123,7 +124,7 @@
> @Fallback(NullOnNotFoundOr404.class)
> @Nullable
> GoogleCloudStorageObject getObject(@PathParam("bucket") String
> bucketName, @PathParam("object") String objectName,
> -GetObjectOptions options);
> + HttpRequestOptions options);
I think GetObjectOptions is still useful because it provides GCE specific query
params. Because it inherits from GetOptions which implements from
HttpRequestOptions it can still be used for the getObject call here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/151/files#r32355985
Re: [jclouds-labs-google] Added support for Preemptible instances in GCE (#146)
Great addition! It would also be good to add add a simple unit test case. To run the unit tests you run `mvn clean install` from the command line. If you have any questions we are happy to answer them! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/146#issuecomment-110917816
[jclouds-labs-google] Update ObjectApi and getBlob, fix testGetRangeOutOfRange (#151)
This uncovers a new test failure `BaseBlobIntegrationTest#testGetIfMatch` testGetIfMatch was not actually testing the intended behavior. Neither request was attaching a If-Match header and the test was passing just fine. Now it is attaching an If-Match header but with quotes around the etag causing the request to be rejected. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/151 -- Commit Summary -- * Update ObjectApi and getBlob, fix testGetRangeOutOfRange -- File Changes -- M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/GoogleCloudStorageBlobStore.java (9) M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/features/ObjectApi.java (5) M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/options/GetObjectOptions.java (4) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/ObjectApiMockTest.java (4) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/151.patch https://github.com/jclouds/jclouds-labs-google/pull/151.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/151
[jclouds-labs-google] Update ComposeObjectTemplate and update MockTests (#150)
This change will make implementing the blobstore multipart uploads using simple uploads and compose much cleaner. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/150 -- Commit Summary -- * Update ComposeObjectTemplate and update MockTests -- File Changes -- M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/strategy/internal/SequentialMultipartUploadStrategy.java (4) M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/templates/ComposeObjectTemplate.java (85) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/ObjectApiLiveTest.java (11) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/ObjectApiMockTest.java (10) M google-cloud-storage/src/test/resources/object_compose_request.json (45) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/internal/BaseGoogleComputeEngineApiLiveTest.java (2) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/150.patch https://github.com/jclouds/jclouds-labs-google/pull/150.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/150
[jclouds-labs-google] Update UrlMapApiLiveTest (#148)
Updated the test to use two different backend services and have a mapping in addition to the default mapping. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/148 -- Commit Summary -- * Update UrlMapApiLiveTest -- File Changes -- M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/UrlMapApiLiveTest.java (83) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/148.patch https://github.com/jclouds/jclouds-labs-google/pull/148.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/148
[jclouds-labs-google] Fix DefaultObjectAccessControlsApiLiveTest and ResumableUploadApiLive… (#145)
…Test DefaultObjectAccessControlsApiLiveTest was failing due to the bucket name exceeding the maximum name length of 63 characters. [described here](https://cloud.google.com/storage/docs/bucket-naming) ResumableUploadApiLiveTest was failing due to an off by one error. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/145 -- Commit Summary -- * Fix DefaultObjectAccessControlsApiLiveTest and ResumableUploadApiLiveTest -- File Changes -- M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/DefaultObjectAccessControlsApiLiveTest.java (2) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/ResumableUploadApiLiveTest.java (2) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/145.patch https://github.com/jclouds/jclouds-labs-google/pull/145.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/145
Re: [jclouds-labs-google] Disable MultipartUploads tests (#144)
> public void testPutFileParallel() throws SkipException {
> - // Implement Parallel uploads
> + throw new SkipException("Implement Parallel uploads");
> + // TODO: Implement Parallel uploads
I have checked and they are passing. I have re-enabled `testFileGetParallel`
and `testPutFileParallel`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/144/files#r31478983
[jclouds-labs-google] Disable MultipartUploads tests (#144)
Until they are implemented they should be disabled. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/144 -- Commit Summary -- * disable MultipartUploads tests -- File Changes -- M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/blobstore/integration/GoogleCloudStorageBlobIntegrationLiveTest.java (18) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/144.patch https://github.com/jclouds/jclouds-labs-google/pull/144.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/144
Re: [jclouds-labs-google] Updates to InstanceApi. 100% coverage (#110)
#142 --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/110#issuecomment-106930345
[jclouds-labs-google] Adding builder to NewInstance, add tests. (#142)
This PR adds a builder to NewInstance making it possible to add serviceAccounts and scopes to a instance. This is related to JCLOUDS-906 but does not propagate the changes to the Compute Abstraction. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/142 -- Commit Summary -- * Adding builder to NewInstance, add tests. -- File Changes -- M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/NewInstance.java (87) M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Tags.java (4) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java (29) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiMockTest.java (37) A google-compute-engine/src/test/resources/instance_insert_full.json (47) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/142.patch https://github.com/jclouds/jclouds-labs-google/pull/142.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/142
Re: [jclouds-labs-google] WIP: Add support to start and stop instances in the ComputeService (#141)
I started coding this up and ran into the same problem. I'll look into it. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/141#issuecomment-106641024
[jclouds-labs-google] JCLOUDS-908: Temporary fix to exception thrown in InstanceToNodeMetad… (#140)
…ata due to instance names Temporary solution related to #139 You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/140 -- Commit Summary -- * JCLOUDS-908: Temporary fix to exception thrown in InstanceToNodeMetadata due to instance names -- File Changes -- M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/InstanceToNodeMetadata.java (16) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/140.patch https://github.com/jclouds/jclouds-labs-google/pull/140.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/140
Re: [jclouds-labs-google] WIP: JCLOUDS-908 fix to InstanceToNodeMetadata (#139)
Sure, I will create the 1.9.x PR. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/139#issuecomment-105678509
Re: [jclouds-labs-google] WIP: JCLOUDS-908 fix to InstanceToNodeMetadata (#139)
I updated tags to be immutable. Does this PR look good to go? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/139#issuecomment-104402040
Re: [jclouds-labs-google] WIP: JCLOUDS-908 fix to InstanceToNodeMetadata (#139)
When do you think we will do another release? If it is soon I would consider adding the try-catch block to catch the IllegalArgumentException as a temporary solution to deal with instance names triggering exceptions and then encourage people to upgrade to the new version when it is released. I am hesitant to backport everything because it is tough to reason about the effects of the change on users and unexpected change is bad. That being said, as long as you don't backport removing network from GoogleComputeEngineTemplate options, the main negative side effect I would foresee is resources not being deleted when users expected them to be. (ie. When users delete all nodes in a group the network will not be deleted.) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/139#issuecomment-104074798
Re: [jclouds-labs-google] WIP: JCLOUDS-908 fix to InstanceToNodeMetadata (#139)
rebuild please --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/139#issuecomment-104072636
Re: [jclouds-labs-google] WIP: JCLOUDS-908 fix to InstanceToNodeMetadata (#139)
You understand correctly. I think it was more important when we were adding more then one tag to instances. Ill go ahead and remove it. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/139#issuecomment-104038082
Re: [jclouds-labs-google] WIP: JCLOUDS-908 fix to InstanceToNodeMetadata (#139)
Also note: As of #138 the functionality for filtering out firewall-tags is broken because there is no longer a '-port-' in the tag name. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/139#issuecomment-104032221
[jclouds-labs-google] WIP: JCLOUDS-908 fix to InstanceToNodeMetadata (#139)
This is probably also the root cause of JCLOUDS-657 (unconfirmed) InstanceToNodeMetadata filters out firewall tags added to instances. This is causing BaseComputeService#listNodes to fail when it attempts to list nodes like j-t2. When constructing a `Predicate isFirewallTag` DnsNameValidator is called and an IllegalArgumentException is thrown. Possible solutions: - We could remove the functionality for filtering out firewall tags now that there is only one firewall tag added to each instance instead of one per inbound port. - We could add a try/catch block and ignore the IllegalArgumentException. I lean toward the first option of removing functionality for filtering out tags added by jclouds. @nacx WDYT? You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/139 -- Commit Summary -- * fix to InstanceToNodeMetadata -- File Changes -- M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/FirewallTagNamingConvention.java (3) M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/InstanceToNodeMetadata.java (16) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/139.patch https://github.com/jclouds/jclouds-labs-google/pull/139.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/139
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
Yes, when you create a new project the default network is created for you. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/138#issuecomment-103589491
Re: [jclouds-labs-google] Add support for disks & ip forward during node creation (#135)
Whats the status here? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/135#issuecomment-101833908
Re: [jclouds-examples] remove old google example (#80)
I think so. WDYT? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/80#issuecomment-101818345
Re: [jclouds-labs-google] Updates to InstanceApi. 100% coverage (#110)
You are correct. This should change. I think NewInstance needs a Builder pattern so that you can customize new instances more than the two creation options there are now. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/110#issuecomment-101749778
[jclouds-examples] remove old google example (#80)
I propose we remove the older google example because it is more difficult to run You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-examples/pull/80 -- Commit Summary -- * remove old google example -- File Changes -- D google/pom.xml (68) D google/src/main/java/org/jclouds/examples/google/computeengine/Constants.java (32) D google/src/main/java/org/jclouds/examples/google/computeengine/CreateServer.java (240) D google/src/main/java/org/jclouds/examples/google/computeengine/DeleteServer.java (102) D google/src/main/java/org/jclouds/examples/google/computeengine/ExecuteCommand.java (181) -- Patch Links -- https://github.com/jclouds/jclouds-examples/pull/80.patch https://github.com/jclouds/jclouds-examples/pull/80.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/80
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
any update here? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/138#issuecomment-100322860
Re: [jclouds] JCLOUDS-894: Expose multipart upload component operations (#737)
Thanks for the heads up on this! I added the unimplemented placeholder like Atmos and will update in the future. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/737#issuecomment-98610063
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
>FirewallApi firewallApi = api.firewalls();
>
>for (Firewall firewall : concat(firewallApi.list())) {
> - if (firewall == null ||
> !firewall.network().equals(network.selfLink())) {
> + if (firewall == null) {
It should not and I was unable to reproduce. Removing this check.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/138/files#r29568927
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
> */
> - private Network getOrCreateNetwork(GoogleComputeEngineTemplateOptions
> templateOptions, String sharedResourceName) {
> - String networkName = templateOptions.network() != null ?
> toName(templateOptions.network()) : sharedResourceName;
> - return
> networkMap.getUnchecked(NetworkAndAddressRange.create(networkName,
> DEFAULT_INTERNAL_NETWORK_RANGE, null));
> + private Network getNetwork(Set networks) {
> + String networkName;
> + if (networks == null || networks.isEmpty()){
> + networkName = DEFAULT_NETWORK_NAME;
> + }
> + else {
> + Iterator iterator = networks.iterator();
> + networkName = nameFromNetworkString(iterator.next());
> + assert !iterator.hasNext() : "Error: Please specify only one
> network in TemplateOptions when using GCE.";
Updating to IllegalArgumentException that is more applicable in this case.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/138/files#r29568484
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
> */
> - private Network getOrCreateNetwork(GoogleComputeEngineTemplateOptions
> templateOptions, String sharedResourceName) {
> - String networkName = templateOptions.network() != null ?
> toName(templateOptions.network()) : sharedResourceName;
> - return
> networkMap.getUnchecked(NetworkAndAddressRange.create(networkName,
> DEFAULT_INTERNAL_NETWORK_RANGE, null));
> + private Network getNetwork(Set networks) {
> + String networkName;
> + if (networks == null || networks.isEmpty()){
> + networkName = DEFAULT_NETWORK_NAME;
> + }
> + else {
> + Iterator iterator = networks.iterator();
> + networkName = nameFromNetworkString(iterator.next());
> + assert !iterator.hasNext() : "Error: Please specify only one
> network in TemplateOptions when using GCE.";
> + }
> + Network network = api.networks().get(networkName);
> + return checkNotNull(network, "Error: no network with name %s was
> found", networkName);
Agreed. Updating.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/138/files#r29568443
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
> @@ -125,37 +124,30 @@
> protected synchronized void cleanUpIncidentalResourcesOfDeadNodes(Set extends NodeMetadata> deadNodes) {
>Set orphanedGroups = findOrphanedGroups.apply(deadNodes);
>for (String orphanedGroup : orphanedGroups) {
> - cleanUpNetworksAndFirewallsForGroup(orphanedGroup);
> + cleanUpFirewallsForGroup(orphanedGroup);
It appears to be called when I run the tests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/138/files#r29568431
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
>}
>
> - for (AtomicReference operation : operations) {
> + List ports = simplifyPorts(inboundPorts);
> + String name = naming.name(ports);
> + Firewall firewall = firewallApi.get(name);
> + AtomicReference operation = null;
> + if (firewall == null){
If firewall is not null then the specific firewall we are about to create
already exists. We are appending a hash to the end of the firewall name to both
make the name unique and indicate which ports it opens to tcp and udp. If a
firewall with the name already exists then we assume it opens the correct ports
and has the correct tag.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/138/files#r29568413
Re: [jclouds-examples] GCS-Example initial commit (#66)
It would be great to have a working well documented GCS example. @hsbhathiya I encourage you to follow up on this! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/66#issuecomment-98595975
[jclouds-site] Updating Google guide (#165)
danbroudy wants to merge 1 commit into jclouds:master from danbroudy:googleGuide: You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-site/pull/165 -- Commit Summary -- * updating google guide -- File Changes -- M guides/google.md (35) -- Patch Links -- https://github.com/jclouds/jclouds-site/pull/165.patch https://github.com/jclouds/jclouds-site/pull/165.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/165
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
Updated to remove Network from GoogleComputeEngineTemplateOptions [JCLOUDS-896](https://issues.apache.org/jira/browse/JCLOUDS-896). --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/138#issuecomment-98236147
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
rebuild please --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/138#issuecomment-98234936
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
I would do that update in a subsequent PR. This PR is ready for review. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/138#issuecomment-98266286
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
rebuild --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/138#issuecomment-98234808
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
Now that there is only one firewall per group we could remove the `FirewallTagNamingConvention`. I would probably implement this by creating one firewall rule with a `GroupNamingConvention.sharedNameForGroup()` for the firewall name and the group name as a target tag. This is problematic if users create multiple groups with the same name and different inbound ports. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/138#issuecomment-98266008
[jclouds-examples] Update compute-basic to use JSON credential (#77)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-examples/pull/77 -- Commit Summary -- * Update compute-basic to use JSON credential -- File Changes -- M compute-basics/README.md (4) M compute-basics/src/main/java/org/jclouds/examples/compute/basics/MainApp.java (37) -- Patch Links -- https://github.com/jclouds/jclouds-examples/pull/77.patch https://github.com/jclouds/jclouds-examples/pull/77.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/77
Re: [jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
Turns out there was a problem extracting the group name from firewall rules with names like `-port- `. I removed port from the name and that should fix the problem. I also updated the mock test that was failing for a similar reason. I will update this again later this afternoon with a few more fixes. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/138#issuecomment-97981986
Re: [jclouds-examples] adding google-lb example (#75)
Lets leave it as is then. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/75#issuecomment-96800586
[jclouds-labs-google] WIP: JCLOUDS-892 Compute Abstraction Network and Firewall changes (#138)
This is a WIP branch. This branch changes the compute abstraction to use the default network when creating nodes in a group. A user can still create nodes in a different network by creating a network and then passing in the URI through `GoogleComputeEngineTemplateOptions`. Node deletion and clean up makes no attempt to delete networks. This change encourages users to use the default network. If users want to use a different network then they are in charge of creating and destroying the network. This PR also addresses the issue of creating one firewall rule per inbound port. There is still work to be done. WDYT? You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/138 -- Commit Summary -- * stashing changes, now uses default network * removing network management -- File Changes -- M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineService.java (36) M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceAdapter.java (13) M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/config/GoogleComputeEngineServiceContextModule.java (18) D google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/CreateNetworkIfNeeded.java (74) D google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/FindNetworkOrCreate.java (45) M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/FirewallTagNamingConvention.java (14) M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/InstanceToNodeMetadata.java (1) M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/strategy/CreateNodesWithGroupEncodedIntoNameThenAddToSet.java (107) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceMockTest.java (51) D google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/functions/CreateNetworkIfNeededTest.java (130) D google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/functions/FindNetworkOrCreateTest.java (130) A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/SimplifyPortsTest.java (54) M google-compute-engine/src/test/resources/GoogleComputeEngineServiceExpectTest/firewall_list.json (3) M google-compute-engine/src/test/resources/firewall_insert_2.json (4) M google-compute-engine/src/test/resources/instance_insert_2.json (2) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/138.patch https://github.com/jclouds/jclouds-labs-google/pull/138.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/138
Re: [jclouds-examples] adding google-lb example (#75)
Updated to remove some dependencies. Question: Do you think I should also remove the logback-classic dependency and the logback.xml file because I'm not using it or should I leave it as a part of the example? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/75#issuecomment-96012478
Re: [jclouds-examples] adding google-lb example (#75)
> + > +--> > +http://maven.apache.org/POM/4.0.0"; > xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; > + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 > http://maven.apache.org/xsd/maven-4.0.0.xsd";> > + 4.0.0 > + org.apache.jclouds.examples > + google-lb > + 1.9.0 > + google-lb > + jclouds-labs-google example that shows using the compute > specific api and constructing a load balancer. > + > + > +1.9.0 > + > + > + Sounds good to me. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/75/files#r29066565
[jclouds-examples] adding google-lb example (#75)
Add an example of using JSON keys to authenticate to Google Compute Engine. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-examples/pull/75 -- Commit Summary -- * adding google-lb example -- File Changes -- A google-lb/README.md (47) A google-lb/pom.xml (182) A google-lb/src/main/assembly/jar-with-dependencies.xml (42) A google-lb/src/main/java/org/jclouds/examples/google/lb/MainApp.java (274) A google-lb/src/main/resources/logback.xml (36) -- Patch Links -- https://github.com/jclouds/jclouds-examples/pull/75.patch https://github.com/jclouds/jclouds-examples/pull/75.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/75
Re: [jclouds-labs-google] JCLOUDS-683: Use OAuth from the main repo (#136)
Just one live test should be enough to show its working. Thanks for doing this! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/136#issuecomment-93556511
Re: [jclouds-labs-google] JCLOUDS-683: Use OAuth from the main repo (#136)
Did you run live tests? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/136#issuecomment-93553884
Re: [jclouds-labs-google] [JCLOUDS-442] reduces the firewalls created by createNodesInGroup (#20)
Happy to help! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/20#issuecomment-93213658
Re: [jclouds-labs-google] [JCLOUDS-442] reduces the firewalls created by createNodesInGroup (#20)
> retry(operationDonePredicate, operationCompleteCheckTimeout,
> operationCompleteCheckInterval,
> MILLISECONDS).apply(operation);
> - checkState(!operation.get().getHttpError().isPresent(),"Could not
> create firewall, operation failed" + operation);
> + checkState(!operation.get().getHttpError().isPresent(), "Could not
> patch firewall, operation failed " + operation);
> + }
> + }
> +
> + private Set createFirewallRulesFromInboundPorts(int[] inboundPorts)
> {
> + Set rules = Sets.newLinkedHashSet();
> + for (int port : inboundPorts) {
> + rules.add(Rule.permitTcpRule(port));
This shouldn't add one rule per port. Rules can specify arrays of ranges. We
need some logic to find the minimal number of rules for the given inbound
ports. We should specify one rule for Tcp and one rule for Udp and each of
those rules should enumerate all the ports.
using the ports form this pastebin as an example: http://pastebin.com/3WxHWj2X
[22, 4369, 6000-7999, 8087, 8093, 8098-8099, 8985] covers all the ports
```
"allowed": [
{
"IPProtocol": "tcp",
"ports":["22", "4369", "6000-7999", "8087", "8093", "8098-8099", "8985"],
},
{
"IPProtocol": "udp",
"ports": ["22", "4369", "6000-7999", "8087", "8093", "8098-8099", "8985"] ,
}
]
```
https://cloud.google.com/compute/docs/networking#firewalls_1 is a good resource
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/20/files#r28387912
Re: [jclouds-labs-google] [JCLOUDS-442] reduces the firewalls created by createNodesInGroup (#20)
> * @see
> org.jclouds.googlecomputeengine.features.FirewallApi#patch(String,
> org.jclouds.googlecomputeengine.options.FirewallOptions)
> */
> - private void getOrCreateFirewalls(GoogleComputeEngineTemplateOptions
> templateOptions, Network network,
> - FirewallTagNamingConvention naming) {
> -
> + private void
> getAndUpdateOrCreateFirewalls(GoogleComputeEngineTemplateOptions
> templateOptions, Network network,
> + String sharedResourceName) {
> + String firewallName =
> templateOptions.getNetworkName().or(sharedResourceName);
Ping. Seeing jclouds-labs-google was not promoted to core if we want to make
these kinds of changes now is the time to do it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/20/files#r28215641
Re: [jclouds-labs-google] Add support for disks & ip forward during node creation (#135)
> + this.diskMode = diskMode;
> + this.isBootDisk = isBootDisk;
> + this.diskSizeGb = diskSizeGb;
> + }
> +
> + public AutoCreateDiskOptions(final AttachDisk.Type diskType, final
> AttachDisk.Mode diskMode, final boolean isBootDisk, final int diskSizeGb,
> final String diskName) {
> + this(diskType, diskMode, isBootDisk, diskSizeGb);
> + // TODO: Validate against regex according to GCE spec.
> + this.diskName = diskName;
> + }
> +
> + public String getDiskName(final String nodeName) {
> + if (null != diskName)
> +return diskName;
> + else
> +return "jclouds-" + UUID.randomUUID().toString();
Do you think it makes sense to include some aspect of the nodeName in the
diskName? I am still figuring out the jclouds resource naming
conventions/scheme.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/135/files#r28214469
Re: [jclouds-labs-google] Add support for disks & ip forward during node creation (#135)
> +
> + public AutoCreateDiskOptions getAutoCreateDiskOptions() {
> + return autoCreateDiskOptions;
> + }
> +
> + /**
> +* @return the AttachDisks for this instance.
> +*/
> + public List getDisks() {
> + return disks;
> + }
> +
> + // /**
> + // * @see #shouldKeepBootDisk()
> + // */
> + // public GoogleComputeEngineTemplateOptions keepBootDisk(boolean
In general there isn't commented out code in the repo so these will have to go
before check-in.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/135/files#r28214395
Re: [jclouds-labs-google] Add support for disks & ip forward during node creation (#135)
> + }
> +
> + // Add metadata from template and for ssh key and image id
> + newInstance.metadata().putAll(options.getUserMetadata());
> +
> + LoginCredentials credentials = resolveNodeCredentials(template);
> + if (options.getPublicKey() != null) {
> + newInstance.metadata().put("sshKeys", format("%s:%s
> %s@localhost", credentials.getUser(), options.getPublicKey(),
> credentials.getUser()));
> + }
> +
> + String zone = template.getLocation().getId();
> + InstanceApi instanceApi = api.instancesInZone(zone);
> + Operation create = instanceApi.create(newInstance);
> +
> + // TODO: What does this AtomicReference do?
> + // TODO: What's the differenc between AttachDisk and
> Instance.AttachedDisk and can they be refactored together?
Refactoring Instance.AttachedDisk and AttachDisk to be one object is a good
idea. I'm not sure what this AtomicReference does, the comment is a little
cryptic.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/135/files#r28214376
Re: [jclouds-labs-google] Add support for disks & ip forward during node creation (#135)
> + @Override public NodeAndInitialCredentials
> createNodeWithGroupEncodedIntoName(String group, String name, Template
> template) {
> + GoogleComputeEngineTemplateOptions options =
> GoogleComputeEngineTemplateOptions.class.cast(template.getOptions());
> + checkNotNull(options.network(), "template options must specify
> a network");
> + checkNotNull(template.getHardware().getUri(), "hardware must
> have a URI");
> + checkNotNull(template.getImage().getUri(), "image URI is null");
> +
> + List disks = Lists.newArrayList();
> + disks.add(AttachDisk.newBootDisk(template.getImage().getUri()));
> +
> + disks.addAll(options.getDisks());
> + if (null != options.getAutoCreateDiskOptions()) {
> + AutoCreateDiskOptions diskOptions =
> options.getAutoCreateDiskOptions();
> + checkArgument(template.getLocation().getScope() ==
> LocationScope.ZONE);
> + DiskApi diskApi =
> api.disksInZone(template.getLocation().getId());
> + Operation op =
> diskApi.create(diskOptions.getDiskName(name), new
> DiskCreationOptions.Builder().sizeGb(diskOptions.diskSizeGb).build());
> + // TODO: Is there a way to avoid the op, and just
> AttachDisk.create? where do we get the URI from? does it have to pre-exist?
I don't think there is a way to avoid the operation. You have to create the
disk before attaching it.
https://cloud.google.com/compute/docs/instances#startinstanceapi
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/135/files#r28214283
Re: [jclouds-labs-google] Fix if-statement that always returns true (#131)
That was my understanding as well. Sounds correct. On Thu, Mar 26, 2015 at 12:39 PM, Andrew Gaul wrote: > [image: :+1:] waiting for master to reopen before pushing. > > @snotling <https://github.com/snotling> I think the term multipart is > overloaded here. jclouds single-part upload should map to a GCS multi-part > upload which sends a metadata part and a data part. jclouds multi-part > upload should map to multiple jclouds single-part uploads followed by a GCS > compose operation. > > @hsbhathiya <https://github.com/hsbhathiya> @danbroudy > <https://github.com/danbroudy> Does this sound correct? > > — > Reply to this email directly or view it on GitHub > <https://github.com/jclouds/jclouds-labs-google/pull/131#issuecomment-86686449> > . > --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/131#issuecomment-86691675
[jclouds-labs-google] JCLOUDS-805: GCS update SequentialMultipartUploadStrategy and MultipartU... (#128)
...ploadSlicingAlgorithm This is a WIP branch. I am curious if I am missing something substantial and if diverging from the other MultipartUploadSlicingAlgorithm examples in the jclouds repo is discouraged. Was algorithm.getParts() used for a particular reason? You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/128 -- Commit Summary -- * JCLOUDS-805: GCS update SequentialMultipartUploadStrategy and MultipartUploadSlicingAlgorithm -- File Changes -- M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/strategy/internal/MultipartUploadSlicingAlgorithm.java (72) M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/blobstore/strategy/internal/SequentialMultipartUploadStrategy.java (12) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/blobstore/integration/GoogleCloudStorageBlobIntegrationLiveTest.java (27) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/128.patch https://github.com/jclouds/jclouds-labs-google/pull/128.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/128
Re: [jclouds-labs-google] Json Keys for running GCS tests + BaseGoogleCloudStorageApiMockTest + ObjectApiMockTest. (#127)
> + .predefinedAcl(PredefinedAcl.PUBLIC_READ_WRITE);
> +
> + assertEquals(objectApi().simpleUpload("bucket_name", "text/plain",
> +p.getPayload().getContentMetadata().getContentLength(),
> p.getPayload(), options),
> +new ParseGoogleCloudStorageObject().expected());
> +
> + RecordedRequest request = assertSent(server, "POST",
> "/upload/storage/v1/b/bucket_name/o" +
> + "?uploadType=media&name=new_object&predefinedAcl=publicReadWrite",
> null);
> + assertEquals(request.getHeader("Content-Type"), "text/plain");
> + assertEquals(new String(request.getBody(), UTF_8), testPayload);
> + }
> +
> + public void delete() throws Exception {
> + server.enqueue(new MockResponse());
> +
> + // Question: Should this be returning True on not found?
This is the same question raised in #122. Happy to change a TODO.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/127/files#r24617783
Re: [jclouds-labs-google] Json Keys for running GCS tests + BaseGoogleCloudStorageApiMockTest + ObjectApiMockTest. (#127)
Convenience. I am happy to break them apart if that would be easier! I got used to running the tests with the JSON format. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/127#issuecomment-74147193
Re: [jclouds-labs-google] Json Keys for running GCS tests + BaseGoogleCloudStorageApiMockTest + ObjectApiMockTest. (#127)
> @@ -37,8 +37,8 @@ > private String contentType; > private String crc32c; > private String md5Hash; > - private Map metadata = Maps.newLinkedHashMap(); > - private List acl = Lists.newArrayList(); > + private Map metadata; > + private List acl; Initializing these as empty overwrites metadata and acls when using the PATCH operation. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/127/files#r24617654
Re: [jclouds-labs-google] Json Keys for running GCS tests + BaseGoogleCloudStorageApiMockTest + ObjectApiMockTest. (#127)
Ran Live tests for Storage:
```
Results :
Failed tests:
GoogleCloudStorageBlobIntegrationLiveTest>BaseBlobIntegrationTest.testPutMultipartInputStream:555->BaseBlobIntegrationTest.testPut:575
» HttpResponse
BucketApiLiveTest.testCreateBucket:88 expected object to not be null
GoogleCloudStorageBlobIntegrationLiveTest.testMultipartChunkedFileStream:235->addMultipartBlobToContainer:253
» HttpResponse
BucketApiLiveTest.testUpdateBucketWithOptions:146 » Authorization {
"error": ...
ObjectApiLiveTest.testUpdateObject:292 » Authorization {
"error": {
"errors...
ObjectApiLiveTest.deleteBucket:424 » IllegalState {
"error": {
"errors": [
...
Tests run: 101, Failures: 6, Errors: 0, Skipped: 22
```
Existing Jira Issue tracking this - JCLOUDS-805
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/127#issuecomment-73996847
[jclouds-labs-google] Json Keys for running GCS tests + BaseGoogleCloudStorageApiMockTest + ObjectApiMockTest. (#127)
This PR enables running the GCS test with the Json Key format To do so you provide test.google-cloud.json-key as a parameter with the path to the .json file as the value. This is relevant to #112 and JCLOUDS-805 You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/127 -- Commit Summary -- * enabling running live tests with .json key format * Adding BaseGoogleCloudStorageApiMockTest + ObjectApiMockTest -- File Changes -- M google-cloud-storage/pom.xml (6) M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/ListPageWithPrefixes.java (2) M google-cloud-storage/src/main/java/org/jclouds/googlecloudstorage/domain/templates/ObjectTemplate.java (16) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/blobstore/integration/GoogleCloudStorageBlobIntegrationLiveTest.java (1) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/blobstore/integration/GoogleCloudStorageBlobLiveTest.java (1) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/blobstore/integration/GoogleCloudStorageContainerIntegrationLiveTest.java (1) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/blobstore/integration/GoogleCloudStorageContainerLiveTest.java (1) A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/features/ObjectApiMockTest.java (247) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/internal/BaseGoogleCloudStorageApiLiveTest.java (8) A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/internal/BaseGoogleCloudStorageApiMockTest.java (138) M google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/internal/BaseGoogleCloudStorageParseTest.java (7) A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/ParseGoogleCloudStorageObject.java (66) A google-cloud-storage/src/test/java/org/jclouds/googlecloudstorage/parse/ParseGoogleCloudStorageObjectListTest.java (99) A google-cloud-storage/src/test/resources/object_compose_request.json (54) A google-cloud-storage/src/test/resources/object_get.json (21) A google-cloud-storage/src/test/resources/object_list.json (47) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/127.patch https://github.com/jclouds/jclouds-labs-google/pull/127.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/127
Re: [jclouds-labs-google] Updated BackendServiceOptions to AutoValue + Builder (#125)
I am planning to add more code. I apologize for the the delay. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/125#issuecomment-73933342
Re: [jclouds-labs-google] Updated BackendServiceOptions to AutoValue + Builder (#125)
Happy to squash. I figured it may be easier to review in individual commits. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/125#issuecomment-73953109
Re: [jclouds-labs-google] Make deleteObject() return false on 404 response (#122)
Im adding ObjectApiMockTest because there doesn't appear to be a unit test and it would be good to convert GCS to Mock --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/122#issuecomment-73935078
[jclouds-labs-google] adding start and stop to Instance Api (#126)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/126 -- Commit Summary -- * adding start and stop to Instance Api -- File Changes -- M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java (24) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java (29) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiMockTest.java (18) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/126.patch https://github.com/jclouds/jclouds-labs-google/pull/126.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/126
Re: [jclouds-labs-google] JCLOUDS-808: Enables working with .json key files, adding GoogleComputeEngineCredenti... (#124)
I set it up in a way that it would be easy to extend to google cloud storage. It is not currently being used with google cloud storage in this commit. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/124#issuecomment-72570969
Re: [jclouds-labs-google] JCLOUDS-808: Enables working with .json key files, adding GoogleComputeEngineCredenti... (#124)
Updated. Thanks for the feedback! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/124#issuecomment-72253640
Re: [jclouds-labs-google] JCLOUDS-808: Enables working with .json key files, adding GoogleComputeEngineCredenti... (#124)
I moved GoogleCredentalsFromJson into googlecloud so that in the future it can be used with google-cloud-storage. Happy to squash if you agree this move is a good idea. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/124#issuecomment-72091083
Re: [jclouds-labs-google] JCLOUDS-808: Enables working with .json key files, adding GoogleComputeEngineCredenti... (#124)
Im now looking into integrating with GCS. Not ready to merge yet --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/124#issuecomment-71950645
Re: [jclouds-labs-google] JCLOUDS-808: Enables working with .json key files, adding GoogleComputeEngineCredenti... (#124)
> + * this work for additional information regarding copyright ownership. > + * The ASF licenses this file to You under the Apache License, Version 2.0 > + * (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > +package org.jclouds.googlecomputeengine; > + > +import org.apache.commons.lang.StringEscapeUtils; Dependency removed. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/124/files#r23739061
Re: [jclouds-labs-google] JCLOUDS-808: Enables working with .json key files, adding GoogleComputeEngineCredenti... (#124)
> +
> + private GoogleComputeEngineCredentialSupplierFromJson(String jsonString){
> + creds = parseJsonKeyString(jsonString);
> + }
> +
> + /**
> +* Function for parsing JSON Key file downloaded from GCP developers
> console.
> +*
> +* @param pathToJsonFile.
> +* @return Credentials object with Credentials.identity and
> Credentials.credential correctly set.
> +*/
> + public static Credentials parseJsonKeyString(String jsonString) {
> + // Parse JsonFile to extract Service Account and PrivateKey.
> + final JsonObject json = new
> JsonParser().parse(jsonString).getAsJsonObject();
> + String client_email =
> json.get("client_email").toString().replace("\"", "");
> + String private_key =
> StringEscapeUtils.unescapeJava(json.get("private_key").toString().replace("\"",
> ""));
Without the unescapeJava it was was incorrectly treating \n as two separate
characters instead of newlines. The client email doesn't have any \n characters
so it is a non-issue.
I found a simpler way to fix the newline character.
`json.get("private_key").toString().replace("\"", "").replace("\\n", "\n");`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/124/files#r23739041
Re: [jclouds-labs-google] JCLOUDS-808: Enables working with .json key files, adding GoogleComputeEngineCredenti... (#124)
I think `GoogleComputeEngineCredentialSupplierFromJson` is too long of a name and I am open to suggestions. Also passing in a JSON string into the eclipse maven build was causing problems so I left it that you pass in a file when running the tests but pass in a JSON string when instantiating a `GoogleComputeEngineCredentialSupplierFromJson` normally --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/124#issuecomment-71896830
[jclouds-labs-google] Updated BackendServiceOptions to AutoValue + Builder (#125)
Converts another Options class to AutoValue + Builder For consistency I think all Options classes should be converted to AutoValue + Builder. Sound good? Should I continue down this path? You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs-google/pull/125 -- Commit Summary -- * Updated BackendServiceOptions to AutoValue + Builder -- File Changes -- M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/options/BackendServiceOptions.java (279) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/BackendServiceApiLiveTest.java (29) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/BackendServiceApiMockTest.java (12) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/GlobalForwardingRuleApiLiveTest.java (3) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/TargetHttpProxyApiLiveTest.java (4) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/UrlMapApiLiveTest.java (3) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/125.patch https://github.com/jclouds/jclouds-labs-google/pull/125.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/125
Re: [jclouds-labs-google] [JCLOUDS-442] reduces the firewalls created by createNodesInGroup (#20)
I agree this is still an issue! Im a little foggy on what is a problematic call, how Jclouds handles it before this PR and how it will handle it afterwards. Ill look into the code a little more. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/20#issuecomment-71746467
[jclouds-labs-google] JCLOUDS-808: Enables working with .json key files, adding GoogleComputeEngineCredenti... (#124)
...alSupplier
This is WIP branch with an idea of how to move towards using JSON key files
instead of PEM keys.
Something along the lines of the following:
```
ComputeServiceContext context =
ContextBuilder.newBuilder("google-compute-engine")
.credentialsSupplier(new
GoogleComputeEngineCredentialSupplier("/path/to/file.json))
.buildView(ComputeServiceContext.class);
```
This also (optionally) changes how the live tests are run.
If you set the parameter `test.google-compute-engine.json-key` to
/path/to/file.json it will populate
`test.google-compute-engine.identity` and
`test.google-compute-engine.credential` from the file.
This will also require documentation updates ect.
@nacx WDYT?
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs-google/pull/124
-- Commit Summary --
* Enables working with .json key files, adding
GoogleComputeEngineCredentialSupplier
-- File Changes --
A
google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineCredentialSupplier.java
(73)
M
google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceLiveTest.java
(18)
M
google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/internal/BaseGoogleComputeEngineApiLiveTest.java
(2)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/124.patch
https://github.com/jclouds/jclouds-labs-google/pull/124.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/124
[jclouds-labs-google] Fixed InstanceApiLiveTest, made Instance.status @Nullable (#120)
Fixes bug in InstanceApiLiveTest. Also Instance Status is sometimes not returned when repeatedly calling instance get and the instance status is transitioning from one state to another. This was causing intermittent test failures. You can merge this Pull Request by running: git pull https://github.com/GoogleCloudPlatform/jclouds-labs-google liveTestFix Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-labs-google/pull/120 -- Commit Summary -- * Fixed InstanceApiLiveTest, made Instance.status @Nullable -- File Changes -- M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Instance.java (2) M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java (7) -- Patch Links -- https://github.com/jclouds/jclouds-labs-google/pull/120.patch https://github.com/jclouds/jclouds-labs-google/pull/120.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/120
Re: [jclouds-labs-google] Added Project:setUsageExportBucket and ProjectApiMockTest (#112)
> @@ -66,4 +67,14 @@ public void testDeleteItemFromMetadata() {
>assertEquals(project.commonInstanceMetadata().size(),
> initialMetadataSize);
>assertEquals(project.commonInstanceMetadata().fingerprint(),
> initialFingerprint);
> }
> +
> + @Test(groups = "live", dependsOnMethods = "getProject")
> + public void testSetUsageExportBucket() {
> + Operation o = api.project().setUsageExportBucket("test-bucket",
> "test-");
> + while (o.error().errors().isEmpty()) {
> + o = api.operations().get(o.selfLink());
> + }
The loop is because the `waitOperationDone` times out. I wanted a way to test
`setUsageExportBucket ` without setting up a storage bucket that the project
has "owner" access to. I thought a reasonable way to do this would be to point
to an invalid bucket and check that the error messages are as expected.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/112/files#r22561159
Re: [jclouds-labs-google] Added Project:setUsageExportBucket and ProjectApiMockTest (#112)
> @@ -63,11 +63,6 @@ > protected Predicate> operationDone; > protected URI projectUrl; > > - protected Supplier userProject; > - protected Predicate> > globalOperationDonePredicate; > - protected Predicate> > regionOperationDonePredicate; > - protected Predicate> > zoneOperationDonePredicate; Live tests unaffected by deletion. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/112/files#r22560745
Re: [jclouds-labs-google] Added Project:setUsageExportBucket and ProjectApiMockTest (#112)
> @@ -63,11 +63,6 @@ > protected Predicate> operationDone; > protected URI projectUrl; > > - protected Supplier userProject; > - protected Predicate> > globalOperationDonePredicate; > - protected Predicate> > regionOperationDonePredicate; > - protected Predicate> > zoneOperationDonePredicate; These appear to be unused. Eclipse did not flag them as unused which gave me pause. Im running the live tests to check. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/112/files#r22558445
Re: [jclouds-labs-google] Added Project:setUsageExportBucket and ProjectApiMockTest (#112)
> @@ -66,4 +68,15 @@ public void testDeleteItemFromMetadata() {
>assertEquals(project.commonInstanceMetadata().size(),
> initialMetadataSize);
>assertEquals(project.commonInstanceMetadata().fingerprint(),
> initialFingerprint);
> }
> +
> + @Test(groups = "live", dependsOnMethods = "getProject")
> + public void testSetUsageExportBucket() {
> + Operation o = api.project().setUsageExportBucket("test-bucket",
> "test-");
> +
> + while (o.status() == Status.PENDING) {
I updated this to what may be a more agreeable while loop.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/112/files#r22558322
Re: [jclouds-labs-google] Options cleanup (#117)
Squashed --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/117#issuecomment-68948108
Re: [jclouds-labs-google] Fixed instance authentication (#118)
Tests running now but I'm pretty confident that [this](https://github.com/danbroudy/jclouds-labs-google/commits/compute-auth) branch will now pass all live test! Feel free to do what you want with the minor edits. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/118#issuecomment-68017034
Re: [jclouds-labs-google] Fixed instance authentication (#118)
Looks like the suggestion to add ``` eTo.loginPassword = fromNullable(this.getLoginPassword()); eTo.loginPrivateKey = fromNullable(this.getLoginPrivateKey()); ``` was not a good one. When I run all the tests together these lines cause failure instead of fix it. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/118#issuecomment-68015563
Re: [jclouds-labs-google] Fixed instance authentication (#118)
Good news! Looks like it is a very minor issue.
Its two problems I encountered when I was taking a swing at this.
One problem is getting the "options did not clone correctly" warning when I try
to run the `GoogleComputeEngineServiceLiveTest` on its own.
This is due to comparing null to an Optional.Absent()
The fix:
Add in GoogleComputeEngineTemplateOptions.java
`import static com.google.common.base.Optional.fromNullable;`
```
public void copyTo(TemplateOptions to) {
.
.
.
eTo.loginPassword = fromNullable(this.getLoginPassword());
eTo.loginPrivateKey = fromNullable(this.getLoginPrivateKey());
}
```
The other issue is that rarely (but it happens) Instance.get does not return a
status so we need to make status Nullable in Instance.java
Instances.java
228: `@Nullable public abstract Status status();`
If that wasn't clear, I have the changes in a branch
[here](https://github.com/danbroudy/jclouds-labs-google/commits/compute-auth)
Sound good?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/118#issuecomment-68013457
Re: [jclouds-labs-google] Options cleanup (#117)
Im surprised to see this fail. It looks like it is a storage test that is unrelated. Ill do another push to trigger it again to see if it is a persistent problem. One problem the most recent commit resolves is the TargetHttpProxyLiveTest. It tries to create the same UrlMap twice which is causing it to fail the second time. This was then leading to the resources not being deleted so the first test fails the next time. Warning there may be some resources out there on your project that are worth deleting! Another point of interest: UrlMapOptions are used for both Insert and Patch operations. This means sometimes name and defaultService are required (in the insert case) and sometimes they are not (in the patch case). I attempted to resolve this with two different well documented build options. Is this a good way to handle cases like this? Is this an improvement? WDYT? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-google/pull/117#issuecomment-68011936
