Merge "Do not pass rich objects in Cloud LoadBalancer create"

2 files changed, 223 insertions, 74 deletions

diff --git a/contrib/rackspace/rackspace/resources/cloud_loadbalancer.py b/contrib/rackspace/rackspace/resources/cloud_loadbalancer.py
index 219bdf479..fcd834fc8 100644
--- a/contrib/rackspace/rackspace/resources/cloud_loadbalancer.py
+++ b/contrib/rackspace/rackspace/resources/cloud_loadbalancer.py
@@ -25,7 +25,6 @@ from heat.engine import constraints
 from heat.engine import function
 from heat.engine import properties
 from heat.engine import resource
-from heat.engine import scheduler
 from heat.engine import support
 
 try:
@@ -483,10 +482,11 @@ class CloudLoadBalancer(resource.Resource):
 
         return (session_persistence, connection_logging, metadata)
 
-    def _check_active(self):
+    def _check_active(self, lb=None):
         """Update the loadbalancer state, check the status."""
-        loadbalancer = self.clb.get(self.resource_id)
-        if loadbalancer.status == self.ACTIVE_STATUS:
+        if not lb:
+            lb = self.clb.get(self.resource_id)
+        if lb.status == self.ACTIVE_STATUS:
             return True
         else:
             return False
@@ -502,46 +502,6 @@ class CloudLoadBalancer(resource.Resource):
             return True
         return False
 
-    def _configure_post_creation(self, loadbalancer):
-        """Configure all load balancer properties post creation.
-
-        These properties can only be set after the load balancer is created.
-        """
-        if self.properties[self.ACCESS_LIST]:
-            while not self._check_active():
-                yield
-            loadbalancer.add_access_list(self.properties[self.ACCESS_LIST])
-
-        if self.properties[self.ERROR_PAGE]:
-            while not self._check_active():
-                yield
-            loadbalancer.set_error_page(self.properties[self.ERROR_PAGE])
-
-        if self.properties[self.SSL_TERMINATION]:
-            while not self._check_active():
-                yield
-            ssl_term = self.properties[self.SSL_TERMINATION]
-            loadbalancer.add_ssl_termination(
-                ssl_term[self.SSL_TERMINATION_SECURE_PORT],
-                ssl_term[self.SSL_TERMINATION_PRIVATEKEY],
-                ssl_term[self.SSL_TERMINATION_CERTIFICATE],
-                intermediateCertificate=ssl_term[
-                    self.SSL_TERMINATION_INTERMEDIATE_CERTIFICATE],
-                enabled=True,
-                secureTrafficOnly=ssl_term[
-                    self.SSL_TERMINATION_SECURE_TRAFFIC_ONLY])
-
-        if self._valid_HTTPS_redirect_with_HTTP_prot():
-            while not self._check_active():
-                yield
-            loadbalancer.update(httpsRedirect=True)
-
-        if self.CONTENT_CACHING in self.properties:
-            enabled = self.properties[self.CONTENT_CACHING] == 'ENABLED'
-            while not self._check_active():
-                yield
-            loadbalancer.content_caching = enabled
-
     def _process_node(self, node):
         if not node.get(self.NODE_ADDRESSES):
             yield node
@@ -601,22 +561,111 @@ class CloudLoadBalancer(resource.Resource):
         lb_name = (self.properties.get(self.NAME) or
                    self.physical_resource_name())
         LOG.debug("Creating loadbalancer: %s" % {lb_name: lb_body})
