Address review

src/middlewared/middlewared/plugins/failover_/reboot.py

@@ -3,7 +3,7 @@
 # Licensed under the terms of the TrueNAS Enterprise License Agreement
 # See the file LICENSE.IX for complete terms and conditions
 import asyncio
-from dataclasses import dataclass
+from dataclasses import asdict, dataclass
 import errno
 import time
 
@@ -28,7 +28,8 @@ class Config:
         cli_namespace = 'system.failover.reboot'
         namespace = 'failover.reboot'
 
-    remote_reboot_reasons: dict[str, RemoteRebootReason] = {}
+    remote_reboot_reasons_key: str
+    remote_reboot_reasons: dict[str, RemoteRebootReason]
 
     @private
     async def add_remote_reason(self, code: str, reason: str):
@@ -44,12 +45,15 @@ async def add_remote_reason(self, code: str, reason: str):
                 'timeout': 2,
                 'connect_timeout': 2,
             })
-        except Exception:
-            self.logger.warning('Unexpected error querying remote reboot boot id', exc_info=True)
+        except Exception as e:
+            if not (isinstance(e, CallError) and e.errno == CallError.ENOMETHOD):
+                self.logger.warning('Unexpected error querying remote reboot boot id', exc_info=True)
+
             # Remote system is inaccessible, so, when it comes back, another reboot will be required.
             boot_id = None
 
         self.remote_reboot_reasons[code] = RemoteRebootReason(boot_id, reason)
+        await self.persist_remote_reboot_reasons()
 
         await self.send_event()
 
@@ -63,8 +67,10 @@ async def info(self):
                 'timeout': 2,
                 'connect_timeout': 2,
             })
-        except Exception:
-            self.logger.warning('Unexpected error querying remote reboot info', exc_info=True)
+        except Exception as e:
+            if not (isinstance(e, CallError) and e.errno == CallError.ENOMETHOD):
+                self.logger.warning('Unexpected error querying remote reboot info', exc_info=True)
+
             other_node = None
 
         if other_node is not None:
@@ -91,6 +97,8 @@ async def info(self):
         }
 
         if changed:
+            await self.persist_remote_reboot_reasons()
+
             await self.send_event(info)
 
         return info
@@ -152,6 +160,21 @@ async def send_event(self, info=None):
 
         self.middleware.send_event('failover.reboot.info', 'CHANGED', id=None, fields=info)
 
+    @private
+    async def load_remote_reboot_reasons(self):
+        self.remote_reboot_reasons_key = f'remote_reboot_reasons_{await self.middleware.call("failover.node")}'
+        self.remote_reboot_reasons = {
+            k: RemoteRebootReason(**v)
+            for k, v in (await self.middleware.call('keyvalue.get',self.remote_reboot_reasons_key, {})).items()
+        }
+
+    @private
+    async def persist_remote_reboot_reasons(self):
+        await self.middleware.call('keyvalue.set', self.remote_reboot_reasons_key, {
+            k: asdict(v)
+            for k, v in self.remote_reboot_reasons.items()
+        })
+
 
 async def reboot_info(middleware, *args, **kwargs):
     await middleware.call('failover.reboot.send_event')
@@ -162,6 +185,8 @@ def remote_reboot_info(middleware, *args, **kwargs):
 
 
 async def setup(middleware):
+    await middleware.call('failover.reboot.load_remote_reboot_reasons')
+
     middleware.event_register('failover.reboot.info', 'Sent when a system reboot is required.', roles=['FAILOVER_READ'])
 
     middleware.event_subscribe('system.reboot.info', remote_reboot_info)

src/middlewared/middlewared/plugins/security/update.py

@@ -1,12 +1,10 @@
 import middlewared.sqlalchemy as sa
 
 from middlewared.plugins.failover_.disabled_reasons import DisabledReasonsEnum
+from middlewared.plugins.system.reboot import RebootReason
 from middlewared.schema import accepts, Bool, Dict, Int, Patch
 from middlewared.service import ConfigService, ValidationError, job, private
 
-FIPS_REBOOT_CODE = 'FIPS'
-FIPS_REBOOT_REASON = 'FIPS configuration was changed.'
-
 
 class SystemSecurityModel(sa.Model):
     __tablename__ = 'system_security'
@@ -36,12 +34,12 @@ async def configure_fips_on_ha(self, is_ha, job):
         await self.middleware.call('failover.call_remote', 'etc.generate', ['fips'])
 
         remote_reboot_reasons = await self.middleware.call('failover.call_remote', 'system.reboot.list_reasons')
-        if FIPS_REBOOT_CODE in remote_reboot_reasons:
+        if RebootReason.FIPS.name in remote_reboot_reasons:
             # means FIPS is being toggled but other node is already pending a reboot,
             # so it means the user toggled FIPS twice and somehow the other node
             # didn't reboot (even though we do this automatically). This is an edge
             # case and means someone or something is doing things behind our backs
-            await self.middleware.call('failover.call_remote', 'system.reboot.remove_reason', [FIPS_REBOOT_CODE])
+            await self.middleware.call('failover.call_remote', 'system.reboot.remove_reason', [RebootReason.FIPS.name])
         else:
             try:
                 # we automatically reboot (and wait for) the other controller
@@ -50,7 +48,8 @@ async def configure_fips_on_ha(self, is_ha, job):
             except Exception:
                 # something extravagant happened, so we'll just play it safe and say that
                 # another reboot is required
-                await self.middleware.call('failover.reboot.add_remote_reason', FIPS_REBOOT_CODE, FIPS_REBOOT_REASON)
+                await self.middleware.call('failover.reboot.add_remote_reason', RebootReason.FIPS.name,
+                                           RebootReason.FIPS.value)
 
     @private
     async def validate(self, is_ha, ha_disabled_reasons):
@@ -104,7 +103,7 @@ async def do_update(self, job, data):
             # TODO: We likely need to do some SSH magic as well
             #  let's investigate the exact configuration there
             await self.middleware.call('etc.generate', 'fips')
-            await self.middleware.call('system.reboot.toggle_reason', FIPS_REBOOT_CODE, FIPS_REBOOT_REASON)
+            await self.middleware.call('system.reboot.toggle_reason', RebootReason.FIPS.name, RebootReason.FIPS.value)
             await self.configure_fips_on_ha(is_ha, job)
 
         return await self.config()

src/middlewared/middlewared/plugins/system/reboot.py

@@ -1,8 +1,14 @@
+import enum
+
 from middlewared.api import api_method
 from middlewared.api.current import SystemRebootInfoArgs, SystemRebootInfoResult
 from middlewared.service import private, Service
 
 
+class RebootReason(enum.Enum):
+    FIPS = 'FIPS configuration was changed.'
+
+
 class SystemRebootService(Service):
 
     class Config: