fix(cldy): make the Cloudability upload path actually retry

[peatey]

Fixes #238.

The bug

A failed Cloudability connection test at startup left the agent running with an empty StorageServices slice for the life of the process. uploadData iterated that empty slice, fell through to os.Remove, and deleted every tarball as if it had been uploaded — so samples were silently discarded until the pod was restarted.

Reported bugs

• Startup failure permanently disabled the uploader. Storage services are now built through ensureStorageServices, which rebuilds them lazily when the slice is empty instead of only once at startup. The cooldown is stamped after the build finishes, not before, so a build that outlasts UploadFrequency cannot defeat its own gate.
• Empty StorageServices silently discarded tarballs. UploadData returns ErrNoStorageServices before any os.Remove, so the tarball survives and is retried on a later cycle.
• testUpload backoff was nanoseconds. time.Duration(math.Pow(2, i)) made the three retries fire ~2ns, ~4ns and ~8ns apart. Now seconds-scale, with no sleep after the final attempt, behind an injectable seam so tests stay fast.

Defects those symptoms were hiding

• doWithRetry had the identical nanosecond backoff, and never closed the response body of a failed attempt — leaking a connection per retry.
• The presigned S3 PUT could not be retried at all. This is the request in the issue's connection reset by peer log. doWithRetry re-issues the same *http.Request, and Client.Do consults GetBody only for redirects and its own internal rewind — never for a caller's retry loop. Attempt 1 sent the file, the transport closed it, and attempt 2 put a 0-byte body on a live presigned S3 URL; only the Content-MD5 header prevented a truncated object being stored. The body is now rebuilt from GetBody before each attempt, and a request carrying a body without GetBody is attempted once rather than retried with a broken one.
• Retaining tarballs exposed a wedge in set.operateAndRemove. It discarded its progress list on the first error while those files had already been removed, stranding phantom entries that failed every subsequent cycle. It now retains partial progress and joins the errors.
• Retrying every retained tarball each cycle made the pass O(queue depth). A small backlog outran the tick, and dropped ticks stalled the rest of the loop. A pass now starts no new entry once its budget is spent, and abandons the batch when the destination as a whole is unavailable.
• MetricsCollectorServiceImpl.testUpload was a line-for-line twin carrying both original defects, on a live production path. Both call sites now share one uploadProbe loop so they cannot diverge again.

Tests

Tests no longer reach frontdoor.apptio.com — a suite-level fake serves the login, presign and probe endpoints. That took the package from ~100s to ~4s while keeping every spec.

Every new spec was mutation-checked: the bug it covers was reintroduced, the spec confirmed to fail, and the bug reverted. 66/66 specs green; -race reports zero data races.

Three specs fail under -race (should only login once, should log back in if required, should emit each time emission interval is satisifed). These are pre-existing and timing-sensitive — verified failing identically on a clean develop worktree. CI runs go test ./... without -race.

Follow-ups, deliberately not in scope

• UPLOAD_RETRY_COUNT is still discarded by NewApptioClient (maxRetries: 3 is hardcoded). Plumbing it needs a backoff cap first: 1<<i is uncapped, so the documented default of 5 would mean ~330s per request and ~990s per upload.
• doWithRetry's terminal error does not wrap the transport error, so callers cannot classify a connection failure via errors.As.
• Upload failure still does not reach Healthy() — the third suggestion in Failed Cloudability testUpload at startup permanently disables uploads (no retry; samples dropped) #238, which lives in the emitter rather than cldy/.
• The drain budget is checked between entries, not during one, so a single slow entry can still overrun the tick. Bounding that properly needs a context deadline plumbed through the client.

Review

Three adversarial review rounds, each of which found real defects in the previous round's fixes. The final round found no correctness regression versus develop and cleared it to ship.

https://claude.ai/code/session_01P95uFVdQCZBHvfzDUv2xdd