-        loadbalancer = self.clb.create(lb_name, **lb_body)
-        self.resource_id_set(str(loadbalancer.id))
+        lb = self.clb.create(lb_name, **lb_body)
+        self.resource_id_set(str(lb.id))
+
+    def check_create_complete(self, *args):
+        lb = self.clb.get(self.resource_id)
+        return (self._check_active(lb) and
+                self._create_access_list(lb) and
+                self._create_errorpage(lb) and
+                self._create_ssl_term(lb) and
+                self._create_redirect(lb) and
+                self._create_cc(lb))
+
+    def _create_access_list(self, lb):
+        if not self.properties[self.ACCESS_LIST]:
+            return True
+
+        old_access_list = lb.get_access_list()
+        new_access_list = self.properties[self.ACCESS_LIST]
+        if not self._access_list_needs_update(old_access_list,
+                                              new_access_list):
+            return True
+
+        try:
+            lb.add_access_list(new_access_list)
+        except Exception as exc:
+            if lb_immutable(exc):
+                return False
+            raise
+        return False
+
+    def _create_errorpage(self, lb):
+        if not self.properties[self.ERROR_PAGE]:
+            return True
+
+        old_errorpage = lb.get_error_page()
+        new_errorpage_content = self.properties[self.ERROR_PAGE]
+        new_errorpage = {'errorpage': {'content': new_errorpage_content}}
+        if not self._errorpage_needs_update(old_errorpage, new_errorpage):
+            return True
+
+        try:
+            lb.set_error_page(new_errorpage_content)
+        except Exception as exc:
+            if lb_immutable(exc):
+                return False
+            raise
+        return False
 
-        post_create = scheduler.TaskRunner(self._configure_post_creation,
-                                           loadbalancer)
-        post_create(timeout=600)
-        return loadbalancer
+    def _create_ssl_term(self, lb):
+        if not self.properties[self.SSL_TERMINATION]:
+            return True
 
-    def check_create_complete(self, loadbalancer):
-        return self._check_active()
+        old_ssl_term = lb.get_ssl_termination()
+        new_ssl_term = self.properties[self.SSL_TERMINATION]
+        new_ssl_term['enabled'] = True
+        if not self._ssl_term_needs_update(old_ssl_term, new_ssl_term):
+            return True
+
+        try:
+            lb.add_ssl_termination(**new_ssl_term)
+        except Exception as exc:
+            if lb_immutable(exc):
+                return False
+            raise
+        return False
+
+    def _create_redirect(self, lb):
+        if not self._valid_HTTPS_redirect_with_HTTP_prot():
+            return True
+
+        old_redirect = lb.httpsRedirect
+        new_redirect = self.properties[self.HTTPS_REDIRECT]
+        if not self._redirect_needs_update(old_redirect, new_redirect):
+            return True
+
+        try:
+            lb.update(httpsRedirect=True)
+        except Exception as exc:
+            if lb_immutable(exc):
+                return False
+            raise
+        return False
+
+    def _create_cc(self, lb):
+        if not self.properties[self.CONTENT_CACHING]:
+            return True
+
+        old_cc = lb.content_caching
+        new_cc = self.properties[self.CONTENT_CACHING] == 'ENABLED'
+        if not self._cc_needs_update(old_cc, new_cc):
+            return True
+
+        try:
+            lb.content_caching = new_cc
+        except Exception as exc:
+            if lb_immutable(exc):
+                return False
+            raise
+        return False
 
     def handle_check(self):
-        loadbalancer = self.clb.get(self.resource_id)
+        lb = self.clb.get(self.resource_id)
         if not self._check_active():
-            raise exception.Error(_("Cloud LoadBalancer is not ACTIVE "
-                                    "(was: %s)") % loadbalancer.status)
+            raise exception.Error(_("Cloud Loadbalancer is not ACTIVE "
+                                    "(was: %s)") % lb.status)
 
     def handle_update(self, json_snippet, tmpl_diff, prop_diff):
         return prop_diff
@@ -721,6 +770,14 @@ class CloudLoadBalancer(resource.Resource):
     def _ssl_term_needs_update(self, old, new):
         return self._needs_update_comparison_nullable(old, new)  # dict
 
+    def _access_list_needs_update(self, old, new):
+        old = set([frozenset(s.items()) for s in old])
+        new = set([frozenset(s.items()) for s in new])
+        return old != new
+
+    def _redirect_needs_update(self, old, new):
+        return self._needs_update_comparison_bool(old, new)  # bool
+
     def _update_props(self, lb, prop_diff):
         old_props = {}
         new_props = {}
