Merge lp:~ev/charms/xenial/cassandra/snap into lp:~cassandra-charmers/charms/xenial/cassandra/trunk

[Evan]

[Stuart Bishop (stub)]

Comments inline.

[Evan (ev)]

Thanks for the thorough review! I've fixed the raised issues and mentioned the individual revnos inline.

[Stuart Bishop (stub)]

This is all looking good. A few comments inline, but nothing series except for the encrypt() method failing under trusty.

We will want integration tests, which starts with creating TestSnapDeployment(Test3UnitDeployment) in tests/test_integration.py. The tests and harness on this branch are Juju1 specific (I can sort Juju 2.0 mostly by syncing testing/amuletfixture.py to the PostgreSQL charm's version).

The integration tests at the moment will almost certainly fail for snap, as they 'juju run --unit ... nodetool ...' so we either need the charm to create symlinks in /usr/local/bin so the standard tool names work, or update the callsites in the test suite (just 3 or 4 I see). Ideally the snap would use the standard tool names like 'nodetool' rather than 'cassandra.nodetool', but from what I can tell that would mean maintaining a separate snap for each cli tool?

[lp:~ev/charms/xenial/cassandra/snap]

390. By Evan on 2016-06-28

Add bug reference.

391. By Evan on 2016-06-28

Correct default for edition.

392. By Evan on 2016-06-28

Prefer platform.dist to host.lsb_release

393. By Evan on 2016-06-29

Do not try to decode from a string

394. By Evan on 2016-06-29

Fix tests; reset log mocks between calls.

395. By Evan on 2016-06-29

Revert move from lsb_release to platform: https://bugs.python.org/issue1322#msg263313

[Stuart Bishop (stub)]

I think this is past the point where it can land. I'm running tests.

review: Approve

[Stuart Bishop (stub)]

I needed to make some more changes, which at at lp:~stub/charms/trusty/cassandra/xenial. The charm is now multi-series, and I'm pushing it to a new home at https://launchpad.net/cassandra-charm ( git+ssh://git.launchpad.net/cassandra-charm ). Further work should land on the git branch (let me know if you have much outstanding work that can't be easily moved in).

There are no integration tests of the snap install yet. Which is fine, as there is no documentation for the new feature either :-) We should fix both of these issues on the git branch.

=== modified file 'config.yaml'
--- config.yaml	2016-02-26 16:47:03 +0000
+++ config.yaml	2016-06-29 15:55:10 +0000
@@ -74,16 +74,16 @@
         type: string
         default: community
         description: >
-            One of 'community' or 'dse'. 'community' uses the
-            Apache Cassandra packages. 'dse' is for DataStax
-            Enterprise. Selecting 'dse' overrides the jvm setting.
+            One of 'community', 'dse', or 'apache-snap'. 'community' uses the
+            Apache Cassandra packages. 'dse' is for DataStax Enterprise.
+            Selecting 'dse' overrides the jvm setting. 'apache-snap' uses a
+            snap package of Apache Cassandra.
     jre:
         type: string
         default: openjdk
         description: >
             Which Java runtime environment to use. May be 'openjdk' or
             'oracle'.
-            
     # Storage configuration
     wait_for_storage_broker:
         type: boolean

=== modified file 'hooks/actions.py'
--- hooks/actions.py	2016-06-09 10:59:24 +0000
+++ hooks/actions.py	2016-06-29 15:55:10 +0000
@@ -277,7 +277,9 @@
 @action
 def install_cassandra_packages():
     helpers.install_packages(helpers.get_cassandra_packages())
-    if helpers.get_jre() != 'oracle':
+    if helpers.get_cassandra_edition() == 'apache-snap':
+        helpers.install_cassandra_snap()
+    elif helpers.get_jre() != 'oracle':
         subprocess.check_call(['update-java-alternatives',
                                '--jre-headless',
                                '--set', 'java-1.8.0-openjdk-amd64'])
@@ -286,6 +288,7 @@
 @action
 def ensure_cassandra_package_status():
     helpers.ensure_package_status(helpers.get_cassandra_packages())
+    helpers.ensure_cassandra_snap_installed()
 
 
 def _fetch_oracle_jre():
@@ -361,6 +364,8 @@
 def install_oracle_jre():
     if helpers.get_jre() != 'oracle':
         return
