cloudnative-pg / cloudnative-pg/cloudnative-pg
volumeSnapshot backup: CSI error retry classifier only recognizes Azure-shaped messages; non-Azure errors (e.g. DigitalOcean 429) become non-retryable and orphan the VolumeSnapshot
- Dominant language
- Go
- Stars
- 9.3k
- Forks
- 759
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 44
Description
### Summary
`isCSIErrorMessageRetriable` in `pkg/reconciler/backup/volumesnapshot/errors.go` (v1.28.1) decides whether a VolumeSnapshot provisioning error is retryable by string-matching the error message. The patterns it recognizes are Azure CSI driver shaped (`Retriable: true`, `HTTPStatusCode: NNN`) plus two Kubernetes conflict/timeout strings. Errors from other CSI drivers match none of them and are always classified non-retryable, including transient ones like HTTP 429 rate-limit responses. When that happens, `handleSnapshotErrors` fails the Backup and returns without deleting the VolumeSnapshot it created. The orphaned VolumeSnapshot and its bound VolumeSnapshotContent remain, and the external-snapshotter sidecar keeps calling `CreateSnapshot` against the provider indefinitely. On providers that rate-limit per volume, one transient 429 becomes a sustained backup outage.
### Environment
- CloudNativePG v1.28.1
- DigitalOcean CSI driver csi-digitalocean v4.16.0 (ships csi-snapshotter / external-snapshotter v8.4.0)
- godo v1.167.0 (DigitalOcean Go client used by the driver)
### Root cause
`errors.go` (v1.28.1):
```go
var (
retryableStatusCodes = []int{408, 429, 500, 502, 503, 504}
httpStatusCodeRegex = regexp.MustCompile(`HTTPStatusCode:\s(\d{3})`)
)
func isCSIErrorMessageRetriable(msg string) bool {
isRetryableFuncs := []func(string) bool{
isExplicitlyRetriableError, // contains "Retriable: true" (Azure CSI)
isRetryableHTTPError, // regex `HTTPStatusCode:\s(\d{3})` (Azure CSI)
isConflictError, // contains "the object has been modified"
isContextDeadlineExceededError, // contains "deadline exceeded" / "timed out"
}
// ...
}
```
`429` is in `retryableStatusCodes`, but the only path that reaches it requires the message to contain the literal token `HTTPStatusCode: 429`. That format is specific to the Azure CSI driver (the comment on `isExplicitlyRetriableError` says so). The DigitalOcean driver returns the gRPC error produced by godo, whose `ErrorResponse.Error()` formats as:
```
POST https://api.digitalocean.com/v2/volumes//snapshots: 429 (request "") per-volume snapshot limit exceeded
```
The driver surfaces that as `status.Error(codes.Internal, err.Error())`, and external-snapshotter records it on the VolumeSnapshot status as a gRPC status string of approximately the form:
```
... rpc error: code = Internal desc = ... : 429 (request "...") per-volume snapshot limit exceeded
```
No `HTTPStatusCode:` token, no `Retriable: true`, no conflict or deadline string. `isCSIErrorMessageRetriable` returns false and the 429 is treated as terminal.
`reconciler.go`, `handleSnapshotErrors`:
```go
if !snapshotErr.isRetryable() {
return nil, snapshotErr
}
// deadline / requeue handling is below this point and only runs for retryable errors
```
On the non-retryable path the Backup is marked failed and the function returns. Nothing deletes the VolumeSnapshot created earlier in the reconcile. The bound VolumeSnapshotContent stays, and csi-snapshotter requeues it (external-snapshotter treats `codes.Internal` as non-final and requeues via `AddRateLimited`), so `CreateSnapshot` keeps hitting the provider on the sidecar's backoff interval (csi-snapshotter v8.x default cap: 5 minutes).
### Impact
DigitalOcean enforces one snapshot per 10 minutes per volume and returns 429 when exceeded. A single failed backup leaves an orphan VolumeSnapshotContent whose sidecar retries every <= 5 minutes, which keeps the 10-minute per-volume window permanently armed. Every subsequent scheduled backup then 429s on contact and creates another orphan. One transient rate-limit response becomes a self-sustaining outage. CNPG-managed object-store (barman) backups are unaffected; only the volumeSnapshot path breaks. Recovery requires manually deleting the orphan VolumeSnapshots/VolumeSnapshotContents so the retries stop.
The comment in `errors.go` already notes that the gRPC status code is not exposed through the Kubernetes VolumeSnapshot API, so heuristics are currently the only option. The proposals below work within that constraint.
### Proposed changes
Two independent fixes. The second is the higher-impact one.
1. Recognize the gRPC-status / generic rate-limit shape in addition to the Azure format. external-snapshotter records the CSI error as a gRPC status string (`rpc error: code = desc = `). Matching the well-defined gRPC tokens `code = ResourceExhausted` and `code = Unavailable`, and/or an explicit `Too Many Requests` / `rate limit` signal, covers DigitalOcean and most non-Azure drivers without depending on a vendor-specific prefix. Bare-number matching has false-positive risk; anchoring on `ResourceExhausted` / `Unavailable` / `Too Many Requests` avoids it.
2. Delete the VolumeSnapshot when a provisioning error is terminal. When `isCSIErrorMessageRetriable` is false and the Backup is failed, CNPG should delete the VolumeSnapshot it created so the bound VolumeSnapshotContent is released and the csi-snapshotter sidecar stops calling `CreateSnapshot`. Today a terminal classification still leaves the orphan retrying forever, which is what turns a one-off error into an outage on rate-limited providers. Cleaning up on terminal failure also makes a misclassification (proposal 1 missing a format) fail safe instead of fatal.
### Repro sketch
1. Run a CNPG cluster on DOKS with `backup.volumeSnapshot` configured and a ScheduledBackup using `method: volumeSnapshot`.
2. Drive volumeSnapshot backups on the same volume more than once per 10 minutes (or under existing per-volume snapshot pressure) so the DO API returns 429.
3. Observe: the Backup goes to `failed`; the VolumeSnapshot and VolumeSnapshotContent remain; csi-snapshotter logs repeated `CreateSnapshot` 429s; subsequent scheduled backups fail on contact until the orphan VS/VSC are deleted manually.
Proposal 2 seems worth doing regardless of the classification work, since it bounds the blast radius of any unrecognized terminal error.
Contributor guide
Assessment
This issue has not been assessed yet.