From 535e71797031c3d3e3a5e2023c5ede470b02e3a7 Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
Date: Thu, 18 May 2017 10:28:43 +0200
Subject: [PATCH] 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                      |  7 ++++++-
 .../tests/unit/backup/drivers/test_backup_posix.py  | 13 +++++++++++++
 .../nfs_backup_no_overwrite-be7b545453baf7a3.yaml   |  5 +++++
 3 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/nfs_backup_no_overwrite-be7b545453baf7a3.yaml

diff --git a/cinder/backup/drivers/posix.py b/cinder/backup/drivers/posix.py
index b7f151b2542..d6accc0358c 100644
--- a/cinder/backup/drivers/posix.py
+++ b/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
diff --git a/cinder/tests/unit/backup/drivers/test_backup_posix.py b/cinder/tests/unit/backup/drivers/test_backup_posix.py
index aab9856fe46..70ac5005d96 100644
--- a/cinder/tests/unit/backup/drivers/test_backup_posix.py
+++ b/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)
diff --git a/releasenotes/notes/nfs_backup_no_overwrite-be7b545453baf7a3.yaml b/releasenotes/notes/nfs_backup_no_overwrite-be7b545453baf7a3.yaml
new file mode 100644
index 00000000000..73227a65c41
--- /dev/null
+++ b/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.