NFS Backup: Fix overwritting backups

When using the NFS backup driver if we do multiple backups using the
same container we end up overwriting older backups.

The issue comes from a misunderstanding in the Posix backup driver of
the purpose of the "prefix" metadata used in the ChunkedBackupDriver
base class.

This prefix is for the name of the backup objects to store, but unlike
the prefix for the volumes, here it must be unique as the base driver
will only add numbers to identify the chunk (for the volume we add the
volume id).  Unfortunately the Posix driver just assumed that the prefix
had the same meaning as the prefix for volumes thus making one backups
override one another.

This patch changes the prefix generated by the Posix driver so we have
the following format: "volume_$VOL_ID_$TIMESTAMP_backup_$BACK_ID", thus
allowing multiple backups in the same container.

The new name is backward compatible with existing backups because the
new prefix will only be used on new backups as the prefix for already
existing backups is stored in the DB.

Change-Id: I2903c27633facde6370d95ba0b9e06025ccaef26
Closes-Bug: #1628768

cinder/backup/drivers/posix.py

@@ -23,6 +23,7 @@ import stat
 
 from oslo_config import cfg
 from oslo_log import log as logging
+from oslo_utils import timeutils
 
 from cinder.backup import chunkeddriver
 from cinder import exception
@@ -129,7 +130,11 @@ class PosixBackupDriver(chunkeddriver.ChunkedBackupDriver):
         os.remove(path)
 
     def _generate_object_name_prefix(self, backup):
-        return 'backup'
+        timestamp = timeutils.utcnow().strftime("%Y%m%d%H%M%S")
+        prefix = 'volume_%s_%s_backup_%s' % (backup.volume_id, timestamp,
+                                             backup.id)
+        LOG.debug('_generate_object_name_prefix: %s', prefix)
+        return prefix
 
     def get_extra_metadata(self, backup, volume):
         return None

cinder/tests/unit/backup/drivers/test_backup_posix.py

@@ -24,6 +24,7 @@ from six.moves import builtins
 
 from cinder.backup.drivers import posix
 from cinder import context
+from cinder import objects
 from cinder import test
 from cinder.tests.unit import fake_constants as fake
 
@@ -175,3 +176,15 @@ class PosixBackupDriverTestCase(test.TestCase):
         self.assertRaises(OSError,
                           self.driver.delete_object, FAKE_CONTAINER,
                           FAKE_OBJECT_NAME)
+
+    @mock.patch.object(posix.timeutils, 'utcnow')
+    def test_generate_object_name_prefix(self, utcnow_mock):
+        timestamp = '20170518102205'
+        utcnow_mock.return_value.strftime.return_value = timestamp
+        backup = objects.Backup(self.ctxt, volume_id=fake.VOLUME_ID,
+                                id=fake.BACKUP_ID)
+        res = self.driver._generate_object_name_prefix(backup)
+        expected = 'volume_%s_%s_backup_%s' % (backup.volume_id,
+                                               timestamp,
+                                               backup.id)
+        self.assertEqual(expected, res)

releasenotes/notes/nfs_backup_no_overwrite-be7b545453baf7a3.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fix NFS backup driver, we now support multiple backups on the same
+    container, they are no longer overwritten.