From d0e8291471b8f65a49fd659daa052a9e66d1230b Mon Sep 17 00:00:00 2001 From: Dan Chaffelson Date: Thu, 9 Jul 2026 16:40:40 +0100 Subject: [PATCH] Surface controller services left disabled by schedule_all_controllers When enabling, NiFi silently skips INVALID controller services, so schedule_all_controllers could report success while leaving services disabled. It now checks for any controller left in a non-ENABLED state after an enable and, by default, logs a warning naming them; with the new opt-in strict=True it raises ValueError instead. Default behaviour and return type are unchanged, and the disable path is untouched. Adds tests for the all-valid (no warning), default-warn, and strict-raise paths using the existing INVALID controller fixture. --- nipyapi/canvas.py | 41 ++++++++++++++++++++++++++++----- tests/test_canvas.py | 54 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 81 insertions(+), 14 deletions(-) diff --git a/nipyapi/canvas.py b/nipyapi/canvas.py index 4c42d422..d726839a 100644 --- a/nipyapi/canvas.py +++ b/nipyapi/canvas.py @@ -2435,7 +2435,7 @@ def _schedule_controller_state(cont_id, tgt_state): raise ValueError("Scheduling request timed out") -def schedule_all_controllers(pg_id, scheduled): +def schedule_all_controllers(pg_id, scheduled, strict=False): """ Enable or Disable all Controller Services in a Process Group. @@ -2444,21 +2444,31 @@ def schedule_all_controllers(pg_id, scheduled): to reach the target state before returning. Note: - When enabling, INVALID controllers (those with validation errors) are - skipped since NiFi cannot enable them. The function waits only for - VALID controllers to reach ENABLED state. When disabling, all - controllers are included since any controller can be disabled. + When enabling, NiFi silently skips INVALID controllers (those with + validation errors) since they cannot be enabled, so the bulk operation + can report success while leaving some services disabled. This function + waits only for VALID controllers to reach ENABLED state, then checks for + any controllers left in a non-ENABLED state: by default it logs a + warning naming them; with strict=True it raises instead. When disabling, + all controllers are included since any controller can be disabled. Args: pg_id (str): The UUID of the Process Group scheduled (bool or str): True/False for ENABLED/DISABLED, or one of "ENABLED", "DISABLED". + strict (bool): When enabling, if True raise a ValueError if any + controller service is left in a non-ENABLED state (for example + because it is INVALID). When False (default) such controllers are + reported via a logged warning and the operation returns normally. + Has no effect when disabling. Returns: ActivateControllerServicesEntity: The result of the operation Raises: - ValueError: If scheduled is not a bool or valid state string. + ValueError: If scheduled is not a bool or valid state string, or if + strict is True and one or more controllers were left non-ENABLED + after an enable operation. """ assert isinstance(pg_id, str) @@ -2496,6 +2506,25 @@ def _all_controllers_in_state(): ) if not state_complete: raise ValueError(f"Timed out waiting for controllers to reach state {target_state}") + + # When enabling, NiFi silently skips INVALID controllers, so the bulk call + # can succeed while leaving services disabled. Surface any that remain in a + # non-ENABLED state rather than returning success silently. + if target_state == "ENABLED": + not_enabled = [c for c in list_all_controllers(pg_id) if c.component.state != "ENABLED"] + if not_enabled: + detail = ", ".join( + f"{c.component.name} (id={c.id}, " + f"validation_status={c.component.validation_status})" + for c in not_enabled + ) + msg = ( + f"{len(not_enabled)} controller service(s) in process group {pg_id} " + f"were not enabled and remain in a non-ENABLED state: {detail}" + ) + if strict: + raise ValueError(msg) + log.warning(msg) return result diff --git a/tests/test_canvas.py b/tests/test_canvas.py index 9cd0100d..d53b9005 100644 --- a/tests/test_canvas.py +++ b/tests/test_canvas.py @@ -1,5 +1,6 @@ """Tests for `nipyapi` package.""" +import logging import pytest import time import uuid @@ -936,7 +937,7 @@ def test_schedule_controller(fix_pg, fix_cont): assert r6.component.state == 'DISABLED' -def test_schedule_all_controllers(fix_pg, fix_cont): +def test_schedule_all_controllers(fix_pg, fix_cont, caplog): f_pg = fix_pg.generate() f_c1 = fix_cont(parent_pg=f_pg) f_c2 = fix_cont(parent_pg=f_pg) @@ -950,9 +951,11 @@ def test_schedule_all_controllers(fix_pg, fix_cont): with pytest.raises(ValueError): _ = canvas.schedule_all_controllers(f_pg.id, 'pie') - # Test bool True -> ENABLED - r1 = canvas.schedule_all_controllers(f_pg.id, True) + # Test bool True -> ENABLED (all controllers valid -> no skip warning) + with caplog.at_level(logging.WARNING, logger='nipyapi.canvas'): + r1 = canvas.schedule_all_controllers(f_pg.id, True) assert r1.state == 'ENABLED' + assert not any('non-ENABLED' in rec.message for rec in caplog.records) c1 = canvas.get_controller(f_c1.id, 'id') c2 = canvas.get_controller(f_c2.id, 'id') assert c1.component.state == 'ENABLED' @@ -983,11 +986,13 @@ def test_schedule_all_controllers(fix_pg, fix_cont): assert c2.component.state == 'DISABLED' -def test_schedule_all_controllers_with_invalid(fix_pg, fix_cont): +def test_schedule_all_controllers_with_invalid(fix_pg, fix_cont, caplog): """Test schedule_all_controllers skips INVALID controllers when enabling. NiFi server correctly skips INVALID controllers (they can't be enabled). - The client should only wait for VALID controllers to reach ENABLED state. + The client should only wait for VALID controllers to reach ENABLED state, + and (default strict=False) log a warning naming the skipped controllers + rather than returning success silently. """ f_pg = fix_pg.generate() # Create valid controller via fixture @@ -1001,9 +1006,16 @@ def test_schedule_all_controllers_with_invalid(fix_pg, fix_cont): assert valid_ctrl.component.validation_status == 'VALID' assert invalid_ctrl.component.validation_status == 'INVALID' - # Enable all - should complete without timeout - result = canvas.schedule_all_controllers(f_pg.id, True) + # Enable all - should complete without timeout and warn about the skipped + # INVALID controller (default strict=False does not raise) + with caplog.at_level(logging.WARNING, logger='nipyapi.canvas'): + result = canvas.schedule_all_controllers(f_pg.id, True) assert result.state == 'ENABLED' + assert any( + invalid_ctrl.id in rec.message and 'non-ENABLED' in rec.message + for rec in caplog.records + if rec.levelno == logging.WARNING + ) # Valid controller should be ENABLED, invalid stays DISABLED valid_ctrl = canvas.get_controller(valid_ctrl.id, 'id') @@ -1011,13 +1023,39 @@ def test_schedule_all_controllers_with_invalid(fix_pg, fix_cont): assert valid_ctrl.component.state == 'ENABLED' assert invalid_ctrl.component.state == 'DISABLED' - # Disable all - should also work + # Disable all - should also work (no warning on the disable path) result = canvas.schedule_all_controllers(f_pg.id, False) assert result.state == 'DISABLED' valid_ctrl = canvas.get_controller(valid_ctrl.id, 'id') assert valid_ctrl.component.state == 'DISABLED' +def test_schedule_all_controllers_strict(fix_pg, fix_cont): + """strict=True raises when a controller is left non-ENABLED after enabling. + + Default strict=False only warns (covered elsewhere); this verifies the + opt-in deterministic behaviour for automation. + """ + f_pg = fix_pg.generate() + valid_ctrl = fix_cont(parent_pg=f_pg, kind='CSVReader') + invalid_ctrl = fix_cont(parent_pg=f_pg, kind='StandardSSLContextService') + valid_ctrl = canvas.get_controller(valid_ctrl.id, 'id') + invalid_ctrl = canvas.get_controller(invalid_ctrl.id, 'id') + assert valid_ctrl.component.validation_status == 'VALID' + assert invalid_ctrl.component.validation_status == 'INVALID' + + # Default (strict=False) must not raise even with an INVALID controller + result = canvas.schedule_all_controllers(f_pg.id, True) + assert result.state == 'ENABLED' + + # Reset to a disabled baseline before the strict attempt + canvas.schedule_all_controllers(f_pg.id, False) + + # strict=True must raise because the INVALID controller cannot be enabled + with pytest.raises(ValueError): + _ = canvas.schedule_all_controllers(f_pg.id, True, strict=True) + + def test_delete_controller(fix_pg, fix_cont): f_pg = fix_pg.generate() f_c1 = fix_cont(parent_pg=f_pg)