Merge pull request #908 from untergeek/feature/906

Allow unsetting of shard allocation w/ empty value

curator/actions.py

@@ -160,7 +160,8 @@ def __init__(self, ilo, key=None, value=None, allocation_type='require',
             assigned to at least some of your nodes to have any effect.
         :arg value: An arbitrary metadata attribute value.  Must correspond to
             values associated with `key` assigned to at least some of your nodes
-            to have any effect.
+            to have any effect. If a `None` value is provided, it will remove
+            any setting associated with that `key`.
         :arg allocation_type: Type of allocation to apply. Default is `require`
         :arg wait_for_completion: Wait (or not) for the operation
             to complete before returning.  (default: `False`)
@@ -174,8 +175,6 @@ def __init__(self, ilo, key=None, value=None, allocation_type='require',
         verify_index_list(ilo)
         if not key:
             raise MissingArgument('No value for "key" provided')
-        if not value:
-            raise MissingArgument('No value for "value" provided')
         if allocation_type not in ['require', 'include', 'exclude']:
             raise ValueError(
                 '{0} is an invalid allocation_type.  Must be one of "require", '

curator/validators/options.py

@@ -128,7 +128,7 @@ def timeout_override(action):
         }
 
 def value():
-    return { Required('value'): Any(str, unicode) }
+    return { Required('value', default=None): Any(str, unicode, None) }
 
 def wait_for_active_shards():
     return {

docs/Changelog.rst

@@ -8,6 +8,8 @@ Changelog
 
 **New Features**
 
+  * Allow allocation action to unset a key/value pair by using an empty value.
+    Requested in #906. (untergeek)
   * Added support for the Rollover API. Requested in #898, and by countless
     others.
   * Added ``warn_if_no_indices`` option for ``alias`` action in response to

docs/asciidoc/actions.asciidoc

@@ -134,13 +134,14 @@ Required settings
 ~~~~~~~~~~~~~~~~~
 
 * <<option_key,key>> (required)
-* <<option_value,value>> (required)
 
 [float]
 Optional settings
 ~~~~~~~~~~~~~~~~~
 * <<option_allocation_type,allocation_type>> (has a default value which can
     optionally be changed)
+* <<option_value,value>> (has a default value which can
+    optionally be changed)
 * <<option_wfc,wait_for_completion>> (has a default value which can optionally
     be changed)
 * <<option_ignore_empty,ignore_empty_list>> (can override the default)

docs/asciidoc/options.asciidoc

@@ -455,13 +455,13 @@ from the client-defined default would be desirable.
 [[option_value]]
 == value
 
-NOTE: This setting is required when using the <<allocation,allocation action>>
-    or the <<cluster_routing,cluster_routing action>>.
+NOTE: This setting is optional when using the <<allocation,allocation action>>
+    and required when using the <<cluster_routing,cluster_routing action>>.
 
 === <<allocation,allocation>> value
 
 For the <<allocation,allocation action>>, the value of this setting should
-correspond to a node setting on one or more nodes in your cluster.
+correspond to a node setting on one or more nodes in your cluster
 
 For example, you might have set
 
@@ -476,6 +476,42 @@ the special attribute names `_ip`, `_name`, `_id`, or `_host` for
 <<option_key,key>>, value can match the one of the node ip addresses, names,
 ids, or host names, respectively.
 
+NOTE: To remove a routing allocation, the value of this setting should be left
+empty, or the `value` setting not even included as an option.
+
+For example, you might have set
+
+[source,sh]
+-----------
+PUT test/_settings
+{
+  "index.routing.allocation.exclude.size": "small"
+}
+-----------
+
+to keep index `test` from allocating shards on nodes that have `node.tag: small`.
+To remove this shard routing allocation setting, you might use an action file
+similar to this:
+
+[source,yaml]
+-----------
+---
+  actions:
+    1:
+      action: allocation
+      description: ->
+        Unset 'index.routing.allocation.exclude.size' for index 'test' by
+        passing an empty value.
+      options:
+        key: size
+        value:
+        allocation_type: exclude
+      filters:
+      - filtertype: pattern
+        kind: regex
+        value: '^test$'
+-----------
+
 === <<cluster_routing,cluster_routing>> value
 
 For the <<cluster_routing,cluster_routing action>>, the acceptable values for

requirements.txt

@@ -1,5 +1,5 @@
 urllib3>=1.8.3
-elasticsearch>=2.4.0,<3.0.0
+elasticsearch>=5.0.0,<6.0.0
 click>=6.0
 pyyaml>=3.10
 voluptuous>=0.9.3

test/integration/test_allocation.py

@@ -84,6 +84,36 @@ def test_exclude(self):
             self.client.indices.get_settings(index='my_index')['my_index']['settings']['index']['routing']['allocation'][at][key])
         self.assertNotIn('routing',
             self.client.indices.get_settings(index='not_my_index')['not_my_index']['settings']['index'])
+    def test_remove_exclude_with_none_value(self):
+        key = 'tag'
+        value = ''
+        at = 'exclude'
+        self.write_config(
+            self.args['configfile'], testvars.client_config.format(host, port))
+        self.write_config(self.args['actionfile'],
+            testvars.allocation_test.format(key, value, at, False))
+        self.create_index('my_index')
+        self.create_index('not_my_index')
+        # Put a setting in place before we start the test.
+        self.client.indices.put_settings(
+            index='my_index', 
+            body={'index.routing.allocation.{0}.{1}'.format(at, key): 'bar'}
+        )
+        # Ensure we _have_ it here first.
+        self.assertEquals('bar',
+            self.client.indices.get_settings(index='my_index')['my_index']['settings']['index']['routing']['allocation'][at][key])
+        test = clicktest.CliRunner()
+        result = test.invoke(
+                    curator.cli,
+                    [
+                        '--config', self.args['configfile'],
+                        self.args['actionfile']
+                    ],
+                    )
+        self.assertNotIn('routing',
+            self.client.indices.get_settings(index='my_index')['my_index']['settings']['index'])
+        self.assertNotIn('routing',
+            self.client.indices.get_settings(index='not_my_index')['not_my_index']['settings']['index'])
     def test_invalid_allocation_type(self):
         key = 'tag'
         value = 'value'

test/unit/test_action_allocation.py

@@ -26,15 +26,6 @@ def test_create_body_no_key(self):
         client.indices.stats.return_value = testvars.stats_one
         ilo = curator.IndexList(client)
         self.assertRaises(curator.MissingArgument, curator.Allocation, ilo)
-    def test_create_body_no_value(self):
-        client = Mock()
-        client.info.return_value = {'version': {'number': '2.4.1'} }
-        client.indices.get_settings.return_value = testvars.settings_one
-        client.cluster.state.return_value = testvars.clu_state_one
-        client.indices.stats.return_value = testvars.stats_one
-        ilo = curator.IndexList(client)
-        self.assertRaises(curator.MissingArgument,
-            curator.Allocation, ilo, key='key')
     def test_create_body_invalid_allocation_type(self):
         client = Mock()
         client.info.return_value = {'version': {'number': '2.4.1'} }