Generate an init file that can be used with exported images #477

gbenhaim · 2017-03-14T17:09:21Z

The generated init file will represent the env and will contain
relative paths to the exported images.

Signed-off-by: gbenhaim [email protected]

closes #473

nvgoldin

Looks good!

About the overall approach - I'm still not sure if "positive" checking wouldn't have made it easier:

You have a list of "filters", which can even be callables(like re.match partials).
You traverse through the graph, always keeping your current 'path'. For each node you visit you check if any of the callables returns True. If it does you delete it and never visit his sons(so BFS would make sense here).
So when you finish travelling, you get only the nodes which passed(or should I say did not pass) the filters.

But, you'd still have to decide how to construct "paths" when you meet lists versus dicts... so perhaps its not worth it.

nvgoldin · 2017-03-14T23:13:25Z

lago/plugins/vm.py

@@ -27,6 +27,7 @@
 create an alternative implementation of the provisioning details for the VM,
 for example, using a remote libvirt instance or similar.
 """
+from copy import deepcopy


We have our "specific" implementation of deepcopy in 'utils.py', I guess here its safe? (it was added to avoid the cross-references issue iirc)

What do you mean by "cross-references" ?

See the discussion here: #346 (review)

Not 100% if its relevant, just checking.

nvgoldin · 2017-03-14T23:14:31Z

lago/plugins/vm.py

@@ -364,6 +365,10 @@ def metadata(self):
    def disks(self):
        return self._spec['disks'][:]

+    @property
+    def spec(self):


This makes the new spec somewhat "read only", no? was that the intention?

Yes.
Any change to the spec will effect the structure of the env. IMO changes to the env should be done with specific object's methods (in this case the 'vm' object), and not directly to its data.

nvgoldin · 2017-03-14T23:15:31Z

lago/utils.py

+    all the metadata dicts from all domains disks will be removed.
+
+    Args:
+        spec (dict): spec file to remove keys from


Its a dict(not a file) :) as you wrote..

nvgoldin · 2017-03-14T23:16:17Z

lago/utils.py

+        paths (list): list of paths to the keys that should be removed
+
+    Returns:
+        None


Please add 'raises' here(look elsewhere in the file the exact syntax so it would format sphinixily nicely)

nvgoldin · 2017-03-14T23:17:09Z

lago/utils.py

+        elif len(path) == 1:
+            key = path.pop()
+            if not isinstance(spec, dict):
+                raise LagoUserException(


I would print a more detailed message here, that hints what to do or what the user did wrong.

nvgoldin · 2017-03-14T23:21:55Z

lago/utils.py

+                        'does not exist'.format('/'.join(path), current)
+                    )
+                except TypeError:
+                    raise LagoUserException(


when is this condition met and why? and how is it different from line 726?

Example for each exception (by the order I catch them):

You have tried to remove the following key - "template". Keys can not be removed from type <type 'list'> Please verify that path - "domains/*/disks/template" is valid

Glob char * should refer only to dict or list, not to <type 'int'> Please fix path - "domains/*/memory/*/some_key"

Malformed path "domains/*/gpu/*", key "gpu" does not exist

Malformed path "domains/*/disks/root/vda", can not get by key from type <type 'list'>

nvgoldin · 2017-03-14T23:24:32Z

lago/virt.py

+            for _, domain in spec['domains'].viewitems():
+                for disk in domain['disks']:
+                    if disk['type'] == 'template':
+                        disk['template_type'] = 'qcow2'


Not sure if this is the right place for this logic. Should probably be a different method.

This method is for generating an init file that can be used with the exported images 'out of the box'.
For general spec you should use get_env_spec

nvgoldin · 2017-03-14T23:27:15Z

lago/virt.py

+                        '$LAGO_INITFILE_PATH', os.path.basename(disk['path'])
+                    )
+
+            with open(os.path.join(dst_dir, 'LagoInitFile'), 'wt') as f:


The dumping filename should be configuable, moreover it should drop it to stdout by default(like ansible_hosts cmd), so you could do something like:

lago dump-init >> my-init-el73-2011

Not sure if its an issue but if the file already exists(LagoInitFile) it will override it.

This method is called only when exporting images, so it makes sense to write directly to a file.
I agree that the filename should be configurable.

nvgoldin · 2017-03-14T23:28:20Z

lago/prefix.py

@@ -961,7 +961,8 @@ def _create_disks(
                    'dev': disk['dev'],
                    'format': disk['format'],
                    'metadata': metadata,
-                    'type': disk['type']
+                    'type': disk['type'],
+                    'name': disk['name']


nvgoldin · 2017-03-14T23:29:34Z

lago/utils.py

+            spec.pop(key, None)
+        else:
+            current = path[0]
+            if current == '*':


probably better to add this as a default to the function:

def .... (..., sep='*')

nvgoldin · 2017-03-15T17:31:15Z

lago/utils.py

    """

    def remove_key(path, spec):
        if len(path) == 0:
            return
        elif len(path) == 1:
            key = path.pop()
