From 84c50ab91cc8d2b080a064a81285b82fdb8df9fc Mon Sep 17 00:00:00 2001
From: Liang Chen <cbjchen@cn.ibm.com>
Date: Fri, 13 Sep 2013 14:28:15 +0800
Subject: [PATCH] Validate OS::Nova::Server block_device_mapping property

Make sure either volume_id or snapshot_id exists but not both. Also this patch
ensures that a bootable volume is specified when image is not given.

Fixes bug #1215267

Change-Id: Ia51f6c7aec9c0b257318992ee58febc392ea3d84
---
 heat/engine/resources/server.py | 24 +++++++++++---
 heat/tests/test_server.py       | 58 +++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/heat/engine/resources/server.py b/heat/engine/resources/server.py
index 5db2e9cd..eb1a37a5 100644
--- a/heat/engine/resources/server.py
+++ b/heat/engine/resources/server.py
@@ -360,14 +360,30 @@ class Server(resource.Resource):
         if key_name:
             nova_utils.get_keypair(self.nova(), key_name)
 
+        # either volume_id or snapshot_id needs to be specified, but not both
+        # for block device mapping.
+        bdm = self.properties.get('block_device_mapping') or []
+        bootable_vol = False
+        for mapping in bdm:
+            if mapping['device_name'] is 'vda':
+                    bootable_vol = True
+
+            if mapping.get('volume_id') and mapping.get('snapshot_id'):
+                raise exception.ResourcePropertyConflict('volume_id',
+                                                         'snapshot_id')
+            if not mapping.get('volume_id') and not mapping.get('snapshot_id'):
+                msg = _('Either volume_id or snapshot_id must be specified for'
+                        ' device mapping %s') % mapping['device_name']
+                raise exception.StackValidationFailed(message=msg)
+
         # make sure the image exists if specified.
         image = self.properties.get('image', None)
         if image:
             nova_utils.get_image_id(self.nova(), image)
-        else:
-            # TODO(sbaker) confirm block_device_mapping is populated
-            # for boot-by-volume (see LP bug #1215267)
-            pass
+        elif not image and not bootable_vol:
+            msg = _('Neither image nor bootable volume is specified for'
+                    ' instance %s') % self.name
+            raise exception.StackValidationFailed(message=msg)
 
     def handle_delete(self):
         '''
diff --git a/heat/tests/test_server.py b/heat/tests/test_server.py
index f39c1449..99aefb81 100644
--- a/heat/tests/test_server.py
+++ b/heat/tests/test_server.py
@@ -799,3 +799,61 @@ class ServersTest(HeatTestCase):
                 'delete_on_termination': True
             }
         ]))
+
+    def test_validate_conflict_block_device_mapping_props(self):
+        stack_name = 'test_validate_conflict_block_device_mapping_props'
+        (t, stack) = self._setup_test_stack(stack_name)
+
+        bdm = [{'device_name': 'vdb', 'snapshot_id': '1234',
+                'volume_id': '1234'}]
+        t['Resources']['WebServer']['Properties']['block_device_mapping'] = bdm
+        server = servers.Server('server_create_image_err',
+                                t['Resources']['WebServer'], stack)
+        self.m.StubOutWithMock(server, 'nova')
+        server.nova().MultipleTimes().AndReturn(self.fc)
+        self.m.ReplayAll()
+
+        self.assertRaises(exception.ResourcePropertyConflict, server.validate)
+        self.m.VerifyAll()
+
+    def test_validate_insufficient_block_device_mapping_props(self):
+        stack_name = 'test_validate_insufficient_block_device_mapping_props'
+        (t, stack) = self._setup_test_stack(stack_name)
+
+        bdm = [{'device_name': 'vdb', 'volume_size': '1',
+                'delete_on_termination': True}]
+        t['Resources']['WebServer']['Properties']['block_device_mapping'] = bdm
+        server = servers.Server('server_create_image_err',
+                                t['Resources']['WebServer'], stack)
+        self.m.StubOutWithMock(server, 'nova')
+        server.nova().MultipleTimes().AndReturn(self.fc)
+        self.m.ReplayAll()
+
+        ex = self.assertRaises(exception.StackValidationFailed,
+                               server.validate)
+        msg = 'Either volume_id or snapshot_id must be specified for device' +\
+              ' mapping vdb'
+        self.assertEqual(msg, str(ex))
+
+        self.m.VerifyAll()
+
+    def test_validate_without_image_or_bootable_volume(self):
+        stack_name = 'test_validate_without_image_or_bootable_volume'
+        (t, stack) = self._setup_test_stack(stack_name)
+
+        del t['Resources']['WebServer']['Properties']['image']
+        bdm = [{'device_name': 'vdb', 'volume_id': '1234'}]
+        t['Resources']['WebServer']['Properties']['block_device_mapping'] = bdm
+        server = servers.Server('server_create_image_err',
+                                t['Resources']['WebServer'], stack)
+        self.m.StubOutWithMock(server, 'nova')
+        server.nova().MultipleTimes().AndReturn(self.fc)
+        self.m.ReplayAll()
+
+        ex = self.assertRaises(exception.StackValidationFailed,
+                               server.validate)
+        msg = 'Neither image nor bootable volume is specified for instance %s'\
+            % server.name
+        self.assertEqual(msg, str(ex))
+
+        self.m.VerifyAll()
-- 
2.45.2