Merge lp:~gnuoy/charms/trusty/percona-cluster/1454317 into lp:~openstack-charmers-archive/charms/trusty/percona-cluster/next

[Liam Young]

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #4246 percona-cluster-next for gnuoy mp258981
    UNIT FAIL: unit-test failed

UNIT Results (max last 2 lines):
make: *** [unit_test] Error 1
ERROR:root:Make target returned non-zero.

Full unit test output: http://paste.ubuntu.com/11115163/
Build: http://10.245.162.77:8080/job/charm_unit_test/4246/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #4521 percona-cluster-next for gnuoy mp258981
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/11115164/
Build: http://10.245.162.77:8080/job/charm_lint_check/4521/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #4100 percona-cluster-next for gnuoy mp258981
    AMULET FAIL: amulet-test missing

AMULET Results (max last 2 lines):
INFO:root:Search string not found in makefile target commands.
ERROR:root:No make target was executed.

Full amulet test output: http://paste.ubuntu.com/11115168/
Build: http://10.245.162.77:8080/job/charm_amulet_test/4100/

[Mario Splivalo (mariosplivalo)]

This looks ok to me. Verified that passwords are stored correctly in relations, adding more units doesn't break things.

The issues are amulet tests, which were included with this commit: http://bazaar.launchpad.net/~openstack-charmers/charms/trusty/percona-cluster/next/revision/54

Apparently it's an amulet issue - test raises amulet.SKIP if no AMULET_OS_VIP is configured, but amulet considers that as an error and fails the test. Same issue as with rev54 commit, probably the test should be written in a manner so that it doesn't raise. amulet.SKIP.

Otherwise, peachy!

review: Approve

[Edward Hope-Morley (hopem)]

Liam, i don't see these changes in lp:charm-helpers (e.g. wipe_disk_passwords() is not in contrib.database.mysql).

review: Needs Fixing

[Edward Hope-Morley (hopem)]

Ok found it (linked charm-helpers MP to LP)

[Edward Hope-Morley (hopem)]

This could do with a cleanup+resync.

review: Resubmit

[lp:~gnuoy/charms/trusty/percona-cluster/1454317]

63. By Liam Young on 2015-09-30

Merged next in

64. By Liam Young on 2015-09-30

Rebase changes

65. By Liam Young on 2015-09-30

Fix typos and lint

Unmerged revisions

65. By Liam Young on 2015-09-30

Fix typos and lint

64. By Liam Young on 2015-09-30

Rebase changes

63. By Liam Young on 2015-09-30

Merged next in

62. By Liam Young on 2015-05-13

Only add passwords to the peer relation if the unit is clustered, the leader and remove stale passwords after a slave sync. Also replace eligible_leader with is_elected_leader as eligible_leader is deprecated

61. By Liam Young on 2015-05-13

Sync charmhelpers with db fixes

=== modified file 'charm-helpers-hooks.yaml'
--- charm-helpers-hooks.yaml	2015-08-26 13:22:41 +0000
+++ charm-helpers-hooks.yaml	2015-09-30 08:52:41 +0000
@@ -1,4 +1,4 @@
-branch: lp:charm-helpers
+branch: lp:~gnuoy/charm-helpers/no-create-if-none
 destination: hooks/charmhelpers
 include:
     - core

=== modified file 'charmhelpers/contrib/database/mysql.py'
--- charmhelpers/contrib/database/mysql.py	2015-08-03 14:52:57 +0000
+++ charmhelpers/contrib/database/mysql.py	2015-09-30 08:52:41 +0000
@@ -134,6 +134,14 @@
         finally:
             cursor.close()
 
+    def wipe_disk_passwords(self):
+        """Remove all passwords stored on disk"""
+        dirname = os.path.dirname(self.root_passwd_file_template)
+        path = os.path.join(dirname, '*.passwd')
+        for f in glob.glob(path):
+            log("Removing password file {}".format(f), level=DEBUG)
+            os.unlink(f)
+
     def migrate_passwords_to_peer_relation(self, excludes=None):
         """Migrate any passwords storage on disk to cluster peer relation."""
         dirname = os.path.dirname(self.root_passwd_file_template)
@@ -156,7 +164,7 @@
                 # NOTE cluster relation not yet ready - skip for now
                 pass
 
-    def get_mysql_password_on_disk(self, username=None, password=None):
+    def get_mysql_password_on_disk(self, username=None, password=None, create_if_none=True):
         """Retrieve, generate or store a mysql password for the provided
         username on disk."""
         if username:
