In some cases, guest agents may leave temporary config files

The code in these three places manipulates the configuration files. I
was (admittedly) having more than the acceptable dose of failures in
execute_with_timeout() and one of them that failed was the move of the
my.cnf file. And this left a turd in /tmp. The failures I was having
with execute_with_timeout relate to other tests that were improperly
mocking/unmocking and some of those problems have now been fixed.

Found that the same kind of thing can happen in cassandra and
mongo. Since default umask is 664, this leaves a file with somewhat
questionable permissions and potentially (at least for MySQL) some
passwords in /tmp.

The files will now be deleted.

Some test cases have been added to exercise the code paths(s) in
mongo, cassandra and mysql. The tests verify that the files are
actually deleted by mocking the os.unlink call. setUp and tearDown
have been updated in each case to make sure that the monkey patching
is properly reversed.

Change-Id: I93302fc5b1b18c9dd7116370a945b94d1824c75a
Closes-Bug: #1354136

4 files changed, 131 insertions, 23 deletions

diff --git a/trove/guestagent/datastore/cassandra/service.py b/trove/guestagent/datastore/cassandra/service.py
index 8f66220e..c0f9b3b2 100644
--- a/trove/guestagent/datastore/cassandra/service.py
+++ b/trove/guestagent/datastore/cassandra/service.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import os
 import yaml
 from trove.common import cfg
 from trove.common import utils
@@ -119,12 +120,19 @@ class CassandraApp(object):
     def write_config(self, config_contents):
         LOG.debug('Defining temp config holder at %s.' %
                   system.CASSANDRA_TEMP_CONF)
-        with open(system.CASSANDRA_TEMP_CONF, 'w+') as conf:
-            conf.write(config_contents)
-        LOG.info(_('Writing new config.'))
-        utils.execute_with_timeout("sudo", "mv",
-                                   system.CASSANDRA_TEMP_CONF,
-                                   system.CASSANDRA_CONF)
+
+        try:
+            with open(system.CASSANDRA_TEMP_CONF, 'w+') as conf:
+                conf.write(config_contents)
+
+            LOG.info(_('Writing new config.'))
+
+            utils.execute_with_timeout("sudo", "mv",
+                                       system.CASSANDRA_TEMP_CONF,
+                                       system.CASSANDRA_CONF)
+        except Exception:
+            os.unlink(system.CASSANDRA_TEMP_CONF)
+            raise
 
     def read_conf(self):
         """Returns cassandra.yaml in dict structure."""
diff --git a/trove/guestagent/datastore/mongodb/service.py b/trove/guestagent/datastore/mongodb/service.py
index 0999b447..0344efd1 100644
--- a/trove/guestagent/datastore/mongodb/service.py
+++ b/trove/guestagent/datastore/mongodb/service.py
@@ -15,6 +15,7 @@
 
 import re
 
+import os
 from trove.common import cfg
 from trove.common import utils as utils
 from trove.common import exception
@@ -163,13 +164,21 @@ class MongoDBApp(object):
         LOG.info(_("Updating MongoDB config"))
         if config_contents:
             LOG.info(_("Writing %s") % system.TMP_CONFIG)
-            with open(system.TMP_CONFIG, 'w') as t:
-                t.write(config_contents)
-
-            LOG.info(_("Moving %(a)s to %(b)s")
-                     % {'a': system.TMP_CONFIG, 'b': system.CONFIG})
-            utils.execute_with_timeout("mv", system.TMP_CONFIG, system.CONFIG,
-                                       run_as_root=True, root_helper="sudo")
+            try:
+                with open(system.TMP_CONFIG, 'w') as t:
+                    t.write(config_contents)
+
+                LOG.info(_("Moving %(a)s to %(b)s")
+                         % {'a': system.TMP_CONFIG,
+                            'b': system.CONFIG})
+                utils.execute_with_timeout("mv",
+                                           system.TMP_CONFIG,
+                                           system.CONFIG,
+                                           run_as_root=True,
+                                           root_helper="sudo")
+            except Exception:
+                os.unlink(system.TMP_CONFIG)
+                raise
         else:
             LOG.info(_("Empty config_contents. Do nothing"))
 
diff --git a/trove/guestagent/datastore/mysql/service.py b/trove/guestagent/datastore/mysql/service.py
index 8551b956..810bdba0 100644
--- a/trove/guestagent/datastore/mysql/service.py
+++ b/trove/guestagent/datastore/mysql/service.py
@@ -780,16 +780,22 @@ class MySqlApp(object):
         if admin_password is None:
             admin_password = get_auth_password()
 
