openstack/python-fuelclient
Code Issues Proposed changes

Warn about deleting from DB node that is not offline

Browse Source 
The --force parameter can be used to force the removal. Otherwise error is
printed and script exits without doing anything.

Usage:

fuel node --node-id 1 2 --delete-from-db

fuel node --node-id 1 --delete-from-db --force

Also, fixed illogical use of check_errors in run_cli_command function.

Implements blueprint node-removal-task

Closes-Bug: #1435281
Change-Id: I0c9e27450c09eb7ea6224ffc30c9284427cd5d1c
This commit is contained in:
Przemyslaw Kaminski
2015-03-20 10:08:15 +01:00
parent b223dcaf5f
commit 3624051242
3 changed files with 69 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
fuelclient/cli/actions/node.py
View File

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

14
fuelclient/tests/base.py
View File

@@ -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:

51
fuelclient/tests/test_client.py
View File

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