diff --git a/src/slurm_plugin/clustermgtd.py b/src/slurm_plugin/clustermgtd.py index d2253452..5ee5a751 100644 --- a/src/slurm_plugin/clustermgtd.py +++ b/src/slurm_plugin/clustermgtd.py @@ -961,7 +961,11 @@ def _maintain_nodes(self, partitions_name_map, compute_resource_nodes_map): log.info("Found the following unhealthy dynamic nodes: %s", print_with_count(unhealthy_dynamic_nodes)) self._handle_unhealthy_dynamic_nodes(unhealthy_dynamic_nodes) if unhealthy_static_nodes: - log.info("Found the following unhealthy static nodes: %s", print_with_count(unhealthy_static_nodes)) + # Include the node states, which carry the reason the nodes are considered unhealthy. + log.info( + "Found the following unhealthy static nodes: %s", + print_with_count([f"{node} in state {node.state_string}" for node in unhealthy_static_nodes]), + ) self._handle_unhealthy_static_nodes(unhealthy_static_nodes) # evaluate partitions to put in protected mode and ICEs nodes to terminate diff --git a/src/slurm_plugin/slurm_resources.py b/src/slurm_plugin/slurm_resources.py index 842e6a53..19ed0c50 100644 --- a/src/slurm_plugin/slurm_resources.py +++ b/src/slurm_plugin/slurm_resources.py @@ -589,20 +589,30 @@ def is_state_healthy(self, consider_drain_as_unhealthy, consider_down_as_unhealt return True else: if log_warn_if_unhealthy: - logger.warning("Node state check: node %s in DRAINED, node state: %s", self, self.state_string) + logger.warning( + "Node state check: node %s in DRAINED and not within the replacement protection window, " + "it is considered unhealthy and will be replaced, node state: %s", + self, + self.state_string, + ) return False # Check to see if node is in DOWN, ignoring any node currently being replaced elif self.is_down() and consider_down_as_unhealthy: if self.is_being_replaced: logger.debug( - "Node state check: node %s in DOWN but is currently being replaced, ignoring. Node state: ", + "Node state check: node %s in DOWN but is currently being replaced, ignoring, node state: %s", self, self.state_string, ) return True else: if log_warn_if_unhealthy: - logger.warning("Node state check: node %s in DOWN, node state: %s", self, self.state_string) + logger.warning( + "Node state check: node %s in DOWN and not within the replacement protection window, " + "it is considered unhealthy and will be replaced, node state: %s", + self, + self.state_string, + ) return False return True diff --git a/tests/slurm_plugin/slurm_resources/test_slurm_resources.py b/tests/slurm_plugin/slurm_resources/test_slurm_resources.py index dd0b4762..13b1a41a 100644 --- a/tests/slurm_plugin/slurm_resources/test_slurm_resources.py +++ b/tests/slurm_plugin/slurm_resources/test_slurm_resources.py @@ -718,6 +718,40 @@ def test_slurm_node_is_state_healthy( ) +@pytest.mark.parametrize( + "node, is_being_replaced, expected_warning", + [ + pytest.param( + StaticNode("queue-st-c5xlarge-1", "some_ip", "hostname", "DOWN+CLOUD+MAINTENANCE+RESERVED", "queue"), + False, + "node queue-st-c5xlarge-1(some_ip) in DOWN and not within the replacement protection window", + id="down_without_protection_window", + ), + pytest.param( + StaticNode("queue-st-c5xlarge-1", "some_ip", "hostname", "IDLE+CLOUD+DRAIN", "queue"), + False, + "node queue-st-c5xlarge-1(some_ip) in DRAINED and not within the replacement protection window", + id="drained_without_protection_window", + ), + pytest.param( + StaticNode("queue-st-c5xlarge-1", "some_ip", "hostname", "DOWN+CLOUD+MAINTENANCE+RESERVED", "queue"), + True, + None, + id="down_within_protection_window_not_warned", + ), + ], +) +def test_slurm_node_state_unhealthy_reason_is_logged(node, is_being_replaced, expected_warning, caplog): + """An unhealthy state is reported together with the fact that the node is not protected from replacement.""" + caplog.set_level(logging.WARNING) + node.is_being_replaced = is_being_replaced + node.is_state_healthy(consider_drain_as_unhealthy=True, consider_down_as_unhealthy=True) + if expected_warning: + assert_that(caplog.text).contains(expected_warning, node.state_string) + else: + assert_that(caplog.text).is_empty() + + @pytest.mark.parametrize( "node, instance, max_count, count_map, is_static_nodes_in_replacement, is_replacement_timeout, " "bootstrap_failure_messages, is_failing_health_check, is_node_bootstrap_failure",