Volans has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/366736 )
Change subject: Transports: convert hosts to ClusterShell's NodeSet
......................................................................
Transports: convert hosts to ClusterShell's NodeSet
Breaking change in the API.
- in preparation for the multi-query support, start moving the
transports to accept a ClusterShell's NodeSet instead of a list of
nodes. With the new multi-query support the backends too will return
only NodeSets.
Bug: T170394
Change-Id: I501b59d1dc2d9bd0c975de7ab5ccfe421c27c9ea
---
M cumin/cli.py
M cumin/tests/unit/transports/test_clustershell.py
M cumin/tests/unit/transports/test_init.py
M cumin/transports/__init__.py
M cumin/transports/clustershell.py
5 files changed, 26 insertions(+), 23 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/software/cumin
refs/changes/36/366736/1
diff --git a/cumin/cli.py b/cumin/cli.py
index 49b023c..10f5a91 100644
--- a/cumin/cli.py
+++ b/cumin/cli.py
@@ -310,7 +310,7 @@
return 0
worker = transport.Transport.new(config, logger)
- worker.hosts = hosts
+ worker.hosts = NodeSet.fromlist(hosts)
if args.timeout is not None:
worker.commands = [transports.Command(command, timeout=args.timeout)
for command in args.commands]
diff --git a/cumin/tests/unit/transports/test_clustershell.py
b/cumin/tests/unit/transports/test_clustershell.py
index b6f41b5..44dce32 100644
--- a/cumin/tests/unit/transports/test_clustershell.py
+++ b/cumin/tests/unit/transports/test_clustershell.py
@@ -34,8 +34,7 @@
'fanout': 3}}
self.worker = clustershell.worker_class(self.config)
- self.nodes = ['node1', 'node2']
- self.nodes_set = clustershell.NodeSet.NodeSet.fromlist(self.nodes)
+ self.nodes = clustershell.NodeSet.NodeSet('node[1-2]')
self.commands = [Command('command1'), Command('command2', ok_codes=[0,
100], timeout=5)]
self.task_self = task_self
# Mock default handlers
@@ -62,7 +61,7 @@
self.worker.handler = 'sync'
self.worker.execute()
self.worker.task.shell.assert_called_once_with(
- 'command1', nodes=self.nodes_set,
handler=self.worker._handler_instance, timeout=None)
+ 'command1', nodes=self.nodes,
handler=self.worker._handler_instance, timeout=None)
assert clustershell.DEFAULT_HANDLERS['sync'].called
def test_execute_default_async_handler(self):
@@ -70,7 +69,7 @@
self.worker.handler = 'async'
self.worker.execute()
self.worker.task.shell.assert_called_once_with(
- 'command1', nodes=self.nodes_set,
handler=self.worker._handler_instance, timeout=None)
+ 'command1', nodes=self.nodes,
handler=self.worker._handler_instance, timeout=None)
assert clustershell.DEFAULT_HANDLERS['async'].called
def test_execute_timeout(self):
@@ -86,7 +85,7 @@
self.worker.execute()
assert isinstance(self.worker._handler_instance,
ConcreteBaseEventHandler)
self.worker.task.shell.assert_called_once_with(
- 'command1', nodes=self.nodes_set,
handler=self.worker._handler_instance, timeout=None)
+ 'command1', nodes=self.nodes,
handler=self.worker._handler_instance, timeout=None)
def test_execute_no_commands(self):
"""Calling execute() without commands should return without doing
anything."""
@@ -112,8 +111,7 @@
self.worker.batch_size = 1
self.worker.execute()
self.worker.task.shell.assert_called_once_with(
- 'command1', nodes=clustershell.NodeSet.NodeSet(self.nodes[0]),
handler=self.worker._handler_instance,
- timeout=None)
+ 'command1', nodes=self.nodes[:1],
handler=self.worker._handler_instance, timeout=None)
def test_get_results(self):
"""Calling get_results() should call ClusterShell iter_buffers with
the right parameters."""
@@ -173,12 +171,12 @@
def setup_method(self, *args): # pylint: disable=arguments-differ
"""Initialize default properties and instances."""
- self.nodes = ['node1', 'node2']
+ self.nodes = clustershell.NodeSet.NodeSet('node[1-2]')
self.commands = [Command('command1', ok_codes=[0, 100]),
Command('command2', timeout=5)]
self.worker = mock.MagicMock()
self.worker.current_node = 'node1'
self.worker.command = 'command1'
- self.worker.nodes = clustershell.NodeSet.NodeSet.fromlist(self.nodes)
+ self.worker.nodes = self.nodes
self.handler = None
self.args = args
@@ -220,7 +218,7 @@
def test_instantiation(self):
"""An instance of ConcreteBaseEventHandler should be an instance of
BaseEventHandler and initialize colorama."""
- assert sorted(self.handler.nodes.keys()) == self.nodes
+ assert sorted(self.handler.nodes.keys()) == list(self.nodes)
self.colorama.init.assert_called_once_with()
@mock.patch('cumin.transports.clustershell.tqdm')
@@ -258,14 +256,13 @@
@mock.patch('cumin.transports.clustershell.tqdm')
def test_ev_read_single_host(self, tqdm):
"""Calling ev_read() should print the worker message if matching a
single host."""
- nodes = ['node1']
- self.nodes = nodes
+ self.nodes = clustershell.NodeSet.NodeSet('node1')
self.handler = ConcreteBaseEventHandler(
- nodes, self.commands, batch_size=len(self.nodes), batch_sleep=0.0,
first_batch=self.nodes)
+ self.nodes, self.commands, batch_size=len(self.nodes),
batch_sleep=0.0, first_batch=self.nodes)
output = 'node1 output'
- self.worker.nodes = clustershell.NodeSet.NodeSet.fromlist(nodes)
- self.worker.current_node = nodes[0]
+ self.worker.nodes = self.nodes
+ self.worker.current_node = self.nodes[0]
self.worker.current_msg = output
self.handler.ev_read(self.worker)
tqdm.write.assert_has_calls([mock.call(output)])
diff --git a/cumin/tests/unit/transports/test_init.py
b/cumin/tests/unit/transports/test_init.py
index 603a72c..bfd762f 100644
--- a/cumin/tests/unit/transports/test_init.py
+++ b/cumin/tests/unit/transports/test_init.py
@@ -3,6 +3,8 @@
import mock
import pytest
+from ClusterShell.NodeSet import NodeSet
+
import cumin # noqa: F401 (dynamically used in TestCommand)
from cumin import transports
@@ -332,7 +334,7 @@
"""Initialize default properties and instances."""
# pylint: disable=attribute-defined-outside-init
self.worker = ConcreteBaseWorker({})
- self.hosts = ['node1', 'node2']
+ self.hosts = NodeSet('node[1-2]')
self.commands = [transports.Command('command1'),
transports.Command('command2')]
def test_hosts_getter(self):
@@ -343,7 +345,7 @@
def test_hosts_setter(self):
"""Raise WorkerError if trying to set it not to an iterable, set it
otherwise."""
- with pytest.raises(transports.WorkerError, match='hosts must be a
list'):
+ with pytest.raises(transports.WorkerError, match="must be an instance
of ClusterShell's NodeSet or None"):
self.worker.hosts = 'not-list'
self.worker.hosts = self.hosts
diff --git a/cumin/transports/__init__.py b/cumin/transports/__init__.py
index 22d5596..fb5c4fe 100644
--- a/cumin/transports/__init__.py
+++ b/cumin/transports/__init__.py
@@ -5,6 +5,8 @@
from abc import ABCMeta, abstractmethod, abstractproperty
+from ClusterShell.NodeSet import NodeSet
+
from cumin import CuminError
@@ -274,9 +276,11 @@
"""Setter for the hosts property with validation, raise WorkerError if
not valid.
Arguments:
- value -- a list of hosts to target for the execution of the commands
+ value -- a ClusterShell.NodeSet.NodeSet instance or None to reset it.
"""
- validate_list('hosts', value)
+ if value is not None and not isinstance(value, NodeSet):
+ raise_error('hosts', "must be an instance of ClusterShell's
NodeSet or None", value)
+
self._hosts = value
@property
diff --git a/cumin/transports/clustershell.py b/cumin/transports/clustershell.py
index 2dbb368..b0a69ff 100644
--- a/cumin/transports/clustershell.py
+++ b/cumin/transports/clustershell.py
@@ -42,7 +42,7 @@
raise RuntimeError('An EventHandler is mandatory.')
# Schedule only the first command for the first batch, the following
ones must be handled by the EventHandler
- first_batch = NodeSet.NodeSet.fromlist(self.hosts[:self.batch_size])
+ first_batch = self.hosts[:self.batch_size]
# Instantiate handler
self._handler_instance = self.handler( # pylint: disable=not-callable
@@ -50,7 +50,7 @@
batch_sleep=self.batch_sleep, logger=self.logger,
first_batch=first_batch)
self.logger.info("Executing commands {commands} on '{num}' hosts:
{hosts}".format(
- commands=self.commands, num=len(self.hosts),
hosts=NodeSet.NodeSet.fromlist(self.hosts)))
+ commands=self.commands, num=len(self.hosts), hosts=self.hosts))
self.task.shell(self.commands[0].command, nodes=first_batch,
handler=self._handler_instance,
timeout=self.commands[0].timeout)
@@ -130,7 +130,7 @@
If subclasses defines a self.pbar_ko tqdm progress bar, it will be
updated on timeout.
Arguments:
- nodes -- the list of nodes with which this worker was initiliazed.
+ nodes -- the ClusterShell's NodeSet with which this worker was
initiliazed.
commands -- the list of Command objects that has to be executed on the
nodes.
**kwargs -- optional additional keyword arguments that might be used
by classes that extend this base class.
"""
--
To view, visit https://gerrit.wikimedia.org/r/366736
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I501b59d1dc2d9bd0c975de7ab5ccfe421c27c9ea
Gerrit-PatchSet: 1
Gerrit-Project: operations/software/cumin
Gerrit-Branch: master
Gerrit-Owner: Volans <rcocci...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
