diff --git a/fuelclient/cli/actions/node.py b/fuelclient/cli/actions/node.py index 7f7caf41..858b81e2 100644 --- a/fuelclient/cli/actions/node.py +++ b/fuelclient/cli/actions/node.py @@ -20,8 +20,7 @@ from fuelclient.cli.actions.base import check_all from fuelclient.cli.actions.base import check_any import fuelclient.cli.arguments as Args from fuelclient.cli.arguments import group -from fuelclient.cli.error import ActionException -from fuelclient.cli.error import ArgumentException +from fuelclient.cli import error from fuelclient.cli.formatting import format_table from fuelclient.objects.environment import Environment from fuelclient.objects.node import Node @@ -129,7 +128,7 @@ class NodeAction(Action): if params.all: env.unassign_all() else: - raise ArgumentException( + raise error.ArgumentException( "You have to select which nodes to remove " "with --node-id. Try --all for removing all nodes." ) @@ -218,7 +217,7 @@ class NodeAction(Action): def get_env_id(self, node_collection): env_ids = set(n.env_id for n in node_collection) if len(env_ids) != 1: - raise ActionException( + raise error.ActionException( "Inputed nodes assigned to multiple environments!") else: return env_ids.pop() @@ -299,11 +298,27 @@ class NodeAction(Action): """To delete nodes from fuel db: fuel node --node-id 1 --delete-from-db fuel node --node-id 1 2 --delete-from-db + (this works only for offline nodes) + fuel node --node-id 1 --delete-from-db --force + (this forces deletion of nodes with status other than offline) """ + if not params.force: + node_collection = NodeCollection.init_with_ids(params.node) + + online_nodes = [node for node in node_collection.data + if node['online']] + + if online_nodes: + raise error.ActionException( + "Nodes with ids {0} cannot be deleted from cluster " + "because they are online. You might want to use the " + "--force option.".format( + [node['id'] for node in online_nodes])) + NodeCollection.delete_by_ids(params.node) self.serializer.print_to_output( {}, - "Nodes with id {0} have been deleted from Fuel db.".format( + "Nodes with ids {0} have been deleted from Fuel db.".format( params.node) ) diff --git a/fuelclient/tests/base.py b/fuelclient/tests/base.py index c90a1e82..1959512c 100644 --- a/fuelclient/tests/base.py +++ b/fuelclient/tests/base.py @@ -133,7 +133,7 @@ class BaseTestCase(UnitTestCase): cmd = 'tox -evenv -- manage.py loaddata %s' % file_path cls.run_command(cmd, cwd=cls.nailgun_root) - def run_cli_command(self, command_line, check_errors=False): + def run_cli_command(self, command_line, check_errors=True): modified_env = os.environ.copy() command_args = [" ".join(('fuel', command_line))] process_handle = subprocess.Popen( @@ -147,7 +147,7 @@ class BaseTestCase(UnitTestCase): result = CliExectutionResult(process_handle, out, err) log.debug("command_args: '%s',stdout: '%s', stderr: '%s'", command_args[0], out, err) - if not check_errors: + if check_errors: if not result.is_return_code_zero or result.has_errors: self.fail(err) return result @@ -157,14 +157,18 @@ class BaseTestCase(UnitTestCase): self.run_cli_command(command, **kwargs) def check_if_required(self, command): - call = self.run_cli_command(command, check_errors=True) + call = self.run_cli_command(command, check_errors=False) #should not work without env id self.assertIn("required", call.stderr) - def check_for_stdout(self, command, msg): - call = self.run_cli_command(command) + def check_for_stdout(self, command, msg, check_errors=True): + call = self.run_cli_command(command, check_errors=check_errors) self.assertEqual(call.stdout, msg) + def check_for_stderr(self, command, msg, check_errors=True): + call = self.run_cli_command(command, check_errors=check_errors) + self.assertEqual(call.stderr, msg) + def check_all_in_msg(self, command, substrings, **kwargs): output = self.run_cli_command(command, **kwargs) for substring in substrings: diff --git a/fuelclient/tests/test_client.py b/fuelclient/tests/test_client.py index 46a23d8d..601673d3 100644 --- a/fuelclient/tests/test_client.py +++ b/fuelclient/tests/test_client.py @@ -100,19 +100,48 @@ class TestHandlers(base.BaseTestCase): def test_check_wrong_server(self): os.environ["SERVER_ADDRESS"] = "0" - result = self.run_cli_command("-h", check_errors=True) + result = self.run_cli_command("-h") self.assertEqual(result.stderr, '') del os.environ["SERVER_ADDRESS"] - def test_destroy_node(self): + def test_error_when_destroying_online_node(self): + self.load_data_to_nailgun_server() + self.run_cli_commands(( + "env create --name=NewEnv --release=1", + "--env-id=1 node set --node 1 --role=controller" + ), check_errors=False) + msg = ("Nodes with ids [1] cannot be deleted from cluster because " + "they are online. You might want to use the --force option.\n") + self.check_for_stderr( + "node --node 1 --delete-from-db", + msg, + check_errors=False + ) + + def test_force_destroy_online_node(self): self.load_data_to_nailgun_server() self.run_cli_commands(( "env create --name=NewEnv --release=1", "--env-id=1 node set --node 1 --role=controller" )) - msg = ("Nodes with id [1] have been deleted from Fuel db.\n") + msg = ("Nodes with ids [1] have been deleted from Fuel db.\n") self.check_for_stdout( - "node --node 1 --delete-from-db", + "node --node 1 --delete-from-db --force", + msg + ) + + def test_destroy_offline_node(self): + + self.load_data_to_nailgun_server() + node_id = 4 + self.run_cli_commands(( + "env create --name=NewEnv --release=1", + "--env-id=1 node set --node {0} --role=controller".format(node_id) + )) + msg = ("Nodes with ids [{0}] have been deleted from Fuel db.\n".format( + node_id)) + self.check_for_stdout( + "node --node {0} --delete-from-db".format(node_id), msg ) @@ -122,9 +151,9 @@ class TestHandlers(base.BaseTestCase): "env create --name=NewEnv --release=1", "--env-id=1 node set --node 1 2 --role=controller" )) - msg = ("Nodes with id [1, 2] have been deleted from Fuel db.\n") + msg = ("Nodes with ids [1, 2] have been deleted from Fuel db.\n") self.check_for_stdout( - "node --node 1 2 --delete-from-db", + "node --node 1 2 --delete-from-db --force", msg ) @@ -144,7 +173,7 @@ class TestHandlers(base.BaseTestCase): "GET http://127.0.0.1", "/api/v1/tasks/1/" ], - check_errors=True + check_errors=False ) self.check_all_in_msg( "task --task-id 1 --delete --debug", @@ -152,7 +181,7 @@ class TestHandlers(base.BaseTestCase): "DELETE http://127.0.0.1", "/api/v1/tasks/1/?force=0" ], - check_errors=True + check_errors=False ) self.check_all_in_msg( "task --task-id 1 --delete --force --debug", @@ -160,7 +189,7 @@ class TestHandlers(base.BaseTestCase): "DELETE http://127.0.0.1", "/api/v1/tasks/1/?force=1" ], - check_errors=True + check_errors=False ) self.check_all_in_msg( "task --tid 1 --delete --debug", @@ -168,7 +197,7 @@ class TestHandlers(base.BaseTestCase): "DELETE http://127.0.0.1", "/api/v1/tasks/1/?force=0" ], - check_errors=True + check_errors=False ) def test_get_release_list_without_errors(self): @@ -181,7 +210,7 @@ class TestUserActions(base.BaseTestCase): def test_change_password_params(self): cmd = "user change-password" msg = "Expect password [--newpass NEWPASS]" - result = self.run_cli_command(cmd, check_errors=True) + result = self.run_cli_command(cmd, check_errors=False) self.assertTrue(msg, result)