Skip to content

Commit ed69531

Browse files
authored
Fix Rendered condition observedGeneration stale after lifecycle changes (kptdev#1076)
When reconcileRender skips (no render trigger), the Rendered condition's observedGeneration was never updated. After a lifecycle patch bumped metadata.generation, kubectl wait --for=condition=Rendered would hang because kubectl v1.35+ treats observedGeneration < generation as stale. Add refreshRenderedGeneration() which bumps the Rendered condition's observedGeneration when it's True but behind the current generation. Fixes kptdev#1075 Signed-off-by: Fiachra Corcoran <fiachra.corcoran@est.tech>
1 parent 610aff9 commit ed69531

4 files changed

Lines changed: 92 additions & 0 deletions

File tree

controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,7 @@ func (r *PackageRevisionReconciler) reconcileRender(ctx context.Context, pr *por
234234

235235
requested, annotationTrigger, sourceTrigger := renderTrigger(pr)
236236
if !annotationTrigger && !sourceTrigger {
237+
r.refreshRenderedGeneration(ctx, pr)
237238
return nil, nil
238239
}
239240

controllers/packagerevisions/pkg/controllers/packagerevision/status.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,23 @@ func (r *PackageRevisionReconciler) updateRenderStatus(ctx context.Context, pr *
113113
}
114114
}
115115

116+
// refreshRenderedGeneration bumps the Rendered condition's observedGeneration
117+
// when no render is needed but the condition is stale (e.g. after lifecycle patch).
118+
// It preserves all other condition fields (reason, message, lastTransitionTime)
119+
// to avoid misleading status churn.
120+
func (r *PackageRevisionReconciler) refreshRenderedGeneration(ctx context.Context, pr *porchv1alpha2.PackageRevision) {
121+
for _, c := range pr.Status.Conditions {
122+
if c.Type == porchv1alpha2.ConditionRendered && c.Status == metav1.ConditionTrue && c.ObservedGeneration < pr.Generation {
123+
// Only bump observedGeneration — preserve existing reason, message, and transition time.
124+
c.ObservedGeneration = pr.Generation
125+
r.updateRenderStatus(ctx, pr, pr.Status.RenderingPrrResourceVersion, "",
126+
c,
127+
)
128+
return
129+
}
130+
}
131+
}
132+
116133
// setSourceFailed logs the error and sets Ready=False and Rendered=False.
117134
// Rendered is set even though rendering was never attempted — the package
118135
// content didn't land successfully, so "not rendered" is accurate.

controllers/packagerevisions/pkg/controllers/packagerevision/status_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,68 @@ func TestUpdateRenderStatusComplete(t *testing.T) {
158158
assert.Equal(t, metav1.ConditionTrue, captured.Conditions[0].Status)
159159
}
160160

161+
func TestRefreshRenderedGenerationBumpsStale(t *testing.T) {
162+
mockClient := mockclient.NewMockClient(t)
163+
captured := captureStatusPatch(t, mockClient)
164+
165+
r := &PackageRevisionReconciler{Client: mockClient}
166+
pr := basePR()
167+
pr.Generation = 5
168+
originalTime := metav1.NewTime(time.Date(2025, 6, 1, 10, 0, 0, 0, time.UTC))
169+
pr.Status.RenderingPrrResourceVersion = "prev-render-version"
170+
pr.Status.Conditions = []metav1.Condition{
171+
{
172+
Type: porchv1alpha2.ConditionRendered,
173+
Status: metav1.ConditionTrue,
174+
ObservedGeneration: 2,
175+
Reason: porchv1alpha2.ReasonRendered,
176+
Message: "render complete",
177+
LastTransitionTime: originalTime,
178+
},
179+
}
180+
181+
r.refreshRenderedGeneration(t.Context(), pr)
182+
183+
assert.Len(t, captured.Conditions, 1)
184+
assert.Equal(t, porchv1alpha2.ConditionRendered, captured.Conditions[0].Type)
185+
assert.Equal(t, metav1.ConditionTrue, captured.Conditions[0].Status)
186+
assert.Equal(t, int64(5), captured.Conditions[0].ObservedGeneration)
187+
// Verify existing fields are preserved, not overwritten.
188+
assert.Equal(t, porchv1alpha2.ReasonRendered, captured.Conditions[0].Reason)
189+
assert.Equal(t, "render complete", captured.Conditions[0].Message)
190+
assert.Equal(t, originalTime, captured.Conditions[0].LastTransitionTime)
191+
// Verify renderingPrrResourceVersion is preserved.
192+
assert.Equal(t, "prev-render-version", captured.RenderingPrrResourceVersion)
193+
}
194+
195+
func TestRefreshRenderedGenerationSkipsWhenCurrent(t *testing.T) {
196+
mockClient := mockclient.NewMockClient(t)
197+
// No Status().Patch expected — should be a no-op.
198+
199+
r := &PackageRevisionReconciler{Client: mockClient}
200+
pr := basePR()
201+
pr.Generation = 3
202+
pr.Status.Conditions = []metav1.Condition{
203+
{Type: porchv1alpha2.ConditionRendered, Status: metav1.ConditionTrue, ObservedGeneration: 3},
204+
}
205+
206+
r.refreshRenderedGeneration(t.Context(), pr)
207+
}
208+
209+
func TestRefreshRenderedGenerationSkipsWhenNotTrue(t *testing.T) {
210+
mockClient := mockclient.NewMockClient(t)
211+
// No Status().Patch expected — Rendered is False, not our concern.
212+
213+
r := &PackageRevisionReconciler{Client: mockClient}
214+
pr := basePR()
215+
pr.Generation = 5
216+
pr.Status.Conditions = []metav1.Condition{
217+
{Type: porchv1alpha2.ConditionRendered, Status: metav1.ConditionFalse, ObservedGeneration: 2},
218+
}
219+
220+
r.refreshRenderedGeneration(t.Context(), pr)
221+
}
222+
161223
func TestSetRenderFailed(t *testing.T) {
162224
mockClient := mockclient.NewMockClient(t)
163225

test/e2e/crd/lifecycle_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,18 @@ var _ = Describe("Lifecycle", Ordered, Label("lifecycle"), func() {
5151
By("verifying publish metadata")
5252
Expect(pr.Status.PublishedBy).NotTo(BeEmpty())
5353
Expect(pr.Status.PublishedAt).NotTo(BeNil())
54+
55+
By("verifying Rendered condition observedGeneration is current after lifecycle changes")
56+
Expect(k8sClient.Get(env.Ctx, client.ObjectKeyFromObject(pr), pr)).To(Succeed())
57+
foundRendered := false
58+
for _, c := range pr.Status.Conditions {
59+
if c.Type == porchv1alpha2.ConditionRendered {
60+
foundRendered = true
61+
Expect(c.ObservedGeneration).To(Equal(pr.Generation),
62+
"Rendered observedGeneration should match current generation after lifecycle transition")
63+
}
64+
}
65+
Expect(foundRendered).To(BeTrue(), "expected Rendered condition to be present on the PackageRevision")
5466
})
5567

5668
It("should transition Published → DeletionProposed → Published (undo)", func() {

0 commit comments

Comments
 (0)