Fix unreachable success log in EksDeleteClusterOperator.execute_complete - #73109
abhinav-phi wants to merge 1 commit into
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
|
| def test_execute_complete_when_trigger_yields_deleted(self, caplog): | ||
| # EksDeleteClusterTrigger yields {"status": "deleted"}, not "success". | ||
| self.delete_cluster_operator.execute_complete(context={}, event={"status": "deleted"}) | ||
| assert "Cluster deleted successfully." in caplog.messages |
There was a problem hiding this comment.
Good catch — switched to the structured-membership assertion: assert "Cluster deleted successfully." in caplog (exact match on the captured event field, since caplog is backed by the StructlogCapture fixture now). All 9 tests in the class pass locally; amended into the single commit and rebased onto main. Thanks!
46bba12 to
66bb7ed
Compare
| def test_execute_complete_when_trigger_yields_deleted(self, caplog): | ||
| # EksDeleteClusterTrigger yields {"status": "deleted"}, not "success". | ||
| self.delete_cluster_operator.execute_complete(context={}, event={"status": "deleted"}) | ||
| assert "Cluster deleted successfully." in caplog.messages |
There was a problem hiding this comment.
Good catch — switched to the structured-membership assertion: assert "Cluster deleted successfully." in caplog (exact match on the captured event field, since caplog is backed by the StructlogCapture fixture now). All 9 tests in the class pass locally; amended into the single commit and rebased onto main. Thanks!
Sub-item of #72982 (EKS trigger/operator status mismatch).
Root cause:
EksDeleteClusterTriggeryields{"status": "deleted"}when the cluster is gone, butEksDeleteClusterOperator.execute_completeonly logs the success message when the status is"success"— so the "Cluster deleted successfully." log line was unreachable. The trigger's other consumer,EksCreateClusterOperator.execute_failed, already checks for"deleted"correctly, which is why the fix goes on the operator side: changing the trigger's yielded status would break that path.Changes
providers/amazon/src/airflow/providers/amazon/aws/operators/eks.py: accept the"deleted"status the trigger actually yields.providers/amazon/tests/unit/amazon/aws/operators/test_eks.py:test_execute_complete_when_trigger_yields_deletedasserts the success log fires for the trigger's real event.Testing
TestEksDeleteClusterOperator(9 tests) + the full EKS trigger suite (tests/unit/amazon/aws/triggers/test_eks.py, 18 tests): 27 passed locally.ruff checkandruff format --checkclean on both files.I checked the other EKS trigger/operator pairs (create cluster/nodegroup/fargate profile, delete nodegroup/fargate profile): their
AwsBaseWaiterTriggerevents carry"success"/"error"and the operators match, so this was the only mismatch in the EKS family.