File 0046-Snapper-module-improvements.patch of Package salt

Overview Repositories Revisions Requests Users Attributes Meta

File 0046-Snapper-module-improvements.patch of Package salt (Revision 7a0d7bdcecf2ea4db28176a5501a6cd0)

Currently displaying revision 7a0d7bdcecf2ea4db28176a5501a6cd0 , Show latest

From 1ba57479b4ba5db038817b0f0387c246204b2b7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?=
 <psuarezhernandez@suse.com>
Date: Fri, 27 Jan 2017 17:07:25 +0000
Subject: [PATCH 46/46] Snapper module improvements

* Snapper: Adding support for deleting snapshots
* Snapper: Adding support for snapshot metadata modification
* Snapper: Adding support for creating configurations
* Adds 'snapper.delete_snapshots' unit tests
* Adds 'snapper.modify_snapshots' unit tests
* Adds 'snapper.create_config' unit tests
* Removing extra spaces
* pylint fixes
* Adds multiple SUBVOLUME support to the Snapper module
* Only include diff in the state response if `include_diff` is True
* Raises "CommandExecutionError" if snapper command fails
* Updating and fixing the documentation
* Removing posible double '/' from the file paths
* Fixing Snapper unit tests for SUBVOLUME support
* Fixes pre/post snapshot order to get the inverse status
* Some fixes and pylint
---
 salt/modules/snapper.py            | 201 +++++++++++++++++++++++++++++++++----
 salt/states/snapper.py             |  49 ++++++---
 tests/unit/modules/snapper_test.py |  54 ++++++++++
 3 files changed, 270 insertions(+), 34 deletions(-)

diff --git a/salt/modules/snapper.py b/salt/modules/snapper.py
index edecd87..db39ae7 100644
--- a/salt/modules/snapper.py
+++ b/salt/modules/snapper.py
@@ -276,6 +276,60 @@ def get_config(name='root'):
         )
 
 
+def create_config(name=None,
+                  subvolume=None,
+                  fstype=None,
+                  template=None,
+                  extra_opts=None):
+    '''
+    Creates a new Snapper configuration
+
+    name
+        Name of the new Snapper configuration.
+    subvolume
+        Path to the related subvolume.
+    fstype
+        Filesystem type of the subvolume.
+    template
+        Configuration template to use. (Default: default)
+    extra_opts
+        Extra Snapper configuration opts dictionary. It will override the values provided
+        by the given template (if any).
+
+    CLI example:
+
+    .. code-block:: bash
+
+      salt '*' snapper.create_config name=myconfig subvolume=/foo/bar/ fstype=btrfs
+      salt '*' snapper.create_config name=myconfig subvolume=/foo/bar/ fstype=btrfs template="default"
+      salt '*' snapper.create_config name=myconfig subvolume=/foo/bar/ fstype=btrfs extra_opts='{"NUMBER_CLEANUP": False}'
+    '''
+    def raise_arg_error(argname):
+        raise CommandExecutionError(
+            'You must provide a "{0}" for the new configuration'.format(argname)
+        )
+
+    if not name:
+        raise_arg_error("name")
+    if not subvolume:
+        raise_arg_error("subvolume")
+    if not fstype:
+        raise_arg_error("fstype")
+    if not template:
+        template = ""
+
+    try:
+        snapper.CreateConfig(name, subvolume, fstype, template)
+        if extra_opts:
+            set_config(name, **extra_opts)
+        return get_config(name)
+    except dbus.DBusException as exc:
+        raise CommandExecutionError(
+            'Error encountered while creating the new configuration: {0}'
+            .format(_dbus_exception_to_reason(exc, locals()))
+        )
+
+
 def create_snapshot(config='root', snapshot_type='single', pre_number=None,
                     description=None, cleanup_algorithm='number', userdata=None,
                     **kwargs):
@@ -295,14 +349,14 @@ def create_snapshot(config='root', snapshot_type='single', pre_number=None,
     cleanup_algorithm
         Set the cleanup algorithm for the snapshot.
 
-        number
-            Deletes old snapshots when a certain number of snapshots
-            is reached.
-        timeline
-            Deletes old snapshots but keeps a number of hourly,
-            daily, weekly, monthly and yearly snapshots.
-        empty-pre-post
-            Deletes pre/post snapshot pairs with empty diffs.
+    number
+        Deletes old snapshots when a certain number of snapshots
+        is reached.
+    timeline
+        Deletes old snapshots but keeps a number of hourly,
+        daily, weekly, monthly and yearly snapshots.
+    empty-pre-post
+        Deletes pre/post snapshot pairs with empty diffs.
     userdata
         Set userdata for the snapshot (key-value pairs).
 
@@ -347,6 +401,95 @@ def create_snapshot(config='root', snapshot_type='single', pre_number=None,
     return new_nr
 
 
+def delete_snapshot(snapshots_ids=None, config="root"):
+    '''
+    Deletes an snapshot
+
+    config
+        Configuration name. (Default: root)
+
+    snapshots_ids
+        List of the snapshots IDs to be deleted.
+
+    CLI example:
+
+    .. code-block:: bash
+
+        salt '*' snapper.delete_snapshot 54
+        salt '*' snapper.delete_snapshot config=root 54
+        salt '*' snapper.delete_snapshot config=root snapshots_ids=[54,55,56]
+    '''
+    if not snapshots_ids:
+        raise CommandExecutionError('Error: No snapshot ID has been provided')
+    try:
+        current_snapshots_ids = [x['id'] for x in list_snapshots(config)]
+        if not isinstance(snapshots_ids, list):
+            snapshots_ids = [snapshots_ids]
+        if not set(snapshots_ids).issubset(set(current_snapshots_ids)):
+            raise CommandExecutionError(
+                "Error: Snapshots '{0}' not found".format(", ".join(
+                    [str(x) for x in set(snapshots_ids).difference(
+                        set(current_snapshots_ids))]))
+            )
+        snapper.DeleteSnapshots(config, snapshots_ids)
+        return {config: {"ids": snapshots_ids, "status": "deleted"}}
+    except dbus.DBusException as exc:
+        raise CommandExecutionError(_dbus_exception_to_reason(exc, locals()))
+
+
+def modify_snapshot(snapshot_id=None,
+                    description=None,
+                    userdata=None,
+                    cleanup=None,
+                    config="root"):
+    '''
+    Modify attributes of an existing snapshot.
+
+    config
+        Configuration name. (Default: root)
+
+    snapshot_id
+        ID of the snapshot to be modified.
+
+    cleanup
+        Change the cleanup method of the snapshot. (str)
+
+    description
+        Change the description of the snapshot. (str)
+
+    userdata
+        Change the userdata dictionary of the snapshot. (dict)
+
+    CLI example:
+
+    .. code-block:: bash
+
+        salt '*' snapper.modify_snapshot 54 description="my snapshot description"
+        salt '*' snapper.modify_snapshot 54 description="my snapshot description"
+        salt '*' snapper.modify_snapshot 54 userdata='{"foo": "bar"}'
+        salt '*' snapper.modify_snapshot snapshot_id=54 cleanup="number"
+    '''
+    if not snapshot_id:
+        raise CommandExecutionError('Error: No snapshot ID has been provided')
+
+    snapshot = get_snapshot(config=config, number=snapshot_id)
+    try:
+        # Updating only the explicitely provided attributes by the user
+        updated_opts = {
+            'description': description if description is not None else snapshot['description'],
+            'cleanup': cleanup if cleanup is not None else snapshot['cleanup'],
+            'userdata': userdata if userdata is not None else snapshot['userdata'],
+        }
+        snapper.SetSnapshot(config,
+                            snapshot_id,
+                            updated_opts['description'],
+                            updated_opts['cleanup'],
+                            updated_opts['userdata'])
+        return get_snapshot(config=config, number=snapshot_id)
+    except dbus.DBusException as exc:
+        raise CommandExecutionError(_dbus_exception_to_reason(exc, locals()))
+
+
 def _get_num_interval(config, num_pre, num_post):
     '''
     Returns numerical interval based on optionals num_pre, num_post values
@@ -463,8 +606,12 @@ def status(config='root', num_pre=None, num_post=None):
         snapper.CreateComparison(config, int(pre), int(post))
         files = snapper.GetFiles(config, int(pre), int(post))
         status_ret = {}
+        SUBVOLUME = list_configs()[config]['SUBVOLUME']
         for file in files:
-            status_ret[file[0]] = {'status': status_to_string(file[1])}
+            # In case of SUBVOLUME is included in filepath we remove it
+            # to prevent from filepath starting with double '/'
+            _filepath = file[0][len(SUBVOLUME):] if file[0].startswith(SUBVOLUME) else file[0]
+            status_ret[os.path.normpath(SUBVOLUME + _filepath)] = {'status': status_to_string(file[1])}
         return status_ret
     except dbus.DBusException as exc:
         raise CommandExecutionError(
@@ -520,14 +667,19 @@ def undo(config='root', files=None, num_pre=None, num_post=None):
             'Given file list contains files that are not present'
             'in the changed filelist: {0}'.format(changed - requested))
 
-    cmdret = __salt__['cmd.run']('snapper undochange {0}..{1} {2}'.format(
-        pre, post, ' '.join(requested)))
-    components = cmdret.split(' ')
-    ret = {}
-    for comp in components:
-        key, val = comp.split(':')
-        ret[key] = val
-    return ret
+    cmdret = __salt__['cmd.run']('snapper -c {0} undochange {1}..{2} {3}'.format(
+       config, pre, post, ' '.join(requested)))
+
+    try:
+        components = cmdret.split(' ')
+        ret = {}
+        for comp in components:
+            key, val = comp.split(':')
+            ret[key] = val
+        return ret
+    except ValueError as exc:
+        raise CommandExecutionError(
+            'Error while processing Snapper response: {0}'.format(cmdret))
 
 
 def _get_jid_snapshots(jid, config='root'):
@@ -601,13 +753,20 @@ def diff(config='root', filename=None, num_pre=None, num_post=None):
         if filename:
             files = [filename] if filename in files else []
 
-        pre_mount = snapper.MountSnapshot(config, pre, False) if pre else ""
-        post_mount = snapper.MountSnapshot(config, post, False) if post else ""
+        SUBVOLUME = list_configs()[config]['SUBVOLUME']
+        pre_mount = snapper.MountSnapshot(config, pre, False) if pre else SUBVOLUME
+        post_mount = snapper.MountSnapshot(config, post, False) if post else SUBVOLUME
 
         files_diff = dict()
         for filepath in [filepath for filepath in files if not os.path.isdir(filepath)]:
-            pre_file = pre_mount + filepath
-            post_file = post_mount + filepath
+
+            _filepath = filepath
+            if filepath.startswith(SUBVOLUME):
+                _filepath = filepath[len(SUBVOLUME):]
+
+            # Just in case, removing posible double '/' from the final file paths
+            pre_file = os.path.normpath(pre_mount + "/" + _filepath).replace("//", "/")
+            post_file = os.path.normpath(post_mount + "/" + _filepath).replace("//", "/")
 
             if os.path.isfile(pre_file):
                 pre_file_exists = True
diff --git a/salt/states/snapper.py b/salt/states/snapper.py
index 2711550..be50bc4 100644
--- a/salt/states/snapper.py
+++ b/salt/states/snapper.py
@@ -23,7 +23,7 @@ The snapper state module allows you to manage state implicitly, in addition
 to explicit rules, in order to define a baseline and iterate with explicit
 rules as they show that they work in production.
 
-The workflow is: once you have a workin and audited system, you would create
+The workflow is: once you have a working and audited system, you would create
 your baseline snapshot (eg. with ``salt tgt snapper.create_snapshot``) and
 define in your state this baseline using the identifier of the snapshot
 (in this case: 20):
@@ -33,10 +33,20 @@ define in your state this baseline using the identifier of the snapshot
     my_baseline:
       snapper.baseline_snapshot:
         - number: 20
+        - include_diff: False
         - ignore:
           - /var/log
           - /var/cache
 
+Baseline snapshots can be also referenced by tag. Most recent baseline snapshot
+is used in case of multiple snapshots with the same tag:
+
+    my_baseline_external_storage:
+      snapper.baseline_snapshot:
+        - tag: my_custom_baseline_tag
+        - config: external
+        - ignore:
+          - /mnt/tmp_files/
 
 If you have this state, and you haven't done changes to the system since the
 snapshot, and you add a user, the state will show you the changes (including
@@ -107,25 +117,39 @@ def __virtual__():
     return 'snapper' if 'snapper.diff' in __salt__ else False
 
 
-def _get_baseline_from_tag(tag):
+def _get_baseline_from_tag(config, tag):
     '''
     Returns the last created baseline snapshot marked with `tag`
     '''
     last_snapshot = None
-    for snapshot in __salt__['snapper.list_snapshots']():
+    for snapshot in __salt__['snapper.list_snapshots'](config):
         if tag == snapshot['userdata'].get("baseline_tag"):
             if not last_snapshot or last_snapshot['timestamp'] < snapshot['timestamp']:
                 last_snapshot = snapshot
     return last_snapshot
 
 
-def baseline_snapshot(name, number=None, tag=None, config='root', ignore=None):
+def baseline_snapshot(name, number=None, tag=None, include_diff=True, config='root', ignore=None):
     '''
     Enforces that no file is modified comparing against a previously
     defined snapshot identified by number.
 
+    number
+        Number of selected baseline snapshot.
+
+    tag
+        Tag of the selected baseline snapshot. Most recent baseline baseline
+        snapshot is used in case of multiple snapshots with the same tag.
+        (`tag` and `number` cannot be used at the same time)
+
+    include_diff
+        Include a diff in the response (Default: True)
+
+    config
+        Snapper config name (Default: root)
+
     ignore
-        List of files to ignore
+        List of files to ignore. (Default: None)
     '''
     if not ignore:
         ignore = []
@@ -146,7 +170,7 @@ def baseline_snapshot(name, number=None, tag=None, config='root', ignore=None):
         return ret
 
     if tag:
-        snapshot = _get_baseline_from_tag(tag)
+        snapshot = _get_baseline_from_tag(config, tag)
         if not snapshot:
             ret.update({'result': False,
                         'comment': 'Baseline tag "{0}" not found'.format(tag)})
@@ -154,7 +178,7 @@ def baseline_snapshot(name, number=None, tag=None, config='root', ignore=None):
         number = snapshot['id']
 
     status = __salt__['snapper.status'](
-        config, num_pre=number, num_post=0)
+        config, num_pre=0, num_post=number)
 
     for target in ignore:
         if os.path.isfile(target):
@@ -164,18 +188,17 @@ def baseline_snapshot(name, number=None, tag=None, config='root', ignore=None):
                 status.pop(target_file, None)
 
     for file in status:
-        status[file]['actions'] = status[file].pop("status")
-
         # Only include diff for modified files
-        if "modified" in status[file]['actions']:
+        if "modified" in status[file]["status"] and include_diff:
+            status[file].pop("status")
             status[file].update(__salt__['snapper.diff'](config,
                                                          num_pre=0,
                                                          num_post=number,
-                                                         filename=file)[file])
+                                                         filename=file).get(file, {}))
 
     if __opts__['test'] and status:
-        ret['pchanges'] = ret["changes"]
-        ret['changes'] = {}
+        ret['pchanges'] = status
+        ret['changes'] = ret['pchanges']
         ret['comment'] = "{0} files changes are set to be undone".format(len(status.keys()))
         ret['result'] = None
     elif __opts__['test'] and not status:
diff --git a/tests/unit/modules/snapper_test.py b/tests/unit/modules/snapper_test.py
index 43f8898..a5d9b76 100644
--- a/tests/unit/modules/snapper_test.py
+++ b/tests/unit/modules/snapper_test.py
@@ -202,6 +202,26 @@ class SnapperTestCase(TestCase):
         self.assertEqual(snapper.status_to_string(128), ["extended attributes changed"])
         self.assertEqual(snapper.status_to_string(256), ["ACL info changed"])
 
+    @patch('salt.modules.snapper.snapper.CreateConfig', MagicMock())
+    @patch('salt.modules.snapper.snapper.GetConfig', MagicMock(return_value=DBUS_RET['ListConfigs'][0]))
+    def test_create_config(self):
+        opts = {
+            'name': 'testconfig',
+            'subvolume': '/foo/bar/',
+            'fstype': 'btrfs',
+            'template': 'mytemplate',
+            'extra_opts': {"NUMBER_CLEANUP": False},
+        }
+        with patch('salt.modules.snapper.set_config', MagicMock()) as set_config_mock:
+            self.assertEqual(snapper.create_config(**opts), DBUS_RET['ListConfigs'][0])
+            set_config_mock.assert_called_with("testconfig", **opts['extra_opts'])
+
+        with patch('salt.modules.snapper.set_config', MagicMock()) as set_config_mock:
+            del opts['extra_opts']
+            self.assertEqual(snapper.create_config(**opts), DBUS_RET['ListConfigs'][0])
+            assert not set_config_mock.called
+            self.assertRaises(CommandExecutionError, snapper.create_config)
+
     @patch('salt.modules.snapper.snapper.CreateSingleSnapshot', MagicMock(return_value=1234))
     @patch('salt.modules.snapper.snapper.CreatePreSnapshot', MagicMock(return_value=1234))
     @patch('salt.modules.snapper.snapper.CreatePostSnapshot', MagicMock(return_value=1234))
@@ -216,6 +236,36 @@ class SnapperTestCase(TestCase):
             }
             self.assertEqual(snapper.create_snapshot(**opts), 1234)
 
+    @patch('salt.modules.snapper.snapper.DeleteSnapshots', MagicMock())
+    @patch('salt.modules.snapper.snapper.ListSnapshots', MagicMock(return_value=DBUS_RET['ListSnapshots']))
+    def test_delete_snapshot_id_success(self):
+        self.assertEqual(snapper.delete_snapshot(snapshots_ids=43), {"root": {"ids": [43], "status": "deleted"}})
+        self.assertEqual(snapper.delete_snapshot(snapshots_ids=[42, 43]), {"root": {"ids": [42, 43], "status": "deleted"}})
+
+    @patch('salt.modules.snapper.snapper.DeleteSnapshots', MagicMock())
+    @patch('salt.modules.snapper.snapper.ListSnapshots', MagicMock(return_value=DBUS_RET['ListSnapshots']))
+    def test_delete_snapshot_id_fail(self):
+        self.assertRaises(CommandExecutionError, snapper.delete_snapshot)
+        self.assertRaises(CommandExecutionError, snapper.delete_snapshot, snapshots_ids=1)
+        self.assertRaises(CommandExecutionError, snapper.delete_snapshot, snapshots_ids=[1, 2])
+
+    @patch('salt.modules.snapper.snapper.SetSnapshot', MagicMock())
+    def test_modify_snapshot(self):
+        _ret = {
+            'userdata': {'userdata2': 'uservalue2'},
+            'description': 'UPDATED DESCRIPTION', 'timestamp': 1457006571,
+            'cleanup': 'number', 'user': 'root', 'type': 'pre', 'id': 42
+        }
+        _opts = {
+            'config': 'root',
+            'snapshot_id': 42,
+            'cleanup': 'number',
+            'description': 'UPDATED DESCRIPTION',
+            'userdata': {'userdata2': 'uservalue2'},
+        }
+        with patch('salt.modules.snapper.get_snapshot', MagicMock(side_effect=[DBUS_RET['ListSnapshots'][0], _ret])):
+            self.assertDictEqual(snapper.modify_snapshot(**_opts), _ret)
+
     @patch('salt.modules.snapper._get_last_snapshot', MagicMock(return_value={'id': 42}))
     def test__get_num_interval(self):
         self.assertEqual(snapper._get_num_interval(config=None, num_pre=None, num_post=None), (42, 0))  # pylint: disable=protected-access
@@ -234,6 +284,7 @@ class SnapperTestCase(TestCase):
     @patch('salt.modules.snapper._get_num_interval', MagicMock(return_value=(42, 43)))
     @patch('salt.modules.snapper.snapper.GetComparison', MagicMock())
     @patch('salt.modules.snapper.snapper.GetFiles', MagicMock(return_value=DBUS_RET['GetFiles']))
+    @patch('salt.modules.snapper.snapper.ListConfigs', MagicMock(return_value=DBUS_RET['ListConfigs']))
     def test_status(self):
         if six.PY3:
             self.assertCountEqual(snapper.status(), MODULE_RET['GETFILES'])
@@ -288,6 +339,7 @@ class SnapperTestCase(TestCase):
     @patch('salt.modules.snapper._is_text_file', MagicMock(return_value=True))
     @patch('os.path.isfile', MagicMock(side_effect=[False, True]))
     @patch('salt.utils.fopen', mock_open(read_data=FILE_CONTENT["/tmp/foo2"]['post']))
+    @patch('salt.modules.snapper.snapper.ListConfigs', MagicMock(return_value=DBUS_RET['ListConfigs']))
     def test_diff_text_file(self):
         if sys.version_info < (2, 7):
             self.assertEqual(snapper.diff(), {"/tmp/foo2": MODULE_RET['DIFF']['/tmp/foo26']})
@@ -302,6 +354,7 @@ class SnapperTestCase(TestCase):
     @patch('salt.modules.snapper._is_text_file', MagicMock(return_value=True))
     @patch('os.path.isfile', MagicMock(side_effect=[True, True, False, True]))
     @patch('os.path.isdir', MagicMock(return_value=False))
+    @patch('salt.modules.snapper.snapper.ListConfigs', MagicMock(return_value=DBUS_RET['ListConfigs']))
     @skipIf(sys.version_info < (2, 7), 'Python 2.7 required to compare diff properly')
     def test_diff_text_files(self):
         fopen_effect = [
@@ -331,6 +384,7 @@ class SnapperTestCase(TestCase):
             "f18f971f1517449208a66589085ddd3723f7f6cefb56c141e3d97ae49e1d87fa",
         ])
     })
+    @patch('salt.modules.snapper.snapper.ListConfigs', MagicMock(return_value=DBUS_RET['ListConfigs']))
     def test_diff_binary_files(self):
         fopen_effect = [
             mock_open(read_data="dummy binary").return_value,
-- 
2.10.1

Places

File 0046-Snapper-module-improvements.patch of Package salt (Revision 7a0d7bdcecf2ea4db28176a5501a6cd0)

Places