Generate an init file that can be used with exported images by gbenhaim · Pull Request #477 · lago-project/lago

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 galbh2@gmail.com

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

 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

        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

+    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

+        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

+        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

+                        '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

+            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_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

                    'metadata': metadata,
-                    'type': disk['type']
+                    'type': disk['type'],
+                    'name': disk['name']


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

+            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

        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

-                    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 <galbh2@gmail.com>

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 <galbh2@gmail.com>

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

Uh oh!

Conversation

gbenhaim commented Mar 14, 2017

Uh oh!

nvgoldin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbenhaim commented Mar 16, 2017

Uh oh!

gbenhaim commented Mar 16, 2017

Uh oh!

gbenhaim commented Mar 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants