Merge "Add 'remove-devices' action."

actions.yaml

@@ -33,3 +33,28 @@ openstack-upgrade:
     set to True.
 diskusage:
   description: Run swift-recon -d on the specified unit.  Returns values in GB.
+remove-devices:
+  description: |
+    Removes the device(s) from the ring. This should normally just be used for
+    a device that has failed. For a device you wish to decommission, it's best
+    to set its weight to 0, wait for it to drain all its data, then use this
+    remove-from-ring action.
+  params:
+    ring:
+      type: string
+      description: |
+        Swift ring to remove the device(s) from. Valid options are 'account',
+        'container', 'object' or 'all'.
+    search-value:
+      type: string
+      description: |
+        The search-value can be of the form:
+        .
+        d<device_id>r<region>z<zone>-<ip>:<port>R<r_ip>:<r_port>/
+         <device_name>_<meta>
+        .
+        Where <r_ip> and <r_port> are replication ip and port. Any part is
+        optional, but you must include at least one part.
+  required:
+    - ring
+    - search-value

actions/actions.py

@@ -38,6 +38,7 @@ _add_path(_parent)
 from charmhelpers.core.host import service_pause, service_resume
 from charmhelpers.core.hookenv import (
     action_fail,
+    action_get,
     action_set,
 )
 from charmhelpers.contrib.openstack.utils import (
@@ -45,7 +46,13 @@ from charmhelpers.contrib.openstack.utils import (
     clear_unit_paused,
 )
 from hooks.swift_hooks import CONFIGS
-from lib.swift_utils import assess_status, services
+from lib.swift_utils import (
+    assess_status,
+    balance_rings,
+    remove_from_ring,
+    services,
+    SWIFT_CONF_DIR,
+)
 
 
 def get_action_parser(actions_yaml_path, action_name,
@@ -113,9 +120,37 @@ def diskusage(args):
         raise
 
 
+def remove_devices(args):
+    """ Removes the device(s) from the ring(s).
+
+    Removes the device(s) from the ring(s) based on the search pattern.
+
+    :raises SwiftProxyCharmException: if pattern action_get('search-value')
+        doesn't match any device in the ring.
+    """
+    rings_valid = ['account', 'container', 'object', 'all']
+    rings_to_update = []
+    ring = action_get('ring')
+    if ring not in rings_valid:
+        action_fail("Invalid ring name '{}'. Should be one of: {}".format(
+            ring, ', '.join(rings_valid)))
+        return
+    if ring == 'all':
+        rings_to_update.extend(['account', 'container', 'object'])
+    else:
+        rings_to_update.append(ring)
+    for ring_to_update in rings_to_update:
+        ring_to_update_builder = ring_to_update + '.builder'
+        ring_to_update_path = os.path.join(SWIFT_CONF_DIR,
+                                           ring_to_update_builder)
+        remove_from_ring(ring_to_update_path, action_get('search-value'))
+    balance_rings()
+
+
 # A dictionary of all the defined actions to callables (which take
 # parsed arguments).
-ACTIONS = {"pause": pause, "resume": resume, 'diskusage': diskusage}
+ACTIONS = {"pause": pause, "resume": resume, 'diskusage': diskusage,
+           'remove-devices': remove_devices}
 
 
 def main(argv):

actions/remove-devices

@@ -0,0 +1 @@
+actions.py

lib/swift_utils.py

@@ -504,6 +504,26 @@ def add_to_ring(ring_path, node):
     log(msg, level=INFO)
 
 
+def remove_from_ring(ring_path, search_value):
+    """ Removes the device(s) from the ring.
+
+    Removes the device(s) from the ring based on the search pattern.
+
+    :param ring_path: path to the ring
+    :type ring_path: str
+    :param search_value: search pattern
+    :type search_value: str
+    :raises: SwiftProxyCharmException
+    """
+    cmd = ['swift-ring-builder', ring_path, 'remove', search_value]
+    try:
+        subprocess.check_call(cmd)
+    except subprocess.CalledProcessError as e:
+        raise SwiftProxyCharmException(
+            "Failed to remove device(s) for {} pattern on {}. Output: "
+            "{}".format(search_value, ring_path, e))
+
+
 def _ring_port(ring_path, node):
     """Determine correct port from relation settings for a given ring file."""
     for name in ['account', 'object', 'container']:

unit_tests/test_actions.py

@@ -311,3 +311,27 @@ class DiskUsageTestCase(CharmTestCase):
         self.check_output.return_value = self.TEST_RECON_OUTPUT
         actions.actions.diskusage([])
         self.action_set.assert_called_once_with({'output': self.TEST_RESULT})
+
+
+class RemoveDevicesTestCase(CharmTestCase):
+
+    def setUp(self):
+        super(RemoveDevicesTestCase, self).setUp(
+            actions.actions, ["action_fail",
+                              "action_get",
+                              "remove_from_ring",
+                              "balance_rings"])
+
+    def test_ring_valid(self):
+        self.action_get.side_effect = ['account', 'd1']
+        actions.actions.remove_devices([])
+        self.remove_from_ring.assert_called_once_with(
+            '/etc/swift/account.builder', 'd1')
+        self.balance_rings.assert_called_once()
+
+    def test_ring_invalid(self):
+        self.action_get.side_effect = ['other', 'd1']
+        actions.actions.remove_devices([])
+        self.action_fail.assert_called()
+        self.remove_from_ring.assert_not_called()
+        self.balance_rings.assert_not_called()

unit_tests/test_swift_utils.py

@@ -429,6 +429,14 @@ class SwiftUtilsTestCase(unittest.TestCase):
             'weight': 100
         })
 
+    @mock.patch.object(subprocess, 'check_call')
+    def test_remove_from_ring(self, check_call):
+        swift_utils.remove_from_ring('/etc/swift/account.builder', 'd1')
+        check_call.assert_called_once_with(['swift-ring-builder',
+                                            '/etc/swift/account.builder',
+                                            'remove',
+                                            'd1'])
+
     @mock.patch('lib.swift_utils.config')
     @mock.patch('lib.swift_utils.set_os_workload_status')
     @mock.patch('lib.swift_utils.SwiftRingContext.__call__')