search

brooklyncentral / brooklyn

This project has moved and is now part of the ASF
https://github.com/apache/incubator-brooklyn
72 stars 27 forks source link

add support for persisting to object store #1452

Closed ahgittin closed 10 years ago

ahgittin commented 10 years ago

READY FOR REVIEW - now includes HA, CLI.

basis is to refactor file-based support to use a common PersistenceObjectStore interface backed by either filesystem or jclouds blobstore (running w softlayer only so far, but using jclouds).

to run integration tests against SL, set up a named location such as the following:

brooklyn.location.named.softlayer-objectstore-amsterdam-1=jclouds:swift:https://ams01.objectstorage.softlayer.net/auth/v1.0
brooklyn.location.named.softlayer-objectstore-amsterdam-1.identity=IBMOS1234:yourname
brooklyn.location.named.softlayer-objectstore-amsterdam-1.credential=....
buildhive commented 10 years ago

Brooklyn Central » brooklyn #2424 FAILURE Looks like there's a problem with this pull request (what's this?)

ahgittin commented 10 years ago

@andreaturli great job on the initial work, i've completed it, ready for review by you and @aledsage . note i've put comments in the conversation so when you're reviewing online the biggest structural changes will make sense.

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2426 FAILURE Looks like there's a problem with this pull request (what's this?)

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2428 FAILURE Looks like there's a problem with this pull request (what's this?)

ahgittin commented 10 years ago

unrelated failure, closing and reopening to force rebuild

need to fix this:

2014-06-05 16:20:32,204 INFO  TESTNG FAILED: "Command line test" - brooklyn.entity.rebind.RebindCatalogEntityTest.testRestoresAppFromCatalogClassloader() finished in 78 ms
java.lang.UnsupportedOperationException: Catalogs cannot be added until the base catalog is loaded
    at brooklyn.catalog.internal.BasicBrooklynCatalog.loadManualAdditionsCatalog(BasicBrooklynCatalog.java:153)
buildhive commented 10 years ago

Brooklyn Central » brooklyn #2429 FAILURE Looks like there's a problem with this pull request (what's this?)

andreaturli commented 10 years ago

Sorry my bad, I hadn't cleaned up the brooklyn.entity.rebind.persister.jclouds.BlobStoreTest as it references a named location which is available only on my brooklyn.properties

ahgittin commented 10 years ago

good comments @aledsage, will go through them tomorrow.

the build failure this time seems legit; I'm setting @Test(groups="Integration") on the class, but alwaysRun=true on the setup/teardown seem to mean those methods run.

can someone remind me why we use alwaysRun=true? pretty sure it's the culprint here (e.g. in BlobStoreTest).

i'll pick up the comments tomorrow. some of them are on your code @andreaturli so i may need to ping you if there's anything else non-obvious! that said, once i understood how you structured it and how jclouds/blobstores work with listing directories, it was pretty straightforward to do this.

ahgittin commented 10 years ago

aha, we should use @BeforeMethod, or @BeforeClass if you want it to run once, not @BeforeTest which has weird semantics. my mistake to introduce @BeforeTest here but glad it is solved, it was driving me bonkers. http://testng.org/doc/documentation-main.html

ahgittin commented 10 years ago

actually i couldn't make any combo of @BeforeMethod and @BeforeGroups do what we want, which seems obvious, run the setUp for the test method only if the test method isn't being skipped, where the subclass defines the group. have just repeated all the test methods. :(

ahgittin commented 10 years ago

last commit does a lot of good things i think, addressing all the excellent review comments @aledsage with some big changes, including some high level. the first below probably being the most important. does this sound sensible?

in a dev environment you should now see

2014-06-06 13:41:02,704 INFO  Persistence mode REBIND, directory /Users/alex/.brooklyn/brooklyn-persisted-state/data backed up to /Users/alex/.brooklyn/brooklyn-persisted-state/data-2014-06-06-0141-02.bak
2014-06-06 13:41:06,068 INFO  Rebind complete: 0 apps, 0 entities, 0 locations, 0 policies, 0 enrichers
2014-06-06 13:41:06,084 INFO  Management node started as HA MASTER autodetected
2014-06-06 13:41:06,097 INFO  Starting brooklyn web-console on loopback interface because no security config is set
2014-06-06 13:41:06,187 INFO  Brooklyn running in development mode (autodetected)
2014-06-06 13:41:06,188 INFO  Using classpath://brooklyn.war from revised location /Users/alex/Data/cloudsoft/dev/gits/brooklyn/usage/jsgui/target/brooklyn-jsgui-0.7.0-SNAPSHOT.war
2014-06-06 13:41:07,523 INFO  Started Brooklyn console at http://127.0.0.1:8081/, running classpath://brooklyn.war and []
2014-06-06 13:41:07,524 INFO  Launched Brooklyn; will now block until shutdown issued. Shutdown via GUI or API or process interrupt.

and in a prod env you'll see even less

2014-06-06 13:41:02,704 INFO  Persistence mode REBIND, directory /Users/alex/.brooklyn/brooklyn-persisted-state/data backed up to /Users/alex/.brooklyn/brooklyn-persisted-state/data-2014-06-06-0141-02.bak
2014-06-06 13:41:06,068 INFO  Rebind complete: 0 apps, 0 entities, 0 locations, 0 policies, 0 enrichers
2014-06-06 13:41:06,084 INFO  Management node started as HA MASTER autodetected
2014-06-06 13:41:07,523 INFO  Started Brooklyn console at http://127.0.0.1:8081/, running classpath://brooklyn.war and []
2014-06-06 13:41:07,524 INFO  Launched Brooklyn; will now block until shutdown issued. Shutdown via GUI or API or process interrupt.
buildhive commented 10 years ago

Brooklyn Central » brooklyn #2443 FAILURE Looks like there's a problem with this pull request (what's this?)

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2445 FAILURE Looks like there's a problem with this pull request (what's this?)

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2449 SUCCESS This pull request looks good (what's this?)

ahgittin commented 10 years ago

yay it passed!

aledsage commented 10 years ago

@ahgittin we use alwaysRun=true for BeforeMethod because if there is a test in that class subsequently marked as groups="Integration" then otherwise the setUp method wouldn't be run for it. (Note it would be run in testng in eclipse I believe, but would fail on the mvn command line.) We were bitten a few times by that, e.g. by a slow-running test getting the annotation added, and then failing; or someone adding an integration test into the class.

ahgittin commented 10 years ago

that's right ... so alwaysRun=true has nothing to do with the fact that @BeforeMethod annotated test is running even when test is being skipped due to group being set on subclass.

in rebasing on master (so that merge can happen) i've added the new test introduced in master also marked Integration in new class HighAvailabilityManagerJcloudsObjectStoreTest being added here. (may cause a failure if it tests the commit before i did that!)

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2456 FAILURE Looks like there's a problem with this pull request (what's this?)

buildhive commented 10 years ago

Brooklyn Central » brooklyn #2458 SUCCESS This pull request looks good (what's this?)

ahgittin commented 10 years ago

merging this on basis @aledsage has reviewed most of it and it is going to cause more merge conflicts and hassles if we don't merge, viz #1462 #1439. @aledsage can you take a look over https://github.com/brooklyncentral/brooklyn/pull/1452#issuecomment-45361317 just to sanity check. any comments there we can merge subsequently.