Skip to content
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.

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

remove PortableTemplateBuilder #926

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -81,7 +81,6 @@
import org.apache.brooklyn.location.jclouds.api.JcloudsLocationPublic;
import org.apache.brooklyn.location.jclouds.networking.JcloudsPortForwarderExtension;
import org.apache.brooklyn.location.jclouds.networking.creator.DefaultAzureArmNetworkCreator;
import org.apache.brooklyn.location.jclouds.templates.PortableTemplateBuilder;
import org.apache.brooklyn.location.jclouds.templates.customize.TemplateBuilderCustomizer;
import org.apache.brooklyn.location.jclouds.templates.customize.TemplateBuilderCustomizers;
import org.apache.brooklyn.location.jclouds.templates.customize.TemplateOptionCustomizer;
@@ -1368,23 +1367,16 @@ public Template buildTemplate(ComputeService computeService, ConfigBag config, C
/** returns the jclouds Template which describes the image to be built, for the given config and compute service */
public Template buildTemplate(ComputeService computeService, ConfigBag config, JcloudsLocationCustomizer customizersDelegate) {
TemplateBuilder templateBuilder = config.get(TEMPLATE_BUILDER);
if (templateBuilder==null) {
templateBuilder = new PortableTemplateBuilder<PortableTemplateBuilder<?>>();
if (templateBuilder == null) {
templateBuilder = computeService.templateBuilder();
} else {
LOG.debug("jclouds using templateBuilder {} as custom base for provisioning in {} for {}", new Object[] {
LOG.debug("jclouds using templateBuilder {} as custom base for provisioning in {} for {}", new Object[]{
templateBuilder, this, getCreationString(config)});
}
if (templateBuilder instanceof PortableTemplateBuilder<?>) {
if (((PortableTemplateBuilder<?>)templateBuilder).imageChooser()==null) {
Function<Iterable<? extends Image>, Image> chooser = getImageChooser(computeService, config);
templateBuilder.imageChooser(chooser);
} else {
// an image chooser is already set, so do nothing
}
} else {
// template builder supplied, and we cannot check image chooser status; warn, for now
LOG.warn("Cannot check imageChooser status for {} due to manually supplied black-box TemplateBuilder; "
+ "it is recommended to use a PortableTemplateBuilder if you supply a TemplateBuilder", getCreationString(config));

Function<Iterable<? extends Image>, Image> imageChooser = getImageChooser(computeService, config);
if (imageChooser != null) {
templateBuilder.imageChooser(imageChooser);
}

if (!Strings.isEmpty(config.get(CLOUD_REGION_ID))) {
@@ -1410,34 +1402,30 @@ public Template buildTemplate(ComputeService computeService, ConfigBag config, J
}
}

if (templateBuilder instanceof PortableTemplateBuilder) {
((PortableTemplateBuilder<?>)templateBuilder).attachComputeService(computeService);
// do the default last, and only if nothing else specified (guaranteed to be a PTB if nothing else specified)
if (groovyTruth(config.get(DEFAULT_IMAGE_ID))) {
if (((PortableTemplateBuilder<?>)templateBuilder).isBlank()) {
templateBuilder.imageId(config.get(DEFAULT_IMAGE_ID).toString());
}
}
if (!Strings.isBlank(config.get(DEFAULT_IMAGE_ID))) {
templateBuilder.imageId(config.get(DEFAULT_IMAGE_ID).toString());
}

customizersDelegate.customize(this, computeService, templateBuilder);

LOG.debug("jclouds using templateBuilder {} for provisioning in {} for {}", new Object[] {
templateBuilder, this, getCreationString(config)});
LOG.debug("jclouds using templateBuilder {} for provisioning in {} for {}", new Object[]{
templateBuilder, this, getCreationString(config)});

// Finally try to build the template
Template template = null;
Image image;
try {
template = templateBuilder.build();
if (template==null) throw new IllegalStateException("No matching template; check image and hardware constraints (e.g. OS, RAM); using "+templateBuilder);
if (template == null)
throw new IllegalStateException("No matching template; check image and hardware constraints (e.g. OS, RAM); using " + templateBuilder);
image = template.getImage();
LOG.debug("jclouds found template "+template+" (image "+image+") for provisioning in "+this+" for "+getCreationString(config));
if (image==null) throw new IllegalStateException("No matching image in template at "+toStringNice()+"; check image constraints (OS, providers, ID); using "+templateBuilder);
LOG.debug("jclouds found template " + template + " (image " + image + ") for provisioning in " + this + " for " + getCreationString(config));
if (image == null)
throw new IllegalStateException("No matching image in template at " + toStringNice() + "; check image constraints (OS, providers, ID); using " + templateBuilder);
} catch (AuthorizationException e) {
LOG.warn("Error resolving template -- not authorized (rethrowing: "+e+"); template is: "+template);
throw new IllegalStateException("Not authorized to access cloud "+toStringNice()+"; "+
"check identity, credentials, and endpoint (identity='"+getIdentity()+"', credential length "+getCredential().length()+")", e);
LOG.warn("Error resolving template -- not authorized (rethrowing: " + e + "); template is: " + template);
throw new IllegalStateException("Not authorized to access cloud " + toStringNice() + "; " +
"check identity, credentials, and endpoint (identity='" + getIdentity() + "', credential length " + getCredential().length() + ")", e);
} catch (Exception e) {
try {
IOException ioe = Exceptions.getFirstThrowableOfType(e, IOException.class);
@@ -1447,17 +1435,16 @@ public Template buildTemplate(ComputeService computeService, ConfigBag config, J
}
if (listedAvailableTemplatesOnNoSuchTemplate.compareAndSet(false, true)) {
// delay subsequent log.warns (put in synch block) so the "Loading..." message is obvious
LOG.warn("Unable to match required VM template constraints "+templateBuilder+" when trying to provision VM in "+this+" (rethrowing): "+e);
LOG.warn("Unable to match required VM template constraints " + templateBuilder + " when trying to provision VM in " + this + " (rethrowing): " + e);
logAvailableTemplates(config);
}
} catch (Exception e2) {
LOG.warn("Error loading available images to report (following original error matching template which will be rethrown): "+e2, e2);
throw new IllegalStateException("Unable to access cloud "+this+" to resolve "+templateBuilder+": "+e, e);
LOG.warn("Error loading available images to report (following original error matching template which will be rethrown): " + e2, e2);
throw new IllegalStateException("Unable to access cloud " + this + " to resolve " + templateBuilder + ": " + e, e);
}
throw new IllegalStateException("Unable to match required VM template constraints "+templateBuilder+" when trying to provision VM in "+this+"; "
+ "see list of images in log. Root cause: "+e, e);
throw new IllegalStateException("Unable to match required VM template constraints " + templateBuilder + " when trying to provision VM in " + this + "; "
+ "see list of images in log. Root cause: " + e, e);
}
TemplateOptions options = template.getOptions();

// For windows, we need a startup-script to be executed that will enable winrm access.
// If there is already conflicting userMetadata, then don't replace it (and just warn).
@@ -1472,19 +1459,19 @@ public Template buildTemplate(ComputeService computeService, ConfigBag config, J
String startupScriptKey = "sysprep-specialize-script-cmd";
Object metadataMapRaw = config.get(USER_METADATA_MAP);
if (metadataMapRaw instanceof Map) {
Map<?,?> metadataMap = (Map<?, ?>) metadataMapRaw;
Map<?, ?> metadataMap = (Map<?, ?>) metadataMapRaw;
if (metadataMap.containsKey(startupScriptKey)) {
LOG.warn("Not adding startup-script for Windows VM on "+provider+", because already has key "+startupScriptKey+" in config "+USER_METADATA_MAP.getName());
LOG.warn("Not adding startup-script for Windows VM on " + provider + ", because already has key " + startupScriptKey + " in config " + USER_METADATA_MAP.getName());
} else {
Map<Object, Object> metadataMapReplacement = MutableMap.copyOf(metadataMap);
metadataMapReplacement.put(startupScriptKey, initScript);
config.put(USER_METADATA_MAP, metadataMapReplacement);
LOG.debug("Adding startup-script to enable WinRM for Windows VM on "+provider);
LOG.debug("Adding startup-script to enable WinRM for Windows VM on " + provider);
}
} else if (metadataMapRaw == null) {
Map<String, String> metadataMapReplacement = MutableMap.of(startupScriptKey, initScript);
config.put(USER_METADATA_MAP, metadataMapReplacement);
LOG.debug("Adding startup-script to enable WinRM for Windows VM on "+provider);
LOG.debug("Adding startup-script to enable WinRM for Windows VM on " + provider);
}
} else {
// For AWS and vCloudDirector, we just set user_metadata_string.
@@ -1493,26 +1480,27 @@ public Template buildTemplate(ComputeService computeService, ConfigBag config, J
boolean userMetadataMap = config.containsKey(JcloudsLocationConfig.USER_METADATA_MAP);
if (!(userMetadataString || userMetadataMap)) {
config.put(JcloudsLocationConfig.USER_METADATA_STRING, WinRmMachineLocation.getDefaultUserMetadataString(config()));
LOG.debug("Adding startup-script to enable WinRM for Windows VM on "+provider);
LOG.debug("Adding startup-script to enable WinRM for Windows VM on " + provider);
} else {
LOG.warn("Not adding startup-script for Windows VM on "+provider+", because already has config "
+(userMetadataString ? USER_METADATA_STRING.getName() : USER_METADATA_MAP.getName()));
LOG.warn("Not adding startup-script for Windows VM on " + provider + ", because already has config "
+ (userMetadataString ? USER_METADATA_STRING.getName() : USER_METADATA_MAP.getName()));
}
}
}

TemplateOptions templateOptions = template.getOptions();

for (Map.Entry<ConfigKey<?>, ? extends TemplateOptionCustomizer> entry : SUPPORTED_TEMPLATE_OPTIONS_PROPERTIES.entrySet()) {
ConfigKey<?> key = entry.getKey();
TemplateOptionCustomizer code = entry.getValue();
if (config.containsKey(key) && config.get(key) != null) {
code.apply(options, config, config.get(key));
code.apply(templateOptions, config, config.get(key));
}
}

return template;
}


/**
* See {@link https://issues.apache.org/jira/browse/JCLOUDS-1108}.
*
@@ -2621,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);
Copy link
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;
Original file line number Diff line number Diff line change
@@ -22,6 +22,7 @@

import org.jclouds.compute.domain.NodeMetadata;
import org.jclouds.compute.domain.Processor;
import org.jclouds.compute.domain.Template;
import org.jclouds.domain.Location;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -47,7 +48,7 @@ public static Predicate<NodeMetadata> except(final Predicate<NodeMetadata> predi
return Predicates.not(predicateToExclude);
}

public static Predicate<NodeMetadata> matching(final ReusableMachineTemplate template) {
public static Predicate<NodeMetadata> matching(final Template template) {
return new Predicate<NodeMetadata>() {
@Override
public boolean apply(NodeMetadata input) {
@@ -82,40 +83,31 @@ public static Predicate<NodeMetadata> compose(final Predicate<NodeMetadata> ...p
* (Caveat: If explicit Hardware, Image, and/or Template were specified in the template,
* then the hash code probably will not detect it.)
**/
public static boolean matches(ReusableMachineTemplate template, NodeMetadata m) {
public static boolean matches(Template template, NodeMetadata m) {
try {
// tags and user metadata

if (! m.getTags().containsAll( template.getTags(false) )) return false;
if (! m.getTags().containsAll( template.getOptions().getTags())) return false;

if (! isSubMapOf(template.getUserMetadata(false), m.getUserMetadata())) return false;
if (! isSubMapOf(template.getOptions().getUserMetadata(), m.getUserMetadata())) return false;


// common hardware parameters
if (m.getHardware().getRam() < template.getHardware().getRam()) return false;

if (template.getMinRam()!=null && m.getHardware().getRam() < template.getMinRam()) return false;

if (template.getMinCores()!=null) {
if (template.getHardware().getProcessors().get(0) != null) {
double numCores = 0;
for (Processor p: m.getHardware().getProcessors()) numCores += p.getCores();
if (numCores+0.001 < template.getMinCores()) return false;
}

if (template.getIs64bit()!=null) {
if (m.getOperatingSystem().is64Bit() != template.getIs64bit()) return false;
if (numCores+0.001 < template.getHardware().getProcessors().get(0).getCores()) return false;
}

if (template.getOsFamily()!=null) {
if (template.getImage().getOperatingSystem().getFamily() != null) {
if (m.getOperatingSystem() == null ||
!template.getOsFamily().equals(m.getOperatingSystem().getFamily())) return false;
!template.getImage().getOperatingSystem().getFamily().equals(m.getOperatingSystem().getFamily())) return false;
}
if (template.getOsNameMatchesRegex()!=null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This deleted line does not have an equivalent in the new version. Does this mean a potential a change in behaviour?

if (m.getOperatingSystem() == null || m.getOperatingSystem().getName()==null ||
!m.getOperatingSystem().getName().matches(template.getOsNameMatchesRegex())) return false;
}

if (template.getLocationId()!=null) {
if (!isLocationContainedIn(m.getLocation(), template.getLocationId())) return false;

if (template.getLocation().getId() != null) {
if (!isLocationContainedIn(m.getLocation(), template.getLocation().getId())) return false;
}

// TODO other TemplateBuilder fields and TemplateOptions

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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;
@@ -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 {
@@ -80,7 +78,7 @@ public void tearDown() throws Exception {
protected LocalManagementContext newManagementContext() {
return LocalManagementContextForTests.builder(true).useAdditionalProperties(customBrooklynProperties()).build();
}

/**
* For overriding.
*/
@@ -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()
@@ -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());
}

/**
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;

@@ -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))
@@ -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)
@@ -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",
@@ -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",
@@ -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);
@@ -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();
@@ -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();
Original file line number Diff line number Diff line change
@@ -94,7 +94,7 @@ public void setUp() throws Exception {

super.setUp();

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

@Override
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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++) {
@@ -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();
Original file line number Diff line number Diff line change
@@ -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();
Original file line number Diff line number Diff line change
@@ -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)
Copy link
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();
@@ -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));
Original file line number Diff line number Diff line change
@@ -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());
}

/**
Original file line number Diff line number Diff line change
@@ -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() {

Large diffs are not rendered by default.