On 04/15/2016 05:48 PM, Jose Ricardo Ziviani wrote:
- This commit changes the way the template update works. Now, it really updates the existing template instead of creating a new one (and deleting the old one)
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com> --- i18n.py | 3 +++ model/templates.py | 70 ++++++++++++++++++++++++++++++++------------------ tests/test_template.py | 6 ++++- 3 files changed, 53 insertions(+), 26 deletions(-)
diff --git a/i18n.py b/i18n.py index 7cce796..e4656cb 100644 --- a/i18n.py +++ b/i18n.py @@ -155,8 +155,10 @@ messages = { "KCHVMIF0011E": _("Cannot change MAC address of a running virtual machine"),
"KCHTMPL0001E": _("Template %(name)s already exists"), + "KCHTMPL0002E": _("Source media %(path)s not found"), "KCHTMPL0003E": _("Network '%(network)s' specified for template %(template)s does not exist"), "KCHTMPL0004E": _("Storage pool %(pool)s specified for template %(template)s does not exist"), + "KCHTMPL0005E": _("Template %(name)s does not exist."), "KCHTMPL0006E": _("Invalid parameter '%(param)s' specified for CDROM."), "KCHTMPL0007E": _("Network %(network)s specified for template %(template)s is not active"), "KCHTMPL0008E": _("Template name must be a string"), @@ -181,6 +183,7 @@ messages = { "KCHTMPL0029E": _("Disk format must be 'raw', for logical, iscsi, and scsi pools."), "KCHTMPL0030E": _("Memory expects an object with one or both parameters: 'current' and 'maxmemory'"), "KCHTMPL0031E": _("Memory value (%(mem)sMiB) must be equal or lesser than maximum memory value (%(maxmem)sMiB)"), + "KCHTMPL0032E": _("Unable to update template due error: %(err)s"),
"KCHPOOL0001E": _("Storage pool %(name)s already exists"), "KCHPOOL0002E": _("Storage pool %(name)s does not exist"), diff --git a/model/templates.py b/model/templates.py index a02099c..d0742ca 100644 --- a/model/templates.py +++ b/model/templates.py @@ -51,7 +51,7 @@ class TemplatesModel(object): self.objstore = kargs['objstore'] self.conn = kargs['conn']
- def create(self, params): + def _validate_template_params(self, params): name = params.get('name', '').strip()
conn = self.conn.get() @@ -72,8 +72,7 @@ class TemplatesModel(object):
# image does not exists: raise error elif not os.path.exists(path): - raise InvalidParameter("Unable to find file %(path)s" % - {"path": path}) + raise InvalidParameter("KCHTMPL0002E", {'path': path})
# create magic object to discover file type file_type = magic.open(magic.MAGIC_NONE) @@ -99,10 +98,6 @@ class TemplatesModel(object): params["disks"] = params.get('disks', []) params["disks"].append({"base": path})
- return self.save_template(params) - - def save_template(self, params): - # Creates the template class with necessary information t = LibvirtVMTemplate(params, scan=True, conn=self.conn)
@@ -119,16 +114,18 @@ class TemplatesModel(object): if 'volume' in disk.keys(): self.template_volume_validate(volume, disk['pool'])
- # template with the same name already exists: raise exception - name = params['name'] - with self.objstore as session: - if name in session.get_list('template'): - raise InvalidOperation("KCHTMPL0001E", {'name': name}) + return t + + def create(self, params): + template = self._validate_template_params(params)
# Store template on objectstore + name = params['name'] try: with self.objstore as session: - session.store('template', name, t.info, + if name in session.get_list('template'): + raise InvalidOperation("KCHTMPL0001E", {'name': name}) + session.store('template', name, template.info, get_kimchi_version()) except InvalidOperation: raise @@ -191,7 +188,8 @@ class TemplateModel(object):
temp = self.lookup(name) temp['name'] = clone_name - ident = self.templates.save_template(temp)
+ temp['source_media'] = temp.get('base', temp['cdrom'])
Not sure I understood this block. To create a template the user must to pass a value for source_media and cdrom or disks[base] are not allowed. Said that, I don't see reasons to have it here.
+ ident = self.templates.create(temp) return ident
def delete(self, name): @@ -204,31 +202,36 @@ class TemplateModel(object): raise OperationFailed('KCHTMPL0021E', {'err': e.message})
def update(self, name, params): - old_t = self.lookup(name) - new_t = copy.copy(old_t) + edit_template = self.lookup(name) +
+ edit_template['source_media'] = edit_template.get( + 'base', edit_template['cdrom']) + + if 'cdrom' in params: + edit_template['source_media'] = params['cdrom']
source_media is not a valid parameter for template update. It is only required to create a new Template. To update it, you need to use disks[base] or cdrom. The same it was before source_media being added.
# Merge graphics settings graph_args = params.get('graphics') if graph_args: - graphics = dict(new_t['graphics']) + graphics = dict(edit_template['graphics']) graphics.update(graph_args) params['graphics'] = graphics
# Merge cpu_info settings new_cpu_info = params.get('cpu_info') if new_cpu_info: - cpu_info = dict(new_t['cpu_info']) + cpu_info = dict(edit_template['cpu_info']) cpu_info.update(new_cpu_info) params['cpu_info'] = cpu_info
# Fix memory values, because method update does not work recursively new_mem = params.get('memory') if new_mem is not None: - params['memory'] = copy.copy(old_t.get('memory')) + params['memory'] = copy.copy(edit_template.get('memory')) params['memory'].update(new_mem) validate_memory(params['memory'])
- new_t.update(params) + edit_template.update(params)
for net_name in params.get(u'networks', []): try: @@ -238,13 +241,30 @@ class TemplateModel(object): raise InvalidParameter("KCHTMPL0003E", {'network': net_name, 'template': name})
- self.delete(name) + template = self.templates._validate_template_params(edit_template) + try: - ident = self.templates.save_template(new_t) - except: - ident = self.templates.save_template(old_t) + with self.objstore as session: + if name not in session.get_list('template'): + raise InvalidOperation("KCHTMPL0005E", {'name': name}) + + if template.info['name'] != name and \ + template.info['name'] in session.get_list('template'): + raise InvalidOperation("KCHTMPL0001E", + {'name': template.info['name']}) + + if template.info['name'] != name: + session.delete('template', name) + + session.store('template', template.info['name'], + template.info, get_kimchi_version()) + + except InvalidOperation: raise - return ident + except Exception, e: + raise OperationFailed('KCHTMPL0032E', {'err': e.message}) + + return template.info['name']
def validate_memory(memory): diff --git a/tests/test_template.py b/tests/test_template.py index 158bbeb..f0ef030 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -256,7 +256,11 @@ class TemplateTests(unittest.TestCase): self.assertEquals(1024, update_tmpl['memory']['maxmemory'])
# Update cdrom - cdrom_data = {'cdrom': '/tmp/mock2.iso'} + cdrom_data = {'cdrom': 'inexistent.iso'} + resp = self.request(new_tmpl_uri, json.dumps(cdrom_data), 'PUT') + self.assertEquals(400, resp.status) + + cdrom_data = {'cdrom': '/tmp/existent.iso'} resp = self.request(new_tmpl_uri, json.dumps(cdrom_data), 'PUT') self.assertEquals(200, resp.status) update_tmpl = json.loads(resp.read())