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

[Matt Bruzek]

These are the changes needed to make cassandra run on power PC.

The charm should check if the deb package exists and use that to install.

There is a configuration value that have to be updated for ppc64le only.
There is a configuration value that needs to change for openjdk.

[Tim Van Steenburgh (tvansteenburgh)]

Matt,

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

Thanks,
Tim

[Matt Bruzek (mbruzek)]

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)

[Tim Van Steenburgh (tvansteenburgh)]

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.

[lp:~mbruzek/charms/precise/cassandra/ppc64le]

35. By Matt Bruzek on 2014-08-25

Improving charm proof errors and adding a test that can be run from the automated framework.

[Matt Bruzek (mbruzek)]

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.

[amir sanjar (asanjar)]

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}'

[Charles Butler (lazypower)]

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!

review: Approve

=== modified file 'config.yaml'
--- config.yaml	2013-11-22 16:43:13 +0000
+++ config.yaml	2014-08-25 17:51:57 +0000
@@ -220,15 +220,19 @@
             Used with endpoint_snitch=GossipingPropertyFileSnitch to prefer the internal ip when
             possible, as the Ec2MultiRegionSnitch does. Used with cassandra >= 1.2.x
     dc_suffix:
+        default: 
         type: string
         description:
             Add a suffix to a datacenter name. Used by the Ec2Snitch and Ec2MultiRegionSnitch to 
             append a string to the EC2 region name. Used with cassandra >= 1.2.x
     datacenter:
+        default: 
         type: string
         description:
             The datacenter used by the enpoint_snitch. i.e. "DC1"
-    rack: 
+    rack:
+        default:
         type: string
         description:
             The rack used by the enpoint_snitch. i.e. "Rack1"
+

=== modified file 'hooks/cassandra-common'
--- hooks/cassandra-common	2014-05-19 20:07:30 +0000
+++ hooks/cassandra-common	2014-08-25 17:51:57 +0000
@@ -13,7 +13,7 @@
 CASSANDRA_GROUP="cassandra"
 NAGIOS_EXPORT_DIR="/var/lib/nagios/export"
 
-set -e
+set -ex
 
 export LANG=en_US.UTF-8
 
@@ -79,20 +79,33 @@
   APT_REPO_KEY=$(config-get apt-repo-key)
   # Check for configured extra packages to install from config
   EXTRA_PACKAGES="$(config-get extra_packages)"
-  # Add the key
-  if [[ -n "${APT_REPO_KEY}" ]]; then
-    apt-key adv --keyserver keyserver.ubuntu.com --recv-keys ${APT_REPO_KEY}
-  fi
-  if [[ -n "${APT_REPO_SPEC}" ]]; then 
-    echo "${APT_REPO_SPEC}" > /etc/apt/sources.list.d/cassandra.list
-  fi
-  # Update the repositories
+  # Get the debian package in the files directory.
+  DEBIAN_PACKAGE_FILE=($CHARM_DIR/files/cassandra_*.deb)
+  # Does the debain cassandra package exist in the charm directory?
+  if [[ -f ${DEBIAN_PACKAGE_FILE} ]]; then
+    # The apt-get update command is necessary to install java.
+    apt-get update -qq
+    # Install the prerequisites for cassandra.
+    DEBIAN_FRONTEND=noninteractive apt-get -qq install -y openjdk-7-jre-headless libjna-java python-support python-cheetah dnsutils bzr ${EXTRA_PACKAGES}
+    # Use the debian package manager to install the cassandra package.
+    dpkg -i ${DEBIAN_PACKAGE_FILE}
+  else
+    # Add the key
+    if [[ -n "${APT_REPO_KEY}" ]]; then
+      # Use port 80 to get the keyserver key.
+      apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys ${APT_REPO_KEY}
+    fi
+    if [[ -n "${APT_REPO_SPEC}" ]]; then 
+      echo "${APT_REPO_SPEC}" > /etc/apt/sources.list.d/cassandra.list
+    fi
+    # Update the repositories
 
-  apt-get update
-  # Ensure that cassandra does not startup before we have configured it
-  disable_cassandra_start
-  # Install the package
-  DEBIAN_FRONTEND=noninteractive apt-get -qq install -y cassandra python-cheetah dnsutils bzr ${EXTRA_PACKAGES}
+    apt-get update
+    # Ensure that cassandra does not startup before we have configured it
+    disable_cassandra_start
+    # Install the package
+    DEBIAN_FRONTEND=noninteractive apt-get -qq install -y cassandra python-cheetah dnsutils bzr ${EXTRA_PACKAGES}
+  fi
   bzr_ci || :
 }
 
@@ -349,9 +362,19 @@
   # Open port ready for expose
   open-port ${CLIENT_PORT}/TCP
 
-  # Default cassandra-env.sh for some cassandra 1.0.x specifies -Xss128k
-  # while 160k min is required
-  sed -i -e "s/-Xss128k/-Xss256k/" ${CASSANDRA_ENV}
+  # Get the architecture from the uname command.
+  ARCHITECTURE=`uname -m`
+  if [ "$ARCHITECTURE" == "ppc64le" ]; then
+    # On the ppc64le architecture requires a higher value for stack size.
+    sed -i -e "s/-Xss[0-9]*k/-Xss1664k/" ${CASSANDRA_ENV}
+  else
+    # Default cassandra-env.sh for some cassandra 1.0.x specifies -Xss128k
+    # while 160k min is required
+    sed -i -e "s/-Xss[0-9]*k/-Xss256k/" ${CASSANDRA_ENV}
+  fi
+  # OpenJDK does not handle the UseCondCardMark flag.
+  sed -i -e "s/-XX:+UseCondCardMark//" ${CASSANDRA_ENV}
+
   # Configure memory settings as specified in configuration
   if [ "$AUTO_MEMORY" = "False" ]; then
     juju-log "Configuring Manual Memory Setttings"

=== added file 'tests/01-test'
--- tests/01-test	1970-01-01 00:00:00 +0000
+++ tests/01-test	2014-08-25 17:51:57 +0000
@@ -0,0 +1,3 @@
+#!/bin/bash
+# Run the cassandra tests that are in the Makefile.
+make