[PATCH V6 0/7] template supports networks
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> V5 -> V6 address ming's commnet. raise InvalidParameter instead of NotFoundError when netowrk specified by template does not exist V4 -> V5: rebase to the latest commit. for "move RollbackContext..." merged. V3 -> V4: after template supports networks, we should change the test case accordingly aline refactor controller, rebase to the latest commit. V2 -> V3: fix typo. support creating a vm without network. V1 -> V2: update mockmodel and test case add 'networks' option for template get/create/update ShaoHe Feng (7): template supports networks: let template xml support more networks template supports networks: fix test case template supports networks: update API template supports networks: update controller and json schema template supports networks: update model template supports networks: update mockmodel template supports networks: update test case docs/API.md | 4 ++ src/kimchi/API.json | 12 ++++++ src/kimchi/control/templates.py | 3 +- src/kimchi/mockmodel.py | 16 +++++++- src/kimchi/model.py | 33 ++++++++++++----- src/kimchi/osinfo.py | 5 ++- src/kimchi/vmtemplate.py | 20 ++++++++-- tests/test_model.py | 82 ++++++++++++++++++++++++++++++++--------- tests/test_osinfo.py | 2 +- tests/test_rest.py | 58 +++++++++++++++++++++++++++++ tests/test_vmtemplate.py | 6 +-- 11 files changed, 201 insertions(+), 40 deletions(-) -- 1.8.4.2
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> If a VM is just in one network, it may not access other machines in different networks. In most product environment, more networks are needed. Such as a network management, a service manages all the VMs in one network. Such management system is isolated from the internet for security reasons. And a more public network is needed for a VM to access the internet. So a vm with more networks is a common scenario. Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 5 +++-- src/kimchi/vmtemplate.py | 20 ++++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index da70b30..72c2a74 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -179,8 +179,9 @@ isolinks = { }, } -defaults = {'network': 'default', 'storagepool': '/storagepools/default', - 'domain': 'kvm', 'arch': os.uname()[4] +defaults = {'networks': ['default'], + 'storagepool': '/storagepools/default', + 'domain': 'kvm', 'arch': os.uname()[4] } def lookup(distro, version): diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index dd43faa..5d31f2a 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -182,11 +182,26 @@ class VMTemplate(object): ret.append(info) return ret + def _get_networks_xml(self): + network = """ + <interface type='network'> + <source network='%(network)s'/> + <model type='%(nic_model)s'/> + </interface> + """ + networks = "" + net_info = {"nic_model": self.info['nic_model']} + for nw in self.info['networks']: + net_info['network'] = nw + networks += network % net_info + return networks + def to_vm_xml(self, vm_name, vm_uuid, libvirt_stream = False, qemu_stream_dns = False): params = dict(self.info) params['name'] = vm_name params['uuid'] = vm_uuid params['disks'] = self._get_disks_xml(vm_uuid) + params['networks'] = self._get_networks_xml() params['qemu-namespace'] = '' params['cdroms'] = '' params['qemu-stream-cmdline'] = '' @@ -220,10 +235,7 @@ class VMTemplate(object): <devices> %(disks)s %(cdroms)s - <interface type='network'> - <source network='%(network)s'/> - <model type='%(nic_model)s'/> - </interface> + %(networks)s <graphics type='vnc' /> <sound model='ich6' /> <memballoon model='virtio' /> -- 1.8.4.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/02/2014 08:22 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
If a VM is just in one network, it may not access other machines in different networks.
In most product environment, more networks are needed.
Such as a network management, a service manages all the VMs in one network. Such management system is isolated from the internet for security reasons.
And a more public network is needed for a VM to access the internet.
So a vm with more networks is a common scenario.
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 5 +++-- src/kimchi/vmtemplate.py | 20 ++++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index da70b30..72c2a74 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -179,8 +179,9 @@ isolinks = { }, }
-defaults = {'network': 'default', 'storagepool': '/storagepools/default', - 'domain': 'kvm', 'arch': os.uname()[4] +defaults = {'networks': ['default'], + 'storagepool': '/storagepools/default', + 'domain': 'kvm', 'arch': os.uname()[4] }
def lookup(distro, version): diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index dd43faa..5d31f2a 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -182,11 +182,26 @@ class VMTemplate(object): ret.append(info) return ret
+ def _get_networks_xml(self): + network = """ + <interface type='network'> + <source network='%(network)s'/> + <model type='%(nic_model)s'/> + </interface> + """ + networks = "" + net_info = {"nic_model": self.info['nic_model']} + for nw in self.info['networks']: + net_info['network'] = nw + networks += network % net_info + return networks + def to_vm_xml(self, vm_name, vm_uuid, libvirt_stream = False, qemu_stream_dns = False): params = dict(self.info) params['name'] = vm_name params['uuid'] = vm_uuid params['disks'] = self._get_disks_xml(vm_uuid) + params['networks'] = self._get_networks_xml() params['qemu-namespace'] = '' params['cdroms'] = '' params['qemu-stream-cmdline'] = '' @@ -220,10 +235,7 @@ class VMTemplate(object): <devices> %(disks)s %(cdroms)s - <interface type='network'> - <source network='%(network)s'/> - <model type='%(nic_model)s'/> - </interface> + %(networks)s <graphics type='vnc' /> <sound model='ich6' /> <memballoon model='virtio' />
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> after template supports networks, we should change the test case accordingly Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/model.py | 19 +++++++++---------- tests/test_osinfo.py | 2 +- tests/test_vmtemplate.py | 6 +++--- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/kimchi/model.py b/src/kimchi/model.py index a6790b8..572b316 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1374,16 +1374,15 @@ class LibvirtVMTemplate(VMTemplate): return pool def _network_validate(self): - name = self.info['network'] - try: - conn = self.conn.get() - network = conn.networkLookupByName(name) - except libvirt.libvirtError: - raise InvalidParameter('Network specified by template does not exist') - if not network.isActive(): - raise InvalidParameter('Storage specified by template is not active') - - return network + names = self.info['networks'] + for name in names: + try: + conn = self.conn.get() + network = conn.networkLookupByName(name) + except libvirt.libvirtError: + raise InvalidParameter('Network specified by template does not exist') + if not network.isActive(): + raise InvalidParameter('Network specified by template is not active') def _get_storage_path(self): pool = self._storage_validate() diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py index fda8ada..69fdaf9 100644 --- a/tests/test_osinfo.py +++ b/tests/test_osinfo.py @@ -32,7 +32,7 @@ class OSInfoTests(unittest.TestCase): self.assertEquals(name, 'unknown') self.assertEquals('unknown', entry['os_distro']) self.assertEquals('unknown', entry['os_version']) - self.assertEquals('default', entry['network']) + self.assertEquals(['default'], entry['networks']) def test_fedora_lookup(self): cd = 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x...' diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py index 7f032e7..dc9c0ef 100644 --- a/tests/test_vmtemplate.py +++ b/tests/test_vmtemplate.py @@ -32,7 +32,7 @@ class VMTemplateTests(unittest.TestCase): def test_minimal_construct(self): fields = (('name', 'test'), ('os_distro', 'unknown'), ('os_version', 'unknown'), ('cpus', 1), - ('memory', 1024), ('cdrom', ''), ('network', 'default'), + ('memory', 1024), ('cdrom', ''), ('networks', ['default']), ('disk_bus', 'ide'), ('nic_model', 'e1000')) args = {'name': 'test'} @@ -58,10 +58,10 @@ class VMTemplateTests(unittest.TestCase): provided parameters. """ args = {'name': 'test', 'os_distro': 'opensuse', 'os_version': '12.3', - 'cpus': 2, 'memory': 2048, 'network': 'foo', + 'cpus': 2, 'memory': 2048, 'networks': ['foo'], 'cdrom': '/cd.iso'} t = VMTemplate(args) self.assertEquals(2, t.info.get('cpus')) self.assertEquals(2048, t.info.get('memory')) - self.assertEquals('foo', t.info.get('network')) + self.assertEquals(['foo'], t.info.get('networks')) self.assertEquals('/cd.iso', t.info.get('cdrom')) -- 1.8.4.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/02/2014 08:22 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
after template supports networks, we should change the test case accordingly
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/model.py | 19 +++++++++---------- tests/test_osinfo.py | 2 +- tests/test_vmtemplate.py | 6 +++--- 3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index a6790b8..572b316 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -1374,16 +1374,15 @@ class LibvirtVMTemplate(VMTemplate): return pool
def _network_validate(self): - name = self.info['network'] - try: - conn = self.conn.get() - network = conn.networkLookupByName(name) - except libvirt.libvirtError: - raise InvalidParameter('Network specified by template does not exist') - if not network.isActive(): - raise InvalidParameter('Storage specified by template is not active') - - return network + names = self.info['networks'] + for name in names: + try: + conn = self.conn.get() + network = conn.networkLookupByName(name) + except libvirt.libvirtError: + raise InvalidParameter('Network specified by template does not exist') + if not network.isActive(): + raise InvalidParameter('Network specified by template is not active')
def _get_storage_path(self): pool = self._storage_validate() diff --git a/tests/test_osinfo.py b/tests/test_osinfo.py index fda8ada..69fdaf9 100644 --- a/tests/test_osinfo.py +++ b/tests/test_osinfo.py @@ -32,7 +32,7 @@ class OSInfoTests(unittest.TestCase): self.assertEquals(name, 'unknown') self.assertEquals('unknown', entry['os_distro']) self.assertEquals('unknown', entry['os_version']) - self.assertEquals('default', entry['network']) + self.assertEquals(['default'], entry['networks'])
def test_fedora_lookup(self): cd = 'http://fedora.mirrors.tds.net/pub/fedora/releases/17/Live/x86_64/Fedora-17-x...' diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py index 7f032e7..dc9c0ef 100644 --- a/tests/test_vmtemplate.py +++ b/tests/test_vmtemplate.py @@ -32,7 +32,7 @@ class VMTemplateTests(unittest.TestCase): def test_minimal_construct(self): fields = (('name', 'test'), ('os_distro', 'unknown'), ('os_version', 'unknown'), ('cpus', 1), - ('memory', 1024), ('cdrom', ''), ('network', 'default'), + ('memory', 1024), ('cdrom', ''), ('networks', ['default']), ('disk_bus', 'ide'), ('nic_model', 'e1000'))
args = {'name': 'test'} @@ -58,10 +58,10 @@ class VMTemplateTests(unittest.TestCase): provided parameters. """ args = {'name': 'test', 'os_distro': 'opensuse', 'os_version': '12.3', - 'cpus': 2, 'memory': 2048, 'network': 'foo', + 'cpus': 2, 'memory': 2048, 'networks': ['foo'], 'cdrom': '/cd.iso'} t = VMTemplate(args) self.assertEquals(2, t.info.get('cpus')) self.assertEquals(2048, t.info.get('memory')) - self.assertEquals('foo', t.info.get('network')) + self.assertEquals(['foo'], t.info.get('networks')) self.assertEquals('/cd.iso', t.info.get('cdrom'))
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> add 'networks' option for template get/create/update Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- docs/API.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/API.md b/docs/API.md index 9edc551..10c94cf 100644 --- a/docs/API.md +++ b/docs/API.md @@ -120,6 +120,8 @@ Represents a snapshot of the Virtual Machine's primary monitor. * cdrom *(required)*: A volume name or URI to an ISO image. * storagepool *(optional)*: URI of the storagepool. Default is '/storagepools/default' + * networks *(optional)*: list of networks will be assigned to the new VM. + Default is '[default]' * disks *(optional)*: An array of requested disks with the following optional fields (either *size* or *volume* must be specified): * index: The device index @@ -144,6 +146,7 @@ Represents a snapshot of the Virtual Machine's primary monitor. * memory: The amount of memory assigned to the VM * cdrom: A volume name or URI to an ISO image * storagepool: URI of the storagepool where template allocates vm storage. + * networks *(optional)*: list of networks will be assigned to the new VM. * disks: An array of requested disks with the following optional fields (either *size* or *volume* must be specified): * index: The device index @@ -162,6 +165,7 @@ Represents a snapshot of the Virtual Machine's primary monitor. * memory: The amount of memory assigned to the VM * cdrom: A volume name or URI to an ISO image * storagepool: URI of the storagepool where template allocates vm storage. + * networks *(optional)*: list of networks will be assigned to the new VM. * disks: An array of requested disks with the following optional fields (either *size* or *volume* must be specified): * index: The device index -- 1.8.4.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/02/2014 08:22 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
add 'networks' option for template get/create/update
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- docs/API.md | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/docs/API.md b/docs/API.md index 9edc551..10c94cf 100644 --- a/docs/API.md +++ b/docs/API.md @@ -120,6 +120,8 @@ Represents a snapshot of the Virtual Machine's primary monitor. * cdrom *(required)*: A volume name or URI to an ISO image. * storagepool *(optional)*: URI of the storagepool. Default is '/storagepools/default' + * networks *(optional)*: list of networks will be assigned to the new VM. + Default is '[default]' * disks *(optional)*: An array of requested disks with the following optional fields (either *size* or *volume* must be specified): * index: The device index @@ -144,6 +146,7 @@ Represents a snapshot of the Virtual Machine's primary monitor. * memory: The amount of memory assigned to the VM * cdrom: A volume name or URI to an ISO image * storagepool: URI of the storagepool where template allocates vm storage. + * networks *(optional)*: list of networks will be assigned to the new VM. * disks: An array of requested disks with the following optional fields (either *size* or *volume* must be specified): * index: The device index @@ -162,6 +165,7 @@ Represents a snapshot of the Virtual Machine's primary monitor. * memory: The amount of memory assigned to the VM * cdrom: A volume name or URI to an ISO image * storagepool: URI of the storagepool where template allocates vm storage. + * networks *(optional)*: list of networks will be assigned to the new VM. * disks: An array of requested disks with the following optional fields (either *size* or *volume* must be specified): * index: The device index
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> for json schema, verify the 'networks' option when creating or updating template. for controller, add 'networks' attribute when GET template. Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/API.json | 12 ++++++++++++ src/kimchi/control/templates.py | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 7b90826..527bb60 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -125,6 +125,12 @@ "type": "string", "pattern": "^/storagepools/[^/]+/?$" }, + "networks": { + "description": "list of which networks will be assigned to the new VM.", + "type": "array", + "items": { "type": "string" }, + "uniqueItems": true + }, "folder": { "description": "Folder", "type": "array", @@ -198,6 +204,12 @@ "type": "string", "pattern": "^/storagepools/[^/]+/?$" }, + "networks": { + "description": "list of which networks will be assigned to the new VM.", + "type": "array", + "items": { "type": "string" }, + "uniqueItems": true + }, "folder": { "description": "Folder", "type": "array", diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py index 30875cd..bf40e2c 100644 --- a/src/kimchi/control/templates.py +++ b/src/kimchi/control/templates.py @@ -35,7 +35,7 @@ class Template(Resource): super(Template, self).__init__(model, ident) self.update_params = ["name", "folder", "icon", "os_distro", "storagepool", "os_version", "cpus", - "memory", "cdrom", "disks"] + "memory", "cdrom", "disks", "networks"] self.uri_fmt = "/templates/%s" @property @@ -49,4 +49,5 @@ class Template(Resource): 'cdrom': self.info['cdrom'], 'disks': self.info['disks'], 'storagepool': self.info['storagepool'], + 'networks': self.info['networks'], 'folder': self.info.get('folder', [])} -- 1.8.4.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/02/2014 08:22 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
for json schema, verify the 'networks' option when creating or updating template. for controller, add 'networks' attribute when GET template.
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/API.json | 12 ++++++++++++ src/kimchi/control/templates.py | 3 ++- 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 7b90826..527bb60 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -125,6 +125,12 @@ "type": "string", "pattern": "^/storagepools/[^/]+/?$" }, + "networks": { + "description": "list of which networks will be assigned to the new VM.", + "type": "array", + "items": { "type": "string" }, + "uniqueItems": true + }, "folder": { "description": "Folder", "type": "array", @@ -198,6 +204,12 @@ "type": "string", "pattern": "^/storagepools/[^/]+/?$" }, + "networks": { + "description": "list of which networks will be assigned to the new VM.", + "type": "array", + "items": { "type": "string" }, + "uniqueItems": true + }, "folder": { "description": "Folder", "type": "array", diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py index 30875cd..bf40e2c 100644 --- a/src/kimchi/control/templates.py +++ b/src/kimchi/control/templates.py @@ -35,7 +35,7 @@ class Template(Resource): super(Template, self).__init__(model, ident) self.update_params = ["name", "folder", "icon", "os_distro", "storagepool", "os_version", "cpus", - "memory", "cdrom", "disks"] + "memory", "cdrom", "disks", "networks"] self.uri_fmt = "/templates/%s"
@property @@ -49,4 +49,5 @@ class Template(Resource): 'cdrom': self.info['cdrom'], 'disks': self.info['disks'], 'storagepool': self.info['storagepool'], + 'networks': self.info['networks'], 'folder': self.info.get('folder', [])}
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> check all networks exist Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/model.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 572b316..cbde74a 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -678,6 +678,13 @@ class Model(object): def templates_create(self, params): name = params['name'] + for net_name in params.get(u'networks', []): + try: + self._get_network(net_name) + except NotFoundError: + raise InvalidParameter("Network '%s' specified by template " + "does not exist" % net_name) + with self.objstore as session: if name in session.get_list('template'): raise InvalidOperation("Template already exists") @@ -698,6 +705,13 @@ class Model(object): except Exception as e: raise InvalidParameter("Storagepool specified is not valid: %s." % e.message) + for net_name in params.get(u'networks', []): + try: + self._get_network(net_name) + except NotFoundError: + raise InvalidParameter("Network '%s' specified by template " + "does not exist" % net_name) + self.template_delete(name) try: ident = self.templates_create(new_t) -- 1.8.4.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/02/2014 08:22 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
check all networks exist
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/model.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 572b316..cbde74a 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -678,6 +678,13 @@ class Model(object):
def templates_create(self, params): name = params['name'] + for net_name in params.get(u'networks', []): + try: + self._get_network(net_name) + except NotFoundError: + raise InvalidParameter("Network '%s' specified by template " + "does not exist" % net_name) + with self.objstore as session: if name in session.get_list('template'): raise InvalidOperation("Template already exists") @@ -698,6 +705,13 @@ class Model(object): except Exception as e: raise InvalidParameter("Storagepool specified is not valid: %s." % e.message)
+ for net_name in params.get(u'networks', []): + try: + self._get_network(net_name) + except NotFoundError: + raise InvalidParameter("Network '%s' specified by template " + "does not exist" % net_name) + self.template_delete(name) try: ident = self.templates_create(new_t)
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> check all networks exist Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 348127a..9488078 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -200,6 +200,13 @@ class MockModel(object): name = params['name'] if name in self._mock_templates: raise InvalidOperation("Template already exists") + for net_name in params.get(u'networks', []): + try: + self._get_network(net_name) + except NotFoundError: + raise InvalidParameter("Network '%s' specified by template " + "does not exist" % net_name) + t = MockVMTemplate(params, self) self._mock_templates[name] = t return name @@ -217,6 +224,13 @@ class MockModel(object): except Exception as e: raise InvalidParameter("Storagepool specified is not valid: %s." % e.message) + for net_name in params.get(u'networks', []): + try: + self._get_network(net_name) + except NotFoundError: + raise InvalidParameter("Network '%s' specified by template " + "does not exist" % net_name) + self.template_delete(name) try: ident = self.templates_create(new_t) @@ -457,7 +471,7 @@ class MockModel(object): try: return self._mock_networks[name] except KeyError: - raise NotFoundError() + raise NotFoundError("Network '%s'" % name) def network_lookup(self, name): network = self._get_network(name) -- 1.8.4.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/02/2014 08:22 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
check all networks exist
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 348127a..9488078 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -200,6 +200,13 @@ class MockModel(object): name = params['name'] if name in self._mock_templates: raise InvalidOperation("Template already exists") + for net_name in params.get(u'networks', []): + try: + self._get_network(net_name) + except NotFoundError: + raise InvalidParameter("Network '%s' specified by template " + "does not exist" % net_name) + t = MockVMTemplate(params, self) self._mock_templates[name] = t return name @@ -217,6 +224,13 @@ class MockModel(object): except Exception as e: raise InvalidParameter("Storagepool specified is not valid: %s." % e.message)
+ for net_name in params.get(u'networks', []): + try: + self._get_network(net_name) + except NotFoundError: + raise InvalidParameter("Network '%s' specified by template " + "does not exist" % net_name) + self.template_delete(name) try: ident = self.templates_create(new_t) @@ -457,7 +471,7 @@ class MockModel(object): try: return self._mock_networks[name] except KeyError: - raise NotFoundError() + raise NotFoundError("Network '%s'" % name)
def network_lookup(self, name): network = self._get_network(name)
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> update test_rest and test_model Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 82 +++++++++++++++++++++++++++++++++++++++++------------ tests/test_rest.py | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 18 deletions(-) diff --git a/tests/test_model.py b/tests/test_model.py index c03cc3f..d106114 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -260,6 +260,7 @@ class ModelTests(unittest.TestCase): disk_path = '/tmp/kimchi-images/%s-0.img' % vm_info['uuid'] self.assertTrue(os.access(disk_path, os.F_OK)) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_create(self): inst = kimchi.model.Model('test:///default', objstore_loc=self.tmp_store) @@ -272,28 +273,73 @@ class ModelTests(unittest.TestCase): params['cdrom'] = os.path.abspath(__file__) self.assertRaises(InvalidParameter, inst.templates_create, params) + with RollbackContext() as rollback: + net_name = 'test-network' + net_args = {'name': net_name, + 'connection': 'nat', + 'subnet': '127.0.100.0/24'} + inst.networks_create(net_args) + rollback.prependDefer(inst.network_delete, net_name) + + params = {'name': 'test', 'memory': 1024, 'cpus': 1} + inst.templates_create(params) + rollback.prependDefer(inst.template_delete, 'test') + info = inst.template_lookup('test') + for key in params.keys(): + self.assertEquals(params[key], info[key]) + self.assertEquals("default", info["networks"][0]) + + # create template with with non-existent network + params['name'] = 'new-test' + params['networks'] = ["no-exist"] + self.assertRaises(InvalidParameter, inst.templates_create, params) + + params['networks'] = ['default', 'test-network'] + inst.templates_create(params) + rollback.prependDefer(inst.template_delete, params['name']) + info = inst.template_lookup(params['name']) + for key in params.keys(): + self.assertEquals(params[key], info[key]) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_update(self): inst = kimchi.model.Model('qemu:///system', objstore_loc=self.tmp_store) - - orig_params = {'name': 'test', 'memory': '1024', 'cpus': '1'} - inst.templates_create(orig_params) - - params = {'name': 'new-test'} - self.assertEquals('new-test', inst.template_update('test', params)) - - params = {'name': 'new-test', 'memory': '512', 'cpus': '2'} - inst.template_update('new-test', params) - info = inst.template_lookup('new-test') - for key in params.keys(): - self.assertEquals(params[key], info[key]) - - params = {'name': 'new-test', 'memory': 1024, 'cpus': 1} - inst.template_update('new-test', params) - info = inst.template_lookup('new-test') - for key in params.keys(): - self.assertEquals(params[key], info[key]) + with RollbackContext() as rollback: + net_name = 'test-network' + net_args = {'name': net_name, + 'connection': 'nat', + 'subnet': '127.0.100.0/24'} + inst.networks_create(net_args) + rollback.prependDefer(inst.network_delete, net_name) + + orig_params = {'name': 'test', 'memory': 1024, 'cpus': 1} + inst.templates_create(orig_params) + + params = {'name': 'new-test'} + self.assertEquals('new-test', inst.template_update('test', params)) + self.assertRaises(NotFoundError, inst.template_delete, 'test') + + params = {'name': 'new-test', 'memory': 512, 'cpus': 2} + inst.template_update('new-test', params) + rollback.prependDefer(inst.template_delete, 'new-test') + + info = inst.template_lookup('new-test') + for key in params.keys(): + self.assertEquals(params[key], info[key]) + self.assertEquals("default", info["networks"][0]) + + params = {'name': 'new-test', 'memory': 1024, 'cpus': 1, + 'networks': ['default', 'test-network']} + inst.template_update('new-test', params) + info = inst.template_lookup('new-test') + for key in params.keys(): + self.assertEquals(params[key], info[key]) + + # test update with with non-existent network + params = {'networks': ["no-exist"]} + self.assertRaises(InvalidParameter, inst.template_update, + 'new-test', params) def test_vm_edit(self): inst = kimchi.model.Model('qemu:///system', diff --git a/tests/test_rest.py b/tests/test_rest.py index e626d2f..a960868 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -351,6 +351,64 @@ class RestTests(unittest.TestCase): # Verify the volume was deleted self.assertHTTPStatus(404, vol_uri) + def test_template_customise_network(self): + with RollbackContext() as rollback: + tmpl = {'name': 'test', 'cdrom': '/nonexistent.iso', + 'disks': [{'size': 1}]} + req = json.dumps(tmpl) + resp = self.request('/templates', req, 'POST') + self.assertEquals(201, resp.status) + # Delete the template + rollback.prependDefer(self.request, + '/templates/test', '{}', 'DELETE') + tmpl_res = json.loads(resp.read()) + self.assertTrue(type(tmpl_res['networks']) is list) + self.assertEquals("default", tmpl_res['networks'][0]) + + tmpl['name'] = "failed_tmpl" + # Create a Template with non-array network fails with 400 + tmpl['networks'] = "test-network" + req = json.dumps(tmpl) + resp = self.request('/templates', req, 'POST') + self.assertEquals(400, resp.status) + + # Create a Template with non-existent network fails with 400 + tmpl['networks'] = ["test-network"] + req = json.dumps(tmpl) + resp = self.request('/templates', req, 'POST') + self.assertEquals(400, resp.status) + + # Create a network + req = json.dumps({'name': 'test-network', + 'connection': 'nat', + 'net': '127.0.1.0/24'}) + resp = self.request('/networks', req, 'POST') + self.assertEquals(201, resp.status) + # Delete the network + rollback.prependDefer(self.request, + '/networks/test-network', '{}', 'DELETE') + + tmpl['name'] = "test" + # Update a Template with non-array network fails with 400 + tmpl['networks'] = "bad-network" + req = json.dumps(tmpl) + resp = self.request('/templates/test', req, 'PUT') + self.assertEquals(400, resp.status) + # Update a Template with non-existent network fails with 400 + tmpl['networks'] = ["bad-network"] + req = json.dumps(tmpl) + resp = self.request('/templates/test', req, 'PUT') + self.assertEquals(400, resp.status) + + # Update a Template with existent network, successful + tmpl['networks'] = ["default", "test-network"] + req = json.dumps(tmpl) + resp = self.request('/templates/test', req, 'PUT') + self.assertEquals(200, resp.status) + tmpl_res = json.loads(resp.read()) + self.assertTrue(type(tmpl_res['networks']) is list) + self.assertEquals(tmpl['networks'], tmpl_res['networks']) + def test_unnamed_vms(self): # Create a Template req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'}) -- 1.8.4.2
Just a typo below On 01/02/2014 08:22 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
update test_rest and test_model
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 82 +++++++++++++++++++++++++++++++++++++++++------------ tests/test_rest.py | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 18 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py index c03cc3f..d106114 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -260,6 +260,7 @@ class ModelTests(unittest.TestCase): disk_path = '/tmp/kimchi-images/%s-0.img' % vm_info['uuid'] self.assertTrue(os.access(disk_path, os.F_OK))
+ @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_create(self): inst = kimchi.model.Model('test:///default', objstore_loc=self.tmp_store) @@ -272,28 +273,73 @@ class ModelTests(unittest.TestCase): params['cdrom'] = os.path.abspath(__file__) self.assertRaises(InvalidParameter, inst.templates_create, params)
+ with RollbackContext() as rollback: + net_name = 'test-network' + net_args = {'name': net_name, + 'connection': 'nat', + 'subnet': '127.0.100.0/24'} + inst.networks_create(net_args) + rollback.prependDefer(inst.network_delete, net_name) + + params = {'name': 'test', 'memory': 1024, 'cpus': 1} + inst.templates_create(params) + rollback.prependDefer(inst.template_delete, 'test') + info = inst.template_lookup('test') + for key in params.keys(): + self.assertEquals(params[key], info[key]) + self.assertEquals("default", info["networks"][0]) + + # create template with with non-existent network
Typo: with with
+ params['name'] = 'new-test' + params['networks'] = ["no-exist"] + self.assertRaises(InvalidParameter, inst.templates_create, params) + + params['networks'] = ['default', 'test-network'] + inst.templates_create(params) + rollback.prependDefer(inst.template_delete, params['name']) + info = inst.template_lookup(params['name']) + for key in params.keys(): + self.assertEquals(params[key], info[key]) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_template_update(self): inst = kimchi.model.Model('qemu:///system', objstore_loc=self.tmp_store) - - orig_params = {'name': 'test', 'memory': '1024', 'cpus': '1'} - inst.templates_create(orig_params) - - params = {'name': 'new-test'} - self.assertEquals('new-test', inst.template_update('test', params)) - - params = {'name': 'new-test', 'memory': '512', 'cpus': '2'} - inst.template_update('new-test', params) - info = inst.template_lookup('new-test') - for key in params.keys(): - self.assertEquals(params[key], info[key]) - - params = {'name': 'new-test', 'memory': 1024, 'cpus': 1} - inst.template_update('new-test', params) - info = inst.template_lookup('new-test') - for key in params.keys(): - self.assertEquals(params[key], info[key]) + with RollbackContext() as rollback: + net_name = 'test-network' + net_args = {'name': net_name, + 'connection': 'nat', + 'subnet': '127.0.100.0/24'} + inst.networks_create(net_args) + rollback.prependDefer(inst.network_delete, net_name) + + orig_params = {'name': 'test', 'memory': 1024, 'cpus': 1} + inst.templates_create(orig_params) + + params = {'name': 'new-test'} + self.assertEquals('new-test', inst.template_update('test', params)) + self.assertRaises(NotFoundError, inst.template_delete, 'test') + + params = {'name': 'new-test', 'memory': 512, 'cpus': 2} + inst.template_update('new-test', params) + rollback.prependDefer(inst.template_delete, 'new-test') + + info = inst.template_lookup('new-test') + for key in params.keys(): + self.assertEquals(params[key], info[key]) + self.assertEquals("default", info["networks"][0]) + + params = {'name': 'new-test', 'memory': 1024, 'cpus': 1, + 'networks': ['default', 'test-network']} + inst.template_update('new-test', params) + info = inst.template_lookup('new-test') + for key in params.keys(): + self.assertEquals(params[key], info[key]) + + # test update with with non-existent network + params = {'networks': ["no-exist"]} + self.assertRaises(InvalidParameter, inst.template_update, + 'new-test', params)
def test_vm_edit(self): inst = kimchi.model.Model('qemu:///system', diff --git a/tests/test_rest.py b/tests/test_rest.py index e626d2f..a960868 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -351,6 +351,64 @@ class RestTests(unittest.TestCase): # Verify the volume was deleted self.assertHTTPStatus(404, vol_uri)
+ def test_template_customise_network(self): + with RollbackContext() as rollback: + tmpl = {'name': 'test', 'cdrom': '/nonexistent.iso', + 'disks': [{'size': 1}]} + req = json.dumps(tmpl) + resp = self.request('/templates', req, 'POST') + self.assertEquals(201, resp.status) + # Delete the template + rollback.prependDefer(self.request, + '/templates/test', '{}', 'DELETE') + tmpl_res = json.loads(resp.read()) + self.assertTrue(type(tmpl_res['networks']) is list) + self.assertEquals("default", tmpl_res['networks'][0]) + + tmpl['name'] = "failed_tmpl" + # Create a Template with non-array network fails with 400 + tmpl['networks'] = "test-network" + req = json.dumps(tmpl) + resp = self.request('/templates', req, 'POST') + self.assertEquals(400, resp.status) + + # Create a Template with non-existent network fails with 400 + tmpl['networks'] = ["test-network"] + req = json.dumps(tmpl) + resp = self.request('/templates', req, 'POST') + self.assertEquals(400, resp.status) + + # Create a network + req = json.dumps({'name': 'test-network', + 'connection': 'nat', + 'net': '127.0.1.0/24'}) + resp = self.request('/networks', req, 'POST') + self.assertEquals(201, resp.status) + # Delete the network + rollback.prependDefer(self.request, + '/networks/test-network', '{}', 'DELETE') + + tmpl['name'] = "test" + # Update a Template with non-array network fails with 400 + tmpl['networks'] = "bad-network" + req = json.dumps(tmpl) + resp = self.request('/templates/test', req, 'PUT') + self.assertEquals(400, resp.status) + # Update a Template with non-existent network fails with 400 + tmpl['networks'] = ["bad-network"] + req = json.dumps(tmpl) + resp = self.request('/templates/test', req, 'PUT') + self.assertEquals(400, resp.status) + + # Update a Template with existent network, successful + tmpl['networks'] = ["default", "test-network"] + req = json.dumps(tmpl) + resp = self.request('/templates/test', req, 'PUT') + self.assertEquals(200, resp.status) + tmpl_res = json.loads(resp.read()) + self.assertTrue(type(tmpl_res['networks']) is list) + self.assertEquals(tmpl['networks'], tmpl_res['networks']) + def test_unnamed_vms(self): # Create a Template req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'})
participants (2)
-
Aline Manera -
shaohef＠linux.vnet.ibm.com