Added basic migration capability to org.apache.brooklyn.entity.webapp.* #924
Conversation
|
incubator-brooklyn-pull-requests #1868 FAILURE |
|
incubator-brooklyn-pull-requests #1867 SUCCESS |
adriannieto
force-pushed
the
feature/basic-migration
branch
from
88ea61f
to
1197888
Compare
|
incubator-brooklyn-pull-requests #1869 SUCCESS |
| @Override | ||
| @AfterMethod(alwaysRun = true) | ||
| public void tearDown() throws Exception { | ||
| app.stop(); |
There was a problem hiding this comment.
The call to Entities.destroyAll in the superclass method will take care of this for you.
There was a problem hiding this comment.
Thank you, I will remove it then.
|
@adriannieto thanks for your contribution. I've made a few comments on and asked a few questions about the implementation as it is. It's interesting to see that nothing in the implementation of You can refer to vanilla-bash-netcat-w-client.yaml for an example of how initialisers are used from Yaml. |
|
@sjcorbett thank you very much for the review, I was reading further about how to initialise the effectors and I will modify everything according to your suggestions. Example YAML: |
|
@sjcorbett Hello, Jenkins is complaining because it can't merge automatically, but this branch builds on my machine. Should I rebase to the master before continuing the review? I wasn't sure that a push force to my branch were a good idea. |
|
Yes please, rebase on master. Force pushing your branch is absolutely fine. |
adriannieto
force-pushed
the
feature/basic-migration
branch
from
f08538e
to
d6a2265
Compare
|
@adriannieto My apologies for leaving this for an entire month. I've had a quick scan of the updated changes and things look good. I have a few more comments to make that I'll leave shortly. Your branch conflicts with master but it only involves a couple of imports in |
|
@sjcorbett thank you, I will rebase it ASAP. Just one question, are the package locations fine? For example I added AllowsMigration definition to base package, do you want any modification on this? |
| public Object[][] webAppEntities() { | ||
| Object[][] dataProviderResult; | ||
|
|
||
| Reflections reflections = new Reflections("org.apache.brooklyn.entity.webapp"); |
There was a problem hiding this comment.
This and getEntitiesFromClasses is quite cute but it would be much (much!) clearer to just return a reference to the classes explicitly. i.e. new Object[][]{TomcatServer.class}, {Jboss7Server.class}, ...}
There was a problem hiding this comment.
My mistake, should return an array of the specs. But I think you get my point.
There was a problem hiding this comment.
Ok, I was considering this just to be "future-proof" just in case you add a new webapp entity. But as you're moving to YAML based entities maybe this test will not be necessary anymore.
There was a problem hiding this comment.
Yeah, I appreciate the sentiment. In truth things change rarely enough that it is fine to list items.
This effector is pluggable via brooklyn.initializers,
Example YAML:
```
name: My Migrable WebApp
location: aws-ireland
services:
- type: org.apache.brooklyn.entity.webapp.tomcat.TomcatServer
name: AppServer HelloWorld
brooklyn.config:
wars.root: http://search.maven.org/remotecontent?filepath=io/brooklyn/example/brooklyn-example-hello-world-sql-webapp/0.6.0/brooklyn-example-hello-world-sql-webapp-0.6.0.war
brooklyn.initializers:
- type: org.apache.brooklyn.entity.webapp.JavaWebAppMigrateEffector
```
adriannieto
force-pushed
the
feature/basic-migration
branch
from
d6a2265
to
4594914
Compare
| // The entity is sensors should rewired automatically on stop() + restart() | ||
| } else { | ||
| // Restart the entity to fetch again all the dependencies (ie. attributeWhenReady ones) | ||
| ((Startable) applicationChild).restart(); |
There was a problem hiding this comment.
Note to self/other reviewers since GitHub has decoupled conversation from code - see #924 (comment) for discussion on this loop before merging.
|
Migration in Brooklyn has been a great mystery to me, and this PR is the only relevant one I saw mentioned in the mailing list when I went searching. I'm excited to see it as it answers some questions for me, but I'm having trouble following what happened/should have happened with PRs like this as of Brooklyn's graduation from incubation... Did this PR and discussion continue elsewhere, or did it just happen to land at a natural stopping point just prior to the graduation? |
|
Hi @mogul Glad to hear that you are excited about Apache Brooklyn! |
|
Yes, no problem but I'm not sure about if I will be able to work on this However you could use it as an starting point.
|
|
@mogul This PR reached a natural stopping point. I don't think it would take much more work to get it merged. We'd be happy to answer any questions you have on the mailing list. |
This PR adds an Effector migrate(locationSpec) to Entities that override JavaWebAppSoftwareProcess. This Effector moves the Entity to a new location. Currently it doesn't support nested entities to be migrated.
This is a work in progress feature, my plan is to extend this support to non-clustered databases in the future.