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

[Aline Manera]

On 04/18/2016 03:12 PM, Paulo Ricardo Paz Vital wrote:
> On Apr 18 01:09PM, Aline Manera wrote:
>>
>> On 04/15/2016 03:54 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           |  4 ++--
>>>   model/templates.py | 11 ++++++++++-
>>>   2 files changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/API.json b/API.json
>>> index ff505b1..2926620 100644
>>> --- a/API.json
>>> +++ b/API.json
>>> @@ -469,9 +469,9 @@
>>>                   },
>>>                   "memory": { "$ref": "#/kimchitype/memory" },
>>>                   "source_media": {
>>> -                    "description": "Path for installation media (ISO, disk, remote ISO)",
>>> +                    "description": "Path for installation media (ISO, disk, remote ISO) or netboot",
>>>                       "type" : "string",
>>> -                    "pattern" : "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$",
>>> +                    "pattern" : "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$|(netboot)",
>> What happens if my ISO file and image file is named as 'netboot'?
> Code still works in the normal way, just because the regex is clear. Or user
> inputs (1) a complete file path (starting with '/') or URL link (starting with
> 'http(s):' or '(t)ftp(s):'); or (2) the word 'netboot'.

Yeap! I haven't remembered about the path regex.

> So, even if a ISO file or image files is named as 'netboot', user needs to
> provide the full path, matching the beginning of the regex, or it will not work.
>
> See my test here:
>
> $ curl -k -u test -H "Content-Type: application/json" -H "Accept: application/json" 'https://localhost:8001/plugins/kimchi/templates' -X POST -d '{"source_media": "/var/lib/libvirt/images/netboot"}'
> Enter host password for user 'test':
>
> {
>    "cpu_info":{
>      "maxvcpus":1,
>      "vcpus":1
>    },
>    "graphics":{
>      "type":"vnc",
>      "listen":"127.0.0.1"
>    },
>    "cdrom":"/var/lib/libvirt/images/netboot",
>    "networks":[
>      "default"
>    ],
>    "icon":"plugins/kimchi/images/icon-ubuntu.png",
>    "os_distro":"ubuntu",
>    "name":"ubuntu14.04.1461002267700",
>    "disks":[
>      {
>        "index":0,
>        "format":"qcow2",
>        "pool":{
>          "type":"dir",
>          "name":"/plugins/kimchi/storagepools/default"
>        },
>        "size":10
>      }
>    ],
>    "invalid":{},
>    "os_version":"14.04",
>    "memory":{
>      "current":1024,
>      "maxmemory":1024
>    },
>    "folder":[]
> }
>> It is better to do not mix things.
>>
>> I suggest to have source_media type changed to a dict with path and type
>> values.
>>
>> Example:
>>
>> source_media: {type: disk/netboot, path: <only applicable for disk type}
>>
>> To specify a disk input, use type=disk and require a path value
>> To create a netboot template, specify type=netboot and raise an error if any
>> path is specified.
>>
> I got your point and this also works, but doesn't this idea move back, let's
> say half-step, to the point before Ramon's patch to automatically detect
> the type of media?

The user will need to specify netboot or local boot, right? So I am not 
sure what you meant about that.

> I mean, implementing your suggestion I'll be moving most part of the user's
> imput check to the model/templates.py files, instead of let JSON regex works.
> IMO, the current implementation keeps things simple to read and maintain.
>

The regex will keep on JSON schema as it is today but in a different 
data structure.
For example:

source_media: {type: object,
                           required: true,
                           properties: {"type": "string", "pattern": 
"^disk|netboot$", required: true},
                                             {"path": "string", 
"pattern": "<the same pattern used today>"}

The only validation on model will be to make sure a 'path' is set when 
type=disk.

IMO that way we keep the API semantic and it is better to a developer 
identify what the parameter is for.

Let me know your thoughts on it.

>>>                       "required": true
>>>                   },
>>>                   "disks": {
>>> diff --git a/model/templates.py b/model/templates.py
>>> index a02099c..043fe49 100644
>>> --- a/model/templates.py
>>> +++ b/model/templates.py
>>> @@ -65,6 +65,12 @@ class TemplatesModel(object):
>>>           # get source_media
>>>           path = params.pop("source_media")
>>>
>>> +        # Check if source_media is 'netboot'
>>> +        if path == 'netboot':
>>> +            params['os_distro'] = 'unknown'
>>> +            params['os_version'] = 'unknown'
>>> +            return self.save_template(params)
>>> +
>>>           # not local image: set as remote ISO
>>>           if urlparse.urlparse(path).scheme in ["http", "https", "tftp", "ftp",
>>>                                                 "ftps"]:
>>> @@ -104,7 +110,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)
>>>
>>>           # Validate cpu info
>>>           t.cpuinfo_validate()