diff --git a/contrib/rackspace/rackspace/tests/test_cloud_loadbalancer.py b/contrib/rackspace/rackspace/tests/test_cloud_loadbalancer.py
index b3492e314..c1b5a1e7b 100644
--- a/contrib/rackspace/rackspace/tests/test_cloud_loadbalancer.py
+++ b/contrib/rackspace/rackspace/tests/test_cloud_loadbalancer.py
@@ -234,6 +234,7 @@ class FakeLoadBalancer(object):
         self.port = None
         self.name = None
         self.halfClosed = None
+        self.content_caching = False
 
     def get(self, *args, **kwargs):
         pass
@@ -295,6 +296,9 @@ class FakeLoadBalancer(object):
     def get_ssl_termination(self, *args, **kwargs):
         pass
 
+    def get_access_list(self, *args, **kwargs):
+        pass
+
 
 class LoadBalancerWithFakeClient(lb.CloudLoadBalancer):
     def cloud_lb(self):
@@ -380,6 +384,7 @@ class LoadBalancerTest(common.HeatTestCase):
 
         fake_lb = FakeLoadBalancer(name=lb_name)
         fake_lb.status = 'ACTIVE'
+        fake_lb.resource_id = 1234
 
         self.m.StubOutWithMock(rsrc.clb, 'create')
         rsrc.clb.create(lb_name, **lb_body).AndReturn(fake_lb)
@@ -611,6 +616,10 @@ class LoadBalancerTest(common.HeatTestCase):
         rsrc, fake_lb = self._mock_loadbalancer(template,
                                                 self.lb_name,
                                                 self.expected_body)
+        self.m.StubOutWithMock(fake_lb, 'get_access_list')
+        fake_lb.get_access_list().AndReturn([])
+        fake_lb.get_access_list().AndReturn(access_list)
+
         self.m.StubOutWithMock(fake_lb, 'add_access_list')
         fake_lb.add_access_list(access_list)
 
@@ -638,6 +647,11 @@ class LoadBalancerTest(common.HeatTestCase):
         rsrc, fake_lb = self._mock_loadbalancer(template,
                                                 self.lb_name,
                                                 self.expected_body)
+        self.m.StubOutWithMock(fake_lb, 'get_error_page')
+        fake_lb.get_error_page().AndReturn({u'errorpage': {u'content': u''}})
+        fake_lb.get_error_page().AndReturn(
+            {u'errorpage': {u'content': error_page}})
+
         self.m.StubOutWithMock(fake_lb, 'set_error_page')
         fake_lb.set_error_page(error_page)
 
@@ -646,27 +660,27 @@ class LoadBalancerTest(common.HeatTestCase):
         self.m.VerifyAll()
 
     def test_post_creation_ssl_termination(self):
-        ssl_termination = {
+        ssl_termination_template = {
             'securePort': 443,
             'privatekey': 'afwefawe',
             'certificate': 'fawefwea',
             'intermediateCertificate': "intermediate_certificate",
             'secureTrafficOnly': False
         }
+        ssl_termination_api = copy.deepcopy(ssl_termination_template)
+        ssl_termination_api['enabled'] = True
 
         template = self._set_template(self.lb_template,
-                                      sslTermination=ssl_termination)
+                                      sslTermination=ssl_termination_template)
         rsrc, fake_lb = self._mock_loadbalancer(template,
                                                 self.lb_name,
                                                 self.expected_body)
+        self.m.StubOutWithMock(fake_lb, 'get_ssl_termination')
+        fake_lb.get_ssl_termination().AndReturn({})
+        fake_lb.get_ssl_termination().AndReturn(ssl_termination_api)
+
         self.m.StubOutWithMock(fake_lb, 'add_ssl_termination')
-        fake_lb.add_ssl_termination(
-            ssl_termination['securePort'],
-            ssl_termination['privatekey'],
-            ssl_termination['certificate'],
-            intermediateCertificate=ssl_termination['intermediateCertificate'],
-            enabled=True,
-            secureTrafficOnly=ssl_termination['secureTrafficOnly'])
+        fake_lb.add_ssl_termination(**ssl_termination_api)
 
         self.m.ReplayAll()
         scheduler.TaskRunner(rsrc.create)()
@@ -911,6 +925,32 @@ class LoadBalancerTest(common.HeatTestCase):
         self.assertEqual((rsrc.UPDATE, rsrc.COMPLETE), rsrc.state)
         self.m.VerifyAll()
 
+    def test_create_immutable_exception(self):
+        access_list = [{"address": '192.168.1.1/0',
+                        'type': 'ALLOW'},
+                       {'address': '172.165.3.43',
+                        'type': 'DENY'}]
+
+        template = self._set_template(self.lb_template,
+                                      accessList=access_list)
+        rsrc, fake_lb = self._mock_loadbalancer(template,
+                                                self.lb_name,
+                                                self.expected_body)
+        self.m.StubOutWithMock(fake_lb, 'get_access_list')
+        fake_lb.get_access_list().AndReturn({})
+        fake_lb.get_access_list().AndReturn({})
+        fake_lb.get_access_list().AndReturn(access_list)
+
+        self.m.StubOutWithMock(fake_lb, 'add_access_list')
+        msg = ("Load Balancer '%s' has a status of 'PENDING_UPDATE' and "
+               "is considered immutable." % rsrc.resource_id)
+        fake_lb.add_access_list(access_list).AndRaise(Exception(msg))
+        fake_lb.add_access_list(access_list)
+
+        self.m.ReplayAll()
+        scheduler.TaskRunner(rsrc.create)()
+        self.m.VerifyAll()
+
     def test_update_lb_name(self):
         rsrc, fake_lb = self._mock_loadbalancer(self.lb_template,
                                                 self.lb_name,
@@ -1066,16 +1106,18 @@ class LoadBalancerTest(common.HeatTestCase):
         self.m.VerifyAll()
 
     def test_lb_redirect_HTTP_with_SSL_term(self):
-        ssl_termination = {
+        ssl_termination_template = {
             'privatekey': private_key,
             'intermediateCertificate': 'fwaefawe',
             'secureTrafficOnly': True,
             'securePort': 443,
             'certificate': cert
         }
+        ssl_termination_api = copy.deepcopy(ssl_termination_template)
+        ssl_termination_api['enabled'] = True
         template = self._set_template(
-            self.lb_template, sslTermination=ssl_termination, protocol="HTTP",
-            httpsRedirect=True)
+            self.lb_template, sslTermination=ssl_termination_template,
+            protocol="HTTP", httpsRedirect=True)
 
         expected = self._set_expected(
             self.expected_body, protocol="HTTP", httpsRedirect=False)
@@ -1083,10 +1125,31 @@ class LoadBalancerTest(common.HeatTestCase):
         rsrc, fake_lb = self._mock_loadbalancer(template,
                                                 self.lb_name,
                                                 expected)
+
+        self.m.UnsetStubs()
+        self.m.StubOutWithMock(rsrc.clb, 'create')
+        rsrc.clb.create(self.lb_name, **expected).AndReturn(fake_lb)
+        self.m.StubOutWithMock(rsrc.clb, 'get')
+        rsrc.clb.get(mox.IgnoreArg()).AndReturn(fake_lb)
+        rsrc.clb.get(mox.IgnoreArg()).AndReturn(fake_lb)
+
+        fake_lb1 = copy.deepcopy(fake_lb)
+        fake_lb1.httpsRedirect = True
+        rsrc.clb.get(mox.IgnoreArg()).AndReturn(fake_lb1)
+        rsrc.clb.get(mox.IgnoreArg()).AndReturn(fake_lb1)
+        rsrc.clb.get(mox.IgnoreArg()).AndReturn(fake_lb1)
+
+        self.m.StubOutWithMock(fake_lb, 'get_ssl_termination')
+        fake_lb.get_ssl_termination().AndReturn({})
+        fake_lb.get_ssl_termination().AndReturn(ssl_termination_api)
+        self.m.StubOutWithMock(fake_lb1, 'get_ssl_termination')
+        fake_lb1.get_ssl_termination().AndReturn(ssl_termination_api)
+        fake_lb1.get_ssl_termination().AndReturn(ssl_termination_api)
+        fake_lb1.get_ssl_termination().AndReturn(ssl_termination_api)
+
         self.m.ReplayAll()
         scheduler.TaskRunner(rsrc.create)()
         self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state)
