diff --git a/internal/controller/kustomizationhealth_controller.go b/internal/controller/kustomizationhealth_controller.go index c6f4d06..06470f9 100644 --- a/internal/controller/kustomizationhealth_controller.go +++ b/internal/controller/kustomizationhealth_controller.go @@ -280,25 +280,24 @@ func (r *KustomizationHealthReconciler) checkKustomizationResourceHealth(ctx con } } -// getFailureConditionTime returns the LastTransitionTime of the most recent failure-indicating -// condition on the object. The timestamp accurately reflects when the failure actually occurred, -// letting the rollout controller compare it against deployment/retry time rather than "now". +// getFailureConditionTime returns a witness timestamp for the resource's failure +// state. Prefers Stalled=True (kstatus-standard authoritative failure signal); when +// absent, falls back to the newest lastTransitionTime across any condition so the +// caller still gets a stable witness instead of nil. Returns nil only when the +// object has no conditions with parseable timestamps; the caller falls back to now() +// in that case. func getFailureConditionTime(obj *unstructured.Unstructured) *metav1.Time { conditions, found, err := unstructured.NestedSlice(obj.Object, "status", "conditions") if err != nil || !found { return nil } - var latest *metav1.Time + var stalled *metav1.Time + var newest *metav1.Time for _, c := range conditions { condMap, ok := c.(map[string]interface{}) if !ok { continue } - condType, _, _ := unstructured.NestedString(condMap, "type") - condStatus, _, _ := unstructured.NestedString(condMap, "status") - if !isFailureCondition(condType, condStatus) { - continue - } tsStr, _, _ := unstructured.NestedString(condMap, "lastTransitionTime") if tsStr == "" { continue @@ -308,26 +307,21 @@ func getFailureConditionTime(obj *unstructured.Unstructured) *metav1.Time { continue } t := metav1.NewTime(ts) - if latest == nil || ts.After(latest.Time) { - latest = &t + condType, _, _ := unstructured.NestedString(condMap, "type") + condStatus, _, _ := unstructured.NestedString(condMap, "status") + if condType == "Stalled" && condStatus == "True" { + if stalled == nil || ts.After(stalled.Time) { + stalled = &t + } + } + if newest == nil || ts.After(newest.Time) { + newest = &t } } - return latest -} - -// isFailureCondition returns true when (condType, condStatus) indicates failure. -// Covers kstatus-standard conditions (Stalled, Ready) and common Kubernetes resource -// conditions including Deployments (Progressing=False, ReplicaFailure=True). -func isFailureCondition(condType, condStatus string) bool { - switch condType { - // True = problem - case "Stalled", "ReplicaFailure", "Degraded", "Failed": - return condStatus == "True" - // False = problem - case "Ready", "Available", "Progressing", "Healthy", "Synced": - return condStatus == "False" - } - return false + if stalled != nil { + return stalled + } + return newest } // updateHealthCheckStatus updates the HealthCheck status with proper timestamp handling. diff --git a/internal/controller/kustomizationhealth_controller_test.go b/internal/controller/kustomizationhealth_controller_test.go index 8669f77..7ea1e91 100644 --- a/internal/controller/kustomizationhealth_controller_test.go +++ b/internal/controller/kustomizationhealth_controller_test.go @@ -1132,35 +1132,6 @@ var _ = Describe("KustomizationHealth stale-failure guard", func() { }) }) -var _ = DescribeTable("isFailureCondition", - func(condType, condStatus string, expected bool) { - Expect(isFailureCondition(condType, condStatus)).To(Equal(expected)) - }, - // True = problem - Entry("Stalled=True", "Stalled", "True", true), - Entry("Stalled=False", "Stalled", "False", false), - Entry("ReplicaFailure=True", "ReplicaFailure", "True", true), - Entry("ReplicaFailure=False", "ReplicaFailure", "False", false), - Entry("Degraded=True", "Degraded", "True", true), - Entry("Degraded=False", "Degraded", "False", false), - Entry("Failed=True", "Failed", "True", true), - Entry("Failed=False", "Failed", "False", false), - // False = problem - Entry("Ready=False", "Ready", "False", true), - Entry("Ready=True", "Ready", "True", false), - Entry("Available=False", "Available", "False", true), - Entry("Available=True", "Available", "True", false), - Entry("Progressing=False", "Progressing", "False", true), - Entry("Progressing=True", "Progressing", "True", false), - Entry("Healthy=False", "Healthy", "False", true), - Entry("Healthy=True", "Healthy", "True", false), - Entry("Synced=False", "Synced", "False", true), - Entry("Synced=True", "Synced", "True", false), - // Unknown type - Entry("unknown type True", "SomeCondition", "True", false), - Entry("unknown type False", "SomeCondition", "False", false), -) - var _ = Describe("getFailureConditionTime", func() { makeObj := func(conditions []map[string]interface{}) *unstructured.Unstructured { obj := &unstructured.Unstructured{} @@ -1179,45 +1150,46 @@ var _ = Describe("getFailureConditionTime", func() { Expect(getFailureConditionTime(makeObj(nil))).To(BeNil()) }) - It("returns nil when all conditions are healthy", func() { + It("returns the newest condition timestamp when no Stalled=True exists", func() { obj := makeObj([]map[string]interface{}{ {"type": "Available", "status": "True", "lastTransitionTime": "2025-01-01T00:00:00Z"}, {"type": "Progressing", "status": "True", "lastTransitionTime": "2025-01-01T01:00:00Z"}, }) - Expect(getFailureConditionTime(obj)).To(BeNil()) + result := getFailureConditionTime(obj) + Expect(result).NotTo(BeNil()) + Expect(result.Time.UTC()).To(Equal(time.Date(2025, 1, 1, 1, 0, 0, 0, time.UTC))) }) - It("returns the timestamp of a single failure condition", func() { + It("prefers Stalled=True over a newer non-Stalled condition", func() { obj := makeObj([]map[string]interface{}{ - {"type": "Progressing", "status": "False", "lastTransitionTime": "2025-06-01T12:00:00Z"}, + {"type": "Stalled", "status": "True", "lastTransitionTime": "2025-06-01T10:00:00Z"}, + {"type": "Available", "status": "True", "lastTransitionTime": "2025-06-01T15:00:00Z"}, }) result := getFailureConditionTime(obj) Expect(result).NotTo(BeNil()) - Expect(result.Time.UTC()).To(Equal(time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC))) + Expect(result.Time.UTC()).To(Equal(time.Date(2025, 6, 1, 10, 0, 0, 0, time.UTC))) }) - It("returns the latest timestamp when multiple failure conditions exist", func() { + It("ignores Stalled=False and falls back to newest condition", func() { obj := makeObj([]map[string]interface{}{ + {"type": "Stalled", "status": "False", "lastTransitionTime": "2025-06-01T09:00:00Z"}, {"type": "Progressing", "status": "False", "lastTransitionTime": "2025-06-01T10:00:00Z"}, - {"type": "ReplicaFailure", "status": "True", "lastTransitionTime": "2025-06-01T11:00:00Z"}, }) result := getFailureConditionTime(obj) Expect(result).NotTo(BeNil()) - Expect(result.Time.UTC()).To(Equal(time.Date(2025, 6, 1, 11, 0, 0, 0, time.UTC))) + Expect(result.Time.UTC()).To(Equal(time.Date(2025, 6, 1, 10, 0, 0, 0, time.UTC))) }) - It("ignores healthy conditions when picking the latest", func() { - // Available=True transitions after the failure condition — must not override the result. + It("returns the timestamp of a single condition", func() { obj := makeObj([]map[string]interface{}{ - {"type": "Available", "status": "True", "lastTransitionTime": "2025-06-01T15:00:00Z"}, - {"type": "Progressing", "status": "False", "lastTransitionTime": "2025-06-01T10:00:00Z"}, + {"type": "Progressing", "status": "False", "lastTransitionTime": "2025-06-01T12:00:00Z"}, }) result := getFailureConditionTime(obj) Expect(result).NotTo(BeNil()) - Expect(result.Time.UTC()).To(Equal(time.Date(2025, 6, 1, 10, 0, 0, 0, time.UTC))) + Expect(result.Time.UTC()).To(Equal(time.Date(2025, 6, 1, 12, 0, 0, 0, time.UTC))) }) - It("returns nil when failure condition has no lastTransitionTime", func() { + It("returns nil when conditions have no parseable lastTransitionTime", func() { obj := makeObj([]map[string]interface{}{ {"type": "Progressing", "status": "False"}, })