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

Open
#10,718 4 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.