You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by suriyapriya <no...@github.com> on 2014/03/13 06:43:10 UTC

[jclouds-examples] support for GCE provider (#34)

You can merge this Pull Request by running:

  git pull https://github.com/suriyapriya/jclouds-examples master

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds-examples/pull/34

-- Commit Summary --

  * support for GCE provider
  * update README
  * update README
  * update README
  * update README

-- File Changes --

    M compute-basics/README.md (26)
    M compute-basics/pom.xml (24)
    M compute-basics/src/main/java/org/jclouds/examples/compute/basics/MainApp.java (40)

-- Patch Links --

https://github.com/jclouds/jclouds-examples/pull/34.patch
https://github.com/jclouds/jclouds-examples/pull/34.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
> @@ -202,6 +214,20 @@ public static void main(String[] args) {
>                    Predicates.<NodeMetadata> and(not(TERMINATED), inGroup(groupName)));
>              System.out.printf("<< destroyed nodes %s%n", destroyed);
>              break;
> +         case LISTIMAGES:
> +            Set<? extends Image> imageList = compute.listImages();

Rename to `images`? The type indicates that it's a set, so calling it a list is a bit misleading?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860329

Re: [jclouds-examples] support for GCE provider (#34)

Posted by suriyapriya <no...@github.com>.
> @@ -135,6 +144,9 @@ public static void main(String[] args) {
>              // Default template chooses the smallest size on an operating system
>              // that tested to work with java, which tends to be Ubuntu or CentOS
>              TemplateBuilder templateBuilder = compute.templateBuilder();
> +
> +            if(provider.equalsIgnoreCase("google-compute-engine"))
> +                templateBuilder.osFamily(OsFamily.CENTOS);

[JCLOUDS-495](https://issues.apache.org/jira/browse/JCLOUDS-495) is the jira issue

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10641502

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
>      </dependency>
>      <dependency>
>        <groupId>org.apache.jclouds.driver</groupId>
>        <artifactId>jclouds-enterprise</artifactId>
> -      <version>1.7.1</version>
> +      <version>${jclouds.version}</version>

Thanks for this cleanup!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860320

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Ignasi Barrera <no...@github.com>.
Thanks @suriyapriya. Merged!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34#issuecomment-37796883

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Ignasi Barrera <no...@github.com>.
Thanks! As I already merged this PR, I'd open a new one.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34#issuecomment-39300260

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Ignasi Barrera <no...@github.com>.
LGTM. Thanks @suriyapriya! Could you squash the commits into a single one so we can cleanly merge it?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34#issuecomment-37725456

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
> @@ -218,6 +244,16 @@ public static void main(String[] args) {
>        }
>     }
>  
> +   private static String getPrivateKeyFromFile(String filename) {
> +      try {
> +         return Strings2.toStringAndClose(new FileInputStream(filename));
> +      } catch (java.io.IOException e) {
> +         System.err.println("Exception : " + e);
> +         e.printStackTrace();
> +      }
> +      return null;

Will the rest of the class be OK if this returns null? If not, should we go "bang" here?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860338

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
> @@ -202,6 +214,20 @@ public static void main(String[] args) {
>                    Predicates.<NodeMetadata> and(not(TERMINATED), inGroup(groupName)));
>              System.out.printf("<< destroyed nodes %s%n", destroyed);
>              break;
> +         case LISTIMAGES:
> +            Set<? extends Image> imageList = compute.listImages();
> +            System.out.printf(">> No of images %d\n", imageList.size());
> +            for (Image img : imageList) {
> +               System.out.println(">>>>  " + img);
> +            }
> +            break;
> +         case LISTNODES:
> +            Set<? extends ComputeMetadata> nodeList = compute.listNodes();

`nodes` rather than `nodeList`?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860331

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Ignasi Barrera <no...@github.com>.
> @@ -135,6 +144,9 @@ public static void main(String[] args) {
>              // Default template chooses the smallest size on an operating system
>              // that tested to work with java, which tends to be Ubuntu or CentOS
>              TemplateBuilder templateBuilder = compute.templateBuilder();
> +
> +            if(provider.equalsIgnoreCase("google-compute-engine"))
> +                templateBuilder.osFamily(OsFamily.CENTOS);

IIRC this was needed because the GCE [default template filter](https://github.com/jclouds/jclouds-labs-google/blob/master/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/GoogleComputeEngineApiMetadata.java#L68-L69) wasn't finding any template. Mind opening a JIRA issue (if it does not exist yet) so we can properly fix it?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10634688

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
> @@ -202,6 +214,20 @@ public static void main(String[] args) {
>                    Predicates.<NodeMetadata> and(not(TERMINATED), inGroup(groupName)));
>              System.out.printf("<< destroyed nodes %s%n", destroyed);
>              break;
> +         case LISTIMAGES:
> +            Set<? extends Image> imageList = compute.listImages();
> +            System.out.printf(">> No of images %d\n", imageList.size());
> +            for (Image img : imageList) {
> +               System.out.println(">>>>  " + img);
> +            }
> +            break;
> +         case LISTNODES:
> +            Set<? extends ComputeMetadata> nodeList = compute.listNodes();
> +            System.out.printf(">> No of nodes/instances %d\n", nodeList.size());
> +            for (ComputeMetadata nodeentry : nodeList) {

`node` rather than `nodeentry`?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860335

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
> @@ -202,6 +214,20 @@ public static void main(String[] args) {
>                    Predicates.<NodeMetadata> and(not(TERMINATED), inGroup(groupName)));
>              System.out.printf("<< destroyed nodes %s%n", destroyed);
>              break;
> +         case LISTIMAGES:
> +            Set<? extends Image> imageList = compute.listImages();
> +            System.out.printf(">> No of images %d\n", imageList.size());
> +            for (Image img : imageList) {
> +               System.out.println(">>>>  " + img);
> +            }
> +            break;
> +         case LISTNODES:
> +            Set<? extends ComputeMetadata> nodeList = compute.listNodes();
> +            System.out.printf(">> No of nodes/instances %d\n", nodeList.size());

Use `%n` rather than `\n` for consistency throughout this class?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860333

Re: [jclouds-examples] support for GCE provider (#34)

Posted by suriyapriya <no...@github.com>.
@nacx followed jclouds style, thanks

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34#issuecomment-37787652

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Ignasi Barrera <no...@github.com>.
> @@ -135,6 +144,9 @@ public static void main(String[] args) {
>              // Default template chooses the smallest size on an operating system
>              // that tested to work with java, which tends to be Ubuntu or CentOS
>              TemplateBuilder templateBuilder = compute.templateBuilder();
> +
> +            if(provider.equalsIgnoreCase("google-compute-engine"))
> +                templateBuilder.osFamily(OsFamily.CENTOS);

I have a horrible memory :) Thx!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10641977

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
> @@ -107,7 +113,10 @@ public static void main(String[] args) {
>        if (action == Action.EXEC && args.length < PARAMETERS + 1)
>           throw new IllegalArgumentException("please quote the command to exec as the last parameter");
>        String command = (action == Action.EXEC) ? args[5] : "echo hello";
> -      
> +
> +      if (provider.equalsIgnoreCase("google-compute-engine"))

This comparison is repeated...extract as a variable?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860343

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
> @@ -202,6 +214,20 @@ public static void main(String[] args) {
>                    Predicates.<NodeMetadata> and(not(TERMINATED), inGroup(groupName)));
>              System.out.printf("<< destroyed nodes %s%n", destroyed);
>              break;
> +         case LISTIMAGES:
> +            Set<? extends Image> imageList = compute.listImages();
> +            System.out.printf(">> No of images %d\n", imageList.size());

Use `%n` rather than `\n` for consistency throughout this class?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860328

Re: [jclouds-examples] support for GCE provider (#34)

Posted by suriyapriya <no...@github.com>.
@demobox @nacx 
Thanks for the comments. I will fix it. Should i create a new PR or reopen this PR ?.


---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34#issuecomment-39297101

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
> @@ -218,6 +244,16 @@ public static void main(String[] args) {
>        }
>     }
>  
> +   private static String getPrivateKeyFromFile(String filename) {
> +      try {
> +         return Strings2.toStringAndClose(new FileInputStream(filename));
> +      } catch (java.io.IOException e) {
> +         System.err.println("Exception : " + e);

Change message to something like `format("Exception reading private key from '%s':", filename)`

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860337

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Ignasi Barrera <no...@github.com>.
Thanks @suriyapriya! One last minor thing: could you format your code to follow the jclouds style? Use a 3 space indent and add a space after keywords like `if`, `for`, etc.
Just change that and re-squash directly and I'll merge the PR. Thx!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34#issuecomment-37772425

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
>  To destroy all nodes of the group *mygroup*:
>  
>      java -jar target/compute-basics-jar-with-dependencies.jar provider identity credential mygroup destroy
>  
> +To list the nodes:  *groupname* parameter is not used

[minor] "To list all nodes (the *groupname* parameter is not used):"?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860316

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
>  To destroy all nodes of the group *mygroup*:
>  
>      java -jar target/compute-basics-jar-with-dependencies.jar provider identity credential mygroup destroy
>  
> +To list the nodes:  *groupname* parameter is not used
> +
> +    java -jar target/compute-basics-jar-with-dependencies.jar provider identity credential mygroup listnodes
> +
> +To list the images:  *groupname* parameter is not used

[minor] "To list all images (the *groupname* parameter is not used):"?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860318

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Ignasi Barrera <no...@github.com>.
Closed #34.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34

Re: [jclouds-examples] support for GCE provider (#34)

Posted by suriyapriya <no...@github.com>.
Squashed the commits, pls merge.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34#issuecomment-37769976

Re: [jclouds-examples] support for GCE provider (#34)

Posted by Andrew Phillips <no...@github.com>.
> @@ -135,6 +144,9 @@ public static void main(String[] args) {
>              // Default template chooses the smallest size on an operating system
>              // that tested to work with java, which tends to be Ubuntu or CentOS
>              TemplateBuilder templateBuilder = compute.templateBuilder();
> +
> +            if(provider.equalsIgnoreCase("google-compute-engine"))

[minor] Spacing: `if (provider...`

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/34/files#r10860324