-        with open(TMP_MYCNF, 'w') as t:
-            t.write(config_contents)
-        utils.execute_with_timeout("sudo", "mv", TMP_MYCNF,
-                                   MYSQL_CONFIG)
-
-        self._write_temp_mycnf_with_admin_account(MYSQL_CONFIG,
-                                                  TMP_MYCNF,
-                                                  admin_password)
-        utils.execute_with_timeout("sudo", "mv", TMP_MYCNF,
-                                   MYSQL_CONFIG)
+        try:
+            with open(TMP_MYCNF, 'w') as t:
+                t.write(config_contents)
+
+            utils.execute_with_timeout("sudo", "mv", TMP_MYCNF,
+                                       MYSQL_CONFIG)
+
+            self._write_temp_mycnf_with_admin_account(MYSQL_CONFIG,
+                                                      TMP_MYCNF,
+                                                      admin_password)
+
+            utils.execute_with_timeout("sudo", "mv", TMP_MYCNF,
+                                       MYSQL_CONFIG)
+        except Exception:
+            os.unlink(TMP_MYCNF)
+            raise
 
         self.wipe_ib_logfiles()
 
diff --git a/trove/tests/unittests/guestagent/test_dbaas.py b/trove/tests/unittests/guestagent/test_dbaas.py
index 6a43e650..c2a6410c 100644
--- a/trove/tests/unittests/guestagent/test_dbaas.py
+++ b/trove/tests/unittests/guestagent/test_dbaas.py
@@ -487,6 +487,8 @@ class MySqlAppTest(testtools.TestCase):
         super(MySqlAppTest, self).setUp()
         self.orig_utils_execute_with_timeout = dbaas.utils.execute_with_timeout
         self.orig_time_sleep = time.sleep
