Merge "Fix cloud-init configuration file issue with duplicate headers"

releasenotes/notes/bug-2011309-bac68bbc7ece4db9.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes a bug where the cloud-init configuration file contained duplicate
+    headers when userdata was provided.
+    `LP#[2011309] <https://storyboard.openstack.org/#!/story/[2011309]>`__

trove/instance/models.py

@@ -19,6 +19,7 @@ import base64
 import json
 import os.path
 import re
+import yaml
 
 from datetime import datetime
 from datetime import timedelta
@@ -66,6 +67,8 @@ LOG = logging.getLogger(__name__)
 AGENT_INVALID_STATUSES = ["BUILD", "REBOOT", "RESIZE", "PROMOTE", "EJECT",
                           "UPGRADE"]
 
+CLOUDINIT_HEADER = "#cloud-config\n"
+
 
 def ip_visible(ip, white_list_regex, black_list_regex):
     if re.search(white_list_regex, ip) and not re.search(black_list_regex, ip):
@@ -969,29 +972,26 @@ class BaseInstance(SimpleInstance):
 
     def prepare_cloud_config(self, files):
         # This method returns None if the files argument is None
-        userdata = None
+        if not files:
+            return ""
 
-        if files:
-            userdata = (
-                "#cloud-config\n"
-                "write_files:\n"
-            )
+        injected_config_owner = CONF.get('injected_config_owner')
+        injected_config_group = CONF.get('injected_config_group')
 
-            injected_config_owner = CONF.get('injected_config_owner')
-            injected_config_group = CONF.get('injected_config_group')
-            for filename, content in files.items():
-                ud = encodeutils.safe_encode(content)
-                body_userdata = (
-                    "- encoding: b64\n"
-                    "  owner: %s:%s\n"
-                    "  path: %s\n"
-                    "  content: %s\n" % (
-                        injected_config_owner, injected_config_group, filename,
-                        encodeutils.safe_decode(base64.b64encode(ud)))
-                )
-                userdata = userdata + body_userdata
+        write_files = []
+        for filename, content in files.items():
+            ud = encodeutils.safe_encode(content)
+            write_files.append({
+                "encoding": "b64",
+                "owner": f"{injected_config_owner}:{injected_config_group}",
+                "path": filename,
+                "content": encodeutils.safe_decode(base64.b64encode(ud))
+            })
 
-        return userdata if userdata else ""
+        cloud_config = {
+            "write_files": write_files
+        }
+        return CLOUDINIT_HEADER + yaml.dump(cloud_config)
 
     @property
     def datastore_registry_ext(self):
@@ -1121,6 +1121,28 @@ class BaseInstance(SimpleInstance):
                 userdata = f.read()
         return userdata
 
+    @staticmethod
+    def combine_cloudinit_userdata(cloudinit, userdata):
+        cloudinit = yaml.safe_load(cloudinit)
+        try:
+            # in case the userdata is not a valid yaml
+            userdata = yaml.safe_load(userdata)
+        except yaml.YAMLError as e:
+            LOG.error("Failed to parse userdata: %s. The error was: %s",
+                      userdata,
+                      str(e))
+            return CLOUDINIT_HEADER + yaml.dump(cloudinit)
+        if isinstance(userdata, dict):
+            # in case the userdata contains write_files directive
+            if userdata.get('write_files') and cloudinit.get('write_files'):
+                cloudinit['write_files'].extend(userdata['write_files'])
+                userdata.pop('write_files')
+            cloudinit.update(userdata)
+        else:
+            LOG.error("Userdata is not a valid cloudinit config: %s",
+                      userdata)
+        return CLOUDINIT_HEADER + yaml.dump(cloudinit)
+
 
 class FreshInstance(BaseInstance):
 

trove/taskmanager/models.py

@@ -1110,8 +1110,9 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin):
             if not userdata:
                 userdata = self.prepare_cloud_config(files)
             else:
-                userdata = userdata + self.prepare_cloud_config(files)
-
+                userdata = self.combine_cloudinit_userdata(
+                    self.prepare_cloud_config(files),
+                    userdata)
             files = {}
 
         server = self.nova_client.servers.create(

trove/tests/unittests/taskmanager/test_models.py

@@ -256,6 +256,25 @@ class FreshInstanceTasksTest(BaseFreshInstanceTasksTest):
         user_data = self.freshinstancetasks.prepare_cloud_config(files)
         self.assertTrue(user_data.startswith('#cloud-config'))
 
+    def test_create_instance_combine_cloudinit_userdata(self):
+        cloudcfg = """
+        #cloud-config
+        write_files:
+          - path: /etc/myconfig.conf
+            content: This is the content of the file.
+            owner: root:root
+            encoding: b64
+        """
+        userdata = """
+        #cloud-config
+        run_command:
+             - echo "hello world"
+        """
+        cloudinit = self.freshinstancetasks.combine_cloudinit_userdata(
+            cloudcfg, userdata)
+        self.assertTrue(cloudinit.startswith('#cloud-config'))
+        self.assertEqual(cloudinit.count('cloud-config'), 1)
+
     @patch.object(DBInstance, 'get_by')
     def test_create_instance_guestconfig(self, patch_get_by):
         cfg.CONF.set_override('guest_config', self.guestconfig)