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

Rebind CLI #1185

Merged
merged 8 commits into from
Feb 4, 2014
Merged
Show file tree
Hide file tree
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
5 changes: 4 additions & 1 deletion api/src/main/java/brooklyn/entity/proxying/EntitySpec.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ public static <T extends Entity> EntitySpec<T> create(EntitySpec<T> spec) {
.configure(spec.getConfig())
.configure(spec.getFlags())
.policySpecs(spec.getPolicySpecs())
.policies(spec.getPolicies());
.policies(spec.getPolicies())
.enricherSpecs(spec.getEnricherSpecs())
.enrichers(spec.getEnrichers())
.addInitializers(spec.getInitializers());

if (spec.getParent() != null) result.parent(spec.getParent());
if (spec.getImplementation() != null) result.impl(spec.getImplementation());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ public String getIconUrl() {
public void setDisplayName(String newDisplayName) {
displayName.set(newDisplayName);
displayNameAutoGenerated = false;
getManagementSupport().getEntityChangeListener().onChanged();
}

/** allows subclasses to set the default display name to use if none is provided */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ public class BrooklynConfigKeys {
+ "apps/${entity.applicationId}/"
+ "entities/${entity.entityType.simpleName}_"
+ "${entity.id}");


public static final BasicAttributeSensorAndConfigKey<String> EXPANDED_INSTALL_DIR = new TemplatedStringAttributeSensorAndConfigKey(
Copy link
Member

Choose a reason for hiding this comment

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

can you add javadoc with the reason for this, or comments on the confusing lines -- in particular its interplay with INSTALL_DIR, eg things like entity.setAttribute(SoftwareProcess.INSTALL_DIR, expandedInstallDir); in AbstractSoftwareProcessSshDriver and calls to setExpandedInstallDir to the installDir in some of the entities

Copy link
Member Author

Choose a reason for hiding this comment

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

Added:

/**
 * Indicates the directory in which the unpacked install artifacts can be found. For example, for Tomcat if INSTALL_DIR is 
 * {@code /path/to/installdir}, then the tgz file will be downloaded to {@code /path/to/installdir/apache-tomcat-7.0.47.tar.gz}.
 * When that is unpacked, the actual files will (normally) be found in {@code /path/to/installdir/apache-tomcat-7.0.47/}.
 * The EXPANDED_INSTALL_DIR attribute will be set to this during installation, allowing the appropriate files (e.g. /bin/startup.sh)
 * to be found. 
 * <p>
 * However, an enterprise user may have a bespoke build where the unpacked dir has a different name. If so, the config key
 * could be set to something like:
 *     <pre>
 *     "${(config['install.dir']}/acme-tomcat-1.0.0"
 *     </pre>
 */

Thinking about this more, I'm not sure how well this will cope with someone setting the EXPANDED_INSTALL_DIR configuration. We might just overwrite it in the driver's install method, logging a warning! I'll need to look at that more.

"expandedinstall.dir",
"Directory for installed artifacts (e.g. expanded dir after unpacking .tgz)",
null);

/** @deprecated since 0.7.0; use {@link #INSTALL_DIR} */
public static final ConfigKey<String> SUGGESTED_INSTALL_DIR = INSTALL_DIR.getConfigKey();
/** @deprecated since 0.7.0; use {@link #RUN_DIR} */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ public boolean apply(@Nullable Entity input) {
};
}

public static <T> Predicate<Entity> displayNameEqualTo(final T val) {
return new Predicate<Entity>() {
@Override
public boolean apply(@Nullable Entity input) {
return Objects.equal(input.getDisplayName(), val);
}
};
}

public static Predicate<Entity> applicationIdEqualTo(final String val) {
return new Predicate<Entity>() {
@Override
Expand Down
18 changes: 17 additions & 1 deletion core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import brooklyn.util.exceptions.Exceptions;
import brooklyn.util.flags.FlagUtils;
import brooklyn.util.javalang.Reflections;
import brooklyn.util.time.Duration;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
Expand Down Expand Up @@ -74,6 +75,10 @@ public RebindManagerImpl(ManagementContextInternal managementContext) {
/**
* Must be called before setPerister()
*/
public void setPeriodicPersistPeriod(Duration period) {
this.periodicPersistPeriod = period.toMilliseconds();
}

public void setPeriodicPersistPeriod(long periodMillis) {
this.periodicPersistPeriod = periodMillis;
}
Expand Down Expand Up @@ -302,7 +307,6 @@ private Entity newEntity(EntityMemento memento, Reflections reflections) {
// a proxy for if another entity needs to reference it during the init phase.
InternalEntityFactory entityFactory = managementContext.getEntityFactory();
Entity entity = entityFactory.constructEntity(entityClazz);

FlagUtils.setFieldsFromFlags(ImmutableMap.of("id", entityId), entity);
if (entity instanceof AbstractApplication) {
FlagUtils.setFieldsFromFlags(ImmutableMap.of("mgmt", managementContext), entity);
Expand All @@ -327,7 +331,19 @@ private Entity newEntity(EntityMemento memento, Reflections reflections) {
flags.put("id", entityId);
if (AbstractApplication.class.isAssignableFrom(entityClazz)) flags.put("mgmt", managementContext);
Entity entity = (Entity) invokeConstructor(reflections, entityClazz, new Object[] {flags}, new Object[] {flags, null}, new Object[] {null}, new Object[0]);

// In case the constructor didn't take the Map arg, then also set it here.
// e.g. for top-level app instances such as WebClusterDatabaseExampleApp will (often?) not have
// interface + constructor.
// TODO On serializing the memento, we should capture which interfaces so can recreate
// the proxy+spec (including for apps where there's not an obvious interface).
FlagUtils.setFieldsFromFlags(ImmutableMap.of("id", entityId), entity);
if (entity instanceof AbstractApplication) {
FlagUtils.setFieldsFromFlags(ImmutableMap.of("mgmt", managementContext), entity);
}
((AbstractEntity)entity).setManagementContext(managementContext);
managementContext.prePreManage(entity);

return entity;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ public TemplatedStringAttributeSensorAndConfigKey(TemplatedStringAttributeSensor
super(orig, defaultValue);
}

@Override
protected String convertConfigToSensor(String value, Entity entity) {
if (value == null) return null;
Copy link
Member

Choose a reason for hiding this comment

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

:)

return TemplateProcessor.processTemplateContents(value, (EntityInternal)entity, ImmutableMap.<String,Object>of());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
public interface EntityChangeListener {

public static final EntityChangeListener NOOP = new EntityChangeListener() {
@Override public void onChanged() {}
@Override public void onAttributeChanged(AttributeSensor<?> attribute) {}
@Override public void onConfigChanged(ConfigKey<?> key) {}
@Override public void onLocationsChanged() {}
Expand All @@ -17,6 +18,8 @@ public interface EntityChangeListener {
@Override public void onEffectorCompleted(Effector<?> effector) {}
};

void onChanged();

void onAttributeChanged(AttributeSensor<?> attribute);

void onConfigChanged(ConfigKey<?> key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,10 @@ public EntityChangeListener getEntityChangeListener() {
}

private class EntityChangeListenerImpl implements EntityChangeListener {
@Override
public void onChanged() {
getManagementContext().getRebindManager().getChangeListener().onChanged(entity);
}
@Override
public void onChildrenChanged() {
getManagementContext().getRebindManager().getChangeListener().onChanged(entity);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
*/
public class Infinispan5SshDriver extends JavaSoftwareProcessSshDriver implements Infinispan5Driver {

private String expandedInstallDir;

public Infinispan5SshDriver(Infinispan5Server entity, SshMachineLocation machine) {
super(entity, machine);
}
Expand All @@ -41,18 +39,13 @@ protected Integer getPort() {
return entity.getAttribute(Infinispan5Server.PORT);
}

private String getExpandedInstallDir() {
if (expandedInstallDir == null) throw new IllegalStateException("expandedInstallDir is null; most likely install was not called");
return expandedInstallDir;
}

@Override
public void install() {
DownloadResolver resolver = Entities.newDownloader(this);
List<String> urls = resolver.getTargets();
String saveAs = resolver.getFilename();
// FIXME will saveAs be "infinispan-${version}-all.zip"?
expandedInstallDir = getInstallDir(); // unpacks to current directory, rather than sub-directory
setExpandedInstallDir(getInstallDir()); // unpacks to current directory, rather than sub-directory
Copy link
Member

Choose a reason for hiding this comment

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

re earlier comment

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this one is correct - the zip unpacks to the current directory rather than creating a sub-dir, so the "expanded install dir" is the same as the "install dir".


List<String> commands = ImmutableList.<String>builder()
.addAll(BashCommands.commandsToDownloadUrlsAs(urls, saveAs))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static brooklyn.util.GroovyJavaMethods.elvis;
import static brooklyn.util.GroovyJavaMethods.truth;
import static com.google.common.base.Preconditions.checkNotNull;

import java.io.File;
import java.io.StringReader;
Expand Down Expand Up @@ -51,6 +52,7 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
// we cache these in case the entity becomes unmanaged
private volatile String installDir;
private volatile String runDir;
private volatile String expandedInstallDir;

/** include this flag in newScript creation to prevent entity-level flags from being included;
* any SSH-specific flags passed to newScript override flags from the entity,
Expand Down Expand Up @@ -120,6 +122,12 @@ protected String getEntityVersionLabel(String separator) {

public String getInstallDir() {
if (installDir != null) return installDir;

String existingVal = getEntity().getAttribute(SoftwareProcess.INSTALL_DIR);
if (Strings.isNonBlank(existingVal)) { // e.g. on rebind
installDir = existingVal;
return installDir;
}

// deprecated in 0.7.0
Maybe<Object> minstallDir = getEntity().getConfigRaw(SoftwareProcess.INSTALL_DIR, true);
Expand All @@ -142,6 +150,12 @@ public String getInstallDir() {
public String getRunDir() {
if (runDir != null) return runDir;

String existingVal = getEntity().getAttribute(SoftwareProcess.RUN_DIR);
if (Strings.isNonBlank(existingVal)) { // e.g. on rebind
runDir = existingVal;
return runDir;
}

// deprecated in 0.7.0
Maybe<Object> mRunDir = getEntity().getConfigRaw(SoftwareProcess.RUN_DIR, true);
if (!mRunDir.isPresent() || mRunDir.get()==null) {
Expand All @@ -160,6 +174,29 @@ public String getRunDir() {
return runDir;
}

public void setExpandedInstallDir(String val) {
checkNotNull(val, "expandedInstallDir");
String oldVal = getEntity().getAttribute(SoftwareProcess.EXPANDED_INSTALL_DIR);
if (Strings.isNonBlank(oldVal) && !oldVal.equals(val)) {
log.info("Resetting expandedInstallDir (to "+val+" from "+oldVal+") for "+getEntity());
}

getEntity().setAttribute(SoftwareProcess.EXPANDED_INSTALL_DIR, val);
}

public String getExpandedInstallDir() {
if (expandedInstallDir != null) return expandedInstallDir;

String untidiedVal = ConfigToAttributes.apply(getEntity(), SoftwareProcess.EXPANDED_INSTALL_DIR);
if (Strings.isNonBlank(untidiedVal)) {
expandedInstallDir = Os.tidyPath(untidiedVal);
entity.setAttribute(SoftwareProcess.INSTALL_DIR, expandedInstallDir);
Copy link
Member

Choose a reason for hiding this comment

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

maybe this is just a mistake? see earlier comment

Copy link
Member Author

Choose a reason for hiding this comment

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

Well spotted; yes a mistake.

return expandedInstallDir;
} else {
throw new IllegalStateException("expandedInstallDir is null; most likely install was not called for "+getEntity());
}
}

public SshMachineLocation getMachine() { return getLocation(); }
public String getHostname() { return entity.getAttribute(Attributes.HOSTNAME); }
public String getAddress() { return entity.getAttribute(Attributes.ADDRESS); }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ public interface SoftwareProcess extends Entity, Startable {
@SetFromFlag("downloadAddonUrls")
BasicAttributeSensorAndConfigKey<Map<String,String>> DOWNLOAD_ADDON_URLS = Attributes.DOWNLOAD_ADDON_URLS;

@SetFromFlag("expandedInstallDir")
BasicAttributeSensorAndConfigKey<String> EXPANDED_INSTALL_DIR = BrooklynConfigKeys.EXPANDED_INSTALL_DIR;

@SetFromFlag("installDir")
BasicAttributeSensorAndConfigKey<String> INSTALL_DIR = BrooklynConfigKeys.INSTALL_DIR;
@Deprecated
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package brooklyn.entity.brooklynnode;

import static com.google.common.base.Preconditions.*;
import static java.lang.String.*;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.lang.String.format;

import java.io.ByteArrayInputStream;
import java.io.File;
Expand All @@ -27,8 +27,6 @@

public class BrooklynNodeSshDriver extends JavaSoftwareProcessSshDriver implements BrooklynNodeDriver {

private String expandedInstallDir;

public BrooklynNodeSshDriver(BrooklynNodeImpl entity, SshMachineLocation machine) {
super(entity, machine);
}
Expand All @@ -51,11 +49,6 @@ private String getPidFile() {
return "pid_java";
}

protected String getExpandedInstallDir() {
if (expandedInstallDir == null) throw new IllegalStateException("expandedInstallDir is null; most likely install was not called");
return expandedInstallDir;
}

@Override
public void install() {
String uploadUrl = entity.getConfig(BrooklynNode.DISTRO_UPLOAD_URL);
Expand All @@ -66,7 +59,7 @@ public void install() {
DownloadResolver resolver = Entities.newDownloader(this, ImmutableMap.of("filename", "brooklyn-dist-"+getVersion()+"-dist.tar.gz"));
List<String> urls = resolver.getTargets();
String saveAs = resolver.getFilename();
expandedInstallDir = getInstallDir()+"/"+resolver.getUnpackedDirectoryName(format("brooklyn-%s", getVersion()));
setExpandedInstallDir(getInstallDir()+"/"+resolver.getUnpackedDirectoryName(format("brooklyn-%s", getVersion())));

newScript("createInstallDir")
.body.append("mkdir -p "+getInstallDir())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ public class MariaDbSshDriver extends AbstractSoftwareProcessSshDriver implement

public static final Logger log = LoggerFactory.getLogger(MariaDbSshDriver.class);

private String expandedInstallDir;

public MariaDbSshDriver(MariaDbNodeImpl entity, SshMachineLocation machine) {
super(entity, machine);

Expand Down Expand Up @@ -94,17 +92,12 @@ public String getInstallFilename() {
return String.format("mariadb-%s-%s.tar.gz", getVersion(), getOsTag());
}

private String getExpandedInstallDir() {
if (expandedInstallDir == null) throw new IllegalStateException("expandedInstallDir is null; most likely install was not called");
return expandedInstallDir;
}

@Override
public void install() {
DownloadResolver resolver = Entities.newDownloader(this, ImmutableMap.of("filename", getInstallFilename()));
List<String> urls = resolver.getTargets();
String saveAs = resolver.getFilename();
expandedInstallDir = getInstallDir() + "/" + resolver.getUnpackedDirectoryName(format("mariadb-%s-%s", getVersion(), getOsTag()));
setExpandedInstallDir(getInstallDir() + "/" + resolver.getUnpackedDirectoryName(format("mariadb-%s-%s", getVersion(), getOsTag())));

List<String> commands = new LinkedList<String>();
commands.add(BashCommands.INSTALL_TAR);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package brooklyn.entity.database.mysql;

import static brooklyn.util.GroovyJavaMethods.truth;
import static brooklyn.util.ssh.BashCommands.*;
import static brooklyn.util.ssh.BashCommands.commandsToDownloadUrlsAs;
import static brooklyn.util.ssh.BashCommands.installPackage;
import static brooklyn.util.ssh.BashCommands.ok;
import static java.lang.String.format;

import java.io.InputStream;
Expand Down Expand Up @@ -43,8 +45,6 @@ public class MySqlSshDriver extends AbstractSoftwareProcessSshDriver implements

public static final Logger log = LoggerFactory.getLogger(MySqlSshDriver.class);

private String expandedInstallDir;

public MySqlSshDriver(MySqlNodeImpl entity, SshMachineLocation machine) {
super(entity, machine);

Expand Down Expand Up @@ -92,17 +92,12 @@ public String getInstallFilename() {
return String.format("mysql-%s-%s.tar.gz", getVersion(), getOsTag());
}

private String getExpandedInstallDir() {
if (expandedInstallDir == null) throw new IllegalStateException("expandedInstallDir is null; most likely install was not called");
return expandedInstallDir;
}

@Override
public void install() {
DownloadResolver resolver = Entities.newDownloader(this, ImmutableMap.of("filename", getInstallFilename()));
List<String> urls = resolver.getTargets();
String saveAs = resolver.getFilename();
expandedInstallDir = getInstallDir() + "/" + resolver.getUnpackedDirectoryName(format("mysql-%s-%s", getVersion(), getOsTag()));
setExpandedInstallDir(getInstallDir() + "/" + resolver.getUnpackedDirectoryName(format("mysql-%s-%s", getVersion(), getOsTag())));
Copy link
Member

Choose a reason for hiding this comment

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

if EXPANDED_INSTALL_DIR is really a config key shouldn't this be something like setDefaultExpandedInstallDir ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a BasicAttributeSensorAndConfigKey (or more strictly a TemplatedStringAttributeSensorAndConfigKey). So this method sets the attribute rather than the default-config. Does that make sense to you with the current naming?

Copy link
Member

Choose a reason for hiding this comment

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

right, so maybe rename the method forceExpandedInstallDir and as you suggested, log a warning if the config key has been manually set ?


List<String> commands = new LinkedList<String>();
commands.add(BashCommands.INSTALL_TAR);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,12 @@ public class RubyRepSshDriver extends AbstractSoftwareProcessSshDriver implement

public static final Logger log = LoggerFactory.getLogger(RubyRepSshDriver.class);

private String expandedInstallDir;

public RubyRepSshDriver(EntityLocal entity, SshMachineLocation machine) {
super(entity, machine);

entity.setAttribute(Attributes.LOG_FILE_LOCATION, getLogFileLocation());
}

private String getExpandedInstallDir() {
if (expandedInstallDir == null) throw new IllegalStateException("expandedInstallDir is null; most likely install was not called");
return expandedInstallDir;
}

protected String getLogFileLocation() {
return getRunDir() + "/log/rubyrep.log";
}
Expand All @@ -60,7 +53,7 @@ public void install() {
.failOnNonZeroResultCode()
.execute();

expandedInstallDir = getInstallDir()+"/"+resolver.getUnpackedDirectoryName(format("rubyrep-%s", getVersion()));
setExpandedInstallDir(getInstallDir()+"/"+resolver.getUnpackedDirectoryName(format("rubyrep-%s", getVersion())));
}

@Override
Expand Down
Loading