+    if helpers.get_cassandra_edition() == 'apache-snap':
+        return
 
     tarball = _fetch_oracle_jre()
     _install_oracle_jre_tarball(tarball)
@@ -368,6 +373,9 @@
 
 @action
 def emit_java_version():
+    if helpers.get_cassandra_edition() == 'apache-snap':
+        return
+
     # Log the version for posterity. Could be useful since Oracle JRE
     # security updates are not automated.
     version = subprocess.check_output(['java', '-version'],
@@ -426,7 +434,7 @@
                                rack={}
                                ''').format(datacenter, rack)
     rackdc_path = helpers.get_cassandra_rackdc_file()
-    host.write_file(rackdc_path, rackdc_properties.encode('UTF-8'))
+    helpers.write_config(rackdc_path, rackdc_properties)
 
 
 def needs_reset_auth_keyspace_replication():

=== modified file 'hooks/helpers.py'
--- hooks/helpers.py	2016-06-09 10:59:24 +0000
+++ hooks/helpers.py	2016-06-29 15:55:10 +0000
@@ -125,6 +125,46 @@
     dpkg.communicate(input=''.join(selections).encode('US-ASCII'))
 
 
+# FOR CHARMHELPERS
+@logged
+def install_cassandra_snap():
+    '''Ensure that the Cassandra snap is installed at the current revision.
+    '''
+    cmd = ['snap', 'list', 'cassandra']
+    output = subprocess.check_output(cmd, universal_newlines=True)
+    if 'cassandra' not in output:
+        with autostart_disabled(['snap.cassandra.cassandra']):
+            subprocess.check_call(['snap', 'install', 'cassandra'])
+    else:
+        # snap will exit 1 if the snap is already the newest version.
+        # Filed as http://pad.lv/1595064
+        subprocess.call(['snap', 'refresh', 'cassandra'])
+
+    plug = 'cassandra:mount-observe'
+    slot = 'ubuntu-core:mount-observe'
+    subprocess.check_call(['snap', 'connect', plug, slot])
+
+
+# FOR CHARMHELPERS
+@logged
+def ensure_cassandra_snap_installed():
+    '''Check if the cassandra snap is installed and raise RuntimeError if it is
+    not.
+    '''
+    cmd = ['snap', 'list', 'cassandra']
+    output = subprocess.check_output(cmd, universal_newlines=True)
+    if 'cassandra' not in output:
+        raise RuntimeError('Cassandra snap not installed.')
+
+
+def get_snap_env(envar):
+    '''Return the value of an environment variable present in the snap wrapper
+    scripts.
+    '''
+    cmd = ['/snap/bin/cassandra.env-get', envar]
+    return subprocess.check_output(cmd, universal_newlines=True).strip('\n')
+
+
 def get_seed_ips():
     '''Return the set of seed ip addresses.
 
@@ -149,7 +189,9 @@
     '''
     import relations
     storage = relations.StorageRelation()
-    if storage.mountpoint:
+    if get_cassandra_edition() == 'apache-snap':
+        root = get_snap_env('SNAP_DATA')
+    elif storage.mountpoint:
         root = os.path.join(storage.mountpoint, 'cassandra')
     else:
         root = '/var/lib/cassandra'
@@ -169,10 +211,13 @@
     component = os.sep
     for p in absdir.split(os.sep)[1:-1]:
         component = os.path.join(component, p)
-        if not os.path.exists(p):
+        if not os.path.exists(component):
             host.mkdir(component)
     assert component == os.path.split(absdir)[0]
-    host.mkdir(absdir, owner='cassandra', group='cassandra', perms=0o750)
+    if get_cassandra_edition() == 'apache-snap':
+        host.mkdir(absdir, owner='root', group='root', perms=0o750)
+    else:
+        host.mkdir(absdir, owner='cassandra', group='cassandra', perms=0o750)
     return absdir
 
 
@@ -277,6 +322,15 @@
             host.write_file(backup_path, f.read(), perms=0o600)
 
 
+def get_snap_version(snap):
+    '''Get the version string for an installed snap.'''
+    cmd = ['snap', 'list', snap]
+    out = subprocess.check_output(cmd, universal_newlines=True)
+    match = re.search('\n{}\s*(\S*)'.format(snap), out)
+    if match:
+        return match.groups(0)[0]
+
+
 # FOR CHARMHELPERS
 def get_package_version(package):
     cache = fetch.apt_cache()
@@ -305,22 +359,34 @@
 def get_cassandra_edition():
     config = hookenv.config()
     edition = config['edition'].lower()
-    if edition not in ('community', 'dse'):
+    if edition not in ('community', 'dse', 'apache-snap'):
         hookenv.log('Unknown edition {!r}. Using community.'.format(edition),
                     ERROR)
         edition = 'community'
+    release = host.lsb_release()['DISTRIB_CODENAME']
+    if edition == 'apache-snap' and release in ['precise', 'trusty']:
+        msg = '{!r} cannot be used with {!r}. Using community.'
+        msg = msg.format(release, edition)
+        hookenv.log(msg, ERROR)
+        edition = 'community'
+
     return edition
 
 
 def get_cassandra_service():
     '''Cassandra upstart service'''
-    if get_cassandra_edition() == 'dse':
+    if get_cassandra_edition() == 'apache-snap':
+        return 'snap.cassandra.cassandra'
+    elif get_cassandra_edition() == 'dse':
         return 'dse'
     return 'cassandra'
 
 
 def get_cassandra_version():
-    if get_cassandra_edition() == 'dse':
+    edition = get_cassandra_edition()
+    if edition == 'apache-snap':
+        return get_snap_version('cassandra')
+    elif edition == 'dse':
         dse_ver = get_package_version('dse-full')
         if not dse_ver:
             return None
@@ -338,12 +404,37 @@
 
 
 def get_cassandra_config_dir():
-    if get_cassandra_edition() == 'dse':
+    edition = get_cassandra_edition()
+    if edition == 'apache-snap':
+        return get_snap_env('CASSANDRA_CONF')
+    elif edition == 'dse':
         return '/etc/dse/cassandra'
     else:
         return '/etc/cassandra'
 
 
+def get_snap_config_file(filename):
+    '''Get the contents of the named configuration file from the current snap
+    data directory.
+    '''
+    cmd = ['/snap/bin/cassandra.config-get', filename]
+    return subprocess.check_output(cmd, universal_newlines=True)
+
+
+def set_snap_config_file(filename, contents):
+    '''Install a new copy of the configuration file with the provided contents
+    in the current snap data directory.
+    '''
+    cmd = ['/snap/bin/cassandra.config-set', filename]
+    cs = subprocess.Popen(cmd, stdin=subprocess.PIPE, universal_newlines=True)
+    _, err = cs.communicate(input=contents)
+    if err:
+        hookenv.log('Error calling {}:\n{}'.format(' '.join(cmd), err))
+    if cs.returncode != 0:
+        msg = '{} exited with code {}'.format(' '.join(cmd), cs.returncode)
+        raise RuntimeError(msg)
+
+
 def get_cassandra_yaml_file():
     return os.path.join(get_cassandra_config_dir(), "cassandra.yaml")
 
@@ -359,7 +450,10 @@
 
 def get_cassandra_pid_file():
     edition = get_cassandra_edition()
-    if edition == 'dse':
+    if edition == 'apache-snap':
+        home = get_snap_env('CASSANDRA_HOME')
+        pid_file = os.path.join(home, 'cassandra.pid')
+    elif edition == 'dse':
         pid_file = "/var/run/dse/dse.pid"
     else:
         pid_file = "/var/run/cassandra/cassandra.pid"
@@ -367,16 +461,22 @@
 
 
 def get_cassandra_packages():
-    edition = get_cassandra_edition()
-    if edition == 'dse':
-        packages = set(['dse-full'])
-    else:
-        packages = set(['cassandra'])  # 'cassandra-tools'
+    packages = set()
 
     packages.add('ntp')
     packages.add('run-one')
     packages.add('netcat')
 
+    edition = get_cassandra_edition()
+    if edition == 'apache-snap':
+        packages.add('snapd')
+        return packages
+        
+    if edition == 'dse':
+        packages.add('dse-full')
+    else:
+        packages.add('cassandra')  # 'cassandra-tools'
+
     jre = get_jre()
     if jre == 'oracle':
         # We can't use a packaged version of the Oracle JRE, as we
@@ -464,6 +564,7 @@
     # Guard against changing perms on a running db. Although probably
     # harmless, it causes shutil.chown() to fail.
     assert not is_cassandra_running()
+
     db_dirs = get_all_database_directories()
     ensure_database_directory(db_dirs['commitlog_directory'])
     ensure_database_directory(db_dirs['saved_caches_directory'])
@@ -549,7 +650,19 @@
 
 
 def encrypt_password(password):
-    return bcrypt.hashpw(password, bcrypt.gensalt())
+    password = password.encode('ascii')
+    # Java doesn't understand bcrypt 2b yet:
+    # cassandra.AuthenticationFailed: Failed to authenticate to localhost:
+    # code=0000 [Server error] message="java.lang.IllegalArgumentException:
+    # Invalid salt revision"
+    try:
+        salt = bcrypt.gensalt(prefix=b'2a')
+        # Newer versions of bcrypt return a bytestring.
+        return bcrypt.hashpw(password, salt).decode('ascii')
+    except TypeError:
+        # Trusty bcrypt doesn't support prefix=
+        salt = bcrypt.gensalt()
+        return bcrypt.hashpw(password, salt)
 
 
 @logged
@@ -563,6 +676,7 @@
         hookenv.log('Creating SUPERUSER {}'.format(username))
     else:
         hookenv.log('Creating user {}'.format(username))
+
     if has_cassandra_version('2.2'):
         query(session,
               'INSERT INTO system_auth.roles '
@@ -609,7 +723,11 @@
 
 
 def get_cqlshrc_path():
-    return os.path.expanduser('~root/.cassandra/cqlshrc')
+    if get_cassandra_edition() == 'apache-snap':
+        base = get_cassandra_config_dir()
+        return os.path.join(base, 'cql/cqlshrc')
+    else:
+        return os.path.expanduser('~root/.cassandra/cqlshrc')
 
 
 def superuser_username():
@@ -672,7 +790,11 @@
 
 
 def nodetool(*cmd, timeout=120):
-    cmd = ['nodetool'] + [str(i) for i in cmd]
+    if get_cassandra_edition() == 'apache-snap':
+        nodetool_cmd = '/snap/bin/cassandra.nodetool'
+    else:
+        nodetool_cmd = 'nodetool'
+    cmd = [nodetool_cmd] + [str(i) for i in cmd]
     i = 0
     until = time.time() + timeout
     for _ in backoff('nodetool to work'):
@@ -710,24 +832,39 @@
     return len(get_bootstrapped_ips())
 
 
+def write_config(path, contents):
+    '''Write out the config file at path with the provided contents, encoding
+    in UTF-8 first. If using a snap edition, write to the snap config
+    directory.
+    '''
+    if get_cassandra_edition() == 'apache-snap':
+        set_snap_config_file(os.path.basename(path), contents)
+    else:
+        contents = contents.encode('UTF-8')
+        host.write_file(path, contents)
+
+
 def read_cassandra_yaml():
-    cassandra_yaml_path = get_cassandra_yaml_file()
-    with open(cassandra_yaml_path, 'rb') as f:
+    if get_cassandra_edition() == 'apache-snap':
+        f = get_snap_config_file('cassandra.yaml')
         return yaml.safe_load(f)
+    else:
+        cassandra_yaml_path = get_cassandra_yaml_file()
+        with open(cassandra_yaml_path, 'rb') as f:
+            return yaml.safe_load(f)
 
 
 @logged
 def write_cassandra_yaml(cassandra_yaml):
-    cassandra_yaml_path = get_cassandra_yaml_file()
-    host.write_file(cassandra_yaml_path,
-                    yaml.safe_dump(cassandra_yaml).encode('UTF-8'))
+    write_config(get_cassandra_yaml_file(), yaml.safe_dump(cassandra_yaml))
 
 
 def configure_cassandra_yaml(overrides={}, seeds=None):
-    cassandra_yaml_path = get_cassandra_yaml_file()
     config = hookenv.config()
 
-    maybe_backup(cassandra_yaml_path)  # Its comments may be useful.
+    if get_cassandra_edition() != 'apache-snap':
+        cassandra_yaml_path = get_cassandra_yaml_file()
+        maybe_backup(cassandra_yaml_path)  # Its comments may be useful.
 
     cassandra_yaml = read_cassandra_yaml()
 
@@ -784,6 +921,7 @@
 
 
 def is_cassandra_running():
+    edition = get_cassandra_edition()
     pid_file = get_cassandra_pid_file()
 
     try:
@@ -797,7 +935,12 @@
             # is not running.
             os.kill(pid, 0)
 
-            if subprocess.call(["nodetool", "status"],
+            if edition == 'apache-snap':
+                # /snap/bin is not on PATH for the root user.
+                nodetool = '/snap/bin/cassandra.nodetool'
+            else:
+                nodetool = 'nodetool'
+            if subprocess.call([nodetool, "status"],
                                stdout=subprocess.DEVNULL,
                                stderr=subprocess.DEVNULL) == 0:
                 hookenv.log(

=== modified file 'tests/test_helpers.py'
--- tests/test_helpers.py	2016-03-17 10:54:10 +0000
+++ tests/test_helpers.py	2016-06-29 15:55:10 +0000
@@ -163,6 +163,10 @@
         # Autostart wasn't messed with.
         self.assertFalse(autostart_disabled.called)
 
+    def test_encrypt_password(self):
+        self.assertEqual(type(helpers.encrypt_password('')), str)
+
+
     @patch('subprocess.Popen')
     def test_ensure_package_status(self, popen):
         for status in ['install', 'hold']:
@@ -380,7 +384,10 @@
         hookenv.config()['edition'] = 'dse'
         self.assertEqual(helpers.get_jre(), 'oracle')
 
-    def test_get_cassandra_edition(self):
+    @patch('charmhelpers.core.host.lsb_release')
+    def test_get_cassandra_edition(self, lsb_release):
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'trusty'}
+
         hookenv.config()['edition'] = 'community'
         self.assertEqual(helpers.get_cassandra_edition(), 'community')
 
@@ -389,10 +396,21 @@
 
         self.assertFalse(hookenv.log.called)
 
+        hookenv.log.reset_mock()
         hookenv.config()['edition'] = 'typo'  # Default to community
         self.assertEqual(helpers.get_cassandra_edition(), 'community')
         hookenv.log.assert_any_call(ANY, hookenv.ERROR)  # Logs an error.
 
+        hookenv.log.reset_mock()
+        hookenv.config()['edition'] = 'apache-snap'  # Default to community
+        self.assertEqual(helpers.get_cassandra_edition(), 'community')
+        hookenv.log.assert_any_call(ANY, hookenv.ERROR)  # Logs an error.
+
+        lsb_release.reset_mock()
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'xenial'}
+        hookenv.config()['edition'] = 'apache-snap'
+        self.assertEqual(helpers.get_cassandra_edition(), 'apache-snap')
+
     @patch('helpers.get_cassandra_edition')
     def test_get_cassandra_service(self, get_edition):
         get_edition.return_value = 'whatever'
@@ -460,6 +478,30 @@
                          '/foo/cassandra-rackdc.properties')
 
     @patch('helpers.get_cassandra_edition')
+    def test_write_config(self, get_edition):
+        get_edition.return_value = 'whatever'
+        expected = 'some config'
+        with tempfile.TemporaryDirectory() as tmpdir:
+            path = os.path.join(tmpdir, 'foo')
+            helpers.write_config(path, expected)
+            with open(path, 'r') as fp:
+                self.assertEqual(fp.read(), expected)
+
+    @patch('subprocess.Popen')
+    @patch('helpers.get_cassandra_edition')
+    def test_write_config_snap(self, get_edition, popen):
+        get_edition.return_value = 'apache-snap'
+        popen.return_value.returncode = 0
+        popen.return_value.communicate.return_value = ('', '')
+        helpers.write_config('/some/path/to/config.yaml', 'some config')
+        expected = 'some config'
+        self.assertEqual(
+            [call(['/snap/bin/cassandra.config-set', 'config.yaml'],
+            stdin=subprocess.PIPE, universal_newlines=True),
+            call().communicate(input=expected)], popen.mock_calls)
+
+
+    @patch('helpers.get_cassandra_edition')
     def test_get_cassandra_pid_file(self, get_edition):
         get_edition.return_value = 'whatever'
         self.assertEqual(helpers.get_cassandra_pid_file(),