From fcf8fee577cce1e01872ce0ac24c28da0e5d833e Mon Sep 17 00:00:00 2001 From: themylogin Date: Wed, 18 Sep 2024 16:39:00 +0200 Subject: [PATCH] Address review --- .../middlewared/plugins/failover_/reboot.py | 37 ++++++++++++++++--- .../middlewared/plugins/security/update.py | 13 +++---- .../middlewared/plugins/system/reboot.py | 6 +++ 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/middlewared/middlewared/plugins/failover_/reboot.py b/src/middlewared/middlewared/plugins/failover_/reboot.py index dd8468ba858c..fa609a491ff5 100644 --- a/src/middlewared/middlewared/plugins/failover_/reboot.py +++ b/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) diff --git a/src/middlewared/middlewared/plugins/security/update.py b/src/middlewared/middlewared/plugins/security/update.py index 6ae99af81717..59d309473696 100644 --- a/src/middlewared/middlewared/plugins/security/update.py +++ b/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() diff --git a/src/middlewared/middlewared/plugins/system/reboot.py b/src/middlewared/middlewared/plugins/system/reboot.py index 5328313a5aa3..7d72f56baf92 100644 --- a/src/middlewared/middlewared/plugins/system/reboot.py +++ b/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: