Merge lp:~adam-collard/charms/trusty/swift-storage/guard-paused-unit-service-restarts into lp:~openstack-charmers-archive/charms/trusty/swift-storage/next

[Adam Collard]

This is a follow up branch to https://code.launchpad.net/~adam-collard/charms/trusty/swift-storage/add-pause-resume-actions/+merge/268233 to prevent config-changed from blindly (re)starting services when the unit is paused.

Normal flow is pause(), (optionally) change config -> changes should be persisted on disk, but nothing started. resume() -> services started.

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

charm_lint_check #8938 swift-storage-next for adam-collard mp269528
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/8938/

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

charm_unit_test #8258 swift-storage-next for adam-collard mp269528
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/8258/

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

charm_amulet_test #6082 swift-storage-next for adam-collard mp269528
    AMULET FAIL: amulet-test failed

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

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

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

charm_lint_check #8939 swift-storage-next for adam-collard mp269528
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/8939/

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

charm_unit_test #8259 swift-storage-next for adam-collard mp269528
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/8259/

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

charm_amulet_test #6083 swift-storage-next for adam-collard mp269528
    AMULET FAIL: amulet-test failed

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

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

[lp:~adam-collard/charms/trusty/swift-storage/guard-paused-unit-service-restarts]

97. By Adam Collard on 2015-09-02

