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/

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

charm_lint_check #9227 swift-storage-next for adam-collard mp269860
    LINT OK: passed

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

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

charm_unit_test #8528 swift-storage-next for adam-collard mp269860
    UNIT OK: passed

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

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

charm_amulet_test #6189 swift-storage-next for adam-collard mp269860
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6189/

[Alberto Donato (ack)]

Looks good, +1

one minor comment inline.

review: Approve

[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)

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

charm_unit_test #8531 swift-storage-next for adam-collard mp269860
    UNIT OK: passed

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

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

charm_lint_check #9230 swift-storage-next for adam-collard mp269860
    LINT OK: passed

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

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

charm_amulet_test #6192 swift-storage-next for adam-collard mp269860
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/6192/

[Liam Young (gnuoy)]

Approve

review: Approve

=== 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-02 10:52:42 +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-02 10:52:42 +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-02 10:52:42 +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():
+        for service in SWIFT_SVCS:
+            service_restart(service)
 
 
 def _is_storage_ready(partition):

=== modified file 'tests/basic_deployment.py'
--- tests/basic_deployment.py	2015-09-02 10:52:42 +0000
+++ tests/basic_deployment.py	2015-09-02 10:52:42 +0000
@@ -501,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."