[PATCH 0/2 V2] Do not allow user disable/delete a network used by VM or template
From: Aline Manera <alinefm@br.ibm.com> v1 -> V2: - Update test cases (by Royce) - Make function names uniform (by Ming) Aline Manera (2): Do not allow user disable/delete a network used by VM or template UI: Disable stop/undefine buttons when network is in use docs/API.md | 2 ++ src/kimchi/control/networks.py | 1 + src/kimchi/i18n.py | 4 ++-- src/kimchi/mockmodel.py | 24 ++++++++++++++++++++++++ src/kimchi/model/networks.py | 31 +++++++++++++++++++++++++++---- tests/test_model.py | 18 +++++++++++++++--- tests/test_rest.py | 12 ++++++++++-- ui/js/src/kimchi.network.js | 27 ++++++++++++++++++++------- ui/pages/tabs/network.html.tmpl | 2 +- 9 files changed, 102 insertions(+), 19 deletions(-) -- 1.7.10.4
From: Aline Manera <alinefm@br.ibm.com> Add a new parameter to network resource to identify the network is in use by a template or VM, and do not allow user disable/delete it in this case. The 'default' network is always 'in use' as Kimchi uses it when creating a new template. Also update API and test cases to reflect this change Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- docs/API.md | 2 ++ src/kimchi/control/networks.py | 1 + src/kimchi/i18n.py | 4 ++-- src/kimchi/mockmodel.py | 24 ++++++++++++++++++++++++ src/kimchi/model/networks.py | 31 +++++++++++++++++++++++++++---- tests/test_model.py | 18 +++++++++++++++--- tests/test_rest.py | 12 ++++++++++-- 7 files changed, 81 insertions(+), 11 deletions(-) diff --git a/docs/API.md b/docs/API.md index 698fc66..672ef14 100644 --- a/docs/API.md +++ b/docs/API.md @@ -469,6 +469,8 @@ A interface represents available interface on host. * interface: The name of a network interface on the host. For bridge network, the interface can be a bridge or nic/bonding device. For isolated or NAT network, the interface is ignored. + * in_use: True if network is in use by a template or virtual machine; + False, otherwise. ### Resource: Network diff --git a/src/kimchi/control/networks.py b/src/kimchi/control/networks.py index dd7e08d..95e8523 100644 --- a/src/kimchi/control/networks.py +++ b/src/kimchi/control/networks.py @@ -39,6 +39,7 @@ class Network(Resource): def data(self): return {'name': self.ident, 'vms': self.info['vms'], + 'in_use': self.info['in_use'], 'autostart': self.info['autostart'], 'connection': self.info['connection'], 'interface': self.info['interface'], diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index ddb2fcb..589c8cb 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -180,8 +180,8 @@ messages = { "KCHNET0014E": _("Network interface must be a string"), "KCHNET0015E": _("Network VLAN ID must be an integer between 1 and 4094"), "KCHNET0016E": _("Specify name and type to create a Network"), - "KCHNET0017E": _("Unable to delete network %(name)s. There are still some VMs linked to this network."), - "KCHNET0018E": _("Unable to deactivate network %(name)s. There are some VMs running linked to this network."), + "KCHNET0017E": _("Unable to delete network %(name)s. There are some virtual machines and/or templates linked to this network."), + "KCHNET0018E": _("Unable to deactivate network %(name)s. There are some virtual machines and/or templates linked to this network."), "KCHDR0001E": _("Debug report %(name)s does not exist"), "KCHDR0002E": _("Debug report tool not found in system"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 72764b0..b70cc1b 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -596,18 +596,42 @@ class MockModel(object): vms.append(name) return vms + def _is_network_used_by_template(self, network): + for name, tmpl in self._mock_templates.iteritems(): + if network in tmpl.info['networks']: + return True + return False + + def _is_network_in_use(self, name): + # The network "default" is used for Kimchi proposal and should not be + # deactivate or deleted. Otherwise, we will allow user create + # inconsistent templates from scratch + if name == 'default': + return True + + vms = self._get_vms_attach_to_a_network(name) + return bool(vms) or self._is_network_used_by_template(name) + def network_lookup(self, name): network = self._get_network(name) network.info['vms'] = self._get_vms_attach_to_a_network(name) + network.info['in_use'] = self._is_network_in_use(name) + return network.info def network_activate(self, name): self._get_network(name).info['state'] = 'active' def network_deactivate(self, name): + if self._is_network_in_use(name): + raise InvalidOperation("KCHNET0018E", {'name': name}) + self._get_network(name).info['state'] = 'inactive' def network_delete(self, name): + if self._is_network_in_use(name): + raise InvalidOperation("KCHNET0017E", {'name': name}) + # firstly, we should check the network actually exists network = self._get_network(name) del self._mock_networks[network.name] diff --git a/src/kimchi/model/networks.py b/src/kimchi/model/networks.py index 19d8756..0242156 100644 --- a/src/kimchi/model/networks.py +++ b/src/kimchi/model/networks.py @@ -198,6 +198,7 @@ class NetworksModel(object): class NetworkModel(object): def __init__(self, **kargs): self.conn = kargs['conn'] + self.objstore = kargs['objstore'] def lookup(self, name): network = self.get_network(self.conn.get(), name) @@ -228,9 +229,29 @@ class NetworkModel(object): 'subnet': subnet, 'dhcp': dhcp, 'vms': self._get_vms_attach_to_a_network(name), + 'in_use': self._is_network_in_use(name), 'autostart': network.autostart() == 1, 'state': network.isActive() and "active" or "inactive"} + def _is_network_in_use(self, name): + # The network "default" is used for Kimchi proposal and should not be + # deactivate or deleted. Otherwise, we will allow user create + # inconsistent templates from scratch + if name == 'default': + return True + + vms = self._get_vms_attach_to_a_network(name) + return bool(vms) or self._is_network_used_by_template(name) + + def _is_network_used_by_template(self, network): + with self.objstore as session: + templates = session.get_list('template') + for tmpl in templates: + tmpl_net = session.get('template', tmpl)['networks'] + if network in tmpl_net: + return True + return False + def _get_vms_attach_to_a_network(self, network, filter="all"): DOM_STATE_MAP = {'nostate': 0, 'running': 1, 'blocked': 2, 'paused': 3, 'shutdown': 4, 'shutoff': 5, @@ -255,17 +276,19 @@ class NetworkModel(object): network.create() def deactivate(self, name): - network = self.get_network(self.conn.get(), name) - if self._get_vms_attach_to_a_network(name, "running"): + if self._is_network_in_use(name): raise InvalidOperation("KCHNET0018E", {'name': name}) + + network = self.get_network(self.conn.get(), name) network.destroy() def delete(self, name): + if self._is_network_in_use(name): + raise InvalidOperation("KCHNET0017E", {'name': name}) + network = self.get_network(self.conn.get(), name) if network.isActive(): raise InvalidOperation("KCHNET0005E", {'name': name}) - if self._get_vms_attach_to_a_network(name): - raise InvalidOperation("KCHNET0017E", {'name': name}) self._remove_vlan_tagged_bridge(network) network.undefine() diff --git a/tests/test_model.py b/tests/test_model.py index a3d8eff..60d0b94 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -481,17 +481,29 @@ class ModelTests(unittest.TestCase): iso = path + 'ubuntu12.04.iso' iso_gen.construct_fake_iso(iso, True, '12.04', 'ubuntu') + args = {'name': 'test-pool', + 'path': '/tmp/kimchi-images', + 'type': 'dir'} + inst.storagepools_create(args) + rollback.prependDefer(inst.storagepool_delete, 'test-pool') + params = {'name': 'test', 'memory': 1024, 'cpus': 1, - 'networks': ['test-network'], 'cdrom': iso} + 'networks': ['test-network'], 'cdrom': iso, + 'storagepool': '/storagepools/test-pool'} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') + # Try to delete network + # It should fail as it is associated to a template + self.assertRaises(InvalidOperation, inst.network_delete, net_name) + # Update template to release network and then delete it + params = {'networks': []} + inst.template_update('test', params) inst.network_delete(net_name) - shutil.rmtree(path) + shutil.rmtree(path) info = inst.template_lookup('test') self.assertEquals(info['invalid']['cdrom'], [iso]) - self.assertEquals(info['invalid']['networks'], [net_name]) @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_clone(self): diff --git a/tests/test_rest.py b/tests/test_rest.py index 92c1a83..831517f 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1108,7 +1108,16 @@ class RestTests(unittest.TestCase): self.assertEquals(201, resp.status) shutil.rmtree(path) - # Delete the network + # Try to delete network + # It should fail as it is associated to a template + resp = json.loads(request(host, port, '/networks/test-network', + '{}', 'DELETE').read()) + self.assertIn("KCHNET0017E", resp["reason"]) + + # Update template to release network and then delete it + params = {'networks': []} + req = json.dumps(params) + resp = request(host, port, '/templates/test', req, 'PUT') resp = request(host, port, '/networks/test-network', '{}', 'DELETE') self.assertEquals(204, resp.status) @@ -1120,7 +1129,6 @@ class RestTests(unittest.TestCase): # Verify the template res = json.loads(self.request('/templates/test').read()) self.assertEquals(res['invalid']['cdrom'], [iso]) - self.assertEquals(res['invalid']['networks'], ['test-network']) self.assertEquals(res['invalid']['storagepools'], ['test-storagepool']) # Delete the template -- 1.7.10.4
From: Aline Manera <alinefm@br.ibm.com> Verify if network is in use and if so disable the stop and undefine buttons for it. Signed-off-by: Aline Manera <alinefm@br.ibm.com> --- ui/js/src/kimchi.network.js | 27 ++++++++++++++++++++------- ui/pages/tabs/network.html.tmpl | 2 +- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js index a768833..7c4bc77 100644 --- a/ui/js/src/kimchi.network.js +++ b/ui/js/src/kimchi.network.js @@ -30,6 +30,7 @@ kimchi.initNetworkListView = function() { for (var i = 0; i < data.length; i++) { var network = { name : data[i].name, + in_use : data[i].in_use, state : data[i].state === "active" ? "up" : "down" }; if (data[i].connection === "bridge") { @@ -59,6 +60,8 @@ kimchi.getNetworkItemHtml = function(network) { if(i18n["network_type_" + network.type]) { network.type = i18n["network_type_" + network.type]; } + + var disable_in_use = network.in_use ? "ui-state-disabled" : ""; var networkItem = kimchi.template($('#networkItem').html(), { name : network.name, state : network.state, @@ -66,9 +69,10 @@ kimchi.getNetworkItemHtml = function(network) { interface: network.interface, addrSpace : network.addrSpace, startClass : network.state === "up" ? "hide-action-item" : "", - stopClass : network.state === "down" ? "hide-action-item" : "", - deleteClass : network.state === "up" ? "ui-state-disabled" : "", - deleteDisabled: network.state === "up" ? "disabled" : "" + stopClass : network.state === "down" ? "hide-action-item" : disable_in_use, + stopDisabled : network.in_use ? "disabled" : "", + deleteClass : network.state === "up" || network.in_use ? "ui-state-disabled" : "", + deleteDisabled: network.state === "up" || network.in_use ? "disabled" : "" }); return networkItem; }; @@ -92,11 +96,16 @@ kimchi.addNetworkActions = function(network) { $("[nwAct='start']", menu).addClass("hide-action-item"); $("[nwAct='start']", menu).removeClass("ui-state-disabled"); $("[nwAct='stop']", menu).removeClass("hide-action-item"); + if (network.in_use) { + $("[nwAct='stop']", menu).addClass("ui-state-disabled"); + } $(".network-state", $("#" + network.name)).switchClass("nw-loading", "up"); }, function(err) { $(".network-state", $("#" + network.name)).switchClass("nw-loading","down"); $("[nwAct='start']", menu).removeClass("ui-state-disabled"); - $("[nwAct='delete']", menu).removeClass("ui-state-disabled"); + if (!network.in_use) { + $("[nwAct='delete']", menu).removeClass("ui-state-disabled"); + } $(":first-child", $("[nwAct='delete']", menu)).removeAttr("disabled"); kimchi.message.error(err.responseJSON.reason); }); @@ -107,12 +116,16 @@ kimchi.addNetworkActions = function(network) { $("[nwAct='start']", menu).removeClass("hide-action-item"); $("[nwAct='stop']", menu).addClass("hide-action-item"); $("[nwAct='stop']", menu).removeClass("ui-state-disabled"); - $("[nwAct='delete']", menu).removeClass("ui-state-disabled"); - $(":first-child", $("[nwAct='delete']", menu)).removeAttr("disabled"); + if (!network.in_use) { + $("[nwAct='delete']", menu).removeClass("ui-state-disabled"); + $(":first-child", $("[nwAct='delete']", menu)).removeAttr("disabled"); + } $(".network-state", $("#" + network.name)).switchClass("nw-loading", "down"); }, function(err) { $(".network-state", $("#" + network.name)).switchClass("nw-loading", "up"); - $("[nwAct='stop']", menu).removeClass("ui-state-disabled"); + if (!network.in_use) { + $("[nwAct='stop']", menu).removeClass("ui-state-disabled"); + } kimchi.message.error(err.responseJSON.reason); }); } else if ($(evt.currentTarget).attr("nwAct") === "delete") { diff --git a/ui/pages/tabs/network.html.tmpl b/ui/pages/tabs/network.html.tmpl index bbb43e3..e49b257 100644 --- a/ui/pages/tabs/network.html.tmpl +++ b/ui/pages/tabs/network.html.tmpl @@ -95,7 +95,7 @@ <span class="ui-button-secondary dropdown action-button">$_("Actions")</span> <ul class='popover actionsheet right-side menu-container'> <li nwAct="start" class='{startClass}'><a>$_("Start")</a></li> - <li nwAct="stop" class='{stopClass}'><a>$_("Stop")</a></li> + <li nwAct="stop" class='{stopClass}'><a {deleteDisabled}>$_("Stop")</a></li> <li nwAct="delete" class='{deleteClass}'><a {deleteDisabled} class='red'>$_("Delete")</a></li> </ul> </span> -- 1.7.10.4
Reviewed-bt: Crístian Viana <vianac@linux.vnet.ibm.com> Am 12-03-2014 23:29, schrieb Aline Manera:
From: Aline Manera <alinefm@br.ibm.com>
v1 -> V2: - Update test cases (by Royce) - Make function names uniform (by Ming)
Aline Manera (2): Do not allow user disable/delete a network used by VM or template UI: Disable stop/undefine buttons when network is in use
docs/API.md | 2 ++ src/kimchi/control/networks.py | 1 + src/kimchi/i18n.py | 4 ++-- src/kimchi/mockmodel.py | 24 ++++++++++++++++++++++++ src/kimchi/model/networks.py | 31 +++++++++++++++++++++++++++---- tests/test_model.py | 18 +++++++++++++++--- tests/test_rest.py | 12 ++++++++++-- ui/js/src/kimchi.network.js | 27 ++++++++++++++++++++------- ui/pages/tabs/network.html.tmpl | 2 +- 9 files changed, 102 insertions(+), 19 deletions(-)
participants (2)
-
Aline Manera -
Crístian Viana