Proposal: distinguish gate-pending from issuance-error in the renewal backoff loop #234

Context

ReadyToRequestFunc (introduced in PR #20) lets a driver defer issuance until pod-side conditions are met. cert-manager/csi-driver PR #638 recently added a --pod-readiness-gate flag that exposes this for IP-SAN use cases (the kubelet/CNI race with NodePublishVolume).

Now that there's a real-world consumer of ReadyToRequestFunc, the question of how csi-lib treats "not yet ready" vs. "issuance failed" has surfaced concretely. I'd like maintainer direction on the right fix before opening a PR.

Problem

In manager.issue(), a false return from ReadyToRequestFunc is converted to an error. This error flows back to startRenewalRoutine, where it's wrapped by wait.ExponentialBackoffWithContext(ctx, m.backoffConfig, ...). m.backoffConfig is the renewal backoff — defaults: 30s base, 5min cap, factor 2.0. Those defaults are appropriate for issuance failures (signer down, CSR rejected, apiserver blip etc.)

But they are wrong for the gate-pending state, where the underlying condition (pod IP from CNI, condition flipped, annotation written) typically resolves in seconds. Because the same backoff applies, pods wait 30s+ for the next gate evaluation when the gate would have passed in 2s.

Evidence (production logs)

From a production cluster running csi-driver with --pod-readiness-gate=pod-ip:ipv6:

01:05:16.195 NodePublishVolume RPC received
01:05:17.199 Triggering new issuance
01:05:17.203 Failed: "pod has no ipv6 address yet" → backoff
01:05:59.922 Triggering new issuance (+42.7s wasted in backoff)
01:06:00.408 CertificateRequest created
01:06:00.419 Keypair written

Wall-clock from NodePublishVolume to cert available: 44s. Actual issuance work: ~1.5s. The other 42.7s is RenewalBackoff sleeping on a condition (pod IPv6) that resolved within ~2s at the OS level.
Tail-case from the same log stream: one pod waited 4m 43s across three backoff cycles before its IPv6 appeared and issuance succeeded.

The signal we have from the existing interface

type ReadyToRequestFunc func(meta metadata.Metadata) (bool, string)

The signature is (bool, string), not error. I take this as deliberate: the library author modelled gate-pending as a wait state, not a failure. The manager loop is collapsing that distinction by synthesising an error from the not-ready return and routing it through the issuance-error backoff.

The proposal below restores the distinction the interface already implies.

Proposed direction

Three pieces, all small. Concretely:

1. Export a sentinel for gate-pending in manager/interfaces.go:

   var ErrNotReadyToRequest = errors.New("driver is not ready to request a certificate")

2. Wrap the not-ready return inside issue() manager/manager.go:

if ready, reason := m.readyToRequest(meta); !ready {
    return fmt.Errorf("%w: %s", ErrNotReadyToRequest, reason)
}

3. Add GateBackoffConfig to manager.Options, route by error class:

type Options struct {
    // ... existing fields ...

    // RenewalBackoffConfig configures backoff for certificate issuance
    // failures (signer errors, apiserver errors, denials).
    // If nil: 30s base, factor 2.0, jitter 0.5, cap 5min.
    RenewalBackoffConfig *wait.Backoff

    // GateBackoffConfig configures backoff for ReadyToRequestFunc returning
    // false — an expected wait state, distinct from an issuance failure.
    // Gate-pending conditions (e.g. waiting on CNI to assign a pod IP)
    // typically resolve orders of magnitude faster than issuance errors
    // and should be polled more aggressively.
    // If nil: 1s base, factor 2.0, jitter 0.5, cap 10s.
    GateBackoffConfig *wait.Backoff
}

In startRenewalRoutine, replace single wait.ExponentialBackoffWithContext call with manual loop

for {
    if ctx.Err() != nil {
        return
    }
    issueCtx, cancel := context.WithTimeout(ctx, m.issueRenewalTimeout)
    err := m.issue(issueCtx, volumeID)
    cancel()
    if err == nil {
        return
    }

    var delay time.Duration
    if errors.Is(err, ErrNotReadyToRequest) {
        log.V(4).Info("readiness gate not yet met", "reason", err.Error())
        delay = gateBackoff.Step()                   <<<<<< Readiness Gate backoff
        issuanceBackoff = m.backoffConfig   // reset; this isn't an issuance failure
    } else {
        log.Error(err, "Failed to issue certificate, retrying with backoff")
        if m.metrics != nil {
            m.metrics.IncrementIssueErrorCountTotal(m.nodeNameHash, volumeID)
        }
        delay = issuanceBackoff.Step()            <<<<<< Issuance backoff
        gateBackoff = m.gateBackoffConfig   // reset; this isn't a gate-pending wait
    }
    select {
    case <-ctx.Done():
        return
    case <-time.After(delay):
    }
}

Open questions for direction

1. Sentinel-error vs. typed error. ErrNotReadyToRequest as a plain sentinel, or a typed error like type NotReadyError struct{ Reason string }? Sentinel is simpler; typed lets the caller introspect the reason without parsing the wrapped message.
2. Default base of 1s. Acceptable, or do you prefer 2s/3s/5s? I picked 1s based on the IPv6 production data; happy to be guided.

cc @SgtCoDFish — flagging this since you reviewed PR #638 and the apiserver-load concern in csi-driver#646.

Related

• feat: add --pod-readiness-gate flag to defer certificate issuance until pod conditions are met csi-driver#638 (the consumer introducing --pod-readiness-gate).
• cert-manager/csi-driver PR #663 - The apiserver-load fix landing first; precondition for this proposal
• Original ReadyToRequestFunc PR: Add option to skip blocking pod startup if driver is not ready to create a request yet #20

I'd be happy to open the PR once we've aligned on open questions above & overall direction