You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2015/03/17 11:39:22 UTC
[GitHub] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
GitHub user ahgittin opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/554
fixes for BrooklynNode and java appservers to run in clocker, using subnet hostname
WIP - testing in progress, submitted for early review
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/incubator-brooklyn clocker-fixes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/554.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 #554
----
commit e78dee4b20466f1bdf4569613ca59b1f3518d706
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2015-03-17T10:37:21Z
fixes for BrooklynNode and java appservers to run in clocker, using subnet 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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26685357
--- Diff: core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java ---
@@ -127,13 +143,16 @@ public static BrooklynMemento newBrooklynMemento(ManagementContext managementCon
/**
* Inspects an entity to create a corresponding memento.
+ * <p>
+ * @deprecated since 0.7.0, see {@link #newBasicMemento(BrooklynObject)}
*/
+ @Deprecated
--- End diff --
Do we really need to keep all of these? The code is so specific that I am pretty sure no one know it exists. Also removing it won't mess with serialized state. I'd say delete it right now.
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/554
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26657534
--- Diff: core/src/test/java/brooklyn/location/access/PortForwardManagerRebindTest.java ---
@@ -86,6 +96,41 @@ public void testAssociationPreservedOnRebind() throws Exception {
}
@Test
+ public void testAssociationPreservedOnStateExport() throws Exception {
+ String publicIpId = "5.6.7.8";
+ String publicAddress = "5.6.7.8";
+
+ TestEntity origEntity = origApp.createAndManageChild(EntitySpec.create(TestEntity.class).impl(MyEntity.class));
+ PortForwardManager origPortForwardManager = origEntity.getConfig(MyEntity.PORT_FORWARD_MANAGER);
+
+ origPortForwardManager.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40080), origSimulatedMachine, 80);
+
+ String label = origManagementContext.getManagementNodeId()+"-"+Time.makeDateSimpleStampString();
+ PersistenceObjectStore targetStore = BrooklynPersistenceUtils.newPersistenceObjectStore(origManagementContext, null,
+ "tmp/web-persistence-"+label+"-"+Identifiers.makeRandomId(4));
+ File dir = ((FileBasedObjectStore)targetStore).getBaseDir();
+ // only register the parent dir because that will prevent leaks for the random ID
+ Os.deleteOnExitEmptyParentsUpTo(dir.getParentFile(), dir.getParentFile());
--- End diff --
Personal preference for tearDown or try-finally, rather than "deleteOnExit". Stuff gets left behind on dev machines when tests are terminated prematurely. But no strong feelings.
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26713113
--- Diff: software/webapp/src/main/java/brooklyn/entity/webapp/JavaWebAppSshDriver.java ---
@@ -89,11 +89,11 @@ protected String inferRootUrl() {
if (isProtocolEnabled("https")) {
Integer port = getHttpsPort();
checkNotNull(port, "HTTPS_PORT sensors not set; is an acceptable port available?");
- return String.format("https://%s:%s/", getHostname(), port);
+ return String.format("https://%s:%s/", getSubnetHostname(), port);
--- End diff --
Discovered a problem in EC2 with the naming strategy introduced in 89c126340dcf350a3d887782a5ea79a672fe2f17 -- have fixed it with an additional commit here, and detailed comments.
Now confirmed that subnet hostname in AWS does the right thing.
Merging with the add'l commit but @aledsage please look at b68363e8636a3e0c3cea28a2d39aa448e8d50650 in case you see a better way. (Bear in mind jclouds wants a `Function<...Image...>` and `Image` doesn't make the cloud provider available!)
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26657989
--- Diff: software/webapp/src/main/java/brooklyn/entity/webapp/JavaWebAppSshDriver.java ---
@@ -89,11 +89,11 @@ protected String inferRootUrl() {
if (isProtocolEnabled("https")) {
Integer port = getHttpsPort();
checkNotNull(port, "HTTPS_PORT sensors not set; is an acceptable port available?");
- return String.format("https://%s:%s/", getHostname(), port);
+ return String.format("https://%s:%s/", getSubnetHostname(), port);
--- End diff --
Am I right that `getSubnetHostname()` will return the same as `getHostname()` if not running in Clocker or a `SubnetTier`?
What about for AWS - will the subnetHostname still be the public hostname, rather than the private IP of the VM?
Assuming so, this looks sensible.
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26657763
--- Diff: core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java ---
@@ -68,10 +71,23 @@
private MementosGenerators() {}
+ /** @deprecated since 0.7.0 use {@link #newBasicMemento(BrooklynObject)} */
--- End diff --
This isn't the real memento as per new java doc. The old name confused me
and caused the manual export failures.
On 18 Mar 2015 11:59, "Aled Sage" <no...@github.com> wrote:
> In core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java
> <https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26656722>
> :
>
> > @@ -68,10 +71,23 @@
> >
> > private MementosGenerators() {}
> >
> > + /** @deprecated since 0.7.0 use {@link #newBasicMemento(BrooklynObject)} */
>
> Why rename from newMemento(BrooklynObject) to
> newBasicMemento(BrooklynObject)? Do we have anything but "basic", or are
> we expecting that in the near future? Personally I prefer newMemento, as
> "basic" implies there is some other kind of memento out there.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-brooklyn/pull/554/files#r26656722>.
>
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26657705
--- Diff: core/src/test/java/brooklyn/location/access/PortForwardManagerRebindTest.java ---
@@ -86,6 +96,41 @@ public void testAssociationPreservedOnRebind() throws Exception {
}
@Test
+ public void testAssociationPreservedOnStateExport() throws Exception {
+ String publicIpId = "5.6.7.8";
+ String publicAddress = "5.6.7.8";
+
+ TestEntity origEntity = origApp.createAndManageChild(EntitySpec.create(TestEntity.class).impl(MyEntity.class));
+ PortForwardManager origPortForwardManager = origEntity.getConfig(MyEntity.PORT_FORWARD_MANAGER);
+
+ origPortForwardManager.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40080), origSimulatedMachine, 80);
+
+ String label = origManagementContext.getManagementNodeId()+"-"+Time.makeDateSimpleStampString();
+ PersistenceObjectStore targetStore = BrooklynPersistenceUtils.newPersistenceObjectStore(origManagementContext, null,
+ "tmp/web-persistence-"+label+"-"+Identifiers.makeRandomId(4));
+ File dir = ((FileBasedObjectStore)targetStore).getBaseDir();
+ // only register the parent dir because that will prevent leaks for the random ID
+ Os.deleteOnExitEmptyParentsUpTo(dir.getParentFile(), dir.getParentFile());
+ BrooklynPersistenceUtils.writeMemento(origManagementContext, targetStore, MementoCopyMode.LOCAL);
+
+ RebindTestUtils.waitForPersisted(origApp);
+ log.info("Using manual export dir "+dir+" for rebind instead of "+mementoDir);
--- End diff --
I'm testing rest-like manual export distinct from normal persist-and-rebind.
On 18 Mar 2015 12:17, "Aled Sage" <no...@github.com> wrote:
> In
> core/src/test/java/brooklyn/location/access/PortForwardManagerRebindTest.java
> <https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26657630>
> :
>
> > +
> > + TestEntity origEntity = origApp.createAndManageChild(EntitySpec.create(TestEntity.class).impl(MyEntity.class));
> > + PortForwardManager origPortForwardManager = origEntity.getConfig(MyEntity.PORT_FORWARD_MANAGER);
> > +
> > + origPortForwardManager.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40080), origSimulatedMachine, 80);
> > +
> > + String label = origManagementContext.getManagementNodeId()+"-"+Time.makeDateSimpleStampString();
> > + PersistenceObjectStore targetStore = BrooklynPersistenceUtils.newPersistenceObjectStore(origManagementContext, null,
> > + "tmp/web-persistence-"+label+"-"+Identifiers.makeRandomId(4));
> > + File dir = ((FileBasedObjectStore)targetStore).getBaseDir();
> > + // only register the parent dir because that will prevent leaks for the random ID
> > + Os.deleteOnExitEmptyParentsUpTo(dir.getParentFile(), dir.getParentFile());
> > + BrooklynPersistenceUtils.writeMemento(origManagementContext, targetStore, MementoCopyMode.LOCAL);
> > +
> > + RebindTestUtils.waitForPersisted(origApp);
> > + log.info("Using manual export dir "+dir+" for rebind instead of "+mementoDir);
>
> Why use the manual export dir? If we need to do that here, why do we not
> need to do it in all our other tests?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-brooklyn/pull/554/files#r26657630>.
>
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#issuecomment-82947132
@ahgittin looks good - only a couple of minor comments/questions.
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26687088
--- Diff: core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java ---
@@ -127,13 +143,16 @@ public static BrooklynMemento newBrooklynMemento(ManagementContext managementCon
/**
* Inspects an entity to create a corresponding memento.
+ * <p>
+ * @deprecated since 0.7.0, see {@link #newBasicMemento(BrooklynObject)}
*/
+ @Deprecated
--- End diff --
I've been burned by that assumption before. I'm almost certain you're right but I think not worth it now. We will have fun as soon as 070 is out getting rid of all these!!!
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26657630
--- Diff: core/src/test/java/brooklyn/location/access/PortForwardManagerRebindTest.java ---
@@ -86,6 +96,41 @@ public void testAssociationPreservedOnRebind() throws Exception {
}
@Test
+ public void testAssociationPreservedOnStateExport() throws Exception {
+ String publicIpId = "5.6.7.8";
+ String publicAddress = "5.6.7.8";
+
+ TestEntity origEntity = origApp.createAndManageChild(EntitySpec.create(TestEntity.class).impl(MyEntity.class));
+ PortForwardManager origPortForwardManager = origEntity.getConfig(MyEntity.PORT_FORWARD_MANAGER);
+
+ origPortForwardManager.associate(publicIpId, HostAndPort.fromParts(publicAddress, 40080), origSimulatedMachine, 80);
+
+ String label = origManagementContext.getManagementNodeId()+"-"+Time.makeDateSimpleStampString();
+ PersistenceObjectStore targetStore = BrooklynPersistenceUtils.newPersistenceObjectStore(origManagementContext, null,
+ "tmp/web-persistence-"+label+"-"+Identifiers.makeRandomId(4));
+ File dir = ((FileBasedObjectStore)targetStore).getBaseDir();
+ // only register the parent dir because that will prevent leaks for the random ID
+ Os.deleteOnExitEmptyParentsUpTo(dir.getParentFile(), dir.getParentFile());
+ BrooklynPersistenceUtils.writeMemento(origManagementContext, targetStore, MementoCopyMode.LOCAL);
+
+ RebindTestUtils.waitForPersisted(origApp);
+ log.info("Using manual export dir "+dir+" for rebind instead of "+mementoDir);
--- End diff --
Why use the manual export dir? If we need to do that here, why do we not need to do it in all our other tests?
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26687377
--- Diff: software/webapp/src/main/java/brooklyn/entity/webapp/JavaWebAppSshDriver.java ---
@@ -89,11 +89,11 @@ protected String inferRootUrl() {
if (isProtocolEnabled("https")) {
Integer port = getHttpsPort();
checkNotNull(port, "HTTPS_PORT sensors not set; is an acceptable port available?");
- return String.format("https://%s:%s/", getHostname(), port);
+ return String.format("https://%s:%s/", getSubnetHostname(), port);
--- End diff --
Testing EC2 now to be sure, but from the code I think so.
---
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] incubator-brooklyn pull request: fixes for BrooklynNode and java a...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/554#discussion_r26656722
--- Diff: core/src/main/java/brooklyn/entity/rebind/dto/MementosGenerators.java ---
@@ -68,10 +71,23 @@
private MementosGenerators() {}
+ /** @deprecated since 0.7.0 use {@link #newBasicMemento(BrooklynObject)} */
--- End diff --
Why rename from `newMemento(BrooklynObject)` to `newBasicMemento(BrooklynObject)`? Do we have anything but "basic", or are we expecting that in the near future? Personally I prefer `newMemento`, as "basic" implies there is some other kind of memento out there.
---
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.
---