Cleanup deleted load balancers in housekeeper's db_cleanup

When load balancer is deleted the corresponding DB entry is marked
as DELETED and is never actually removed along with a VIP
associated whit this load balancer.

This adds a new method to db_cleanup routine that scans the DB for
load balancers with DELETED provisioning_status and deletes them
from db if they are older than load_balancer_expiry_age. Corresponding
VIP entries are deleted in cascade.

Added new config option `load_balancer_expiry_age` to the `house_keeping`
config section.

Also changed the default value of exp_age argument to
CONF.house_keeping.amphora_expiry_age in check_amphora_expiry_age
method.

DocImpact
Closes-Bug #1573725

Change-Id: I4f99d38f44f218ac55a76ef062ed9ea401c0a02d

doc/source/config-reference/octavia-config-table.rst

@@ -218,6 +218,8 @@ Use the following options in the /etc/octavia/octavia.conf file.
      -
    * - ``amphora_expiry_age`` = ``604800``
      - (IntOpt) Amphora expiry age in seconds
+   * - ``load_balancer_expiry_age`` = ``604800``
+     - (IntOpt) Load balancer expiry age in seconds
    * - ``cert_expiry_buffer`` = ``1209600``
      - (IntOpt) Seconds until certificate expiration
    * - ``cert_interval`` = ``3600``

etc/octavia.conf

@@ -211,6 +211,9 @@
 # Amphora expiry age in seconds. Default is 1 week
 # amphora_expiry_age = 604800
 
+# Load balancer expiry age in seconds. Default is 1 week
+# load_balancer_expiry_age = 604800
+
 [amphora_agent]
 # agent_server_ca = /etc/octavia/certs/client_ca.pem
 # agent_server_cert = /etc/octavia/certs/server.pem

octavia/cmd/house_keeping.py

@@ -52,17 +52,20 @@ def spare_amphora_check():
 
 
 def db_cleanup():
-    """Perform db cleanup for old amphora."""
+    """Perform db cleanup for old resources."""
     # Read the interval from CONF
     interval = CONF.house_keeping.cleanup_interval
     LOG.info(_LI("DB cleanup interval is set to %d sec"), interval)
     LOG.info(_LI('Amphora expiry age is %s seconds'),
              CONF.house_keeping.amphora_expiry_age)
+    LOG.info(_LI('Load balancer expiry age is %s seconds'),
+             CONF.house_keeping.load_balancer_expiry_age)
 
     db_cleanup = house_keeping.DatabaseCleanup()
     while db_cleanup_thread_event.is_set():
-        LOG.debug("Initiating the cleanup of old amphora...")
+        LOG.debug("Initiating the cleanup of old resources...")
         db_cleanup.delete_old_amphorae()
+        db_cleanup.cleanup_load_balancers()
         time.sleep(interval)
 
 

octavia/common/config.py

