You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by bostko <gi...@git.apache.org> on 2015/06/26 16:52:39 UTC
[GitHub] incubator-brooklyn pull request: Fix NodeJsWebAppFixtureIntegratio...
GitHub user bostko opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/722
Fix NodeJsWebAppFixtureIntegrationTest
NFY
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bostko/incubator-brooklyn test_fixes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/722.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 #722
----
commit 60d27c22a6175089e346113ce5745719f5e7ab43
Author: Valentin Aitken <va...@cloudsoftcorp.com>
Date: 2015-06-26T10:13:21Z
Fix java version check
commit 8360f3f6c96f8818c596809400a39492f04144e1
Author: Valentin Aitken <va...@cloudsoftcorp.com>
Date: 2015-06-26T12:52:00Z
yum fix Add EPEL if missing
This fixes NodeJsWebAppFixtureIntegrationTest
commit b08a6107043bc3274c93e482686e40f94826a1fb
Author: Valentin Aitken <va...@cloudsoftcorp.com>
Date: 2015-06-26T14:10:43Z
Fix NodeJS Integration test to stop the service with the generic stop
----
---
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: Fix NodeJsWebAppFixtureIntegratio...
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/722#discussion_r33450175
--- Diff: software/webapp/src/test/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppFixtureIntegrationTest.java ---
@@ -153,36 +140,16 @@ public void testReportsServiceDownWhenKilled() throws Exception {
LOG.info("success getting service up false in primary mgmt universe");
}
-
+
/**
* Stop the given underlying entity, but without our entity instance being told!
*/
protected void killEntityBehindBack(Entity tokill) throws Exception {
- // Previously was calling entity.getDriver().kill(); but now our entity instance is a proxy so can't do that
- ManagementContext newManagementContext = null;
- File tempDir = Os.newTempDir(getClass());
- try {
- ManagementContext managementContext = ((EntityInternal)tokill).getManagementContext();
- BrooklynMemento brooklynMemento = MementosGenerators.newBrooklynMemento(managementContext);
-
- BrooklynMementoPersisterToMultiFile oldPersister = new BrooklynMementoPersisterToMultiFile(tempDir , getClass().getClassLoader());
- oldPersister.checkpoint(brooklynMemento, PersistenceExceptionHandlerImpl.builder().build());
- oldPersister.waitForWritesCompleted(30*1000, TimeUnit.MILLISECONDS);
-
- BrooklynMementoPersisterToMultiFile newPersister = new BrooklynMementoPersisterToMultiFile(tempDir , getClass().getClassLoader());
- newManagementContext = new LocalManagementContextForTests();
- newManagementContext.getRebindManager().setPersister(newPersister, PersistenceExceptionHandlerImpl.builder().build());
- newManagementContext.getRebindManager().rebind(getClass().getClassLoader(), null, ManagementNodeState.MASTER);
- newManagementContext.getRebindManager().startPersistence();
- SoftwareProcess entity2 = (SoftwareProcess) newManagementContext.getEntityManager().getEntity(tokill.getId());
- entity2.stop();
- } finally {
- if (newManagementContext != null) ((ManagementContextInternal)newManagementContext).terminate();
- Os.deleteRecursively(tempDir.getAbsolutePath());
- }
- LOG.info("called to stop {} in parallel mgmt universe", entity);
+ ((SoftwareProcessDriver)((DriverDependentEntity<?>) Entities.deproxy(tokill)).getDriver()).stop();
--- End diff --
nice! much cleaner.
---
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: Fix NodeJsWebAppFixtureIntegratio...
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/722#discussion_r33456625
--- Diff: software/base/src/main/java/brooklyn/entity/java/JavaSoftwareProcessSshDriver.java ---
@@ -336,7 +336,7 @@ protected int tryJavaInstall(String version, String command) {
log.debug("Checking Java version at {}@{}", getEntity(), getLocation());
// sed gets stdin like 'java version "1.7.0_45"'
ProcessTaskWrapper<Integer> versionCommand = Entities.submit(getEntity(), SshTasks.newSshExecTaskFactory(
- getLocation(), "java -version 2>&1 | grep \"java version\" | sed 's/.*\"\\(.*\\).*\"/\\1/'"));
+ getLocation(), "java -version 2>&1 | grep \" version\" | sed 's/.*\"\\(.*\\).*\"/\\1/'"));
--- End diff --
not sure how universal `grep -E` is so i'd be against that. can you add a mention to `openjdk version "1.8.0_45"` in the comment? things get missed in the git history, especially if not on the first line (although they shouldn't -- apologies from me for not noticing 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] incubator-brooklyn pull request: Fix NodeJsWebAppFixtureIntegratio...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/722#issuecomment-124007960
@bostko Could you close this.
---
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: Fix NodeJsWebAppFixtureIntegratio...
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/722#discussion_r33449984
--- Diff: software/webapp/src/test/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppFixtureIntegrationTest.java ---
@@ -143,7 +130,7 @@ public void testReportsServiceDownWhenKilled() throws Exception {
LOG.info("test=testReportsServiceDownWithKilled; entity="+entity+"; app="+entity.getApplication());
Entities.start(entity.getApplication(), ImmutableList.of(loc));
- EntityTestUtils.assertAttributeEqualsEventually(MutableMap.of("timeout", 120*1000), entity, Startable.SERVICE_UP, true);
+ EntityTestUtils.assertAttributeEqualsEventually(MutableMap.of("timeout", 120 * 1000), entity, Startable.SERVICE_UP, true);
--- End diff --
can use `Duration.minutes(2)`
---
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: Fix NodeJsWebAppFixtureIntegratio...
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/722#discussion_r34238600
--- Diff: software/webapp/src/test/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppFixtureIntegrationTest.java ---
@@ -191,7 +159,7 @@ public void testInitialNamedDeployments() {
Entities.start(entity.getApplication(), ImmutableList.of(loc));
- Asserts.succeedsEventually(MutableMap.of("timeout", 60*1000), new Runnable() {
+ Asserts.succeedsEventually(MutableMap.of("timeout", Duration.minutes(2)), new Runnable() {
--- End diff --
Is the timeout changed on purpose to two minutes?
---
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: Fix NodeJsWebAppFixtureIntegratio...
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/722#discussion_r33450066
--- Diff: software/base/src/main/java/brooklyn/entity/java/JavaSoftwareProcessSshDriver.java ---
@@ -336,7 +336,7 @@ protected int tryJavaInstall(String version, String command) {
log.debug("Checking Java version at {}@{}", getEntity(), getLocation());
// sed gets stdin like 'java version "1.7.0_45"'
ProcessTaskWrapper<Integer> versionCommand = Entities.submit(getEntity(), SshTasks.newSshExecTaskFactory(
- getLocation(), "java -version 2>&1 | grep \"java version\" | sed 's/.*\"\\(.*\\).*\"/\\1/'"));
+ getLocation(), "java -version 2>&1 | grep \" version\" | sed 's/.*\"\\(.*\\).*\"/\\1/'"));
--- End diff --
worth a comment on the reason for this change -- presumably some java gives different output?
---
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: Fix NodeJsWebAppFixtureIntegratio...
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/722#discussion_r33456465
--- Diff: software/base/src/main/java/brooklyn/entity/java/JavaSoftwareProcessSshDriver.java ---
@@ -336,7 +336,7 @@ protected int tryJavaInstall(String version, String command) {
log.debug("Checking Java version at {}@{}", getEntity(), getLocation());
// sed gets stdin like 'java version "1.7.0_45"'
ProcessTaskWrapper<Integer> versionCommand = Entities.submit(getEntity(), SshTasks.newSshExecTaskFactory(
- getLocation(), "java -version 2>&1 | grep \"java version\" | sed 's/.*\"\\(.*\\).*\"/\\1/'"));
+ getLocation(), "java -version 2>&1 | grep \" version\" | sed 's/.*\"\\(.*\\).*\"/\\1/'"));
--- End diff --
I've included a comment in the commit.
Here is what `java -version` returns on my side
```
openjdk version "1.8.0_45"
OpenJDK Runtime Environment (build 1.8.0_45-b14)
OpenJDK 64-Bit Server VM (build 25.45-b02, mixed mode)
```
However we could consider using `grep -E "(openjdk|java) version"`
---
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: Fix NodeJsWebAppFixtureIntegratio...
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/722#discussion_r33450455
--- Diff: utils/common/src/main/java/brooklyn/util/ssh/BashCommands.java ---
@@ -376,7 +376,8 @@ public static String installPackageOr(Map<?,?> flags, String packageDefaultName,
chainGroup(
"echo yum exists, doing update",
ok(sudo("yum check-update")),
- sudo(yumInstall))));
+ ok(sudo("yum -y install epel-release")),
+ sudo(yumInstall))));
--- End diff --
bad indentation
---
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: Fix NodeJsWebAppFixtureIntegratio...
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/722#issuecomment-116629151
Regarding java version check change.
`java -version` returns different output if openjdk is used
Example:
```
openjdk version "1.8.0_45"
OpenJDK Runtime Environment (build 1.8.0_45-b14)
OpenJDK 64-Bit Server VM (build 25.45-b02, mixed mode)
```
---
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: Fix NodeJsWebAppFixtureIntegratio...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/722#issuecomment-119898083
good to merge
---
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: Fix NodeJsWebAppFixtureIntegratio...
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/722#discussion_r33455551
--- Diff: software/webapp/src/test/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppFixtureIntegrationTest.java ---
@@ -153,36 +140,16 @@ public void testReportsServiceDownWhenKilled() throws Exception {
LOG.info("success getting service up false in primary mgmt universe");
}
-
+
/**
* Stop the given underlying entity, but without our entity instance being told!
*/
protected void killEntityBehindBack(Entity tokill) throws Exception {
- // Previously was calling entity.getDriver().kill(); but now our entity instance is a proxy so can't do that
- ManagementContext newManagementContext = null;
- File tempDir = Os.newTempDir(getClass());
- try {
- ManagementContext managementContext = ((EntityInternal)tokill).getManagementContext();
- BrooklynMemento brooklynMemento = MementosGenerators.newBrooklynMemento(managementContext);
-
- BrooklynMementoPersisterToMultiFile oldPersister = new BrooklynMementoPersisterToMultiFile(tempDir , getClass().getClassLoader());
- oldPersister.checkpoint(brooklynMemento, PersistenceExceptionHandlerImpl.builder().build());
- oldPersister.waitForWritesCompleted(30*1000, TimeUnit.MILLISECONDS);
-
- BrooklynMementoPersisterToMultiFile newPersister = new BrooklynMementoPersisterToMultiFile(tempDir , getClass().getClassLoader());
- newManagementContext = new LocalManagementContextForTests();
- newManagementContext.getRebindManager().setPersister(newPersister, PersistenceExceptionHandlerImpl.builder().build());
- newManagementContext.getRebindManager().rebind(getClass().getClassLoader(), null, ManagementNodeState.MASTER);
- newManagementContext.getRebindManager().startPersistence();
- SoftwareProcess entity2 = (SoftwareProcess) newManagementContext.getEntityManager().getEntity(tokill.getId());
- entity2.stop();
- } finally {
- if (newManagementContext != null) ((ManagementContextInternal)newManagementContext).terminate();
- Os.deleteRecursively(tempDir.getAbsolutePath());
- }
- LOG.info("called to stop {} in parallel mgmt universe", entity);
+ ((SoftwareProcessDriver)((DriverDependentEntity<?>) Entities.deproxy(tokill)).getDriver()).stop();
--- End diff --
It was thanks to Svet
---
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: Fix NodeJsWebAppFixtureIntegratio...
Posted by bostko <gi...@git.apache.org>.
Github user bostko closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/722
---
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: Fix NodeJsWebAppFixtureIntegratio...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/722#issuecomment-116593079
good fixes. a few very minor comments, then good to merge.
---
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.
---