Merge lp://staging/~mbruzek/charms/precise/cassandra/ppc64le into lp://staging/charms/cassandra

Matt,

Can you list the two configuration values that need to be changed?

Thanks,
Tim

Sorry I was not clear. The charm code had to be changed that edits the Cassandra configuration file cassandra-env.sh to get the charm to deploy.

For reference those values are:
UseCondCardMark (had to be removed the OpenJDK does not recognize this value)
-Xss (had to be increased to 1664k which is 10x the original value in the file)

Hey Matt, good stuff! Nice to see the Cassandra charm being updated to run on ppc64le! Here's what I found:

PPC64LE
-------
The changes for ppc64le worked great. I tested on ppc64le with a bundled cassandra .deb [1], and on x86_64 with and without a bundled .deb. The charm started up just fine in all scenarios.

Charm Proof
-----------
W: config.yaml: option datacenter does not have the keys: default
W: config.yaml: option dc_suffix does not have the keys: default
W: config.yaml: option rack does not have the keys: default

These warnings were not caused by your change, but it would be nice to go ahead and get them cleaned up.

Tests
-----
There are tests in the charm that can be executed like so:

cd charms/trusty/cassandra/tests && make

I have two comments on the tests:

1. They fail. They fail on the current store revision also, so the failure is not a result of this change, but it would be nice to get them fixed, although not necessarily in this merge.
2. The tests are not detected by bundletester [2], so they will not be run by our automated testing infrastructure. The easiest way to fix it in this case is to:

cd charms/trusty/cassandra/tests
echo -e '#!/bin/bash\nmake' > 01-test && chmod +x 01-test

You can then run the tests by executing `bundletester -F` from within the charm dir. (By default, bundletester looks for executable files, in a tests/ dir, that match the "[0-9]*" glob pattern.)

Again, I'm not suggesting that these tests must be fixed in this merge, merely documenting the findings of my review. IMO it would be acceptable to fix the tests in a separate merge.

[1] http://www.apache.org/dist/cassandra/debian/pool/main/c/cassandra/cassandra_2.1.0~rc6_all.deb
[2]* lp:~tvansteenburgh/charmtester/bundletester

* This is not the official bundletester repo, but has some fixes that haven't been merged yet.

Thanks for the review Tim.

I have improved the charm proof status with the default keys and added a 01-test as you described to allow the Makefile to be called from the automated tester.

Matt it looks good. The only recommendation would be to install JAVA SDK package as default for performance and java life cycle management tools (i.e jps).
example: jps | awk '{print $2}'

Awesome merge MBruzek!

Thank you for your continued effort on this. +1 on the merge, but I'd like to see additional info on the offline installer in the readme. theres no notation of placing a 'fat packed' offline-able install mode by simply placing a cassandra_*.deb in files.

I'll merge this tentatively with a follow up MP to update the readme.

Thanks again for the effort!