-        self.m.VerifyAll()
 
     def test_update_lb_half_closed(self):
         rsrc, fake_lb = self._mock_loadbalancer(self.lb_template,
@@ -1304,26 +1367,41 @@ class LoadBalancerTest(common.HeatTestCase):
 
     def test_update_ssl_termination_delete(self):
         template = copy.deepcopy(self.lb_template)
-        lb_name = list(six.iterkeys(template['Resources']))[0]
-        template['Resources'][lb_name]['Properties']['sslTermination'] = {
+        ssl_termination_template = {
             'securePort': 443, 'privatekey': private_key, 'certificate': cert,
-            'secureTrafficOnly': False}
+            'intermediateCertificate': '', 'secureTrafficOnly': False}
+        ssl_termination_api = copy.deepcopy(ssl_termination_template)
+        ssl_termination_api['enabled'] = True
+        lb_name = list(six.iterkeys(template['Resources']))[0]
+        template['Resources'][lb_name]['Properties']['sslTermination'] = \
+            ssl_termination_template
         # The SSL termination config is done post-creation, so no need
         # to modify self.expected_body
         rsrc, fake_lb = self._mock_loadbalancer(template,
                                                 self.lb_name,
                                                 self.expected_body)
 
+        self.m.StubOutWithMock(fake_lb, 'get_ssl_termination')
+        fake_lb.get_ssl_termination().AndReturn({})
+
+        self.m.StubOutWithMock(fake_lb, 'add_ssl_termination')
+        fake_lb.add_ssl_termination(**ssl_termination_api)
+
+        fake_lb.get_ssl_termination().AndReturn(ssl_termination_api)
+
         self.m.ReplayAll()
         scheduler.TaskRunner(rsrc.create)()
 
+        self.m.UnsetStubs()
         update_template = copy.deepcopy(rsrc.t)
         del update_template['Properties']['sslTermination']
 
+        self.m.StubOutWithMock(rsrc.clb, 'get')
+        rsrc.clb.get(mox.IgnoreArg()).MultipleTimes().AndReturn(
+            fake_lb)
+
         self.m.StubOutWithMock(fake_lb, 'get_ssl_termination')
-        fake_lb.get_ssl_termination().AndReturn({
-            'securePort': 443, 'privatekey': private_key, 'certificate': cert,
-            'secureTrafficOnly': False})
+        fake_lb.get_ssl_termination().AndReturn(ssl_termination_api)
 
         self.m.StubOutWithMock(fake_lb, 'delete_ssl_termination')
         fake_lb.delete_ssl_termination()
@@ -1430,12 +1508,26 @@ class LoadBalancerTest(common.HeatTestCase):
                                                 self.lb_name,
                                                 self.expected_body)
 
+        self.m.StubOutWithMock(fake_lb, 'get_error_page')
+        fake_lb.get_error_page().AndReturn({})
+
+        self.m.StubOutWithMock(fake_lb, 'set_error_page')
+        fake_lb.set_error_page(error_page)
+
+        fake_lb.get_error_page().AndReturn({'errorpage':
+                                            {'content': error_page}})
+
         self.m.ReplayAll()
         scheduler.TaskRunner(rsrc.create)()
 
+        self.m.UnsetStubs()
         update_template = copy.deepcopy(rsrc.t)
         del update_template['Properties']['errorPage']
 
+        self.m.StubOutWithMock(rsrc.clb, 'get')
+        rsrc.clb.get(mox.IgnoreArg()).MultipleTimes().AndReturn(
+            fake_lb)
+
         self.m.StubOutWithMock(fake_lb, 'clear_error_page')
         fake_lb.clear_error_page()