@@ -303,6 +303,9 @@ house_keeping_opts = [
     cfg.IntOpt('amphora_expiry_age',
                default=604800,
                help=_('Amphora expiry age in seconds')),
+    cfg.IntOpt('load_balancer_expiry_age',
+               default=604800,
+               help=_('Load balancer expiry age in seconds')),
     cfg.IntOpt('cert_interval',
                default=3600,
                help=_('Certificate check interval in seconds')),

octavia/controller/housekeeping/house_keeping.py

@@ -65,6 +65,7 @@ class DatabaseCleanup(object):
     def __init__(self):
         self.amp_repo = repo.AmphoraRepository()
         self.amp_health_repo = repo.AmphoraHealthRepository()
+        self.lb_repo = repo.LoadBalancerRepository()
 
     def delete_old_amphorae(self):
         """Checks the DB for old amphora and deletes them based on it's age."""
@@ -81,6 +82,23 @@ class DatabaseCleanup(object):
                 self.amp_repo.delete(session, id=amp.id)
                 LOG.info(_LI('Deleted Amphora id : %s') % amp.id)
 
+    def cleanup_load_balancers(self):
+        """Checks the DB for old load balancers and triggers their removal."""
+        exp_age = datetime.timedelta(
+            seconds=CONF.house_keeping.load_balancer_expiry_age)
+
+        session = db_api.get_session()
+        load_balancers = self.lb_repo.get_all(
+            session, provisioning_status=constants.DELETED)
+
+        for lb in load_balancers:
+            if self.lb_repo.check_load_balancer_expired(session, lb.id,
+                                                        exp_age):
+                LOG.info(_LI('Attempting to delete load balancer id : %s'),
+                         lb.id)
+                self.lb_repo.delete(session, id=lb.id)
+                LOG.info(_LI('Deleted load balancer id : %s') % lb.id)
+
 
 class CertRotation(object):
     def __init__(self):

octavia/db/repositories.py

@@ -328,6 +328,33 @@ class LoadBalancerRepository(BaseRepository):
             session.add(lb)
             return True
 
+    def check_load_balancer_expired(self, session, lb_id, exp_age=None):
+        """Checks if a given load balancer is expired.
+
+        :param session: A Sql Alchemy database session.
+        :param lb_id: id of an load balancer object
+        :param exp_age: A standard datetime delta which is used to see for how
+        long can a load balancer live without updates before it is considered
+        expired (default: CONF.house_keeping.load_balancer_expiry_age)
+        :returns: boolean
+        """
+        if not exp_age:
+            exp_age = datetime.timedelta(
+                seconds=CONF.house_keeping.load_balancer_expiry_age)
+
+        timestamp = datetime.datetime.utcnow() - exp_age
+        lb = self.get(session, id=lb_id)
+        if lb:
+            # If a load balancer was never updated use its creation timestamp
+            last_update = lb.updated_at or lb.created_at
+            # Convert string to timestamp
+            last_update = datetime.datetime.strptime(last_update,
+                                                     "%Y-%m-%d %H:%M:%S.%f")
+            return last_update < timestamp
+        else:
+            # Load balancer was just deleted.
+            return True
+
 
 class VipRepository(BaseRepository):
     model_class = models.Vip
@@ -609,15 +636,16 @@ class AmphoraHealthRepository(BaseRepository):
 
         :param session: A Sql Alchemy database session.
         :param amphora_id: id of an amphora object
-        :param exp_age: A standard datetime which is used to see if an
-        amphora needs to be updated (default: now - 10s)
+        :param exp_age: A standard datetime delta which is used to see for how
+        long can an amphora live without updates before it is considered
+        expired (default: CONF.house_keeping.amphora_expiry_age)
         :returns: boolean
         """
         if not exp_age:
-            timestamp = datetime.datetime.utcnow() - datetime.timedelta(
-                seconds=10)
-        else:
-            timestamp = datetime.datetime.utcnow() - exp_age
+            exp_age = datetime.timedelta(
+                seconds=CONF.house_keeping.amphora_expiry_age)
+
+        timestamp = datetime.datetime.utcnow() - exp_age
         amphora_health = self.get(session, amphora_id=amphora_id)
         if amphora_health is not None:
             return amphora_health.last_update < timestamp

octavia/tests/functional/db/test_repositories.py

@@ -1248,6 +1248,36 @@ class LoadBalancerRepositoryTest(BaseRepositoryTest):
         lb = self.lb_repo.get(self.session, id=lb_id)
         self.assertEqual(constants.PENDING_UPDATE, lb.provisioning_status)
 
+    def test_check_load_balancer_expired_default_exp_age(self):
+        """When exp_age defaults to load_balancer_expiry_age."""
+        newdate = datetime.datetime.utcnow() - datetime.timedelta(minutes=10)
+        self.lb_repo.create(self.session, id=self.FAKE_UUID_1,
+                            project_id=self.FAKE_UUID_2,
+                            provisioning_status=constants.ACTIVE,
+                            operating_status=constants.ONLINE,
+                            enabled=True,
+                            updated_at=newdate)
+        check_res = self.lb_repo.check_load_balancer_expired(
+            self.session, self.FAKE_UUID_1)
+        # Default load_balancer_expiry_age value is 1 week so load balancer
+        # shouldn't be considered expired.
+        self.assertFalse(check_res)
+
+    def test_check_load_balancer_expired_with_exp_age(self):
+        """When exp_age is passed as an argument."""
+        exp_age = datetime.timedelta(
+            seconds=self.FAKE_EXP_AGE)
+        newdate = datetime.datetime.utcnow() - datetime.timedelta(minutes=10)
+        self.lb_repo.create(self.session, id=self.FAKE_UUID_1,
+                            project_id=self.FAKE_UUID_2,
+                            provisioning_status=constants.ACTIVE,
+                            operating_status=constants.ONLINE,
+                            enabled=True,
+                            updated_at=newdate)
+        check_res = self.lb_repo.check_load_balancer_expired(
+            self.session, self.FAKE_UUID_1, exp_age)
+        self.assertTrue(check_res)
+
 
 class VipRepositoryTest(BaseRepositoryTest):
 
@@ -1538,12 +1568,14 @@ class AmphoraHealthRepositoryTest(BaseRepositoryTest):
         self.assertIsInstance(new_amphora_health, models.AmphoraHealth)
         self.assertEqual(amphora_health, new_amphora_health)
 
-    def test_check_amphora_out_of_date(self):
-        """When exp_age is None."""
+    def test_check_amphora_expired_default_exp_age(self):
+        """When exp_age defaults to CONF.house_keeping.amphora_expiry_age."""
         self.create_amphora_health(self.amphora.id)
         checkres = self.amphora_health_repo.check_amphora_expired(
             self.session, self.amphora.id)
-        self.assertTrue(checkres)
+        # Default amphora_expiry_age value is 1 week so amphora shouldn't be
+        # considered expired.
+        self.assertFalse(checkres)
 
     def test_check_amphora_expired_with_exp_age(self):
         """When exp_age is passed as an argument."""

octavia/tests/unit/controller/housekeeping/test_house_keeping.py

@@ -92,6 +92,7 @@ class TestDatabaseCleanup(base.TestCase):
         self.amp_health_repo = mock.MagicMock()
         self.amp_repo = mock.MagicMock()
         self.amp = repo.AmphoraRepository()
+        self.lb = repo.LoadBalancerRepository()
 
         self.dbclean.amp_repo = self.amp_repo
         self.dbclean.amp_health_repo = self.amp_health_repo
@@ -133,6 +134,30 @@ class TestDatabaseCleanup(base.TestCase):
         self.assertTrue(self.amp_health_repo.check_amphora_expired.called)
         self.assertFalse(self.amp_repo.delete.called)
 
+    @mock.patch('octavia.db.api.get_session')
+    def test_delete_old_load_balancer(self, session):
+        """Check delete of load balancers in DELETED provisioning status."""
+        self.CONF.house_keeping.load_balancer_expiry_age = self.FAKE_EXP_AGE
+        session.return_value = session
+        load_balancer = self.lb.create(session, id=self.FAKE_UUID_1,
+                                       provisioning_status=constants.DELETED,
+                                       operating_status=constants.OFFLINE,
+                                       enabled=True)
+
+        for expired_status in [True, False]:
+            lb_repo = mock.MagicMock()
+            self.dbclean.lb_repo = lb_repo
+            lb_repo.get_all.return_value = [load_balancer]
+            lb_repo.check_load_balancer_expired.return_value = (
+                expired_status)
+            self.dbclean.cleanup_load_balancers()
+            self.assertTrue(lb_repo.get_all.called)
+            self.assertTrue(lb_repo.check_load_balancer_expired.called)
+            if expired_status:
+                self.assertTrue(lb_repo.delete.called)
+            else:
+                self.assertFalse(lb_repo.delete.called)
+
 
 class TestCertRotation(base.TestCase):
     def setUp(self):

releasenotes/notes/load-balancer-expiry-age-a473f9147552f1b1.yaml

@@ -0,0 +1,10 @@
+---
+
+features:
+  - Stale load balancer entries with DELETED provisioning_status are now
+    cleaned-up by housekeeper after if they are older than
+    `load_balancer_expiry_age`.
+upgrade:
+  - New option `load_balancer_expiry_age` is added to the `house_keeping`
+    config section. It defines load balancer expiry age in seconds, the
+    default value is 604800.