From 4856a068e8f3b9701671fb32a048a81833c17512 Mon Sep 17 00:00:00 2001
From: Robert Gildein <robert.gildein@canonical.com>
Date: Wed, 19 Apr 2023 16:46:41 +0200
Subject: [PATCH] Improve snap channel refresh mechanism

- stop vault.service before rephrasing it
- added a warning note that changing the channel config option will
  cause the vault to be sealed

Related-Bug: 2007587
Change-Id: I240ebb4bd14932a6bf95f41da3f2cd7776742266
(cherry picked from commit 9e927889d0e29de919816c315b2c6f5643f53049)
---
 src/config.yaml                            |  3 ++
 src/reactive/vault_handlers.py             | 15 ++++----
 unit_tests/test_reactive_vault_handlers.py | 41 ++++++++++------------
 3 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/src/config.yaml b/src/config.yaml
index d9bf2a6..a14ad96 100644
--- a/src/config.yaml
+++ b/src/config.yaml
@@ -56,6 +56,9 @@ options:
     default: 1.7/stable
     description: >-
       The snap channel to install from.
+      WARNING: Changing this value will cause ALL the vault units to become
+      sealed, due to the snap refresh and the service being restarted on each
+      unit.
   dns-ha-access-record:
     type: string
     default:
diff --git a/src/reactive/vault_handlers.py b/src/reactive/vault_handlers.py
index 0d1e78b..c6b51d7 100644
--- a/src/reactive/vault_handlers.py
+++ b/src/reactive/vault_handlers.py
@@ -42,8 +42,8 @@ from charmhelpers.core.hookenv import (
 from charmhelpers.core.host import (
     service,
     service_reload,
-    service_restart,
     service_running,
+    service_stop,
     write_file,
     is_container,
 )
@@ -185,12 +185,13 @@ def snap_refresh():
     channel = config('channel') or 'stable'
     if validate_snap_channel(channel):
         clear_flag('snap.channel.invalid')
-        snap.refresh('vault', channel=channel)
-        if vault.can_restart():
-            log("Restarting vault", level=DEBUG)
-            service_restart('vault')
-            if config('totally-unsecure-auto-unlock'):
-                vault.prepare_vault()
+        if snap.get_installed_channel("vault") != channel:
+            log("Stopping the vault.service to perform a snap refresh")
+            service_stop("vault")
+            snap.refresh("vault", channel=channel)
+            log("Vault was refreshed to {}".format(channel))
+            start_vault()
+            log("The vault.service has been started")
     else:
         set_flag('snap.channel.invalid')
 
diff --git a/unit_tests/test_reactive_vault_handlers.py b/unit_tests/test_reactive_vault_handlers.py
index 1945b08..9714dff 100644
--- a/unit_tests/test_reactive_vault_handlers.py
+++ b/unit_tests/test_reactive_vault_handlers.py
@@ -57,8 +57,8 @@ class TestHandlers(unit_tests.test_utils.CharmTestCase):
             'log',
             'network_get_primary_address',
             'open_port',
-            'service_restart',
             'service_running',
+            'service_stop',
             'service',
             'set_state',
             'status_set',
@@ -549,31 +549,28 @@ class TestHandlers(unit_tests.test_utils.CharmTestCase):
         self.config.assert_called_with('channel')
         self.set_flag.assert_called_with('snap.channel.invalid')
 
-    @patch.object(handlers.vault, 'can_restart')
-    def test_snap_refresh_restartable(self, can_restart):
-        conf = {
-            'channel': 'edge',
-            'totally-unsecure-auto-unlock': False}
+    @mock.patch.object(handlers, "start_vault")
+    def test_snap_refresh_version_not_changed(self, mock_start_vault):
+        conf = {'channel': '1.8/edge'}
         self.config.side_effect = lambda x: conf[x]
-        can_restart.return_value = True
+        self.snap.get_installed_channel.return_value = "1.8/edge"
         handlers.snap_refresh()
-        self.snap.refresh.assert_called_with('vault', channel='edge')
-        self.service_restart.assert_called_with('vault')
-        self.clear_flag.assert_called_with('snap.channel.invalid')
-        config_calls = [
-            mock.call('channel'),
-            mock.call('totally-unsecure-auto-unlock')]
-        self.config.assert_has_calls(config_calls)
+        self.config.assert_called_once_with("channel")
+        self.clear_flag.assert_called_once_with('snap.channel.invalid')
+        self.snap.refresh.assert_not_called()
+        mock_start_vault.assert_not_called()
 
-    @patch.object(handlers.vault, 'can_restart')
-    def test_snap_refresh_not_restartable(self, can_restart):
-        self.config.return_value = 'edge'
-        can_restart.return_value = False
+    @mock.patch.object(handlers, "start_vault")
+    def test_snap_refresh_version_changed(self, mock_start_vault):
+        conf = {'channel': '1.8/edge'}
+        self.config.side_effect = lambda x: conf[x]
+        self.snap.get_installed_channel.return_value = "1.8/stable"
         handlers.snap_refresh()
-        self.snap.refresh.assert_called_with('vault', channel='edge')
-        self.config.assert_called_with('channel')
-        self.service_restart.assert_not_called()
-        self.clear_flag.assert_called_with('snap.channel.invalid')
+        self.config.assert_called_with("channel")
+        self.clear_flag.assert_called_once_with('snap.channel.invalid')
+        self.service_stop.assert_called_once_with("vault")
+        self.snap.refresh.assert_called_once_with("vault", channel="1.8/edge")
+        mock_start_vault.assert_called_once()
 
     def test_snap_refresh_invalid_channel(self):
         self.config.return_value = 'foorbar'