From e61d89aa47cba71bb4dda12d836fde8a8fa7092c Mon Sep 17 00:00:00 2001
From: Rodrigo Barbieri <rodrigo.barbieri2010@gmail.com>
Date: Wed, 10 May 2023 13:33:08 -0300
Subject: [PATCH] Set nova config for rbd instance folder cleanup after
 evacuations

After evacuations and revert resizes when using rbd storage backend,
the instance folder is usually left behind and causes issues when
migrating the instance back to the host.
With the config option set, the nova-compute service will cleanup
those folders as part of the periodic checks that run for instances
that have been evacuated/migrated.

Closes-bug: #2019141
Change-Id: I846ccb0a95d04139b41fdad6cbf465d303d6cc09
---
 hooks/nova_compute_context.py            | 22 +++++++++++++++++
 templates/train/nova.conf                |  3 +++
 templates/yoga/nova.conf                 |  3 +++
 unit_tests/test_nova_compute_contexts.py | 30 ++++++++++++++++++++++++
 4 files changed, 58 insertions(+)

diff --git a/hooks/nova_compute_context.py b/hooks/nova_compute_context.py
index 8a4231b4..c6ab5f82 100644
--- a/hooks/nova_compute_context.py
+++ b/hooks/nova_compute_context.py
@@ -17,6 +17,7 @@ import os
 import platform
 import shutil
 import socket
+import subprocess
 import uuid
 
 from typing import (
@@ -97,6 +98,16 @@ def _network_manager():
     return manager()
 
 
+def is_local_fs(path):
+    result = False
+    try:
+        subprocess.check_call(["df", "-l", path])
+        result = True
+    except subprocess.CalledProcessError as e:
+        log("Error invoking df -l {}: {}".format(path, e), level=DEBUG)
+    return result
+
+
 def get_availability_zone():
     use_juju_az = config('customize-failure-domain')
     juju_az = os.environ.get('JUJU_AVAILABILITY_ZONE')
@@ -324,6 +335,17 @@ class NovaComputeLibvirtContext(context.OSContextGenerator):
 
         if config('libvirt-image-backend'):
             ctxt['libvirt_images_type'] = config('libvirt-image-backend')
+            if config('libvirt-image-backend') == 'rbd':
+                instances_path = config('instances-path')
+                if instances_path in ('', None):
+                    instances_path = '/var/lib/nova/instances'
+                if is_local_fs(instances_path):
+                    ctxt['ensure_libvirt_rbd_instance_dir_cleanup'] = True
+                else:
+                    log("Skipped enabling "
+                        "'ensure_libvirt_rbd_instance_dir_cleanup' because"
+                        " instances-path is not a local mount.",
+                        level=INFO)
 
         ctxt['force_raw_images'] = config('force-raw-images')
         ctxt['inject_password'] = config('inject-password')
diff --git a/templates/train/nova.conf b/templates/train/nova.conf
index 9f07d1b3..6fc71653 100644
--- a/templates/train/nova.conf
+++ b/templates/train/nova.conf
@@ -370,6 +370,9 @@ lock_path=/var/lock/nova
 
 [workarounds]
 disable_libvirt_livesnapshot = False
+{% if ensure_libvirt_rbd_instance_dir_cleanup -%}
+ensure_libvirt_rbd_instance_dir_cleanup = {{ ensure_libvirt_rbd_instance_dir_cleanup }}
+{% endif -%}
 
 {% include "parts/section-ephemeral" %}
 
diff --git a/templates/yoga/nova.conf b/templates/yoga/nova.conf
index 4824972e..16e702d1 100644
--- a/templates/yoga/nova.conf
+++ b/templates/yoga/nova.conf
@@ -353,6 +353,9 @@ lock_path=/var/lock/nova
 
 [workarounds]
 disable_libvirt_livesnapshot = False
+{% if ensure_libvirt_rbd_instance_dir_cleanup -%}
+ensure_libvirt_rbd_instance_dir_cleanup = {{ ensure_libvirt_rbd_instance_dir_cleanup }}
+{% endif -%}
 
 {% include "parts/section-ephemeral" %}
 
diff --git a/unit_tests/test_nova_compute_contexts.py b/unit_tests/test_nova_compute_contexts.py
index 09f3be7f..f49d91f5 100644
--- a/unit_tests/test_nova_compute_contexts.py
+++ b/unit_tests/test_nova_compute_contexts.py
@@ -386,6 +386,36 @@ class NovaComputeContextTests(CharmTestCase):
              'reserved_host_disk': 0,
              'reserved_host_memory': 512}, libvirt())
 
+    @patch.object(context.subprocess, 'check_call')
+    def test_ensure_rbd_cleanup_rbd(self, call_mock):
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'focal'}
+        self.os_release.return_value = 'ussuri'
+        self.test_config.set('libvirt-image-backend', 'rbd')
+        libvirt = context.NovaComputeLibvirtContext()
+        result = libvirt()
+        self.assertIn('ensure_libvirt_rbd_instance_dir_cleanup', result)
+        self.assertTrue(result['ensure_libvirt_rbd_instance_dir_cleanup'])
+        call_mock.assert_called_once_with(
+            ['df', '-l', '/var/lib/nova/instances'])
+
+    @patch.object(context.subprocess, 'check_call')
+    def test_ensure_rbd_cleanup_rbd_non_local_mount(self, call_mock):
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'focal'}
+        self.os_release.return_value = 'ussuri'
+        call_mock.side_effect = [context.subprocess.CalledProcessError(
+            1, 'df -l /var/foo/bar')]
+        self.test_config.set('libvirt-image-backend', 'rbd')
+        self.test_config.set('instances-path', '/var/foo/bar')
+        libvirt = context.NovaComputeLibvirtContext()
+        self.assertNotIn('ensure_libvirt_rbd_instance_dir_cleanup', libvirt())
+        call_mock.assert_called_once_with(['df', '-l', '/var/foo/bar'])
+
+    def test_ensure_rbd_cleanup_non_rbd(self):
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'focal'}
+        self.os_release.return_value = 'ussuri'
+        libvirt = context.NovaComputeLibvirtContext()
+        self.assertNotIn('ensure_libvirt_rbd_instance_dir_cleanup', libvirt())
+
     def test_libvirt_context_inject_password(self):
         self.lsb_release.return_value = {'DISTRIB_CODENAME': 'zesty'}
         self.os_release.return_value = 'ocata'