Skip to content

remove PortableTemplateBuilder #926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andreaturli wants to merge 2 commits into
base: master
Choose a base branch
from
Open

remove PortableTemplateBuilder #926

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07daa71
remove PortableTemplateBuilder
andreaturli Aug 30, 2017
6579dfb
use jclouds:stub
andreaturli Dec 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...louds/src/main/java/org/apache/brooklyn/location/jclouds/DefaultConnectivityResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,10 @@ protected Iterable<HostAndPort> getManagementCandidates(
LOG.debug("Using reachable addresses for management candidates of {}", location);
try {
final Predicate<? super HostAndPort> predicate = options.reachableAddressPredicate();
return getReachableAddresses(node, predicate, options.reachableAddressTimeout());
Iterable<HostAndPort> reachableAddresses = getReachableAddresses(node, predicate, options.reachableAddressTimeout());
if (!Iterables.isEmpty(reachableAddresses)) {
return reachableAddresses;
}
} catch (RuntimeException e) {
if (options.propagatePollForReachableFailure()) {
throw Exceptions.propagate(e);
Expand Down
2 changes: 1 addition & 1 deletion locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2609,7 +2609,7 @@ protected LoginCredentials waitForSshable(
protected LoginCredentials waitForSshable(
HostAndPort hostAndPort, Iterable<LoginCredentials> credentialsToTry, ConfigBag setup) {
String waitForSshable = setup.get(WAIT_FOR_SSHABLE);
checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForSshable called despite waitForSshable=%s for %s", waitForSshable, hostAndPort);
// checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForSshable called despite waitForSshable=%s for %s", waitForSshable, hostAndPort);

@rdowner rdowner Jan 8, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid committing commented-out lines of code. Is this line supposed to be deleted, or was it commented out to help debugging?

checkArgument(!Iterables.isEmpty(credentialsToTry), "waitForSshable called without credentials for %s", hostAndPort);

Duration timeout = null;
Expand Down
53 changes: 24 additions & 29 deletions ...ds/src/test/java/org/apache/brooklyn/location/jclouds/AbstractJcloudsStubbedUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@
*/
package org.apache.brooklyn.location.jclouds;

import static com.google.common.base.Preconditions.checkNotNull;

import java.util.Map;

import com.google.common.collect.ImmutableMap;
import org.apache.brooklyn.api.location.Location;
import org.apache.brooklyn.api.location.LocationSpec;
import org.apache.brooklyn.config.ConfigKey;
Expand All @@ -38,34 +35,35 @@
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;

import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableMap;
import java.util.Map;

/**
* Stubs out all comms with the cloud provider.
*
* Expects sub-classes to call {@link #initNodeCreatorAndJcloudsLocation(NodeCreator, Map)} before
* <p>
* Expects sub-classes to call {@link #initStubbedJcloudsLocation(Map)} before
* the test methods are called.
*/
public abstract class AbstractJcloudsStubbedUnitTest extends AbstractJcloudsLiveTest {

private static final Logger LOG = LoggerFactory.getLogger(AbstractJcloudsStubbedUnitTest.class);

// TODO These values are hard-coded into the JcloudsStubTemplateBuilder, so best not to mess!
public static final String LOCATION_SPEC = "jclouds:aws-ec2:us-east-1";

public static final String LOCATION_SPEC = "jclouds:stub";
public static final String PUBLIC_IP_ADDRESS = "144.175.1.1";
public static final String PRIVATE_IP_ADDRESS = "10.1.1.1";

protected NodeCreator nodeCreator;
protected ComputeServiceRegistry computeServiceRegistry;
@BeforeMethod(alwaysRun=true)

@BeforeMethod(alwaysRun = true)
@Override
public void setUp() throws Exception {
super.setUp();
RecordingSshTool.clear();
RecordingWinRmTool.clear();
}
@AfterMethod(alwaysRun=true)

@AfterMethod(alwaysRun = true)
@Override
public void tearDown() throws Exception {
try {
Expand All @@ -80,7 +78,7 @@ public void tearDown() throws Exception {
protected LocalManagementContext newManagementContext() {
return LocalManagementContextForTests.builder(true).useAdditionalProperties(customBrooklynProperties()).build();
}

/**
* For overriding.
*/
Expand All @@ -89,20 +87,18 @@ protected LocalManagementContext newManagementContext() {
}

/**
* Expect sub-classes to call this - either in their {@link BeforeMethod} or at the very
* Expect sub-classes to call this - either in their {@link BeforeMethod} or at the very
* start of the test method (to allow custom config per test).
*/
protected void initNodeCreatorAndJcloudsLocation(NodeCreator nodeCreator, Map<?, ?> jcloudsLocationConfig) throws Exception {
this.nodeCreator = nodeCreator;
this.computeServiceRegistry = new StubbedComputeServiceRegistry(nodeCreator, false);
protected JcloudsLocation initStubbedJcloudsLocation(Map<?, ?> jcloudsLocationConfig) throws Exception {
final Map<ConfigKey<?>, Object> defaults = ImmutableMap.<ConfigKey<?>, Object>builder()
.put(JcloudsLocationConfig.COMPUTE_SERVICE_REGISTRY, computeServiceRegistry)
.put(JcloudsLocationConfig.TEMPLATE_BUILDER, JcloudsStubTemplateBuilder.create(getProvider(), getRegion()))
.put(JcloudsLocationConfig.ACCESS_IDENTITY, "stub-identity")
.put(JcloudsLocationConfig.ACCESS_CREDENTIAL, "stub-credential")
.put(SshMachineLocation.SSH_TOOL_CLASS, RecordingSshTool.class.getName())
.put(WinRmMachineLocation.WINRM_TOOL_CLASS, RecordingWinRmTool.class.getName())
.put(JcloudsLocation.POLL_FOR_FIRST_REACHABLE_ADDRESS_PREDICATE, Predicates.alwaysTrue())
.put(JcloudsLocation.WAIT_FOR_SSHABLE, Boolean.FALSE)
.put(JcloudsLocationConfig.USE_JCLOUDS_SSH_INIT, Boolean.FALSE)
.put(JcloudsLocationConfig.POLL_FOR_FIRST_REACHABLE_ADDRESS, Boolean.FALSE)
.put(JcloudsLocationConfig.LOOKUP_AWS_HOSTNAME, Boolean.FALSE)
.build();
final ImmutableMap.Builder<Object, Object> flags = ImmutableMap.builder()
Expand All @@ -117,8 +113,7 @@ protected void initNodeCreatorAndJcloudsLocation(NodeCreator nodeCreator, Map<?,
LOG.debug("Overridden default value for {} with: {}", new Object[]{key, overrideVal});
}
}
this.jcloudsLocation = (JcloudsLocation)managementContext.getLocationRegistry().getLocationManaged(
getLocationSpec(), flags.build());
return (JcloudsLocation) managementContext.getLocationRegistry().getLocationManaged(getLocationSpec(), flags.build());
}

/**
Expand All @@ -127,27 +122,27 @@ protected void initNodeCreatorAndJcloudsLocation(NodeCreator nodeCreator, Map<?,
protected String getLocationSpec() {
return LOCATION_SPEC;
}

protected NodeCreator newNodeCreator() {
return new BasicNodeCreator();
}

protected String getProvider() {
LocationSpec<?> spec = mgmt().getLocationRegistry().getLocationSpec(getLocationSpec()).get();
return getRequiredConfig(spec, JcloudsLocation.CLOUD_PROVIDER);
}

protected String getRegion() {
LocationSpec<? extends Location> spec = mgmt().getLocationRegistry().getLocationSpec(getLocationSpec()).get();
return getRequiredConfig(spec, JcloudsLocation.CLOUD_REGION_ID);
}

protected String getRequiredConfig(LocationSpec<?> spec, ConfigKey<String> key) {
String result = (String) spec.getConfig().get(key);
if (result != null) {
return result;
}
result = (String) spec.getFlags().get(key.getName());
return checkNotNull(result, "config "+key.getName());
return result;
}
}
15 changes: 8 additions & 7 deletions ...s/src/test/java/org/apache/brooklyn/location/jclouds/DefaultConnectivityResolverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.jclouds.compute.domain.NodeMetadata;
import org.jclouds.compute.domain.NodeMetadataBuilder;
import org.jclouds.domain.LoginCredentials;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

Expand All @@ -49,7 +50,7 @@ public class DefaultConnectivityResolverTest extends AbstractJcloudsStubbedUnitT

@Test
public void testRespectsHostAndPortOverride() throws Exception {
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
// ideally would confirm that no credentials are tested either.
ConnectivityResolverOptions options = newResolveOptions()
.portForwardSshOverride(HostAndPort.fromParts("10.1.1.4", 4361))
Expand All @@ -65,7 +66,7 @@ public void testRespectsHostAndPortOverride() throws Exception {
public void testObtainsHostnameFromAwsMachine() throws Exception {
final String expectedHostname = "ec2-awshostname";
RecordingSshTool.setCustomResponse(".*curl.*169.254.169.254.*", new RecordingSshTool.CustomResponse(0, expectedHostname, ""));
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of(
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of(
JcloudsLocationConfig.LOOKUP_AWS_HOSTNAME, true));
ConnectivityResolverOptions options = newResolveOptions()
.waitForConnectable(true)
Expand All @@ -90,7 +91,7 @@ public RecordingSshTool.CustomResponse generate(RecordingSshTool.ExecParams exec
return new RecordingSshTool.CustomResponse(valid ? 0 : 1, "", "");
}
});
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
DefaultConnectivityResolver customizer = new DefaultConnectivityResolver();
final ConfigBag config = ConfigBag.newInstanceExtending(jcloudsLocation.config().getBag(), ImmutableMap.of(
JcloudsLocationConfig.WAIT_FOR_SSHABLE, "1ms",
Expand All @@ -113,7 +114,7 @@ public RecordingWinRmTool.CustomResponse generate(RecordingWinRmTool.ExecParams
return new RecordingWinRmTool.CustomResponse(valid ? 0 : 1, "", "");
}
});
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
DefaultConnectivityResolver customizer = new DefaultConnectivityResolver();
final ConfigBag config = ConfigBag.newInstanceExtending(jcloudsLocation.config().getBag(), ImmutableMap.of(
JcloudsLocationConfig.WAIT_FOR_WINRM_AVAILABLE, "1ms",
Expand All @@ -131,7 +132,7 @@ public RecordingWinRmTool.CustomResponse generate(RecordingWinRmTool.ExecParams
*/
@Test
public void testResolveChecksCredentials() throws Exception {
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
final HostAndPort authorisedHostAndPort = HostAndPort.fromParts("10.0.0.2", 22);
final HostAndPort otherHostAndPort = HostAndPort.fromParts("10.0.0.1", 22);
final Set<HostAndPort> reachableIps = Sets.newHashSet(authorisedHostAndPort, otherHostAndPort);
Expand Down Expand Up @@ -191,7 +192,7 @@ public void testMode(NetworkMode mode, Set<HostAndPort> reachableIps, String exp
DefaultConnectivityResolver.NETWORK_MODE, mode,
DefaultConnectivityResolver.CHECK_CREDENTIALS, false));

initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of(
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of(
JcloudsLocationConfig.CONNECTIVITY_RESOLVER, customizer));

ConnectivityResolverOptions options = newResolveOptionsForIps(reachableIps, Duration.millis(100)).build();
Expand Down Expand Up @@ -220,7 +221,7 @@ public void testModeUnavailable(NetworkMode mode, Set<HostAndPort> reachableIps)
DefaultConnectivityResolver.NETWORK_MODE, mode,
DefaultConnectivityResolver.CHECK_CREDENTIALS, false));

initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of(
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of(
JcloudsLocationConfig.CONNECTIVITY_RESOLVER, customizer));

ConnectivityResolverOptions options = newResolveOptionsForIps(reachableIps, Duration.ONE_MILLISECOND).build();
Expand Down
2 changes: 1 addition & 1 deletion ...va/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverStubbedRebindTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void setUp() throws Exception {

super.setUp();

initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...est/java/org/apache/brooklyn/location/jclouds/JcloudsByonLocationResolverStubbedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public class JcloudsByonLocationResolverStubbedTest extends AbstractJcloudsStubb
@Override
public void setUp() throws Exception {
super.setUp();
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...c/test/java/org/apache/brooklyn/location/jclouds/JcloudsLocationExtensionStubbedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ protected String getLocationSpec() {

@Test
public void testHasExtension() throws Exception {
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
MachineLocation machine = jcloudsLocation.obtain();

HttpExecutorFactory extension = machine.getExtension(HttpExecutorFactory.class);
Expand Down
4 changes: 2 additions & 2 deletions .../src/test/java/org/apache/brooklyn/location/jclouds/JcloudsMaxConcurrencyStubbedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public void testConcurrentCreateCalls() throws Exception {
CountDownLatch latch = new CountDownLatch(1);
creationConcurrencyMonitor.setLatch(latch);

initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of(JcloudsLocation.MAX_CONCURRENT_MACHINE_CREATIONS, 2));
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of(JcloudsLocation.MAX_CONCURRENT_MACHINE_CREATIONS, 2));

List<ListenableFuture<?>> futures = new ArrayList<>();
for (int i = 0; i < 3; i++) {
Expand All @@ -187,7 +187,7 @@ public void testConcurrentDeletionCalls() throws Exception {
CountDownLatch latch = new CountDownLatch(1);
deletionConcurrencyMonitor.setLatch(latch);

initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of(JcloudsLocation.MAX_CONCURRENT_MACHINE_DELETIONS, 2));
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of(JcloudsLocation.MAX_CONCURRENT_MACHINE_DELETIONS, 2));

for (int i = 0; i < 3; i++) {
obtainMachine();
Expand Down
27 changes: 2 additions & 25 deletions ...a/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocationAddressOverwriteTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,41 +45,18 @@ public class JcloudsSshMachineLocationAddressOverwriteTest extends AbstractJclou
@SuppressWarnings("unused")
private static final Logger log = LoggerFactory.getLogger(JcloudsImageChoiceStubbedLiveTest.class);

private List<String> privateAddresses;
private List<String> publicAddresses;

@Override
@BeforeMethod(alwaysRun=true)
public void setUp() throws Exception {
super.setUp();
privateAddresses = ImmutableList.of("172.168.10.11");
publicAddresses = ImmutableList.of("173.194.32.123");
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of(JcloudsLocationConfig.USE_MACHINE_PUBLIC_ADDRESS_AS_PRIVATE_ADDRESS.getName(), true));
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of(JcloudsLocationConfig.USE_MACHINE_PUBLIC_ADDRESS_AS_PRIVATE_ADDRESS.getName(), true));
}

@Override
protected NodeCreator newNodeCreator() {
return new AbstractNodeCreator() {
@Override protected NodeMetadata newNode(String group, Template template) {
NodeMetadata result = new NodeMetadataBuilder()
.id("myid")
.credentials(LoginCredentials.builder().identity("myuser").credential("mypassword").build())
.loginPort(22)
.status(Status.RUNNING)
.publicAddresses(publicAddresses)
.privateAddresses(privateAddresses)
.build();
return result;
}
};
}


@Test
public void testSetPrivateIpToPublicIp() throws Exception {
JcloudsSshMachineLocation machine = obtainMachine(ImmutableMap.of());

assertEquals(publicAddresses, machine.getPublicAddresses());
assertEquals(machine.getPublicAddresses(), ImmutableList.of(PUBLIC_IP_ADDRESS));

assertEquals(machine.getPublicAddresses().size(), 1);
String publicAddress = machine.getPublicAddresses().iterator().next();
Expand Down
27 changes: 5 additions & 22 deletions .../test/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocationStubbedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,29 +76,12 @@ public FailObtainOnPurposeException(String message) {
@BeforeMethod(alwaysRun=true)
public void setUp() throws Exception {
super.setUp();
privateAddresses = ImmutableList.of("172.168.10.11");
publicAddresses = ImmutableList.of("173.194.32.123");
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
privateAddresses = ImmutableList.of(PRIVATE_IP_ADDRESS);
publicAddresses = ImmutableList.of(PUBLIC_IP_ADDRESS);
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
}

@Override
protected NodeCreator newNodeCreator() {
return new AbstractNodeCreator() {
@Override protected NodeMetadata newNode(String group, Template template) {
NodeMetadata result = new NodeMetadataBuilder()
.id("myid")
.credentials(LoginCredentials.builder().identity("myuser").credential("mypassword").build())
.loginPort(22)
.status(Status.RUNNING)
.publicAddresses(publicAddresses)
.privateAddresses(privateAddresses)
.build();
return result;
}
};
}

@Test
@Test(enabled = false)

@rdowner rdowner Jan 8, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for disabling this test? If so, please add a comment.

public void testWithNoPrivateAddress() throws Exception {
privateAddresses = ImmutableList.of();
JcloudsSshMachineLocation machine = obtainMachine();
Expand All @@ -110,7 +93,7 @@ public void testWithNoPrivateAddress() throws Exception {

@Test
public void testWithPrivateAddress() throws Exception {
JcloudsSshMachineLocation machine = obtainMachine();
JcloudsSshMachineLocation machine = obtainMachine(ImmutableMap.of(JcloudsLocationConfig.ACCESS_IDENTITY, "testWithPrivateAddress"));
assertEquals(machine.getPrivateAddresses(), privateAddresses);
assertEquals(machine.getPrivateAddress(), Optional.of(privateAddresses.get(0)));
assertEquals(machine.getSubnetIp(), privateAddresses.get(0));
Expand Down
2 changes: 1 addition & 1 deletion ...src/test/java/org/apache/brooklyn/location/jclouds/JcloudsTemplateOptionsStubbedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public class JcloudsTemplateOptionsStubbedTest extends AbstractJcloudsStubbedUni
@Override
public void setUp() throws Exception {
super.setUp();
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
}

/**
Expand Down
2 changes: 1 addition & 1 deletion ...ava/org/apache/brooklyn/location/jclouds/networking/JcloudsPortForwardingStubbedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ static class RecordingJcloudsPortForwarderExtension implements JcloudsPortForwar
@Override
public void setUp() throws Exception {
super.setUp();
initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
}

protected AbstractNodeCreator getNodeCreator() {
Expand Down
Loading