You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2015/11/16 11:25:50 UTC
[GitHub] incubator-brooklyn pull request: Fix jclouds sshable
GitHub user aledsage opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/1034
Fix jclouds sshable
(Unrelated to other changes, fixes `NetworkingUtilsTest.testIsPortAvailableReportsTrueWhenPortIsFree`).
Lots of fixes to get the `JcloudsLoginLiveTest` working again.
- registerJcloudsSshMachineLocation: use the userCredentials that are passed into the method, rather than looking up what we expect from the config.
(Hopefully fixes testSpecifyingPasswordAndSshKeysPrefersKeys).
- Fix waitForSshable to when password supplied on JcloudsLocation config, and using useJcloudsSshInit=false.
- Fix use of DISABLE_ROOT_AND_PASSWORD_SSH: if for root user with password-based login, then explicitly set PasswordAuthentication=yes.
If root user, then explicitly set PermitRootLogin=true
- Fix getPublicHostnameAws so pass in the credentials (some code-paths will not have configured the credentials in the generic config).
Same for getPrivateHostnameAws.
- Tidy up the createTemporarySshMachineLocaton
- Logging of timing: include how long to get semaphore
- Logging of failure: include how long for each stage
Previously hit a problem when the jclouds location was configured with
a password + a blank ssh key, and using useJcloudsSshInit=false.
It failed to ssh to the newly provisioned AWS VM in waitForSshable. It
got the node.getCredentials, which gives us the ssh-key from the AWS
key-pair. But then we instantiate an SshMachineLocation to check if we
can connection (before continuing with the user-setup). That
machineLocation inherits the password (which has not been set on the
machine), so it fails to ssh - the password takes precedence over the
ssh-key.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/incubator-brooklyn fix/jclouds-sshable
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/1034.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1034
----
commit af39886a454fbadbfb9d033103a502a0b93530fb
Author: Aled Sage <al...@gmail.com>
Date: 2015-11-05T22:21:07Z
Improve SshMachineLocation.toString
commit 2d5a30ba9e6a10341ccb8ac513b9d57e449ae5f4
Author: Aled Sage <al...@gmail.com>
Date: 2015-11-05T22:36:17Z
Fix NetworkingUtilsTest.testIsPortAvailableReportsTrueWhenPortIsFree
commit 0bf42c35d6e4b50930a5f3c3a341587a48b787e5
Author: Aled Sage <al...@gmail.com>
Date: 2015-11-09T08:13:58Z
Adds ReachableSocketFinder
Used by JcloudsLocation to determine which IP of the node is reachable.
Previously it used the jclouds’:
managementContext.utils().sshForNode().apply(node)
but that fails with certain user/login configurations - e.g. when “node”
doesn’t have login-credentials, but the JcloudsLocation.obtain knows
about the credentials through some other configuration.
Code is based on the Jclouds ConcurrentOpenSocketFinder. However, it
is much simplified because we don’t abort-early if the VM’s status
goes to failed (we don’t concurrently poll the cloud-provider to find
out the state, as jclouds was doing).
commit 6a57b26c3a4d4a6820050f9f738d87e87834aa44
Author: Aled Sage <al...@gmail.com>
Date: 2015-11-09T08:28:56Z
Adds TODO question to Networking.isReaachable
commit 48b5196eff9be3bb961ce0a0ba4eb87f81f52a8b
Author: Aled Sage <al...@gmail.com>
Date: 2015-10-21T16:53:28Z
JcloudsLoginLiveTest: run all tests in AWS
The Rackspace config wasn't working (probably because image ids
have changed).
commit e4e2bcbb15ed843b1e9b80f971e08e29f2ff4fe6
Author: Aled Sage <al...@gmail.com>
Date: 2015-11-05T22:28:35Z
Fix JcloudsLocation login
- registerJcloudsSshMachineLocation: use the userCredentials that
are passed into the method, rather than looking up what we expect
from the config.
(Hopefully fixes testSpecifyingPasswordAndSshKeysPrefersKeys).
- Fix waitForSshable to when password supplied on JcloudsLocation
config, and using useJcloudsSshInit=false.
- Fix use of DISABLE_ROOT_AND_PASSWORD_SSH: if for root user with
password-based login, then explicitly set PasswordAuthentication=yes.
If root user, then explicitly set PermitRootLogin=true
- Fix getPublicHostnameAws so pass in the credentials (some code-paths
will not have configured the credentials in the generic config).
Same for getPrivateHostnameAws.
- Tidy up the createTemporarySshMachineLocaton
- Logging of timing: include how long to get semaphore
- Logging of failure: include how long for each stage
Previously hit a problem when the jclouds location was configured with
a password + a blank ssh key, and using useJcloudsSshInit=false.
It failed to ssh to the newly provisioned AWS VM in waitForSshable. It
got the node.getCredentials, which gives us the ssh-key from the AWS
key-pair. But then we instantiate an SshMachineLocation to check if we
can connection (before continuing with the user-setup). That
machineLocation inherits the password (which has not been set on the
machine), so it fails to ssh - the password takes precedence over the
ssh-key.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix jclouds sshable
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1034#issuecomment-157460725
Thanks @neykov - merging now.
I didn't address your comment "Will be able to get rid of compoundFuture as well, leaving out the need for ListeningExecutorService (and could even use Brooklyn's task manager)." We could look at that in a future PR, but I'm not quite sure what you're proposing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix jclouds sshable
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/1034
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix jclouds sshable
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1034#discussion_r45055337
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -1653,8 +1662,10 @@ protected LoginCredentials createUser(ComputeService computeService, NodeMetadat
LOG.warn("exit code {} when creating user for {}; usage may subsequently fail", exitcode, node);
}
} finally {
- getManagementContext().getLocationManager().unmanage(sshLoc);
- Streams.closeQuietly(sshLoc);
+ if (sshLoc != null) {
--- End diff --
Can't be null
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix jclouds sshable
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1034#issuecomment-157364242
Good to merge after addressing the potential NPE error, others are minor.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix jclouds sshable
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1034#discussion_r45052171
--- Diff: utils/common/src/test/java/org/apache/brooklyn/util/net/ReachableSocketFinderTest.java ---
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * 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.apache.brooklyn.util.net;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.fail;
+
+import java.net.ServerSocket;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.concurrent.Callable;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.javalang.JavaClassNames;
+import org.apache.brooklyn.util.time.Duration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
+import com.google.common.net.HostAndPort;
+import com.google.common.util.concurrent.ListenableFuture;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.common.util.concurrent.MoreExecutors;
+
+public class ReachableSocketFinderTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(ReachableSocketFinderTest.class);
+
+ private HostAndPort socket1;
+ private HostAndPort socket2;
+ private Map<HostAndPort, Boolean> reachabilityResults;
+ private ListeningExecutorService executor;
+ private Predicate<HostAndPort> socketTester;
+ private ReachableSocketFinder finder;
+
+ @BeforeMethod(alwaysRun=true)
+ public void setUp() throws Exception {
+ socket1 = HostAndPort.fromParts("1.1.1.1", 1111);
+ socket2 = HostAndPort.fromParts("1.1.1.2", 1112);
+ reachabilityResults = Maps.newConcurrentMap();
+ executor = MoreExecutors.listeningDecorator(Executors.newCachedThreadPool());
+ socketTester = new Predicate<HostAndPort>() {
+ @Override public boolean apply(HostAndPort input) {
+ return Boolean.TRUE.equals(reachabilityResults.get(input));
+ }
+ };
+
+ finder = new ReachableSocketFinder(socketTester, executor);
+ }
+
+ @AfterMethod(alwaysRun=true)
+ public void tearDown() throws Exception {
+ if (executor != null) executor.shutdownNow();
+ }
+
+ @Test(expectedExceptions=IllegalStateException.class)
+ public void testWhenNoSocketsThrowsIllegalState() throws Exception {
+ finder.findOpenSocketOnNode(ImmutableList.<HostAndPort>of(), Duration.TEN_SECONDS);
+ }
+
+ @Test
+ public void testReturnsReachableSocket() throws Exception {
+ reachabilityResults.put(socket1, true);
+ reachabilityResults.put(socket2, false);
+ assertEquals(finder.findOpenSocketOnNode(ImmutableList.<HostAndPort>of(socket1, socket2), Duration.TEN_SECONDS), socket1);
+
+ reachabilityResults.put(socket1, false);
+ reachabilityResults.put(socket2, true);
+ assertEquals(finder.findOpenSocketOnNode(ImmutableList.<HostAndPort>of(socket1, socket2), Duration.TEN_SECONDS), socket2);
+ }
+
+ @Test
+ public void testPollsUntilPortReachable() throws Exception {
+ reachabilityResults.put(socket1, false);
+ reachabilityResults.put(socket2, false);
+ final ListenableFuture<HostAndPort> future = executor.submit(new Callable<HostAndPort>() {
+ @Override public HostAndPort call() throws Exception {
+ return finder.findOpenSocketOnNode(ImmutableList.<HostAndPort>of(socket1, socket2), Duration.TEN_SECONDS);
+ }});
+
+ // Should keep trying
+ Asserts.succeedsContinually(new Runnable() {
+ @Override public void run() {
+ assertFalse(future.isDone());
+ }});
+
+ // When port is reached, it completes
+ reachabilityResults.put(socket1, true);
+ assertEquals(future.get(30, TimeUnit.SECONDS), socket1);
+ }
+
+ // Mark as integration, as can't rely (in Apache infra) for a port to stay unused during test!
+ @Test(groups="Integration")
+ public void testReturnsRealReachableSocket() throws Exception {
+ ReachableSocketFinder realFinder = new ReachableSocketFinder(executor);
+ ServerSocket socket = connectToPort();
+ try {
+ HostAndPort addr = HostAndPort.fromParts(socket.getInetAddress().getHostAddress(), socket.getLocalPort());
+ HostAndPort wrongAddr = HostAndPort.fromParts(socket.getInetAddress().getHostAddress(), findAvailablePort());
+
+ assertEquals(realFinder.findOpenSocketOnNode(ImmutableList.of(addr, wrongAddr), Duration.ONE_MINUTE), addr);
+ } finally {
+ if (socket != null) {
+ socket.close();
+ }
+ }
+ }
+
+ // Mark as integration, as can't rely (in Apache infra) for a port to stay unused during test!
+ // And slow test - takes 5 seconds.
+ @Test(groups="Integration")
+ public void testFailsIfRealSocketUnreachable() throws Exception {
+ ReachableSocketFinder realFinder = new ReachableSocketFinder(executor);
+ HostAndPort wrongAddr = HostAndPort.fromParts(Networking.getLocalHost().getHostAddress(), findAvailablePort());
+
+ try {
+ HostAndPort result = realFinder.findOpenSocketOnNode(ImmutableList.of(wrongAddr), Duration.FIVE_SECONDS);
+ fail("Expected failure, but got "+result);
+ } catch (NoSuchElementException e) {
+ // success
+ }
+ }
+
+ private ServerSocket connectToPort() throws Exception {
+ int port = findAvailablePort();
+ LOG.info("acquired port on "+port+" for test "+JavaClassNames.niceClassAndMethod());
+ return new ServerSocket(port);
--- End diff --
Could use port 0 to avoid someone else stealing the port.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix jclouds sshable
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1034#discussion_r45090159
--- Diff: utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java ---
@@ -486,6 +487,12 @@ public static boolean isLocalhost(String remoteAddress) {
}
public static boolean isReachable(HostAndPort endpoint) {
+ // TODO Should we create an unconnected socket, and then use the calls below (see jclouds' InetSocketAddressConnect):
+ // socket.setReuseAddress(false);
+ // socket.setSoLinger(false, 1);
+ // socket.setSoTimeout(timeout);
+ // socket.connect(socketAddress, timeout);
--- End diff --
Let's do the actual change in a different PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix jclouds sshable
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1034#discussion_r45051833
--- Diff: utils/common/src/main/java/org/apache/brooklyn/util/net/Networking.java ---
@@ -486,6 +487,12 @@ public static boolean isLocalhost(String remoteAddress) {
}
public static boolean isReachable(HostAndPort endpoint) {
+ // TODO Should we create an unconnected socket, and then use the calls below (see jclouds' InetSocketAddressConnect):
+ // socket.setReuseAddress(false);
+ // socket.setSoLinger(false, 1);
+ // socket.setSoTimeout(timeout);
+ // socket.connect(socketAddress, timeout);
--- End diff --
Don't see the point of the above, but +1 for the `,conntect` timeout.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---