Fix deployment marked as failed when healthy container completes rolling update
Prevent deployment status from regressing to FAILED after it's marked as FINISHED by: 1. Calling completeDeployment() first in post_deployment() before any operations that could fail 2. Wrapping all post-deployment side effects in try-catch blocks 3. Adding FINISHED to terminal states that cannot be changed 4. Protecting ExecuteRemoteCommand from overwriting FINISHED status This fixes the issue where a deployment with a healthy container and successful rolling update was still marked as Failed in the UI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
d2a1b96598
commit
a2e5b2d67d
2 changed files with 38 additions and 7 deletions
|
|
@ -486,15 +486,38 @@ private function decide_what_to_do()
|
||||||
|
|
||||||
private function post_deployment()
|
private function post_deployment()
|
||||||
{
|
{
|
||||||
GetContainersStatus::dispatch($this->server);
|
// Mark deployment as complete FIRST, before any other operations
|
||||||
|
// This ensures the deployment status is FINISHED even if subsequent operations fail
|
||||||
$this->completeDeployment();
|
$this->completeDeployment();
|
||||||
|
|
||||||
|
// Then handle side effects - these should not fail the deployment
|
||||||
|
try {
|
||||||
|
GetContainersStatus::dispatch($this->server);
|
||||||
|
} catch (\Exception $e) {
|
||||||
|
\Log::warning('Failed to dispatch GetContainersStatus for deployment '.$this->deployment_uuid.': '.$e->getMessage());
|
||||||
|
}
|
||||||
|
|
||||||
if ($this->pull_request_id !== 0) {
|
if ($this->pull_request_id !== 0) {
|
||||||
if ($this->application->is_github_based()) {
|
if ($this->application->is_github_based()) {
|
||||||
ApplicationPullRequestUpdateJob::dispatch(application: $this->application, preview: $this->preview, deployment_uuid: $this->deployment_uuid, status: ProcessStatus::FINISHED);
|
try {
|
||||||
|
ApplicationPullRequestUpdateJob::dispatch(application: $this->application, preview: $this->preview, deployment_uuid: $this->deployment_uuid, status: ProcessStatus::FINISHED);
|
||||||
|
} catch (\Exception $e) {
|
||||||
|
\Log::warning('Failed to dispatch PR update for deployment '.$this->deployment_uuid.': '.$e->getMessage());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
$this->run_post_deployment_command();
|
|
||||||
$this->application->isConfigurationChanged(true);
|
try {
|
||||||
|
$this->run_post_deployment_command();
|
||||||
|
} catch (\Exception $e) {
|
||||||
|
\Log::warning('Post deployment command failed for '.$this->deployment_uuid.': '.$e->getMessage());
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
$this->application->isConfigurationChanged(true);
|
||||||
|
} catch (\Exception $e) {
|
||||||
|
\Log::warning('Failed to mark configuration as changed for deployment '.$this->deployment_uuid.': '.$e->getMessage());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private function deploy_simple_dockerfile()
|
private function deploy_simple_dockerfile()
|
||||||
|
|
@ -3934,13 +3957,17 @@ private function transitionToStatus(ApplicationDeploymentStatus $status): void
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check if deployment is in a terminal state (FAILED or CANCELLED).
|
* Check if deployment is in a terminal state (FINISHED, FAILED or CANCELLED).
|
||||||
* Terminal states cannot be changed.
|
* Terminal states cannot be changed.
|
||||||
*/
|
*/
|
||||||
private function isInTerminalState(): bool
|
private function isInTerminalState(): bool
|
||||||
{
|
{
|
||||||
$this->application_deployment_queue->refresh();
|
$this->application_deployment_queue->refresh();
|
||||||
|
|
||||||
|
if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::FINISHED->value) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::FAILED->value) {
|
if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::FAILED->value) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -140,9 +140,13 @@ public function execute_remote_command(...$commands)
|
||||||
// If we exhausted all retries and still failed
|
// If we exhausted all retries and still failed
|
||||||
if (! $commandExecuted && $lastError) {
|
if (! $commandExecuted && $lastError) {
|
||||||
// Now we can set the status to FAILED since all retries have been exhausted
|
// Now we can set the status to FAILED since all retries have been exhausted
|
||||||
|
// But only if the deployment hasn't already been marked as FINISHED
|
||||||
if (isset($this->application_deployment_queue)) {
|
if (isset($this->application_deployment_queue)) {
|
||||||
$this->application_deployment_queue->status = ApplicationDeploymentStatus::FAILED->value;
|
$this->application_deployment_queue->refresh();
|
||||||
$this->application_deployment_queue->save();
|
if ($this->application_deployment_queue->status !== ApplicationDeploymentStatus::FINISHED->value) {
|
||||||
|
$this->application_deployment_queue->status = ApplicationDeploymentStatus::FAILED->value;
|
||||||
|
$this->application_deployment_queue->save();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
throw $lastError;
|
throw $lastError;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue