You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2022/07/29 13:53:37 UTC
[brooklyn-server] 04/06: misc clean-up for script constants
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 <al...@cloudsoft.io>
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);