From 96f2ca7cecfca43000ddba929fc1bc348c845b27 Mon Sep 17 00:00:00 2001 From: Omer Date: Tue, 28 Jun 2022 15:25:03 +0200 Subject: [PATCH] Fix HealthMonitorToErrorOnRevertTask revert method So far, whenever we had to revert HealthMonitorToErrorOnRevertTask, we called mark_health_mon_prov_status_error with the wrong parameter (health_mon[constants.POOL_ID] == the pool id). This would set the pool's provisioning status to ERROR. Later in the revert task we call mark_pool_prov_status_active with the correct argument (health_mon[constants.POOL_ID] == the pool id), so we set it back to ACTIVE, and the task continues correctly. This means we might have missed setting the health monitor's provisioning status to ERROR, and therefore we might have missed some errors. This commit changes the parameter to be the correct one in both API v1 and v2, and it modifies the tests accordingly. Story 2010113 Task 45690 Change-Id: I5a749c751d184ec10957a31b569419d57fe8daf6 --- octavia/controller/worker/v1/tasks/lifecycle_tasks.py | 2 +- octavia/controller/worker/v2/tasks/lifecycle_tasks.py | 2 +- .../unit/controller/worker/v1/tasks/test_lifecycle_tasks.py | 2 +- .../unit/controller/worker/v2/tasks/test_lifecycle_tasks.py | 2 +- ...ealth-monitor-to-error-revert-task-feb38ba7641a4892.yaml | 6 ++++++ 5 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/fix-health-monitor-to-error-revert-task-feb38ba7641a4892.yaml diff --git a/octavia/controller/worker/v1/tasks/lifecycle_tasks.py b/octavia/controller/worker/v1/tasks/lifecycle_tasks.py index d46e462fae..41b8288ba3 100644 --- a/octavia/controller/worker/v1/tasks/lifecycle_tasks.py +++ b/octavia/controller/worker/v1/tasks/lifecycle_tasks.py @@ -52,7 +52,7 @@ class HealthMonitorToErrorOnRevertTask(BaseLifecycleTask): pass def revert(self, health_mon, listeners, loadbalancer, *args, **kwargs): - self.task_utils.mark_health_mon_prov_status_error(health_mon.pool_id) + self.task_utils.mark_health_mon_prov_status_error(health_mon.id) self.task_utils.mark_pool_prov_status_active(health_mon.pool_id) self.task_utils.mark_loadbalancer_prov_status_active(loadbalancer.id) for listener in listeners: diff --git a/octavia/controller/worker/v2/tasks/lifecycle_tasks.py b/octavia/controller/worker/v2/tasks/lifecycle_tasks.py index db521291e8..8e1ad46ea0 100644 --- a/octavia/controller/worker/v2/tasks/lifecycle_tasks.py +++ b/octavia/controller/worker/v2/tasks/lifecycle_tasks.py @@ -55,7 +55,7 @@ class HealthMonitorToErrorOnRevertTask(BaseLifecycleTask): def revert(self, health_mon, listeners, loadbalancer, *args, **kwargs): self.task_utils.mark_health_mon_prov_status_error( - health_mon[constants.POOL_ID]) + health_mon[constants.HEALTHMONITOR_ID]) self.task_utils.mark_pool_prov_status_active( health_mon[constants.POOL_ID]) self.task_utils.mark_loadbalancer_prov_status_active( diff --git a/octavia/tests/unit/controller/worker/v1/tasks/test_lifecycle_tasks.py b/octavia/tests/unit/controller/worker/v1/tasks/test_lifecycle_tasks.py index 82348db9ad..a766e619ac 100644 --- a/octavia/tests/unit/controller/worker/v1/tasks/test_lifecycle_tasks.py +++ b/octavia/tests/unit/controller/worker/v1/tasks/test_lifecycle_tasks.py @@ -28,7 +28,7 @@ class TestLifecycleTasks(base.TestCase): self.AMPHORA.id = self.AMPHORA_ID self.HEALTH_MON = mock.MagicMock() self.HEALTH_MON_ID = uuidutils.generate_uuid() - self.HEALTH_MON.pool_id = self.HEALTH_MON_ID + self.HEALTH_MON.id = self.HEALTH_MON_ID self.L7POLICY = mock.MagicMock() self.L7POLICY_ID = uuidutils.generate_uuid() self.L7POLICY.id = self.L7POLICY_ID diff --git a/octavia/tests/unit/controller/worker/v2/tasks/test_lifecycle_tasks.py b/octavia/tests/unit/controller/worker/v2/tasks/test_lifecycle_tasks.py index cbeb54663a..7c14f6fbe5 100644 --- a/octavia/tests/unit/controller/worker/v2/tasks/test_lifecycle_tasks.py +++ b/octavia/tests/unit/controller/worker/v2/tasks/test_lifecycle_tasks.py @@ -126,7 +126,7 @@ class TestLifecycleTasks(base.TestCase): self.LOADBALANCER) mock_health_mon_prov_status_error.assert_called_once_with( - self.POOL_ID) + self.HEALTH_MON_ID) mock_loadbalancer_prov_status_active.assert_called_once_with( self.LOADBALANCER_ID) mock_listener_prov_status_active.assert_called_once_with( diff --git a/releasenotes/notes/fix-health-monitor-to-error-revert-task-feb38ba7641a4892.yaml b/releasenotes/notes/fix-health-monitor-to-error-revert-task-feb38ba7641a4892.yaml new file mode 100644 index 0000000000..2ff75aa5fb --- /dev/null +++ b/releasenotes/notes/fix-health-monitor-to-error-revert-task-feb38ba7641a4892.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fix a bug that prevented the provisioning_state of a health-monitor to be + set to ERROR when an error occurred while creating, updating or deleting a + health-monitor. \ No newline at end of file