From 1500621f86fcc589d695a760a2382a5fca4748bb Mon Sep 17 00:00:00 2001 From: Kampit Ojha <97059622+kampitojha@users.noreply.github.com> Date: Tue, 14 Jul 2026 20:40:48 +0530 Subject: [PATCH] Fix: vela status reports Healthy on workflowFailed with empty services (#7233) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: vela status reports Healthy on workflowFailed with empty services getAppHealth returned true when status.services was empty (vacuous truth), so failed apps that never produced component status showed Healthy: ✅. Treat terminal/unhealthy phases, failed workflow steps, and empty services as unhealthy, with unit tests. Signed-off-by: kampitojha * Style: fix import order in status_health_test.go Signed-off-by: kampitojha * Fix: treat failed workflow substeps as unhealthy in vela status Address review feedback: scan SubStepsStatus for failed phases so step-group workflows do not report Healthy when a nested substep failed. Signed-off-by: kampitojha * Fix: satisfy exhaustive linter on ApplicationPhase switch Add default case so golangci exhaustive does not fail on intentional partial phase handling in getAppHealth. Signed-off-by: kampitojha --------- Signed-off-by: kampitojha Co-authored-by: kampitojha (cherry picked from commit ca9152164f75d512df6eb6fba199ee6e8014ef63) --- references/cli/status.go | 40 ++++++ references/cli/status_health_test.go | 186 +++++++++++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100644 references/cli/status_health_test.go diff --git a/references/cli/status.go b/references/cli/status.go index 0a6d26489..07a2e45bb 100644 --- a/references/cli/status.go +++ b/references/cli/status.go @@ -483,6 +483,46 @@ func getAppPhaseColor(appPhase commontypes.ApplicationPhase) *color.Color { } func getAppHealth(app *v1beta1.Application) bool { + // Terminal / unhealthy application phases must never report as healthy. + // This includes cases where no component services were recorded yet (e.g. CUE + // parameter errors fail the workflow before any service status is written). + // Previously empty Services made this function return true (vacuous truth), + // so `vela status` incorrectly showed Healthy: ✅ on workflowFailed apps. + switch app.Status.Phase { + case commontypes.ApplicationWorkflowFailed, + commontypes.ApplicationWorkflowTerminated, + commontypes.ApplicationUnhealthy, + commontypes.ApplicationDeleting: + return false + default: + // Other phases (starting/rendering/runningWorkflow/running/...) continue + // with service and workflow-step health checks below. + } + + // A failed workflow step (or substep in a step-group) means the app is not + // healthy, even while the controller is still retrying (phase may still be + // runningWorkflow). + if app.Status.Workflow != nil { + for i := range app.Status.Workflow.Steps { + step := &app.Status.Workflow.Steps[i] + if step.Phase == workflowv1alpha1.WorkflowStepPhaseFailed { + return false + } + for j := range step.SubStepsStatus { + if step.SubStepsStatus[j].Phase == workflowv1alpha1.WorkflowStepPhaseFailed { + return false + } + } + } + } + + // No service status yet: cannot claim the application is healthy. + // This covers intermediate phases and edge cases where phase is running + // but services were never populated. + if len(app.Status.Services) == 0 { + return false + } + for _, s := range app.Status.Services { if !s.Healthy { return false diff --git a/references/cli/status_health_test.go b/references/cli/status_health_test.go new file mode 100644 index 000000000..7075614c7 --- /dev/null +++ b/references/cli/status_health_test.go @@ -0,0 +1,186 @@ +/* +Copyright 2021 The KubeVela Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cli + +import ( + "testing" + + workflowv1alpha1 "github.com/kubevela/workflow/api/v1alpha1" + "github.com/stretchr/testify/require" + + "github.com/oam-dev/kubevela/apis/core.oam.dev/common" + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" +) + +func TestGetAppHealth(t *testing.T) { + r := require.New(t) + + healthyService := common.ApplicationComponentStatus{ + Name: "web", + Healthy: true, + } + unhealthyService := common.ApplicationComponentStatus{ + Name: "web", + Healthy: false, + } + unhealthyTrait := common.ApplicationComponentStatus{ + Name: "web", + Healthy: true, + Traits: []common.ApplicationTraitStatus{{ + Type: "scaler", + Healthy: false, + }}, + } + + tests := []struct { + name string + app *v1beta1.Application + want bool + }{ + { + name: "running with healthy services", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationRunning, + Services: []common.ApplicationComponentStatus{healthyService}, + }}, + want: true, + }, + { + name: "running with unhealthy service", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationRunning, + Services: []common.ApplicationComponentStatus{unhealthyService}, + }}, + want: false, + }, + { + name: "running with unhealthy trait", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationRunning, + Services: []common.ApplicationComponentStatus{unhealthyTrait}, + }}, + want: false, + }, + { + name: "workflowFailed with empty services must not be healthy", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationWorkflowFailed, + Services: nil, + Workflow: &common.WorkflowStatus{ + Finished: true, + Terminated: true, + Steps: []workflowv1alpha1.WorkflowStepStatus{{ + StepStatus: workflowv1alpha1.StepStatus{ + Name: "web", + Phase: workflowv1alpha1.WorkflowStepPhaseFailed, + }, + }}, + }, + }}, + want: false, + }, + { + name: "workflowTerminated with empty services must not be healthy", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationWorkflowTerminated, + }}, + want: false, + }, + { + name: "unhealthy phase must not be healthy", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationUnhealthy, + Services: []common.ApplicationComponentStatus{unhealthyService}, + }}, + want: false, + }, + { + name: "runningWorkflow with failed step and empty services must not be healthy", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationRunningWorkflow, + Workflow: &common.WorkflowStatus{ + Steps: []workflowv1alpha1.WorkflowStepStatus{{ + StepStatus: workflowv1alpha1.StepStatus{ + Name: "web", + Phase: workflowv1alpha1.WorkflowStepPhaseFailed, + }, + }}, + }, + }}, + want: false, + }, + { + name: "runningWorkflow empty services intermediate is not healthy", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationRunningWorkflow, + Services: nil, + }}, + want: false, + }, + { + name: "runningWorkflow wait healthy with unhealthy service", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationRunningWorkflow, + Services: []common.ApplicationComponentStatus{unhealthyService}, + }}, + want: false, + }, + { + name: "running with empty services is not healthy", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationRunning, + Services: nil, + }}, + want: false, + }, + { + name: "workflowSuspending with healthy services stays healthy", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationWorkflowSuspending, + Services: []common.ApplicationComponentStatus{healthyService}, + }}, + want: true, + }, + { + name: "failed substep in step-group must not be healthy even if services healthy", + app: &v1beta1.Application{Status: common.AppStatus{ + Phase: common.ApplicationRunningWorkflow, + Services: []common.ApplicationComponentStatus{healthyService}, + Workflow: &common.WorkflowStatus{ + Steps: []workflowv1alpha1.WorkflowStepStatus{{ + StepStatus: workflowv1alpha1.StepStatus{ + Name: "group", + Type: "step-group", + Phase: workflowv1alpha1.WorkflowStepPhaseRunning, + }, + SubStepsStatus: []workflowv1alpha1.StepStatus{{ + Name: "child", + Phase: workflowv1alpha1.WorkflowStepPhaseFailed, + }}, + }}, + }, + }}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r.Equal(tt.want, getAppHealth(tt.app), tt.name) + }) + } +}