@@ -170,7 +178,7 @@
             log("Using existing password file '%s'" % passwd_file, level=DEBUG)
             with open(passwd_file, 'r') as passwd:
                 _password = passwd.read().strip()
-        else:
+        elif create_if_none:
             log("Generating new password file '%s'" % passwd_file, level=DEBUG)
             if not os.path.isdir(os.path.dirname(passwd_file)):
                 # NOTE: need to ensure this is not mysql root dir (which needs
@@ -207,7 +215,8 @@
         for key in keys:
             yield key
 
-    def get_mysql_password(self, username=None, password=None):
+    def get_mysql_password(self, username=None, password=None,
+                           create_if_none=True):
         """Retrieve, generate or store a mysql password for the provided
         username using peer relation cluster."""
         excludes = []
@@ -227,12 +236,18 @@
             # cluster relation is not yet started; use on-disk
             _password = None
 
-        # If none available, generate new one
+        # If none available, check if one is on disk, if not
+        # get_mysql_password_on_disk will generate a new one if create_if_none
+        # is true
         if not _password:
-            _password = self.get_mysql_password_on_disk(username, password)
+            _password = self.get_mysql_password_on_disk(
+                username,
+                password,
+                create_if_none=create_if_none,
+            )
 
         # Put on wire if required
-        if self.migrate_passwd_to_peer_relation:
+        if _password and self.migrate_passwd_to_peer_relation:
             self.migrate_passwords_to_peer_relation(excludes=excludes)
 
         return _password

=== modified file 'charmhelpers/contrib/network/ip.py'
--- charmhelpers/contrib/network/ip.py	2015-09-02 09:32:16 +0000
+++ charmhelpers/contrib/network/ip.py	2015-09-30 08:52:41 +0000
@@ -23,7 +23,7 @@
 from functools import partial
 
 from charmhelpers.core.hookenv import unit_get
-from charmhelpers.fetch import apt_install
+from charmhelpers.fetch import apt_install, apt_update
 from charmhelpers.core.hookenv import (
     log,
     WARNING,
@@ -32,13 +32,15 @@
 try:
     import netifaces
 except ImportError:
-    apt_install('python-netifaces')
+    apt_update(fatal=True)
+    apt_install('python-netifaces', fatal=True)
     import netifaces
 
 try:
     import netaddr
 except ImportError:
-    apt_install('python-netaddr')
+    apt_update(fatal=True)
+    apt_install('python-netaddr', fatal=True)
     import netaddr
 
 

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-09-02 09:32:16 +0000
+++ charmhelpers/core/hookenv.py	2015-09-30 08:52:41 +0000
@@ -623,6 +623,38 @@
     return unit_get('private-address')
 
 
+@cached
+def storage_get(attribute="", storage_id=""):
+    """Get storage attributes"""
+    _args = ['storage-get', '--format=json']
+    if storage_id:
+        _args.extend(('-s', storage_id))
+    if attribute:
+        _args.append(attribute)
+    try:
+        return json.loads(subprocess.check_output(_args).decode('UTF-8'))
+    except ValueError:
+        return None
+
+
+@cached
+def storage_list(storage_name=""):
+    """List the storage IDs for the unit"""
+    _args = ['storage-list', '--format=json']
+    if storage_name:
+        _args.append(storage_name)
+    try:
+        return json.loads(subprocess.check_output(_args).decode('UTF-8'))
+    except ValueError:
+        return None
+    except OSError as e:
+        import errno
+        if e.errno == errno.ENOENT:
+            # storage-list does not exist
+            return []
+        raise
+
+
 class UnregisteredHookError(Exception):
     """Raised when an undefined hook is called"""
     pass

=== modified file 'charmhelpers/core/hugepage.py'
--- charmhelpers/core/hugepage.py	2015-08-26 13:11:30 +0000
+++ charmhelpers/core/hugepage.py	2015-09-30 08:52:41 +0000
@@ -25,11 +25,13 @@
     fstab_mount,
     mkdir,
 )
