You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by Graeme-Miller <gi...@git.apache.org> on 2016/11/25 16:25:45 UTC
[GitHub] brooklyn-server pull request #468: Added functionality to overwrite private ...
GitHub user Graeme-Miller opened a pull request:
https://github.com/apache/brooklyn-server/pull/468
Added functionality to overwrite private ip/hostname as public ip/hostname
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Graeme-Miller/brooklyn-server add_private_endpoint_overwrite
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/468.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #468
----
commit 2e8e98d5956927adaab887acf7bd9539a556b90b
Author: graeme.miller <gr...@cloudsoftcorp.com>
Date: 2016-11-24T14:13:00Z
Added functionality to overwrite pricate ip/hostname as public ip/hostname
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #468: Added functionality to overwrite private ip/host...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/468
@Graeme-Miller would be useful to include the explanation as a comment. Also examples of concrete clouds where to expect this behaviour if supported by open source jclouds providers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #468: Added functionality to overwrite private ip/host...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/468
hi @Graeme-Miller, just for info, what's the use-case here, why add this capability?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #468: Added functionality to overwrite private ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/468#discussion_r89749300
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java ---
@@ -293,6 +293,12 @@
public static final ConfigKey<Map<String,Object>> TEMPLATE_OPTIONS = ConfigKeys.newConfigKey(
new TypeToken<Map<String, Object>>() {}, "templateOptions", "Additional jclouds template options");
+ @Beta
+ public static final ConfigKey<Boolean> USE_PUBLIC_ENDPOINT_AS_PRIVATE_ENDPOINT = ConfigKeys.newBooleanConfigKey(
+ "usePublicEndpointAsPrivateEndpoint",
--- End diff --
@Graeme-Miller I agree with @neykov that we should have a comment here to explain why it exists, and an example of when one would use it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #468: Added functionality to overwrite private ip/host...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the issue:
https://github.com/apache/brooklyn-server/pull/468
makes sense thanks @Graeme-Miller
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #468: Added functionality to overwrite private ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/468#discussion_r89639719
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---
@@ -3356,6 +3356,12 @@ protected String getPrivateHostname(NodeMetadata node, Optional<HostAndPort> ssh
}
protected String getPrivateHostname(NodeMetadata node, Optional<HostAndPort> sshHostAndPort, Supplier<? extends LoginCredentials> userCredentials, ConfigBag setup) {
+ Boolean usePublicEndpointAsPrivateEndpoint = (setup != null) ? setup.get(USE_PUBLIC_ENDPOINT_AS_PRIVATE_ENDPOINT) : false;
+ if(usePublicEndpointAsPrivateEndpoint) {
+ LOG.debug("Overriding private hostname as public hostname because config "+USE_PUBLIC_ENDPOINT_AS_PRIVATE_ENDPOINT+" is set to true");
--- End diff --
Minor: prefer `USE_PUBLIC_ENDPOINT_AS_PRIVATE_ENDPOINT.getName()`, because the toString includes that it is a ConfigKey and its type.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #468: Added functionality to overwrite private ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/468#discussion_r89639989
--- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocationConfig.java ---
@@ -293,6 +293,12 @@
public static final ConfigKey<Map<String,Object>> TEMPLATE_OPTIONS = ConfigKeys.newConfigKey(
new TypeToken<Map<String, Object>>() {}, "templateOptions", "Additional jclouds template options");
+ @Beta
+ public static final ConfigKey<Boolean> USE_PUBLIC_ENDPOINT_AS_PRIVATE_ENDPOINT = ConfigKeys.newBooleanConfigKey(
+ "usePublicEndpointAsPrivateEndpoint",
--- End diff --
I wonder if there is a better name for this - it's not clear that it refers to VMs provisioned using the JcloudsLocation, and thus risks being confused with the cloud endpoint. Perhaps would be better as:
```
@Beta
public static final ConfigKey<Boolean> USE_MACHINE_PUBLIC_ADDRESS_AS_PRIVATE_ADDRESS = ConfigKeys.newBooleanConfigKey(
"useMachinePublicAddressAsPrivateAddress",
"When true we will use the public IP/Hostname of a provisioned VM as the private IP/Hostname",
false);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #468: Added functionality to overwrite private ip/host...
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on the issue:
https://github.com/apache/brooklyn-server/pull/468
@neykov the issue I saw was with Azure, which is not currently packaged as part of Brooklyn.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #468: Added functionality to overwrite private ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/468
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #468: Added functionality to overwrite private ip/host...
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on the issue:
https://github.com/apache/brooklyn-server/pull/468
@geomacy a number of blueprints (e.g. Clocker) rely on the ability of nodes to communicate via the subnet IP.
This functionality is useful in a situation where a cloud you are deploying to gives you a private IP, but you cannot configure that cloud to allow nodes to talk to one another via that IP. In this case, you could use this functionality to ensure that only the public IPs are used.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #468: Added functionality to overwrite private ip/host...
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:
https://github.com/apache/brooklyn-server/pull/468
I think my comment was sensible although not addressed. I suppose there are some other compelling use cases - that I'm ignoring now - where this configkey will be useful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #468: Added functionality to overwrite private ip/host...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/468
Thanks @Graeme-Miller - merging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #468: Added functionality to overwrite private ip/host...
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the issue:
https://github.com/apache/brooklyn-server/pull/468
@aledsage re "@andreaturli and @Graeme-Miller looked into this for a while, but didn't manage to get the VMs to be added to the same Azure subnet" that's not entirely true. Adding
```
templateOptions:
virtualNetworkName: your-net
subnetNames:
- your-subnet
```
one can place VM in the specified subnet. Notice also that when in the sam subnet VMs can reach one another using private ip address *without* fiddling with `Azure endpoint` aka fw rules.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #468: Added functionality to overwrite private ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/468#discussion_r89640337
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocationEndpointOverwriteTest.java ---
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds;
+
+import static org.testng.Assert.assertEquals;
+
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+
+import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.domain.NodeMetadata.Status;
+import org.jclouds.compute.domain.NodeMetadataBuilder;
+import org.jclouds.compute.domain.OsFamily;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.domain.LoginCredentials;
+
+import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry.AbstractNodeCreator;
+import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry.NodeCreator;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
+import org.apache.brooklyn.util.core.internal.ssh.SshTool;
+import org.apache.brooklyn.util.core.internal.winrm.WinRmTool;
+
+public class JcloudsSshMachineLocationEndpointOverwriteTest extends AbstractJcloudsStubbedUnitTest {
+
+ @SuppressWarnings("unused")
+ private static final Logger log = LoggerFactory.getLogger(JcloudsImageChoiceStubbedLiveTest.class);
+
+ private List<String> privateAddresses;
+ private List<String> publicAddresses;
+
+ @BeforeMethod(alwaysRun=true)
+ public void setUp() throws Exception {
+ super.setUp();
+ privateAddresses = ImmutableList.of("172.168.10.11");
+ publicAddresses = ImmutableList.of("173.194.32.123");
+ initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of(JcloudsLocationConfig.USE_PUBLIC_ENDPOINT_AS_PRIVATE_ENDPOINT.getName(), true));
+ }
+
+ protected NodeCreator newNodeCreator() {
+ return new AbstractNodeCreator() {
+ @Override protected NodeMetadata newNode(String group, Template template) {
+ NodeMetadata result = new NodeMetadataBuilder()
+ .id("myid")
+ .credentials(LoginCredentials.builder().identity("myuser").credential("mypassword").build())
+ .loginPort(22)
+ .status(Status.RUNNING)
+ .publicAddresses(publicAddresses)
+ .privateAddresses(privateAddresses)
+ .build();
+ return result;
+ }
+ };
+ }
+
+
+ @Test
+ public void testSetPrivateIpToPublicIp() throws Exception {
+ JcloudsSshMachineLocation machine = obtainMachine(ImmutableMap.of());
+
+ assertEquals(machine.getPublicAddresses().size(), 1);
+ String publicAddress = machine.getPublicAddresses().iterator().next();
+
+ assertEquals(machine.getPrivateAddress().get(), publicAddress);
--- End diff --
Why is this not asserting that the value(s) are equal to the field `publicAddresses`? That would feel like a stronger assertion - checking that the address was the same as the public address we injected into the `NodeMetadata`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #468: Added functionality to overwrite private ...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/468#discussion_r89640601
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocationEndpointOverwriteTest.java ---
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.jclouds;
+
+import static org.testng.Assert.assertEquals;
+
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+
+import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.domain.NodeMetadata.Status;
+import org.jclouds.compute.domain.NodeMetadataBuilder;
+import org.jclouds.compute.domain.OsFamily;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.domain.LoginCredentials;
+
+import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry.AbstractNodeCreator;
+import org.apache.brooklyn.location.jclouds.StubbedComputeServiceRegistry.NodeCreator;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
+import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
+import org.apache.brooklyn.util.core.internal.ssh.SshTool;
+import org.apache.brooklyn.util.core.internal.winrm.WinRmTool;
+
+public class JcloudsSshMachineLocationEndpointOverwriteTest extends AbstractJcloudsStubbedUnitTest {
+
+ @SuppressWarnings("unused")
+ private static final Logger log = LoggerFactory.getLogger(JcloudsImageChoiceStubbedLiveTest.class);
+
+ private List<String> privateAddresses;
+ private List<String> publicAddresses;
+
+ @BeforeMethod(alwaysRun=true)
+ public void setUp() throws Exception {
+ super.setUp();
+ privateAddresses = ImmutableList.of("172.168.10.11");
+ publicAddresses = ImmutableList.of("173.194.32.123");
+ initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of(JcloudsLocationConfig.USE_PUBLIC_ENDPOINT_AS_PRIVATE_ENDPOINT.getName(), true));
+ }
+
+ protected NodeCreator newNodeCreator() {
+ return new AbstractNodeCreator() {
+ @Override protected NodeMetadata newNode(String group, Template template) {
+ NodeMetadata result = new NodeMetadataBuilder()
+ .id("myid")
+ .credentials(LoginCredentials.builder().identity("myuser").credential("mypassword").build())
+ .loginPort(22)
+ .status(Status.RUNNING)
+ .publicAddresses(publicAddresses)
+ .privateAddresses(privateAddresses)
+ .build();
+ return result;
+ }
+ };
+ }
+
+
+ @Test
+ public void testSetPrivateIpToPublicIp() throws Exception {
+ JcloudsSshMachineLocation machine = obtainMachine(ImmutableMap.of());
+
+ assertEquals(machine.getPublicAddresses().size(), 1);
+ String publicAddress = machine.getPublicAddresses().iterator().next();
+
+ assertEquals(machine.getPrivateAddress().get(), publicAddress);
--- End diff --
Can we also check that `Machines.getSubnetHostname(machine)` and `Machines.getSubnetIp(machine)` returns the public address? I'd also check `machine.getPrivateAddresses()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---