mirror of
https://github.com/kubevela/kubevela.git
synced 2026-07-28 18:01:24 +00:00
Fix: vela status reports Healthy on workflowFailed with empty services (#7233)
* 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 <kampitojha@users.noreply.github.com>
* Style: fix import order in status_health_test.go
Signed-off-by: kampitojha <kampitojha@users.noreply.github.com>
* 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 <kampitojha@users.noreply.github.com>
* 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 <kampitojha@users.noreply.github.com>
---------
Signed-off-by: kampitojha <kampitojha@users.noreply.github.com>
Co-authored-by: kampitojha <kampitojha@users.noreply.github.com>
(cherry picked from commit ca9152164f)
This commit is contained in:
Kampit Ojha
committed by
github-actions[bot]
github-actions[bot]
parent
65dedda40a
commit
1500621f86
@@ -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
|
||||
|
||||
186
references/cli/status_health_test.go
Normal file
186
references/cli/status_health_test.go
Normal file
@@ -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)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user