+from charmhelpers.core.strutils import bytes_from_string
+from subprocess import check_output
 
 
 def hugepage_support(user, group='hugetlb', nr_hugepages=256,
                      max_map_count=65536, mnt_point='/run/hugepages/kvm',
-                     pagesize='2MB', mount=True):
+                     pagesize='2MB', mount=True, set_shmmax=False):
     """Enable hugepages on system.
 
     Args:
@@ -49,6 +51,11 @@
         'vm.max_map_count': max_map_count,
         'vm.hugetlb_shm_group': gid,
     }
+    if set_shmmax:
+        shmmax_current = int(check_output(['sysctl', '-n', 'kernel.shmmax']))
+        shmmax_minsize = bytes_from_string(pagesize) * nr_hugepages
+        if shmmax_minsize > shmmax_current:
+            sysctl_settings['kernel.shmmax'] = shmmax_minsize
     sysctl.create(yaml.dump(sysctl_settings), '/etc/sysctl.d/10-hugepage.conf')
     mkdir(mnt_point, owner='root', group='root', perms=0o755, force=False)
     lfstab = fstab.Fstab()

=== modified file 'charmhelpers/core/strutils.py'
--- charmhelpers/core/strutils.py	2015-05-06 13:20:02 +0000
+++ charmhelpers/core/strutils.py	2015-09-30 08:52:41 +0000
@@ -18,6 +18,7 @@
 # along with charm-helpers.  If not, see <http://www.gnu.org/licenses/>.
 
 import six
+import re
 
 
 def bool_from_string(value):
@@ -40,3 +41,32 @@
 
     msg = "Unable to interpret string value '%s' as boolean" % (value)
     raise ValueError(msg)
+
+
+def bytes_from_string(value):
+    """Interpret human readable string value as bytes.
+
+    Returns int
+    """
+    BYTE_POWER = {
+        'K': 1,
+        'KB': 1,
+        'M': 2,
+        'MB': 2,
+        'G': 3,
+        'GB': 3,
+        'T': 4,
+        'TB': 4,
+        'P': 5,
+        'PB': 5,
+    }
+    if isinstance(value, six.string_types):
+        value = six.text_type(value)
+    else:
+        msg = "Unable to interpret non-string value '%s' as boolean" % (value)
+        raise ValueError(msg)
+    matches = re.match("([0-9]+)([a-zA-Z]+)", value)
+    if not matches:
+        msg = "Unable to interpret string value '%s' as bytes" % (value)
+        raise ValueError(msg)
+    return int(matches.group(1)) * (1024 ** BYTE_POWER[matches.group(2)])

=== modified file 'hooks/percona_hooks.py'
--- hooks/percona_hooks.py	2015-09-23 18:29:21 +0000
+++ hooks/percona_hooks.py	2015-09-30 08:52:41 +0000
@@ -108,20 +108,28 @@
     render_config()
     apt_update(fatal=True)
     apt_install(determine_packages(), fatal=True)
-    configure_sstuser(config('sst-password'))
+    configure_sstuser(
+        config('sst-password'),
+        migrate_passwd_to_peer_relation=False
+    )
 
 
 def render_config(clustered=False, hosts=[]):
     if not os.path.exists(os.path.dirname(MY_CNF)):
         os.makedirs(os.path.dirname(MY_CNF))
 
+    db_helper = get_db_helper(migrate_passwd_to_peer_relation=False)
+    cfg_passwd = config('sst-password')
+    sst_password = db_helper.get_mysql_password(username='sstuser',
+                                                password=cfg_passwd)
+
     context = {
         'cluster_name': 'juju_cluster',
         'private_address': get_host_ip(),
         'clustered': clustered,
         'cluster_hosts': ",".join(hosts),
         'sst_method': 'xtrabackup',
-        'sst_password': config('sst-password'),
+        'sst_password': sst_password,
         'innodb_file_per_table': config('innodb-file-per-table'),
         'table_open_cache': config('table-open-cache'),
         'lp1366997_workaround': config('lp1366997-workaround')
@@ -251,6 +259,9 @@
                 log("Cluster is bootstrapped - configuring mysql on this node",
                     DEBUG)
                 render_config_restart_on_changed(clustered, hosts)
+                # Bootstrapping from existing cluster invalidates passwords
+                db_helper = get_db_helper()
+                db_helper.wipe_disk_passwords()
             else:
                 log("Not configuring", DEBUG)
 
@@ -557,7 +568,14 @@
     resources = {'res_mysql_vip': res_mysql_vip,
                  'res_mysql_monitor': 'ocf:percona:mysql_monitor'}
 
-    sstpsswd = config('sst-password')
+    if is_elected_leader(DC_RESOURCE_NAME):
+        migrate_passwd = True
+    else:
+        migrate_passwd = False
+    db_helper = get_db_helper(migrate_passwd_to_peer_relation=migrate_passwd)
+    cfg_passwd = config('sst-password')
+    sstpsswd = db_helper.get_mysql_password(username='sstuser',
+                                            password=cfg_passwd)
     resource_params = {'res_mysql_vip': vip_params,
                        'res_mysql_monitor':
                        RES_MONITOR_PARAMS % {'sstpass': sstpsswd}}

=== modified file 'hooks/percona_utils.py'
--- hooks/percona_utils.py	2015-07-30 09:59:33 +0000
+++ hooks/percona_utils.py	2015-09-30 08:52:41 +0000
@@ -179,15 +179,18 @@
                            "BY '{}'")
 
 
-def get_db_helper():
-    return MySQLHelper(rpasswdf_template='/var/lib/charm/%s/mysql.passwd' %
-                       (service_name()),
-                       upasswdf_template='/var/lib/charm/%s/mysql-{}.passwd' %
-                       (service_name()))
-
-
-def configure_sstuser(sst_password):
-    m_helper = get_db_helper()
+def get_db_helper(migrate_passwd_to_peer_relation=True):
+    return MySQLHelper(
+        rpasswdf_template='/var/lib/charm/%s/mysql.passwd' %
+        (service_name()),
+        upasswdf_template='/var/lib/charm/%s/mysql-{}.passwd' %
+        (service_name()),
+        migrate_passwd_to_peer_relation=migrate_passwd_to_peer_relation)
+
+
+def configure_sstuser(sst_password, migrate_passwd_to_peer_relation=True):
+    m_helper = get_db_helper(
+        migrate_passwd_to_peer_relation=migrate_passwd_to_peer_relation)
     m_helper.connect(password=m_helper.get_mysql_root_password())
     m_helper.execute(SQL_SST_USER_SETUP.format(sst_password))
     m_helper.execute(SQL_SST_USER_SETUP_IPV6.format(sst_password))

=== modified file 'tests/charmhelpers/contrib/amulet/utils.py'
--- tests/charmhelpers/contrib/amulet/utils.py	2015-09-15 09:06:11 +0000
+++ tests/charmhelpers/contrib/amulet/utils.py	2015-09-30 08:52:41 +0000
@@ -326,7 +326,7 @@
 
     def service_restarted_since(self, sentry_unit, mtime, service,
                                 pgrep_full=None, sleep_time=20,
-                                retry_count=2, retry_sleep_time=30):
+                                retry_count=30, retry_sleep_time=10):
         """Check if service was been started after a given time.
 
         Args:
