Re: [Kimchi-devel] [PATCH 1/2] Do not allow user disable/delete a network used by VM or template

On 2014年03月12日 12:00, Aline Manera wrote: > From: Aline Manera <alinefm(a)br.ibm.com&gt; > > 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(a)br.ibm.com&gt; > --- > 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 | 3 +-- > tests/test_rest.py | 6 +++--- > 7 files changed, 60 insertions(+), 11 deletions(-) > > diff --git a/docs/API.md b/docs/API.md > index b3b6c49..3fca87a 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 1ae3889..0af90a0 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 b842b6c..b79aead 100644 > --- a/src/kimchi/mockmodel.py > +++ b/src/kimchi/mockmodel.py > @@ -594,18 +594,42 @@ class MockModel(object): > vms.append(name) > return vms > > + def _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._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 0d6ccec..cfea6ba 100644 > --- a/src/kimchi/model/networks.py > +++ b/src/kimchi/model/networks.py > @@ -153,6 +153,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) > @@ -183,9 +184,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._network_used_by_template(name) > + > + def _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, > @@ -210,17 +231,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..db0f5f5 100644 > --- a/tests/test_model.py > +++ b/tests/test_model.py > @@ -486,12 +486,11 @@ class ModelTests(unittest.TestCase): > inst.templates_create(params) > rollback.prependDefer(inst.template_delete, 'test') > > - inst.network_delete(net_name) > + self.assertRaises(InvalidOperation, inst.network_delete, > net_name) Though we need to check network delete fails because it is referred by template. We still need to clear env (network) after this test finished by disconnecting this template with network.

> 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 65d50e0..4a1b6eb 100644 > --- a/tests/test_rest.py > +++ b/tests/test_rest.py > @@ -1109,8 +1109,9 @@ class RestTests(unittest.TestCase): > > shutil.rmtree(path) > # Delete the network > - resp = request(host, port, '/networks/test-network', '{}', > 'DELETE') > - self.assertEquals(204, resp.status) > + resp = json.loads(request(host, port, '/networks/test-network', > + '{}', 'DELETE').read()) > + self.assertIn("KCHNET0017E", resp["reason"]) > > # Delete the storagepool > resp = request(host, port, '/storagepools/test-storagepool', > @@ -1120,7 +1121,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