Update deployment health on failed allocations only if health is unset #5645
Conversation
This fixes a confusing UX where a previously successful deployment's healthy/unhealthy count would get updated if any allocations failed after the deployment was already marked as successful.
| @@ -780,7 +780,7 @@ func TestAllocRunner_HandlesArtifactFailure(t *testing.T) { | |||
|
|
|||
| // Test that alloc runner kills tasks in task group when another task fails | |||
| func TestAllocRunner_TaskFailed_KillTG(t *testing.T) { | |||
| alloc := mock.BatchAlloc() | |||
| alloc := mock.Alloc() | |||
There was a problem hiding this comment.
I modified this existing test to change it to use a service job rather than batch so that it will get the DeploymentStatus field. Could use feedback on whether this is the best way to test this.
There was a problem hiding this comment.
I think this is asserting reasonable things, but I think it would have passed before?
Restoring a Failed alloc with a Healthy DeploymentStatus and asserting ar.AllocState().DeploymentStatus.IsHealthy() would have failed before but worked now I think.
There was a problem hiding this comment.
@schmichael is there an existing test you could point me to as an example? I am not able to get this test's DeploymentStatus to become healthy through the mock driver.
| @@ -780,7 +780,7 @@ func TestAllocRunner_HandlesArtifactFailure(t *testing.T) { | |||
|
|
|||
| // Test that alloc runner kills tasks in task group when another task fails | |||
| func TestAllocRunner_TaskFailed_KillTG(t *testing.T) { | |||
| alloc := mock.BatchAlloc() | |||
| alloc := mock.Alloc() | |||
There was a problem hiding this comment.
I think this is asserting reasonable things, but I think it would have passed before?
Restoring a Failed alloc with a Healthy DeploymentStatus and asserting ar.AllocState().DeploymentStatus.IsHealthy() would have failed before but worked now I think.
Co-Authored-By: preetapan <[email protected]>
Co-Authored-By: preetapan <[email protected]>
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This fixes a confusing UX where a previously successful deployment's
healthy/unhealthy count would get updated if any allocations failed after
the deployment was already marked as successful.