Prevent excessive validation for maxPersonality limit

We have PERSONALITY property on OS::Nova::Server that defaults to {},
but code that triggers check for limits compares value of property
to None -> this property always get validated for limits (if limit is
set).

Change-Id: I552a03cd9f4ab570aa059349ab361bb50c9c0d1e
Closes-bug: #1333283
(cherry picked from commit 78b68ba0da5d6ad71397538e484fb7565333129b)

2 files changed, 27 insertions, 16 deletions

diff --git a/heat/engine/resources/server.py b/heat/engine/resources/server.py
index 4f5f45ced..bfbc2518b 100644
--- a/heat/engine/resources/server.py
+++ b/heat/engine/resources/server.py
@@ -929,7 +929,7 @@ class Server(stack_user.StackUser):
         # retrieve provider's absolute limits if it will be needed
         metadata = self.properties.get(self.METADATA)
         personality = self._personality()
-        if metadata is not None or personality is not None:
+        if metadata is not None or personality:
             limits = nova_utils.absolute_limits(self.nova())
 
         # if 'security_groups' present for the server and explict 'port'
@@ -951,7 +951,7 @@ class Server(stack_user.StackUser):
 
         # verify the number of personality files and the size of each
         # personality file against the provider's absolute limits
-        if personality is not None:
+        if personality:
             msg = _("The personality property may not contain "
                     "greater than %s entries.") % limits['maxPersonality']
             self._check_maximum(len(personality),
diff --git a/heat/tests/test_server.py b/heat/tests/test_server.py
index aad04dbfd..504777d12 100644
--- a/heat/tests/test_server.py
+++ b/heat/tests/test_server.py
@@ -729,8 +729,6 @@ class ServersTest(HeatTestCase):
         server = servers.Server('server_create_image_err',
                                 t['Resources']['WebServer'], stack)
 
-        self.m.StubOutWithMock(server, 'nova')
-        server.nova().MultipleTimes().AndReturn(self.fc)
         self.m.StubOutWithMock(clients.OpenStackClients, 'nova')
         clients.OpenStackClients.nova().MultipleTimes().AndReturn(self.fc)
         self.m.StubOutWithMock(image.ImageConstraint, "validate")
@@ -751,7 +749,7 @@ class ServersTest(HeatTestCase):
         web_server = t['Resources']['WebServer']
         del web_server['Properties']['image']
 
-        def create_server(device_name, mock_nova=True):
+        def create_server(device_name):
             self.m.UnsetStubs()
             web_server['Properties']['block_device_mapping'] = [{
                 "device_name": device_name,
@@ -760,9 +758,6 @@ class ServersTest(HeatTestCase):
             }]
             server = servers.Server('server_with_bootable_volume',
                                     web_server, stack)
-            if mock_nova:
-                self.m.StubOutWithMock(server, 'nova')
-                server.nova().MultipleTimes().AndReturn(self.fc)
             self.m.StubOutWithMock(clients.OpenStackClients, 'nova')
             clients.OpenStackClients.nova().MultipleTimes().AndReturn(self.fc)
             self.m.ReplayAll()
@@ -772,7 +767,7 @@ class ServersTest(HeatTestCase):
         self.assertIsNone(server.validate())
         server = create_server('vda')
         self.assertIsNone(server.validate())
-        server = create_server('vdb', mock_nova=False)
+        server = create_server('vdb')
         ex = self.assertRaises(exception.StackValidationFailed,
                                server.validate)
         self.assertEqual('Neither image nor bootable volume is specified for '
@@ -813,7 +808,6 @@ class ServersTest(HeatTestCase):
                                 t['Resources']['WebServer'], stack)
 
         self.m.StubOutWithMock(server, 'nova')
-        server.nova().MultipleTimes().AndReturn(self.fc)
         self.m.StubOutWithMock(image.ImageConstraint, "validate")
         image.ImageConstraint.validate(
             mox.IgnoreArg(), mox.IgnoreArg()).MultipleTimes().AndReturn(True)
@@ -906,8 +900,6 @@ class ServersTest(HeatTestCase):
         server = servers.Server('server_validate_net_security_groups',
                                 t['Resources']['WebServer'], stack)
 
-        self.m.StubOutWithMock(server, 'nova')
-        server.nova().MultipleTimes().AndReturn(self.fc)
         self.m.StubOutWithMock(clients.OpenStackClients, 'nova')
         clients.OpenStackClients.nova().MultipleTimes().AndReturn(self.fc)
         self.m.ReplayAll()
@@ -1682,8 +1674,6 @@ class ServersTest(HeatTestCase):
         server = servers.Server('server_create_image_err',
                                 t['Resources']['WebServer'], stack)
 
-        self.m.StubOutWithMock(server, 'nova')
-        server.nova().MultipleTimes().AndReturn(self.fc)
         self.m.StubOutWithMock(clients.OpenStackClients, 'nova')
         clients.OpenStackClients.nova().MultipleTimes().AndReturn(self.fc)
         self.m.ReplayAll()
@@ -1700,8 +1690,6 @@ class ServersTest(HeatTestCase):
         server = servers.Server('server_create_image_err',
                                 t['Resources']['WebServer'], stack)
 
-        self.m.StubOutWithMock(server, 'nova')
-        server.nova().MultipleTimes().AndReturn(self.fc)
         self.m.StubOutWithMock(clients.OpenStackClients, 'nova')
         clients.OpenStackClients.nova().MultipleTimes().AndReturn(self.fc)
         self.m.ReplayAll()
@@ -2428,6 +2416,29 @@ class ServersTest(HeatTestCase):
         self.assertEqual((server.UPDATE, server.COMPLETE), server.state)
         self.m.VerifyAll()
 
+    def test_server_dont_validate_personality_if_personality_isnt_set(self):
+        stack_name = 'srv_val'
+        (tmpl, stack) = self._setup_test_stack(stack_name)
+
+        server = servers.Server('server_create_image_err',
+                                tmpl['Resources']['WebServer'], stack)
+
+        # We mock out nova_utils.absolute_limits but we don't specify
+        # how this mock should behave, so mox will verify that this mock
+        # is NOT called during call to server.validate().
+        # This is the way to validate that no excessive calls to Nova
+        # are made during validation.
+        self.m.StubOutWithMock(nova_utils, 'absolute_limits')
+        self.m.StubOutWithMock(clients.OpenStackClients, 'nova')
+        clients.OpenStackClients.nova().MultipleTimes().AndReturn(self.fc)
+        self.m.ReplayAll()
+
+        # Assert here checks that server resource validates, but actually
+        # this call is Act stage of this test. We calling server.validate()
+        # to verify that no excessive calls to Nova are made during validation.
+        self.assertIsNone(server.validate())
+        self.m.VerifyAll()
+
 
 class FlavorConstraintTest(HeatTestCase):