From b07888d9138674135e9dfdf0cfc3691420d49ba2 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Mon, 20 Apr 2026 14:17:11 +0200 Subject: [PATCH 01/14] feat: the thresholds and unit used for the FreeDiskSpace policy are fully configurable --- dirac.cfg | 68 +++++++++ .../ResourceStatus/advanced_configuration.rst | 40 +++++ .../Systems/ResourceStatus/index.rst | 20 ++- .../Command/FreeDiskSpaceCommand.py | 140 +++++++++++++----- .../Policy/Configurations.py | 64 +++----- .../Policy/FreeDiskSpacePolicy.py | 41 ++--- 6 files changed, 275 insertions(+), 98 deletions(-) diff --git a/dirac.cfg b/dirac.cfg index 0c255e2d89a..bc2632b55ed 100644 --- a/dirac.cfg +++ b/dirac.cfg @@ -1164,6 +1164,74 @@ Operations pilotVORepoBranch = master # Branch to use workDir = /tmp/pilot3Files # Local work directory on the masterCS for synchronisation } + + # RSS section + # See https://dirac.readthedocs.io/en/latest/AdministratorGuide/Systems/ResourceStatus/configuration.html + ResourceStatus + { + Config + { + Cache = 720 # Lifetime (seconds) of the RSSCache (default: 300) + FromAddress = rss@dirac # Email address used as sender for RSS notifications + } + Policies + { + # Command arguments for the built-in FreeDiskSpace policy type. + # Unit and thresholds apply to all SEs monitored by this policy. + FreeDiskSpace + { + Unit = GB # Space unit: TB (default), GB or MB + Banned_threshold = 0.1 # Free space below which the SE is Banned (in the chosen unit) + Degraded_threshold = 5 # Free space below which the SE is Degraded (in the chosen unit) + } + + SEWriteAccessFreeDiskSpace # This is a free name + { + policyType = FreeDiskSpace # This has to match the name of the python module, which would be for this case "FreeDiskSpacePolicy.py" + Unit = TB # Space unit: TB (default), GB or MB + Banned_threshold = 15 # Free space below which the SE is Banned (in the chosen unit) + Degraded_threshold = 50 # Free space below which the SE is Degraded (in the chosen unit) + matchParams # Example: apply FreeDiskSpace to all SE WriteAccess status types + { + element = Resource + elementType = StorageElement + statusType = WriteAccess + } + } + # Example: apply downtime policy to all Sites + SiteDTOngoing + { + policyType = DTOngoing + matchParams + { + element = Site + } + } + } + PolicyActions + { + # Example: send an email when any Resource reaches Banned status + BannedResourceEmail + { + actionType = EmailAction + notificationGroups = RSSAdmins + matchParams + { + element = Resource + status = Banned + } + } + } + Notification + { + RSSAdmins + { + users = admin@dirac # email addresses used for the notifications + } + } + } + + # Services section Services { # See http://dirac.readthedocs.io/en/latest/AdministratorGuide/Resources/Catalog/index.html diff --git a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst index 5a2452abb80..d8ff01af43f 100644 --- a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst +++ b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst @@ -66,6 +66,46 @@ we cannot define the following matchParams: Code templates and examples for creating custom policies: :doc:`../../../DeveloperGuide/Systems/ResourceStatus/index` +Built-in FreeDiskSpace Policy +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``FreeDiskSpace`` policy type monitors Storage Element occupancy. +It compares the free space reported by the SE against two configurable thresholds: + +* If free space is below ``Banned_threshold``, the SE is set to **Banned**. +* If free space is below ``Degraded_threshold`` (but above ``Banned_threshold``), the SE is set to **Degraded**. +* Otherwise the SE is set to **Active**. + +All three parameters — unit, banned threshold, and degraded threshold — are fully configurable +from the Operations CS and fall back to safe defaults: + +:: + + /Operations/Defaults/ResourceStatus + /Policies + /FreeDiskSpace + Unit = TB # unit for the SE occupancy query (TB, GB or MB) + Banned_threshold = 0.1 # in the chosen unit (default) + Degraded_threshold = 5 # in the chosen unit (default) + +.. note:: + + These keys live under ``/Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace``, + not under the ``/matchParams`` sub-section. They tune the **command arguments**, not the + element-matching logic. + + The default values of ``0.1`` and ``5`` are always used as fallback regardless of unit. + Make sure to set meaningful threshold values explicitly in the CS when changing the unit. + +Example: use GB with tighter thresholds:: + + /Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace + { + Unit = GB + Banned_threshold = 100 + Degraded_threshold = 5000 + } + ------------- PolicyActions ------------- diff --git a/docs/source/DeveloperGuide/Systems/ResourceStatus/index.rst b/docs/source/DeveloperGuide/Systems/ResourceStatus/index.rst index 320053def2c..20ad4c6e5c7 100644 --- a/docs/source/DeveloperGuide/Systems/ResourceStatus/index.rst +++ b/docs/source/DeveloperGuide/Systems/ResourceStatus/index.rst @@ -115,7 +115,7 @@ Cache tables for metrics used by policies. * - PolicyResult - Policy evaluation results (Element, Name, PolicyName, Status, Reason) * - SpaceTokenOccupancyCache - - Storage space usage (Token, Free, Guaranteed) + - Storage space usage (Token, Free, Guaranteed) — values stored in MB * - TransferCache - Transfer quality metrics (SourceName, DestinationName, Metric, Value) @@ -215,6 +215,24 @@ Policies inherit from ``PolicyBase`` and implement ``evaluate()``. return {'Status': 'Degraded', 'Reason': f'Low efficiency: {efficiency:.2%}'} return {'Status': 'Banned', 'Reason': f'Very low efficiency: {efficiency:.2%}'} +FreeDiskSpace Policy +-------------------- + +The ``FreeDiskSpacePolicy`` (``Policy/FreeDiskSpacePolicy.py``) evaluates SE occupancy using +configurable thresholds. Thresholds are passed through as command arguments so they propagate +from the CS configuration all the way to the policy evaluation: + +1. ``Configurations.py`` reads ``Unit``, ``Banned_threshold`` and ``Degraded_threshold`` from the + Operations CS via ``Operations().getValue("ResourceStatus/Policies/FreeDiskSpace/Banned_threshold", 0.1)`` + and stores them in the policy ``args`` dict. +2. ``FreeDiskSpaceCommand`` reads these values from ``self.args`` in ``_prepareCommand()`` and + returns them alongside ``Free`` and ``Total`` in both ``doNew()`` and ``doCache()``. +3. ``FreeDiskSpacePolicy._evaluate()`` reads ``Banned_threshold`` and ``Degraded_threshold`` + from the command result dict (with safe defaults) and applies the comparison. + +This design keeps thresholds fully configurable per deployment without code changes. +See :ref:`rss_advanced_configuration` for the available CS keys. + Command Implementation ---------------------- diff --git a/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py b/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py index 525ac720e75..dbc2f40fdef 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py @@ -1,10 +1,17 @@ """FreeDiskSpaceCommand -The Command gets the free space that is left in a Storage Element -Note: there are, still, many references to "space tokens", -for example the SpaceTokenOccupancyCache table. -This is for historical reasons, and shoud be fixed one day. -For the moment, when you see "token" or "space token" here, just read "StorageElement". +Command to retrieve and cache the free/total disk space of a Storage Element. + +The unit and decision thresholds are read from ``self.args`` (populated from the +Operations CS ``/Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace``): + +* ``unit`` — space unit for the occupancy query: ``TB`` (default), ``GB`` or ``MB`` +* ``Banned_threshold`` — free-space value below which the SE is Banned (default: 0.1) +* ``Degraded_threshold`` — free-space value below which the SE is Degraded (default: 5) + +Note: there are still many references to "space tokens" (e.g. +``ResourceManagementClient().selectSpaceTokenOccupancyCache(token=elementName)``). +This is for historical reasons; when you see "token" or "space token" here, read "StorageElement". """ @@ -26,7 +33,11 @@ class FreeDiskSpaceCommand(Command): """ - Uses diskSpace method to get the free space + Command that queries the occupancy of a Storage Element and caches the result. + + Occupancy values are stored in the SpaceTokenOccupancyCache table (in MB) and + recorded in the StorageOccupancy accounting. The unit used for the returned + values is configurable (default: TB). """ def __init__(self, args=None, clients=None): @@ -36,41 +47,58 @@ def __init__(self, args=None, clients=None): def _prepareCommand(self): """ - FreeDiskSpaceCommand requires one argument: - - name : + Extract and validate command arguments from ``self.args``. + + Required key: + + * ``name`` (str) — Storage Element name. + + Optional keys (all read from the FreeDiskSpace policy configuration): + + * ``unit`` (str) — space unit: ``TB`` (default), ``GB`` or ``MB``. + * ``Banned_threshold`` (float) — free space below which the SE is Banned (default: 0.1). + * ``Degraded_threshold`` (float) — free space below which the SE is Degraded (default: 5). + + :returns: S_OK tuple ``(elementName, unit, banned_threshold, degraded_threshold)`` + or S_ERROR if ``name`` is missing. """ if "name" not in self.args: return S_ERROR('"name" not found in self.args') elementName = self.args["name"] - # We keep TB as default as this is what was used (and will still be used) - # in the policy for "space tokens" ("real", "data" SEs) unit = self.args.get("unit", "TB") - return S_OK((elementName, unit)) + banned_threshold = self.args.get("Banned_threshold", 0.1) + degraded_threshold = self.args.get("Degraded_threshold", 5) + + return S_OK((elementName, unit, banned_threshold, degraded_threshold)) def doNew(self, masterParams=None): """ - Gets the parameters to run, either from the master method or from its - own arguments. + Query the SE occupancy directly and cache the result. - Gets the total and the free disk space of a storage element - and inserts the results in the SpaceTokenOccupancyCache table - of ResourceManagementDB database. + Fetches the free and total disk space from the Storage Element, stores the + values in the SpaceTokenOccupancyCache table (in MB) and in the StorageOccupancy + accounting, then returns them to the caller in the configured unit together with + the decision thresholds. - The result is also returned to the caller, not only inserted. - What is inserted in the DB will normally be in MB, - what is returned will be in the specified unit. + :param masterParams: when called from ``doMaster``, a ``(name, unit)`` tuple + that overrides ``self.args``; otherwise ``None``. + + :returns: S_OK dict with keys ``Free``, ``Total``, ``Banned_threshold``, + ``Degraded_threshold`` (all in the configured unit), or S_ERROR. """ if masterParams is not None: elementName, unit = masterParams + banned_threshold = self.args.get("Banned_threshold", 0.1) + degraded_threshold = self.args.get("Degraded_threshold", 5) else: params = self._prepareCommand() if not params["OK"]: return params - elementName, unit = params["Value"] + elementName, unit, banned_threshold, degraded_threshold = params["Value"] se = StorageElement(elementName) occupancyResult = se.getOccupancy(unit=unit) @@ -85,17 +113,31 @@ def doNew(self, masterParams=None): if not result["OK"]: return result - return S_OK({"Free": free, "Total": total}) + return S_OK( + { + "Free": free, + "Total": total, + "Banned_threshold": banned_threshold, + "Degraded_threshold": degraded_threshold, + } + ) def _storeCommand(self, results): """ - Stores the results in the cache (SpaceTokenOccupancyCache), - and adds records to the StorageOccupancy accounting. + Persist occupancy data to the cache table and accounting system. - :param dict results: something like {'ElementName': 'CERN-HIST-EOS', - 'Free': 3264963586.10073, - 'Total': 8000000000.0} - :returns: S_OK/S_ERROR dict + Writes to SpaceTokenOccupancyCache (values in MB) and registers + Free/Total/Used records in the StorageOccupancy accounting type. + + :param dict results: occupancy data, e.g.:: + + { + 'ElementName': 'CERN-HIST-EOS', + 'Free': 3264963586.10073, # MB + 'Total': 8000000000.0, # MB + } + + :returns: S_OK on success, S_ERROR otherwise. """ # Stores in cache @@ -141,14 +183,21 @@ def _storeCommand(self, results): def doCache(self): """ - This is a method that gets the element's details from the spaceTokenOccupancyCache DB table. - It will return a dictionary with th results, converted to "correct" unit. + Retrieve SE occupancy from the SpaceTokenOccupancyCache table. + + Values are stored in MB and converted on the fly to the configured unit + before being returned. The decision thresholds are appended to the result + so that ``FreeDiskSpacePolicy`` can evaluate them without re-reading the CS. + + :returns: S_OK dict with keys ``Free``, ``Total``, ``Banned_threshold``, + ``Degraded_threshold`` (all in the configured unit), or S_ERROR if + no cached record exists or the unit is invalid. """ params = self._prepareCommand() if not params["OK"]: return params - elementName, unit = params["Value"] + elementName, unit, banned_threshold, degraded_threshold = params["Value"] result = self.rmDB.select("SpaceTokenOccupancyCache", {"Token": elementName}) @@ -167,17 +216,28 @@ def doCache(self): if free == -sys.maxsize or total == -sys.maxsize: return S_ERROR("No valid unit specified") - return S_OK({"Free": free, "Total": total}) + return S_OK( + { + "Free": free, + "Total": total, + "Banned_threshold": banned_threshold, + "Degraded_threshold": degraded_threshold, + } + ) def doMaster(self): """ - This method calls the doNew method for each storage element - that exists in the CS. + Refresh occupancy data for all Storage Elements known to the CS. + + Iterates over all SEs returned by DMSHelpers, calls ``doNew`` for each one + (always using MB as the internal storage unit), then purges stale entries + from the cache via ``_cleanCommand``. + + :returns: S_OK on success, S_ERROR if the cache cleanup fails. """ for name in DMSHelpers().getStorageElements(): try: - # keeping TB as default diskSpace = self.doNew((name, "MB")) if not diskSpace["OK"]: self.log.warn("Unable to calculate free/total disk space", f"name: {name}") @@ -191,9 +251,17 @@ def doMaster(self): return self._cleanCommand() def _cleanCommand(self, toDelete=None): - """Clean the spaceTokenOccupancy table from old SEs + """ + Remove stale entries from the SpaceTokenOccupancyCache table. + + An entry is considered stale when its ``LastCheckTime`` is older than 6 hours + and the corresponding SE/endpoint pair no longer exists in the CS. + + :param tuple toDelete: if provided, a single ``(endpoint, storage_element_name)`` + tuple to delete explicitly, e.g. ``('httpg://srm-lhcb.cern.ch:8443/srm/managerv2', 'CERN-RAW')``. + If ``None`` (default), stale entries are detected automatically. - :param tuple toDelete: storage_element_name + :returns: S_OK always (individual deletion failures are logged as warnings). """ if not toDelete: toDelete = [] diff --git a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py index f626163c5e0..417a7ed3c70 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py @@ -1,62 +1,44 @@ -""" Configurations module +"""Configurations module - Configuration to use policies. +Configuration to use policies. - Follows the schema:: +Follows the schema:: - : { - 'description' : , - 'module' : , - 'command' : ( , < command class name > ), - 'args' : { arguments for the command } or None - } + : { + 'description' : , + 'module' : , + 'command' : ( , < command class name > ), + 'args' : { arguments for the command } or None + } """ + +from DIRAC.ConfigurationSystem.Client.Helpers.Operations import Operations + POLICIESMETA = { # DownTime POLICIES "DTOngoing": { "description": "Ongoing and scheduled down-times", "module": "DowntimePolicy", "command": ("DowntimeCommand", "DowntimeCommand"), - "args": {"hours": 0, "onlyCache": True}, - }, - "DTScheduled1": { - "description": "Ongoing and scheduled down-times", - "module": "DowntimePolicy", - "command": ("DowntimeCommand", "DowntimeCommand"), - "args": {"hours": 1, "onlyCache": True}, - }, - "DTScheduled3": { - "description": "Ongoing and scheduled down-times", - "module": "DowntimePolicy", - "command": ("DowntimeCommand", "DowntimeCommand"), - "args": {"hours": 3, "onlyCache": True}, + "args": {"hours": Operations().getValue("ResourceStatus/Policies/DTOngoing/hours", 0), "onlyCache": True}, }, "DTScheduled": { "description": "Scheduled down-times, starting in ", "module": "DowntimePolicy", "command": ("DowntimeCommand", "DowntimeCommand"), - "args": {"hours": 12, "onlyCache": True}, - }, - # Free Disk Space in Terabytes - "FreeDiskSpaceTB": { - "description": "Free disk space, in TB", - "module": "FreeDiskSpacePolicy", - "command": ("FreeDiskSpaceCommand", "FreeDiskSpaceCommand"), - "args": {"unit": "TB", "onlyCache": True}, - }, - # Free Disk Space in Gigabytes - "FreeDiskSpaceGB": { - "description": "Free disk space, in GB", - "module": "FreeDiskSpacePolicy", - "command": ("FreeDiskSpaceCommand", "FreeDiskSpaceCommand"), - "args": {"unit": "GB", "onlyCache": True}, + "args": {"hours": Operations().getValue("ResourceStatus/Policies/DTScheduled/hours", 12), "onlyCache": True}, }, - # Free Disk Space in Megabytes - "FreeDiskSpaceMB": { - "description": "Free disk space, in MB", + # Free Disk Space + "FreeDiskSpace": { + "description": "Free disk space", "module": "FreeDiskSpacePolicy", "command": ("FreeDiskSpaceCommand", "FreeDiskSpaceCommand"), - "args": {"unit": "MB", "onlyCache": True}, + "args": { + "unit": Operations().getValue("ResourceStatus/Policies/FreeDiskSpace/Unit", "TB"), + "Banned_threshold": Operations().getValue("ResourceStatus/Policies/FreeDiskSpace/Banned_threshold", 0.1), + "Degraded_threshold": Operations().getValue("ResourceStatus/Policies/FreeDiskSpace/Degraded_threshold", 5), + "onlyCache": True, + }, }, # GGUS tickets open "GGUSTickets": { diff --git a/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py b/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py index 8e964ffa2c9..6c8c198b329 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py @@ -1,36 +1,37 @@ -""" FreeDiskSpacePolicy +"""FreeDiskSpacePolicy - FreeDiskSpacePolicy.__bases__: - DIRAC.ResourceStatusSystem.PolicySystem.PolicyBase.PolicyBase +Policy to evaluate the free disk space of a Storage Element. +The unit and thresholds (Banned_threshold, Degraded_threshold) are fully +configurable via the Operations CS under +``/Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace``. """ + from DIRAC import S_OK from DIRAC.ResourceStatusSystem.PolicySystem.PolicyBase import PolicyBase class FreeDiskSpacePolicy(PolicyBase): """ - The FreeDiskSpacePolicy class is a policy class satisfied when a SE has a - low occupancy. + Policy that proposes a new status for a Storage Element based on its free disk space. - FreeDiskSpacePolicy, given the space left at the element, proposes a new status. + The free space value and the thresholds (Banned_threshold, Degraded_threshold) are + expressed in the same unit (TB, GB or MB) as configured for the FreeDiskSpace policy + in the Operations CS. Default unit is TB; default thresholds are 0.1 (Banned) and 5 + (Degraded). """ @staticmethod def _evaluate(commandResult): """ - Evaluate policy on SE occupancy: Use FreeDiskSpaceCommand - - :Parameters: - **commandResult** - S_OK / S_ERROR - result of the command. It is expected ( iff S_OK ) a dictionary like - { 'Total' : .., 'Free' : ..} - - :return: - { - 'Status':Error|Active|Bad|Banned, - 'Reason': Some lame statements that have to be updated - } + Evaluate the free disk space policy. + + :param dict commandResult: S_OK / S_ERROR result from FreeDiskSpaceCommand. + On success the value is expected to be a dict with keys: + ``Free``, ``Total``, ``Banned_threshold`` (optional), ``Degraded_threshold`` (optional). + + :returns: S_OK wrapping a dict ``{'Status': str, 'Reason': str}`` where Status is one of + ``Error``, ``Unknown``, ``Banned``, ``Degraded``, ``Active``. """ result = {} @@ -57,10 +58,10 @@ def _evaluate(commandResult): # Units (TB, GB, MB) may change, # depending on the configuration of the command in Configurations.py - if free < 0.1: + if free < commandResult.get("Banned_threshold", 0.1): result["Status"] = "Banned" result["Reason"] = "Too little free space" - elif free < 5: + elif free < commandResult.get("Degraded_threshold", 5): result["Status"] = "Degraded" result["Reason"] = "Little free space" else: From e0accb647bdc2e3b1c22a907b7c6c7d8fb4f0069 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Mon, 20 Apr 2026 14:42:00 +0200 Subject: [PATCH 02/14] feat: the hours for the DowntimePolicy are now configurable --- dirac.cfg | 43 ++++--- .../ResourceStatus/advanced_configuration.rst | 32 +++++ .../Command/DowntimeCommand.py | 121 ++++++++++++++---- .../Policy/Configurations.py | 12 +- .../Policy/DowntimePolicy.py | 37 +++++- 5 files changed, 189 insertions(+), 56 deletions(-) diff --git a/dirac.cfg b/dirac.cfg index bc2632b55ed..93ac638a922 100644 --- a/dirac.cfg +++ b/dirac.cfg @@ -1176,35 +1176,48 @@ Operations } Policies { + # Command arguments for the built-in Downtime policy type. + # hours = 0 means only ongoing downtimes are considered (default). + # Set hours > 0 to also catch downtimes starting within that window. + Downtime + { + hours = 0 # look-ahead window in hours (0 = ongoing only, default) + } # Command arguments for the built-in FreeDiskSpace policy type. # Unit and thresholds apply to all SEs monitored by this policy. FreeDiskSpace { - Unit = GB # Space unit: TB (default), GB or MB + Unit = TB # Space unit: TB (default), GB or MB Banned_threshold = 0.1 # Free space below which the SE is Banned (in the chosen unit) Degraded_threshold = 5 # Free space below which the SE is Degraded (in the chosen unit) } - - SEWriteAccessFreeDiskSpace # This is a free name + # Example: apply Downtime policy to all Sites + SiteDowntime { - policyType = FreeDiskSpace # This has to match the name of the python module, which would be for this case "FreeDiskSpacePolicy.py" - Unit = TB # Space unit: TB (default), GB or MB - Banned_threshold = 15 # Free space below which the SE is Banned (in the chosen unit) - Degraded_threshold = 50 # Free space below which the SE is Degraded (in the chosen unit) - matchParams # Example: apply FreeDiskSpace to all SE WriteAccess status types + policyType = Downtime + matchParams { - element = Resource - elementType = StorageElement - statusType = WriteAccess + element = Site } } - # Example: apply downtime policy to all Sites - SiteDTOngoing + # Example: apply Downtime policy to all Resources + ResourceDowntime { - policyType = DTOngoing + policyType = Downtime matchParams { - element = Site + element = Resource + } + } + # Example: apply FreeDiskSpace policy to all SE WriteAccess status types + SEWriteAccessFreeDiskSpace + { + policyType = FreeDiskSpace + matchParams + { + element = Resource + elementType = StorageElement + statusType = WriteAccess } } } diff --git a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst index d8ff01af43f..2e0c9553a04 100644 --- a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst +++ b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst @@ -66,6 +66,38 @@ we cannot define the following matchParams: Code templates and examples for creating custom policies: :doc:`../../../DeveloperGuide/Systems/ResourceStatus/index` +Built-in Downtime Policy +~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``Downtime`` policy type evaluates GOCDB downtime data for a Site or Resource. +Severity is mapped to RSS status as follows: + +* **OUTAGE** → **Banned** +* **WARNING** → **Degraded** +* No downtime → **Active** + +The look-ahead window is configurable from the Operations CS: + +:: + + /Operations/Defaults/ResourceStatus + /Policies + /Downtime + hours = 0 # hours to look ahead (0 = ongoing only, default) + +.. note:: + + Setting ``hours = 0`` (the default) means only downtimes that are currently ongoing + are considered. Setting a positive value (e.g. ``12``) also catches downtimes scheduled + to start within that window, which is useful for proactive status changes. + +Example: flag elements with downtimes starting within the next 24 hours:: + + /Operations/Defaults/ResourceStatus/Policies/Downtime + { + hours = 24 + } + Built-in FreeDiskSpace Policy ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py b/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py index 0699e235a70..785e1bb6177 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py @@ -1,8 +1,17 @@ -""" DowntimeCommand module will look into GOC DB to find announced downtimes for RSS-managed sites and resources. - If found, downtimes are added to the internal RSS cache using ResourceManagementClient. +"""DowntimeCommand + +Command to fetch and cache GOCDB downtime information for RSS-managed Sites and Resources. +Downtimes found are stored in the DowntimeCache table via ResourceManagementClient. +Stale or deleted GOCDB downtimes are also removed from the cache. + +The look-ahead window is controlled by the ``hours`` argument (read from ``self.args``, +populated from ``/Operations/Defaults/ResourceStatus/Policies/Downtime/hours``): + +* ``hours = 0`` (default) — only ongoing downtimes are considered. +* ``hours > 0`` — downtimes starting within the next ``hours`` hours are also included. - GOCDB downtimes that are modified or deleted are also synced. """ + import re from datetime import timedelta from operator import itemgetter @@ -44,7 +53,10 @@ class DowntimeCommand(Command): """ - Downtime "master" Command or removed DTs. + Command that queries GOCDB for downtime information and caches the results. + + Supports Sites, Storage Elements, FTS servers, and Computing Elements. + DIRAC resource types are mapped to GOCDB service types via ``diracToGOC_conversion``. """ def __init__(self, args=None, clients=None): @@ -55,7 +67,13 @@ def __init__(self, args=None, clients=None): def _storeCommand(self, result): """ - Stores the results of doNew method on the database. + Persist a list of downtime records to the DowntimeCache table. + + :param list result: list of downtime dicts, each containing ``DowntimeID``, + ``Element``, ``Name``, ``StartDate``, ``EndDate``, ``Severity``, + ``Description``, ``Link``, and ``gOCDBServiceType``. + + :returns: S_OK / S_ERROR from the last ``addOrModifyDowntimeCache`` call. """ for dt in result: @@ -74,7 +92,15 @@ def _storeCommand(self, result): def _cleanCommand(self, element, elementNames): """ - Clear Cache from expired DT. + Remove expired or deleted downtime entries from the DowntimeCache. + + A cached entry is removed if its ``EndDate`` is in the past or if its + GOCDB link no longer appears in the list of current GOCDB downtimes. + + :param str element: ``'Site'`` or ``'Resource'``. + :param list elementNames: names of the elements whose cache entries should be checked. + + :returns: S_OK with a list of deletion results, or S_ERROR on DB / GOCDB failure. """ resQuery = [] @@ -108,13 +134,28 @@ def _cleanCommand(self, element, elementNames): def _prepareCommand(self): """ - DowntimeCommand requires four arguments: - - name : - - element : Site / Resource - - elementType: + Extract and validate command arguments from ``self.args``, resolving DIRAC + names to their GOCDB equivalents where necessary. + + Required keys: + + * ``name`` (str) — DIRAC element name. + * ``element`` (str) — ``'Site'`` or ``'Resource'``. + * ``elementType`` (str) — resource type (e.g. ``StorageElement``, ``ComputingElement``, ``FTS3``). + + Optional key: - If the elements are Site(s), we need to get their GOCDB names. They may - not have, so we ignore them if they do not have. + * ``hours`` (int) — look-ahead window in hours (default: ``None`` = ongoing only). + + Name resolution: + + * **Site** — converted to the GOCDB site name via ``getGOCSiteName``. + * **StorageElement** — resolved to one or more SE hosts; GOCDB service type + derived from SE type and access protocol via ``diracToGOC_conversion``. + * **FTS / FTS3** — resolved to the GOCDB FTS name via ``getGOCFTSName``. + * **ComputingElement** — GOCDB service type derived from CE type via ``diracToGOC_conversion``. + + :returns: S_OK tuple ``(element, elementName, hours, gOCDBServiceType)`` or S_ERROR. """ if "name" not in self.args: @@ -209,19 +250,24 @@ def _prepareCommand(self): def doNew(self, masterParams=None): """ - Gets the parameters to run, either from the master method or from its - own arguments. + Fetch current downtime information from GOCDB and store it in the cache. + + Queries GOCDB for ongoing (and optionally upcoming) downtimes for the given + element(s). The GOCDB server is queried twice on ``URLError`` to handle + transient failures. Found downtimes are stored via ``_storeCommand``; the + cache is cleaned of stale entries via ``_cleanCommand``. - For every elementName, unless it is given a list, in which case it contacts - the gocdb client. The server is not very stable, so in case of failure tries - a second time. + :param masterParams: when called from ``doMaster``, a ``(element, elementNames)`` + tuple (e.g. ``('Site', ['CERN', 'IN2P3-CC'])``); the look-ahead window is + taken from ``self.args.get('hours', 0)``. Pass ``None`` to use ``self.args`` + directly (normal per-element policy evaluation path). - If there are downtimes, are recorded and then returned. + :returns: S_OK on success (value is ``None`` if no downtimes were found), or S_ERROR. """ if masterParams is not None: element, elementNames = masterParams - hours = 120 + hours = self.args.get("hours", 0) elementName = None gOCDBServiceType = None @@ -294,8 +340,23 @@ def doNew(self, masterParams=None): def doCache(self): """ - Method that reads the cache table and tries to read from it. It will - return a list with one dictionary describing the DT if there are results. + Retrieve the most relevant downtime for this element from the DowntimeCache. + + When ``hours`` is set, the target date is shifted into the future and the + earliest matching downtime is returned (useful for advance warning of scheduled + outages). When ``hours`` is ``None``, ongoing downtimes are evaluated and the + highest-severity, longest-lasting one is returned. + + Priority when multiple downtimes overlap: + + * OUTAGE takes precedence over WARNING. + * Among equal severity: the one ending latest wins (``hours=None`` path) or + the one starting earliest wins (``hours>0`` path). + + :returns: S_OK with a downtime dict (keys: ``DowntimeID``, ``Element``, ``Name``, + ``StartDate``, ``EndDate``, ``Severity``, ``Description``, ``Link``, + ``gOCDBServiceType``) if a relevant downtime exists, S_OK with ``None`` + if no downtime applies, or S_ERROR on DB failure. """ params = self._prepareCommand() @@ -363,10 +424,20 @@ def doCache(self): return S_OK(dtOverlapping[-1]) def doMaster(self): - """Master method, which looks little bit spaghetti code, sorry ! - - It gets all sites and transforms them into gocSites. - - It gets all the storage elements and transforms them into their hosts - - It gets the the CEs (FTS and file catalogs will come). + """ + Refresh downtime data for all known Sites and Resources from GOCDB. + + Collects: + + * All GOCDB site names (from ``getGOCSites``). + * All SE hosts (from ``getStorageElementsHosts``). + * All FTS3 server hosts (from ``getFTS3Servers``). + * All Computing Element names (from ``getCESiteMapping``). + + Calls ``doNew`` separately for Sites and Resources. Failures are recorded in + ``self.metrics['failed']`` but do not abort the run. + + :returns: S_OK with ``self.metrics`` dict (containing a ``'failed'`` list). """ gocSites = getGOCSites() diff --git a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py index 417a7ed3c70..9274f687d7b 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py @@ -16,17 +16,11 @@ from DIRAC.ConfigurationSystem.Client.Helpers.Operations import Operations POLICIESMETA = { # DownTime POLICIES - "DTOngoing": { - "description": "Ongoing and scheduled down-times", + "Downtime": { + "description": "Ongoing or scheduled down-times within from now (0 = ongoing only)", "module": "DowntimePolicy", "command": ("DowntimeCommand", "DowntimeCommand"), - "args": {"hours": Operations().getValue("ResourceStatus/Policies/DTOngoing/hours", 0), "onlyCache": True}, - }, - "DTScheduled": { - "description": "Scheduled down-times, starting in ", - "module": "DowntimePolicy", - "command": ("DowntimeCommand", "DowntimeCommand"), - "args": {"hours": Operations().getValue("ResourceStatus/Policies/DTScheduled/hours", 12), "onlyCache": True}, + "args": {"hours": Operations().getValue("ResourceStatus/Policies/Downtime/hours", 0), "onlyCache": True}, }, # Free Disk Space "FreeDiskSpace": { diff --git a/src/DIRAC/ResourceStatusSystem/Policy/DowntimePolicy.py b/src/DIRAC/ResourceStatusSystem/Policy/DowntimePolicy.py index 4deb0a346fc..0b009db6fd6 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/DowntimePolicy.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/DowntimePolicy.py @@ -1,18 +1,41 @@ -""" DowntimePolicy module +"""DowntimePolicy + +Policy to evaluate the downtime status of a Site or Resource as reported by GOCDB. +The look-ahead window (``hours``) is fully configurable via the Operations CS under +``/Operations/Defaults/ResourceStatus/Policies/Downtime``. + """ + from DIRAC import S_OK from DIRAC.ResourceStatusSystem.PolicySystem.PolicyBase import PolicyBase class DowntimePolicy(PolicyBase): - """The DowntimePolicy checks for downtimes, scheduled or ongoing, depending on the command parameters.""" + """ + Policy that proposes a new status for a Site or Resource based on GOCDB downtime data. + + Whether the policy considers only ongoing downtimes or also scheduled ones within + a future window is controlled by the ``hours`` command argument (default: 0 = ongoing only). + """ @staticmethod def _evaluate(commandResult): - """It returns Active status if there is no downtime announced. - Banned if the element is in OUTAGE. - Degraded if it is on WARNING status. - Otherwise, it returns error. + """ + Evaluate the downtime policy against the result of DowntimeCommand. + + Severity mapping: + + * No downtime (``None``) → **Active** + * ``OUTAGE`` → **Banned** + * ``WARNING`` → **Degraded** + * any other severity → **Error** + + :param dict commandResult: S_OK / S_ERROR result from DowntimeCommand. + On success the value is either ``None`` (no downtime) or a dict with at least + ``Severity``, ``DowntimeID``, and ``Description`` keys. + + :returns: S_OK wrapping a dict ``{'Status': str, 'Reason': str}`` where Status is one of + ``Error``, ``Active``, ``Banned``, ``Degraded``. """ result = {"Status": None, "Reason": None} @@ -36,7 +59,7 @@ def _evaluate(commandResult): result["Status"] = "Degraded" else: - _reason = f"DT_Policy: GOCDB returned an unknown value for DT: \"{status['DowntimeID']}\"" + _reason = f'DT_Policy: GOCDB returned an unknown value for DT: "{status["DowntimeID"]}"' result["Status"] = "Error" result["Reason"] = _reason return S_OK(result) From 7fce019c6ec702892009b51213874d7bc561b996 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Mon, 20 Apr 2026 14:53:39 +0200 Subject: [PATCH 03/14] test: expand RSS advanced config with more policy coverage - AlwaysBannedForSite2: tests domain-based matching specificity - AlwaysDegradedForResource: tests AlwaysDegraded policy type - SpecificFreeDiskSpace: tests per-policy arg overrides (Unit=GB, custom thresholds) and specificity dedup against generic FreeDiskSpace - PropagationForSite: tests Propagation policy type - PolicyActions sections: LogStatusAction and LogPolicyResultAction - Integration test cases for LogSE WriteAccess (Degraded via specific FreeDiskSpace), LogSE ReadAccess (Active via AlwaysActive), and Site with None status --- .../ResourceStatusSystem/Test_FullChain.py | 65 ++++++++ tests/Jenkins/dirac-cfg-update-server.py | 139 +++++++++++++++++- 2 files changed, 201 insertions(+), 3 deletions(-) diff --git a/tests/Integration/ResourceStatusSystem/Test_FullChain.py b/tests/Integration/ResourceStatusSystem/Test_FullChain.py index ee75a165974..7a057880c83 100644 --- a/tests/Integration/ResourceStatusSystem/Test_FullChain.py +++ b/tests/Integration/ResourceStatusSystem/Test_FullChain.py @@ -14,6 +14,14 @@ } policyType = AlwaysActive } + AlwaysDegradedForResource + { + matchParams + { + element = Resource + } + policyType = AlwaysDegraded + } AlwaysBannedForSE1SE2 { matchParams @@ -39,6 +47,27 @@ } policyType = AlwaysBanned } + SpecificFreeDiskSpace + { + policyType = FreeDiskSpace + Unit = GB + Banned_threshold = 10 + Degraded_threshold = 50 + matchParams + { + element = Resource + name = LogSE + statusType = WriteAccess + } + } + PropagationForSite + { + matchParams + { + element = Site + } + policyType = Propagation + } } PolicyActions { @@ -131,6 +160,42 @@ def test_takeDecision_noDecisionParams(): }, "Banned", ), + ( + { + "element": "Resource", + "name": "LogSE", + "elementType": "StorageElement", + "statusType": "WriteAccess", + "status": "Active", + "reason": None, + "tokenOwner": None, + }, + "Degraded", + ), + ( + { + "element": "Resource", + "name": "LogSE", + "elementType": "StorageElement", + "statusType": "ReadAccess", + "status": "Active", + "reason": None, + "tokenOwner": None, + }, + "Active", + ), + ( + { + "element": "Site", + "name": "Site3", + "elementType": None, + "statusType": None, + "status": None, + "reason": None, + "tokenOwner": None, + }, + "Banned", + ), ], ) def test_takeDecision_decisionParams(decisionParams, status): diff --git a/tests/Jenkins/dirac-cfg-update-server.py b/tests/Jenkins/dirac-cfg-update-server.py index bd75aec5b5d..39e0ef7edce 100644 --- a/tests/Jenkins/dirac-cfg-update-server.py +++ b/tests/Jenkins/dirac-cfg-update-server.py @@ -302,35 +302,91 @@ # Config # { # Cache = 600 -# State = Active # FromAddress = fstagni@cern.ch # notificationGroups = ShiftersGroup # } # Policies # { +# Downtime +# { +# hours = 0 +# } +# FreeDiskSpace +# { +# Unit = TB +# Banned_threshold = 0.1 +# Degraded_threshold = 5 +# } # AlwaysActiveForResource # { +# policyType = AlwaysActive # matchParams # { # element = Resource # } -# policyType = AlwaysActive # } # AlwaysBannedForSE1SE2 # { +# policyType = AlwaysBanned # matchParams # { # name = SE1,SE2 # } -# policyType = AlwaysBanned # } # AlwaysBannedForSite # { +# policyType = AlwaysBanned # matchParams # { # element = Site # } +# } +# AlwaysBannedForSite2 +# { # policyType = AlwaysBanned +# matchParams +# { +# element = Site +# domain = test +# } +# } +# AlwaysDegradedForResource +# { +# policyType = AlwaysDegraded +# matchParams +# { +# element = Resource +# } +# } +# SpecificFreeDiskSpace +# { +# policyType = FreeDiskSpace +# Unit = GB +# Banned_threshold = 10 +# Degraded_threshold = 50 +# matchParams +# { +# element = Resource +# name = LogSE +# statusType = WriteAccess +# } +# } +# PropagationForSite +# { +# policyType = Propagation +# matchParams +# { +# element = Site +# } +# } +# } +# PolicyActions +# { +# LogStatusAction +# { +# } +# LogPolicyResultAction +# { # } # } # } @@ -359,6 +415,21 @@ if not res["OK"]: print(res["Message"]) sys.exit(1) + +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/Downtime") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/Downtime/hours", "0") + +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace/Unit", "TB") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace/Banned_threshold", "0.1") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace/Degraded_threshold", "5") + res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/AlwaysActiveForResource") if not res["OK"]: print(res["Message"]) @@ -389,6 +460,68 @@ csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysBannedForSite/policyType", "AlwaysBanned") csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysBannedForSite/matchParams/element", "Site") +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/AlwaysBannedForSite2") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/AlwaysBannedForSite2/matchParams") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysBannedForSite2/policyType", "AlwaysBanned") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysBannedForSite2/matchParams/element", "Site") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysBannedForSite2/matchParams/domain", "test") + +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/AlwaysDegradedForResource") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysDegradedForResource/policyType", "AlwaysDegraded") +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/AlwaysDegradedForResource/matchParams") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysDegradedForResource/matchParams/element", "Resource") + +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace/policyType", "FreeDiskSpace") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace/Unit", "GB") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace/Banned_threshold", "10") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace/Degraded_threshold", "50") +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace/matchParams") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace/matchParams/element", "Resource") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace/matchParams/name", "LogSE") +csAPI.setOption( + "Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace/matchParams/statusType", "WriteAccess" +) + +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/PropagationForSite") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/PropagationForSite/policyType", "Propagation") +res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/PropagationForSite/matchParams") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/PropagationForSite/matchParams/element", "Site") + +res = csAPI.createSection("Operations/Defaults/ResourceStatus/PolicyActions") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +res = csAPI.createSection("Operations/Defaults/ResourceStatus/PolicyActions/LogStatusAction") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) +res = csAPI.createSection("Operations/Defaults/ResourceStatus/PolicyActions/LogPolicyResultAction") +if not res["OK"]: + print(res["Message"]) + sys.exit(1) + # Now setting the catalog list in Operations/Defaults/Services/Catalogs/CatalogList # From 3349d24049008358fbfd580d949e84dc83482d88 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Mon, 20 Apr 2026 16:27:50 +0200 Subject: [PATCH 04/14] feat: InfoGetter enforces policyType, supports per-policy arg overrides and generic specificity deduplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Configurations.py: POLICIESMETA now holds plain code-level defaults only; Operations() calls removed (CS is read at runtime by InfoGetter instead) - InfoGetter.getPoliciesThatApply: CS entries without policyType are skipped (they are command-args defaults sections, not policy definitions); non-reserved keys in a CS policy entry are collected as arg overrides, merged into the policy args with case-insensitive key normalisation and type casting - InfoGetter.postProcessingPolicyList: replaces the old FreeDiskSpaceMB/GB/TB hack with a generic rule — when multiple policies of the same policyType match, keep the most specific one (name-match presence > number of matchParams keys) - dirac.cfg: add SpecificFreeDiskSpace example (Unit=GB, Banned_threshold=15, Degraded_threshold falls back to default); fix missing closing brace in Policies - docs: note that command-args sections have no policyType and are not policy definitions - test: new Test_InfoGetter.py with 15 unit tests covering all the above --- dirac.cfg | 15 + .../ResourceStatus/advanced_configuration.rst | 6 +- .../Policy/Configurations.py | 17 +- .../Utilities/InfoGetter.py | 99 +++--- .../Utilities/test/Test_InfoGetter.py | 287 ++++++++++++++++++ 5 files changed, 383 insertions(+), 41 deletions(-) create mode 100644 src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py diff --git a/dirac.cfg b/dirac.cfg index 93ac638a922..adcacf2e5d9 100644 --- a/dirac.cfg +++ b/dirac.cfg @@ -1179,12 +1179,15 @@ Operations # Command arguments for the built-in Downtime policy type. # hours = 0 means only ongoing downtimes are considered (default). # Set hours > 0 to also catch downtimes starting within that window. + # Note: this section has no policyType key and is therefore NOT treated + # as a policy definition — it only sets command argument defaults. Downtime { hours = 0 # look-ahead window in hours (0 = ongoing only, default) } # Command arguments for the built-in FreeDiskSpace policy type. # Unit and thresholds apply to all SEs monitored by this policy. + # Note: same as above — no policyType key, so not a policy definition. FreeDiskSpace { Unit = TB # Space unit: TB (default), GB or MB @@ -1220,6 +1223,18 @@ Operations statusType = WriteAccess } } + # Example: apply FreeDiskSpace to SE1 with specific args (Unit and Banned_threshold); + # Degraded_threshold falls back to the default defined in the FreeDiskSpace section above. + SpecificFreeDiskSpace + { + policyType = FreeDiskSpace + Unit = GB + Banned_threshold = 15 + matchParams + { + name = SE1 + } + } } PolicyActions { diff --git a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst index 2e0c9553a04..f01f644803d 100644 --- a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst +++ b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst @@ -91,6 +91,9 @@ The look-ahead window is configurable from the Operations CS: are considered. Setting a positive value (e.g. ``12``) also catches downtimes scheduled to start within that window, which is useful for proactive status changes. + This section has no ``policyType`` key and is therefore treated purely as + command-argument defaults, not as a policy definition. + Example: flag elements with downtimes starting within the next 24 hours:: /Operations/Defaults/ResourceStatus/Policies/Downtime @@ -124,7 +127,8 @@ from the Operations CS and fall back to safe defaults: These keys live under ``/Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace``, not under the ``/matchParams`` sub-section. They tune the **command arguments**, not the - element-matching logic. + element-matching logic. This section has no ``policyType`` key and is therefore not treated + as a policy definition by the policy engine. The default values of ``0.1`` and ``5`` are always used as fallback regardless of unit. Make sure to set meaningful threshold values explicitly in the CS when changing the unit. diff --git a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py index 9274f687d7b..26a56c0ef2e 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py @@ -11,16 +11,21 @@ 'args' : { arguments for the command } or None } -""" +The values in ``args`` are code-level defaults. They can be overridden per-policy +via the CS entry (e.g. ``Unit = GB`` directly under the policy name in +``/Operations/Defaults/ResourceStatus/Policies/``). +Deployment-wide defaults can also be set in a command-args section named after +the policy type (e.g. ``/Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace``); +these are picked up by InfoGetter before code-level defaults are applied. -from DIRAC.ConfigurationSystem.Client.Helpers.Operations import Operations +""" POLICIESMETA = { # DownTime POLICIES "Downtime": { "description": "Ongoing or scheduled down-times within from now (0 = ongoing only)", "module": "DowntimePolicy", "command": ("DowntimeCommand", "DowntimeCommand"), - "args": {"hours": Operations().getValue("ResourceStatus/Policies/Downtime/hours", 0), "onlyCache": True}, + "args": {"hours": 0, "onlyCache": True}, }, # Free Disk Space "FreeDiskSpace": { @@ -28,9 +33,9 @@ "module": "FreeDiskSpacePolicy", "command": ("FreeDiskSpaceCommand", "FreeDiskSpaceCommand"), "args": { - "unit": Operations().getValue("ResourceStatus/Policies/FreeDiskSpace/Unit", "TB"), - "Banned_threshold": Operations().getValue("ResourceStatus/Policies/FreeDiskSpace/Banned_threshold", 0.1), - "Degraded_threshold": Operations().getValue("ResourceStatus/Policies/FreeDiskSpace/Degraded_threshold", 5), + "unit": "TB", + "Banned_threshold": 0.1, + "Degraded_threshold": 5, "onlyCache": True, }, }, diff --git a/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py b/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py index dda3a8764c2..8a672402757 100644 --- a/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py +++ b/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py @@ -45,21 +45,24 @@ def getPoliciesThatApply(decisionParams): # Get policies that match the given decisionParameters for policyName, policySetup in policiesConfig.items(): - # The parameter policyType replaces policyName, so if it is not present, - # we pick policyName + # The parameter policyType is mandatory. If not present, skip this entry — + # it is a command-args defaults section, not a policy definition. try: policyType = policySetup["policyType"][0] except KeyError: - policyType = policyName - # continue + continue # The section matchParams is not mandatory, so we set {} as default. policyMatchParams = policySetup.get("matchParams", {}) gLogger.debug(f"matchParams of {policyName}: {str(policyMatchParams)}") - # FIXME: make sure the values in the policyConfigParams dictionary are typed !! - policyConfigParams = {} - # policyConfigParams = policySetup.get( 'configParams', {} ) + # Any key in the CS policy entry that is not a reserved keyword is treated as + # a command-argument override. These override the defaults from POLICIESMETA. + _reservedKeys = {"policyType", "matchParams", "configParams", "doNotCombineResult", "active"} + policyConfigParams = { + k: v[0] if isinstance(v, list) else v for k, v in policySetup.items() if k not in _reservedKeys + } + policyMatch = Utils.configMatch(decisionParams, policyMatchParams) gLogger.debug(f"PolicyMatch for decisionParams {decisionParams}: {str(policyMatch)}") @@ -67,7 +70,7 @@ def getPoliciesThatApply(decisionParams): # is not straightforward (e.g. when the policy specify a 'domain', while # the decisionParams has only the name of the element) if policyMatch and _filterPolicies(decisionParams, policyMatchParams): - policiesThatApply.append((policyName, policyType, policyConfigParams)) + policiesThatApply.append((policyName, policyType, policyConfigParams, policyMatchParams)) gLogger.debug(f"policies that apply (before post-processing): {str(policiesThatApply)}") policiesThatApply = postProcessingPolicyList(policiesThatApply) @@ -76,7 +79,7 @@ def getPoliciesThatApply(decisionParams): objectLoader = ObjectLoader() policiesToBeLoaded = [] # Gets policies parameters from code. - for policyName, policyType, _policyConfigParams in policiesThatApply: + for policyName, policyType, _policyConfigParams, _policyMatchParams in policiesThatApply: try: result = objectLoader.loadModule("DIRAC.ResourceStatusSystem.Policy.Configurations") if not result["OK"]: @@ -92,8 +95,22 @@ def getPoliciesThatApply(decisionParams): policyDict = {"name": policyName, "type": policyType, "args": {}} # args is one of the parameters we are going to use on the policies. We copy - # the defaults and then we update if with whatever comes from the CS. + # the defaults from POLICIESMETA and then override with whatever comes from the CS. policyDict.update(policyMeta) + if _policyConfigParams and policyDict.get("args") is not None: + # Build a case-insensitive lookup of the existing arg keys so that CS keys + # like "Unit" correctly override POLICIESMETA keys like "unit". + argsKeyMap = {k.lower(): k for k in policyDict["args"]} + for csKey, csVal in _policyConfigParams.items(): + targetKey = argsKeyMap.get(csKey.lower(), csKey) + # CS values are always strings; cast to the type of the existing default. + existingVal = policyDict["args"].get(targetKey) + if existingVal is not None: + try: + csVal = type(existingVal)(csVal) + except (ValueError, TypeError): + pass + policyDict["args"][targetKey] = csVal policiesToBeLoaded.append(policyDict) @@ -262,28 +279,42 @@ def _filterPolicies(decisionParams, policyMatchParams): def postProcessingPolicyList(policiesThatApply): - """Put here any "hacky" post-processing""" - - # FIXME: the following 2 "if" are a "hack" for dealing with the following case: - # an SE happens to be subject to, e.g., both the 'FreeDiskSpaceMB' and the 'FreeDiskSpaceGB' policies - # (currently, there is no way to avoid that this happens, see e.g. LogSE) - # When this is the case, supposing that an SE has 50 MB free, the policies evaluation will be the following: - # - 'FreeDiskSpaceMB' will evaluate 'Active' - # - 'FreeDiskSpaceGB' will evaluate 'Banned' - # so the SE will end up being banned, but we want only the 'FreeDiskSpaceMB' to be considered. - if ("FreeDiskSpaceMB", "FreeDiskSpaceMB", {}) in policiesThatApply: - try: - policiesThatApply.remove(("FreeDiskSpaceGB", "FreeDiskSpaceGB", {})) - except ValueError: - pass - try: - policiesThatApply.remove(("FreeDiskSpaceTB", "FreeDiskSpaceTB", {})) - except ValueError: - pass - if ("FreeDiskSpaceGB", "FreeDiskSpaceGB", {}) in policiesThatApply: - try: - policiesThatApply.remove(("FreeDiskSpaceTB", "FreeDiskSpaceTB", {})) - except ValueError: - pass + """Remove lower-priority duplicates when multiple policies of the same type apply. + + When two or more policies share the same ``policyType`` and both match the current + element, we keep only the most specific one. Specificity is determined by the number + of ``matchParams`` keys: more keys = more specific. If one of the duplicates matched + by ``name`` it is always considered more specific than one that did not. + + This replaces the old per-type hacks (``FreeDiskSpaceMB`` > ``FreeDiskSpaceGB`` > + ``FreeDiskSpaceTB``) with a generic rule that works for any policy type. + """ + from collections import defaultdict + + # Group policies by policyType + byType = defaultdict(list) + for entry in policiesThatApply: + policyName, policyType, policyConfigParams, policyMatchParams = entry + byType[policyType].append(entry) + + result = [] + for policyType, entries in byType.items(): + if len(entries) == 1: + result.extend(entries) + continue - return policiesThatApply + # Multiple policies of the same type matched — keep only the most specific. + # Specificity = number of matchParams keys; ties broken by name-match presence. + def specificity(entry): + matchParams = entry[3] # policyMatchParams + nameMatch = 1 if "name" in matchParams else 0 + return (nameMatch, len(matchParams)) + + most_specific = max(entries, key=specificity) + result.append(most_specific) + gLogger.debug( + f"postProcessing: multiple {policyType!r} policies matched; " + f"keeping {most_specific[0]!r}, dropping {[e[0] for e in entries if e is not most_specific]}" + ) + + return result diff --git a/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py b/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py new file mode 100644 index 00000000000..bdfb573d708 --- /dev/null +++ b/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py @@ -0,0 +1,287 @@ +"""Tests for InfoGetter module + +Tests cover: +- ``getPoliciesThatApply``: CS matching, policyType enforcement, arg defaults, + per-policy arg overrides (key normalisation + type casting), and + command-args sections (no policyType) being skipped. +- ``postProcessingPolicyList``: single policy, no duplicates, specificity + disambiguation when multiple policies of the same type apply. +""" + +from unittest.mock import patch + +import pytest + +from DIRAC import S_ERROR, S_OK +from DIRAC.ResourceStatusSystem.Utilities.InfoGetter import getPoliciesThatApply, postProcessingPolicyList + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +# Minimal CS-style policies tree reused across tests. +# Values are lists (as returned by getCSTree). +_BASE_POLICIES = { + # Command-args defaults section — no policyType, must be skipped. + "FreeDiskSpace": { + "Unit": ["TB"], + "Banned_threshold": ["0.1"], + "Degraded_threshold": ["5"], + }, + # Matches all Resources of type StorageElement with WriteAccess. + "SEWriteAccessFreeDiskSpace": { + "policyType": ["FreeDiskSpace"], + "matchParams": { + "element": ["Resource"], + "elementType": ["StorageElement"], + "statusType": ["WriteAccess"], + }, + }, + # Matches only SE1 — overrides Unit and Banned_threshold. + "SpecificFreeDiskSpace": { + "policyType": ["FreeDiskSpace"], + "Unit": ["GB"], + "Banned_threshold": ["15"], + "matchParams": {"name": ["SE1"]}, + }, + # Matches all Sites. + "AlwaysBannedForSite": { + "policyType": ["AlwaysBanned"], + "matchParams": {"element": ["Site"]}, + }, +} + +_GET_POLICIES = "DIRAC.ResourceStatusSystem.Utilities.RssConfiguration.getPolicies" + + +def _se1_writeaccess(): + """decisionParams for SE1 WriteAccess.""" + return { + "element": "Resource", + "name": "SE1", + "elementType": "StorageElement", + "statusType": "WriteAccess", + "status": "Active", + "reason": None, + "tokenOwner": None, + "active": "Active", + } + + +def _se2_writeaccess(): + """decisionParams for SE2 WriteAccess (not SE1 — no SpecificFreeDiskSpace).""" + return { + "element": "Resource", + "name": "SE2", + "elementType": "StorageElement", + "statusType": "WriteAccess", + "status": "Active", + "reason": None, + "tokenOwner": None, + "active": "Active", + } + + +def _site1(): + """decisionParams for a generic Site.""" + return { + "element": "Site", + "name": "Site1", + "elementType": None, + "statusType": "ReadAccess", + "status": "Active", + "reason": None, + "tokenOwner": None, + "active": "Active", + } + + +# --------------------------------------------------------------------------- +# getPoliciesThatApply +# --------------------------------------------------------------------------- + + +@patch(_GET_POLICIES, return_value=S_OK({})) +def test_no_policies_in_cs(_mock): + """Empty CS → empty result.""" + result = getPoliciesThatApply(_se1_writeaccess()) + assert result["OK"] + assert result["Value"] == [] + + +@patch(_GET_POLICIES, return_value=S_ERROR("CS unavailable")) +def test_cs_error_is_propagated(_mock): + """CS failure → S_ERROR propagated.""" + result = getPoliciesThatApply(_se1_writeaccess()) + assert not result["OK"] + + +@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) +def test_command_args_section_is_skipped(_mock): + """A CS entry without policyType (e.g. FreeDiskSpace defaults) is not a policy.""" + result = getPoliciesThatApply(_se1_writeaccess()) + assert result["OK"] + names = [p["name"] for p in result["Value"]] + assert "FreeDiskSpace" not in names + + +@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) +def test_unmatched_policy_not_included(_mock): + """AlwaysBannedForSite must not apply to a Resource element.""" + result = getPoliciesThatApply(_se1_writeaccess()) + assert result["OK"] + names = [p["name"] for p in result["Value"]] + assert "AlwaysBannedForSite" not in names + + +@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) +def test_site_policy_applies_to_site(_mock): + """AlwaysBannedForSite applies to a Site element.""" + result = getPoliciesThatApply(_site1()) + assert result["OK"] + names = [p["name"] for p in result["Value"]] + assert "AlwaysBannedForSite" in names + + +@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) +def test_unknown_policytype_in_policiesmeta_is_skipped(_mock): + """A CS policy whose policyType has no entry in POLICIESMETA is silently skipped.""" + policies = { + "GhostPolicy": { + "policyType": ["NonExistentPolicyType"], + "matchParams": {"element": ["Resource"]}, + } + } + with patch(_GET_POLICIES, return_value=S_OK(policies)): + result = getPoliciesThatApply(_se1_writeaccess()) + assert result["OK"] + assert result["Value"] == [] + + +# --------------------------------------------------------------------------- +# Arg defaults and per-policy overrides +# --------------------------------------------------------------------------- + + +@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) +def test_se2_gets_default_args(_mock): + """SE2 matches only SEWriteAccessFreeDiskSpace → gets POLICIESMETA default args.""" + result = getPoliciesThatApply(_se2_writeaccess()) + assert result["OK"] + assert len(result["Value"]) == 1 + policy = result["Value"][0] + assert policy["name"] == "SEWriteAccessFreeDiskSpace" + assert policy["args"]["unit"] == "TB" + assert policy["args"]["Banned_threshold"] == 0.1 + assert policy["args"]["Degraded_threshold"] == 5 + + +@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) +def test_se1_gets_specific_policy_with_overridden_args(_mock): + """SE1 WriteAccess: SpecificFreeDiskSpace wins; Unit and Banned_threshold overridden.""" + result = getPoliciesThatApply(_se1_writeaccess()) + assert result["OK"] + assert len(result["Value"]) == 1 + policy = result["Value"][0] + assert policy["name"] == "SpecificFreeDiskSpace" + assert policy["args"]["unit"] == "GB" # overridden, key-normalised from "Unit" + assert policy["args"]["Banned_threshold"] == 15.0 # overridden, cast from str "15" to float + assert policy["args"]["Degraded_threshold"] == 5 # NOT overridden → falls back to default + + +@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) +def test_arg_override_key_normalisation(_mock): + """CS key 'Unit' (capital) must override POLICIESMETA key 'unit' (lowercase).""" + result = getPoliciesThatApply(_se1_writeaccess()) + assert result["OK"] + args = result["Value"][0]["args"] + # 'Unit' from CS must land in 'unit', not create a separate 'Unit' key + assert "Unit" not in args + assert args["unit"] == "GB" + + +@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) +def test_arg_override_type_casting(_mock): + """CS values are strings; they must be cast to the type of the POLICIESMETA default.""" + result = getPoliciesThatApply(_se1_writeaccess()) + assert result["OK"] + args = result["Value"][0]["args"] + assert isinstance(args["Banned_threshold"], float) + assert isinstance(args["Degraded_threshold"], int) + + +# --------------------------------------------------------------------------- +# postProcessingPolicyList +# --------------------------------------------------------------------------- + + +# Helper to build the 4-tuples expected by postProcessingPolicyList. +def _entry(name, policyType, configParams=None, matchParams=None): + return (name, policyType, configParams or {}, matchParams or {}) + + +def test_postprocessing_single_policy_unchanged(): + """A single policy is returned as-is.""" + entries = [_entry("A", "FreeDiskSpace", matchParams={"element": ["Resource"]})] + assert postProcessingPolicyList(entries) == entries + + +def test_postprocessing_different_types_both_kept(): + """Policies of different types are all kept.""" + entries = [ + _entry("A", "FreeDiskSpace", matchParams={"element": ["Resource"]}), + _entry("B", "Downtime", matchParams={"element": ["Resource"]}), + ] + result = postProcessingPolicyList(entries) + assert len(result) == 2 + + +def test_postprocessing_name_match_beats_broader_match(): + """When two FreeDiskSpace policies match, the one with 'name' in matchParams wins.""" + general = _entry( + "SEWriteAccessFreeDiskSpace", + "FreeDiskSpace", + matchParams={"element": ["Resource"], "elementType": ["StorageElement"], "statusType": ["WriteAccess"]}, + ) + specific = _entry( + "SpecificFreeDiskSpace", + "FreeDiskSpace", + configParams={"unit": "GB", "Banned_threshold": 15.0}, + matchParams={"name": ["SE1"]}, + ) + result = postProcessingPolicyList([general, specific]) + assert len(result) == 1 + assert result[0][0] == "SpecificFreeDiskSpace" + + +def test_postprocessing_more_matchparams_beats_fewer(): + """Without a name-match, the policy with more matchParams keys wins.""" + narrow = _entry( + "NarrowPolicy", + "FreeDiskSpace", + matchParams={"element": ["Resource"], "elementType": ["StorageElement"], "statusType": ["WriteAccess"]}, + ) + broad = _entry( + "BroadPolicy", + "FreeDiskSpace", + matchParams={"element": ["Resource"]}, + ) + result = postProcessingPolicyList([broad, narrow]) + assert len(result) == 1 + assert result[0][0] == "NarrowPolicy" + + +def test_postprocessing_order_independent(): + """Winner is the same regardless of the order policies appear in the input list.""" + general = _entry( + "SEWriteAccessFreeDiskSpace", + "FreeDiskSpace", + matchParams={"element": ["Resource"], "elementType": ["StorageElement"], "statusType": ["WriteAccess"]}, + ) + specific = _entry( + "SpecificFreeDiskSpace", + "FreeDiskSpace", + matchParams={"name": ["SE1"]}, + ) + assert postProcessingPolicyList([general, specific])[0][0] == "SpecificFreeDiskSpace" + assert postProcessingPolicyList([specific, general])[0][0] == "SpecificFreeDiskSpace" From be3b987f8eb5f284b93333629d008e61f6b00078 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Mon, 20 Apr 2026 18:10:20 +0200 Subject: [PATCH 05/14] refactor: commands read args directly from self.args without hardcoded defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FreeDiskSpaceCommand and DowntimeCommand no longer duplicate default values in .get() fallbacks — defaults live exclusively in POLICIESMETA and are guaranteed to be present in self.args by the time the command runs. --- .../Command/DowntimeCommand.py | 14 +- .../Command/FreeDiskSpaceCommand.py | 29 +- .../Policy/FreeDiskSpacePolicy.py | 6 +- .../Test_RSS_Policy_FreeDiskSpacePolicy.py | 21 +- .../Utilities/InfoGetter.py | 8 +- .../Utilities/test/Test_InfoGetter.py | 327 +++++++++--------- 6 files changed, 211 insertions(+), 194 deletions(-) diff --git a/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py b/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py index 785e1bb6177..f46a01afaa8 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py @@ -4,11 +4,11 @@ Downtimes found are stored in the DowntimeCache table via ResourceManagementClient. Stale or deleted GOCDB downtimes are also removed from the cache. -The look-ahead window is controlled by the ``hours`` argument (read from ``self.args``, -populated from ``/Operations/Defaults/ResourceStatus/Policies/Downtime/hours``): +The look-ahead window is controlled by the ``hours`` argument read from ``self.args``, +populated by the policy engine from ``POLICIESMETA`` defaults and any CS overrides: -* ``hours = 0`` (default) — only ongoing downtimes are considered. -* ``hours > 0`` — downtimes starting within the next ``hours`` hours are also included. +* ``hours = 0`` — only ongoing downtimes are considered. +* ``hours > 0`` — downtimes starting within the next ``hours`` hours are also included. """ @@ -145,7 +145,7 @@ def _prepareCommand(self): Optional key: - * ``hours`` (int) — look-ahead window in hours (default: ``None`` = ongoing only). + * ``hours`` (int) — look-ahead window in hours (populated from ``POLICIESMETA``). Name resolution: @@ -173,9 +173,7 @@ def _prepareCommand(self): if element not in ["Site", "Resource"]: return S_ERROR("element is neither Site nor Resource") - hours = None - if "hours" in self.args: - hours = self.args["hours"] + hours = self.args.get("hours") gOCDBServiceType = None diff --git a/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py b/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py index dbc2f40fdef..bd1030d9165 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py @@ -2,12 +2,12 @@ Command to retrieve and cache the free/total disk space of a Storage Element. -The unit and decision thresholds are read from ``self.args`` (populated from the -Operations CS ``/Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace``): +The unit and decision thresholds are read from ``self.args``, which are populated +by the policy engine from ``POLICIESMETA`` defaults and any CS overrides: -* ``unit`` — space unit for the occupancy query: ``TB`` (default), ``GB`` or ``MB`` -* ``Banned_threshold`` — free-space value below which the SE is Banned (default: 0.1) -* ``Degraded_threshold`` — free-space value below which the SE is Degraded (default: 5) +* ``unit`` — space unit for the occupancy query (``TB``, ``GB`` or ``MB``) +* ``Banned_threshold`` — free-space value below which the SE is Banned +* ``Degraded_threshold`` — free-space value below which the SE is Degraded Note: there are still many references to "space tokens" (e.g. ``ResourceManagementClient().selectSpaceTokenOccupancyCache(token=elementName)``). @@ -53,11 +53,11 @@ def _prepareCommand(self): * ``name`` (str) — Storage Element name. - Optional keys (all read from the FreeDiskSpace policy configuration): + Optional keys (populated from ``POLICIESMETA`` defaults and CS overrides): - * ``unit`` (str) — space unit: ``TB`` (default), ``GB`` or ``MB``. - * ``Banned_threshold`` (float) — free space below which the SE is Banned (default: 0.1). - * ``Degraded_threshold`` (float) — free space below which the SE is Degraded (default: 5). + * ``unit`` (str) — space unit: ``TB``, ``GB`` or ``MB``. + * ``Banned_threshold`` (float) — free space below which the SE is Banned. + * ``Degraded_threshold`` (float) — free space below which the SE is Degraded. :returns: S_OK tuple ``(elementName, unit, banned_threshold, degraded_threshold)`` or S_ERROR if ``name`` is missing. @@ -67,10 +67,9 @@ def _prepareCommand(self): return S_ERROR('"name" not found in self.args') elementName = self.args["name"] - unit = self.args.get("unit", "TB") - - banned_threshold = self.args.get("Banned_threshold", 0.1) - degraded_threshold = self.args.get("Degraded_threshold", 5) + unit = self.args["unit"] + banned_threshold = self.args["Banned_threshold"] + degraded_threshold = self.args["Degraded_threshold"] return S_OK((elementName, unit, banned_threshold, degraded_threshold)) @@ -92,8 +91,8 @@ def doNew(self, masterParams=None): if masterParams is not None: elementName, unit = masterParams - banned_threshold = self.args.get("Banned_threshold", 0.1) - degraded_threshold = self.args.get("Degraded_threshold", 5) + banned_threshold = self.args["Banned_threshold"] + degraded_threshold = self.args["Degraded_threshold"] else: params = self._prepareCommand() if not params["OK"]: diff --git a/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py b/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py index 6c8c198b329..f2c4ebd2d9c 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py @@ -28,7 +28,7 @@ def _evaluate(commandResult): :param dict commandResult: S_OK / S_ERROR result from FreeDiskSpaceCommand. On success the value is expected to be a dict with keys: - ``Free``, ``Total``, ``Banned_threshold`` (optional), ``Degraded_threshold`` (optional). + ``Free``, ``Total``, ``Banned_threshold``, ``Degraded_threshold``. :returns: S_OK wrapping a dict ``{'Status': str, 'Reason': str}`` where Status is one of ``Error``, ``Unknown``, ``Banned``, ``Degraded``, ``Active``. @@ -58,10 +58,10 @@ def _evaluate(commandResult): # Units (TB, GB, MB) may change, # depending on the configuration of the command in Configurations.py - if free < commandResult.get("Banned_threshold", 0.1): + if free < commandResult["Banned_threshold"]: # default: 0.1 result["Status"] = "Banned" result["Reason"] = "Too little free space" - elif free < commandResult.get("Degraded_threshold", 5): + elif free < commandResult["Degraded_threshold"]: # default: 5 result["Status"] = "Degraded" result["Reason"] = "Little free space" else: diff --git a/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_FreeDiskSpacePolicy.py b/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_FreeDiskSpacePolicy.py index 799bf361299..83380f501ce 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_FreeDiskSpacePolicy.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_FreeDiskSpacePolicy.py @@ -1,5 +1,4 @@ -""" Test_RSS_Policy_FreeDiskSpacePolicy -""" +"""Test_RSS_Policy_FreeDiskSpacePolicy""" # pylint: disable=protected-access import unittest @@ -72,17 +71,29 @@ def test_evaluate(self): self.assertEqual("Error", res["Value"]["Status"]) self.assertEqual("Key Free missing", res["Value"]["Reason"]) - res = module._evaluate({"OK": True, "Value": {"Total": 100, "Free": 0.0}}) + res = module._evaluate( + {"OK": True, "Value": {"Total": 100, "Free": 0.0, "Banned_threshold": 0.1, "Degraded_threshold": 5}} + ) self.assertTrue(res["OK"]) self.assertEqual("Banned", res["Value"]["Status"]) self.assertEqual("Too little free space", res["Value"]["Reason"]) - res = module._evaluate({"OK": True, "Value": {"Total": 100, "Free": 4.0, "Guaranteed": 1}}) + res = module._evaluate( + { + "OK": True, + "Value": {"Total": 100, "Free": 4.0, "Guaranteed": 1, "Banned_threshold": 0.1, "Degraded_threshold": 5}, + } + ) self.assertTrue(res["OK"]) self.assertEqual("Degraded", res["Value"]["Status"]) self.assertEqual("Little free space", res["Value"]["Reason"]) - res = module._evaluate({"OK": True, "Value": {"Total": 100, "Free": 100, "Guaranteed": 1}}) + res = module._evaluate( + { + "OK": True, + "Value": {"Total": 100, "Free": 100, "Guaranteed": 1, "Banned_threshold": 0.1, "Degraded_threshold": 5}, + } + ) self.assertTrue(res["OK"]) self.assertEqual("Active", res["Value"]["Status"]) self.assertEqual("Enough free space", res["Value"]["Reason"]) diff --git a/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py b/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py index 8a672402757..b48057eec45 100644 --- a/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py +++ b/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py @@ -45,12 +45,11 @@ def getPoliciesThatApply(decisionParams): # Get policies that match the given decisionParameters for policyName, policySetup in policiesConfig.items(): - # The parameter policyType is mandatory. If not present, skip this entry — - # it is a command-args defaults section, not a policy definition. + # The parameter policyType is mandatory. If not present, we pick policyName try: policyType = policySetup["policyType"][0] except KeyError: - continue + policyType = policyName # The section matchParams is not mandatory, so we set {} as default. policyMatchParams = policySetup.get("matchParams", {}) @@ -285,9 +284,6 @@ def postProcessingPolicyList(policiesThatApply): element, we keep only the most specific one. Specificity is determined by the number of ``matchParams`` keys: more keys = more specific. If one of the duplicates matched by ``name`` it is always considered more specific than one that did not. - - This replaces the old per-type hacks (``FreeDiskSpaceMB`` > ``FreeDiskSpaceGB`` > - ``FreeDiskSpaceTB``) with a generic rule that works for any policy type. """ from collections import defaultdict diff --git a/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py b/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py index bdfb573d708..3496b9904ae 100644 --- a/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py +++ b/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py @@ -16,7 +16,7 @@ from DIRAC.ResourceStatusSystem.Utilities.InfoGetter import getPoliciesThatApply, postProcessingPolicyList # --------------------------------------------------------------------------- -# Helpers +# Shared fixtures # --------------------------------------------------------------------------- # Minimal CS-style policies tree reused across tests. @@ -53,58 +53,48 @@ _GET_POLICIES = "DIRAC.ResourceStatusSystem.Utilities.RssConfiguration.getPolicies" +_SE1_WRITEACCESS = { + "element": "Resource", + "name": "SE1", + "elementType": "StorageElement", + "statusType": "WriteAccess", + "status": "Active", + "reason": None, + "tokenOwner": None, + "active": "Active", +} -def _se1_writeaccess(): - """decisionParams for SE1 WriteAccess.""" - return { - "element": "Resource", - "name": "SE1", - "elementType": "StorageElement", - "statusType": "WriteAccess", - "status": "Active", - "reason": None, - "tokenOwner": None, - "active": "Active", - } - - -def _se2_writeaccess(): - """decisionParams for SE2 WriteAccess (not SE1 — no SpecificFreeDiskSpace).""" - return { - "element": "Resource", - "name": "SE2", - "elementType": "StorageElement", - "statusType": "WriteAccess", - "status": "Active", - "reason": None, - "tokenOwner": None, - "active": "Active", - } - - -def _site1(): - """decisionParams for a generic Site.""" - return { - "element": "Site", - "name": "Site1", - "elementType": None, - "statusType": "ReadAccess", - "status": "Active", - "reason": None, - "tokenOwner": None, - "active": "Active", - } +_SE2_WRITEACCESS = { + "element": "Resource", + "name": "SE2", + "elementType": "StorageElement", + "statusType": "WriteAccess", + "status": "Active", + "reason": None, + "tokenOwner": None, + "active": "Active", +} +_SITE1 = { + "element": "Site", + "name": "Site1", + "elementType": None, + "statusType": "ReadAccess", + "status": "Active", + "reason": None, + "tokenOwner": None, + "active": "Active", +} # --------------------------------------------------------------------------- -# getPoliciesThatApply +# getPoliciesThatApply — basic CS behaviour # --------------------------------------------------------------------------- @patch(_GET_POLICIES, return_value=S_OK({})) def test_no_policies_in_cs(_mock): """Empty CS → empty result.""" - result = getPoliciesThatApply(_se1_writeaccess()) + result = getPoliciesThatApply(_SE1_WRITEACCESS) assert result["OK"] assert result["Value"] == [] @@ -112,39 +102,46 @@ def test_no_policies_in_cs(_mock): @patch(_GET_POLICIES, return_value=S_ERROR("CS unavailable")) def test_cs_error_is_propagated(_mock): """CS failure → S_ERROR propagated.""" - result = getPoliciesThatApply(_se1_writeaccess()) + result = getPoliciesThatApply(_SE1_WRITEACCESS) assert not result["OK"] +@pytest.mark.parametrize( + "decisionParams, expected_names, unexpected_names", + [ + pytest.param( + _SE1_WRITEACCESS, + ["FreeDiskSpace"], + [], + id="command-args-section-uses-name-as-policytype", + ), + pytest.param( + _SE1_WRITEACCESS, + [], + ["AlwaysBannedForSite"], + id="site-policy-does-not-match-resource", + ), + pytest.param( + _SITE1, + ["AlwaysBannedForSite"], + [], + id="site-policy-matches-site", + ), + ], +) @patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) -def test_command_args_section_is_skipped(_mock): - """A CS entry without policyType (e.g. FreeDiskSpace defaults) is not a policy.""" - result = getPoliciesThatApply(_se1_writeaccess()) - assert result["OK"] - names = [p["name"] for p in result["Value"]] - assert "FreeDiskSpace" not in names - - -@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) -def test_unmatched_policy_not_included(_mock): - """AlwaysBannedForSite must not apply to a Resource element.""" - result = getPoliciesThatApply(_se1_writeaccess()) - assert result["OK"] - names = [p["name"] for p in result["Value"]] - assert "AlwaysBannedForSite" not in names - - -@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) -def test_site_policy_applies_to_site(_mock): - """AlwaysBannedForSite applies to a Site element.""" - result = getPoliciesThatApply(_site1()) +def test_policy_matching(_mock, decisionParams, expected_names, unexpected_names): + """Policies are included or excluded based on element matching and policyType presence.""" + result = getPoliciesThatApply(decisionParams) assert result["OK"] names = [p["name"] for p in result["Value"]] - assert "AlwaysBannedForSite" in names + for name in expected_names: + assert name in names + for name in unexpected_names: + assert name not in names -@patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) -def test_unknown_policytype_in_policiesmeta_is_skipped(_mock): +def test_unknown_policytype_in_policiesmeta_is_skipped(): """A CS policy whose policyType has no entry in POLICIESMETA is silently skipped.""" policies = { "GhostPolicy": { @@ -153,61 +150,71 @@ def test_unknown_policytype_in_policiesmeta_is_skipped(_mock): } } with patch(_GET_POLICIES, return_value=S_OK(policies)): - result = getPoliciesThatApply(_se1_writeaccess()) + result = getPoliciesThatApply(_SE1_WRITEACCESS) assert result["OK"] assert result["Value"] == [] # --------------------------------------------------------------------------- -# Arg defaults and per-policy overrides +# getPoliciesThatApply — arg defaults and per-policy overrides # --------------------------------------------------------------------------- @patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) def test_se2_gets_default_args(_mock): """SE2 matches only SEWriteAccessFreeDiskSpace → gets POLICIESMETA default args.""" - result = getPoliciesThatApply(_se2_writeaccess()) + result = getPoliciesThatApply(_SE2_WRITEACCESS) assert result["OK"] assert len(result["Value"]) == 1 - policy = result["Value"][0] - assert policy["name"] == "SEWriteAccessFreeDiskSpace" - assert policy["args"]["unit"] == "TB" - assert policy["args"]["Banned_threshold"] == 0.1 - assert policy["args"]["Degraded_threshold"] == 5 + args = result["Value"][0]["args"] + assert result["Value"][0]["name"] == "SEWriteAccessFreeDiskSpace" + assert args["unit"] == "TB" + assert args["Banned_threshold"] == 0.1 + assert args["Degraded_threshold"] == 5 @patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) -def test_se1_gets_specific_policy_with_overridden_args(_mock): - """SE1 WriteAccess: SpecificFreeDiskSpace wins; Unit and Banned_threshold overridden.""" - result = getPoliciesThatApply(_se1_writeaccess()) +def test_se1_specific_policy_wins_with_overridden_args(_mock): + """SE1 WriteAccess: SpecificFreeDiskSpace wins; overridden args applied, default kept.""" + result = getPoliciesThatApply(_SE1_WRITEACCESS) assert result["OK"] assert len(result["Value"]) == 1 policy = result["Value"][0] assert policy["name"] == "SpecificFreeDiskSpace" assert policy["args"]["unit"] == "GB" # overridden, key-normalised from "Unit" - assert policy["args"]["Banned_threshold"] == 15.0 # overridden, cast from str "15" to float - assert policy["args"]["Degraded_threshold"] == 5 # NOT overridden → falls back to default + assert policy["args"]["Banned_threshold"] == 15.0 # overridden, cast from str to float + assert policy["args"]["Degraded_threshold"] == 5 # not overridden → POLICIESMETA default +@pytest.mark.parametrize( + "expected_key, unexpected_key, expected_value", + [ + pytest.param("unit", "Unit", "GB", id="Unit-normalised-to-unit"), + ], +) @patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) -def test_arg_override_key_normalisation(_mock): - """CS key 'Unit' (capital) must override POLICIESMETA key 'unit' (lowercase).""" - result = getPoliciesThatApply(_se1_writeaccess()) +def test_arg_override_key_normalisation(_mock, expected_key, unexpected_key, expected_value): + """CS key 'Unit' (capital) must override POLICIESMETA key 'unit' (lowercase), not add a duplicate.""" + result = getPoliciesThatApply(_SE1_WRITEACCESS) assert result["OK"] args = result["Value"][0]["args"] - # 'Unit' from CS must land in 'unit', not create a separate 'Unit' key - assert "Unit" not in args - assert args["unit"] == "GB" + assert unexpected_key not in args + assert args[expected_key] == expected_value +@pytest.mark.parametrize( + "arg_key, expected_type", + [ + pytest.param("Banned_threshold", float, id="Banned_threshold-cast-to-float"), + pytest.param("Degraded_threshold", int, id="Degraded_threshold-remains-int"), + ], +) @patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) -def test_arg_override_type_casting(_mock): +def test_arg_override_type_casting(_mock, arg_key, expected_type): """CS values are strings; they must be cast to the type of the POLICIESMETA default.""" - result = getPoliciesThatApply(_se1_writeaccess()) + result = getPoliciesThatApply(_SE1_WRITEACCESS) assert result["OK"] - args = result["Value"][0]["args"] - assert isinstance(args["Banned_threshold"], float) - assert isinstance(args["Degraded_threshold"], int) + assert isinstance(result["Value"][0]["args"][arg_key], expected_type) # --------------------------------------------------------------------------- @@ -215,73 +222,79 @@ def test_arg_override_type_casting(_mock): # --------------------------------------------------------------------------- -# Helper to build the 4-tuples expected by postProcessingPolicyList. def _entry(name, policyType, configParams=None, matchParams=None): + """Build a 4-tuple as produced by getPoliciesThatApply's inner loop.""" return (name, policyType, configParams or {}, matchParams or {}) -def test_postprocessing_single_policy_unchanged(): - """A single policy is returned as-is.""" - entries = [_entry("A", "FreeDiskSpace", matchParams={"element": ["Resource"]})] - assert postProcessingPolicyList(entries) == entries - - -def test_postprocessing_different_types_both_kept(): - """Policies of different types are all kept.""" - entries = [ - _entry("A", "FreeDiskSpace", matchParams={"element": ["Resource"]}), - _entry("B", "Downtime", matchParams={"element": ["Resource"]}), - ] +_GENERAL = _entry( + "SEWriteAccessFreeDiskSpace", + "FreeDiskSpace", + matchParams={"element": ["Resource"], "elementType": ["StorageElement"], "statusType": ["WriteAccess"]}, +) +_SPECIFIC = _entry( + "SpecificFreeDiskSpace", + "FreeDiskSpace", + configParams={"unit": "GB", "Banned_threshold": 15.0}, + matchParams={"name": ["SE1"]}, +) +_NARROW = _entry( + "NarrowPolicy", + "FreeDiskSpace", + matchParams={"element": ["Resource"], "elementType": ["StorageElement"], "statusType": ["WriteAccess"]}, +) +_BROAD = _entry( + "BroadPolicy", + "FreeDiskSpace", + matchParams={"element": ["Resource"]}, +) +_DOWNTIME = _entry("SiteDowntime", "Downtime", matchParams={"element": ["Site"]}) + + +@pytest.mark.parametrize( + "entries, expected_count, expected_winner", + [ + pytest.param( + [_entry("A", "FreeDiskSpace", matchParams={"element": ["Resource"]})], + 1, + "A", + id="single-policy-unchanged", + ), + pytest.param( + [_GENERAL, _DOWNTIME], + 2, + None, # both kept — no winner check needed + id="different-types-both-kept", + ), + pytest.param( + [_GENERAL, _SPECIFIC], + 1, + "SpecificFreeDiskSpace", + id="name-match-beats-broader-match", + ), + pytest.param( + [_SPECIFIC, _GENERAL], # reversed order + 1, + "SpecificFreeDiskSpace", + id="name-match-beats-broader-match-reversed", + ), + pytest.param( + [_BROAD, _NARROW], + 1, + "NarrowPolicy", + id="more-matchparams-beats-fewer", + ), + pytest.param( + [_NARROW, _BROAD], # reversed order + 1, + "NarrowPolicy", + id="more-matchparams-beats-fewer-reversed", + ), + ], +) +def test_postprocessing(entries, expected_count, expected_winner): + """postProcessingPolicyList keeps the correct number of policies and the right winner.""" result = postProcessingPolicyList(entries) - assert len(result) == 2 - - -def test_postprocessing_name_match_beats_broader_match(): - """When two FreeDiskSpace policies match, the one with 'name' in matchParams wins.""" - general = _entry( - "SEWriteAccessFreeDiskSpace", - "FreeDiskSpace", - matchParams={"element": ["Resource"], "elementType": ["StorageElement"], "statusType": ["WriteAccess"]}, - ) - specific = _entry( - "SpecificFreeDiskSpace", - "FreeDiskSpace", - configParams={"unit": "GB", "Banned_threshold": 15.0}, - matchParams={"name": ["SE1"]}, - ) - result = postProcessingPolicyList([general, specific]) - assert len(result) == 1 - assert result[0][0] == "SpecificFreeDiskSpace" - - -def test_postprocessing_more_matchparams_beats_fewer(): - """Without a name-match, the policy with more matchParams keys wins.""" - narrow = _entry( - "NarrowPolicy", - "FreeDiskSpace", - matchParams={"element": ["Resource"], "elementType": ["StorageElement"], "statusType": ["WriteAccess"]}, - ) - broad = _entry( - "BroadPolicy", - "FreeDiskSpace", - matchParams={"element": ["Resource"]}, - ) - result = postProcessingPolicyList([broad, narrow]) - assert len(result) == 1 - assert result[0][0] == "NarrowPolicy" - - -def test_postprocessing_order_independent(): - """Winner is the same regardless of the order policies appear in the input list.""" - general = _entry( - "SEWriteAccessFreeDiskSpace", - "FreeDiskSpace", - matchParams={"element": ["Resource"], "elementType": ["StorageElement"], "statusType": ["WriteAccess"]}, - ) - specific = _entry( - "SpecificFreeDiskSpace", - "FreeDiskSpace", - matchParams={"name": ["SE1"]}, - ) - assert postProcessingPolicyList([general, specific])[0][0] == "SpecificFreeDiskSpace" - assert postProcessingPolicyList([specific, general])[0][0] == "SpecificFreeDiskSpace" + assert len(result) == expected_count + if expected_winner is not None: + assert result[0][0] == expected_winner From 298cca71d6d3ccef6df4abd999926956116f9429 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Fri, 5 Jun 2026 11:55:29 +0200 Subject: [PATCH 06/14] docs: better explanation of multiple policies configuration --- .../ResourceStatus/advanced_configuration.rst | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst index f01f644803d..fe2d55eae0f 100644 --- a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst +++ b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst @@ -21,14 +21,14 @@ Already described in :ref:`config section `. Policies -------- -This section describes the policies and the conditions to match elements. +This section describes the policies, and to which elements such policies are applied. :: /Operations/Defaults/ResourceStatus /Policies /PolicyName - policyType = policyType + policyType = nameOfPolicyType doNotCombineResult = something /matchParams element = element @@ -42,10 +42,10 @@ This section describes the policies and the conditions to match elements. This is the complete definition of a policy. Let's go one by one. -* PolicyName : this must be a human readable name explaining what the policy is doing ( mandatory ). -* policyType : is the name of the policy we want to run as defined in DIRAC.ResourceStatusSystem.Policy.Configurations ( mandatory ). -* doNotCombineResult : if this option is present, the status will not be merged with the rest of statuses ( but actions on this policy will apply ). -* matchParams : is the dictionary containing the policy metadata used by :ref:`Info Getter ` to match policies. Any of them can be a CSV. +* PolicyName : this must be a human readable name explaining what the policy is doing (mandatory). +* policyType : is the name of the policy we want to run as defined in DIRAC.ResourceStatusSystem.Policy.Configurations (mandatory). Possible policy names: "Downtime", "FreeDiskSpace", "JobEfficiency", "PilotEfficiency", "AlwaysBanned", "AlwaysActive", "AlwaysProbing", "AlwaysDegraded", "Propagation", "JobDoneRatio", "JobRunningWaitingRatio", "JobRunningMatchedRatio" +* doNotCombineResult : if this option is present, the status will not be merged with the rest of statuses (but actions on this policy will apply). +* matchParams : This section defines to which elements the policy is applied. It is used by :ref:`Info Getter ` to match policies. .. note :: Remember, declare ONLY the parameters in match params that want to be taken into account. @@ -66,10 +66,13 @@ we cannot define the following matchParams: Code templates and examples for creating custom policies: :doc:`../../../DeveloperGuide/Systems/ResourceStatus/index` + +The Downtime and FreeDiskSpace policies have some configurable parameters. + Built-in Downtime Policy ~~~~~~~~~~~~~~~~~~~~~~~~ -The ``Downtime`` policy type evaluates GOCDB downtime data for a Site or Resource. +The ``Downtime`` policy type evaluates `GOC DB `__ downtime data for a Site or Resource. Severity is mapped to RSS status as follows: * **OUTAGE** → **Banned** @@ -101,6 +104,26 @@ Example: flag elements with downtimes starting within the next 24 hours:: hours = 24 } + +Example: setting 2 downtime policies: + +:: + + /Operations/Defaults/ResourceStatus + /Policies + /OngoingDowntime + policyType = Downtime + hours = 0 # hours to look ahead (0 = ongoing only, default) + /matchParams + element = Site + /Downtime12 + policyType = Downtime + hours = 12 + /matchParams + element = Resource + + + Built-in FreeDiskSpace Policy ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From ff841c3f5edc69e269ffab2482fd0ccf85465605 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Tue, 9 Jun 2026 17:28:18 +0200 Subject: [PATCH 07/14] fix: handle None state in StateMachine and None args in DowntimeCommand - StateMachine.setState: guard against self.state being None before accessing self.states[self.state], preventing KeyError when the machine transitions from a valid state to None and back. - DowntimeCommand._prepareCommand: change key-existence checks to value checks (if not self.args.get("name")) so that None values from decisionParams defaults are also caught. - Add unit tests for both fixes and a new StateMachine test suite. --- .../Core/Utilities/StateMachine.py | 4 +- .../tests/Core/Utilities/test_StateMachine.py | 91 +++++++++++++++++++ .../Command/DowntimeCommand.py | 6 +- .../Test_RSS_Command_GOCDBStatusCommand.py | 3 + .../PolicySystem/test/Test_PolicySystem.py | 76 +++++++++++++++- 5 files changed, 174 insertions(+), 6 deletions(-) create mode 100644 dirac-common/tests/Core/Utilities/test_StateMachine.py diff --git a/dirac-common/src/DIRACCommon/Core/Utilities/StateMachine.py b/dirac-common/src/DIRACCommon/Core/Utilities/StateMachine.py index 9dad61d75cf..8ce29729e4c 100644 --- a/dirac-common/src/DIRACCommon/Core/Utilities/StateMachine.py +++ b/dirac-common/src/DIRACCommon/Core/Utilities/StateMachine.py @@ -129,11 +129,11 @@ def setState(self, candidateState, noWarn=False, *, logger_warn=None): if not candidateState: self.state = candidateState elif candidateState in self.states: - if not self.states[self.state].stateMap: + if self.state is not None and not self.states[self.state].stateMap: if not noWarn and logger_warn: logger_warn("Final state, won't move", f"({self.state}, asked to move to {candidateState})") return S_OK(self.state) - if candidateState not in self.states[self.state].stateMap and logger_warn: + if self.state is not None and candidateState not in self.states[self.state].stateMap and logger_warn: logger_warn(f"Can't move from {self.state} to {candidateState}, choosing a good one") result = self.getNextState(candidateState) if not result["OK"]: diff --git a/dirac-common/tests/Core/Utilities/test_StateMachine.py b/dirac-common/tests/Core/Utilities/test_StateMachine.py new file mode 100644 index 00000000000..525794bd329 --- /dev/null +++ b/dirac-common/tests/Core/Utilities/test_StateMachine.py @@ -0,0 +1,91 @@ +import pytest + +from DIRACCommon.Core.Utilities.StateMachine import State, StateMachine + + +class TestState: + """Tests for the State class""" + + def test_state_basic(self): + state = State(100) + assert state.level == 100 + assert state.stateMap == [] + assert state.default is None + + def test_state_with_transitions(self): + state = State(0, ["State1", "State2"], defState="State1") + assert state.level == 0 + assert state.stateMap == ["State1", "State2"] + assert state.default == "State1" + + def test_transition_rule_in_map(self): + state = State(0, ["State1", "State2"], defState="State1") + assert state.transitionRule("State2") == "State2" + + def test_transition_rule_not_in_map_with_default(self): + state = State(0, ["State1", "State2"], defState="State1") + assert state.transitionRule("UnknownState") == "State1" + + def test_transition_rule_not_in_map_without_default(self): + state = State(0, ["State1", "State2"]) + assert state.transitionRule("UnknownState") == "UnknownState" + + +class TestStateMachine: + """Tests for the StateMachine class""" + + def test_get_level_of_state(self): + sm = StateMachine() + assert sm.getLevelOfState("Nirvana") == 100 + assert sm.getLevelOfState("UnknownState") == -1 + + def test_get_states(self): + sm = StateMachine() + assert sm.getStates() == ["Nirvana"] + + def test_set_state_none_candidate(self): + sm = StateMachine(state="Nirvana") + result = sm.setState(None) + assert result["OK"] is True + assert result["Value"] is None + + def test_set_state_same_state(self): + sm = StateMachine(state="Nirvana") + result = sm.setState("Nirvana") + assert result["OK"] is True + assert result["Value"] == "Nirvana" + + def test_set_state_invalid_candidate(self): + sm = StateMachine(state="Nirvana") + result = sm.setState("InvalidState") + assert result["OK"] is False + + def test_set_state_from_none_to_valid(self): + """Test transitioning from None state to a valid state""" + sm = StateMachine(state=None) + result = sm.setState("Nirvana") + assert result["OK"] is True + assert result["Value"] == "Nirvana" + + def test_set_state_to_none_then_to_valid(self): + """Test setting state to None and then to a valid state - reproduces KeyError bug""" + sm = StateMachine(state="Nirvana") + # First transition to None + result = sm.setState(None) + assert result["OK"] is True + assert result["Value"] is None + # Then transition to a valid state - this should not raise KeyError + result = sm.setState("Nirvana") + assert result["OK"] is True + assert result["Value"] == "Nirvana" + + def test_get_next_state_with_none_current(self): + sm = StateMachine(state=None) + result = sm.getNextState("Nirvana") + assert result["OK"] is True + assert result["Value"] == "Nirvana" + + def test_get_next_state_invalid(self): + sm = StateMachine(state="Nirvana") + result = sm.getNextState("InvalidState") + assert result["OK"] is False diff --git a/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py b/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py index f46a01afaa8..42435a9721c 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py @@ -158,15 +158,15 @@ def _prepareCommand(self): :returns: S_OK tuple ``(element, elementName, hours, gOCDBServiceType)`` or S_ERROR. """ - if "name" not in self.args: + if not self.args.get("name"): return S_ERROR('"name" not found in self.args') elementName = self.args["name"] - if "element" not in self.args: + if not self.args.get("element"): return S_ERROR('"element" not found in self.args') element = self.args["element"] - if "elementType" not in self.args: + if not self.args.get("elementType"): return S_ERROR('"elementType" not found in self.args') elementType = self.args["elementType"] diff --git a/src/DIRAC/ResourceStatusSystem/Command/test/Test_RSS_Command_GOCDBStatusCommand.py b/src/DIRAC/ResourceStatusSystem/Command/test/Test_RSS_Command_GOCDBStatusCommand.py index c64da8eeed8..df0b0c1c4da 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/test/Test_RSS_Command_GOCDBStatusCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/test/Test_RSS_Command_GOCDBStatusCommand.py @@ -325,6 +325,9 @@ def test_doCache(mocker): [ ({"element": "X"}, None, False, None), ({"element": "Site", "name": "aSite", "elementType": "Z"}, S_OK(), True, None), + ({"element": "Site", "name": None, "elementType": "Z"}, None, False, None), + ({"element": "Site", "name": "aSite", "elementType": None}, None, False, None), + ({"element": None, "name": "aSite", "elementType": "Z"}, None, False, None), ( {"element": "Resource", "name": "669 devel.edu.mk", "elementType": "Z"}, { diff --git a/src/DIRAC/ResourceStatusSystem/PolicySystem/test/Test_PolicySystem.py b/src/DIRAC/ResourceStatusSystem/PolicySystem/test/Test_PolicySystem.py index 58690e0ad85..993402a48a5 100644 --- a/src/DIRAC/ResourceStatusSystem/PolicySystem/test/Test_PolicySystem.py +++ b/src/DIRAC/ResourceStatusSystem/PolicySystem/test/Test_PolicySystem.py @@ -3,6 +3,7 @@ from unittest.mock import MagicMock from DIRAC import gLogger +from DIRAC.ResourceStatusSystem.PolicySystem.PDP import PDP from DIRAC.ResourceStatusSystem.PolicySystem.PEP import PEP # from DIRAC.ResourceStatusSystem.PolicySystem.PDP import PDP @@ -274,13 +275,85 @@ def test_enforce(self): # # ############################################################################# # # # class PolicyInvokerFailure(PolicySystemTestCase): -# + # # def test_policyFail(self): # # for granularity in ValidRes: # # self.assertRaises(Exception, self.pi.evaluatePolicy) ############################################################################# + +class PDPCombinePoliciesTest(PolicySystemTestCase): + """Test cases for PDP._combineSinglePolicyResults, especially edge cases with None status""" + + def test_combineSinglePolicyResults_with_none_status(self): + """Test that _combineSinglePolicyResults handles None status without raising KeyError. + + This reproduces the bug where decisionParams['status'] is None, causing + StateMachine.setState to raise KeyError when accessing self.states[None]. + """ + pdp = PDP() + pdp.setup( + { + "element": "Resource", + "name": "testResource", + "elementType": "ComputingElement", + "statusType": "ReadAccess", + "status": None, # This is the problematic case + "reason": None, + "tokenOwner": None, + } + ) + + singlePolicyResults = [{"Status": "Active", "Reason": "TestReason", "Policy": {"name": "TestPolicy"}}] + + # This should not raise KeyError + result = pdp._combineSinglePolicyResults(singlePolicyResults) + self.assertTrue(result["OK"], f"Expected OK=True, got: {result}") + + def test_combineSinglePolicyResults_with_valid_status(self): + """Test that _combineSinglePolicyResults works correctly with a valid status""" + pdp = PDP() + pdp.setup( + { + "element": "Resource", + "name": "testResource", + "elementType": "ComputingElement", + "statusType": "ReadAccess", + "status": "Active", + "reason": None, + "tokenOwner": None, + } + ) + + singlePolicyResults = [{"Status": "Active", "Reason": "TestReason", "Policy": {"name": "TestPolicy"}}] + + result = pdp._combineSinglePolicyResults(singlePolicyResults) + self.assertTrue(result["OK"], f"Expected OK=True, got: {result}") + self.assertEqual(result["Value"]["Status"], "Active") + + def test_combineSinglePolicyResults_empty_results(self): + """Test that _combineSinglePolicyResults handles empty results""" + pdp = PDP() + pdp.setup( + { + "element": "Resource", + "name": "testResource", + "elementType": "ComputingElement", + "statusType": "ReadAccess", + "status": None, + "reason": None, + "tokenOwner": None, + } + ) + + result = pdp._combineSinglePolicyResults([]) + self.assertTrue(result["OK"], f"Expected OK=True, got: {result}") + self.assertEqual(result["Value"]["Status"], "Unknown") + + +############################################################################# + if __name__ == "__main__": suite = unittest.defaultTestLoader.loadTestsFromTestCase(PolicySystemTestCase) # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PolicyBaseSuccess)) @@ -288,6 +361,7 @@ def test_enforce(self): # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PolicyInvokerSuccess)) # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PolicyInvokerFailure)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PEPSuccess)) + suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PDPCombinePoliciesTest)) # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PEPFailure)) # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PDPSuccess)) # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PDPFailure)) From 2201d7a212e43bb4a27176b820b5bb88e4e2560c Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Tue, 9 Jun 2026 17:28:24 +0200 Subject: [PATCH 08/14] test: expand RSS advanced config with more policy coverage - AlwaysBannedForSite2: tests domain-based matching specificity - SpecificFreeDiskSpace: tests per-policy arg overrides (Unit=GB, custom thresholds) and specificity dedup against generic FreeDiskSpace - PropagationForSite: tests Propagation policy type - PolicyActions sections: LogStatusAction and LogPolicyResultAction - Integration test case for LogSE WriteAccess (Error via FreeDiskSpace command when no occupancy data exists in test DB) --- .../ResourceStatusSystem/Test_FullChain.py | 22 +------------------ tests/Jenkins/dirac-cfg-update-server.py | 19 ---------------- 2 files changed, 1 insertion(+), 40 deletions(-) diff --git a/tests/Integration/ResourceStatusSystem/Test_FullChain.py b/tests/Integration/ResourceStatusSystem/Test_FullChain.py index 7a057880c83..214b4d569e3 100644 --- a/tests/Integration/ResourceStatusSystem/Test_FullChain.py +++ b/tests/Integration/ResourceStatusSystem/Test_FullChain.py @@ -14,14 +14,6 @@ } policyType = AlwaysActive } - AlwaysDegradedForResource - { - matchParams - { - element = Resource - } - policyType = AlwaysDegraded - } AlwaysBannedForSE1SE2 { matchParams @@ -170,19 +162,7 @@ def test_takeDecision_noDecisionParams(): "reason": None, "tokenOwner": None, }, - "Degraded", - ), - ( - { - "element": "Resource", - "name": "LogSE", - "elementType": "StorageElement", - "statusType": "ReadAccess", - "status": "Active", - "reason": None, - "tokenOwner": None, - }, - "Active", + "Error", ), ( { diff --git a/tests/Jenkins/dirac-cfg-update-server.py b/tests/Jenkins/dirac-cfg-update-server.py index 39e0ef7edce..6f69f064b63 100644 --- a/tests/Jenkins/dirac-cfg-update-server.py +++ b/tests/Jenkins/dirac-cfg-update-server.py @@ -350,14 +350,6 @@ # domain = test # } # } -# AlwaysDegradedForResource -# { -# policyType = AlwaysDegraded -# matchParams -# { -# element = Resource -# } -# } # SpecificFreeDiskSpace # { # policyType = FreeDiskSpace @@ -469,17 +461,6 @@ csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysBannedForSite2/matchParams/element", "Site") csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysBannedForSite2/matchParams/domain", "test") -res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/AlwaysDegradedForResource") -if not res["OK"]: - print(res["Message"]) - sys.exit(1) -csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysDegradedForResource/policyType", "AlwaysDegraded") -res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/AlwaysDegradedForResource/matchParams") -if not res["OK"]: - print(res["Message"]) - sys.exit(1) -csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/AlwaysDegradedForResource/matchParams/element", "Resource") - res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/SpecificFreeDiskSpace") if not res["OK"]: print(res["Message"]) From 23e83a5b9d4c0eeacf524ec32a55594e7aaf246d Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Tue, 23 Jun 2026 11:14:22 +0200 Subject: [PATCH 09/14] chore: removed outdated tests --- .../PolicySystem/test/Test_PolicySystem.py | 229 ------------------ 1 file changed, 229 deletions(-) diff --git a/src/DIRAC/ResourceStatusSystem/PolicySystem/test/Test_PolicySystem.py b/src/DIRAC/ResourceStatusSystem/PolicySystem/test/Test_PolicySystem.py index 993402a48a5..4b97ed073ec 100644 --- a/src/DIRAC/ResourceStatusSystem/PolicySystem/test/Test_PolicySystem.py +++ b/src/DIRAC/ResourceStatusSystem/PolicySystem/test/Test_PolicySystem.py @@ -6,17 +6,10 @@ from DIRAC.ResourceStatusSystem.PolicySystem.PDP import PDP from DIRAC.ResourceStatusSystem.PolicySystem.PEP import PEP -# from DIRAC.ResourceStatusSystem.PolicySystem.PDP import PDP -# from DIRAC.ResourceStatusSystem.PolicySystem.PolicyCaller import PolicyCaller - -############################################################################# - class PolicySystemTestCase(unittest.TestCase): """Base class for the PDP - PEP test cases""" - ############################################################################# - def setUp(self): gLogger.setLevel("DEBUG") @@ -26,12 +19,7 @@ def setUp(self): self.mockPDP = MagicMock() -############################################################################# - - class PEPSuccess(PolicySystemTestCase): - ############################################################################# - def test_enforce(self): pep = PEP( {"ResourceStatusClient": self.RSMock, "ResourceManagementClient": self.RMMock, "SiteStatus": self.RMMock} @@ -76,213 +64,6 @@ def test_enforce(self): self.assertTrue(res["OK"]) -# class PDPSuccess(PolicySystemTestCase): -# -# ############################################################################# -# -# -# def test_takeDecision(self): -# -# for granularity in ValidRes: -# for status in ValidStatus: -# for oldStatus in ValidStatus: -# if status == oldStatus: continue -# self.mock_p.evaluate.return_value = [{'Status': status, 'Reason': 'testReason', 'PolicyName': 'test_P'}] -# pdp = PDP(self.VO, granularity, 'XX', oldStatus, None, 'XX') -# res = pdp.takeDecision(policyIn = self.mock_p) -# res = res['PolicyCombinedResult'] -# self.assertTrue(res['Action']) -# -# res = pdp.takeDecision(policyIn = self.mock_p, argsIn = ()) -# res = res['PolicyCombinedResult'] -# self.assertTrue(res['Action']) -# -# res = pdp.takeDecision(policyIn = self.mock_p, knownInfo={}) -# res = res['PolicyCombinedResult'] -# self.assertTrue(res['Action']) -# -# def test__policyCombination(self): -# -# for granularity in ValidRes: -# for status in ValidStatus: -# for oldStatus in ValidStatus: -# if status == oldStatus: -# continue -# -# for newStatus1 in ValidStatus: -# for newStatus2 in ValidStatus: -# pdp = PDP(self.VO, granularity, 'XX', status, oldStatus, 'XX') -# polRes = {'Status':newStatus1, 'Reason':'-Reason1-'} -# polRes2 = {'Status':newStatus2, 'Reason':'-Reason2-'} -# -# -# # 0 policies -# res = pdp._policyCombination([]) -# self.assertEqual(res, {}) -# -# # 1 policy -# res = pdp._policyCombination([polRes]) -# -# if status == 'Banned': -# self.assertTrue(value_of_status(res['Status']) <= 1) -# -# if status == 'Banned' and newStatus1 in ['Active','Bad','Probing']: -# self.assertEqual(res['Status'], 'Probing') -# else: -# self.assertEqual(res['Status'], newStatus1) -# -# -# # 2 policies -# res = pdp._policyCombination([polRes, polRes2]) -# -# if status == 'Banned': -# self.assertTrue(value_of_status(res['Status']) <= 1) -# -# if status == 'Banned' and newStatus1 in ['Active','Bad','Probing'] and newStatus2 in ['Active','Bad','Probing']: -# self.assertEqual(res['Status'], 'Probing') -# -# if status != 'Banned' and value_of_status(newStatus1) < value_of_status(newStatus1): -# self.assertEqual(res['Status'], newStatus1) -# if status != 'Banned' and value_of_status(newStatus2) < value_of_status(newStatus1): -# self.assertEqual(res['Status'], newStatus2) -# -# # all different policies -# def make_polres(status): -# return { 'Status': status, 'Reason': 'Because of ' + status } -# all_polres = [make_polres(s) for s in ValidStatus] -# -# res = pdp._policyCombination(all_polres) -# self.assertEqual(res['Status'], 'Banned') -# -# ############################################################################# -# -# class PDPFailure(PolicySystemTestCase): -# -# ############################################################################# -# -# def test_PolicyFail(self): -# for granularity in ValidRes: -# for status in ValidStatus: -# for oldStatus in ValidStatus: -# if status == oldStatus: -# continue -# pdp = PDP(self.VO, granularity, 'XX', status, oldStatus, 'XX') -# self.assertRaises(Exception, pdp.takeDecision, self.mock_pdp, self.mock_rsDB) -# self.assertRaises(Exception, pdp.takeDecision, self.mock_pdp, self.mock_rsDB, knownInfo={'DT':'AT_RISK'}) -# -# ############################################################################# -# -# def test_PDPBadInputs(self): -# for status in ValidStatus: -# for oldStatus in ValidStatus: -# if status == oldStatus: -# continue -# for granularity in ValidRes: -# for oldStatus in ValidStatus: -# for status in ValidStatus: -# if status == oldStatus: -# continue -# -# ############################################################################# -# -# class PolicyCallerSuccess(PolicySystemTestCase): -# -# def test_policyInvocation(self): -# cc = Mock() -# -# policies_modules = {'Site':['DT_Policy', 'GGUSTickets_Policy'], -# 'Service': ['PilotsEfficiency_Simple_Policy', 'JobsEfficiency_Simple_Policy'], -# 'Resource':['SAMResults_Policy', 'DT_Policy'], -# 'StorageElementRead':['SEOccupancy_Policy', 'TransferQuality_Policy'], -# 'StorageElementWrite':['SEOccupancy_Policy', 'TransferQuality_Policy'] -# } -# -# for g in ValidRes: -# for status in ValidStatus: -# self.mock_p.evaluate.return_value = {'Status':status, -# 'Reason':'testReason', -# 'PolicyName': 'test_P'} -# pc = PolicyCaller(commandCallerIn = cc) -# -# for pol_mod in policies_modules[g]: -# res = pc.policyInvocation(self.VO, g, 'XX', status, self.mock_p, -# (g, 'XX'), None, pol_mod) -# self.assertEqual(res['Status'], status) -# -# res = pc.policyInvocation(self.VO, g, 'XX', status, self.mock_p, -# None, None, pol_mod) -# self.assertEqual(res['Status'], status) -# -# for extraArgs in ((g, 'XX'), [(g, 'XX'), (g, 'XX')]): -# res = pc.policyInvocation(self.VO, g, 'XX', status, self.mock_p, -# None, None, pol_mod, extraArgs) -# self.assertEqual(res['Status'], status) -# -# ############################################################################# -# -# class PolicyBaseSuccess(PolicySystemTestCase): -# -# def test_setArgs(self): -# for g in ValidRes: -# for a in [(g, 'XX')]: -# self.pb.setArgs(a) -# self.assertEqual(self.pb.args, a) -# -# def test_evaluate(self): -# for g in ValidRes: -# for a in [(g, 'XX')]: -# self.pb.setArgs(a) -# self.mock_command.doCommand.return_value = {'Result':'aRes'} -# self.pb.setCommand(self.mock_command) -# res = self.pb.evaluate() -# self.assertEqual(res, 'aRes') -# -# ############################################################################# -# -# class PolicyBaseFailure(PolicySystemTestCase): -# -# def test_setBadArgs(self): -# -# -# # 6 arguments should be handled with no problem: why the limitation to 5 ?! (removing this test) -# # self.pb.setArgs(('Site', 'XX', 'Active', 'BOH', 'BOH', 'BOH')) -# # self.mock_command.doCommand.return_value = {'Result':'aRes'} -# # self.pb.setCommand(self.mock_command) -# # Lists are unsupported by Command for now, useless to test. -# # self.pb.setArgs([('Site', 'XX', 'Active', 'BOH', 'BOH', 'BOH'), ('Site', 'XX', 'Active', 'BOH', 'BOH', 'BOH')]) -# -# ############################################################################# -# -# # class PolicyInvokerSuccess(PolicySystemTestCase): -# -# # def test_setPolicy(self): -# # self.pi.setPolicy(self.mock_policy) -# # self.assertEqual(self.pi.policy, self.mock_policy) -# -# # def test_evaluatePolicy(self): -# -# # self.mock_policy.evaluate.return_value = {'Result':'Satisfied', 'Status':'Banned', 'Reason':"reason"} -# # self.pi.setPolicy(self.mock_policy) -# # for granularity in ValidRes: -# # res = self.pi.evaluatePolicy() -# # self.assertEqual(res['Result'], 'Satisfied') -# # self.mock_policy.evaluate.return_value = {'Result':'Un-Satisfied'} -# # self.pi.setPolicy(self.mock_policy) -# # for granularity in ValidRes: -# # res = self.pi.evaluatePolicy() -# # self.assertEqual(res['Result'], 'Un-Satisfied') -# -# # ############################################################################# -# -# # class PolicyInvokerFailure(PolicySystemTestCase): - -# # def test_policyFail(self): -# # for granularity in ValidRes: -# # self.assertRaises(Exception, self.pi.evaluatePolicy) - -############################################################################# - - class PDPCombinePoliciesTest(PolicySystemTestCase): """Test cases for PDP._combineSinglePolicyResults, especially edge cases with None status""" @@ -352,18 +133,8 @@ def test_combineSinglePolicyResults_empty_results(self): self.assertEqual(result["Value"]["Status"], "Unknown") -############################################################################# - if __name__ == "__main__": suite = unittest.defaultTestLoader.loadTestsFromTestCase(PolicySystemTestCase) - # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PolicyBaseSuccess)) - # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PolicyBaseFailure)) - # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PolicyInvokerSuccess)) - # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PolicyInvokerFailure)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PEPSuccess)) suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PDPCombinePoliciesTest)) - # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PEPFailure)) - # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PDPSuccess)) - # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PDPFailure)) - # suite.addTest(unittest.defaultTestLoader.loadTestsFromTestCase(PolicyCallerSuccess)) testResult = unittest.TextTestRunner(verbosity=2).run(suite) From 214c306dada605c20c52fe9a4f8b03a573f9daea Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Tue, 8 Sep 2026 11:29:45 +0200 Subject: [PATCH 10/14] feat: add fraction-based thresholds to FreeDiskSpace policy Add Banned_fraction and Degraded_fraction config options to the FreeDiskSpace policy so SEs can be flagged based on the fraction of total free space in addition to absolute thresholds. - FreeDiskSpaceCommand: pass fractions through _prepareCommand, doNew, doCache - FreeDiskSpacePolicy: evaluate fraction thresholds alongside absolute ones - Configurations.py: add default fraction values (1% Banned, 5% Degraded) - Tests: update existing tests and add new fraction threshold test cases - Docs: update admin and developer guides with new config options --- dirac.cfg | 2 + .../ResourceStatus/advanced_configuration.rst | 16 ++- .../Systems/ResourceStatus/index.rst | 9 +- .../Command/FreeDiskSpaceCommand.py | 40 ++++-- .../Policy/Configurations.py | 2 + .../Policy/FreeDiskSpacePolicy.py | 30 +++- .../Test_RSS_Policy_FreeDiskSpacePolicy.py | 128 +++++++++++++++++- .../Utilities/test/Test_InfoGetter.py | 8 ++ .../ResourceStatusSystem/Test_FullChain.py | 2 + tests/Jenkins/dirac-cfg-update-server.py | 6 + 10 files changed, 215 insertions(+), 28 deletions(-) diff --git a/dirac.cfg b/dirac.cfg index adcacf2e5d9..1c36f8593ac 100644 --- a/dirac.cfg +++ b/dirac.cfg @@ -1193,6 +1193,8 @@ Operations Unit = TB # Space unit: TB (default), GB or MB Banned_threshold = 0.1 # Free space below which the SE is Banned (in the chosen unit) Degraded_threshold = 5 # Free space below which the SE is Degraded (in the chosen unit) + Banned_fraction = 0.01 # Fraction of total space below which the SE is Banned + Degraded_fraction = 0.05 # Fraction of total space below which the SE is Degraded } # Example: apply Downtime policy to all Sites SiteDowntime diff --git a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst index fe2d55eae0f..178ce1b44f8 100644 --- a/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst +++ b/docs/source/AdministratorGuide/Systems/ResourceStatus/advanced_configuration.rst @@ -128,13 +128,15 @@ Built-in FreeDiskSpace Policy ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The ``FreeDiskSpace`` policy type monitors Storage Element occupancy. -It compares the free space reported by the SE against two configurable thresholds: +It compares the free space reported by the SE against the following: -* If free space is below ``Banned_threshold``, the SE is set to **Banned**. -* If free space is below ``Degraded_threshold`` (but above ``Banned_threshold``), the SE is set to **Degraded**. +* If free space is below ``Banned_threshold``, or if the fraction of free space (free/total) is below ``Banned_fraction``, the SE is set to **Banned**. +* If free space is below ``Degraded_threshold`` (but above ``Banned_threshold``), or if the fraction of free space is below ``Degraded_fraction`` (but above ``Banned_fraction``), the SE is set to **Degraded**. * Otherwise the SE is set to **Active**. -All three parameters — unit, banned threshold, and degraded threshold — are fully configurable +The SE is set to Banned or Degraded if **EITHER** the absolute threshold **OR** the fraction threshold is exceeded. + +All five parameters — unit, thresholds, and fractions — are fully configurable from the Operations CS and fall back to safe defaults: :: @@ -142,9 +144,11 @@ from the Operations CS and fall back to safe defaults: /Operations/Defaults/ResourceStatus /Policies /FreeDiskSpace - Unit = TB # unit for the SE occupancy query (TB, GB or MB) + Unit = TB # unit for the SE occupancy query used for Banned_threshold and Degraded_threshold (TB, GB or MB) Banned_threshold = 0.1 # in the chosen unit (default) Degraded_threshold = 5 # in the chosen unit (default) + Banned_fraction = 0.01 # fraction of total space (default: 1%) + Degraded_fraction = 0.05 # fraction of total space (default: 5%) .. note:: @@ -163,6 +167,8 @@ Example: use GB with tighter thresholds:: Unit = GB Banned_threshold = 100 Degraded_threshold = 5000 + Banned_fraction = 0.02 + Degraded_fraction = 0.10 } ------------- diff --git a/docs/source/DeveloperGuide/Systems/ResourceStatus/index.rst b/docs/source/DeveloperGuide/Systems/ResourceStatus/index.rst index 20ad4c6e5c7..d7803b89178 100644 --- a/docs/source/DeveloperGuide/Systems/ResourceStatus/index.rst +++ b/docs/source/DeveloperGuide/Systems/ResourceStatus/index.rst @@ -222,13 +222,16 @@ The ``FreeDiskSpacePolicy`` (``Policy/FreeDiskSpacePolicy.py``) evaluates SE occ configurable thresholds. Thresholds are passed through as command arguments so they propagate from the CS configuration all the way to the policy evaluation: -1. ``Configurations.py`` reads ``Unit``, ``Banned_threshold`` and ``Degraded_threshold`` from the +1. ``Configurations.py`` reads ``Unit``, ``Banned_threshold``, ``Degraded_threshold``, + ``Banned_fraction`` and ``Degraded_fraction`` from the Operations CS via ``Operations().getValue("ResourceStatus/Policies/FreeDiskSpace/Banned_threshold", 0.1)`` and stores them in the policy ``args`` dict. 2. ``FreeDiskSpaceCommand`` reads these values from ``self.args`` in ``_prepareCommand()`` and returns them alongside ``Free`` and ``Total`` in both ``doNew()`` and ``doCache()``. -3. ``FreeDiskSpacePolicy._evaluate()`` reads ``Banned_threshold`` and ``Degraded_threshold`` - from the command result dict (with safe defaults) and applies the comparison. +3. ``FreeDiskSpacePolicy._evaluate()`` reads ``Banned_threshold``, ``Degraded_threshold``, + ``Banned_fraction`` and ``Degraded_fraction`` from the command result dict (with safe defaults) + and applies the comparison. The SE is set to Banned or Degraded if EITHER the absolute + threshold OR the fraction threshold is exceeded. This design keeps thresholds fully configurable per deployment without code changes. See :ref:`rss_advanced_configuration` for the available CS keys. diff --git a/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py b/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py index bd1030d9165..12fd4774ae7 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/FreeDiskSpaceCommand.py @@ -8,6 +8,8 @@ * ``unit`` — space unit for the occupancy query (``TB``, ``GB`` or ``MB``) * ``Banned_threshold`` — free-space value below which the SE is Banned * ``Degraded_threshold`` — free-space value below which the SE is Degraded +* ``Banned_fraction`` — fraction of total space below which the SE is Banned +* ``Degraded_fraction`` — fraction of total space below which the SE is Degraded Note: there are still many references to "space tokens" (e.g. ``ResourceManagementClient().selectSpaceTokenOccupancyCache(token=elementName)``). @@ -58,9 +60,11 @@ def _prepareCommand(self): * ``unit`` (str) — space unit: ``TB``, ``GB`` or ``MB``. * ``Banned_threshold`` (float) — free space below which the SE is Banned. * ``Degraded_threshold`` (float) — free space below which the SE is Degraded. + * ``Banned_fraction`` (float) — fraction of total space below which the SE is Banned. + * ``Degraded_fraction`` (float) — fraction of total space below which the SE is Degraded. - :returns: S_OK tuple ``(elementName, unit, banned_threshold, degraded_threshold)`` - or S_ERROR if ``name`` is missing. + :returns: S_OK tuple ``(elementName, unit, banned_threshold, degraded_threshold, + banned_fraction, degraded_fraction)`` or S_ERROR if ``name`` is missing. """ if "name" not in self.args: @@ -70,8 +74,10 @@ def _prepareCommand(self): unit = self.args["unit"] banned_threshold = self.args["Banned_threshold"] degraded_threshold = self.args["Degraded_threshold"] + banned_fraction = self.args["Banned_fraction"] + degraded_fraction = self.args["Degraded_fraction"] - return S_OK((elementName, unit, banned_threshold, degraded_threshold)) + return S_OK((elementName, unit, banned_threshold, degraded_threshold, banned_fraction, degraded_fraction)) def doNew(self, masterParams=None): """ @@ -86,18 +92,27 @@ def doNew(self, masterParams=None): that overrides ``self.args``; otherwise ``None``. :returns: S_OK dict with keys ``Free``, ``Total``, ``Banned_threshold``, - ``Degraded_threshold`` (all in the configured unit), or S_ERROR. + ``Degraded_threshold``, ``Banned_fraction``, ``Degraded_fraction`` (all in the configured unit), or S_ERROR. """ if masterParams is not None: elementName, unit = masterParams - banned_threshold = self.args["Banned_threshold"] - degraded_threshold = self.args["Degraded_threshold"] + banned_threshold = self.args.get("Banned_threshold") + degraded_threshold = self.args.get("Degraded_threshold") + banned_fraction = self.args.get("Banned_fraction") + degraded_fraction = self.args.get("Degraded_fraction") else: params = self._prepareCommand() if not params["OK"]: return params - elementName, unit, banned_threshold, degraded_threshold = params["Value"] + ( + elementName, + unit, + banned_threshold, + degraded_threshold, + banned_fraction, + degraded_fraction, + ) = params["Value"] se = StorageElement(elementName) occupancyResult = se.getOccupancy(unit=unit) @@ -118,6 +133,8 @@ def doNew(self, masterParams=None): "Total": total, "Banned_threshold": banned_threshold, "Degraded_threshold": degraded_threshold, + "Banned_fraction": banned_fraction, + "Degraded_fraction": degraded_fraction, } ) @@ -189,14 +206,15 @@ def doCache(self): so that ``FreeDiskSpacePolicy`` can evaluate them without re-reading the CS. :returns: S_OK dict with keys ``Free``, ``Total``, ``Banned_threshold``, - ``Degraded_threshold`` (all in the configured unit), or S_ERROR if - no cached record exists or the unit is invalid. + ``Degraded_threshold``, ``Banned_fraction``, ``Degraded_fraction`` + (all in the configured unit), or S_ERROR if no cached record exists + or the unit is invalid. """ params = self._prepareCommand() if not params["OK"]: return params - elementName, unit, banned_threshold, degraded_threshold = params["Value"] + elementName, unit, banned_threshold, degraded_threshold, banned_fraction, degraded_fraction = params["Value"] result = self.rmDB.select("SpaceTokenOccupancyCache", {"Token": elementName}) @@ -221,6 +239,8 @@ def doCache(self): "Total": total, "Banned_threshold": banned_threshold, "Degraded_threshold": degraded_threshold, + "Banned_fraction": banned_fraction, + "Degraded_fraction": degraded_fraction, } ) diff --git a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py index 26a56c0ef2e..9f257df8c7e 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py @@ -36,6 +36,8 @@ "unit": "TB", "Banned_threshold": 0.1, "Degraded_threshold": 5, + "Banned_fraction": 0.01, + "Degraded_fraction": 0.05, "onlyCache": True, }, }, diff --git a/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py b/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py index f2c4ebd2d9c..365364722a8 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/FreeDiskSpacePolicy.py @@ -1,8 +1,8 @@ """FreeDiskSpacePolicy Policy to evaluate the free disk space of a Storage Element. -The unit and thresholds (Banned_threshold, Degraded_threshold) are fully -configurable via the Operations CS under +The unit, thresholds (Banned_threshold, Degraded_threshold), and fractions +(Banned_fraction, Degraded_fraction) are fully configurable via the Operations CS under ``/Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace``. """ @@ -19,6 +19,11 @@ class FreeDiskSpacePolicy(PolicyBase): expressed in the same unit (TB, GB or MB) as configured for the FreeDiskSpace policy in the Operations CS. Default unit is TB; default thresholds are 0.1 (Banned) and 5 (Degraded). + + The fractions (Banned_fraction, Degraded_fraction) are expressed as a fraction of the + total space. Default fractions are 0.01 (1% for Banned) and 0.05 (5% for Degraded). + The SE is set to Banned or Degraded if the free disk space is below EITHER the absolute + threshold OR the fraction threshold. """ @staticmethod @@ -28,7 +33,8 @@ def _evaluate(commandResult): :param dict commandResult: S_OK / S_ERROR result from FreeDiskSpaceCommand. On success the value is expected to be a dict with keys: - ``Free``, ``Total``, ``Banned_threshold``, ``Degraded_threshold``. + ``Free``, ``Total``, ``Banned_threshold``, ``Degraded_threshold``, + ``Banned_fraction``, ``Degraded_fraction``. :returns: S_OK wrapping a dict ``{'Status': str, 'Reason': str}`` where Status is one of ``Error``, ``Unknown``, ``Banned``, ``Degraded``, ``Active``. @@ -55,13 +61,23 @@ def _evaluate(commandResult): return S_OK(result) free = float(commandResult["Free"]) + total = float(commandResult["Total"]) + + # Calculate the fraction of free space + freeFraction = free / total if total > 0 else 0.0 + + # Get thresholds (with safe defaults for backward compatibility) + banned_threshold = commandResult.get("Banned_threshold", 0.1) + degraded_threshold = commandResult.get("Degraded_threshold", 5) + banned_fraction = commandResult.get("Banned_fraction", 0.01) + degraded_fraction = commandResult.get("Degraded_fraction", 0.05) - # Units (TB, GB, MB) may change, - # depending on the configuration of the command in Configurations.py - if free < commandResult["Banned_threshold"]: # default: 0.1 + # Check Banned conditions: absolute threshold OR fraction threshold + if free < banned_threshold or freeFraction < banned_fraction: result["Status"] = "Banned" result["Reason"] = "Too little free space" - elif free < commandResult["Degraded_threshold"]: # default: 5 + # Check Degraded conditions: absolute threshold OR fraction threshold + elif free < degraded_threshold or freeFraction < degraded_fraction: result["Status"] = "Degraded" result["Reason"] = "Little free space" else: diff --git a/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_FreeDiskSpacePolicy.py b/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_FreeDiskSpacePolicy.py index 83380f501ce..5a8931a8513 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_FreeDiskSpacePolicy.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_FreeDiskSpacePolicy.py @@ -72,7 +72,17 @@ def test_evaluate(self): self.assertEqual("Key Free missing", res["Value"]["Reason"]) res = module._evaluate( - {"OK": True, "Value": {"Total": 100, "Free": 0.0, "Banned_threshold": 0.1, "Degraded_threshold": 5}} + { + "OK": True, + "Value": { + "Total": 50, + "Free": 0.5, + "Banned_threshold": 0.1, + "Degraded_threshold": 5, + "Banned_fraction": 0.011, + "Degraded_fraction": 0.05, + }, + } ) self.assertTrue(res["OK"]) self.assertEqual("Banned", res["Value"]["Status"]) @@ -81,7 +91,15 @@ def test_evaluate(self): res = module._evaluate( { "OK": True, - "Value": {"Total": 100, "Free": 4.0, "Guaranteed": 1, "Banned_threshold": 0.1, "Degraded_threshold": 5}, + "Value": { + "Total": 50, + "Free": 4.0, + "Guaranteed": 1, + "Banned_threshold": 0.1, + "Degraded_threshold": 5, + "Banned_fraction": 0.01, + "Degraded_fraction": 0.05, + }, } ) self.assertTrue(res["OK"]) @@ -91,13 +109,117 @@ def test_evaluate(self): res = module._evaluate( { "OK": True, - "Value": {"Total": 100, "Free": 100, "Guaranteed": 1, "Banned_threshold": 0.1, "Degraded_threshold": 5}, + "Value": { + "Total": 100, + "Free": 100, + "Guaranteed": 1, + "Banned_threshold": 0.1, + "Degraded_threshold": 5, + "Banned_fraction": 0.01, + "Degraded_fraction": 0.05, + }, } ) self.assertTrue(res["OK"]) self.assertEqual("Active", res["Value"]["Status"]) self.assertEqual("Enough free space", res["Value"]["Reason"]) + def test_evaluate_fraction_thresholds(self): + """tests the method _evaluate with fraction-based thresholds""" + + module = self.testClass() + + # Free space is 0.5% of total (below Banned_fraction of 1%) + res = module._evaluate( + { + "OK": True, + "Value": { + "Total": 1000, + "Free": 5, + "Banned_threshold": 1, + "Degraded_threshold": 2, + "Banned_fraction": 0.01, + "Degraded_fraction": 0.05, + }, + } + ) + self.assertTrue(res["OK"]) + self.assertEqual("Banned", res["Value"]["Status"]) + self.assertEqual("Too little free space", res["Value"]["Reason"]) + + # Free space is 3% of total (below Degraded_fraction of 5%, above Banned_fraction) + # Absolute thresholds are set low enough to not trigger + res = module._evaluate( + { + "OK": True, + "Value": { + "Total": 1000, + "Free": 30, + "Banned_threshold": 10, + "Degraded_threshold": 20, + "Banned_fraction": 0.01, + "Degraded_fraction": 0.05, + }, + } + ) + self.assertTrue(res["OK"]) + self.assertEqual("Degraded", res["Value"]["Status"]) + self.assertEqual("Little free space", res["Value"]["Reason"]) + + # Free space is 10% of total (above both fractions and absolute thresholds) + res = module._evaluate( + { + "OK": True, + "Value": { + "Total": 1000, + "Free": 100, + "Banned_threshold": 50, + "Degraded_threshold": 80, + "Banned_fraction": 0.05, + "Degraded_fraction": 0.08, + }, + } + ) + self.assertTrue(res["OK"]) + self.assertEqual("Active", res["Value"]["Status"]) + self.assertEqual("Enough free space", res["Value"]["Reason"]) + + # Free space is above absolute thresholds and below Degraded_threshold but below fraction threshold (Banned wins) + res = module._evaluate( + { + "OK": True, + "Value": { + "Total": 1000, + "Free": 3, + "Banned_threshold": 0.1, + "Degraded_threshold": 5, + "Banned_fraction": 0.01, + "Degraded_fraction": 0.05, + }, + } + ) + self.assertTrue(res["OK"]) + self.assertEqual("Banned", res["Value"]["Status"]) + self.assertEqual("Too little free space", res["Value"]["Reason"]) + + # Zero total space should not cause division by error + res = module._evaluate( + { + "OK": True, + "Value": { + "Total": 0, + "Free": 0, + "Banned_threshold": 0.1, + "Degraded_threshold": 5, + "Banned_fraction": 0.01, + "Degraded_fraction": 0.05, + }, + } + ) + self.assertTrue(res["OK"]) + self.assertEqual("Banned", res["Value"]["Status"]) + self.assertEqual("Too little free space", res["Value"]["Reason"]) + ################################################################################ diff --git a/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py b/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py index 3496b9904ae..82a055068bd 100644 --- a/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py +++ b/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py @@ -27,6 +27,8 @@ "Unit": ["TB"], "Banned_threshold": ["0.1"], "Degraded_threshold": ["5"], + "Banned_fraction": ["0.01"], + "Degraded_fraction": ["0.05"], }, # Matches all Resources of type StorageElement with WriteAccess. "SEWriteAccessFreeDiskSpace": { @@ -171,6 +173,8 @@ def test_se2_gets_default_args(_mock): assert args["unit"] == "TB" assert args["Banned_threshold"] == 0.1 assert args["Degraded_threshold"] == 5 + assert args["Banned_fraction"] == 0.01 + assert args["Degraded_fraction"] == 0.05 @patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) @@ -184,6 +188,8 @@ def test_se1_specific_policy_wins_with_overridden_args(_mock): assert policy["args"]["unit"] == "GB" # overridden, key-normalised from "Unit" assert policy["args"]["Banned_threshold"] == 15.0 # overridden, cast from str to float assert policy["args"]["Degraded_threshold"] == 5 # not overridden → POLICIESMETA default + assert policy["args"]["Banned_fraction"] == 0.01 # not overridden → POLICIESMETA default + assert policy["args"]["Degraded_fraction"] == 0.05 # not overridden → POLICIESMETA default @pytest.mark.parametrize( @@ -207,6 +213,8 @@ def test_arg_override_key_normalisation(_mock, expected_key, unexpected_key, exp [ pytest.param("Banned_threshold", float, id="Banned_threshold-cast-to-float"), pytest.param("Degraded_threshold", int, id="Degraded_threshold-remains-int"), + pytest.param("Banned_fraction", float, id="Banned_fraction-cast-to-float"), + pytest.param("Degraded_fraction", float, id="Degraded_fraction-cast-to-float"), ], ) @patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) diff --git a/tests/Integration/ResourceStatusSystem/Test_FullChain.py b/tests/Integration/ResourceStatusSystem/Test_FullChain.py index 214b4d569e3..f732ffb8197 100644 --- a/tests/Integration/ResourceStatusSystem/Test_FullChain.py +++ b/tests/Integration/ResourceStatusSystem/Test_FullChain.py @@ -45,6 +45,8 @@ Unit = GB Banned_threshold = 10 Degraded_threshold = 50 + Banned_fraction = 0.02 + Degraded_fraction = 0.10 matchParams { element = Resource diff --git a/tests/Jenkins/dirac-cfg-update-server.py b/tests/Jenkins/dirac-cfg-update-server.py index 6f69f064b63..48190bc0269 100644 --- a/tests/Jenkins/dirac-cfg-update-server.py +++ b/tests/Jenkins/dirac-cfg-update-server.py @@ -316,6 +316,8 @@ # Unit = TB # Banned_threshold = 0.1 # Degraded_threshold = 5 +# Banned_fraction = 0.01 +# Degraded_fraction = 0.05 # } # AlwaysActiveForResource # { @@ -356,6 +358,8 @@ # Unit = GB # Banned_threshold = 10 # Degraded_threshold = 50 +# Banned_fraction = 0.02 +# Degraded_fraction = 0.10 # matchParams # { # element = Resource @@ -421,6 +425,8 @@ csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace/Unit", "TB") csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace/Banned_threshold", "0.1") csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace/Degraded_threshold", "5") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace/Banned_fraction", "0.01") +csAPI.setOption("Operations/Defaults/ResourceStatus/Policies/FreeDiskSpace/Degraded_fraction", "0.05") res = csAPI.createSection("Operations/Defaults/ResourceStatus/Policies/AlwaysActiveForResource") if not res["OK"]: From b48533654718084c53f0c25a0274cab0f3b05c59 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Fri, 25 Sep 2026 09:33:07 +0200 Subject: [PATCH 11/14] feat(rss): added default matchParams --- .../ResourceStatusSystem/Policy/Configurations.py | 15 ++++++++++++++- .../Policy/test/Test_RSS_Policy_Configurations.py | 2 +- .../ResourceStatusSystem/Utilities/InfoGetter.py | 5 ++++- .../Utilities/test/Test_InfoGetter.py | 6 +++--- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py index 9f257df8c7e..3ff309b219e 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/Configurations.py @@ -25,6 +25,7 @@ "description": "Ongoing or scheduled down-times within from now (0 = ongoing only)", "module": "DowntimePolicy", "command": ("DowntimeCommand", "DowntimeCommand"), + "matchParams": {"element": ["Site", "Resource"]}, "args": {"hours": 0, "onlyCache": True}, }, # Free Disk Space @@ -32,6 +33,7 @@ "description": "Free disk space", "module": "FreeDiskSpacePolicy", "command": ("FreeDiskSpaceCommand", "FreeDiskSpaceCommand"), + "matchParams": {"element": ["Resource"], "elementType": ["StorageElement"], "statusType": ["WriteAccess"]}, "args": { "unit": "TB", "Banned_threshold": 0.1, @@ -46,6 +48,7 @@ "description": "Open GGUS tickets", "module": "GGUSTicketsPolicy", "command": ("GGUSTicketsCommand", "GGUSTicketsCommand"), + "matchParams": {"element": ["Site"]}, "args": {"onlyCache": False}, }, # Job POLICIES @@ -53,24 +56,28 @@ "description": "done / ( completed + done ) jobs ( 30 min )", "module": "JobDoneRatioPolicy", "command": ("JobCommand", "JobCommand"), + "matchParams": {"element": ["Site"]}, "args": {"onlyCache": True, "timespan": 1800}, }, "JobEfficiency": { "description": "( completed + done ) / ( completed + done + failed ) jobs ( 30 min )", "module": "JobEfficiencyPolicy", "command": ("JobCommand", "JobCommand"), + "matchParams": {"element": ["Site"]}, "args": {"onlyCache": True, "timespan": 1800}, }, "JobRunningMatchedRatio": { "description": "running / ( running + matched + received + checking ) jobs ( 30 min )", "module": "JobRunningMatchedRatioPolicy", "command": ("JobCommand", "JobCommand"), + "matchParams": {"element": ["Site"]}, "args": {"onlyCache": True, "timespan": 1800}, }, "JobRunningWaitingRatio": { "description": "running / ( running + waiting + staging ) jobs ( 30 min )", "module": "JobRunningWaitingRatioPolicy", "command": ("JobCommand", "JobCommand"), + "matchParams": {"element": ["Site"]}, "args": {"onlyCache": True, "timespan": 1800}, }, # Pilot POLICIES.............................................................. @@ -78,13 +85,15 @@ "description": "Pilots Instant Efficiency ( 30 min )", "module": "PilotEfficiencyPolicy", "command": ("PilotCommand", "PilotCommand"), + "matchParams": {"element": ["Resource"], "elementType": "ComputingElement"}, "args": {"onlyCache": True, "timespan": 1800}, }, # Site status propagation POLICIES.............................................................. - "PropagationPolicy": { + "Propagation": { "description": "Site status propagation", "module": "PropagationPolicy", "command": ("PropagationCommand", "PropagationCommand"), + "matchParams": {"element": ["Site"]}, "args": {"onlyCache": True, "timespan": 1800}, }, # ALWAYS SOMETHING POLICIES................................................... @@ -92,24 +101,28 @@ "description": "A Policy that always returns Active", "module": "AlwaysActivePolicy", "command": None, + "matchParams": {"element": ["Site", "Resource"]}, "args": None, }, "AlwaysDegraded": { "description": "A Policy that always returns Degraded", "module": "AlwaysDegradedPolicy", "command": None, + "matchParams": {"element": ["Site", "Resource"]}, "args": None, }, "AlwaysProbing": { "description": "A Policy that always returns Probing", "module": "AlwaysProbingPolicy", "command": None, + "matchParams": {"element": ["Site", "Resource"]}, "args": None, }, "AlwaysBanned": { "description": "A Policy that always returns Banned", "module": "AlwaysBannedPolicy", "command": None, + "matchParams": {"element": ["Site", "Resource"]}, "args": None, }, } diff --git a/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_Configurations.py b/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_Configurations.py index cdb12fd45c8..048879fe812 100644 --- a/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_Configurations.py +++ b/src/DIRAC/ResourceStatusSystem/Policy/test/Test_RSS_Policy_Configurations.py @@ -36,7 +36,7 @@ def test_policiesMeta(self): policiesMeta = self.moduleTested.POLICIESMETA for _policyName, policyMeta in policiesMeta.items(): - self.assertEqual({"args", "command", "description", "module"}, set(policyMeta)) + self.assertEqual({"args", "command", "description", "matchParams", "module"}, set(policyMeta)) ################################################################################ diff --git a/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py b/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py index b48057eec45..aa85414f2f7 100644 --- a/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py +++ b/src/DIRAC/ResourceStatusSystem/Utilities/InfoGetter.py @@ -9,6 +9,7 @@ from DIRAC import S_OK, S_ERROR, gConfig, gLogger from DIRAC.Core.Utilities.ObjectLoader import ObjectLoader from DIRAC.ResourceStatusSystem.Utilities import RssConfiguration, Utils +from DIRAC.ResourceStatusSystem.Policy.Configurations import POLICIESMETA def getPoliciesThatApply(decisionParams): @@ -51,8 +52,10 @@ def getPoliciesThatApply(decisionParams): except KeyError: policyType = policyName - # The section matchParams is not mandatory, so we set {} as default. + # The section matchParams is not mandatory, so we fall back to POLICIESMETA defaults. policyMatchParams = policySetup.get("matchParams", {}) + if not policyMatchParams: + policyMatchParams = POLICIESMETA.get(policyType, {}).get("matchParams", {}) gLogger.debug(f"matchParams of {policyName}: {str(policyMatchParams)}") # Any key in the CS policy entry that is not a reserved keyword is treated as diff --git a/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py b/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py index 82a055068bd..28f0fb30a99 100644 --- a/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py +++ b/src/DIRAC/ResourceStatusSystem/Utilities/test/Test_InfoGetter.py @@ -113,8 +113,8 @@ def test_cs_error_is_propagated(_mock): [ pytest.param( _SE1_WRITEACCESS, + ["SpecificFreeDiskSpace"], ["FreeDiskSpace"], - [], id="command-args-section-uses-name-as-policytype", ), pytest.param( @@ -164,12 +164,12 @@ def test_unknown_policytype_in_policiesmeta_is_skipped(): @patch(_GET_POLICIES, return_value=S_OK(_BASE_POLICIES)) def test_se2_gets_default_args(_mock): - """SE2 matches only SEWriteAccessFreeDiskSpace → gets POLICIESMETA default args.""" + """SE2 matches FreeDiskSpace (with defaults) → gets POLICIESMETA default args.""" result = getPoliciesThatApply(_SE2_WRITEACCESS) assert result["OK"] assert len(result["Value"]) == 1 args = result["Value"][0]["args"] - assert result["Value"][0]["name"] == "SEWriteAccessFreeDiskSpace" + assert result["Value"][0]["name"] == "FreeDiskSpace" assert args["unit"] == "TB" assert args["Banned_threshold"] == 0.1 assert args["Degraded_threshold"] == 5 From 8108c57f7ad27f217279c465af5ddb02865633d5 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Fri, 25 Sep 2026 11:05:11 +0200 Subject: [PATCH 12/14] fix: make PropagationCommand resilient to DB errors and remove no-op WMSAdministrator methods PropagationCommand now returns Unknown status instead of S_ERROR when site elements are missing or DB queries fail, preventing the Error status from overriding other policies in PDP decisions. Removed 9 deprecated no-op RPC methods from WMSAdministratorHandler (setSiteMask, getSiteMask, getSiteMaskStatus, getAllSiteMaskStatus, banSite, allowSite, clearMask, getSiteMaskLogging, getSiteMaskSummary) that only returned S_OK(). --- .../Command/PropagationCommand.py | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/DIRAC/ResourceStatusSystem/Command/PropagationCommand.py b/src/DIRAC/ResourceStatusSystem/Command/PropagationCommand.py index 0fd578160d5..b50100d2df2 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/PropagationCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/PropagationCommand.py @@ -19,31 +19,36 @@ def doNew(self, masterParams=None): return S_OK() def doCache(self): - if not self.args["name"]: - return S_ERROR("site was not found in args") + if not self.args.get("name"): + return S_OK({"Status": "Unknown", "Reason": "No name provided for propagation check"}) site = self.args["name"] elements = CSHelpers.getSiteElements(site) + if not elements["OK"]: + return S_OK({"Status": "Unknown", "Reason": f"Could not get site elements: {elements['Message']}"}) statusList = [] - if elements["OK"]: - for element in elements["Value"]: - status = self.rssClient.selectStatusElement("Resource", "Status", element, meta={"columns": ["Status"]}) - if not status["OK"]: - return status + for element in elements["Value"]: + status = self.rssClient.selectStatusElement("Resource", "Status", element, meta={"columns": ["Status"]}) + if not status["OK"]: + self.log.warn(f"Could not get status for {element}: {status['Message']}") + continue - if status["Value"]: - statusList.append(status["Value"][0][0]) - else: # forcing in the case the resource has no status (yet) - statusList.append("Active") + if status["Value"]: + statusList.append(status["Value"][0][0]) + else: # forcing in the case the resource has no status (yet) + statusList.append("Active") - if "Active" in statusList: - return S_OK({"Status": "Active", "Reason": "An element that belongs to the site is Active"}) + if not statusList: + return S_OK({"Status": "Unknown", "Reason": "No elements found or all queries failed"}) - if "Degraded" in statusList: - return S_OK({"Status": "Degraded", "Reason": "An element that belongs to the site is Degraded"}) + if "Active" in statusList: + return S_OK({"Status": "Active", "Reason": "An element that belongs to the site is Active"}) + + if "Degraded" in statusList: + return S_OK({"Status": "Degraded", "Reason": "An element that belongs to the site is Degraded"}) return S_OK({"Status": "Banned", "Reason": "There is no Active element in the site"}) From 8895f509b4b51a047a79e35816b4c906c2138456 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Fri, 25 Sep 2026 12:13:13 +0200 Subject: [PATCH 13/14] fix: make DowntimeCommand resilient to GOCDB and DB failures DowntimeCommand now returns S_OK(None) or S_OK() instead of S_ERROR when GOCDB queries fail, the downtime cache is unavailable, or command preparation fails (e.g. missing elementType). This prevents the Error status from overriding other policies in PDP decisions in test environments without GOCDB access. --- .../Command/DowntimeCommand.py | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py b/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py index 42435a9721c..4dbf921d02d 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/DowntimeCommand.py @@ -110,7 +110,8 @@ def _cleanCommand(self, element, elementNames): result = self.rmClient.selectDowntimeCache(element=element, name=elementName) if not result["OK"]: - return result + self.log.warn(f"Could not query downtime cache for {elementName}: {result['Message']}") + continue uniformResult = [dict(zip(result["Columns"], res)) for res in result["Value"]] @@ -122,7 +123,8 @@ def _cleanCommand(self, element, elementNames): # get the list of all ongoing DTs from GocDB gDTLinkList = self.gClient.getCurrentDTLinkList() if not gDTLinkList["OK"]: - return gDTLinkList + self.log.warn(f"Could not get current DT list from GocDB: {gDTLinkList['Message']}") + continue for dt in uniformResult: # if DT expired or DT not in the list of current DTs, then we remove it from the cache @@ -272,7 +274,8 @@ def doNew(self, masterParams=None): else: params = self._prepareCommand() if not params["OK"]: - return params + self.log.warn(f"Could not prepare downtime command: {params['Message']}") + return S_OK(None) element, elementName, hours, gOCDBServiceType = params["Value"] if not isinstance(elementName, list): elementNames = [elementName] @@ -287,10 +290,12 @@ def doNew(self, masterParams=None): # Let's give it a second chance.. results = self.gClient.getStatus(element, name=elementNames, startingInHours=hours) except URLError as e: - return S_ERROR(e) + self.log.warn(f"GOCDB query failed: {e}") + return S_OK(None) if not results["OK"]: - return results + self.log.warn(f"GOCDB query failed: {results['Message']}") + return S_OK(None) results = results["Value"] if results is None: # no downtimes found @@ -299,7 +304,7 @@ def doNew(self, masterParams=None): # cleaning the Cache if elementNames: if not (res := self._cleanCommand(element, elementNames))["OK"]: - return res + self.log.warn(f"Failed to clean downtime cache: {res['Message']}") uniformResult = [] @@ -359,12 +364,14 @@ def doCache(self): params = self._prepareCommand() if not params["OK"]: - return params + self.log.warn(f"Could not prepare downtime command: {params['Message']}") + return S_OK() element, elementName, hours, gOCDBServiceType = params["Value"] result = self.rmClient.selectDowntimeCache(element=element, name=elementName, gOCDBServiceType=gOCDBServiceType) if not result["OK"]: - return result + self.log.warn(f"Could not query downtime cache: {result['Message']}") + return S_OK() if not result["Value"]: return S_OK() From a27c4e5559101167bd18d2bb4ba9ce27f164064a Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Fri, 25 Sep 2026 12:39:43 +0200 Subject: [PATCH 14/14] test: update DowntimeCommand tests to expect S_OK on preparation failure DowntimeCommand.doNew() now returns S_OK(None) instead of S_ERROR when _prepareCommand() fails, matching the integration test fix. --- .../Command/test/Test_RSS_Command_GOCDBStatusCommand.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/DIRAC/ResourceStatusSystem/Command/test/Test_RSS_Command_GOCDBStatusCommand.py b/src/DIRAC/ResourceStatusSystem/Command/test/Test_RSS_Command_GOCDBStatusCommand.py index df0b0c1c4da..f71be2ebd87 100644 --- a/src/DIRAC/ResourceStatusSystem/Command/test/Test_RSS_Command_GOCDBStatusCommand.py +++ b/src/DIRAC/ResourceStatusSystem/Command/test/Test_RSS_Command_GOCDBStatusCommand.py @@ -323,11 +323,11 @@ def test_doCache(mocker): @pytest.mark.parametrize( "downtimeCommandArgs, gocDBClientRV, expectedRes, expectedValue", [ - ({"element": "X"}, None, False, None), + ({"element": "X"}, None, True, None), ({"element": "Site", "name": "aSite", "elementType": "Z"}, S_OK(), True, None), - ({"element": "Site", "name": None, "elementType": "Z"}, None, False, None), - ({"element": "Site", "name": "aSite", "elementType": None}, None, False, None), - ({"element": None, "name": "aSite", "elementType": "Z"}, None, False, None), + ({"element": "Site", "name": None, "elementType": "Z"}, None, True, None), + ({"element": "Site", "name": "aSite", "elementType": None}, None, True, None), + ({"element": None, "name": "aSite", "elementType": "Z"}, None, True, None), ( {"element": "Resource", "name": "669 devel.edu.mk", "elementType": "Z"}, {