From 3d6648be5d54d2b80829fb6363a4c4c86b30845e Mon Sep 17 00:00:00 2001 From: Ashish Date: Tue, 4 Aug 2026 18:40:07 +0530 Subject: [PATCH 1/4] feat(stackresource): publish rolled-up status.summary verdict The agent now publishes its conclusion, not just its evidence: a summary block (state Waiting|Building|Deploying|Ready|Failed + reason + message + observedGeneration) written by the status derivation every pass, so the hub maps it 1:1 to release timeline events instead of re-deriving state from condition order, values, absence, PortCheck and failure details. - summarize() in status derivation, single writer, stamped before StatusHash so the hash covers it; precedence: Failed verdict > Building > Waiting > runtime crash > Ready/Deploying - Deploying detail picks port dial > readiness detail > verdict message, with a "previous revision still serving traffic" note while the old revision holds traffic - Exported ReasonBuildFailed / ReasonPortNotListening; literals replaced - BuildLastFailureDetail now stamps Type=runtime_crash (was unset; readers no longer normalize empty Type) - CRDs regenerated in config/deploy/crds and chart crds (identical) Claude-Session: https://claude.ai/code/session_01TpRLszVTJudSd86RreufSC --- api/core/v1alpha1/stack_resource_types.go | 34 +++ api/core/v1alpha1/status_hash_test.go | 11 + api/core/v1alpha1/zz_generated.deepcopy.go | 20 ++ .../core.stackdome.io_stackresources.yaml | 27 ++ .../core.stackdome.io_stackresources.yaml | 27 ++ .../stackresource/image_build_reconciler.go | 4 +- .../controller/stackresource/status_derive.go | 121 ++++++++- .../stackresource/status_derive_test.go | 241 ++++++++++++++++++ .../stackresource/workload/readiness.go | 6 +- internal/controller/util.go | 3 + 10 files changed, 480 insertions(+), 14 deletions(-) diff --git a/api/core/v1alpha1/stack_resource_types.go b/api/core/v1alpha1/stack_resource_types.go index b404719..5e8d958 100644 --- a/api/core/v1alpha1/stack_resource_types.go +++ b/api/core/v1alpha1/stack_resource_types.go @@ -478,6 +478,9 @@ type StackResourceStatus struct { // memory) also keeps the grace window intact across operator restarts. // +optional PortCheck *PortCheckStatus `json:"portCheck,omitempty"` + // Summary is the agent's rolled-up verdict, written every pass. + // +optional + Summary *StackResourceStatusSummary `json:"summary,omitempty"` } type PortCheckStatusType string @@ -529,6 +532,37 @@ type LastFailureDetail struct { LastTerminationExitCode *int32 `json:"lastTerminationExitCode,omitempty"` } +// StackResourceSummaryState is the state half of the summary verdict. +type StackResourceSummaryState string + +const ( + SummaryStateWaiting StackResourceSummaryState = "Waiting" + SummaryStateBuilding StackResourceSummaryState = "Building" + SummaryStateDeploying StackResourceSummaryState = "Deploying" + SummaryStateReady StackResourceSummaryState = "Ready" + SummaryStateFailed StackResourceSummaryState = "Failed" +) + +// Reasons the hub branches on. Exported so consumers never mirror literals. +const ( + ReasonBuildFailed = "BuildFailed" + ReasonPortNotListening = "PortNotListening" +) + +// StackResourceStatusSummary is the agent's rolled-up verdict for the +// resource: what it is doing right now and why. Single writer: the status +// derivation. The hub maps it 1:1 to release timeline events. +type StackResourceStatusSummary struct { + // +kubebuilder:validation:Enum=Waiting;Building;Deploying;Ready;Failed + State StackResourceSummaryState `json:"state"` + // +optional + Reason string `json:"reason,omitempty"` + // +optional + Message string `json:"message,omitempty"` + // ObservedGeneration is the generation this verdict was derived for. + ObservedGeneration int64 `json:"observedGeneration"` +} + // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:resource:shortName=sr diff --git a/api/core/v1alpha1/status_hash_test.go b/api/core/v1alpha1/status_hash_test.go index e2320e4..2e5e155 100644 --- a/api/core/v1alpha1/status_hash_test.go +++ b/api/core/v1alpha1/status_hash_test.go @@ -42,6 +42,17 @@ func TestStackResourceStatusHashChangesOnPhaseChange(t *testing.T) { } } +func TestStackResourceStatusHashChangesOnSummaryChange(t *testing.T) { + a := &StackResource{Status: StackResourceStatus{ + Summary: &StackResourceStatusSummary{State: SummaryStateDeploying, ObservedGeneration: 1}, + }} + b := a.DeepCopy() + b.Status.Summary.State = SummaryStateReady + if a.StatusHash() == b.StatusHash() { + t.Fatal("summary change must change the status hash") + } +} + func TestStackStatusHashDoesNotMutateReceiver(t *testing.T) { s := &Stack{Status: StackStatus{Conditions: conditionsFixture()}} _ = s.StatusHash() diff --git a/api/core/v1alpha1/zz_generated.deepcopy.go b/api/core/v1alpha1/zz_generated.deepcopy.go index d62e6de..090df2b 100644 --- a/api/core/v1alpha1/zz_generated.deepcopy.go +++ b/api/core/v1alpha1/zz_generated.deepcopy.go @@ -1162,6 +1162,11 @@ func (in *StackResourceStatus) DeepCopyInto(out *StackResourceStatus) { *out = new(PortCheckStatus) (*in).DeepCopyInto(*out) } + if in.Summary != nil { + in, out := &in.Summary, &out.Summary + *out = new(StackResourceStatusSummary) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StackResourceStatus. @@ -1174,6 +1179,21 @@ func (in *StackResourceStatus) DeepCopy() *StackResourceStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *StackResourceStatusSummary) DeepCopyInto(out *StackResourceStatusSummary) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StackResourceStatusSummary. +func (in *StackResourceStatusSummary) DeepCopy() *StackResourceStatusSummary { + if in == nil { + return nil + } + out := new(StackResourceStatusSummary) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StackResourceSummary) DeepCopyInto(out *StackResourceSummary) { *out = *in diff --git a/charts/stackdome-agent-standalone/crds/core.stackdome.io_stackresources.yaml b/charts/stackdome-agent-standalone/crds/core.stackdome.io_stackresources.yaml index 309d41a..b373b25 100644 --- a/charts/stackdome-agent-standalone/crds/core.stackdome.io_stackresources.yaml +++ b/charts/stackdome-agent-standalone/crds/core.stackdome.io_stackresources.yaml @@ -1005,6 +1005,33 @@ spec: type: integer statusHash: type: string + summary: + description: Summary is the agent's rolled-up verdict, written every + pass. + properties: + message: + type: string + observedGeneration: + description: ObservedGeneration is the generation this verdict + was derived for. + format: int64 + type: integer + reason: + type: string + state: + description: StackResourceSummaryState is the state half of the + summary verdict. + enum: + - Waiting + - Building + - Deploying + - Ready + - Failed + type: string + required: + - observedGeneration + - state + type: object updatedReplicas: format: int32 type: integer diff --git a/config/deploy/crds/core.stackdome.io_stackresources.yaml b/config/deploy/crds/core.stackdome.io_stackresources.yaml index 309d41a..b373b25 100644 --- a/config/deploy/crds/core.stackdome.io_stackresources.yaml +++ b/config/deploy/crds/core.stackdome.io_stackresources.yaml @@ -1005,6 +1005,33 @@ spec: type: integer statusHash: type: string + summary: + description: Summary is the agent's rolled-up verdict, written every + pass. + properties: + message: + type: string + observedGeneration: + description: ObservedGeneration is the generation this verdict + was derived for. + format: int64 + type: integer + reason: + type: string + state: + description: StackResourceSummaryState is the state half of the + summary verdict. + enum: + - Waiting + - Building + - Deploying + - Ready + - Failed + type: string + required: + - observedGeneration + - state + type: object updatedReplicas: format: int32 type: integer diff --git a/internal/controller/stackresource/image_build_reconciler.go b/internal/controller/stackresource/image_build_reconciler.go index 75a1e45..b81e5eb 100644 --- a/internal/controller/stackresource/image_build_reconciler.go +++ b/internal/controller/stackresource/image_build_reconciler.go @@ -73,8 +73,8 @@ func (r *imageBuildReconciler) reconcile(ctx context.Context, resource *v1alpha1 } if imageBuildFailed(existingImageBuild) { - setResourceCondition(resource, v1alpha1.StackResourceBuildReady, false, "BuildFailed", "application build failed terminally") - reportFailed(ctx, "BuildFailed", + setResourceCondition(resource, v1alpha1.StackResourceBuildReady, false, v1alpha1.ReasonBuildFailed, "application build failed terminally") + reportFailed(ctx, v1alpha1.ReasonBuildFailed, fmt.Sprintf("ImageBuild %s reached terminal Failed phase", existingImageBuild.Name)) return resultStop, nil } diff --git a/internal/controller/stackresource/status_derive.go b/internal/controller/stackresource/status_derive.go index cf00312..38488cc 100644 --- a/internal/controller/stackresource/status_derive.go +++ b/internal/controller/stackresource/status_derive.go @@ -2,6 +2,8 @@ package stackresource import ( "fmt" + "strconv" + "strings" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -11,7 +13,8 @@ import ( ) // deriveSummaryStatus is the ONLY writer of the summary status (Available, -// Stalled, Converged, Phase). Mirrors stack/aggregate.go one level down. +// Stalled, Converged, Phase, Summary). Mirrors stack/aggregate.go one level +// down. // // - Sub-reconcilers contribute domain conditions (WorkloadAvailable, // WorkloadConverged, BuildReady, ...) and pass-scoped verdicts. This runs @@ -33,6 +36,7 @@ func deriveSummaryStatus(resource *v1alpha1.StackResource, verdicts *controller. deriveNoVerdict(resource) } + resource.Status.Summary = summarize(resource, verdicts) resource.Status.StatusHash = resource.StatusHash() } @@ -70,7 +74,7 @@ func deriveNoVerdict(resource *v1alpha1.StackResource) { resource.Status.ImageSourceRevision = resource.Spec.BuildSpec.SourceRevision.GetSourceRevisionString() } - converged, convergedReason, convergedMsg := domainCondition(resource, + converged, convergedReason, convergedMsg := domainConditionText(resource, v1alpha1.StackResourceWorkloadConverged, "WorkloadNotConverged", "workload has not converged") setSummaryCondition(resource, v1alpha1.StackResourceConverged, converged, convergedReason, convergedMsg) @@ -113,11 +117,11 @@ func setSummaryCondition(resource *v1alpha1.StackResource, condType v1alpha1.Sta }) } -// domainCondition reads a domain condition and the text it carried. A condition -// from an older generation describes a rollout that no longer exists, so it -// reads as false with the fallback text — neither its status nor its reason may -// leak into the summary. -func domainCondition(resource *v1alpha1.StackResource, condType v1alpha1.StackResourceDomainCondition, +// domainConditionText reads a domain condition and the text it carried. A +// condition from an older generation describes a rollout that no longer exists, +// so it reads as false with the fallback text — neither its status nor its +// reason may leak into the summary. +func domainConditionText(resource *v1alpha1.StackResource, condType v1alpha1.StackResourceDomainCondition, falseReason, falseMsg string) (bool, string, string) { c := meta.FindStatusCondition(resource.Status.Conditions, string(condType)) if c == nil || c.ObservedGeneration != resource.Generation { @@ -126,10 +130,109 @@ func domainCondition(resource *v1alpha1.StackResource, condType v1alpha1.StackRe return c.Status == metav1.ConditionTrue, c.Reason, c.Message } +// domainCondition returns the domain condition only when it has the given +// status at the current generation; nil otherwise. +func domainCondition(resource *v1alpha1.StackResource, condType v1alpha1.StackResourceDomainCondition, status metav1.ConditionStatus) *metav1.Condition { + c := meta.FindStatusCondition(resource.Status.Conditions, string(condType)) + if c == nil || c.Status != status || c.ObservedGeneration != resource.Generation { + return nil + } + return c +} + // domainConditionTrue reports whether a domain condition is True at the current // generation. A stale True counts as false: it must not soften a Failed verdict // or promote Ready. func domainConditionTrue(resource *v1alpha1.StackResource, condType v1alpha1.StackResourceDomainCondition) bool { - isTrue, _, _ := domainCondition(resource, condType, "", "") - return isTrue + return domainCondition(resource, condType, metav1.ConditionTrue) != nil +} + +// summarize rolls the pass up into one verdict. Precedence: terminal failure, +// build in progress, waiting on dependencies, runtime crash, then the rollout +// itself. Build/deps outrank the crash detail: when those gates fail the +// workload reconciler never ran this pass, so a crash entry is a previous +// revision's leftover. +func summarize(resource *v1alpha1.StackResource, verdicts *controller.VerdictCollector) *v1alpha1.StackResourceStatusSummary { + summary := &v1alpha1.StackResourceStatusSummary{ObservedGeneration: resource.Generation} + if v := verdicts.Failed(); v != nil { + summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateFailed, v.Reason, v.Message + return summary + } + notReady := verdicts.NotReady() + if notReady != nil { + if cond := domainCondition(resource, v1alpha1.StackResourceBuildReady, metav1.ConditionFalse); cond != nil { + summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateBuilding, cond.Reason, cond.Message + return summary + } + if cond := domainCondition(resource, v1alpha1.StackResourceDependenciesReady, metav1.ConditionFalse); cond != nil { + summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateWaiting, cond.Reason, cond.Message + return summary + } + if d := failureDetail(resource, v1alpha1.FailureTypeRuntimeCrash); d != nil { + summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateFailed, d.LastTerminationReason, d.LastTerminationMessage + return summary + } + } else if wc := domainCondition(resource, v1alpha1.StackResourceWorkloadConverged, metav1.ConditionTrue); wc != nil && + domainConditionTrue(resource, v1alpha1.StackResourceWorkloadAvailable) { + summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateReady, wc.Reason, wc.Message + return summary + } + summary.State = v1alpha1.SummaryStateDeploying + summary.Reason, summary.Message = deployingDetail(resource, notReady) + if domainConditionTrue(resource, v1alpha1.StackResourceWorkloadAvailable) { + if summary.Message == "" { + summary.Message = "previous revision still serving traffic" + } else { + summary.Message += " (previous revision still serving traffic)" + } + } + return summary +} + +// deployingDetail picks the most specific diagnosis of why the new pods are +// not ready: the last port dial, else the kubelet readiness detail, else the +// verdict (nil on the no-verdict path — fall back to WorkloadConverged). +func deployingDetail(resource *v1alpha1.StackResource, v *controller.Verdict) (reason, message string) { + if pc := resource.Status.PortCheck; pc != nil && pc.Status == v1alpha1.PortCheckStatusTypeFailure { + return v1alpha1.ReasonPortNotListening, portDialMessage(pc.FailingPortNumbers) + } + if resource.Status.PortCheck == nil { + if d := failureDetail(resource, v1alpha1.FailureTypeReadinessFailure); d != nil { + return d.LastTerminationReason, d.LastTerminationMessage + } + } + if v != nil { + return v.Reason, v.Message + } + _, reason, message = domainConditionText(resource, + v1alpha1.StackResourceWorkloadConverged, "WorkloadNotConverged", "workload has not converged") + return reason, message +} + +// failureDetail returns the first failure entry of the given classification +// that carries any detail. +func failureDetail(resource *v1alpha1.StackResource, failureType string) *v1alpha1.LastFailureDetail { + for i := range resource.Status.LastFailureDetails { + d := &resource.Status.LastFailureDetails[i] + if d.Type == failureType && (d.LastTerminationReason != "" || d.LastTerminationMessage != "") { + return d + } + } + return nil +} + +// portDialMessage names the declared ports the last dial proved closed. +func portDialMessage(ports []int32) string { + if len(ports) == 0 { + return "declared ports not accepting connections" + } + strs := make([]string, len(ports)) + for i, p := range ports { + strs[i] = strconv.Itoa(int(p)) + } + noun := "port" + if len(ports) > 1 { + noun = "ports" + } + return fmt.Sprintf("%s %s not accepting connections", noun, strings.Join(strs, ", ")) } diff --git a/internal/controller/stackresource/status_derive_test.go b/internal/controller/stackresource/status_derive_test.go index 9b31688..2c01958 100644 --- a/internal/controller/stackresource/status_derive_test.go +++ b/internal/controller/stackresource/status_derive_test.go @@ -257,6 +257,247 @@ func TestDeriveNoVerdictStampsImageSourceRevision(t *testing.T) { } } +// summaryOf derives and returns the rolled-up summary, failing if none was written. +func summaryOf(t *testing.T, r *v1alpha1.StackResource, c *controller.VerdictCollector) *v1alpha1.StackResourceStatusSummary { + t.Helper() + deriveSummaryStatus(r, c) + if r.Status.Summary == nil { + t.Fatal("summary not written") + } + return r.Status.Summary +} + +func crashDetail() v1alpha1.LastFailureDetail { + return v1alpha1.LastFailureDetail{ + Type: v1alpha1.FailureTypeRuntimeCrash, + LastTerminationReason: "CrashLoopBackOff", + LastTerminationMessage: "back-off restarting", + } +} + +func TestSummaryBuildInProgress(t *testing.T) { + r := deriveTestResource() + setResourceCondition(r, v1alpha1.StackResourceBuildReady, false, "BuildNotReady", "application build is not yet ready") + c := &controller.VerdictCollector{} + c.ReportNotReady("ImageBuildInProgress", "Image build is still in progress") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateBuilding || s.Reason != "BuildNotReady" { + t.Fatalf("want Building/BuildNotReady, got %s/%s", s.State, s.Reason) + } + if s.ObservedGeneration != r.Generation { + t.Fatalf("want gen %d, got %d", r.Generation, s.ObservedGeneration) + } +} + +func TestSummaryFailedVerdict(t *testing.T) { + r := deriveTestResource() + c := &controller.VerdictCollector{} + c.ReportFailed("InvalidSpec", "bad spec") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateFailed || s.Reason != "InvalidSpec" || s.Message != "bad spec" { + t.Fatalf("want Failed/InvalidSpec/bad spec, got %s/%s/%s", s.State, s.Reason, s.Message) + } + if s.ObservedGeneration != r.Generation { + t.Fatalf("want gen %d, got %d", r.Generation, s.ObservedGeneration) + } +} + +// A failed build is reported plainly; suppressing the duplicate is the hub's job. +func TestSummaryBuildFailedVerdict(t *testing.T) { + r := deriveTestResource() + setResourceCondition(r, v1alpha1.StackResourceBuildReady, false, v1alpha1.ReasonBuildFailed, "application build failed terminally") + c := &controller.VerdictCollector{} + c.ReportFailed(v1alpha1.ReasonBuildFailed, "ImageBuild x failed") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateFailed || s.Reason != v1alpha1.ReasonBuildFailed { + t.Fatalf("want Failed/%s, got %s/%s", v1alpha1.ReasonBuildFailed, s.State, s.Reason) + } + if s.Message != "ImageBuild x failed" { + t.Fatalf("Message = %q, want the verdict message", s.Message) + } +} + +func TestSummaryWaitingOnDependencies(t *testing.T) { + r := deriveTestResource() + setResourceCondition(r, v1alpha1.StackResourceDependenciesReady, false, "DependenciesNotReady", "waiting for mysql") + c := &controller.VerdictCollector{} + c.ReportNotReady("DependenciesNotReady", "waiting for mysql") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateWaiting || s.Reason != "DependenciesNotReady" { + t.Fatalf("want Waiting/DependenciesNotReady, got %s/%s", s.State, s.Reason) + } + if s.Message != "waiting for mysql" { + t.Fatalf("Message = %q, want the condition message", s.Message) + } +} + +func TestSummaryRuntimeCrash(t *testing.T) { + r := deriveTestResource() + r.Status.LastFailureDetails = []v1alpha1.LastFailureDetail{crashDetail()} + c := &controller.VerdictCollector{} + c.ReportNotReady("StackResourceDeploymentNotReady", "deployment is not ready") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateFailed || s.Reason != "CrashLoopBackOff" { + t.Fatalf("want Failed/CrashLoopBackOff, got %s/%s", s.State, s.Reason) + } + if s.Message != "back-off restarting" { + t.Fatalf("Message = %q, want the termination message", s.Message) + } +} + +// The build gate ran first, so the workload never rolled this pass: the crash +// entry belongs to the previous revision. +func TestSummaryBuildOutranksStaleCrash(t *testing.T) { + r := deriveTestResource() + r.Status.LastFailureDetails = []v1alpha1.LastFailureDetail{crashDetail()} + setResourceCondition(r, v1alpha1.StackResourceBuildReady, false, "BuildNotReady", "building") + c := &controller.VerdictCollector{} + c.ReportNotReady("ImageBuildInProgress", "Image build is still in progress") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateBuilding || s.Reason != "BuildNotReady" { + t.Fatalf("want Building/BuildNotReady, got %s/%s", s.State, s.Reason) + } +} + +func TestSummaryDeployingWithPortDial(t *testing.T) { + r := deriveTestResource() + r.Status.PortCheck = &v1alpha1.PortCheckStatus{ + Revision: "rev-3", + Status: v1alpha1.PortCheckStatusTypeFailure, + FailingPortNumbers: []int32{80}, + } + c := &controller.VerdictCollector{} + c.ReportNotReady("StackResourceDeploymentNotReady", "deployment is not ready") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateDeploying || s.Reason != v1alpha1.ReasonPortNotListening { + t.Fatalf("want Deploying/%s, got %s/%s", v1alpha1.ReasonPortNotListening, s.State, s.Reason) + } + if s.Message != "port 80 not accepting connections" { + t.Fatalf("Message = %q, want the port-dial diagnosis", s.Message) + } +} + +func TestSummaryDeployingReadinessFallback(t *testing.T) { + r := deriveTestResource() + r.Status.LastFailureDetails = []v1alpha1.LastFailureDetail{{ + Type: v1alpha1.FailureTypeReadinessFailure, + LastTerminationReason: "Unhealthy", + LastTerminationMessage: "readiness probe failed", + }} + c := &controller.VerdictCollector{} + c.ReportNotReady("StackResourceDeploymentNotReady", "deployment is not ready") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateDeploying || s.Reason != "Unhealthy" { + t.Fatalf("want Deploying/Unhealthy, got %s/%s", s.State, s.Reason) + } + if s.Message != "readiness probe failed" { + t.Fatalf("Message = %q, want the kubelet detail", s.Message) + } +} + +func TestSummaryDeployingVerdictFallback(t *testing.T) { + r := deriveTestResource() + c := &controller.VerdictCollector{} + c.ReportNotReady("JobRunning", "job created, waiting for completion") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateDeploying || s.Reason != "JobRunning" { + t.Fatalf("want Deploying/JobRunning, got %s/%s", s.State, s.Reason) + } + if s.Message != "job created, waiting for completion" { + t.Fatalf("Message = %q, want the verdict message", s.Message) + } +} + +func TestSummaryPreviousRevisionSuffix(t *testing.T) { + r := deriveTestResource() + setResourceCondition(r, v1alpha1.StackResourceWorkloadAvailable, true, "DeploymentServing", "serving") + r.Status.PortCheck = &v1alpha1.PortCheckStatus{ + Revision: "rev-3", + Status: v1alpha1.PortCheckStatusTypeFailure, + FailingPortNumbers: []int32{80}, + } + c := &controller.VerdictCollector{} + c.ReportNotReady("StackResourceDeploymentNotReady", "deployment is not ready") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateDeploying { + t.Fatalf("State = %s, want Deploying", s.State) + } + if s.Message != "port 80 not accepting connections (previous revision still serving traffic)" { + t.Fatalf("Message = %q, want the port-dial diagnosis plus the serving suffix", s.Message) + } +} + +func TestSummaryReady(t *testing.T) { + r := deriveTestResource() + setResourceCondition(r, v1alpha1.StackResourceWorkloadConverged, true, "FullyConverged", "all replicas updated") + setResourceCondition(r, v1alpha1.StackResourceWorkloadAvailable, true, "DeploymentServing", "serving") + + s := summaryOf(t, r, &controller.VerdictCollector{}) + + if s.State != v1alpha1.SummaryStateReady || s.Reason != "FullyConverged" { + t.Fatalf("want Ready/FullyConverged, got %s/%s", s.State, s.Reason) + } + if s.Message != "all replicas updated" { + t.Fatalf("Message = %q, want the converged message", s.Message) + } +} + +// Converged without a serving fact is not Ready: nothing asserted traffic flows. +func TestSummaryConvergedWithoutAvailableIsDeploying(t *testing.T) { + r := deriveTestResource() + setResourceCondition(r, v1alpha1.StackResourceWorkloadConverged, true, "FullyConverged", "all replicas updated") + + s := summaryOf(t, r, &controller.VerdictCollector{}) + + if s.State != v1alpha1.SummaryStateDeploying { + t.Fatalf("State = %s, want Deploying (never Ready without WorkloadAvailable)", s.State) + } + if s.Reason != "FullyConverged" { + t.Fatalf("Reason = %s, want FullyConverged from the WorkloadConverged fallback", s.Reason) + } +} + +// A BuildReady=False from a previous generation describes a rollout that no +// longer exists, so it must not produce Building. +func TestSummaryStaleConditionsIgnored(t *testing.T) { + r := deriveTestResource() + meta.SetStatusCondition(&r.Status.Conditions, metav1.Condition{ + Type: string(v1alpha1.StackResourceBuildReady), + Status: metav1.ConditionFalse, + ObservedGeneration: r.Generation - 1, + Reason: "BuildNotReady", + Message: "application build is not yet ready", + }) + c := &controller.VerdictCollector{} + c.ReportNotReady("StackResourceDeploymentNotReady", "deployment is not ready") + + s := summaryOf(t, r, c) + + if s.State != v1alpha1.SummaryStateDeploying { + t.Fatalf("State = %s, want Deploying (a stale condition must not produce Building)", s.State) + } +} + // stubSubReconciler lets the test compose an arbitrary chain. type stubSubReconciler struct { fn func(ctx context.Context, r *v1alpha1.StackResource) (subReconcilerResult, error) diff --git a/internal/controller/stackresource/workload/readiness.go b/internal/controller/stackresource/workload/readiness.go index 2c1eeb2..e0cb773 100644 --- a/internal/controller/stackresource/workload/readiness.go +++ b/internal/controller/stackresource/workload/readiness.go @@ -168,7 +168,7 @@ func recordPortFailure(resource *v1alpha1.StackResource) string { resource.Status.LastFailureDetails = []v1alpha1.LastFailureDetail{{ Type: v1alpha1.FailureTypeReadinessFailure, ContainerName: resource.Name, - LastTerminationReason: "PortNotListening", + LastTerminationReason: v1alpha1.ReasonPortNotListening, LastTerminationMessage: msg, }} return msg @@ -290,6 +290,6 @@ func firstDialablePodIP(pods []corev1.Pod) (string, bool) { // Stalled=True / Available=True("ServingButStalled"), and a non-serving one // into Phase=Failed / Stalled=True / Available=False. func (r *Reconciler) reportPortNotListening(ctx context.Context, resource *v1alpha1.StackResource, msg string) { - r.Status.SetCondition(resource, v1alpha1.StackResourceWorkloadConverged, false, "PortNotListening", msg) - r.Status.ReportFailed(ctx, resource, "PortNotListening", msg) + r.Status.SetCondition(resource, v1alpha1.StackResourceWorkloadConverged, false, v1alpha1.ReasonPortNotListening, msg) + r.Status.ReportFailed(ctx, resource, v1alpha1.ReasonPortNotListening, msg) } diff --git a/internal/controller/util.go b/internal/controller/util.go index a030f93..0b254dc 100644 --- a/internal/controller/util.go +++ b/internal/controller/util.go @@ -169,7 +169,10 @@ func sanitizeTerminationMessage(msg string) string { } func BuildLastFailureDetail(cs corev1.ContainerStatus) corev1alpha1.LastFailureDetail { + // Every caller gates on IsCrashState, so the classification is fixed here. + // Readers match Type exactly and never re-default an empty one. detail := corev1alpha1.LastFailureDetail{ + Type: corev1alpha1.FailureTypeRuntimeCrash, ContainerName: cs.Name, RestartCount: cs.RestartCount, } From b3dd59ecad1231750dae3a79d0b32a103fe422db Mon Sep 17 00:00:00 2001 From: Ashish Date: Tue, 4 Aug 2026 20:32:16 +0530 Subject: [PATCH 2/4] refactor(stackresource): drop unreachable readiness-detail branch in deployingDetail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A readiness_failure entry has one writer, recordPortFailure, which only runs past the port-check grace window and files the PortNotListening Failed verdict in the same pass — so the Deploying path can never see one. Inside the grace window the details are cleared and only PortCheck carries the diagnosis, which the port-dial branch above already handles. Claude-Session: https://claude.ai/code/session_01TpRLszVTJudSd86RreufSC --- .../controller/stackresource/status_derive.go | 11 ++++------ .../stackresource/status_derive_test.go | 20 ------------------- 2 files changed, 4 insertions(+), 27 deletions(-) diff --git a/internal/controller/stackresource/status_derive.go b/internal/controller/stackresource/status_derive.go index 38488cc..ef163c4 100644 --- a/internal/controller/stackresource/status_derive.go +++ b/internal/controller/stackresource/status_derive.go @@ -190,17 +190,14 @@ func summarize(resource *v1alpha1.StackResource, verdicts *controller.VerdictCol } // deployingDetail picks the most specific diagnosis of why the new pods are -// not ready: the last port dial, else the kubelet readiness detail, else the -// verdict (nil on the no-verdict path — fall back to WorkloadConverged). +// not ready: the last port dial, else the verdict (nil on the no-verdict +// path — fall back to WorkloadConverged). No readiness-detail branch: a +// readiness_failure entry is only ever written together with a Failed +// verdict, which never reaches the Deploying path. func deployingDetail(resource *v1alpha1.StackResource, v *controller.Verdict) (reason, message string) { if pc := resource.Status.PortCheck; pc != nil && pc.Status == v1alpha1.PortCheckStatusTypeFailure { return v1alpha1.ReasonPortNotListening, portDialMessage(pc.FailingPortNumbers) } - if resource.Status.PortCheck == nil { - if d := failureDetail(resource, v1alpha1.FailureTypeReadinessFailure); d != nil { - return d.LastTerminationReason, d.LastTerminationMessage - } - } if v != nil { return v.Reason, v.Message } diff --git a/internal/controller/stackresource/status_derive_test.go b/internal/controller/stackresource/status_derive_test.go index 2c01958..0e80b55 100644 --- a/internal/controller/stackresource/status_derive_test.go +++ b/internal/controller/stackresource/status_derive_test.go @@ -391,26 +391,6 @@ func TestSummaryDeployingWithPortDial(t *testing.T) { } } -func TestSummaryDeployingReadinessFallback(t *testing.T) { - r := deriveTestResource() - r.Status.LastFailureDetails = []v1alpha1.LastFailureDetail{{ - Type: v1alpha1.FailureTypeReadinessFailure, - LastTerminationReason: "Unhealthy", - LastTerminationMessage: "readiness probe failed", - }} - c := &controller.VerdictCollector{} - c.ReportNotReady("StackResourceDeploymentNotReady", "deployment is not ready") - - s := summaryOf(t, r, c) - - if s.State != v1alpha1.SummaryStateDeploying || s.Reason != "Unhealthy" { - t.Fatalf("want Deploying/Unhealthy, got %s/%s", s.State, s.Reason) - } - if s.Message != "readiness probe failed" { - t.Fatalf("Message = %q, want the kubelet detail", s.Message) - } -} - func TestSummaryDeployingVerdictFallback(t *testing.T) { r := deriveTestResource() c := &controller.VerdictCollector{} From bd9b10ecf242c810af51230acff3d6889957db52 Mon Sep 17 00:00:00 2001 From: Ashish Date: Tue, 4 Aug 2026 20:34:53 +0530 Subject: [PATCH 3/4] refactor(stackresource): drop dead empty-ports branch in portDialMessage Failure is only written when at least one dialed port is closed, so a Failure PortCheck always carries FailingPortNumbers. The guard was a hub-transplant defensive leftover. Claude-Session: https://claude.ai/code/session_01TpRLszVTJudSd86RreufSC --- internal/controller/stackresource/status_derive.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/controller/stackresource/status_derive.go b/internal/controller/stackresource/status_derive.go index ef163c4..54e98b3 100644 --- a/internal/controller/stackresource/status_derive.go +++ b/internal/controller/stackresource/status_derive.go @@ -219,10 +219,9 @@ func failureDetail(resource *v1alpha1.StackResource, failureType string) *v1alph } // portDialMessage names the declared ports the last dial proved closed. +// Never called with an empty list: a Failure PortCheck always carries the +// closed ports. func portDialMessage(ports []int32) string { - if len(ports) == 0 { - return "declared ports not accepting connections" - } strs := make([]string, len(ports)) for i, p := range ports { strs[i] = strconv.Itoa(int(p)) From eb5d112c5482437f3b956da2f53e6383a0ca40f5 Mon Sep 17 00:00:00 2001 From: Ashish Date: Tue, 4 Aug 2026 20:45:38 +0530 Subject: [PATCH 4/4] refactor(stackresource): flatten summarize into one classify switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence now reads as a single switch — one case per state, first case wins — with the inputs named up front. The serving-traffic note moves to appendServingNote, applied only to Deploying. Claude-Session: https://claude.ai/code/session_01TpRLszVTJudSd86RreufSC --- .../controller/stackresource/status_derive.go | 84 +++++++++++-------- 1 file changed, 49 insertions(+), 35 deletions(-) diff --git a/internal/controller/stackresource/status_derive.go b/internal/controller/stackresource/status_derive.go index 54e98b3..30f9708 100644 --- a/internal/controller/stackresource/status_derive.go +++ b/internal/controller/stackresource/status_derive.go @@ -147,46 +147,60 @@ func domainConditionTrue(resource *v1alpha1.StackResource, condType v1alpha1.Sta return domainCondition(resource, condType, metav1.ConditionTrue) != nil } -// summarize rolls the pass up into one verdict. Precedence: terminal failure, -// build in progress, waiting on dependencies, runtime crash, then the rollout -// itself. Build/deps outrank the crash detail: when those gates fail the -// workload reconciler never ran this pass, so a crash entry is a previous -// revision's leftover. +// summarize rolls the pass up into one verdict. func summarize(resource *v1alpha1.StackResource, verdicts *controller.VerdictCollector) *v1alpha1.StackResourceStatusSummary { - summary := &v1alpha1.StackResourceStatusSummary{ObservedGeneration: resource.Generation} - if v := verdicts.Failed(); v != nil { - summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateFailed, v.Reason, v.Message - return summary + state, reason, message := classify(resource, verdicts) + if state == v1alpha1.SummaryStateDeploying { + message = appendServingNote(resource, message) } + return &v1alpha1.StackResourceStatusSummary{ + State: state, + Reason: reason, + Message: message, + ObservedGeneration: resource.Generation, + } +} + +// classify maps the pass to one state. First case wins: terminal failure, +// build in progress, waiting on dependencies, runtime crash, landed rollout, +// else deploying. Build/deps outrank the crash detail: when those gates fail +// the workload reconciler never ran this pass, so a crash entry is a previous +// revision's leftover. +func classify(resource *v1alpha1.StackResource, verdicts *controller.VerdictCollector) (v1alpha1.StackResourceSummaryState, string, string) { notReady := verdicts.NotReady() - if notReady != nil { - if cond := domainCondition(resource, v1alpha1.StackResourceBuildReady, metav1.ConditionFalse); cond != nil { - summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateBuilding, cond.Reason, cond.Message - return summary - } - if cond := domainCondition(resource, v1alpha1.StackResourceDependenciesReady, metav1.ConditionFalse); cond != nil { - summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateWaiting, cond.Reason, cond.Message - return summary - } - if d := failureDetail(resource, v1alpha1.FailureTypeRuntimeCrash); d != nil { - summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateFailed, d.LastTerminationReason, d.LastTerminationMessage - return summary - } - } else if wc := domainCondition(resource, v1alpha1.StackResourceWorkloadConverged, metav1.ConditionTrue); wc != nil && - domainConditionTrue(resource, v1alpha1.StackResourceWorkloadAvailable) { - summary.State, summary.Reason, summary.Message = v1alpha1.SummaryStateReady, wc.Reason, wc.Message - return summary + buildNotReady := domainCondition(resource, v1alpha1.StackResourceBuildReady, metav1.ConditionFalse) + depsNotReady := domainCondition(resource, v1alpha1.StackResourceDependenciesReady, metav1.ConditionFalse) + crash := failureDetail(resource, v1alpha1.FailureTypeRuntimeCrash) + converged := domainCondition(resource, v1alpha1.StackResourceWorkloadConverged, metav1.ConditionTrue) + + switch { + case verdicts.Failed() != nil: + v := verdicts.Failed() + return v1alpha1.SummaryStateFailed, v.Reason, v.Message + case notReady != nil && buildNotReady != nil: + return v1alpha1.SummaryStateBuilding, buildNotReady.Reason, buildNotReady.Message + case notReady != nil && depsNotReady != nil: + return v1alpha1.SummaryStateWaiting, depsNotReady.Reason, depsNotReady.Message + case notReady != nil && crash != nil: + return v1alpha1.SummaryStateFailed, crash.LastTerminationReason, crash.LastTerminationMessage + case notReady == nil && converged != nil && domainConditionTrue(resource, v1alpha1.StackResourceWorkloadAvailable): + return v1alpha1.SummaryStateReady, converged.Reason, converged.Message + default: + reason, message := deployingDetail(resource, notReady) + return v1alpha1.SummaryStateDeploying, reason, message } - summary.State = v1alpha1.SummaryStateDeploying - summary.Reason, summary.Message = deployingDetail(resource, notReady) - if domainConditionTrue(resource, v1alpha1.StackResourceWorkloadAvailable) { - if summary.Message == "" { - summary.Message = "previous revision still serving traffic" - } else { - summary.Message += " (previous revision still serving traffic)" - } +} + +// appendServingNote marks a deploying message when the previous revision still +// holds traffic — without it, "not ready yet" reads as an outage. +func appendServingNote(resource *v1alpha1.StackResource, message string) string { + if !domainConditionTrue(resource, v1alpha1.StackResourceWorkloadAvailable) { + return message + } + if message == "" { + return "previous revision still serving traffic" } - return summary + return message + " (previous revision still serving traffic)" } // deployingDetail picks the most specific diagnosis of why the new pods are