Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)
+serverId = createServer(zoneId, nova,
Server.Status.ACTIVE).getId();
+Server server = serverApi.get(serverId);
+assertEquals(server.getStatus(), Server.Status.ACTIVE);
+ } finally {
+serverApi.delete(serverId);
+ }
+ }
+ }
+
+ /**
+* This needs to be supported by the provider, and is usually not
supported.
+* However this can be tested on devstack:
+* In apis/openstack-nova:
+* mvn -Plive clean install
-Dtest.openstack-nova.endpoint=http://localhost:5000/v2.0;
-Dtest.openstack-nova.identity=demo:demo
-Dtest.openstack-nova.credential=devstack
-Dtest=org.jclouds.openstack.nova.v2_0.features.ServerApiLiveTest#testCreateWithNetworkOptions
+*/
+ @Test(enabled = false)
Hm...is there some way we can programmatically test whether to run this test or
not, rather than adding a disabled test to the code base, that is liable to rot?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826500
Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)
+ withImage(server.getImage().getId()).
+ name(newName).
+ adminPass(password).
+ ipv4Address(1.1.1.1).
+ ipv6Address(fe80::100);
+
+serverApi.rebuild(serverId, options);
+
+Server rebuiltServer = serverApi.get(serverId);
+
+assertEquals(newName, rebuiltServer.getName());
+assertEquals(1.1.1.1, rebuiltServer.getAccessIPv4());
+assertEquals(fe80::100, rebuiltServer.getAccessIPv6());
+
+ } finally {
+serverApi.delete(serverId);
What if serverId is still null at this point?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826578
Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)
-}
+ }
+ }
+ }
+
+ @Test
+ public void testCreateInWrongAvailabilityZone() {
+ String serverId = null;
+ for (String zoneId : zones) {
+ ServerApi serverApi = api.getServerApiForZone(zoneId);
+ try {
+serverId = createServer(zoneId, err,
Server.Status.ERROR).getId();
+Server server = serverApi.get(serverId);
+assertEquals(server.getStatus(), Server.Status.ERROR);
+ } finally {
+serverApi.delete(serverId);
What if serverId is still null at this point?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826577
Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)
+serverApi.delete(serverId);
+ }
+ }
+ }
+
+ private Server createServer(String regionId, Server.Status serverStatus) {
+ ServerApi serverApi = api.getServerApiForZone(regionId);
+ CreateServerOptions options = new CreateServerOptions();
+ ServerCreated server = serverApi.create(hostName,
imageIdForZone(regionId), flavorRefForZone(regionId), options);
+
+ blockUntilServerInState(server.getId(), serverApi, serverStatus);
+
+ return serverApi.get(server.getId());
+ }
+
+ private Server createServer(String regionId, String availabilityZoneId,
Server.Status serverStatus) {
[minor] Rather than duplicate almost the whole method, add a null switch for
availabilityZone?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826604
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
jclouds-pull-requests #679 UNSTABLE Not sure if these are [real test failures](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/org.apache.jclouds.api$s3/679/testReport/junit/org.jclouds.s3.filters/RequestAuthorizeSignatureTest/testIdempotent/) or not...they're certainly not our usual spurious test failure suspects. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38274316
Re: [jclouds-labs] [JCLOUDS-500] Initial commit for docker (#57)
+limitations under the License. + +-- +project xmlns=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/maven-v4_0_0.xsd; +modelVersion4.0.0/modelVersion +parent +groupIdorg.apache.jclouds.labs/groupId +artifactIdjclouds-labs/artifactId +version1.8.0-SNAPSHOT/version +/parent + +!-- TODO: when out of labs, switch to org.jclouds.provider -- +groupIdorg.apache.jclouds.labs/groupId +artifactIddocker/artifactId +version1.8.0-SNAPSHOT/version Don't think we need this since we're inheriting from the labs parent POM? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/57/files#r10835165
Re: [jclouds-labs] [JCLOUDS-500] Initial commit for docker (#57)
jclouds » jclouds-labs #886 FAILURE GitHub [timeout](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/886/console) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/57#issuecomment-38275658
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
jclouds » jclouds #934 FAILURE Another [GitHub timeout](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/934/console) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38278954
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
jclouds-pull-requests #680 SUCCESS jclouds-java-7-pull-requests #1150 SUCCESS Bingo ;-) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38286311
Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)
-}
+ }
+ }
+ }
+
+ @Test
+ public void testCreateInWrongAvailabilityZone() {
+ String serverId = null;
+ for (String zoneId : zones) {
+ ServerApi serverApi = api.getServerApiForZone(zoneId);
+ try {
+serverId = createServer(zoneId, err,
Server.Status.ERROR).getId();
+Server server = serverApi.get(serverId);
+assertEquals(server.getStatus(), Server.Status.ERROR);
+ } finally {
+serverApi.delete(serverId);
this is mostly reformatting old code for style
Perhaps we can improve it (or at least iron out a possible bug) at the same
time? ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10840353
Re: [jclouds] Fixes potentially deleting a null server id. (#323)
@@ -88,7 +88,9 @@ public void testCreateInAvailabilityZone() {
Server server = serverApi.get(serverId);
assertEquals(server.getStatus(), Server.Status.ACTIVE);
} finally {
-serverApi.delete(serverId);
+if (serverId!=null) {
[minor] Add spaces, i.e. `(serverId != null)`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/323/files#r10850674
Re: [jclouds] Fixes potentially deleting a null server id. (#323)
+1 - good to go for me! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/323#issuecomment-38316357
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -31,7 +31,7 @@
interface Factory {
SshClient create(HostAndPort socket, LoginCredentials credentials);
-
+ boolean existsSshAgent();
`hasSshAgent`? Or is the SSH agent not really something the client _has_,
rather something it _talks to_?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856250
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
}
sessionConnection =
SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host,
socket.getPort())).loginCredentials(
loginCredentials).proxy(checkNotNull(proxyConfig,
proxyConfig)).connectTimeout(timeout).sessionTimeout(timeout).build();
}
+ static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
+ return loginCredentials.getPrivateKey() != null
+ !loginCredentials.getPrivateKey().isEmpty()
Why the empty check? This is new, I guess? Previously, an empty (but non-null)
private key would have been accepted by the code?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856393
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
}
sessionConnection =
SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host,
socket.getPort())).loginCredentials(
loginCredentials).proxy(checkNotNull(proxyConfig,
proxyConfig)).connectTimeout(timeout).sessionTimeout(timeout).build();
}
+ static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
+ return loginCredentials.getPrivateKey() != null
+ !loginCredentials.getPrivateKey().isEmpty()
+ !loginCredentials.getPrivateKey().contains(Proc-Type:
4,ENCRYPTED);
Make `Proc-Type: 4,ENCRYPTED` a constant with a descriptive name?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856402
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -128,23 +132,44 @@ public JschSshClient(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoff this.user = checkNotNull(loginCredentials, loginCredentials).getUser(); this.host = checkNotNull(socket, socket).getHostText(); checkArgument(socket.getPort() 0, ssh port must be greater then zero + socket.getPort()); - checkArgument(loginCredentials.getPassword() != null || loginCredentials.getPrivateKey() != null, - you must specify a password or a key); + checkArgument(loginCredentials.getPassword() != null || hasValidPrivateKey(loginCredentials) || getSSHAgentConnector() != null, Is there a cheaper way to check for the SSH agent than actually making a connector? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312/files#r10856436
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
}
sessionConnection =
SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host,
socket.getPort())).loginCredentials(
loginCredentials).proxy(checkNotNull(proxyConfig,
proxyConfig)).connectTimeout(timeout).sessionTimeout(timeout).build();
}
+ static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
+ return loginCredentials.getPrivateKey() != null
+ !loginCredentials.getPrivateKey().isEmpty()
+ !loginCredentials.getPrivateKey().contains(Proc-Type:
4,ENCRYPTED);
+ }
+
+ static Connector getSSHAgentConnector() {
+ JSch.setConfig(PreferredAuthentications, publickey);
Hm...should we be doing this here, or earlier (and then once, hopefully), or
when we try to connect?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856466
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
}
sessionConnection =
SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host,
socket.getPort())).loginCredentials(
loginCredentials).proxy(checkNotNull(proxyConfig,
proxyConfig)).connectTimeout(timeout).sessionTimeout(timeout).build();
}
+ static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
+ return loginCredentials.getPrivateKey() != null
+ !loginCredentials.getPrivateKey().isEmpty()
+ !loginCredentials.getPrivateKey().contains(Proc-Type:
4,ENCRYPTED);
+ }
+
+ static Connector getSSHAgentConnector() {
+ JSch.setConfig(PreferredAuthentications, publickey);
+ ConnectorFactory cf = ConnectorFactory.getDefault();
+ try {
+ Connector con = cf.createConnector();
+ if (con != null con.isAvailable()) {
+return con;
+ }
+ } catch (AgentProxyException e) {
Do we want to swallow this, or at least warn somewhere?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856480
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
+ !loginCredentials.getPrivateKey().isEmpty()
+ !loginCredentials.getPrivateKey().contains(Proc-Type:
4,ENCRYPTED);
+ }
+
+ static Connector getSSHAgentConnector() {
+ JSch.setConfig(PreferredAuthentications, publickey);
+ ConnectorFactory cf = ConnectorFactory.getDefault();
+ try {
+ Connector con = cf.createConnector();
+ if (con != null con.isAvailable()) {
+return con;
+ }
+ } catch (AgentProxyException e) {
+ }
+ return null;
+ }
This method will be really nasty to mock due to the statics. Should we make
this an injectable factory instead?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856513
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
jsch.addIdentity(loginCredentials.getUser(), privateKey, null,
emptyPassPhrase);
+ } else {
+ Connector con = JschSshClient.getSSHAgentConnector();
If we go for an injectable factory/supplier above, inject it here, too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856567
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
jsch.addIdentity(loginCredentials.getUser(), privateKey, null,
emptyPassPhrase);
+ } else {
+ Connector con = JschSshClient.getSSHAgentConnector();
+ if(con != null ){
[minor] Formatting `if (con != null) {`
And what if con _is_ null? Shouldn't we be going bang here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856603
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
jsch.addIdentity(loginCredentials.getUser(), privateKey, null,
emptyPassPhrase);
+ } else {
+ Connector con = JschSshClient.getSSHAgentConnector();
+ if(con != null ){
+IdentityRepository irepo = new RemoteIdentityRepository(con);
+jsch.setIdentityRepository(irepo);
Inline `irepo`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856612
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -67,5 +70,19 @@ public SshClient create(HostAndPort socket,
LoginCredentials credentials) {
injector.injectMembers(client);// add logger
return client;
}
+
+ @Override
+ public boolean existsSshAgent() {
+ try {
+ConnectorFactory cf = ConnectorFactory.getDefault();
+if (cf != null) {
+ Connector con = cf.createConnector();
+ if (con != null) {
+ return con.isAvailable();
+ }
+}
+ } catch (AgentProxyException e) {}
+ return false;
Put this in the catch block, to make it clear this is only the result if
something goes wrong?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856630
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -67,5 +70,19 @@ public SshClient create(HostAndPort socket,
LoginCredentials credentials) {
injector.injectMembers(client);// add logger
return client;
}
+
+ @Override
+ public boolean existsSshAgent() {
+ try {
Reuse `JschSshClient.getSSHAgentConnector()` here, or the injectable
factory/supplier?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856673
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -206,4 +217,13 @@ public String toString() {
sessionTimeout, sessionTimeout).toString();
}
+ private static ListAuthMethod getAuthMethods(AgentProxy agent) throws
Exception {
Narrow the list of exception types thrown here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856725
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
OpenSSHKeyFile key = new OpenSSHKeyFile();
key.init(loginCredentials.getPrivateKey(), null);
ssh.authPublickey(loginCredentials.getUser(), key);
+ } else {
+ AgentProxy proxy= SshjSshClient.getSSHAgentProxy();
[minor] Space before `=`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856700
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -206,4 +217,13 @@ public String toString() {
sessionTimeout, sessionTimeout).toString();
}
+ private static ListAuthMethod getAuthMethods(AgentProxy agent) throws
Exception {
+ Identity[] identities = agent.getIdentities();
+ ListAuthMethod result = Lists.newArrayList();
+ for (Identity identity : identities) {
+ result.add(new AuthAgent(agent, identity));
+ }
+ return result;
Use a builder? E.g.
```
ImmutableList.BuilderAuthMethod identities = ImmutableList.builder();
for (Identity identity : agent.getIdentities()) {
identities.add(new AuthAgent(agent, identity));
}
return identities.build();
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856774
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
+ !loginCredentials.getPrivateKey().isEmpty()
+ !loginCredentials.getPrivateKey().contains(Proc-Type:
4,ENCRYPTED);
+ }
+
+ static AgentProxy getSSHAgentProxy() {
+ ConnectorFactory cf = ConnectorFactory.getDefault();
+ try {
+ Connector con = cf.createConnector();
+ if (con != null con.isAvailable()) {
+return new AgentProxy(con);
+ }
+ } catch (AgentProxyException e) {
+ }
+ return null;
+ }
+
Same comments here as for the jsch version
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856791
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -62,5 +65,19 @@ public SshClient create(HostAndPort socket,
LoginCredentials credentials) {
injector.injectMembers(client);// add logger
return client;
}
+
+ @Override
+ public boolean existsSshAgent() {
+ try {
+ConnectorFactory cf = ConnectorFactory.getDefault();
+if (cf != null) {
+ Connector con = cf.createConnector();
+ if (con != null) {
+ return con.isAvailable();
+ }
+}
+ } catch (AgentProxyException e) {}
+ return false;
+ }
Same comments as for the jsch version
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856797
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -86,6 +86,16 @@ artifactIdjsch/artifactId scopecompile/scope /dependency +dependency + groupIdcom.jcraft/groupId + artifactIdjsch.agentproxy.jsch/artifactId + version0.0.7/version +/dependency Do we need this, since the sshj version only needs core? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312/files#r10856861
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
+exception + conflictingDependencies +dependency + groupIdcom.jcraft/groupId + artifactIdjsch.agentproxy.core/artifactId + version0.0.7/version +/dependency +dependency + groupIdcom.jcraft/groupId + artifactIdjsch.agentproxy.connector-factory/artifactId + version0.0.7/version +/dependency + /conflictingDependencies + packages +packagecom.jcraft.jsch.agentproxy/package + /packages Do we know what the actual problem is here? Duplicated classes? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312/files#r10856894
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
}
sessionConnection =
SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host,
socket.getPort())).loginCredentials(
loginCredentials).proxy(checkNotNull(proxyConfig,
proxyConfig)).connectTimeout(timeout).sessionTimeout(timeout).build();
}
+ static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
Ah, thanks for the explanation. Then the condition we're looking for here is
indeed `hasNonencryptedPrivateKey`. What I was trying to get at is that an
encrypted private key is still _valid_ (when I hear not valid, I'm more
likely to think corrupted, not supported by the target system etc.), it's
just...well...encrypted.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10859015
Re: [jclouds] Remove Nova instance metadata limit (#324)
Hm...looks like we didn't have a test to verify the limitation anyway ;-) +1 - looks good to me too. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/324#issuecomment-38337083
Re: [jclouds-examples] support for GCE provider (#34)
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)
/dependency
dependency
groupIdorg.apache.jclouds.driver/groupId
artifactIdjclouds-enterprise/artifactId
- version1.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)
@@ -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)
@@ -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)
@@ -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)
@@ -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] Add ssh-agent support via jsch agentproxy (#312)
@@ -86,6 +86,16 @@ artifactIdjsch/artifactId scopecompile/scope /dependency +dependency + groupIdcom.jcraft/groupId + artifactIdjsch.agentproxy.jsch/artifactId + version0.0.7/version +/dependency +dependency + groupIdcom.jcraft/groupId + artifactIdjsch.agentproxy.connector-factory/artifactId + version0.0.7/version Thanks for explaining - works for me, then! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312/files#r10902106
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -34,6 +34,12 @@
}
+ interface SshAgentAvailable {
+
+ boolean isSshAgentAvailable();
+
+ }
[minor] Remove blank lines and rename to something like `SshAgentFeatureFlag`
or `SshClientFeatures` or so?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923788
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -156,6 +157,15 @@ public String getPrivateKey() {
}
/**
+* @return true if there is a private key attached that is not encrypted
+*/
+ public boolean hasUnencryptedPrivateKey() {
+ return getPrivateKey() != null
+ !getPrivateKey().isEmpty()
+ !getPrivateKey().contains(Pems.PROC_TYPE_ENCRYPTED);
Can it be anywhere, or is it actually `startsWith`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923820
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -160,11 +171,14 @@ public Session create() throws Exception {
session.setTimeout(sessionTimeout);
if (loginCredentials.getPrivateKey() == null) {
session.setPassword(loginCredentials.getPassword());
- } else {
-
checkArgument(!loginCredentials.getPrivateKey().contains(Proc-Type:
4,ENCRYPTED),
Same question as above...contains or startsWith?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923879
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
jsch.addIdentity(loginCredentials.getUser(), privateKey, null,
emptyPassPhrase);
+ } else {
Simply `} else if {`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923890
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
injector.injectMembers(client);// add logger
return client;
}
}
+ private static class AgentProvider implements ProviderConnector,
SshClient.SshAgentAvailable {
+
+ @Override
+ public Connector get() {
+ try {
+return ConnectorFactory.getDefault().createConnector();
+ } catch (final AgentProxyException e) {
+return new Connector() {
Rather than returning a dummy connector, make this a provider of an
`OptionalConnector` (`null` is obviously not allowed as a return value for
a Provider)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924240
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
+ }
+ @Override
+ public boolean isAvailable() {
+ return false;
+ }
+ @Override
+ public String getName() {
+ return agent-unavailable;
+ }
+ };
+ }
+ }
+
+ @Override
+ public boolean isSshAgentAvailable() {
+ return get().isAvailable();
What does JSch expect? That we create a new connector for each connection or
can the JSch object be shared, too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924270
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -58,9 +65,40 @@ public Factory(BackoffLimitedRetryHandler
backoffLimitedRetryHandler, Injector i
@Override
public SshClient create(HostAndPort socket, LoginCredentials
credentials) {
- SshClient client = new SshjSshClient(backoffLimitedRetryHandler,
socket, credentials, timeout);
+ SshClient client = new SshjSshClient(backoffLimitedRetryHandler,
socket, credentials, timeout, injector.getInstance(Connector.class));
Rather than an explicit call to an injector here (which isn't terrible, I
should add), create a method in the module that `@Provides` the connector by
creating a new instance of AgentProvider?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924341
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
+ public boolean isAvailable() {
+ return false;
+ }
+ @Override
+ public String getName() {
+ return agent-unavailable;
+ }
+};
+ }
+ }
+
+ @Override
+ public boolean isSshAgentAvailable() {
+ return get().isAvailable();
+ }
+ }
Same comment as for the other variant above. And I guess there's little way to
avoid duplicating this class in both places..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924383
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Also thanks from me for the cleanup, @psiniemi! Just a curiosity question: why do we actually need the connector wrapper class? Could we configure Guice to try to inject the JSch connector _directly_, or do we need a new instance for every connection? In that case, perhaps make it a factory rather than a provider (minor difference, though). Since the connector can be not available, I guess we should be injecting an `OptionalConnector` rather than a straight Connector, though. Then the tests that currently look for `isSshAgentAvailable` could just check whether the Optional is present? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38550385
Re: [jclouds-labs-openstack] Updates to Rackspace Cloud Files CDN (#85)
@@ -43,6 +56,24 @@ public CloudFilesCDNApiLiveTest() {
super();
}
+ public void testEnable() throws Exception {
+ for (String regionId : regions) {
+ assertNotNull(api.cdnApiInRegion(regionId).enable(name));
+ }
+ }
+
+ public void testEnableWithTTL() throws Exception {
+ for (String regionId : regions) {
+ assertNotNull(api.cdnApiInRegion(regionId).enable(name, 77));
+ }
+ }
+
+ public void testDisable() throws Exception {
+ for (String regionId : regions) {
+ assertNotNull(api.cdnApiInRegion(regionId).disable(name));
Any more specific assertions we can make in these three tests?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/85/files#r10955870
Re: [jclouds-labs-openstack] Updates to Rackspace Cloud Files CDN (#85)
@@ -86,6 +117,45 @@ public void testGet() throws Exception {
}
}
+ /**
+* By default, this method is disabled due to daily purge limitations of
25 objects per day.
+*/
+ @Test(enabled = false)
+ public void testPurgeObject() throws Exception {
+ for (String regionId : regions) {
+ String objectName = testPurge;
+ Payload payload = Payloads.newByteSourcePayload(ByteSource.wrap(new
byte[]{1,2,3}));
[minor] Space before `{`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/85/files#r10955879
Re: [jclouds-labs-openstack] Updates to Rackspace Cloud Files CDN (#85)
+ MockWebServer server = mockOpenStackServer();
+ server.enqueue(addCommonHeaders(new
MockResponse().setBody(stringFromResource(/access.json;
+ server.enqueue(addCommonHeaders(new
MockResponse().setResponseCode(404).setBody(stringFromResource(/cdn_container_list.json;
+
+ try {
+ CloudFilesApi api = api(server.getUrl(/).toString(),
rackspace-cloudfiles);
+ CDNApi cdnApi = api.cdnApiInRegion(DFW);
+
+ FluentIterableCDNContainer cdnContainers = cdnApi.list();
+
+ assertEquals(server.getRequestCount(), 2);
+ assertAuthentication(server);
+ assertRequest(server.takeRequest(), GET,
/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/?format=jsonenabled_only=true);
+
+ assertNotNull(cdnContainers);
+ assertEquals(cdnContainers.size(), 0);
`assertTrue(cdnContainers.isEmpty)`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/85/files#r10955981
Re: [jclouds-labs-openstack] Updates to Rackspace Cloud Files CDN (#85)
@@ -81,6 +77,28 @@ public void testList() throws Exception {
}
}
+ public void testListFail() throws Exception {
What failure exactly are we testing here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/85/files#r10955987
Re: [jclouds-labs-openstack] Updates to Rackspace Cloud Files CDN (#85)
assertAuthentication(server);
assertRequest(server.takeRequest(), PUT,
/v1/MossoCloudFS_5bcf396e-39dd-45ff-93a1-712b9aba90a9/container-1);
+ } finally {
+ server.shutdown();
+ }
+ }
+
+ public void testEnableFail() throws Exception {
Which failure are we testing here? Something to do with a 404? In that case,
describe that in the test name?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/85/files#r10956016
Re: [jclouds-labs-openstack] Updates to Rackspace Cloud Files CDN (#85)
In general, thanks a lot for the cleanup, @jdaggett! Just some questions about test naming: there are a lot of `...Fail()` tests that do not obviously seem to fail (e.g. they don't expect an exception). Could we give them more specific names to describe which kind of failure on the server they are intended to handle? Thanks! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/85#issuecomment-38626726
Re: [jclouds-labs-openstack] Fix failing Glance expect tests on master (#84)
+1 - thanks, @jdaggett! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/84#issuecomment-38634346
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@@ -156,6 +157,15 @@ public String getPrivateKey() {
}
/**
+* @return true if there is a private key attached that is not encrypted
+*/
+ public boolean hasUnencryptedPrivateKey() {
+ return getPrivateKey() != null
+ !getPrivateKey().isEmpty()
+ !getPrivateKey().contains(Pems.PROC_TYPE_ENCRYPTED);
OK, makes sense then. Do we need a case-insensitive comparison here, perhaps?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10974718
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Only the minor question about a possible case-insensitive check for the encrypted header from me, otherwise +1 - looks good. @nacx: Any more questions from you? I guess we want want to backport this to 1.7.x? And we should probably create an issue for this, just for housekeeping... --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38737396
Re: [jclouds] JCLOUDS-515: Don't require availability zone when creating volumes in cinder (#327)
I made a comment that was more appropriate to be made over in the issue. Good catch, @everett-toews. Thanks! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/327#issuecomment-38738453
Re: [jclouds] WIP JCLOUDS-514: Add ability to attach block volumes at boot through the Nova ServerApi (#326)
@@ -102,6 +103,27 @@ public String toString() {
}
+ public static class BlockDevice{
+ @Named(volume_size)
+ String volumeSize = ;
+ @Named(volume_id)
+ String volumeId;
+ @Named(delete_on_termination)
+ int deleteOnTermination = 0;
+ @Named(device_name)
+ String deviceName;
+
+ public BlockDevice(String volumeId, String deviceName){
[minor] Space before `{`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/326/files#r10999123
Re: [jclouds] WIP JCLOUDS-514: Add ability to attach block volumes at boot through the Nova ServerApi (#326)
+ String volumeSize = ;
+ @Named(volume_id)
+ String volumeId;
+ @Named(delete_on_termination)
+ int deleteOnTermination = 0;
+ @Named(device_name)
+ String deviceName;
+
+ public BlockDevice(String volumeId, String deviceName){
+ this(volumeId, deviceName, true);
+ }
+
+ public BlockDevice(String volumeId, String deviceName, boolean
deleteWhenInstanceTerminated){
+ this.volumeId = volumeId;
+ this.deviceName = deviceName;
+ this.deleteOnTermination = deleteWhenInstanceTerminated ? 1 : 0;
No need to use a different parameter name here. @everett-toews: would you
normally do something like creating constants for 0 and 1 here? Or is there
some kind of boolean to string JSON mapper we could use?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/326/files#r10999246
Re: [jclouds] WIP JCLOUDS-514: Add ability to attach block volumes at boot through the Nova ServerApi (#326)
@@ -113,6 +135,7 @@ public String toString() {
private SetNetwork novaNetworks = ImmutableSet.of();
private String availabilityZone;
private boolean configDrive;
+ private ListBlockDevice blockDeviceMapping = Lists.newArrayList();
`ImmutableList.of()`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/326/files#r10999274
Re: [jclouds] WIP JCLOUDS-514: Add ability to attach block volumes at boot through the Nova ServerApi (#326)
@@ -463,6 +494,21 @@ public CreateServerOptions networks(String... networks) {
return networks(ImmutableSet.copyOf(networks));
}
+ /**
+* Block volumes that should be attached to the instance at boot time
+*/
+ public ListBlockDevice getBlockDeviceMapping() {
+ return blockDeviceMapping;
+ }
Can you move this method to after `blockDeviceMapping`? A cleanup of this class
that untangled the current mix of getters and builder option methods would be
nice, but that's not really part of this PR, I guess.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/326/files#r10999601
Re: [jclouds] WIP JCLOUDS-514: Add ability to attach block volumes at boot through the Nova ServerApi (#326)
@@ -463,6 +494,21 @@ public CreateServerOptions networks(String... networks) {
return networks(ImmutableSet.copyOf(networks));
}
+ /**
+* Block volumes that should be attached to the instance at boot time
+*/
+ public ListBlockDevice getBlockDeviceMapping() {
+ return blockDeviceMapping;
+ }
+
+ /**
+* @see #getBlockDeviceMapping
+*/
+ public CreateServerOptions blockDeviceMapping(ListBlockDevice
blockDeviceMapping) {
+ this.blockDeviceMapping = blockDeviceMapping;
`ImmutableList.copyOf(blockDeviceMapping)`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/326/files#r10999736
Re: [jclouds-labs-openstack] OS Neutron Extension Router (#83)
jclouds-labs-openstack-pull-requests #185 UNSTABLE Glance tests [are failing](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/org.apache.jclouds.labs$openstack-glance/185/testReport/) - I guess you are expecting that, @zack-shoylev? If I recall, we had a follow-up PR to this planned soon. Could we address the following [Checkstyle violations](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/185/org.apache.jclouds.labs$openstack-neutron/violations/) that we have introduced with this PR? Thanks! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/83#issuecomment-38744058
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
so I'm ok with the current check being case sensitive Thanks for checking the spec, @nacx. Lazy me :-( Good to go on this one - just the usual squash'n'rebase, and an issue number! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38748053
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
+ destroyNode(vm.getId());
Do we need a `try...catch` around here, or can this not throw an exception?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11004547
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
jclouds-pull-requests #707 UNSTABLE Spurious [test failure](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/org.apache.jclouds$jclouds-compute/707/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38775907
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
@nacx: Created [JCLOUDS-516](https://issues.apache.org/jira/browse/JCLOUDS-516) and am about to merge this... --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38777027
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Finally! ;-) Committed to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;a=commit;h=85a1a8c1dd3de5f107ddf8e66c22b2fb3410a4ba) @nacx: Backport to 1.7.x? And many thanks for all your work, @psiniemi! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38777325
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Closed #312. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Will you backport it too? I'll submit a PR just to check, yes... --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/312#issuecomment-38778047
[jclouds] Support 'AssociatePublicIpAddress' in AWS EC2 (#329)
Open for review... You can merge this Pull Request by running: git pull https://github.com/jclouds/jclouds JCLOUDS-509 Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds/pull/329 -- Commit Summary -- * Support #39;AssociatePublicIpAddress#39; in AWS EC2 -- File Changes -- M core/src/main/java/org/jclouds/reflect/FunctionalReflection.java (26) M providers/aws-ec2/pom.xml (2) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/AWSEC2ApiMetadata.java (15) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/binders/BindLaunchSpecificationToFormParams.java (35) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateOptions.java (59) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/AWSEC2CreateNodesInGroupThenAddToSet.java (39) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java (16) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/domain/AWSRunningInstance.java (29) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/domain/LaunchSpecification.java (68) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/options/AWSRunInstancesOptions.java (36) M providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/xml/LaunchSpecificationHandler.java (20) M providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/features/PlacementGroupApiExpectTest.java (13) M providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/features/SpotInstanceApiExpectTest.java (13) M providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/functions/SpotInstanceRequestToAWSRunningInstanceTest.java (4) M providers/aws-ec2/src/test/resources/describe_instances_1.xml (2) M providers/aws-ec2/src/test/resources/describe_instances_2.xml (2) M providers/aws-ec2/src/test/resources/describe_instances_3.xml (2) M providers/aws-ec2/src/test/resources/describe_instances_latest.xml (2) M providers/aws-ec2/src/test/resources/describe_instances_pending.xml (2) M providers/aws-ec2/src/test/resources/describe_security_groups_vpc.xml (2) M providers/aws-ec2/src/test/resources/describe_spot_instance.xml (2) M providers/aws-ec2/src/test/resources/describe_spot_instance_requests.xml (2) M providers/aws-ec2/src/test/resources/describe_spot_instance_tags.xml (2) M providers/aws-ec2/src/test/resources/describe_spot_instances_1.xml (2) M providers/aws-ec2/src/test/resources/describe_spot_price_history.xml (2) M providers/aws-ec2/src/test/resources/request_spot_instances-ebs.xml (2) M providers/aws-ec2/src/test/resources/request_spot_instances.xml (2) M providers/aws-ec2/src/test/resources/run_instances_1.xml (2) -- Patch Links -- https://github.com/jclouds/jclouds/pull/329.patch https://github.com/jclouds/jclouds/pull/329.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/329
[jclouds] JCLOUDS-516: Add ssh agent support via sch agentproxy (#330)
Backport of https://github.com/jclouds/jclouds/pull/312 You can merge this Pull Request by running: git pull https://github.com/jclouds/jclouds JCLOUDS-516-1.7.x Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds/pull/330 -- Commit Summary -- * JCLOUDS-516: Add ssh agent support via sch agentproxy -- File Changes -- M compute/src/main/java/org/jclouds/compute/functions/CreateSshClientOncePortIsListeningOnNode.java (5) M compute/src/main/java/org/jclouds/ssh/SshClient.java (3) M compute/src/test/java/org/jclouds/compute/internal/BaseComputeServiceLiveTest.java (2) M core/src/main/java/org/jclouds/crypto/Pems.java (1) M core/src/main/java/org/jclouds/domain/LoginCredentials.java (10) M drivers/jsch/pom.xml (10) M drivers/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java (19) M drivers/jsch/src/main/java/org/jclouds/ssh/jsch/SessionConnection.java (24) M drivers/jsch/src/main/java/org/jclouds/ssh/jsch/config/JschSshClientModule.java (21) M drivers/sshj/pom.xml (10) M drivers/sshj/src/main/java/org/jclouds/sshj/SSHClientConnection.java (52) M drivers/sshj/src/main/java/org/jclouds/sshj/SshjSshClient.java (17) M drivers/sshj/src/main/java/org/jclouds/sshj/config/SshjSshClientModule.java (23) M project/pom.xml (17) -- Patch Links -- https://github.com/jclouds/jclouds/pull/330.patch https://github.com/jclouds/jclouds/pull/330.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/330
Re: [jclouds] JCLOUDS-516: Add ssh agent support via sch agentproxy (#330)
Just checking to see if the PR builders are happy... --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/330#issuecomment-38779717
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
+ destroyNode(vm.getId());
the point of this code is that destroyNode needs to be called before we
rethrow the exception.
Oh, I agree. Sorry, I didn't express myself clearly. What I meant is the
following: with the code as currently written, what if `destroyNode` here
throws an exception? Then the original exception (the one that probably matters
to the user, which explains why the static NATs could not be created) will not
be visible, and the user will instead be stuck with the cleanup exception.
I was suggesting something like:
```
} catch (RuntimeException re) {
logger.error(-- exception after node has been created, trying to destroy the
created virtualMachine(%s), vm.getId());
try {
destroyNode(vm.getId());
} catch (RuntimeException re2) {
// or just ignore this exception
logger.debug(-- exception trying to destroy the created
virtualMachine(%s): %s, vm.getId(), re2);
}
throw re
}
```
Does that make more sense?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11014653
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+logger.trace( static NATed IPAddress(%s) to
virtualMachine(%s), ip.getId(), vm.getId());
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
Add the exception itself (or at least `re.getMessage()`) to the log line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11014667
Re: [jclouds] Support 'AssociatePublicIpAddress' in AWS EC2 (#329)
@@ -26,19 +39,6 @@ import static org.jclouds.reflect.Reflection2.typeToken; import static org.jclouds.util.Throwables2.propagateIfPossible; -import java.lang.reflect.Method; -import java.lang.reflect.Proxy; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - -import com.google.common.annotations.Beta; -import com.google.common.base.Function; -import com.google.common.base.Objects; -import com.google.common.collect.ImmutableList; -import com.google.common.reflect.Invokable; -import com.google.common.reflect.TypeToken; - These imports have just moved, from what I see. Undo this change? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/329/files#r11016371
Re: [jclouds] Support 'AssociatePublicIpAddress' in AWS EC2 (#329)
@@ -28,8 +25,10 @@ import org.jclouds.ec2.compute.config.EC2ResolveImagesModule; import org.jclouds.rest.internal.BaseHttpApiMetadata; -import com.google.common.collect.ImmutableSet; -import com.google.inject.Module; +import java.net.URI; +import java.util.Properties; + +import static org.jclouds.ec2.reference.EC2Constants.PROPERTY_EC2_AMI_OWNERS; Again, please undo the import reformatting. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/329/files#r11016379
Re: [jclouds] Support 'AssociatePublicIpAddress' in AWS EC2 (#329)
Haven't been able to review this properly, but one initial comment: could you take the move import statements changes out of this PR? That should make the PR smaller and easier to review. Thanks, Lahiru! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/329#issuecomment-38786399
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
jclouds-pull-requests #710 SUCCESS [Clean Checkstyle](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/710/violations/). +1 - good to go for me. @nacx @abayer: any open questions? Otherwise, please squash'n'rebase, and then we can merge! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38786846
Re: [jclouds] JCLOUDS-516: Add ssh agent support via sch agentproxy (#330)
Closed #330. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/330
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
jclouds-java-7-pull-requests #1183 UNSTABLE [Spurious](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1182/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/) [test failures](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1183/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38794219
Re: [jclouds-site] Remove the GSoC announce from the frontpage (#66)
Homepage looks fine:  +1 - good to go for me. Thanks, @nacx! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/66#issuecomment-38794389
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
Committed to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;h=0401959). Thanks, @spark404! @nacx: Do we want to backport this? If so, to which branch(es)? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38794590
[jclouds] JCLOUDS-347: Implement a poor-mans rollback if CloudStack static NAT creation fails (#332)
You can merge this Pull Request by running: git pull https://github.com/jclouds/jclouds JCLOUDS-347-1.7.x Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds/pull/332 -- Commit Summary -- * JCLOUDS-347: Implement a poor-mans rollback if CloudStack static NAT creation fails -- File Changes -- M apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java (48) -- Patch Links -- https://github.com/jclouds/jclouds/pull/332.patch https://github.com/jclouds/jclouds/pull/332.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/332
Re: [jclouds] JCLOUDS-347: Implement a poor-mans rollback if CloudStack static NAT creation fails (#332)
Let's see what the PR builders say... --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/332#issuecomment-38802961
Re: [jclouds-examples] Fixes broken README links (#36)
Many thanks for the updates, @jdaggett! Checked links, all look good. +1 - good to go for me. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/36#issuecomment-38881732
Re: [jclouds] Fixing Jclouds-509 (#333)
Will try to have a look at this soon, but could you in the meantime clean up the four small [Checkstyle violations](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/717/org.apache.jclouds.provider$aws-ec2/violations/) that have been introduced? Thanks, @lahirus! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/333#issuecomment-38882563
Re: [jclouds] Fixing Jclouds-509 (#333)
Another thing that we will certainly want to do, by the way, is run the live test suite against AWS EC2. See the [Running live tests section](http://2bfcdbe0cc28b674f568-6e84ef02f7d038e5f4526bc5b8a12a51.r1.cf1.rackcdn.com/documentation/devguides/provider-testing/) from this cached version of the old site. If you could run those tests and paste the results here (there will be [some failures](https://issues.apache.org/jira/browse/JCLOUDS-462), by the way), that would be great. Otherwise, please let us know and we'll see how best to validate this against AWS. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/333#issuecomment-38882802
Re: [jclouds] JCLOUDS-347: Implement a poor-mans rollback if CloudStack static NAT creation fails (#332)
@abayer: Want to backport this to 1.7.x? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/332#issuecomment-38999088
Re: [jclouds-labs-openstack] Fixes more checkstyle violations. (#87)
jclouds-labs-openstack-pull-requests #190 SUCCESS [Happy Checkstyle](https://jclouds.ci.cloudbees.com/job/jclouds-labs-openstack-pull-requests/190/violations/)!  Thanks, @zack-shoylev! +1 - looks good to me! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-openstack/pull/87#issuecomment-39008282
Re: [jclouds-examples] Remove RestContext from Rackspace examples (#35)
I will submit another PR to revert the changes to the Cloud Servers examples for the time being. Sound good? Sounds good! Thanks, @jdaggett! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/35#issuecomment-39008301
Re: [jclouds] Update ElasticHosts pre-installed images and added new regions (#331)
@nacx: Status of this? Are we still trying to get this in for 1.7.2? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/331#issuecomment-39008500
Re: [jclouds-examples] JCLOUDS-389 - Updates copyright references (#37)
This looks great - thanks, @jdaggett! +1 - good to go for me, after the squash'n'rebase that you mentioned. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/37#issuecomment-39008704
Re: [jclouds] Merge pull request #1 from jclouds/master (#335)
I'm guessing this was a test or an accidental PR ;-) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/335#issuecomment-39008720
Re: [jclouds-karaf] [JCLOUDS-511] Correcting features with dependency-only bundles (#40)
jclouds-karaf-pull-requests #52 UNSTABLE No test failures, just a bunch of [Checkstyle violations](https://jclouds.ci.cloudbees.com/job/jclouds-karaf-pull-requests/52/violations/). None related to this PR, though, from what I can see. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/40#issuecomment-39008811
Re: [jclouds-karaf] [JCLOUDS-511] Correcting features with dependency-only bundles (#40)
Closed #40. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-karaf/pull/40
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ throw Throwables.propagate(ex);
+ } catch (IOException ex) {
+ throw Throwables.propagate(ex);
+ }
+ throw new HttpException(Glance endpoint does not support
API version: + apiVersion);
+ }
+ });
+ }
+
+ @Override
+ public URI apply(Object from) {
+ checkArgument(from instanceof String, you must specify a zone, as a
String argument);
+ MapString, SupplierURI zoneToEndpoint =
zoneToEndpointSupplier.get();
+ checkState(!zoneToEndpoint.isEmpty(), no zone name to endpoint
mappings configured!);
+ checkArgument(zoneToEndpoint.containsKey(from),
+ requested location %s, which is not in the configured
locations: %s, from, zoneToEndpoint);
Do we need both of these checks? Yes, one throws an ISE and the other an IAE,
but would e.g. the IAE be enough?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095379
Re: [jclouds-labs-openstack] JCLOUDS-494: Change EndpointParam parser to negotiate version for Glance API (#82)
+ public ListVersion versions;
+ }
+
+ private final SupplierMapString, SupplierURI zoneToEndpointSupplier;
+ private final String apiVersion;
+ private final LoadingCacheURI, URI endpointCache;
+
+ @Inject
+ public ZoneToEndpointNegotiateVersion(@Zone SupplierMapString,
SupplierURI zoneToEndpointSupplier,
+ @ApiVersion String apiVersionString, final HttpClient client, final
Json json) {
+ this.zoneToEndpointSupplier = checkNotNull(zoneToEndpointSupplier,
zoneToEndpointSupplier);
+ if (!apiVersionString.startsWith(v)) {
+ this.apiVersion = v + apiVersionString;
+ } else {
+ this.apiVersion = apiVersionString;
+ }
Name the constructor param `apiVersion` and do something like:
```
this.apiVersion = apiVersion;
if (!apiVersion.startsWith(v)) {
this.apiVersion = v + apiVersion;
}
```
which makes it a little clearer that this is exceptional case handling?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/82/files#r11095387
