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.

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

remove PortableTemplateBuilder #926

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+151 −1,167
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
80 changes: 34 additions & 46 deletions 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 @@ -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;
Expand Down Expand Up @@ -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))) {
Expand All @@ -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);
Expand All @@ -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).
Expand All @@ -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.
Expand All @@ -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}.
*
Expand Down
34 changes: 13 additions & 21 deletions ...clouds/src/main/java/org/apache/brooklyn/location/jclouds/pool/MachinePoolPredicates.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
Loading