From e19603ac64318d5f8379580c50af4d9db2d067eb Mon Sep 17 00:00:00 2001 From: "Mitch Harding (the weird one)" Date: Tue, 26 Mar 2024 12:40:30 -0400 Subject: [PATCH 1/5] CASMCMS-8952: Status operator should only run when it finds enabled components (cherry picked from commit 467bef38aed53f3b596a4c9cae21679eaae1a52c) --- CHANGELOG.md | 3 +++ src/bos/operators/status.py | 12 +++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86a1d191..e7b47eb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Changed +- If the status operator `_run` method finds no enabled components, stop immediately, as there is + nothing to do. ## [2.10.9] - 2024-03-20 ### Changed diff --git a/src/bos/operators/status.py b/src/bos/operators/status.py index cfcf91cb..4f0171be 100644 --- a/src/bos/operators/status.py +++ b/src/bos/operators/status.py @@ -2,7 +2,7 @@ # # MIT License # -# (C) Copyright 2022-2023 Hewlett Packard Enterprise Development LP +# (C) Copyright 2022-2024 Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -69,14 +69,16 @@ def _act(self, components): def _run(self) -> None: """ A single pass of detecting and acting on components """ components = self.bos_client.components.get_components(enabled=True) + if not components: + LOGGER.debug('No enabled components found') + return component_ids = [component['id'] for component in components] power_states = node_to_powerstate(component_ids) cfs_states = self._get_cfs_components() updated_components = [] - if components: - # Recreate these filters to pull in the latest options values - self.boot_wait_time_elapsed = TimeSinceLastAction(seconds=options.max_boot_wait_time)._match - self.power_on_wait_time_elapsed = TimeSinceLastAction(seconds=options.max_power_on_wait_time)._match + # Recreate these filters to pull in the latest options values + self.boot_wait_time_elapsed = TimeSinceLastAction(seconds=options.max_boot_wait_time)._match + self.power_on_wait_time_elapsed = TimeSinceLastAction(seconds=options.max_power_on_wait_time)._match for component in components: updated_component = self._check_status( component, power_states.get(component['id']), cfs_states.get(component['id'])) From ed8fc7869ca1365373def604c0bedd7010459621 Mon Sep 17 00:00:00 2001 From: "Mitch Harding (the weird one)" Date: Tue, 26 Mar 2024 12:51:21 -0400 Subject: [PATCH 2/5] CASMCMS-8952: Log warning and return immediately if some CFS functions are called without arguments (cherry picked from commit eee4878506827c99a1c4b77f35e3b663f9e5c119) --- CHANGELOG.md | 4 ++++ src/bos/operators/utils/clients/cfs.py | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7b47eb3..71c4abc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- Add code to the beginning of some CFS functions to check if they have been called without + necessary arguments, and if so, to log a warning and return immediately. + ### Changed - If the status operator `_run` method finds no enabled components, stop immediately, as there is nothing to do. diff --git a/src/bos/operators/utils/clients/cfs.py b/src/bos/operators/utils/clients/cfs.py index c6d43122..b6d29f08 100644 --- a/src/bos/operators/utils/clients/cfs.py +++ b/src/bos/operators/utils/clients/cfs.py @@ -55,6 +55,9 @@ def get_components(session=None, **kwargs): def patch_components(data, session=None): + if not data: + LOGGER.warning("patch_components called without data; returning without action.") + return if not session: session = requests_retry_session() LOGGER.debug("PATCH %s with body=%s", COMPONENTS_ENDPOINT, data) @@ -69,6 +72,9 @@ def patch_components(data, session=None): def get_components_from_id_list(id_list): + if not id_list: + LOGGER.warning("get_components_from_id_list called without IDs; returning without action.") + return [] LOGGER.debug("get_components_from_id_list called with %d IDs", len(id_list)) session = requests_retry_session() component_list = [] @@ -83,6 +89,9 @@ def get_components_from_id_list(id_list): def patch_desired_config(node_ids, desired_config, enabled=False, tags=None, clear_state=False): + if not node_ids: + LOGGER.warning("patch_desired_config called without IDs; returning without action.") + return LOGGER.debug("patch_desired_config called on %d IDs with desired_config=%s enabled=%s tags=%s" " clear_state=%s", len(node_ids), desired_config, enabled, tags, clear_state) session = requests_retry_session() @@ -107,6 +116,9 @@ def patch_desired_config(node_ids, desired_config, enabled=False, tags=None, cle def set_cfs(components, enabled, clear_state=False): + if not components: + LOGGER.warning("set_cfs called without components; returning without action.") + return LOGGER.debug("set_cfs called on %d components with enabled=%s clear_state=%s", len(components), enabled, clear_state) configurations = defaultdict(list) From fcbe85566a762c32c118b66b6a07f32b44b254ff Mon Sep 17 00:00:00 2001 From: "Mitch Harding (the weird one)" Date: Tue, 26 Mar 2024 13:24:55 -0400 Subject: [PATCH 3/5] CASMCMS-8952: Add code to PCS functions to handle situation when required arguments are empty --- CHANGELOG.md | 1 + src/bos/operators/utils/clients/pcs.py | 27 ++++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71c4abc1..ab0cecc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add code to the beginning of some CFS functions to check if they have been called without necessary arguments, and if so, to log a warning and return immediately. +- Added similar code to some PCS functions. ### Changed - If the status operator `_run` method finds no enabled components, stop immediately, as there is diff --git a/src/bos/operators/utils/clients/pcs.py b/src/bos/operators/utils/clients/pcs.py index 88e4de7b..e3830c39 100644 --- a/src/bos/operators/utils/clients/pcs.py +++ b/src/bos/operators/utils/clients/pcs.py @@ -1,7 +1,7 @@ # # MIT License # -# (C) Copyright 2023 Hewlett Packard Enterprise Development LP +# (C) Copyright 2023-2024 Hewlett Packard Enterprise Development LP # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -118,9 +118,12 @@ def status(nodes, session=None, **kwargs): PowerControlException: Any non-nominal response from PCS. JSONDecodeError: Error decoding the PCS response """ + status_bucket = defaultdict(set) + if not nodes: + LOGGER.warning("status called without nodes; returning without action.") + return status_bucket session = session or requests_retry_session() power_status_all = _power_status(xname=list(nodes), session=session, **kwargs) - status_bucket = defaultdict(set) for power_status_entry in power_status_all['status']: # IF the returned xname has an error, it itself is the status regardless of # what the powerState field suggests. This is a major departure from how CAPMC handled errors. @@ -139,12 +142,16 @@ def node_to_powerstate(nodes, session=None, **kwargs): For an iterable of nodes ; return a dictionary that maps to the current power state for the node in question. """ power_states = {} + if not nodes: + LOGGER.warning("node_to_powerstate called without nodes; returning without action.") + return power_states session = session or requests_retry_session() status_bucket = status(nodes, session, **kwargs) for pstatus, nodeset in status_bucket.items(): for node in nodeset: power_states[node] = pstatus return power_states + def _transition_create(xnames, operation, task_deadline_minutes=None, deputy_key=None, session=None): """ Interact with PCS to create a request to transition one or more xnames. The transition @@ -202,6 +209,10 @@ def power_on(nodes, session=None, task_deadline_minutes=1, **kwargs): Sends a request to PCS for transitioning nodes in question to a powered on state. Returns: A JSON parsed object response from PCS, which includes the created request ID. """ + if not nodes: + # Should probably raise an exception here, since we don't want to actually call PCS + # with an empty node list. Suggestions welcome for what to raise. + LOGGER.error("power_on called with no nodes!") session = session or requests_retry_session() return _transition_create(xnames=nodes, operation='On', task_deadline_minutes=task_deadline_minutes, session=session, **kwargs) @@ -210,6 +221,10 @@ def power_off(nodes, session=None, task_deadline_minutes=1, **kwargs): Sends a request to PCS for transitioning nodes in question to a powered off state (graceful). Returns: A JSON parsed object response from PCS, which includes the created request ID. """ + if not nodes: + # Should probably raise an exception here, since we don't want to actually call PCS + # with an empty node list. Suggestions welcome for what to raise. + LOGGER.error("power_off called with no nodes!") session = session or requests_retry_session() return _transition_create(xnames=nodes, operation='Off', task_deadline_minutes=task_deadline_minutes, session=session, **kwargs) @@ -218,6 +233,10 @@ def soft_off(nodes, session=None, task_deadline_minutes=1, **kwargs): Sends a request to PCS for transitioning nodes in question to a powered off state (graceful). Returns: A JSON parsed object response from PCS, which includes the created request ID. """ + if not nodes: + # Should probably raise an exception here, since we don't want to actually call PCS + # with an empty node list. Suggestions welcome for what to raise. + LOGGER.error("soft_off called with no nodes!") session = session or requests_retry_session() return _transition_create(xnames=nodes, operation='Soft-Off', task_deadline_minutes=task_deadline_minutes, session=session, **kwargs) @@ -226,6 +245,10 @@ def force_off(nodes, session=None, task_deadline_minutes=1, **kwargs): Sends a request to PCS for transitioning nodes in question to a powered off state (forceful). Returns: A JSON parsed object response from PCS, which includes the created request ID. """ + if not nodes: + # Should probably raise an exception here, since we don't want to actually call PCS + # with an empty node list. Suggestions welcome for what to raise. + LOGGER.error("force_off called with no nodes!") session = session or requests_retry_session() return _transition_create(xnames=nodes, operation='Force-Off', task_deadline_minutes=task_deadline_minutes, session=session, **kwargs) From 54bd8dcacb75ceb21726fad990eb000379a993c3 Mon Sep 17 00:00:00 2001 From: Mitch Harding Date: Wed, 27 Mar 2024 11:49:39 -0400 Subject: [PATCH 4/5] Update src/bos/operators/utils/clients/pcs.py Fix capitalization in code comment --- src/bos/operators/utils/clients/pcs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bos/operators/utils/clients/pcs.py b/src/bos/operators/utils/clients/pcs.py index e3830c39..8782b8f9 100644 --- a/src/bos/operators/utils/clients/pcs.py +++ b/src/bos/operators/utils/clients/pcs.py @@ -125,7 +125,7 @@ def status(nodes, session=None, **kwargs): session = session or requests_retry_session() power_status_all = _power_status(xname=list(nodes), session=session, **kwargs) for power_status_entry in power_status_all['status']: - # IF the returned xname has an error, it itself is the status regardless of + # If the returned xname has an error, it itself is the status regardless of # what the powerState field suggests. This is a major departure from how CAPMC handled errors. xname = power_status_entry.get('xname', '') if power_status_entry['error']: From 2296b04853ecdb50978cde2857707804b9f8327f Mon Sep 17 00:00:00 2001 From: "Mitch Harding (the weird one)" Date: Wed, 27 Mar 2024 12:04:39 -0400 Subject: [PATCH 5/5] CASMCMS-8952: Add PowerControlComponentsEmptyException to be raised by some PCS client functions --- CHANGELOG.md | 2 ++ src/bos/operators/utils/clients/pcs.py | 34 +++++++++++++++++--------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab0cecc1..c605bd8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add code to the beginning of some CFS functions to check if they have been called without necessary arguments, and if so, to log a warning and return immediately. - Added similar code to some PCS functions. +- Created `PowerControlComponentsEmptyException`; raise it when some PCS functions receive + empty component list arguments. ### Changed - If the status operator `_run` method finds no enabled components, stop immediately, as there is diff --git a/src/bos/operators/utils/clients/pcs.py b/src/bos/operators/utils/clients/pcs.py index 8782b8f9..09135bc8 100644 --- a/src/bos/operators/utils/clients/pcs.py +++ b/src/bos/operators/utils/clients/pcs.py @@ -58,6 +58,20 @@ class PowerControlTimeoutException(PowerControlException): """ +class PowerControlComponentsEmptyException(Exception): + """ + Raised when one of the PCS utility functions that requires a non-empty + list of components is passed an empty component list. This will only + happen in the case of a programming bug. + + This exception is not raised for functions that require a node list + but that are able to return a sensible object to the caller that + indicates nothing has been done. For example, the status function. + This exception is instead used for functions that will fail if they run + with an empty node list, but which cannot return an appropriate + "no-op" value to the caller. + """ + def _power_status(xname=None, power_state_filter=None, management_state_filter=None, session=None): """ @@ -177,7 +191,11 @@ def _transition_create(xnames, operation, task_deadline_minutes=None, deputy_key Raises: PowerControlException: Any non-nominal response from PCS, typically as a result of an unexpected payload response, or a failure to create a transition record. + PowerControlComponentsEmptyException: No xnames specified """ + if not xnames: + raise PowerControlComponentsEmptyException( + "_transition_create called with no xnames! (operation=%s)" % operation) session = session or requests_retry_session() try: assert operation in set(['On', 'Off', 'Soft-Off', 'Soft-Restart', 'Hard-Restart', 'Init', 'Force-Off']) @@ -210,9 +228,7 @@ def power_on(nodes, session=None, task_deadline_minutes=1, **kwargs): Returns: A JSON parsed object response from PCS, which includes the created request ID. """ if not nodes: - # Should probably raise an exception here, since we don't want to actually call PCS - # with an empty node list. Suggestions welcome for what to raise. - LOGGER.error("power_on called with no nodes!") + raise PowerControlComponentsEmptyException("power_on called with no nodes!") session = session or requests_retry_session() return _transition_create(xnames=nodes, operation='On', task_deadline_minutes=task_deadline_minutes, session=session, **kwargs) @@ -222,9 +238,7 @@ def power_off(nodes, session=None, task_deadline_minutes=1, **kwargs): Returns: A JSON parsed object response from PCS, which includes the created request ID. """ if not nodes: - # Should probably raise an exception here, since we don't want to actually call PCS - # with an empty node list. Suggestions welcome for what to raise. - LOGGER.error("power_off called with no nodes!") + raise PowerControlComponentsEmptyException("power_off called with no nodes!") session = session or requests_retry_session() return _transition_create(xnames=nodes, operation='Off', task_deadline_minutes=task_deadline_minutes, session=session, **kwargs) @@ -234,9 +248,7 @@ def soft_off(nodes, session=None, task_deadline_minutes=1, **kwargs): Returns: A JSON parsed object response from PCS, which includes the created request ID. """ if not nodes: - # Should probably raise an exception here, since we don't want to actually call PCS - # with an empty node list. Suggestions welcome for what to raise. - LOGGER.error("soft_off called with no nodes!") + raise PowerControlComponentsEmptyException("soft_off called with no nodes!") session = session or requests_retry_session() return _transition_create(xnames=nodes, operation='Soft-Off', task_deadline_minutes=task_deadline_minutes, session=session, **kwargs) @@ -246,9 +258,7 @@ def force_off(nodes, session=None, task_deadline_minutes=1, **kwargs): Returns: A JSON parsed object response from PCS, which includes the created request ID. """ if not nodes: - # Should probably raise an exception here, since we don't want to actually call PCS - # with an empty node list. Suggestions welcome for what to raise. - LOGGER.error("force_off called with no nodes!") + raise PowerControlComponentsEmptyException("force_off called with no nodes!") session = session or requests_retry_session() return _transition_create(xnames=nodes, operation='Force-Off', task_deadline_minutes=task_deadline_minutes, session=session, **kwargs)