From df7430c2237d7c3ad3fcd25fba13cd34ce669181 Mon Sep 17 00:00:00 2001
From: Weii Wang <weii.wang@canonical.com>
Date: Fri, 15 Mar 2024 15:20:31 +0000
Subject: [PATCH] Fix storage ownership update during charm upgrade

---
 src/charm.py                 | 30 ++++++++++++------------------
 tests/unit/wordpress_mock.py |  5 +++++
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/charm.py b/src/charm.py
index 890f63ec..5a691aba 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -368,7 +368,6 @@ def _on_upgrade_charm(self, _event: UpgradeCharmEvent):
             _event: required by ops framework, not used.
         """
         self._setup_replica_data(_event)
-        self._change_uploads_directory_ownership(recursive=True)
 
     def _gen_wp_config(self):
         """Generate the wp-config.php file WordPress needs based on charm config and relations.
@@ -1448,25 +1447,20 @@ def _storage_mounted(self) -> bool:
         mount_info: str = container.pull("/proc/mounts").read()
         return self._WP_UPLOADS_PATH in mount_info
 
-    def _change_uploads_directory_ownership(self, recursive=False):
-        """Change uploads directory ownership.
-
-        Args:
-            recursive: Run chown recursively. Defaults to False.
-        """
-        command_list = [
-            "chown",
-            f"{self._WORDPRESS_USER}:{self._WORDPRESS_GROUP}",
-        ]
-
-        if recursive:
-            command_list.append("-R")
+    def _change_uploads_directory_ownership(self):
+        """Change uploads directory ownership, noop if ownership is correct."""
+        dir_current = self._container().list_files(self._WP_UPLOADS_PATH, itself=True)[0]
+        if dir_current.user == self._WORDPRESS_USER and dir_current.group == self._WORDPRESS_GROUP:
+            return
 
-        command_list.append(self._WP_UPLOADS_PATH)
         self._container().exec(
-            command_list,
-            timeout=120,
-        )
+            [
+                "chown",
+                f"{self._WORDPRESS_USER}:{self._WORDPRESS_GROUP}",
+                "-R",
+                self._WP_UPLOADS_PATH,
+            ]
+        ).wait()
 
     def _reconciliation(self, _event: EventBase) -> None:
         """Reconcile the WordPress charm on juju event.
diff --git a/tests/unit/wordpress_mock.py b/tests/unit/wordpress_mock.py
index d8bcf0e9..4f67599e 100644
--- a/tests/unit/wordpress_mock.py
+++ b/tests/unit/wordpress_mock.py
@@ -394,6 +394,11 @@ def exists(self, path):
 
     def list_files(self, path: str, itself=False):
         """Mock method for :meth:`ops.charm.model.Container.list_files`."""
+        if path == "/var/www/html/wp-content/uploads":
+            file_info_mock = unittest.mock.MagicMock()
+            file_info_mock.user = "_daemon_"
+            file_info_mock.group = "_daemon_"
+            return [file_info_mock]
         if not path.endswith("/"):
             path += "/"
         file_list = []