@@ -334,8 +334,9 @@
           mtime (float): The epoch time to check against
           service (string): service name to look for in process table
           pgrep_full: [Deprecated] Use full command line search mode with pgrep
-          sleep_time (int): Seconds to sleep before looking for process
-          retry_count (int): If service is not found, how many times to retry
+          sleep_time (int): Initial sleep time (s) before looking for file
+          retry_sleep_time (int): Time (s) to sleep between retries
+          retry_count (int): If file is not found, how many times to retry
 
         Returns:
           bool: True if service found and its start time it newer than mtime,
@@ -359,11 +360,12 @@
                                                             pgrep_full)
                 self.log.debug('Attempt {} to get {} proc start time on {} '
                                'OK'.format(tries, service, unit_name))
-            except IOError:
+            except IOError as e:
                 # NOTE(beisner) - race avoidance, proc may not exist yet.
                 # https://bugs.launchpad.net/charm-helpers/+bug/1474030
                 self.log.debug('Attempt {} to get {} proc start time on {} '
-                               'failed'.format(tries, service, unit_name))
+                               'failed\n{}'.format(tries, service,
+                                                   unit_name, e))
                 time.sleep(retry_sleep_time)
                 tries += 1
 
@@ -383,35 +385,62 @@
             return False
 
     def config_updated_since(self, sentry_unit, filename, mtime,
-                             sleep_time=20):
+                             sleep_time=20, retry_count=30,
+                             retry_sleep_time=10):
         """Check if file was modified after a given time.
 
         Args:
           sentry_unit (sentry): The sentry unit to check the file mtime on
           filename (string): The file to check mtime of
           mtime (float): The epoch time to check against
-          sleep_time (int): Seconds to sleep before looking for process
+          sleep_time (int): Initial sleep time (s) before looking for file
+          retry_sleep_time (int): Time (s) to sleep between retries
+          retry_count (int): If file is not found, how many times to retry
 
         Returns:
           bool: True if file was modified more recently than mtime, False if
-                file was modified before mtime,
+                file was modified before mtime, or if file not found.
         """