+        self.orig_unlink = os.unlink
+        self.orig_get_auth_password = dbaas.get_auth_password
         util.init_db()
         self.FAKE_ID = str(uuid4())
         InstanceServiceStatus.create(instance_id=self.FAKE_ID,
@@ -502,11 +504,15 @@ class MySqlAppTest(testtools.TestCase):
         dbaas.operating_system.service_discovery = Mock(return_value=
                                                         mysql_service)
         time.sleep = Mock()
+        os.unlink = Mock()
+        dbaas.get_auth_password = Mock()
 
     def tearDown(self):
         super(MySqlAppTest, self).tearDown()
         dbaas.utils.execute_with_timeout = self.orig_utils_execute_with_timeout
         time.sleep = self.orig_time_sleep
+        os.unlink = self.orig_unlink
+        dbaas.get_auth_password = self.orig_get_auth_password
         InstanceServiceStatus.find_by(instance_id=self.FAKE_ID).delete()
 
     def assert_reported_status(self, expected_status):
@@ -677,6 +683,31 @@ class MySqlAppTest(testtools.TestCase):
         dbaas.utils.execute_with_timeout = mocked
         self.assertRaises(ProcessExecutionError, self.mySqlApp.start_mysql)
 
+    def test_mysql_error_in_write_config_verify_unlink(self):
+        configuration = {'config_contents': 'some junk'}
+        from trove.common.exception import ProcessExecutionError
+        dbaas.utils.execute_with_timeout = (
+            Mock(side_effect=ProcessExecutionError('something')))
+
+        self.assertRaises(ProcessExecutionError,
+                          self.mySqlApp.reset_configuration,
+                          configuration=configuration)
+        self.assertEqual(dbaas.utils.execute_with_timeout.call_count, 1)
+        self.assertEqual(os.unlink.call_count, 1)
+        self.assertEqual(dbaas.get_auth_password.call_count, 1)
+
+    def test_mysql_error_in_write_config(self):
+        configuration = {'config_contents': 'some junk'}
+        from trove.common.exception import ProcessExecutionError
+        dbaas.utils.execute_with_timeout = (
+            Mock(side_effect=ProcessExecutionError('something')))
+
+        self.assertRaises(ProcessExecutionError,
+                          self.mySqlApp.reset_configuration,
+                          configuration=configuration)
+        self.assertEqual(dbaas.utils.execute_with_timeout.call_count, 1)
+        self.assertEqual(dbaas.get_auth_password.call_count, 1)
+
 
 class MySqlAppInstallTest(MySqlAppTest):
 
@@ -1412,6 +1443,8 @@ class CassandraDBAppTest(testtools.TestCase):
         self.appStatus = FakeAppStatus(self.FAKE_ID,
                                        rd_instance.ServiceStatuses.NEW)
         self.cassandra = cass_service.CassandraApp(self.appStatus)
+        self.orig_unlink = os.unlink
+        os.unlink = Mock()
 
     def tearDown(self):
 
@@ -1422,6 +1455,7 @@ class CassandraDBAppTest(testtools.TestCase):
         cass_service.packager.pkg_version = self.pkg_version
         cass_service.packager = self.pkg
         InstanceServiceStatus.find_by(instance_id=self.FAKE_ID).delete()
+        os.unlink = self.orig_unlink
 
     def assert_reported_status(self, expected_status):
         service_status = InstanceServiceStatus.find_by(
@@ -1535,6 +1569,29 @@ class CassandraDBAppTest(testtools.TestCase):
 
         self.assert_reported_status(rd_instance.ServiceStatuses.NEW)
 
+    def test_cassandra_error_in_write_config_verify_unlink(self):
+        from trove.common.exception import ProcessExecutionError
+        cass_service.utils.execute_with_timeout = (
+            Mock(side_effect=ProcessExecutionError('some exception')))
+        configuration = 'this is my configuration'
+
+        self.assertRaises(ProcessExecutionError,
+                          self.cassandra.write_config,
+                          config_contents=configuration)
+        self.assertEqual(cass_service.utils.execute_with_timeout.call_count, 1)
+        self.assertEqual(os.unlink.call_count, 1)
+
+    def test_cassandra_error_in_write_config(self):
+        from trove.common.exception import ProcessExecutionError
+        cass_service.utils.execute_with_timeout = (
+            Mock(side_effect=ProcessExecutionError('some exception')))
+        configuration = 'this is my configuration'
+
+        self.assertRaises(ProcessExecutionError,
+                          self.cassandra.write_config,
+                          config_contents=configuration)
+        self.assertEqual(cass_service.utils.execute_with_timeout.call_count, 1)
+
 
 class CouchbaseAppTest(testtools.TestCase):
 
@@ -1654,6 +1711,7 @@ class MongoDBAppTest(testtools.TestCase):
         self.orig_time_sleep = time.sleep
         self.orig_packager = mongo_system.PACKAGER
         self.orig_service_discovery = operating_system.service_discovery
+        self.orig_os_unlink = os.unlink
 
         operating_system.service_discovery = (
             self.fake_mongodb_service_discovery)
@@ -1665,6 +1723,7 @@ class MongoDBAppTest(testtools.TestCase):
                                        rd_instance.ServiceStatuses.NEW)
         self.mongoDbApp = mongo_service.MongoDBApp(self.appStatus)
         time.sleep = Mock()
+        os.unlink = Mock()
 
     def tearDown(self):
         super(MongoDBAppTest, self).tearDown()
@@ -1673,6 +1732,7 @@ class MongoDBAppTest(testtools.TestCase):
         time.sleep = self.orig_time_sleep
         mongo_system.PACKAGER = self.orig_packager
         operating_system.service_discovery = self.orig_service_discovery
+        os.unlink = self.orig_os_unlink
         InstanceServiceStatus.find_by(instance_id=self.FAKE_ID).delete()
 
     def assert_reported_status(self, expected_status):
@@ -1760,6 +1820,31 @@ class MongoDBAppTest(testtools.TestCase):
 
         self.assertRaises(RuntimeError, self.mongoDbApp.start_db)
 
+    def test_mongodb_error_in_write_config_verify_unlink(self):
+        configuration = {'config_contents': 'some junk'}
+        from trove.common.exception import ProcessExecutionError
+        mongo_service.utils.execute_with_timeout = (
+            Mock(side_effect=ProcessExecutionError('some exception')))
+
+        self.assertRaises(ProcessExecutionError,
+                          self.mongoDbApp.reset_configuration,
+                          configuration=configuration)
+        self.assertEqual(
+            mongo_service.utils.execute_with_timeout.call_count, 1)
+        self.assertEqual(os.unlink.call_count, 1)
+
+    def test_mongodb_error_in_write_config(self):
+        configuration = {'config_contents': 'some junk'}
+        from trove.common.exception import ProcessExecutionError
+        mongo_service.utils.execute_with_timeout = (
+            Mock(side_effect=ProcessExecutionError('some exception')))
+
+        self.assertRaises(ProcessExecutionError,
+                          self.mongoDbApp.reset_configuration,
+                          configuration=configuration)
+        self.assertEqual(
+            mongo_service.utils.execute_with_timeout.call_count, 1)
+
     def test_start_db_with_conf_changes_db_is_running(self):
 
         self.mongoDbApp.start_db = Mock()