-            if not isinstance(spec, dict):
+            if not isinstance(spec, collections.Mapping):


Cool. This should work for OrderedDicts also?

nvgoldin · 2017-03-15T17:31:35Z

lago/utils.py

-                    format(key, spec, type(spec))
+                    'You have tried to remove the following key - "{key}".\n'
+                    'Keys can not be removed from type {spec_type}\n'
+                    'Please verify that path - "{{path}}" is valid'.format(


gbenhaim · 2017-03-16T09:53:08Z

ci merge please

The generated init file will represent the env and will contain relative paths to the exported images. Signed-off-by: gbenhaim <[email protected]>

gbenhaim · 2017-03-16T11:14:37Z

ci test please

gbenhaim · 2017-03-16T12:02:06Z

ci merge please

gbenhaim added the enhancement label Mar 14, 2017

gbenhaim added this to the Demo Tool milestone Mar 14, 2017

gbenhaim requested a review from nvgoldin March 14, 2017 17:09

gbenhaim force-pushed the export_init_file branch from 39bde16 to 5fd1373 Compare March 14, 2017 17:10

nvgoldin reviewed Mar 14, 2017

View reviewed changes

gbenhaim force-pushed the export_init_file branch from 5876889 to db1efb5 Compare March 15, 2017 16:34

nvgoldin approved these changes Mar 15, 2017

View reviewed changes

gbenhaim force-pushed the export_init_file branch from db1efb5 to 57a17c5 Compare March 16, 2017 09:34

Generate an init file that can be used with exported images

3441728

The generated init file will represent the env and will contain relative paths to the exported images. Signed-off-by: gbenhaim <[email protected]>

gbenhaim force-pushed the export_init_file branch from 57a17c5 to 3441728 Compare March 16, 2017 10:53

lago-bot merged commit 0ea1688 into master Mar 16, 2017

lago-bot deleted the export_init_file branch March 16, 2017 12:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate an init file that can be used with exported images #477

Generate an init file that can be used with exported images #477

gbenhaim commented Mar 14, 2017

nvgoldin left a comment

nvgoldin Mar 14, 2017

gbenhaim Mar 15, 2017

nvgoldin Mar 15, 2017

nvgoldin Mar 14, 2017

gbenhaim Mar 15, 2017

nvgoldin Mar 15, 2017

nvgoldin Mar 14, 2017

gbenhaim Mar 15, 2017

nvgoldin Mar 14, 2017

gbenhaim Mar 15, 2017

nvgoldin Mar 14, 2017

gbenhaim Mar 15, 2017

nvgoldin Mar 14, 2017

gbenhaim Mar 15, 2017

nvgoldin Mar 14, 2017

gbenhaim Mar 15, 2017

nvgoldin Mar 14, 2017

gbenhaim Mar 15, 2017

nvgoldin Mar 14, 2017

nvgoldin Mar 14, 2017

gbenhaim Mar 15, 2017

nvgoldin Mar 15, 2017

nvgoldin Mar 15, 2017

gbenhaim commented Mar 16, 2017

gbenhaim commented Mar 16, 2017

gbenhaim commented Mar 16, 2017

Generate an init file that can be used with exported images #477

Generate an init file that can be used with exported images #477

Conversation

gbenhaim commented Mar 14, 2017

nvgoldin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gbenhaim commented Mar 16, 2017

gbenhaim commented Mar 16, 2017

gbenhaim commented Mar 16, 2017