Merge ~xiaofengw/cloud-init:xiaofengw-fix-post-script into cloud-init:master

[Xiaofeng Wang]

VMWare: Trigger the post customization script via cc_scripts module.

cloud-init does not trigger reboots of a VM therefore adding custom
scripts to rc.local does not execute the post scripts. This patch
moves post-scripts into per-instance scripts dir and has cc_scripts
module run the post-scripts.

Also in this branch:
  - Remove the sh interpreter and execute the customization script
    directly.
  - Update the unit test.

LP: #1833192

[Xiaofeng Wang (xiaofengw)]

Test launched:
1. tox test passed.
2. Deploy cloud-init and perform the guest customization(specify a custom script), make sure the custom script is run and only run once.The guest customization finished successfully.
3. Deploy cloud-init and perform the guest customization(without custom script), make sure the guest customization finished successfully.
4. Repeat test 2 with sh, bash, dash custom script.

[Ryan Harper (raharper)]

This looks like a bug; did you file a bug to track the issue? If not, could you do that please?

I've added some comments in-line and some questions so I can provide better feedback.

review: Needs Information

[Xiaofeng Wang (xiaofengw)]

> This looks like a bug; did you file a bug to track the issue? If not, could
> you do that please?
>
> I've added some comments in-line and some questions so I can provide better
> feedback.
The bug is created: https://bugs.launchpad.net/ubuntu/+source/evince/+bug/1833192
Thanks.

ecac298... by Xiaofeng Wang on 2019-06-18

Address the review comments from Ryan.
Changes to be committed:
 modified: cloudinit/sources/DataSourceOVF.py
 modified: cloudinit/sources/helpers/vmware/imc/config_custom_script.py
 modified: tests/unittests/test_vmware/test_custom_script.py

[Ryan Harper (raharper)]

One minor change requested.

review: Needs Fixing

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:ecac298c51dd758d7757834f3fc41f28e2b5a071
https://jenkins.ubuntu.com/server/job/cloud-init-ci/743/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/743/rebuild

review: Approve (continuous-integration)

5e1dac2... by Xiaofeng Wang on 2019-06-20

Address comments to move variable definition to the begining

[Xiaofeng Wang (xiaofengw)]

> One minor change requested.
Fixed, PTAL. Thanks.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:5e1dac265dabf225718d8bcd8383b8760ca2f27f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/779/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/779/rebuild

review: Approve (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
https://jenkins.ubuntu.com/server/job/cloud-init-autoland-test/260/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

review: Needs Fixing (continuous-integration)

[Ryan Harper (raharper)]

Thanks!

review: Approve

[Server Team CI bot (server-team-bot)]

review: Approve (continuous-integration)

diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py
index 70e7a5c..dd941d2 100644
--- a/cloudinit/sources/DataSourceOVF.py
+++ b/cloudinit/sources/DataSourceOVF.py
@@ -148,6 +148,9 @@ class DataSourceOVF(sources.DataSource):
                     product_marker, os.path.join(self.paths.cloud_dir, 'data'))
                 special_customization = product_marker and not hasmarkerfile
                 customscript = self._vmware_cust_conf.custom_script_name
