From 6ad068d1ae175d784481fe8f8e190b2721a221f5 Mon Sep 17 00:00:00 2001
From: Joshua Harlow <harlowja@yahoo-inc.com>
Date: Wed, 17 Apr 2013 10:17:09 -0700
Subject: Altering the order of merging.

---
 cloudinit/handlers/cloud_config.py | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

(limited to 'cloudinit/handlers/cloud_config.py')

diff --git a/cloudinit/handlers/cloud_config.py b/cloudinit/handlers/cloud_config.py
index d30d6338..7678a5b0 100644
--- a/cloudinit/handlers/cloud_config.py
+++ b/cloudinit/handlers/cloud_config.py
@@ -39,7 +39,6 @@ class CloudConfigPartHandler(handlers.Handler):
         self.cloud_buf = None
         self.cloud_fn = paths.get_ipath("cloud_config")
         self.file_names = []
-        self.mergers = [DEF_MERGERS]
 
     def list_types(self):
         return [
@@ -89,13 +88,9 @@ class CloudConfigPartHandler(handlers.Handler):
         return all_mergers
 
     def _merge_part(self, payload, headers):
-        next_mergers = self._extract_mergers(payload, headers)
-        # Use the merger list from the last call, since it is the one
-        # that will be defining how to merge with the next payload.
-        curr_mergers = list(self.mergers[-1])
-        LOG.debug("Merging by applying %s", curr_mergers)
-        self.mergers.append(next_mergers)
-        merger = mergers.construct(curr_mergers)
+        my_mergers = self._extract_mergers(payload, headers)
+        LOG.debug("Merging by applying %s", my_mergers)
+        merger = mergers.construct(my_mergers)
         if self.cloud_buf is None:
             # First time through, merge with an empty dict...
             self.cloud_buf = {}
@@ -105,7 +100,6 @@ class CloudConfigPartHandler(handlers.Handler):
     def _reset(self):
         self.file_names = []
         self.cloud_buf = None
-        self.mergers = [DEF_MERGERS]
 
     def handle_part(self, _data, ctype, filename,  # pylint: disable=W0221
                     payload, _frequency, headers):  # pylint: disable=W0613
-- 
cgit v1.2.3


From 8441fe20fdd1d8bb195bc7d354c9e87d2f446ccd Mon Sep 17 00:00:00 2001
From: Joshua Harlow <harlowja@yahoo-inc.com>
Date: Mon, 22 Apr 2013 12:43:35 -0700
Subject: Continue working on merging.

---
 cloudinit/handlers/cloud_config.py |  7 ++--
 cloudinit/mergers/__init__.py      | 12 +++++--
 cloudinit/mergers/m_dict.py        | 73 ++++++++++++++++++++++++++------------
 cloudinit/mergers/m_list.py        | 59 ++++++++++++++++++++++++------
 4 files changed, 112 insertions(+), 39 deletions(-)

(limited to 'cloudinit/handlers/cloud_config.py')

diff --git a/cloudinit/handlers/cloud_config.py b/cloudinit/handlers/cloud_config.py
index 7678a5b0..2ae9b226 100644
--- a/cloudinit/handlers/cloud_config.py
+++ b/cloudinit/handlers/cloud_config.py
@@ -85,17 +85,16 @@ class CloudConfigPartHandler(handlers.Handler):
         all_mergers.extend(mergers_header)
         if not all_mergers:
             all_mergers = DEF_MERGERS
-        return all_mergers
+        return (payload_yaml, all_mergers)
 
     def _merge_part(self, payload, headers):
-        my_mergers = self._extract_mergers(payload, headers)
+        (payload_yaml, my_mergers) = self._extract_mergers(payload, headers)
         LOG.debug("Merging by applying %s", my_mergers)
         merger = mergers.construct(my_mergers)
         if self.cloud_buf is None:
             # First time through, merge with an empty dict...
             self.cloud_buf = {}
-        self.cloud_buf = merger.merge(self.cloud_buf,
-                                      util.load_yaml(payload))
+        self.cloud_buf = merger.merge(self.cloud_buf, payload_yaml)
 
     def _reset(self):
         self.file_names = []
diff --git a/cloudinit/mergers/__init__.py b/cloudinit/mergers/__init__.py
index 2702496b..221e93b5 100644
--- a/cloudinit/mergers/__init__.py
+++ b/cloudinit/mergers/__init__.py
@@ -55,6 +55,9 @@ class UnknownMerger(object):
         if not meth:
             meth = self._handle_unknown
             args.insert(0, method_name)
+        LOG.debug("Merging '%s' into '%s' using method '%s' of '%s'",
+                  type_name, type_utils.obj_name(merge_with),
+                  meth.__name__, self)
         return meth(*args)
 
 
@@ -66,6 +69,9 @@ class LookupMerger(UnknownMerger):
         else:
             self._lookups = lookups
 
+    def __str__(self):
+        return 'LookupMerger: (%s)' % (len(self._lookups))
+
     # For items which can not be merged by the parent this object
     # will lookup in a internally maintained set of objects and
     # find which one of those objects can perform the merge. If
@@ -78,6 +84,8 @@ class LookupMerger(UnknownMerger):
                 # First one that has that method/attr gets to be
                 # the one that will be called
                 meth = getattr(merger, meth_wanted)
+                LOG.debug(("Merging using located merger '%s'"
+                            " since it had method '%s'"), merger, meth_wanted)
                 break
         if not meth:
             return UnknownMerger._handle_unknown(self, meth_wanted,
@@ -87,9 +95,9 @@ class LookupMerger(UnknownMerger):
 
 def dict_extract_mergers(config):
     parsed_mergers = []
-    raw_mergers = config.get('merge_how')
+    raw_mergers = config.pop('merge_how', None)
     if raw_mergers is None:
-        raw_mergers = config.get('merge_type')
+        raw_mergers = config.pop('merge_type', None)
     if raw_mergers is None:
         return parsed_mergers
     if isinstance(raw_mergers, (str, basestring)):
diff --git a/cloudinit/mergers/m_dict.py b/cloudinit/mergers/m_dict.py
index 929d3865..2c1c845f 100644
--- a/cloudinit/mergers/m_dict.py
+++ b/cloudinit/mergers/m_dict.py
@@ -20,30 +20,59 @@
 class Merger(object):
     def __init__(self, merger, opts):
         self._merger = merger
-        self._not_overwrite = 'not_overwrite' in opts
+        # Affects merging behavior...
+        self._method = 'replace'
+        for m in ['replace', 'no_replace']:
+            if m in opts:
+                self._method = m
+                break
+        # Affect how recursive merging is done on other primitives
+        self._recurse_str = 'recurse_str' in opts
+        self._recurse_dict = True
+        self._recurse_array = 'recurse_array' in opts
+        self._allow_delete = 'allow_delete' in opts
+
+    def __str__(self):
+        s = ('DictMerger: (method=%s,recurse_str=%s,'
+             'recurse_dict=%s,recurse_array=%s,allow_delete=%s)')
+        s  = s % (self._method,
+                  self._recurse_str,
+                  self._recurse_dict,
+                  self._recurse_array,
+                  self._allow_delete)
+        return s
+
+    def _do_dict_replace(self, value, merge_with, do_replace=True):
+
+        def merge_same_key(old_v, new_v):
+            if do_replace:
+                return new_v
+            if isinstance(new_v, (list, tuple)) and self._recurse_array:
+                return self._merger.merge(old_v, new_v)
+            if isinstance(new_v, (str, basestring)) and self._recurse_str:
+                return self._merger.merge(old_v, new_v)
+            if isinstance(new_v, (dict)) and self._recurse_dict:
+                return self._merger.merge(old_v, new_v)
+            # Otherwise leave it be...
+            return old_v
 
-    # This merging algorithm will attempt to merge with
-    # another dictionary, on encountering any other type of object
-    # it will not merge with said object, but will instead return
-    # the original value
-    #
-    # On encountering a dictionary, it will create a new dictionary
-    # composed of the original and the one to merge with, if 'overwrite'
-    # is enabled then keys that exist in the original will be overwritten
-    # by keys in the one to merge with (and associated values). Otherwise
-    # if not in overwrite mode the 2 conflicting keys themselves will
-    # be merged.
-    def _on_dict(self, value, merge_with):
-        if not isinstance(merge_with, (dict)):
-            return value
-        merged = dict(value)
         for (k, v) in merge_with.items():
-            if k in merged:
-                if self._not_overwrite:
-                    # Attempt to merge them....
-                    merged[k] = self._merger.merge(merged[k], v)
+            if k in value:
+                if v is None and self._allow_delete:
+                    value.pop(k)
                 else:
-                    merged[k] = v
+                    value[k] = merge_same_key(value[k], v)
             else:
-                merged[k] = v
+                value[k] = v
+        return value
+
+    def _on_dict(self, value, merge_with):
+        if not isinstance(merge_with, (dict)):
+            return value
+        if self._method == 'replace':
+            merged = self._do_dict_replace(dict(value), merge_with)
+        elif self._method == 'no_replace':
+            merged = self._do_dict_replace(dict(value), merge_with, False)
+        else:
+            raise NotImplementedError("Unknown merge type %s" % (self._method))
         return merged
diff --git a/cloudinit/mergers/m_list.py b/cloudinit/mergers/m_list.py
index 208c5f52..c6a23d85 100644
--- a/cloudinit/mergers/m_list.py
+++ b/cloudinit/mergers/m_list.py
@@ -20,18 +20,55 @@
 class Merger(object):
     def __init__(self, merger, opts):
         self._merger = merger
-        self._extend = 'extend' in opts
+        # Affects merging behavior...
+        self._method = 'replace'
+        for m in ['append', 'prepend', 'replace']:
+            if m in opts:
+                self._method = m
+                break
+        # Affect how recursive merging is done on other primitives
+        self._recurse_str = 'recurse_str' in opts
+        self._recurse_dict = 'recurse_dict' in opts
+        self._recurse_array = 'recurse_array' in opts
+
+    def __str__(self):
+        return 'ListMerger: (m=%s,rs=%s,rd=%s,ra=%s)' % (self._method,
+                                                         self._recurse_str,
+                                                         self._recurse_dict,
+                                                         self._recurse_array)
 
     def _on_tuple(self, value, merge_with):
-        return self._on_list(list(value), merge_with)
+        return tuple(self._on_list(list(value), merge_with))
 
-    # On encountering a list or tuple type this action will be applied
-    # a new list will be returned, if the value to merge with is itself
-    # a list and we have been told to 'extend', then the value here will
-    # be extended with the other list.
     def _on_list(self, value, merge_with):
-        if not self._extend or not isinstance(merge_with, (tuple, list)):
-            return merge_with
-        # Leave the original list alone...
-        value = list(value)
-        return value.extend(merge_with)
+        if (self._method == 'replace' and
+            not isinstance(merge_with, (tuple, list))):
+                return merge_with
+
+        # Ok we now know that what we are merging with is a list or tuple.
+        merged_list = []
+        if self._method == 'prepend':
+            merged_list.extend(merge_with)
+            merged_list.extend(value)
+            return merged_list
+        elif self._method == 'append':
+            merged_list.extend(value)
+            merged_list.extend(merge_with)
+            return merged_list
+
+        def merge_same_index(old_v, new_v):
+            if isinstance(new_v, (list, tuple)) and self._recurse_array:
+                return self._merger.merge(old_v, new_v)
+            if isinstance(new_v, (str, basestring)) and self._recurse_str:
+                return self._merger.merge(old_v, new_v)
+            if isinstance(new_v, (dict)) and self._recurse_dict:
+                return self._merger.merge(old_v, new_v)
+            # Otherwise leave it be...
+            return old_v
+
+        # Ok now we are replacing same indexes
+        merged_list.extend(value)
+        common_len = min(len(merged_list), len(merge_with))
+        for i in xrange(0, common_len):
+            merged_list[i] = merge_same_index(merged_list[i], merge_with[i])
+        return merged_list
-- 
cgit v1.2.3


From 2b351c5435939d16ba06ec0c45847d47f4b21d51 Mon Sep 17 00:00:00 2001
From: Joshua Harlow <harlowja@yahoo-inc.com>
Date: Thu, 9 May 2013 22:34:31 -0700
Subject: Fix the cloud config merging so that it is backwards compat.

The new change for merging works well in the mergedict case
but the default merging type for cloud config needs to reflect
how yaml was loaded in bulk, which is the same as the replacing
keys merging type that is now provided.
---
 cloudinit/handlers/cloud_config.py       | 15 +++++++++++++--
 tests/data/merge_sources/expected11.yaml |  5 +++++
 tests/data/merge_sources/expected12.yaml |  5 +++++
 tests/data/merge_sources/expected2.yaml  |  4 ++--
 tests/data/merge_sources/source11-1.yaml |  5 +++++
 tests/data/merge_sources/source11-2.yaml |  3 +++
 tests/data/merge_sources/source11-3.yaml |  3 +++
 tests/data/merge_sources/source12-1.yaml |  8 ++++++++
 tests/data/merge_sources/source12-2.yaml |  5 +++++
 tests/unittests/test_merging.py          | 15 +++++++++++++++
 tests/unittests/test_userdata.py         |  2 +-
 11 files changed, 65 insertions(+), 5 deletions(-)
 create mode 100644 tests/data/merge_sources/expected11.yaml
 create mode 100644 tests/data/merge_sources/expected12.yaml
 create mode 100644 tests/data/merge_sources/source11-1.yaml
 create mode 100644 tests/data/merge_sources/source11-2.yaml
 create mode 100644 tests/data/merge_sources/source11-3.yaml
 create mode 100644 tests/data/merge_sources/source12-1.yaml
 create mode 100644 tests/data/merge_sources/source12-2.yaml

(limited to 'cloudinit/handlers/cloud_config.py')

diff --git a/cloudinit/handlers/cloud_config.py b/cloudinit/handlers/cloud_config.py
index 2ae9b226..529109ce 100644
--- a/cloudinit/handlers/cloud_config.py
+++ b/cloudinit/handlers/cloud_config.py
@@ -30,7 +30,13 @@ from cloudinit.settings import (PER_ALWAYS)
 LOG = logging.getLogger(__name__)
 
 MERGE_HEADER = 'Merge-Type'
-DEF_MERGERS = mergers.default_mergers()
+
+# Due to the way the loading of yaml configuration was done previously,
+# where previously each cloud config part was appended to a larger yaml
+# file and then finally that file was loaded as one big yaml file we need
+# to mimic that behavior by altering the default strategy to be replacing
+# keys of later mergers.
+DEF_MERGERS = mergers.string_extract_mergers('dict(replace)+list()+str()')
 
 
 class CloudConfigPartHandler(handlers.Handler):
@@ -53,6 +59,8 @@ class CloudConfigPartHandler(handlers.Handler):
         if self.file_names:
             file_lines.append("# from %s files" % (len(self.file_names)))
             for fn in self.file_names:
+                if not fn:
+                    fn = '?'
                 file_lines.append("# %s" % (fn))
             file_lines.append("")
         if self.cloud_buf is not None:
@@ -111,7 +119,10 @@ class CloudConfigPartHandler(handlers.Handler):
             return
         try:
             self._merge_part(payload, headers)
-            self.file_names.append(filename)
+            # Ensure filename is ok to store
+            for i in ("\n", "\r", "\t"):
+                filename = filename.replace(i, " ")
+            self.file_names.append(filename.strip())
         except:
             util.logexc(LOG, "Failed at merging in cloud config part from %s",
                         filename)
diff --git a/tests/data/merge_sources/expected11.yaml b/tests/data/merge_sources/expected11.yaml
new file mode 100644
index 00000000..c0530dc3
--- /dev/null
+++ b/tests/data/merge_sources/expected11.yaml
@@ -0,0 +1,5 @@
+#cloud-config
+
+a: 22
+b: 4
+c: 3
diff --git a/tests/data/merge_sources/expected12.yaml b/tests/data/merge_sources/expected12.yaml
new file mode 100644
index 00000000..0421d2c8
--- /dev/null
+++ b/tests/data/merge_sources/expected12.yaml
@@ -0,0 +1,5 @@
+#cloud-config
+
+a:
+  e:
+    y: 2
diff --git a/tests/data/merge_sources/expected2.yaml b/tests/data/merge_sources/expected2.yaml
index f5312eb1..6eccc2cf 100644
--- a/tests/data/merge_sources/expected2.yaml
+++ b/tests/data/merge_sources/expected2.yaml
@@ -1,3 +1,3 @@
-Blah: 1
+Blah: 3
 Blah2: 2
-Blah3: 3
+Blah3: [1]
diff --git a/tests/data/merge_sources/source11-1.yaml b/tests/data/merge_sources/source11-1.yaml
new file mode 100644
index 00000000..ee29d681
--- /dev/null
+++ b/tests/data/merge_sources/source11-1.yaml
@@ -0,0 +1,5 @@
+#cloud-config
+
+a: 1
+b: 2
+c: 3
diff --git a/tests/data/merge_sources/source11-2.yaml b/tests/data/merge_sources/source11-2.yaml
new file mode 100644
index 00000000..a9914c34
--- /dev/null
+++ b/tests/data/merge_sources/source11-2.yaml
@@ -0,0 +1,3 @@
+#cloud-config
+
+b: 4
diff --git a/tests/data/merge_sources/source11-3.yaml b/tests/data/merge_sources/source11-3.yaml
new file mode 100644
index 00000000..8f2b8944
--- /dev/null
+++ b/tests/data/merge_sources/source11-3.yaml
@@ -0,0 +1,3 @@
+#cloud-config
+
+a: 22
diff --git a/tests/data/merge_sources/source12-1.yaml b/tests/data/merge_sources/source12-1.yaml
new file mode 100644
index 00000000..09e7c899
--- /dev/null
+++ b/tests/data/merge_sources/source12-1.yaml
@@ -0,0 +1,8 @@
+#cloud-config
+
+a:
+  c: 1
+  d: 2
+  e:
+    z: a
+    y: b
diff --git a/tests/data/merge_sources/source12-2.yaml b/tests/data/merge_sources/source12-2.yaml
new file mode 100644
index 00000000..0421d2c8
--- /dev/null
+++ b/tests/data/merge_sources/source12-2.yaml
@@ -0,0 +1,5 @@
+#cloud-config
+
+a:
+  e:
+    y: 2
diff --git a/tests/unittests/test_merging.py b/tests/unittests/test_merging.py
index dddf8c6c..ba1c67d7 100644
--- a/tests/unittests/test_merging.py
+++ b/tests/unittests/test_merging.py
@@ -167,6 +167,21 @@ class TestSimpleRun(helpers.ResourceUsingTestCase):
         d = util.mergemanydict([a, b])
         self.assertEquals(c, d)
 
+    def test_compat_merges_dict(self):
+        a = {
+            'Blah': 1,
+            'Blah2': 2,
+            'Blah3': 3,
+        }
+        b = {
+            'Blah': 1,
+            'Blah2': 2,
+            'Blah3': [1],
+        }
+        c = _old_mergedict(a, b)
+        d = util.mergemanydict([a, b])
+        self.assertEquals(c, d)
+
     def test_compat_merges_list(self):
         a = {'b': [1, 2, 3]}
         b = {'b': [4, 5]}
diff --git a/tests/unittests/test_userdata.py b/tests/unittests/test_userdata.py
index 5fb9acd9..0ebb0484 100644
--- a/tests/unittests/test_userdata.py
+++ b/tests/unittests/test_userdata.py
@@ -108,7 +108,7 @@ p: 1
         contents = util.load_yaml(contents)
         self.assertEquals(contents['run'], ['b', 'c', 'stuff', 'morestuff'])
         self.assertEquals(contents['a'], 'be')
-        self.assertEquals(contents['e'], 'fg')
+        self.assertEquals(contents['e'], [1, 2, 3])
         self.assertEquals(contents['p'], 1)
 
     def test_unhandled_type_warning(self):
-- 
cgit v1.2.3


From b22a82787378e442cb477d2368ccd7653fd95594 Mon Sep 17 00:00:00 2001
From: Joshua Harlow <harlowja@yahoo-inc.com>
Date: Thu, 9 May 2013 22:47:14 -0700
Subject: Adjust comment on why we are merging cloud config the way we are.

---
 cloudinit/handlers/cloud_config.py | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

(limited to 'cloudinit/handlers/cloud_config.py')

diff --git a/cloudinit/handlers/cloud_config.py b/cloudinit/handlers/cloud_config.py
index 529109ce..c97ca3e8 100644
--- a/cloudinit/handlers/cloud_config.py
+++ b/cloudinit/handlers/cloud_config.py
@@ -35,7 +35,19 @@ MERGE_HEADER = 'Merge-Type'
 # where previously each cloud config part was appended to a larger yaml
 # file and then finally that file was loaded as one big yaml file we need
 # to mimic that behavior by altering the default strategy to be replacing
-# keys of later mergers.
+# keys of prior merges.
+#
+#
+# For example
+# #file 1
+# a: 3
+# #file 2
+# a: 22
+# #combined file (comments not included)
+# a: 3
+# a: 22
+#
+# This gets loaded into yaml with final result {'a': 22}
 DEF_MERGERS = mergers.string_extract_mergers('dict(replace)+list()+str()')
 
 
-- 
cgit v1.2.3