From 905a44ec8cf2f188c9bf351aae57f649f0447b6f Mon Sep 17 00:00:00 2001 From: Ivana Yovcheva Date: Thu, 18 May 2017 03:11:55 +0300 Subject: [PATCH 1/4] External ip config for winrm byon location --- ...MachineLocationExternalConfigYamlTest.java | 18 ++-- .../location/byon/ByonLocationResolver.java | 87 ++++++++++--------- 2 files changed, 59 insertions(+), 46 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java index 681367d956..a31a46a1e5 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java @@ -41,6 +41,8 @@ protected LocalManagementContext newTestManagementContext() { BrooklynProperties props = BrooklynProperties.Factory.newEmpty(); props.put("brooklyn.external.inPlaceSupplier1", "org.apache.brooklyn.core.config.external.InPlaceExternalConfigSupplier"); props.put("brooklyn.external.inPlaceSupplier1.byonPassword", "passw0rd"); + props.put("brooklyn.external.inPlaceSupplier1.byonUser", "admin"); + props.put("brooklyn.external.inPlaceSupplier1.ip", "127.0.0.1"); return LocalManagementContextForTests.builder(true) .useProperties(props) @@ -49,13 +51,13 @@ protected LocalManagementContext newTestManagementContext() { } @Test() - public void testWindowsMachinesPasswordExternalProvider() throws Exception { + public void testWindowsMachinesExternalProvider() throws Exception { RecordingWinRmTool.constructorProps.clear(); final String yaml = Joiner.on("\n").join("location:", " byon:", " hosts:", - " - winrm: 127.0.0.1", - " user: admin", + " - winrm: $brooklyn:external(\"inPlaceSupplier1\", \"ip\")", + " user: $brooklyn:external(\"inPlaceSupplier1\", \"byonUser\")", " brooklyn.winrm.config.winrmToolClass: org.apache.brooklyn.util.core.internal.winrm.RecordingWinRmTool", " password: $brooklyn:external(\"inPlaceSupplier1\", \"byonPassword\")", " osFamily: windows", @@ -67,17 +69,19 @@ public void testWindowsMachinesPasswordExternalProvider() throws Exception { BasicApplication app = (BasicApplication) createAndStartApplication(yaml); waitForApplicationTasks(app); + assertEquals(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.ADDRESS.getName()), "127.0.0.1"); + assertEquals(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.USER.getName()), "admin"); assertEquals(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.PASSWORD.getName()), "passw0rd"); } @Test() - public void testWindowsMachinesNoPasswordExternalProvider() throws Exception { + public void testWindowsMachinesNoExternalProvider() throws Exception { RecordingWinRmTool.constructorProps.clear(); final String yaml = Joiner.on("\n").join("location:", " byon:", " hosts:", - " - winrm: 127.0.0.1", - " user: admin", + " - winrm: $brooklyn:external(\"inPlaceSupplier1\", \"ipEmpty\")", + " user: $brooklyn:external(\"inPlaceSupplier1\", \"byonUserEmpty\")", " brooklyn.winrm.config.winrmToolClass: org.apache.brooklyn.util.core.internal.winrm.RecordingWinRmTool", " password: $brooklyn:external(\"inPlaceSupplier1\", \"byonPasswordddd\")", " osFamily: windows", @@ -89,6 +93,8 @@ public void testWindowsMachinesNoPasswordExternalProvider() throws Exception { BasicApplication app = (BasicApplication) createAndStartApplication(yaml); waitForApplicationTasks(app); + assertNull(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.ADDRESS.getName())); + assertNull(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.USER.getName())); assertNull(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.PASSWORD.getName())); } diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java index 156b720a01..888b632b5b 100644 --- a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java +++ b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java @@ -34,6 +34,7 @@ import org.apache.brooklyn.core.config.Sanitizer; import org.apache.brooklyn.core.location.AbstractLocationResolver; import org.apache.brooklyn.core.mgmt.internal.LocalLocationManager; +import org.apache.brooklyn.core.objs.BasicConfigurableObject; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ClassLoaderUtils; import org.apache.brooklyn.util.core.config.ConfigBag; @@ -152,49 +153,55 @@ protected LocationSpec parseMachine(Map va String osFamily = (String) machineConfig.remove(OS_FAMILY.getName()); String ssh = (String) machineConfig.remove("ssh"); - String winrm = (String) machineConfig.remove("winrm"); - Map tcpPortMappings = (Map) machineConfig.get("tcpPortMappings"); - - checkArgument(ssh != null ^ winrm != null, "Must specify exactly one of 'ssh' or 'winrm' for machine: %s", valSanitized); - - UserAndHostAndPort userAndHostAndPort; - String host; - int port; - if (ssh != null) { - userAndHostAndPort = parseUserAndHostAndPort(ssh, 22); + if (machineConfig.containsKey("winrm") && !(machineConfig.get("winrm") instanceof String)) { + machineConfig.put("address", machineConfig.get("winrm")); + machineConfig.remove("winrm"); } else { - // TODO set to null and rely on the MachineLocation. If not then make a dependency to WinRmMachineLocation and use its config key name. - userAndHostAndPort = parseUserAndHostAndPort(winrm, vals.get("winrm.useHttps") != null && (Boolean)vals.get("winrm.useHttps") ? 5986 : 5985); - } - - // If there is a tcpPortMapping defined for the connection-port, then use that for ssh/winrm machine - port = userAndHostAndPort.getHostAndPort().getPort(); - if (tcpPortMappings != null && tcpPortMappings.containsKey(port)) { - String override = tcpPortMappings.get(port); - HostAndPort hostAndPortOverride = HostAndPort.fromString(override); - if (!hostAndPortOverride.hasPort()) { - throw new IllegalArgumentException("Invalid portMapping ('"+override+"') for port "+port+" in "+specForErrMsg); + String winrm = (String) machineConfig.remove("winrm"); + + Map tcpPortMappings = (Map) machineConfig.get("tcpPortMappings"); + + checkArgument(ssh != null ^ winrm != null, "Must specify exactly one of 'ssh' or 'winrm' for machine: %s", valSanitized); + + UserAndHostAndPort userAndHostAndPort; + String host; + int port; + if (ssh != null) { + userAndHostAndPort = parseUserAndHostAndPort(ssh, 22); + } else { + // TODO set to null and rely on the MachineLocation. If not then make a dependency to WinRmMachineLocation and use its config key name. + userAndHostAndPort = parseUserAndHostAndPort(winrm, vals.get("winrm.useHttps") != null && (Boolean)vals.get("winrm.useHttps") ? 5986 : 5985); } - port = hostAndPortOverride.getPort(); - host = hostAndPortOverride.getHostText().trim(); - } else { - host = userAndHostAndPort.getHostAndPort().getHostText().trim(); - } - - machineConfig.put("address", host); - try { - InetAddress.getByName(host); - } catch (Exception e) { - throw new IllegalArgumentException("Invalid host '"+host+"' specified in '"+specForErrMsg+"': "+e); - } - if (userAndHostAndPort.getUser() != null) { - checkArgument(!vals.containsKey("user"), "Must not specify user twice for machine: %s", valSanitized); - machineConfig.put("user", userAndHostAndPort.getUser()); - } - if (userAndHostAndPort.getHostAndPort().hasPort()) { - checkArgument(!vals.containsKey("port"), "Must not specify port twice for machine: %s", valSanitized); - machineConfig.put("port", port); + // If there is a tcpPortMapping defined for the connection-port, then use that for ssh/winrm machine + port = userAndHostAndPort.getHostAndPort().getPort(); + if (tcpPortMappings != null && tcpPortMappings.containsKey(port)) { + String override = tcpPortMappings.get(port); + HostAndPort hostAndPortOverride = HostAndPort.fromString(override); + if (!hostAndPortOverride.hasPort()) { + throw new IllegalArgumentException("Invalid portMapping ('"+override+"') for port "+port+" in "+specForErrMsg); + } + port = hostAndPortOverride.getPort(); + host = hostAndPortOverride.getHostText().trim(); + } else { + host = userAndHostAndPort.getHostAndPort().getHostText().trim(); + } + + machineConfig.put("address", host); + try { + InetAddress.getByName(host); + } catch (Exception e) { + throw new IllegalArgumentException("Invalid host '"+host+"' specified in '"+specForErrMsg+"': "+e); + } + + if (userAndHostAndPort.getUser() != null) { + checkArgument(!vals.containsKey("user"), "Must not specify user twice for machine: %s", valSanitized); + machineConfig.put("user", userAndHostAndPort.getUser()); + } + if (userAndHostAndPort.getHostAndPort().hasPort()) { + checkArgument(!vals.containsKey("port"), "Must not specify port twice for machine: %s", valSanitized); + machineConfig.put("port", port); + } } for (Map.Entry entry : defaults.entrySet()) { if (!machineConfig.containsKey(entry.getKey())) { From f919a3968e0ccf7ee48f05e0b651c65a1be962e9 Mon Sep 17 00:00:00 2001 From: Ivana Yovcheva Date: Wed, 24 May 2017 13:15:48 +0300 Subject: [PATCH 2/4] Fix WinRmMachineLocationExternalConfigYamlTest --- ...MachineLocationExternalConfigYamlTest.java | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java index a31a46a1e5..7257ef33c8 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java @@ -24,6 +24,7 @@ import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.location.winrm.WinRmMachineLocation; +import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.core.internal.winrm.RecordingWinRmTool; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -69,7 +70,7 @@ public void testWindowsMachinesExternalProvider() throws Exception { BasicApplication app = (BasicApplication) createAndStartApplication(yaml); waitForApplicationTasks(app); - assertEquals(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.ADDRESS.getName()), "127.0.0.1"); + assertEquals(RecordingWinRmTool.constructorProps.get(0).get("host"), "127.0.0.1"); assertEquals(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.USER.getName()), "admin"); assertEquals(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.PASSWORD.getName()), "passw0rd"); } @@ -80,7 +81,7 @@ public void testWindowsMachinesNoExternalProvider() throws Exception { final String yaml = Joiner.on("\n").join("location:", " byon:", " hosts:", - " - winrm: $brooklyn:external(\"inPlaceSupplier1\", \"ipEmpty\")", + " - winrm: 127.0.0.1", " user: $brooklyn:external(\"inPlaceSupplier1\", \"byonUserEmpty\")", " brooklyn.winrm.config.winrmToolClass: org.apache.brooklyn.util.core.internal.winrm.RecordingWinRmTool", " password: $brooklyn:external(\"inPlaceSupplier1\", \"byonPasswordddd\")", @@ -93,11 +94,36 @@ public void testWindowsMachinesNoExternalProvider() throws Exception { BasicApplication app = (BasicApplication) createAndStartApplication(yaml); waitForApplicationTasks(app); - assertNull(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.ADDRESS.getName())); assertNull(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.USER.getName())); assertNull(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.PASSWORD.getName())); } + @Test() + public void testWindowsMachinesNoExternalIPProvider() throws Exception { + RecordingWinRmTool.constructorProps.clear(); + final String yaml = Joiner.on("\n").join("location:", + " byon:", + " hosts:", + " - winrm: $brooklyn:external(\"inPlaceSupplier1\", \"ipEmpty\")", + " user: $brooklyn:external(\"inPlaceSupplier1\", \"byonUserEmpty\")", + " brooklyn.winrm.config.winrmToolClass: org.apache.brooklyn.util.core.internal.winrm.RecordingWinRmTool", + " password: $brooklyn:external(\"inPlaceSupplier1\", \"byonPasswordddd\")", + " osFamily: windows", + "services:", + "- type: org.apache.brooklyn.entity.software.base.VanillaWindowsProcess", + " brooklyn.config:", + " launch.command: echo launch", + " checkRunning.command: echo running"); + + try { + BasicApplication app = (BasicApplication) createAndStartApplication(yaml); + waitForApplicationTasks(app); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + Asserts.expectedFailureOfType(e, NullPointerException.class); + } + } + @Override protected Logger getLogger() { return log; From 58847c13d6a7ddd6508c529993a6228073de9a4f Mon Sep 17 00:00:00 2001 From: Ivana Yovcheva Date: Fri, 4 Aug 2017 09:24:22 +0300 Subject: [PATCH 3/4] Move byon config parsing to machine location --- .../location/byon/ByonLocationResolver.java | 58 +------------- .../FixedListMachineProvisioningLocation.java | 76 ++++++++++++++++++- 2 files changed, 78 insertions(+), 56 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java index 888b632b5b..009ee05eef 100644 --- a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java +++ b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java @@ -134,7 +134,7 @@ protected ConfigBag extractConfig(Map locationFlags, String spec, LocationR if (host instanceof String) { machineSpec = parseMachine((String)host, locationClass, defaultProps, spec); } else if (host instanceof Map) { - machineSpec = parseMachine((Map)host, locationClass, defaultProps, spec); + machineSpec = parseMachine((Map)host, locationClass, defaultProps); } else { throw new IllegalArgumentException("Expected machine to be String or Map, but was "+host.getClass().getName()+" ("+host+")"); } @@ -147,68 +147,16 @@ protected ConfigBag extractConfig(Map locationFlags, String spec, LocationR return config; } - protected LocationSpec parseMachine(Map vals, Class locationClass, Map defaults, String specForErrMsg) { - Map valSanitized = Sanitizer.sanitize(vals); + protected LocationSpec parseMachine(Map vals, Class locationClass, Map defaults) { Map machineConfig = MutableMap.copyOf(vals); String osFamily = (String) machineConfig.remove(OS_FAMILY.getName()); - String ssh = (String) machineConfig.remove("ssh"); - if (machineConfig.containsKey("winrm") && !(machineConfig.get("winrm") instanceof String)) { - machineConfig.put("address", machineConfig.get("winrm")); - machineConfig.remove("winrm"); - } else { - String winrm = (String) machineConfig.remove("winrm"); - - Map tcpPortMappings = (Map) machineConfig.get("tcpPortMappings"); - - checkArgument(ssh != null ^ winrm != null, "Must specify exactly one of 'ssh' or 'winrm' for machine: %s", valSanitized); - - UserAndHostAndPort userAndHostAndPort; - String host; - int port; - if (ssh != null) { - userAndHostAndPort = parseUserAndHostAndPort(ssh, 22); - } else { - // TODO set to null and rely on the MachineLocation. If not then make a dependency to WinRmMachineLocation and use its config key name. - userAndHostAndPort = parseUserAndHostAndPort(winrm, vals.get("winrm.useHttps") != null && (Boolean)vals.get("winrm.useHttps") ? 5986 : 5985); - } - - // If there is a tcpPortMapping defined for the connection-port, then use that for ssh/winrm machine - port = userAndHostAndPort.getHostAndPort().getPort(); - if (tcpPortMappings != null && tcpPortMappings.containsKey(port)) { - String override = tcpPortMappings.get(port); - HostAndPort hostAndPortOverride = HostAndPort.fromString(override); - if (!hostAndPortOverride.hasPort()) { - throw new IllegalArgumentException("Invalid portMapping ('"+override+"') for port "+port+" in "+specForErrMsg); - } - port = hostAndPortOverride.getPort(); - host = hostAndPortOverride.getHostText().trim(); - } else { - host = userAndHostAndPort.getHostAndPort().getHostText().trim(); - } - - machineConfig.put("address", host); - try { - InetAddress.getByName(host); - } catch (Exception e) { - throw new IllegalArgumentException("Invalid host '"+host+"' specified in '"+specForErrMsg+"': "+e); - } - - if (userAndHostAndPort.getUser() != null) { - checkArgument(!vals.containsKey("user"), "Must not specify user twice for machine: %s", valSanitized); - machineConfig.put("user", userAndHostAndPort.getUser()); - } - if (userAndHostAndPort.getHostAndPort().hasPort()) { - checkArgument(!vals.containsKey("port"), "Must not specify port twice for machine: %s", valSanitized); - machineConfig.put("port", port); - } - } for (Map.Entry entry : defaults.entrySet()) { if (!machineConfig.containsKey(entry.getKey())) { machineConfig.put(entry.getKey(), entry.getValue()); } } - + Class locationClassHere = locationClass; if (osFamily != null) { locationClassHere = getLocationClass(osFamily); diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java index 87b83f8928..89b0c3ba8d 100644 --- a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java +++ b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java @@ -22,12 +22,14 @@ import java.io.Closeable; import java.io.File; +import java.net.InetAddress; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; +import com.google.common.net.HostAndPort; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.location.LocationSpec; import org.apache.brooklyn.api.location.MachineLocation; @@ -45,6 +47,7 @@ import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.SetFromFlag; +import org.apache.brooklyn.util.net.UserAndHostAndPort; import org.apache.brooklyn.util.stream.Streams; import org.apache.brooklyn.util.text.WildcardGlobs; import org.apache.brooklyn.util.text.WildcardGlobs.PhraseTreatment; @@ -317,7 +320,7 @@ public T obtain(Map flags) throws NoMachinesAvailableException { T desiredMachine = (T) flags.get("desiredMachine"); ConfigBag allflags = ConfigBag.newInstanceExtending(config().getBag()).putAll(flags); Function, MachineLocation> chooser = allflags.get(MACHINE_CHOOSER); - + synchronized (lock) { Set a = getAvailable(); if (a.isEmpty()) { @@ -381,6 +384,7 @@ protected void updateMachineConfig(T machine, Map flags) { // For backwards compatibility, where peristed state did not have this. origConfigs = Maps.newLinkedHashMap(); } + parseMachineConfig((AbstractLocation) machine); Map strFlags = ConfigBag.newInstance(flags).getAllConfig(); Map origConfig = ((ConfigurationSupportInternal)machine.config()).getLocalBag().getAllConfig(); origConfigs.put(machine, origConfig); @@ -388,6 +392,76 @@ protected void updateMachineConfig(T machine, Map flags) { ((ConfigurationSupportInternal)machine.config()).putAll(strFlags); } + + private void parseMachineConfig(AbstractLocation machine) { + String ssh = machine.config().get(ConfigKeys.newStringConfigKey("ssh")); + machine.config().removeKey(ConfigKeys.newStringConfigKey("ssh")); + String winrm = machine.config().get(ConfigKeys.newStringConfigKey("winrm")); + machine.config().removeKey(ConfigKeys.newStringConfigKey("winrm")); + + Map tcpPortMappings = (Map) machine.config().get(ConfigKeys.newConfigKey(Map.class, "tcpPortMappings")); + + if (ssh == null && winrm == null) { + throw new IllegalArgumentException("Must specify exactly one of 'ssh' or 'winrm' for machine: "+machine); + } + + UserAndHostAndPort userAndHostAndPort; + String host; + int port; + if (ssh != null) { + userAndHostAndPort = parseUserAndHostAndPort(ssh, 22); + } else { + // TODO set to null and rely on the MachineLocation. If not then make a dependency to WinRmMachineLocation and use its config key name. + Boolean useHttps = machine.config().get(ConfigKeys.newConfigKey(Boolean.class,"winrm.useHttps")); + userAndHostAndPort = parseUserAndHostAndPort(winrm, useHttps != null && useHttps ? 5986 : 5985); + } + + // If there is a tcpPortMapping defined for the connection-port, then use that for ssh/winrm machine + port = userAndHostAndPort.getHostAndPort().getPort(); + if (tcpPortMappings != null && tcpPortMappings.containsKey(port)) { + String override = tcpPortMappings.get(port); + HostAndPort hostAndPortOverride = HostAndPort.fromString(override); + if (!hostAndPortOverride.hasPort()) { + throw new IllegalArgumentException("Invalid portMapping ('"+override+"') for port "+port+" in machine configuration "+machine.config()); + } + port = hostAndPortOverride.getPort(); + host = hostAndPortOverride.getHostText().trim(); + } else { + host = userAndHostAndPort.getHostAndPort().getHostText().trim(); + } + + machine.config().set(ConfigKeys.newStringConfigKey("address"), host); + try { + InetAddress.getByName(host); + } catch (Exception e) { + throw new IllegalArgumentException("Invalid host '"+host+"' specified in '"+machine+"': "+e); + } + + if (userAndHostAndPort.getUser() != null) { + machine.config().set(ConfigKeys.newStringConfigKey("user"), userAndHostAndPort.getUser()); + } + if (userAndHostAndPort.getHostAndPort().hasPort()) { + machine.config().set(ConfigKeys.newConfigKey(Integer.class,"port"), port); + } + } + + private UserAndHostAndPort parseUserAndHostAndPort(String val) { + String userPart = null; + String hostPart = val; + if (val.contains("@")) { + userPart = val.substring(0, val.indexOf("@")); + hostPart = val.substring(val.indexOf("@")+1); + } + return UserAndHostAndPort.fromParts(userPart, HostAndPort.fromString(hostPart)); + } + + private UserAndHostAndPort parseUserAndHostAndPort(String val, int defaultPort) { + UserAndHostAndPort result = parseUserAndHostAndPort(val); + if (!result.getHostAndPort().hasPort()) { + result = UserAndHostAndPort.fromParts(result.getUser(), result.getHostAndPort().getHostText(), defaultPort); + } + return result; + } protected void restoreMachineConfig(MachineLocation machine) { if (origConfigs == null) { From 946c0deb7c8174dfe220b00fbc07b6a342c2f40f Mon Sep 17 00:00:00 2001 From: Ivana Yovcheva Date: Fri, 4 Aug 2017 13:53:35 +0300 Subject: [PATCH 4/4] Byon dsl config - fix tests failures --- ...nRmMachineLocationExternalConfigYamlTest.java | 2 +- .../FixedListMachineProvisioningLocation.java | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java index 7257ef33c8..bf73dd83b7 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java @@ -120,7 +120,7 @@ public void testWindowsMachinesNoExternalIPProvider() throws Exception { waitForApplicationTasks(app); Asserts.shouldHaveFailedPreviously(); } catch (Exception e) { - Asserts.expectedFailureOfType(e, NullPointerException.class); + Asserts.expectedFailureContains(e, "Must specify exactly one of 'ssh' or 'winrm' for machine"); } } diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java index 89b0c3ba8d..648285a7d5 100644 --- a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java +++ b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java @@ -29,6 +29,7 @@ import java.util.Map; import java.util.Set; +import com.google.common.base.Preconditions; import com.google.common.net.HostAndPort; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.location.LocationSpec; @@ -384,7 +385,10 @@ protected void updateMachineConfig(T machine, Map flags) { // For backwards compatibility, where peristed state did not have this. origConfigs = Maps.newLinkedHashMap(); } - parseMachineConfig((AbstractLocation) machine); + if (((AbstractLocation) machine).config().getBag().getAllConfig().get("provider") != null && + ((AbstractLocation) machine).config().getBag().getAllConfig().get("provider").equals("byon")) { + parseMachineConfig((AbstractLocation) machine); + } Map strFlags = ConfigBag.newInstance(flags).getAllConfig(); Map origConfig = ((ConfigurationSupportInternal)machine.config()).getLocalBag().getAllConfig(); origConfigs.put(machine, origConfig); @@ -399,12 +403,14 @@ private void parseMachineConfig(AbstractLocation machine) { String winrm = machine.config().get(ConfigKeys.newStringConfigKey("winrm")); machine.config().removeKey(ConfigKeys.newStringConfigKey("winrm")); - Map tcpPortMappings = (Map) machine.config().get(ConfigKeys.newConfigKey(Map.class, "tcpPortMappings")); - - if (ssh == null && winrm == null) { - throw new IllegalArgumentException("Must specify exactly one of 'ssh' or 'winrm' for machine: "+machine); + if (ssh ==null && winrm == null && machine.config().get(ConfigKeys.newStringConfigKey("address")) != null) { + return; } + Preconditions.checkArgument(ssh != null || winrm != null, "Must specify exactly one of 'ssh' or 'winrm' for machine: " + machine); + + Map tcpPortMappings = (Map) machine.config().get(ConfigKeys.newConfigKey(Map.class, "tcpPortMappings")); + UserAndHostAndPort userAndHostAndPort; String host; int port;