[Kimchi-devel] [PATCH] [Kimchi 1/3] Add support to create netboot templates.

[Aline Manera]

On 04/20/2016 05:22 PM, pvital at linux.vnet.ibm.com wrote:
> From: Paulo Vital <pvital at linux.vnet.ibm.com>
>
> Changed API.json and model to accept 'netboot' as source media parameter while
> creating a new template.
>
> Now, when creating a new template and specifying 'netboot' as source media
> parameter, it is assumed the template will use netboot process -
> PXE/DHCP/TFTP/(NFS/HTTP/FTP).
>
> This is part of solution to Issue #372.
>
> Signed-off-by: Paulo Vital <pvital at linux.vnet.ibm.com>
> ---
>   API.json           | 18 ++++++++++++++----
>   i18n.py            |  1 +
>   model/templates.py | 27 ++++++++++++++++++++++++---
>   vmtemplate.py      |  4 ++++
>   4 files changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/API.json b/API.json
> index 5ef4f55..285b650 100644
> --- a/API.json
> +++ b/API.json
> @@ -476,7 +476,6 @@
>           },
>           "templates_create": {
>               "type": "object",

> -            "error": "KCHTMPL0016E",

This error is raised when no source_media object is passed. So you need 
to keep it here otherwise jsonschema will raise a default error that 
will not have i18n support.

>               "properties": {
>                   "name": {
>                       "description": "The name of the template",
> @@ -505,9 +504,20 @@
>                   },
>                   "memory": { "$ref": "#/kimchitype/memory" },
>                   "source_media": {

Add additionalProperties:false, to ensure no other parameters will be 
accepted in the source_media object.

> -                    "description": "Path for installation media (ISO, disk, remote ISO)",
> -                    "type" : "string",
> -                    "pattern" : "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$",
> +                    "type" : "object",
> +                    "properties" : {
> +                        "type": {
> +                            "description": "Type of source media: disk or netboot",
> +                            "type": "string",
> +                            "pattern": "^disk|netboot$",
> +                            "required": true
> +                        },
> +                        "path": {
> +                            "description": "Path for installation media (ISO, disk, remote ISO)",
> +                            "type": "string",
> +                            "pattern" : "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$"
> +                        }
> +                    },
>                       "required": true
>                   },
>                   "disks": {
> diff --git a/i18n.py b/i18n.py
> index 6f88fae..0e1de77 100644
> --- a/i18n.py
> +++ b/i18n.py
> @@ -157,6 +157,7 @@ messages = {
>       "KCHTMPL0001E": _("Template %(name)s already exists"),
>       "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": _("Invalid parameter specified for source media: '%(param)s'"),
>       "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"),
> diff --git a/model/templates.py b/model/templates.py
> index a02099c..61314f1 100644
> --- a/model/templates.py
> +++ b/model/templates.py
> @@ -62,10 +62,28 @@ class TemplatesModel(object):
>                   raise InvalidParameter("KCHTMPL0003E", {'network': net_name,
>                                                           'template': name})
>
> -        # get source_media
> -        path = params.pop("source_media")
> +        # Get source_media
> +        source_media = params.get("source_media", None)
> +

> +        if source_media is None:
> +            raise InvalidParameter("KCHTMPL0005E", {'param': 'type = None'})
> +

As you marked source_media and type as required parameters in jsonschema 
you don't need to do that validation here as at this time source_media 
will be a valid input.

> +        if source_media['type'] == 'netboot':
> +            params['os_distro'] = 'unknown'
> +            params['os_version'] = 'unknown'
> +            return self.save_template(params)
> +        elif source_media['type'] == 'disk':
> +            # Get path of source media if it's based on disk type.
> +            path = source_media.get('path', None)

> +        else:
> +            raise InvalidParameter("KCHTMPL0005E", {'param': 'type = %s' %
> +                                                    source_media['type']})
> +

If you add "additionalProperties:false" to jsonschema, you can remove 
the else statement above and source_media will be always a valid parameter.

> +        if path is None:
> +            raise InvalidParameter("KCHTMPL0016E")
>
>           # not local image: set as remote ISO
> +        path = path.encode('utf-8')
>           if urlparse.urlparse(path).scheme in ["http", "https", "tftp", "ftp",
>                                                 "ftps"]:
>               params["cdrom"] = path
> @@ -104,7 +122,10 @@ class TemplatesModel(object):
>       def save_template(self, params):
>
>           # Creates the template class with necessary information
> -        t = LibvirtVMTemplate(params, scan=True, conn=self.conn)

> +        if 'cdrom' not in params.keys():
> +            t = LibvirtVMTemplate(params, conn=self.conn)
> +        else:
> +            t = LibvirtVMTemplate(params, scan=True, conn=self.conn)

Why did you change it? I don't think it is related to this netboot 
feature. Is there any bug you are trying to fix? or... ?

>           # Validate cpu info
>           t.cpuinfo_validate()
> diff --git a/vmtemplate.py b/vmtemplate.py
> index a223beb..3e4418f 100644
> --- a/vmtemplate.py
> +++ b/vmtemplate.py
> @@ -123,6 +123,10 @@ class VMTemplate(object):
>       def _get_os_info(self, args, scan):
>           distro = version = 'unknown'

> +        if 'source_media' in args.keys():
> +            if args['source_media']['type'] == 'netboot':
> +                return distro, version
> +



>           # Identify the cdrom if present
>           iso = args.get('cdrom', '')
>           if len(iso) > 0: