This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit f875953201947729ba73bfd2f6f1ea8d9255a970 Author: Alex Heneveld <[email protected]> AuthorDate: Fri Jul 29 10:47:10 2022 +0100 misc clean-up for script constants use variables, introduce tests, better comments, in the course of reviewing and updating docs for this area --- .../brooklyn/core/entity/BrooklynConfigKeys.java | 18 ++++++++-------- .../util/core/file/BrooklynOsCommands.java | 7 +++--- .../ssh/SshMachineLocationSshToolTest.java | 25 ++++++++++++++++++++-- .../jclouds/provider/CarrenzaLocationLiveTest.java | 3 ++- .../entity/AbstractGoogleComputeLiveTest.java | 3 ++- .../entity/AbstractMultiDistroLiveTest.java | 3 ++- .../brooklyn/entity/AbstractSoftlayerLiveTest.java | 3 ++- .../machine/SetHostnameCustomizerLiveTest.java | 2 +- .../entity/machine/pool/ServerPoolLiveTest.java | 3 ++- .../software/base/AbstractDockerLiveTest.java | 3 ++- ...wareProcessStopsDuringStartJcloudsLiveTest.java | 2 +- .../base/test/location/SecurityGroupLiveTest.java | 3 ++- 12 files changed, 52 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java b/core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java index e61c8df619..40e071219a 100644 --- a/core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java +++ b/core/src/main/java/org/apache/brooklyn/core/entity/BrooklynConfigKeys.java @@ -18,10 +18,9 @@ */ package org.apache.brooklyn.core.entity; -import static org.apache.brooklyn.core.config.ConfigKeys.newConfigKey; -import static org.apache.brooklyn.core.config.ConfigKeys.newConfigKeyWithPrefix; -import static org.apache.brooklyn.core.config.ConfigKeys.newStringConfigKey; - +import com.google.common.annotations.Beta; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.BasicConfigInheritance; @@ -35,9 +34,7 @@ import org.apache.brooklyn.util.core.internal.ssh.ShellTool; import org.apache.brooklyn.util.core.internal.ssh.SshTool; import org.apache.brooklyn.util.time.Duration; -import com.google.common.annotations.Beta; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableMap; +import static org.apache.brooklyn.core.config.ConfigKeys.*; /** Commonly used config keys, for use in entities. Similar to {@link Attributes}. * See also {@link BrooklynServerConfig} for config keys for controlling the server. */ @@ -87,11 +84,11 @@ public class BrooklynConfigKeys { .build(); /** - * Set this configuration value to true to skip the entity startup process as with {@link #ENTITY_STARTED} if the process or + * Set this configuration value to true to skip the entity startup process as with {@link #SKIP_ENTITY_START} if the process or * service represented by the entity is already running, otherwise proceed normally. This is determined using the driver's * {@code isRunning()} method. * <p> - * If this key is set on a {@link Location} then all entities in that location will be treated in this way, again as with {@link #ENTITY_STARTED}. + * If this key is set on a {@link Location} then all entities in that location will be treated in this way, again as with {@link #SKIP_ENTITY_START}. * * @see #SKIP_ENTITY_START */ @@ -297,6 +294,9 @@ public class BrooklynConfigKeys { public static final ConfigKey<String> SSH_CONFIG_DIRECT_HEADER = newConfigKeyWithPrefix(BROOKLYN_SSH_CONFIG_KEY_PREFIX, ShellTool.PROP_DIRECT_HEADER); public static final ConfigKey<Boolean> SSH_CONFIG_NO_DELETE_SCRIPT = newConfigKeyWithPrefix(BROOKLYN_SSH_CONFIG_KEY_PREFIX, ShellTool.PROP_NO_DELETE_SCRIPT); + public static final ConfigKey<Boolean> SSH_CONFIG_SCRIPTS_IGNORE_CERTS = newBooleanConfigKey(BROOKLYN_SSH_CONFIG_KEY_PREFIX + "scripts.ignoreCerts", + "Whether to generate OS commands that ignore certs, e.g. curl -k"); + public static final MapConfigKey<Object> PROVISIONING_PROPERTIES = new MapConfigKey.Builder<Object>(Object.class, "provisioning.properties") .description("Custom properties to be passed in to the location when provisioning a new machine") .defaultValue(ImmutableMap.<String, Object>of()) diff --git a/core/src/main/java/org/apache/brooklyn/util/core/file/BrooklynOsCommands.java b/core/src/main/java/org/apache/brooklyn/util/core/file/BrooklynOsCommands.java index 54ecd2a9f7..ca636becc8 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/file/BrooklynOsCommands.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/file/BrooklynOsCommands.java @@ -22,6 +22,7 @@ import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; +import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.util.ssh.BashCommandsConfigurable; @@ -29,10 +30,10 @@ import org.apache.brooklyn.util.ssh.IptablesCommandsConfigurable; public class BrooklynOsCommands { - public static final ConfigKey<Boolean> OS_COMMANDS_IGNORE_CERTS = ConfigKeys.newBooleanConfigKey("brooklyn.os.commands.ignoreCerts", "Whether to generate OS commands that ignore certs, e.g. curl -k"); + public static final ConfigKey<Boolean> SSH_CONFIG_SCRIPT_IGNORE_CERTS = BrooklynConfigKeys.SSH_CONFIG_SCRIPTS_IGNORE_CERTS; public static BashCommandsConfigurable bash(ManagementContext mgmt) { - return BashCommandsConfigurable.newInstance().withIgnoreCerts( ((ManagementContextInternal)mgmt).getBrooklynProperties().getConfig(OS_COMMANDS_IGNORE_CERTS) ); + return BashCommandsConfigurable.newInstance().withIgnoreCerts( ((ManagementContextInternal)mgmt).getBrooklynProperties().getConfig(SSH_CONFIG_SCRIPT_IGNORE_CERTS) ); } public static IptablesCommandsConfigurable bashIptables(ManagementContext mgmt) { @@ -40,7 +41,7 @@ public class BrooklynOsCommands { } public static BashCommandsConfigurable bash(Entity entity) { - Boolean ignoreCerts = entity.config().get(OS_COMMANDS_IGNORE_CERTS); + Boolean ignoreCerts = entity.config().get(SSH_CONFIG_SCRIPT_IGNORE_CERTS); if (ignoreCerts!=null) return BashCommandsConfigurable.newInstance().withIgnoreCerts(ignoreCerts); return bash( ((EntityInternal)entity).getManagementContext() ); } diff --git a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationSshToolTest.java b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationSshToolTest.java index 20b3f290a2..7f94891449 100644 --- a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationSshToolTest.java +++ b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationSshToolTest.java @@ -25,7 +25,10 @@ import java.util.Map; import org.apache.brooklyn.api.location.LocationSpec; import org.apache.brooklyn.api.location.MachineProvisioningLocation; +import org.apache.brooklyn.core.entity.BrooklynConfigKeys; +import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport; +import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool; import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool.ExecCmd; import org.apache.brooklyn.util.core.internal.ssh.SshTool; @@ -43,7 +46,8 @@ import com.google.common.collect.Iterables; */ public class SshMachineLocationSshToolTest extends BrooklynAppUnitTestSupport { - // TODO See SshEffectorTasks.getSshFlags, called by AbstractSoftwareProcessSshDriver.getSshFlags. + // Also see SshEffectorTasks.getSshFlags, called by AbstractSoftwareProcessSshDriver.getSshFlags. + // // That retrieves all the mgmt.config, entity.config and location.config to search for ssh-related // configuration options. If you *just* instantiate the location directly, then it doesn't get the // mgmt.config options. @@ -72,7 +76,24 @@ public class SshMachineLocationSshToolTest extends BrooklynAppUnitTestSupport { .configure(SshMachineLocation.SSH_TOOL_CLASS, RecordingSshTool.class.getName())); runCustomSshToolClass(machine); } - + + @Test + public void testCustomSshToolClassNotViaBrooklynPropertiesUnlessEffector() throws Exception { + ((ManagementContextInternal)mgmt).getBrooklynProperties().put(BrooklynConfigKeys.SSH_TOOL_CLASS, RecordingSshTool.class.getName()); + SshMachineLocation machine = mgmt.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class) + .configure("address", "localhost")); + // fails because the machine doesn't get the properties; it is the SshEffectorTasks which does it + // see also EntitySshToolTest + Asserts.assertFails(() -> { runCustomSshToolClass(machine); return null; }); + } + + @Test + public void testCustomSshToolClassNotSet() throws Exception { + SshMachineLocation machine = mgmt.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class) + .configure("address", "localhost")); + Asserts.assertFails(() -> { runCustomSshToolClass(machine); return null; }); + } + @Test @SuppressWarnings("deprecation") public void testCustomSshToolClassUsingLegacy() throws Exception { diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/CarrenzaLocationLiveTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/CarrenzaLocationLiveTest.java index 32b7984843..41a178e888 100644 --- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/CarrenzaLocationLiveTest.java +++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/CarrenzaLocationLiveTest.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import org.apache.brooklyn.api.location.NoMachinesAvailableException; +import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.internal.BrooklynProperties; import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext; import org.apache.brooklyn.location.jclouds.JcloudsLocation; @@ -76,7 +77,7 @@ class CarrenzaLocationLiveTest { brooklynProperties.remove("brooklyn.jclouds."+PROVIDER+".hardware-id"); // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty") - brooklynProperties.remove("brooklyn.ssh.config.scriptHeader"); + brooklynProperties.remove(BrooklynConfigKeys.SSH_CONFIG_SCRIPT_HEADER.getName()); brooklynProperties.put("brooklyn.jclouds."+PROVIDER+".jclouds.endpoint", ENDPOINT); brooklynProperties.put("brooklyn.jclouds."+PROVIDER+".imageId", WINDOWS_IMAGE_ID); diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/AbstractGoogleComputeLiveTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/AbstractGoogleComputeLiveTest.java index 7d70dd0672..edef81846f 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/AbstractGoogleComputeLiveTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/AbstractGoogleComputeLiveTest.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import org.apache.brooklyn.api.location.Location; +import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.internal.BrooklynProperties; import org.apache.brooklyn.core.test.BrooklynAppLiveTestSupport; import org.apache.brooklyn.util.collections.MutableMap; @@ -61,7 +62,7 @@ public abstract class AbstractGoogleComputeLiveTest extends BrooklynAppLiveTestS } // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty") - result.remove("brooklyn.ssh.config.scriptHeader"); + result.remove(BrooklynConfigKeys.SSH_CONFIG_SCRIPT_HEADER.getName()); return result; } diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/AbstractMultiDistroLiveTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/AbstractMultiDistroLiveTest.java index 38a6cb37b7..66f2485ec1 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/AbstractMultiDistroLiveTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/AbstractMultiDistroLiveTest.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; import org.apache.brooklyn.api.location.Location; +import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.internal.BrooklynProperties; import org.apache.brooklyn.core.location.Machines; import org.apache.brooklyn.core.test.BrooklynAppLiveTestSupport; @@ -92,7 +93,7 @@ public abstract class AbstractMultiDistroLiveTest extends BrooklynAppLiveTestSup } // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty") - brooklynProperties.remove("brooklyn.ssh.config.scriptHeader"); + brooklynProperties.remove(BrooklynConfigKeys.SSH_CONFIG_SCRIPT_HEADER.getName()); LocalManagementContextForTests localManagementContextForTests = new LocalManagementContextForTests(brooklynProperties); localManagementContextForTests.generateManagementPlaneId(); diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/AbstractSoftlayerLiveTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/AbstractSoftlayerLiveTest.java index f7e7e0703b..ee487800e6 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/AbstractSoftlayerLiveTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/AbstractSoftlayerLiveTest.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import org.apache.brooklyn.api.location.Location; +import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.internal.BrooklynProperties; import org.apache.brooklyn.core.test.BrooklynAppLiveTestSupport; import org.apache.brooklyn.util.collections.MutableMap; @@ -60,7 +61,7 @@ public abstract class AbstractSoftlayerLiveTest extends BrooklynAppLiveTestSuppo } // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty") - result.remove("brooklyn.ssh.config.scriptHeader"); + result.remove(BrooklynConfigKeys.SSH_CONFIG_SCRIPT_HEADER.getName()); return result; } diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/machine/SetHostnameCustomizerLiveTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/machine/SetHostnameCustomizerLiveTest.java index 343814eb21..eb2a1be845 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/machine/SetHostnameCustomizerLiveTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/machine/SetHostnameCustomizerLiveTest.java @@ -75,7 +75,7 @@ public class SetHostnameCustomizerLiveTest extends BrooklynAppLiveTestSupport { } // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty") - brooklynProperties.remove("brooklyn.ssh.config.scriptHeader"); + brooklynProperties.remove(BrooklynConfigKeys.SSH_CONFIG_SCRIPT_HEADER.getName()); mgmt = new LocalManagementContext(brooklynProperties); diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/machine/pool/ServerPoolLiveTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/machine/pool/ServerPoolLiveTest.java index 60b3b0fe6a..187cc9adf3 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/machine/pool/ServerPoolLiveTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/machine/pool/ServerPoolLiveTest.java @@ -26,6 +26,7 @@ import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.core.entity.Attributes; +import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.internal.BrooklynProperties; import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; import org.apache.brooklyn.core.test.entity.TestApplication; @@ -69,7 +70,7 @@ public class ServerPoolLiveTest extends AbstractServerPoolTest { } // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty") - brooklynProperties.remove("brooklyn.ssh.config.scriptHeader"); + brooklynProperties.remove(BrooklynConfigKeys.SSH_CONFIG_SCRIPT_HEADER.getName()); return new LocalManagementContextForTests(brooklynProperties); } diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/AbstractDockerLiveTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/AbstractDockerLiveTest.java index c2f626dff5..72a26a5d13 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/AbstractDockerLiveTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/AbstractDockerLiveTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.apache.brooklyn.api.location.Location; +import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.internal.BrooklynProperties; import org.apache.brooklyn.core.test.BrooklynAppLiveTestSupport; import org.apache.brooklyn.util.collections.MutableMap; @@ -57,7 +58,7 @@ public abstract class AbstractDockerLiveTest extends BrooklynAppLiveTestSupport } // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty") - result.remove("brooklyn.ssh.config.scriptHeader"); + result.remove(BrooklynConfigKeys.SSH_CONFIG_SCRIPT_HEADER.getName()); return result; } diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java index 50ad53b86b..7839de26bd 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessStopsDuringStartJcloudsLiveTest.java @@ -95,7 +95,7 @@ public class SoftwareProcessStopsDuringStartJcloudsLiveTest extends BrooklynAppL brooklynProperties = BrooklynProperties.Factory.newDefault(); // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty") - brooklynProperties.remove("brooklyn.ssh.config.scriptHeader"); + brooklynProperties.remove(BrooklynConfigKeys.SSH_CONFIG_SCRIPT_HEADER.getName()); mgmt = new LocalManagementContextForTests(brooklynProperties); super.setUp(); diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/location/SecurityGroupLiveTest.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/location/SecurityGroupLiveTest.java index 12a5a30e80..4405e99efe 100644 --- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/location/SecurityGroupLiveTest.java +++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/location/SecurityGroupLiveTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableSet; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.location.Location; +import org.apache.brooklyn.core.entity.BrooklynConfigKeys; import org.apache.brooklyn.core.entity.EntityAsserts; import org.apache.brooklyn.core.entity.trait.Startable; import org.apache.brooklyn.core.internal.BrooklynProperties; @@ -88,7 +89,7 @@ public class SecurityGroupLiveTest extends BrooklynAppLiveTestSupport { brooklynProperties.remove("brooklyn.jclouds."+PROVIDER+".hardware-id"); // Also removes scriptHeader (e.g. if doing `. ~/.bashrc` and `. ~/.profile`, then that can cause "stdin: is not a tty") - brooklynProperties.remove("brooklyn.ssh.config.scriptHeader"); + brooklynProperties.remove(BrooklynConfigKeys.SSH_CONFIG_SCRIPT_HEADER.getName()); mgmt = new LocalManagementContextForTests(brooklynProperties);
