diff --git a/salt/utils/etcd_util.py b/salt/utils/etcd_util.py index da2843b279d9..99c175e1a3ea 100644 --- 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 index 90ad36182118..467b50cee317 100644 --- 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)