Merge "In some cases, guest agents may leave temporary config files"

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 a1b58685..31016e3a 100644
--- a/trove/guestagent/datastore/mongodb/service.py
+++ b/trove/guestagent/datastore/mongodb/service.py
@@ -16,6 +16,7 @@
 import json
 import re
 
+import os
 from trove.common import cfg
 from trove.common import utils as utils
 from trove.common import exception
@@ -168,13 +169,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 42615f5f..d6177c97 100644
--- a/trove/tests/unittests/guestagent/test_dbaas.py
+++ b/trove/tests/unittests/guestagent/test_dbaas.py
@@ -490,6 +490,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,
@@ -505,11 +507,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):
@@ -680,6 +686,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):
 
@@ -1415,6 +1446,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):
 
@@ -1425,6 +1458,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(
@@ -1538,6 +1572,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):
 
@@ -1657,6 +1714,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)
@@ -1668,6 +1726,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()
@@ -1676,6 +1735,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):
@@ -1763,6 +1823,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()