You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Ignasi Barrera <no...@github.com> on 2016/01/22 14:54:24 UTC
[jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
* [JCLOUDS-1051](https://issues.apache.org/jira/browse/JCLOUDS-1051): Change the SSh dependencies to JSCH to avoid the errors
caused by the absence of a bundle for the new SSHJ dependency groupId.
* [JCLOUDS-1056](https://issues.apache.org/jira/browse/JCLOUDS-1056): Added digitalocean2 and google-compute-engine.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-karaf/pull/72
-- Commit Summary --
* Provider update and dependency fixes
-- File Changes --
M commands/pom.xml (2)
M commands/src/main/java/org/jclouds/karaf/commands/compute/ComputeCommandWithOptions.java (13)
M feature/pom.xml (4)
M feature/src/main/resources/feature.xml (28)
D itests/src/test/java/org/jclouds/karaf/itests/HPCloudFeaturesInstallationTest.java (46)
M itests/src/test/java/org/jclouds/karaf/itests/MiscFeaturesInstallationTest.java (9)
M pom.xml (30)
M services/pom.xml (2)
M services/src/main/java/org/jclouds/karaf/services/internal/ComputeServiceFactory.java (19)
M utils/pom.xml (4)
M utils/src/main/java/org/jclouds/karaf/utils/EnvHelper.java (22)
-- Patch Links --
https://github.com/jclouds/jclouds-karaf/pull/72.patch
https://github.com/jclouds/jclouds-karaf/pull/72.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Andrew Gaul <no...@github.com>.
> @@ -92,6 +98,22 @@ public static String getComputeCredential(String credential) {
> }
>
> /**
> + * Extracts the credential value from the Google Cloud credentials json file.
> + * @param jsonFile
> + * @return
> + */
> + public static String getGoogleCredentialFromJsonFile(String jsonFile) {
Do google-cloud-storage and google-compute-engine take credentials in different forms? I have been using a PEM file and not a JSON-encoded PEM file for my non-CLI testing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72/files#r50608700
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Andrew Gaul <no...@github.com>.
> @@ -92,6 +98,22 @@ public static String getComputeCredential(String credential) {
> }
>
> /**
> + * Extracts the credential value from the Google Cloud credentials json file.
> + * @param jsonFile
> + * @return
> + */
> + public static String getGoogleCredentialFromJsonFile(String jsonFile) {
Also I only profile the path to the PEM file and somehow GCS expands it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72/files#r50609203
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Andrew Phillips <no...@github.com>.
> I've refactored the PR in three commits to properly scope each change
Thanks, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72#issuecomment-174548302
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Ignasi Barrera <no...@github.com>.
Yes GCE works fine with this way of loading the credentials.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72#issuecomment-174156997
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Andrew Gaul <no...@github.com>.
> @@ -85,7 +86,13 @@ protected ComputeService getComputeService() throws IOException {
> String providerValue = EnvHelper.getComputeProvider(provider);
> String apiValue = EnvHelper.getComputeApi(api);
> String identityValue = EnvHelper.getComputeIdentity(identity);
> - String credentialValue = EnvHelper.getComputeCredential(credential);
> + String credentialValue = null;
> + if (providerValue.equals("google-compute-engine")) {
> + credentialValue = EnvHelper.getGoogleCredentialFromJsonFile(credential);
> + } else {
> + credentialValue = EnvHelper.getComputeCredential(credential);
> + }
You should expand the value of credential since the user may provide property or environment variable instead of a command line flag:
```java
String credentialValue = EnvHelper.getComputeCredential(credential);
if (providerValue.equals("google-compute-engine")) {
credentialValue = EnvHelper.getGoogleCredentialFromJsonFile(credentialValue);
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72/files#r50608536
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Ignasi Barrera <no...@github.com>.
Ok, build passed locally for me now! I've refactored the PR in three commits to properly scope each change. /cc @andrewgaul @demobox
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72#issuecomment-174471878
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Andrew Phillips <no...@github.com>.
Haven't been able to test this, but the code looks good to me. [Failures](https://jclouds.ci.cloudbees.com/job/jclouds-karaf-pull-requests/122/testReport/) don't seem obviously related to this PR, but it would still be good to know what's broken there (seeing as the 1.9.2 release tests passed).
Thanks for taking care of this, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72#issuecomment-173941894
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Andrew Gaul <no...@github.com>.
I have a similar commit for GCS at #73 which fails due to not expanding the credential properly. Even with the same code as this commit, it complains that it cannot find the `-----BEGIN` prologue. @nacx Does GCE work for you?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72#issuecomment-174098780
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -92,6 +98,22 @@ public static String getComputeCredential(String credential) {
> }
>
> /**
> + * Extracts the credential value from the Google Cloud credentials json file.
> + * @param jsonFile
> + * @return
> + */
> + public static String getGoogleCredentialFromJsonFile(String jsonFile) {
Live tests might read a file but karaf expected the actual credentals (the pem contents) and failed to parse a multi-line value. When adding the logic to read a file I added the one to read the json form, which is the Google's recommended one and also the one used in the jclouds-examples.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72/files#r50617317
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Ignasi Barrera <no...@github.com>.
/cc @demobox @iocanel @ccustine @andrewgaul
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72#issuecomment-173926837
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Andrew Gaul <no...@github.com>.
> @@ -320,11 +320,6 @@ limitations under the License.
> <bundle>mvn:org.apache.jclouds.provider/glesys/${jclouds.version}</bundle>
> </feature>
>
> - <feature name='jclouds-hpcloud-compute' description='HP Cloud Compute' version='${project.version}' resolver='(obr)'>
> - <feature version='${project.version}'>jclouds-api-openstack-nova</feature>
> - <bundle>mvn:org.apache.jclouds.provider/hpcloud-compute/${jclouds.version}</bundle>
> - </feature>
Would be nice to disentangle the removal of old providers with the addition of new ones.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72/files#r50608736
Re: [jclouds-karaf] 1051 & 1056: Fix SSH dependencies (#72)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -320,11 +320,6 @@ limitations under the License.
> <bundle>mvn:org.apache.jclouds.provider/glesys/${jclouds.version}</bundle>
> </feature>
>
> - <feature name='jclouds-hpcloud-compute' description='HP Cloud Compute' version='${project.version}' resolver='(obr)'>
> - <feature version='${project.version}'>jclouds-api-openstack-nova</feature>
> - <bundle>mvn:org.apache.jclouds.provider/hpcloud-compute/${jclouds.version}</bundle>
> - </feature>
Yeah. I'll split this into 2 PRs: the first one to fix the build (ssh deps and provider removal), and another one to add the new providers.
The PR build now complains about a google dependency config, and that shouldn't block the ssh fix. Let's properly scope each PR.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/72/files#r50617384