mirror of
https://github.com/kubevela/kubevela.git
synced 2026-08-19 04:26:39 +00:00
Fix(controller): set Ready condition to False on reconcile failure (#7168)
* fix(controller): set Ready condition to False on reconcile failure endWithNegativeCondition set only the failing sub-condition (e.g. Parsed=False/ReconcileError) but left the rollup Ready condition unchanged, so a reconcile failure left Ready at True/ReconcileSuccess from the last successful reconcile. Health checkers polling Ready as the application health signal missed the failure. Also set Ready=False/ReconcileError alongside the sub-condition with the same message, so the rollup reflects the failure on every path that ends through endWithNegativeCondition. Fixes #7164 Signed-off-by: Anuragp22 <anuragp2003b@gmail.com> * test(controller): assert Ready condition propagates sub-condition message Address cubic review on #7168: verify the persisted Ready condition's Message field equals the failing sub-condition's Message, so a future refactor that drops the message propagation in endWithNegativeCondition gets caught by the test. Signed-off-by: Anuragp22 <anuragp2003b@gmail.com> --------- Signed-off-by: Anuragp22 <anuragp2003b@gmail.com>
This commit is contained in:
@@ -527,12 +527,20 @@ func (r *Reconciler) handleFinalizers(ctx monitorContext.Context, app *v1beta1.A
|
||||
return r.result(nil).end(false)
|
||||
}
|
||||
|
||||
func (r *Reconciler) endWithNegativeCondition(ctx context.Context, app *v1beta1.Application, condition condition.Condition, phase common.ApplicationPhase) (ctrl.Result, error) {
|
||||
app.SetConditions(condition)
|
||||
func (r *Reconciler) endWithNegativeCondition(ctx context.Context, app *v1beta1.Application, cond condition.Condition, phase common.ApplicationPhase) (ctrl.Result, error) {
|
||||
// Flip the rollup Ready condition alongside the failing sub-condition so health checkers
|
||||
// polling Ready see the failure instead of the last successful reconcile (#7164).
|
||||
app.SetConditions(cond, condition.Condition{
|
||||
Type: condition.ConditionType(common.ReadyCondition.String()),
|
||||
Status: corev1.ConditionFalse,
|
||||
LastTransitionTime: metav1.Now(),
|
||||
Reason: condition.ReasonReconcileError,
|
||||
Message: cond.Message,
|
||||
})
|
||||
if err := r.patchStatus(ctx, app, phase); err != nil {
|
||||
return r.result(errors.WithMessage(err, "cannot update application status")).ret()
|
||||
}
|
||||
return r.result(fmt.Errorf("object level reconcile error, type: %q, msg: %q", string(condition.Type), condition.Message)).ret()
|
||||
return r.result(fmt.Errorf("object level reconcile error, type: %q, msg: %q", string(cond.Type), cond.Message)).ret()
|
||||
}
|
||||
|
||||
// Application status can be updated by two methods: patch and update.
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
/*
|
||||
Copyright 2026 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 application
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/types"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client/fake"
|
||||
|
||||
"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
|
||||
"github.com/oam-dev/kubevela/apis/core.oam.dev/condition"
|
||||
"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
|
||||
)
|
||||
|
||||
// Regression test for kubevela/kubevela#7164: when a reconcile path ends in a
|
||||
// negative sub-condition (e.g. Parsed=False/ReconcileError), the rollup Ready
|
||||
// condition must also flip to False. Previously Ready stayed True from the last
|
||||
// successful reconcile, so health checkers polling Ready missed the failure.
|
||||
func TestEndWithNegativeConditionFlipsReady(t *testing.T) {
|
||||
readyType := condition.ConditionType(common.ReadyCondition.String())
|
||||
|
||||
app := &v1beta1.Application{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "app-7164", Namespace: "default"},
|
||||
Status: common.AppStatus{
|
||||
ConditionedStatus: condition.ConditionedStatus{
|
||||
Conditions: []condition.Condition{{
|
||||
Type: readyType,
|
||||
Status: corev1.ConditionTrue,
|
||||
Reason: condition.ReasonReconcileSuccess,
|
||||
LastTransitionTime: metav1.Now(),
|
||||
}},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
cli := fake.NewClientBuilder().
|
||||
WithScheme(newTestScheme(t)).
|
||||
WithStatusSubresource(&v1beta1.Application{}).
|
||||
WithObjects(app).
|
||||
Build()
|
||||
r := &Reconciler{Client: cli}
|
||||
|
||||
parsedFailure := condition.ErrorCondition("Parsed", errors.New("ComponentDefinition not found"))
|
||||
_, _ = r.endWithNegativeCondition(context.Background(), app, parsedFailure, common.ApplicationRendering)
|
||||
|
||||
persisted := &v1beta1.Application{}
|
||||
if err := cli.Get(context.Background(), types.NamespacedName{Name: app.Name, Namespace: app.Namespace}, persisted); err != nil {
|
||||
t.Fatalf("failed to read Application back from storage: %v", err)
|
||||
}
|
||||
|
||||
gotReady := findCondition(persisted.Status.Conditions, readyType)
|
||||
assert.NotNil(t, gotReady, "Ready condition must be set after negative reconcile")
|
||||
assert.Equal(t, corev1.ConditionFalse, gotReady.Status, "Ready must be False on reconcile failure")
|
||||
assert.Equal(t, condition.ReasonReconcileError, gotReady.Reason, "Ready reason must be ReconcileError")
|
||||
assert.Equal(t, parsedFailure.Message, gotReady.Message, "Ready message must mirror the failing sub-condition's message")
|
||||
}
|
||||
|
||||
func findCondition(conds []condition.Condition, t condition.ConditionType) *condition.Condition {
|
||||
for i := range conds {
|
||||
if conds[i].Type == t {
|
||||
return &conds[i]
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
Reference in New Issue
Block a user