Merge pull request #54650 from garethgreenaway/2019_2_1_port_51538

[master] Porting #51538 to master
saltstack · Dec 23, 2019 · dac73c8 · dac73c8
2 parents 2184bf9 + 49fdbbe
commit dac73c8
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 13 deletions.
diff --git a/salt/utils/etcd_util.py b/salt/utils/etcd_util.py
@@ -267,12 +267,16 @@ def set(self, key, value, ttl=None, directory=False):
         return self.write(key, value, ttl=ttl, directory=directory)
 
     def write(self, key, value, ttl=None, directory=False):
-        # directories can't have values, but have to have it passed
         if directory:
-            value = None
+            return self.write_directory(key, value, ttl)
+        return self.write_file(key, value, ttl)
+
+    def write_file(self, key, value, ttl=None):
         try:
-            result = self.client.write(key, value, ttl=ttl, dir=directory)
-        except (etcd.EtcdNotFile, etcd.EtcdNotDir, etcd.EtcdRootReadOnly, ValueError) as err:
+            result = self.client.write(key, value, ttl=ttl, dir=False)
+        except (etcd.EtcdNotFile, etcd.EtcdRootReadOnly, ValueError) as err:
+            # If EtcdNotFile is raised, then this key is a directory and
+            # really this is a name collision.
             log.error('etcd: %s', err)
             return None
         except MaxRetryError as err:
@@ -282,10 +286,32 @@ def write(self, key, value, ttl=None, directory=False):
             log.error('etcd: uncaught exception %s', err)
             raise
 
-        if directory:
-            return getattr(result, 'dir')
-        else:
-            return getattr(result, 'value')
+        return getattr(result, 'value')
+
+    def write_directory(self, key, value, ttl=None):
+        if value is not None:
+            log.info('etcd: non-empty value passed for directory: %s', value)
+        try:
+            # directories can't have values, but have to have it passed
+            result = self.client.write(key, None, ttl=ttl, dir=True)
+        except etcd.EtcdNotFile:
+            # When a directory already exists, python-etcd raises an EtcdNotFile
+            # exception. In this case, we just catch and return True for success.
+            log.info('etcd: directory already exists: %s', key)
+            return True
+        except (etcd.EtcdNotDir, etcd.EtcdRootReadOnly, ValueError) as err:
+            # If EtcdNotDir is raised, then the specified path is a file and
+            # thus this is an error.
+            log.error('etcd: %s', err)
+            return None
+        except MaxRetryError as err:
+            log.error("etcd: Could not connect to etcd server: %s", err)
+            return None
+        except Exception as err:
+            log.error('etcd: uncaught exception %s', err)
+            raise
+
+        return getattr(result, 'dir')
 
     def ls(self, path):
         ret = {}

diff --git a/tests/unit/utils/test_etcd_util.py b/tests/unit/utils/test_etcd_util.py
@@ -171,17 +171,33 @@ def test_write(self):
             self.assertEqual(client.write('/some-dir', 'salt', ttl=0, directory=True), True)
             etcd_client.write.assert_called_with('/some-dir', None, ttl=0, dir=True)
 
+            # Check when a file is attempted to be written to a read-only root
             etcd_client.write.side_effect = etcd.EtcdRootReadOnly()
-            self.assertEqual(client.write('/', 'some-val'), None)
+            self.assertEqual(client.write('/', 'some-val', directory=False), None)
 
+            # Check when a directory is attempted to be written to a read-only root
+            etcd_client.write.side_effect = etcd.EtcdRootReadOnly()
+            self.assertEqual(client.write('/', None, directory=True), None)
+
+            # Check when a file is attempted to be written when unable to connect to the service
+            etcd_client.write.side_effect = MaxRetryError(None, None)
+            self.assertEqual(client.write('/some-key', 'some-val', directory=False), None)
+
+            # Check when a directory is attempted to be written when unable to connect to the service
+            etcd_client.write.side_effect = MaxRetryError(None, None)
+            self.assertEqual(client.write('/some-dir', None, directory=True), None)
+
+            # Check when a file is attempted to be written to a directory that already exists (name-collision)
             etcd_client.write.side_effect = etcd.EtcdNotFile()
-            self.assertEqual(client.write('/some-key', 'some-val'), None)
+            self.assertEqual(client.write('/some-dir', 'some-val', directory=False), None)
 
+            # Check when a directory is attempted to be written to a file that already exists (name-collision)
             etcd_client.write.side_effect = etcd.EtcdNotDir()
-            self.assertEqual(client.write('/some-dir', 'some-val'), None)
+            self.assertEqual(client.write('/some-key', None, directory=True), None)
 
-            etcd_client.write.side_effect = MaxRetryError(None, None)
-            self.assertEqual(client.write('/some-key', 'some-val'), None)
+            # Check when a directory is attempted to be written to a directory that already exists (update-ttl)
+            etcd_client.write.side_effect = etcd.EtcdNotFile()
+            self.assertEqual(client.write('/some-dir', None, directory=True), True)
 
             etcd_client.write.side_effect = ValueError
             self.assertEqual(client.write('/some-key', 'some-val'), None)