+                ccScriptsDir = os.path.join(
+                    self.paths.get_cpath("scripts"),
+                    "per-instance")
             except Exception as e:
                 _raise_error_status(
                     "Error parsing the customization Config File",
@@ -201,7 +204,9 @@ class DataSourceOVF(sources.DataSource):
 
                 if customscript:
                     try:
-                        postcust = PostCustomScript(customscript, imcdirpath)
+                        postcust = PostCustomScript(customscript,
+                                                    imcdirpath,
+                                                    ccScriptsDir)
                         postcust.execute()
                     except Exception as e:
                         _raise_error_status(
diff --git a/cloudinit/sources/helpers/vmware/imc/config_custom_script.py b/cloudinit/sources/helpers/vmware/imc/config_custom_script.py
index a7d4ad9..9f14770 100644
--- a/cloudinit/sources/helpers/vmware/imc/config_custom_script.py
+++ b/cloudinit/sources/helpers/vmware/imc/config_custom_script.py
@@ -1,5 +1,5 @@
 # Copyright (C) 2017 Canonical Ltd.
-# Copyright (C) 2017 VMware Inc.
+# Copyright (C) 2017-2019 VMware Inc.
 #
 # Author: Maitreyee Saikia <[e-mail]>
 #
@@ -8,7 +8,6 @@
 import logging
 import os
 import stat
-from textwrap import dedent
 
 from cloudinit import util
 
@@ -20,12 +19,15 @@ class CustomScriptNotFound(Exception):
 
 
 class CustomScriptConstant(object):
-    RC_LOCAL = "/etc/rc.local"
-    POST_CUST_TMP_DIR = "/root/.customization"
-    POST_CUST_RUN_SCRIPT_NAME = "post-customize-guest.sh"
-    POST_CUST_RUN_SCRIPT = os.path.join(POST_CUST_TMP_DIR,
-                                        POST_CUST_RUN_SCRIPT_NAME)
-    POST_REBOOT_PENDING_MARKER = "/.guest-customization-post-reboot-pending"
+    CUSTOM_TMP_DIR = "/root/.customization"
+
+    # The user defined custom script
+    CUSTOM_SCRIPT_NAME = "customize.sh"
+    CUSTOM_SCRIPT = os.path.join(CUSTOM_TMP_DIR,
+                                 CUSTOM_SCRIPT_NAME)
+    POST_CUSTOM_PENDING_MARKER = "/.guest-customization-post-reboot-pending"
+    # The cc_scripts_per_instance script to launch custom script
+    POST_CUSTOM_SCRIPT_NAME = "post-customize-guest.sh"
 
 
 class RunCustomScript(object):
@@ -39,10 +41,19 @@ class RunCustomScript(object):
             raise CustomScriptNotFound("Script %s not found!! "
                                        "Cannot execute custom script!"
                                        % self.scriptpath)
+
+        util.ensure_dir(CustomScriptConstant.CUSTOM_TMP_DIR)
+
+        LOG.debug("Copying custom script to %s",
+                  CustomScriptConstant.CUSTOM_SCRIPT)
+        util.copy(self.scriptpath, CustomScriptConstant.CUSTOM_SCRIPT)
+
         # Strip any CR characters from the decoded script
-        util.load_file(self.scriptpath).replace("\r", "")
-        st = os.stat(self.scriptpath)
-        os.chmod(self.scriptpath, st.st_mode | stat.S_IEXEC)
+        content = util.load_file(
+            CustomScriptConstant.CUSTOM_SCRIPT).replace("\r", "")
+        util.write_file(CustomScriptConstant.CUSTOM_SCRIPT,
+                        content,
+                        mode=0o544)
 
 
 class PreCustomScript(RunCustomScript):
@@ -50,104 +61,34 @@ class PreCustomScript(RunCustomScript):
         """Executing custom script with precustomization argument."""
         LOG.debug("Executing pre-customization script")
         self.prepare_script()
-        util.subp(["/bin/sh", self.scriptpath, "precustomization"])
+        util.subp([CustomScriptConstant.CUSTOM_SCRIPT, "precustomization"])
 
 
 class PostCustomScript(RunCustomScript):
-    def __init__(self, scriptname, directory):
+    def __init__(self, scriptname, directory, ccScriptsDir):
         super(PostCustomScript, self).__init__(scriptname, directory)
-        # Determine when to run custom script. When postreboot is True,
-        # the user uploaded script will run as part of rc.local after
-        # the machine reboots. This is determined by presence of rclocal.
-        # When postreboot is False, script will run as part of cloud-init.
-        self.postreboot = False
-
-    def _install_post_reboot_agent(self, rclocal):
-        """
-        Install post-reboot agent for running custom script after reboot.
-        As part of this process, we are editing the rclocal file to run a
-        VMware script, which in turn is resposible for handling the user
-        script.
-        @param: path to rc local.
-        """
-        LOG.debug("Installing post-reboot customization from %s to %s",
-                  self.directory, rclocal)
-        if not self.has_previous_agent(rclocal):
-            LOG.info("Adding post-reboot customization agent to rc.local")
-            new_content = dedent("""
-                # Run post-reboot guest customization
-                /bin/sh %s
-                exit 0
-                """) % CustomScriptConstant.POST_CUST_RUN_SCRIPT
-            existing_rclocal = util.load_file(rclocal).replace('exit 0\n', '')
-            st = os.stat(rclocal)
-            # "x" flag should be set
-            mode = st.st_mode | stat.S_IEXEC
-            util.write_file(rclocal, existing_rclocal + new_content, mode)
-
-        else:
-            # We don't need to update rclocal file everytime a customization
-            # is requested. It just needs to be done for the first time.
-            LOG.info("Post-reboot guest customization agent is already "
-                     "registered in rc.local")
-        LOG.debug("Installing post-reboot customization agent finished: %s",
-                  self.postreboot)
-
-    def has_previous_agent(self, rclocal):
-        searchstring = "# Run post-reboot guest customization"
-        if searchstring in open(rclocal).read():
-            return True
-        return False
-
-    def find_rc_local(self):
-        """
-        Determine if rc local is present.
-        """
-        rclocal = ""
-        if os.path.exists(CustomScriptConstant.RC_LOCAL):
-            LOG.debug("rc.local detected.")
-            # resolving in case of symlink
-            rclocal = os.path.realpath(CustomScriptConstant.RC_LOCAL)
-            LOG.debug("rc.local resolved to %s", rclocal)
-        else:
-            LOG.warning("Can't find rc.local, post-customization "
-                        "will be run before reboot")
-        return rclocal
-
-    def install_agent(self):
-        rclocal = self.find_rc_local()
-        if rclocal:
-            self._install_post_reboot_agent(rclocal)
-            self.postreboot = True
+        self.ccScriptsDir = ccScriptsDir
+        self.ccScriptPath = os.path.join(
+            ccScriptsDir,
+            CustomScriptConstant.POST_CUSTOM_SCRIPT_NAME)
 
     def execute(self):
         """
-        This method executes post-customization script before or after reboot
-        based on the presence of rc local.
+        This method copy the post customize run script to
+        cc_scripts_per_instance directory and let this
+        module to run post custom script.
         """
         self.prepare_script()
-        self.install_agent()
-        if not self.postreboot:
-            LOG.warning("Executing post-customization script inline")
-            util.subp(["/bin/sh", self.scriptpath, "postcustomization"])
-        else:
-            LOG.debug("Scheduling custom script to run post reboot")
-            if not os.path.isdir(CustomScriptConstant.POST_CUST_TMP_DIR):
-                os.mkdir(CustomScriptConstant.POST_CUST_TMP_DIR)
-            # Script "post-customize-guest.sh" and user uploaded script are
-            # are present in the same directory and needs to copied to a temp
-            # directory to be executed post reboot. User uploaded script is
-            # saved as customize.sh in the temp directory.
-            # post-customize-guest.sh excutes customize.sh after reboot.
-            LOG.debug("Copying post-customization script")
-            util.copy(self.scriptpath,
-                      CustomScriptConstant.POST_CUST_TMP_DIR + "/customize.sh")
-            LOG.debug("Copying script to run post-customization script")
-            util.copy(
-                os.path.join(self.directory,
-                             CustomScriptConstant.POST_CUST_RUN_SCRIPT_NAME),
-                CustomScriptConstant.POST_CUST_RUN_SCRIPT)
-            LOG.info("Creating post-reboot pending marker")
-            util.ensure_file(CustomScriptConstant.POST_REBOOT_PENDING_MARKER)
+
+        LOG.debug("Copying post customize run script to %s",
+                  self.ccScriptPath)
+        util.copy(
+            os.path.join(self.directory,
+                         CustomScriptConstant.POST_CUSTOM_SCRIPT_NAME),
+            self.ccScriptPath)
+        st = os.stat(self.ccScriptPath)
+        os.chmod(self.ccScriptPath, st.st_mode | stat.S_IEXEC)
+        LOG.info("Creating post customization pending marker")
+        util.ensure_file(CustomScriptConstant.POST_CUSTOM_PENDING_MARKER)
 
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_vmware/test_custom_script.py b/tests/unittests/test_vmware/test_custom_script.py
index 2d9519b..f89f815 100644
--- a/tests/unittests/test_vmware/test_custom_script.py
+++ b/tests/unittests/test_vmware/test_custom_script.py
@@ -1,10 +1,12 @@
 # Copyright (C) 2015 Canonical Ltd.
-# Copyright (C) 2017 VMware INC.
+# Copyright (C) 2017-2019 VMware INC.
 #
 # Author: Maitreyee Saikia <[e-mail]>
 #
 # This file is part of cloud-init. See LICENSE file for license information.
 
+import os
+import stat
 from cloudinit import util
 from cloudinit.sources.helpers.vmware.imc.config_custom_script import (
     CustomScriptConstant,
@@ -18,6 +20,10 @@ from cloudinit.tests.helpers import CiTestCase, mock
 class TestVmwareCustomScript(CiTestCase):
     def setUp(self):
         self.tmpDir = self.tmp_dir()
+        # Mock the tmpDir as the root dir in VM.
+        self.execDir = os.path.join(self.tmpDir, ".customization")
+        self.execScript = os.path.join(self.execDir,
+                                       ".customize.sh")
 
     def test_prepare_custom_script(self):
         """
@@ -37,63 +43,67 @@ class TestVmwareCustomScript(CiTestCase):
 
         # Custom script exists.
         custScript = self.tmp_path("test-cust", self.tmpDir)
-        util.write_file(custScript, "test-CR-strip/r/r")
-        postCust = PostCustomScript("test-cust", self.tmpDir)
-        self.assertEqual("test-cust", postCust.scriptname)
-        self.assertEqual(self.tmpDir, postCust.directory)
-        self.assertEqual(custScript, postCust.scriptpath)
-        self.assertFalse(postCust.postreboot)
-        postCust.prepare_script()
-        # Check if all carraige returns are stripped from script.
-        self.assertFalse("/r" in custScript)
+        util.write_file(custScript, "test-CR-strip\r\r")
+        with mock.patch.object(CustomScriptConstant,
+                               "CUSTOM_TMP_DIR",
+                               self.execDir):
+            with mock.patch.object(CustomScriptConstant,
+                                   "CUSTOM_SCRIPT",
+                                   self.execScript):
+                postCust = PostCustomScript("test-cust",
+                                            self.tmpDir,
+                                            self.tmpDir)
+                self.assertEqual("test-cust", postCust.scriptname)
+                self.assertEqual(self.tmpDir, postCust.directory)
+                self.assertEqual(custScript, postCust.scriptpath)
+                postCust.prepare_script()
 
-    def test_rc_local_exists(self):
-        """
-        This test is designed to verify the different scenarios associated
-        with the presence of rclocal.
-        """
-        # test when rc local does not exist
-        postCust = PostCustomScript("test-cust", self.tmpDir)
-        with mock.patch.object(CustomScriptConstant, "RC_LOCAL", "/no/path"):
-            rclocal = postCust.find_rc_local()
-            self.assertEqual("", rclocal)
-
-        # test when rc local exists
-        rclocalFile = self.tmp_path("vmware-rclocal", self.tmpDir)
-        util.write_file(rclocalFile, "# Run post-reboot guest customization",
-                        omode="w")
-        with mock.patch.object(CustomScriptConstant, "RC_LOCAL", rclocalFile):
-            rclocal = postCust.find_rc_local()
-            self.assertEqual(rclocalFile, rclocal)
-            self.assertTrue(postCust.has_previous_agent, rclocal)
-
-        # test when rc local is a symlink
-        rclocalLink = self.tmp_path("dummy-rclocal-link", self.tmpDir)
-        util.sym_link(rclocalFile, rclocalLink, True)
-        with mock.patch.object(CustomScriptConstant, "RC_LOCAL", rclocalLink):
-            rclocal = postCust.find_rc_local()
-            self.assertEqual(rclocalFile, rclocal)
+                # Custom script is copied with exec privilege
+                self.assertTrue(os.path.exists(self.execScript))
+                st = os.stat(self.execScript)
+                self.assertTrue(st.st_mode & stat.S_IEXEC)
+                with open(self.execScript, "r") as f:
+                    content = f.read()
+                self.assertEqual(content, "test-CR-strip")
+                # Check if all carraige returns are stripped from script.
+                self.assertFalse("\r" in content)
 
     def test_execute_post_cust(self):
         """
-        This test is to identify if rclocal was properly populated to be
-        run after reboot.
+        This test is designed to verify the behavior after execute post
+        customization.
         """
-        customscript = self.tmp_path("vmware-post-cust-script", self.tmpDir)
-        rclocal = self.tmp_path("vmware-rclocal", self.tmpDir)
-        # Create a temporary rclocal file
-        open(customscript, "w")
-        util.write_file(rclocal, "tests\nexit 0", omode="w")
-        postCust = PostCustomScript("vmware-post-cust-script", self.tmpDir)
-        with mock.patch.object(CustomScriptConstant, "RC_LOCAL", rclocal):
-            # Test that guest customization agent is not installed initially.
-            self.assertFalse(postCust.postreboot)
-            self.assertIs(postCust.has_previous_agent(rclocal), False)
-            postCust.install_agent()
+        # Prepare the customize package
+        postCustRun = self.tmp_path("post-customize-guest.sh", self.tmpDir)
+        util.write_file(postCustRun, "This is the script to run post cust")
+        userScript = self.tmp_path("test-cust", self.tmpDir)
+        util.write_file(userScript, "This is the post cust script")
 
-            # Assert rclocal has been modified to have guest customization
-            # agent.
-            self.assertTrue(postCust.postreboot)
-            self.assertTrue(postCust.has_previous_agent, rclocal)
+        # Mock the cc_scripts_per_instance dir and marker file.
+        # Create another tmp dir for cc_scripts_per_instance.
+        ccScriptDir = self.tmp_dir()
+        ccScript = os.path.join(ccScriptDir, "post-customize-guest.sh")
+        markerFile = os.path.join(self.tmpDir, ".markerFile")
+        with mock.patch.object(CustomScriptConstant,
+                               "CUSTOM_TMP_DIR",
+                               self.execDir):
+            with mock.patch.object(CustomScriptConstant,
+                                   "CUSTOM_SCRIPT",
+                                   self.execScript):
+                with mock.patch.object(CustomScriptConstant,
+                                       "POST_CUSTOM_PENDING_MARKER",
+                                       markerFile):
+                    postCust = PostCustomScript("test-cust",
+                                                self.tmpDir,
+                                                ccScriptDir)
+                    postCust.execute()
+                    # Check cc_scripts_per_instance and marker file
+                    # are created.
+                    self.assertTrue(os.path.exists(ccScript))
+                    with open(ccScript, "r") as f:
+                        content = f.read()
+                    self.assertEqual(content,
+                                     "This is the script to run post cust")
+                    self.assertTrue(os.path.exists(markerFile))
 
 # vi: ts=4 expandtab