From bc1e8d6782106ec1f0fb4ebb1afa3f7269a4f063 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
(cherry picked from commit 535e71797031c3d3e3a5e2023c5ede470b02e3a7)
(cherry picked from commit 640b9dc2b739b04f1f7d70c2172f5b5fbc3b9b28)
---
 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 b7f151b25..d6accc035 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 1b86bad4f..ff3a21dd7 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
 
@@ -180,3 +181,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 000000000..73227a65c
--- /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.