-        self.log.debug('Checking %s updated since %s' % (filename, mtime))
+        unit_name = sentry_unit.info['unit_name']
+        self.log.debug('Checking that %s updated since %s on '
+                       '%s' % (filename, mtime, unit_name))
         time.sleep(sleep_time)
-        file_mtime = self._get_file_mtime(sentry_unit, filename)
+        file_mtime = None
+        tries = 0
+        while tries <= retry_count and not file_mtime:
+            try:
+                file_mtime = self._get_file_mtime(sentry_unit, filename)
+                self.log.debug('Attempt {} to get {} file mtime on {} '
+                               'OK'.format(tries, filename, unit_name))
+            except IOError as e:
+                # NOTE(beisner) - race avoidance, file may not exist yet.
+                # https://bugs.launchpad.net/charm-helpers/+bug/1474030
+                self.log.debug('Attempt {} to get {} file mtime on {} '
+                               'failed\n{}'.format(tries, filename,
+                                                   unit_name, e))
+                time.sleep(retry_sleep_time)
+                tries += 1
+
+        if not file_mtime:
+            self.log.warn('Could not determine file mtime, assuming '
+                          'file does not exist')
+            return False
+
         if file_mtime >= mtime:
             self.log.debug('File mtime is newer than provided mtime '
-                           '(%s >= %s)' % (file_mtime, mtime))
+                           '(%s >= %s) on %s (OK)' % (file_mtime,
+                                                      mtime, unit_name))
             return True
         else:
-            self.log.warn('File mtime %s is older than provided mtime %s'
-                          % (file_mtime, mtime))
+            self.log.warn('File mtime is older than provided mtime'
+                          '(%s < on %s) on %s' % (file_mtime,
+                                                  mtime, unit_name))
             return False
 
     def validate_service_config_changed(self, sentry_unit, mtime, service,
                                         filename, pgrep_full=None,
-                                        sleep_time=20, retry_count=2,
-                                        retry_sleep_time=30):
+                                        sleep_time=20, retry_count=30,
+                                        retry_sleep_time=10):
         """Check service and file were updated after mtime
 
         Args:
@@ -456,7 +485,9 @@
             sentry_unit,
             filename,
             mtime,
-            sleep_time=0)
+            sleep_time=sleep_time,
+            retry_count=retry_count,
+            retry_sleep_time=retry_sleep_time)
 
         return service_restart and config_update
 

=== modified file 'tests/charmhelpers/contrib/openstack/amulet/utils.py'
--- tests/charmhelpers/contrib/openstack/amulet/utils.py	2015-09-15 09:06:11 +0000
+++ tests/charmhelpers/contrib/openstack/amulet/utils.py	2015-09-30 08:52:41 +0000
@@ -752,7 +752,7 @@
             self.log.debug('SSL is enabled  @{}:{} '
                            '({})'.format(host, port, unit_name))
             return True
-        elif not port and not conf_ssl:
+        elif not conf_ssl:
             self.log.debug('SSL not enabled @{}:{} '
                            '({})'.format(host, port, unit_name))
             return False

=== modified file 'tests/charmhelpers/core/hookenv.py'
--- tests/charmhelpers/core/hookenv.py	2015-09-02 09:32:16 +0000
+++ tests/charmhelpers/core/hookenv.py	2015-09-30 08:52:41 +0000
@@ -623,6 +623,38 @@
     return unit_get('private-address')
 
 
+@cached
+def storage_get(attribute="", storage_id=""):
+    """Get storage attributes"""
+    _args = ['storage-get', '--format=json']
+    if storage_id:
+        _args.extend(('-s', storage_id))
+    if attribute:
+        _args.append(attribute)
+    try:
+        return json.loads(subprocess.check_output(_args).decode('UTF-8'))
+    except ValueError:
+        return None
+
+
+@cached
+def storage_list(storage_name=""):
+    """List the storage IDs for the unit"""
+    _args = ['storage-list', '--format=json']
+    if storage_name:
+        _args.append(storage_name)
+    try:
+        return json.loads(subprocess.check_output(_args).decode('UTF-8'))
+    except ValueError:
+        return None
+    except OSError as e:
+        import errno
+        if e.errno == errno.ENOENT:
+            # storage-list does not exist
+            return []
+        raise
+
+
 class UnregisteredHookError(Exception):
     """Raised when an undefined hook is called"""
     pass