ssa: fix timeout error not mentioning object

Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>

Author: matheuscscp

ssa/manager_wait.go

@@ -22,6 +22,7 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"sync"
 	"time"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -96,12 +97,14 @@ func (m *ResourceManager) WaitForSetWithContext(ctx context.Context, set object.
 	}
 	eventsChan := m.poller.Poll(ctx, set, pollingOpts)
 
+	var mu sync.Mutex
 	lastStatus := make(map[object.ObjMetadata]*event.ResourceStatus)
 
 	done := statusCollector.ListenWithObserver(eventsChan, collector.ObserverFunc(
 		func(statusCollector *collector.ResourceStatusCollector, e event.Event) {
 			var rss []*event.ResourceStatus
 			counts := make(map[status.Status]int)
+			mu.Lock()
 			for _, rs := range statusCollector.ResourceStatuses {
 				if rs == nil {
 					continue
@@ -117,6 +120,7 @@ func (m *ResourceManager) WaitForSetWithContext(ctx context.Context, set object.
 				rss = append(rss, rs)
 				counts[effectiveStatus]++
 			}
+			mu.Unlock()
 
 			// If only Failed or Current statuses are present,
 			// we can consider this a terminal state. Detecting
@@ -152,10 +156,7 @@ func (m *ResourceManager) WaitForSetWithContext(ctx context.Context, set object.
 		return ctx.Err()
 	}
 
-	if statusCollector.Error != nil {
-		return statusCollector.Error
-	}
-
+	mu.Lock()
 	var errs []string
 	for id, rs := range statusCollector.ResourceStatuses {
 		switch {
@@ -172,15 +173,17 @@ func (m *ResourceManager) WaitForSetWithContext(ctx context.Context, set object.
 			errors.Is(ctx.Err(), context.DeadlineExceeded) &&
 				lastStatus[id].Status != status.CurrentStatus:
 			var builder strings.Builder
-			builder.WriteString(fmt.Sprintf("%s status: '%s'",
-				utils.FmtObjMetadata(rs.Identifier), lastStatus[id].Status))
+			fmt.Fprintf(&builder, "%s status: '%s'",
+				utils.FmtObjMetadata(rs.Identifier), lastStatus[id].Status)
 			if rs.Error != nil {
-				builder.WriteString(fmt.Sprintf(": %s", rs.Error))
+				fmt.Fprintf(&builder, ": %s", rs.Error)
 			}
 			errs = append(errs, builder.String())
 		}
 	}
 
+	mu.Unlock()
+
 	if len(errs) > 0 {
 		msg := "failed early due to stalled resources"
 		if errors.Is(ctx.Err(), context.DeadlineExceeded) {
@@ -189,6 +192,10 @@ func (m *ResourceManager) WaitForSetWithContext(ctx context.Context, set object.
 		return fmt.Errorf("%s: [%s]", msg, strings.Join(errs, ", "))
 	}
 
+	if statusCollector.Error != nil {
+		return statusCollector.Error
+	}
+
 	return nil
 }
 

ssa/manager_wait_test.go

@@ -411,6 +411,87 @@ func TestWaitForSet_RateLimiterError(t *testing.T) {
 	}
 }
 
+func TestWaitForSet_RateLimiterErrorIncludesObjectNames(t *testing.T) {
+	g := NewWithT(t)
+
+	id := generateName("rlname")
+
+	// Create real resources on the cluster that will reach Current status.
+	objects, err := readManifest("testdata/test14.yaml", id)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	manager.SetOwnerLabels(objects, "infra", "default")
+	cs, err := manager.ApplyAllStaged(context.Background(), objects, DefaultApplyOptions())
+	g.Expect(err).NotTo(HaveOccurred())
+
+	// Add a non-existent Deployment to the wait set. This simulates the
+	// real-world scenario where a health check includes a resource that
+	// doesn't exist on the cluster (e.g. "does-not-exist").
+	nonExistent := object.ObjMetadata{
+		Name:      "does-not-exist",
+		Namespace: id,
+		GroupKind: schema.GroupKind{Group: "apps", Kind: "Deployment"},
+	}
+	waitSet := append(cs.ToObjMetadataSet(), nonExistent)
+
+	// Use a custom status reader that simulates the rate limiter firing
+	// at the context deadline. In production, the Go rate limiter fires
+	// preemptively when it detects that waiting would exceed the deadline.
+	// We simulate this by sleeping past the deadline before returning the
+	// rate limiter error, ensuring both statusCollector.Error is set AND
+	// ctx.Err() is context.DeadlineExceeded.
+	timeout := 500 * time.Millisecond
+	start := time.Now()
+	manager.poller = polling.NewStatusPoller(manager.client, restMapper, polling.Options{
+		CustomStatusReaders: []engine.StatusReader{
+			kstatusreaders.NewGenericStatusReader(restMapper,
+				func(u *unstructured.Unstructured) (*status.Result, error) {
+					if time.Since(start) > 300*time.Millisecond {
+						// Sleep past the context deadline to ensure
+						// ctx.Err() == context.DeadlineExceeded when
+						// WaitForSetWithContext processes the error.
+						remaining := timeout - time.Since(start)
+						if remaining > 0 {
+							time.Sleep(remaining + 50*time.Millisecond)
+						}
+						return nil, fmt.Errorf("rate: Wait(n=1) would exceed context deadline")
+					}
+					return status.Compute(u)
+				},
+			),
+		},
+	})
+	defer func() {
+		manager.poller = poller
+	}()
+
+	err = manager.WaitForSet(waitSet, WaitOptions{
+		Interval: 100 * time.Millisecond,
+		Timeout:  timeout,
+	})
+
+	g.Expect(err).To(HaveOccurred())
+	errMsg := err.Error()
+	t.Logf("error message: %s", errMsg)
+
+	// The error must include the name of the non-existent resource.
+	// Before the fix, statusCollector.Error was returned directly without
+	// any object context, producing: "rate: Wait(n=1) would exceed context deadline".
+	// After the fix, the per-object status loop runs first, producing:
+	// "timeout waiting for: [Deployment/id/does-not-exist status: 'NotFound']".
+	g.Expect(errMsg).To(ContainSubstring("does-not-exist"),
+		"error must contain the name of the non-existent resource")
+	g.Expect(errMsg).To(ContainSubstring("timeout waiting for"),
+		"error must indicate it was a timeout")
+
+	// ConfigMaps that reached Current status should NOT appear in the error.
+	for i := 1; i <= 4; i++ {
+		cmName := fmt.Sprintf("%s-cm%d", id, i)
+		g.Expect(errMsg).NotTo(ContainSubstring(cmName),
+			"error should not contain ConfigMap %s which reached Current status", cmName)
+	}
+}
+
 func TestWaitForSet_ErrorOnReaderError(t *testing.T) {
 	g := NewWithT(t)