Expand list-comp to full for-loop, use SWIFT_SVCS instead of ACCOUNT + CONTAINER + OBJECT (ack's review)

Unmerged revisions

=== modified file 'charmhelpers/contrib/network/ip.py'
--- charmhelpers/contrib/network/ip.py	2015-03-04 09:54:59 +0000
+++ charmhelpers/contrib/network/ip.py	2015-09-01 16:09:33 +0000
@@ -435,8 +435,12 @@
 
         rev = dns.reversename.from_address(address)
         result = ns_query(rev)
+
         if not result:
-            return None
+            try:
+                result = socket.gethostbyaddr(address)[0]
+            except:
+                return None
     else:
         result = address
 
 
=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2015-08-17 13:44:42 +0000
+++ charmhelpers/contrib/openstack/utils.py	2015-09-01 16:09:33 +0000
@@ -1,5 +1,3 @@
-#!/usr/bin/python
-
 # Copyright 2014-2015 Canonical Limited.
 #
 # This file is part of charm-helpers.
@@ -195,9 +193,9 @@
         error_out(e)
 
 
-def get_os_version_codename(codename):
+def get_os_version_codename(codename, version_map=OPENSTACK_CODENAMES):
     '''Determine OpenStack version number from codename.'''
-    for k, v in six.iteritems(OPENSTACK_CODENAMES):
+    for k, v in six.iteritems(version_map):
         if v == codename:
             return k
     e = 'Could not derive OpenStack version for '\
@@ -429,7 +427,11 @@
     import apt_pkg as apt
     src = config('openstack-origin')
     cur_vers = get_os_version_package(package)
-    available_vers = get_os_version_install_source(src)
+    if "swift" in package:
+        codename = get_os_codename_install_source(src)
+        available_vers = get_os_version_codename(codename, SWIFT_CODENAMES)
+    else:
+        available_vers = get_os_version_install_source(src)
     apt.init()
     return apt.version_compare(available_vers, cur_vers) == 1
 
 
=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-08-17 13:44:42 +0000
+++ charmhelpers/core/hookenv.py	2015-09-01 16:09:33 +0000
@@ -767,21 +767,23 @@
 
 
 def status_get():
-    """Retrieve the previously set juju workload state
-
-    If the status-set command is not found then assume this is juju < 1.23 and
-    return 'unknown'
+    """Retrieve the previously set juju workload state and message
+
+    If the status-get command is not found then assume this is juju < 1.23 and
+    return 'unknown', ""
+
     """
-    cmd = ['status-get']
+    cmd = ['status-get', "--format=json", "--include-data"]
     try:
-        raw_status = subprocess.check_output(cmd, universal_newlines=True)
-        status = raw_status.rstrip()
-        return status
+        raw_status = subprocess.check_output(cmd)
     except OSError as e:
         if e.errno == errno.ENOENT:
-            return 'unknown'
+            return ('unknown', "")
         else:
             raise
+    else:
+        status = json.loads(raw_status.decode("UTF-8"))
+        return (status["status"], status["message"])
 
 
 def translate_exc(from_exc, to_exc):
 
=== modified file 'hooks/swift_storage_hooks.py'
--- hooks/swift_storage_hooks.py	2015-07-17 09:57:19 +0000
+++ hooks/swift_storage_hooks.py	2015-09-01 16:09:33 +0000
@@ -18,6 +18,8 @@
     setup_rsync,
 )
 
+from lib.misc_utils import pause_aware_restart_on_change
+
 from charmhelpers.core.hookenv import (
     Hooks, UnregisteredHookError,
     config,
@@ -32,7 +34,7 @@
     apt_update,
     filter_installed_packages
 )
-from charmhelpers.core.host import restart_on_change, rsync
+from charmhelpers.core.host import rsync
 from charmhelpers.payload.execd import execd_preinstall
 
 from charmhelpers.contrib.openstack.utils import (
@@ -63,7 +65,7 @@
 
 
 @hooks.hook('config-changed')
-@restart_on_change(RESTART_MAP)
+@pause_aware_restart_on_change(RESTART_MAP)
 def config_changed():
     if config('prefer-ipv6'):
         assert_charm_supports_ipv6()
@@ -104,7 +106,7 @@
 
 
 @hooks.hook('swift-storage-relation-changed')
-@restart_on_change(RESTART_MAP)
+@pause_aware_restart_on_change(RESTART_MAP)
 def swift_storage_relation_changed():
     rings_url = relation_get('rings_url')
     swift_hash = relation_get('swift_hash')
 
=== modified file 'lib/misc_utils.py'
--- lib/misc_utils.py	2015-07-17 10:51:25 +0000
+++ lib/misc_utils.py	2015-09-01 16:09:33 +0000
@@ -16,12 +16,14 @@
 from charmhelpers.core.host import (
     mounts,
     umount,
+    restart_on_change,
 )
 
 from charmhelpers.core.hookenv import (
     log,
     INFO,
     ERROR,
+    status_get,
 )
 
 DEFAULT_LOOPBACK_SIZE = '5G'
@@ -83,3 +85,19 @@
         remove_lvm_physical_volume(block_device)
     else:
         zap_disk(block_device)
+
+
+def is_paused(status_get=status_get):
+    """Is the unit paused?"""
+    status, message = status_get()
+    return status == "maintenance" and message.startswith("Paused")
+
+
+def pause_aware_restart_on_change(restart_map):
+    """Avoids restarting services if config changes when unit is paused."""
+    def wrapper(f):
+        if is_paused():
+            return f
+        else:
+            return restart_on_change(restart_map)(f)
+    return wrapper
 
=== modified file 'lib/swift_storage_utils.py'
--- lib/swift_storage_utils.py	2015-08-11 13:59:31 +0000
+++ lib/swift_storage_utils.py	2015-09-01 16:09:33 +0000
@@ -10,6 +10,7 @@
 from misc_utils import (
     ensure_block_device,
     clean_storage,
+    is_paused
 )
 
 from swift_storage_context import (
@@ -151,8 +152,9 @@
     apt_upgrade(options=dpkg_opts, fatal=True, dist=True)
     configs.set_release(openstack_release=new_os_rel)
     configs.write_all()
-    [service_restart(svc) for svc in
-     (ACCOUNT_SVCS + CONTAINER_SVCS + OBJECT_SVCS)]
+    if not is_paused():
+        [service_restart(svc) for svc in
+         (ACCOUNT_SVCS + CONTAINER_SVCS + OBJECT_SVCS)]
 
 
 def _is_storage_ready(partition):
 
=== modified file 'tests/basic_deployment.py'
--- tests/basic_deployment.py	2015-08-22 04:12:14 +0000
+++ tests/basic_deployment.py	2015-09-01 16:09:33 +0000
@@ -429,19 +429,10 @@
 
         # Config file affected by juju set config change, and
         # services which are expected to restart upon config change
-        services = {'swift-account-server': 'account-server.conf',
-                    'swift-account-auditor': 'account-server.conf',
-                    'swift-account-reaper': 'account-server.conf',
-                    'swift-account-replicator': 'account-server.conf',
-                    'swift-container-server': 'container-server.conf',
-                    'swift-container-auditor': 'container-server.conf',
-                    'swift-container-replicator': 'container-server.conf',
-                    'swift-container-updater': 'container-server.conf',
-                    'swift-object-server': 'object-server.conf',
+        services = {'swift-object-server': 'object-server.conf',
                     'swift-object-auditor': 'object-server.conf',
                     'swift-object-replicator': 'object-server.conf',
-                    'swift-object-updater': 'object-server.conf',
-                    'swift-container-sync': 'container-server.conf'}
+                    'swift-object-updater': 'object-server.conf'}
 
         # Make config change, check for service restarts
         u.log.debug('Making config change on {}...'.format(juju_service))
@@ -510,3 +501,51 @@
         u.log.debug('Checking pause/resume actions...')
         self._test_pause()
         self._test_resume()
+
+    def test_920_no_restart_on_config_change_when_paused(self):
+        """Verify that the specified services are not restarted when the config
+           is changed and the unit is paused."""
+        u.log.info('Checking that system services do not get restarted  '
+                   'when charm config changes but unit is paused...')
+        sentry = self.swift_storage_sentry
+        juju_service = 'swift-storage'
+
+        # Expected default and alternate values
+        set_default = {'object-server-threads-per-disk': '4'}
+        set_alternate = {'object-server-threads-per-disk': '2'}
+
+        services = ['swift-account-server',
+                    'swift-account-auditor',
+                    'swift-account-reaper',
+                    'swift-account-replicator',
+                    'swift-container-server',
+                    'swift-container-auditor',
+                    'swift-container-replicator',
+                    'swift-container-updater',
+                    'swift-object-server',
+                    'swift-object-auditor',
+                    'swift-object-replicator',
+                    'swift-object-updater',
+                    'swift-container-sync']
+        if self._get_openstack_release() < self.precise_icehouse:
+            services.remove('swift-container-sync')
+
+        # Pause the unit
+        u.log.debug('Pausing the unit...')
+        pause_action_id = u.run_action(sentry, "pause")
+        assert u.wait_on_action(pause_action_id), "Pause action failed."
+
+        # Make config change, check for service restarts
+        u.log.debug('Making config change on {}...'.format(juju_service))
+        self.d.configure(juju_service, set_alternate)
+
+        for service in services:
+            u.log.debug("Checking that service didn't start while "
+                        "paused: {}".format(service))
+            # No explicit assert because get_process_id_list will do it for us
+            u.get_process_id_list(
+                sentry, service, expect_success=False)
+
+        self.d.configure(juju_service, set_default)
+        resume_action_id = u.run_action(sentry, "resume")
+        assert u.wait_on_action(resume_action_id), "Resume action failed."