Skip to content

[snapshot] restore --wait: name the PVCs it does not await #442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kneumoin wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[snapshot] restore --wait: name the PVCs it does not await #442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions internal/snapshot/cmd/restore/restore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,18 @@ Before any dry-run or real apply, restore reads every target PersistentVolumeCla
same-named claim is already Bound, restore refuses to reuse it because that status may
describe stale data from an earlier operation. Restore never deletes or replaces the claim.

--wait only tracks PersistentVolumeClaims that appear in the restored manifest set. Disk-backed
PVCs for domain objects are recreated asynchronously by the domain controller (not part of this
output), so they are not awaited; the command may return before such volumes finish provisioning.
A Pending PVC on a WaitForFirstConsumer StorageClass completes immediately only while it has no
selected node, live Pod consumer, active provisioning event, or terminal provisioning failure.
Once selected, consumed, or provisioning, it is polled until Bound or --timeout; an observed
provisioning failure is returned with its cause. Success revalidates the post-apply PVC UID,
restore source, and bound PersistentVolume identity.`,
--wait only tracks PersistentVolumeClaims that appear in the restored manifest set, and it
reports when this command's own wait is over, not when a workload can use the restored data.
Disk-backed PVCs for domain objects are recreated asynchronously by the domain controller (not
part of this output), so they are not awaited; the command may return before such volumes finish
provisioning. A Pending PVC on a WaitForFirstConsumer StorageClass is not awaited while it has no
selected node, live Pod consumer, active provisioning event, or terminal provisioning failure:
every such claim is listed once, by namespace/name and StorageClass, as a claim this command does
not wait to become Bound. Once selected, consumed, or provisioning, it is polled until Bound
or --timeout; an observed provisioning failure is returned with its cause. An expired --timeout
means only that the wait could not be confirmed within that period: the objects were already
applied and are not rolled back. Success revalidates the post-apply PVC UID, restore source, and
bound PersistentVolume identity; it does not promise that every restored PVC is Bound.`,
Example: ` # Restore snapshot "my-snap" in namespace "default"
d8 snapshot restore my-snap -n default

Expand Down Expand Up @@ -177,8 +181,8 @@ restore source, and bound PersistentVolume identity.`,
cmd.Flags().Bool(flagDryRun, false, "validate objects via DryRunAll without persisting; skips --wait (use to preflight a restore)")
cmd.Flags().Bool(flagEdit, false, "open resolved manifests in $KUBE_EDITOR/$EDITOR (falling back to vi) before applying; aborts on non-zero exit, unchanged, or empty content")
cmd.Flags().Bool(flagNoAutoEdit, false, "disable the one automatic editor session for interactive Kubernetes Invalid (HTTP 422) preflight responses")
cmd.Flags().Bool(flagWait, false, "wait for manifest PVCs to become Bound; a dormant WaitForFirstConsumer PVC with no selected node, live consumer, or provisioning signal completes without polling")
cmd.Flags().Duration(flagTimeout, 10*time.Minute, "timeout for the --wait Bound check")
cmd.Flags().Bool(flagWait, false, "wait for applicable manifest PVCs to become Bound; a dormant WaitForFirstConsumer PVC with no selected node, live consumer, or provisioning signal is listed by name and is not awaited")
cmd.Flags().Duration(flagTimeout, 10*time.Minute, "bounds the --wait Bound check only; an expired timeout does not roll back the applied objects")

return cmd
}
Expand Down Expand Up @@ -319,6 +323,7 @@ func Run(log *slog.Logger, cmd *cobra.Command, args []string) error {
Mapper: mapper,
ControlPlaneTimeout: restore.DefaultControlPlaneTimeout,
Log: log,
Out: cmd.OutOrStdout(),
}

log.Info("starting snapshot restore",
Expand Down
149 changes: 126 additions & 23 deletions internal/snapshot/restore/restore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"io"
"log/slog"
"os"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -224,6 +225,9 @@ type Config struct {
Mapper meta.RESTMapper
// Log receives progress output.
Log *slog.Logger
// Out receives stable user-facing result output; progress and diagnostics go to Log.
// Command callers pass cmd.OutOrStdout(); the zero value discards.
Out io.Writer

// maxStagedManifestBytes and maxStagedManifestObjects are test seams for
// lowering the finite aggregate staging budgets.
Expand Down Expand Up @@ -685,6 +689,10 @@ func applyDefaults(cfg Config) Config {
cfg.Log = slog.Default()
}

if cfg.Out == nil {
cfg.Out = io.Discard
}

if cfg.editManifests == nil {
cfg.editManifests = editManifestsContext
}
Expand Down Expand Up @@ -2259,6 +2267,10 @@ func applyTargetExists(
// before polling: a Pending claim with no selected node, live consumer, or provisioning
// observation is a normal, non-blocking state. Once provisioning is active, its PVC and
// bounded Event history are both rechecked until Bound or a terminal result.
//
// Every claim classified as not requiring a Bound observation is reported to cfg.Out by
// name, once, before the remaining claims are polled, so the wait never ends with an
// unexplained count of claims it stopped tracking.
func waitPVCsBound(ctx context.Context, cfg Config, pvcs []pvcRef) error {
if len(pvcs) == 0 {
return nil
Expand All @@ -2277,7 +2289,7 @@ func waitPVCsBound(ctx context.Context, cfg Config, pvcs []pvcRef) error {

cfg.Log.Info("waiting for restored PVCs to bind", slog.Int("count", len(pvcs)))

bindingModes := make(map[string]string)
storageClasses := make(map[string]resolvedStorageClass)

type waitRef struct {
pvc pvcRef
Expand All @@ -2287,17 +2299,17 @@ func waitPVCsBound(ctx context.Context, cfg Config, pvcs []pvcRef) error {
activeRefs := make([]waitRef, 0, len(pvcs))

var (
boundCount int
skippedCount int
boundCount int
unawaitedPVCs []unawaitedPVC
)

for _, ref := range pvcs {
mode, err := resolveVolumeBindingMode(waitCtx, cfg, scGVR, ref.storageClassName, bindingModes)
storageClass, err := resolveStorageClass(waitCtx, cfg, scGVR, ref.storageClassName, storageClasses)
if err != nil {
return fmt.Errorf("resolve volume binding mode for PVC %s/%s: %w", ref.namespace, ref.name, err)
}

recheckProvisioning := mode == volumeBindingModeWFC
recheckProvisioning := storageClass.bindingMode == volumeBindingModeWFC
if recheckProvisioning {
active, bound, err := inspectWFFCPVC(waitCtx, cfg, gvr, ref)
if err != nil {
Expand All @@ -2311,7 +2323,11 @@ func waitPVCsBound(ctx context.Context, cfg Config, pvcs []pvcRef) error {
}

if !active {
skippedCount++
unawaitedPVCs = append(unawaitedPVCs, unawaitedPVC{
namespace: ref.namespace,
name: ref.name,
storageClass: storageClass,
})

continue
}
Expand All @@ -2320,6 +2336,10 @@ func waitPVCsBound(ctx context.Context, cfg Config, pvcs []pvcRef) error {
activeRefs = append(activeRefs, waitRef{pvc: ref, recheckProvisioning: recheckProvisioning})
}

if err := reportUnawaitedPVCs(cfg.Out, unawaitedPVCs); err != nil {
return fmt.Errorf("restored objects were applied, but the result could not be written: %w", err)
}

for len(activeRefs) > 0 {
unresolvedRefs := make([]waitRef, 0, len(activeRefs))
unresolvedPhases := make([]string, 0, len(activeRefs))
Expand Down Expand Up @@ -2358,7 +2378,8 @@ func waitPVCsBound(ctx context.Context, cfg Config, pvcs []pvcRef) error {
return waitContextError(
waitCtx,
fmt.Sprintf(
"waiting for PVC %s/%s to become Bound; observed phase %q",
"waiting for restored PVC %s/%s to become Bound; its last observed status.phase was %q;"+
" the restored objects were already applied and are not rolled back by the end of this wait",
firstRef.namespace,
firstRef.name,
unresolvedPhases[0],
Expand All @@ -2369,25 +2390,105 @@ func waitPVCsBound(ctx context.Context, cfg Config, pvcs []pvcRef) error {

cfg.Log.Info("finished waiting for restored PVCs",
slog.Int("bound", boundCount),
slog.Int("skipped_wait_for_first_consumer", skippedCount))
slog.Int("skipped_wait_for_first_consumer", len(unawaitedPVCs)))

return nil
}

// resolveVolumeBindingMode returns the effective volumeBindingMode for a PVC's
// StorageClass, resolving the cluster's default StorageClass when className is empty
// (spec.storageClassName can be legitimately unset). Results are cached per StorageClass
// name so a restore with many PVCs on the same class issues one API call per class, not
// one per PVC; the empty-name case is cached under a distinct key since it requires a
// List rather than a Get.
func resolveVolumeBindingMode(ctx context.Context, cfg Config, scGVR schema.GroupVersionResource, className string, cache map[string]string) (string, error) {
// resolvedStorageClass is the StorageClass a claim binds through, as read during the wait.
type resolvedStorageClass struct {
// name is the resolved object's name. For a claim with an empty
// spec.storageClassName this is the cluster default that was found, not "".
name string
// bindingMode is the class's effective volumeBindingMode, defaulted to Immediate
// when the class omits the field.
bindingMode string
}

// unawaitedPVC is one restored PersistentVolumeClaim that the wait classified as not
// requiring a Bound observation. The claim is kept by identity, not only counted, because
// a count alone does not say which claims the command stopped tracking.
type unawaitedPVC struct {
namespace string
name string
storageClass resolvedStorageClass
}

// reportUnawaitedPVCs writes the single list of restored PVCs the wait does not await,
// ordered by namespace then name. It states only what the wait actually read -- claim
// identity, resolved StorageClass, and its volumeBindingMode -- and does not tell the
// caller what to do next. Nothing is written when every restored claim is awaited.
func reportUnawaitedPVCs(out io.Writer, claims []unawaitedPVC) error {
if out == nil || len(claims) == 0 {
return nil
}

sorted := slices.Clone(claims)
slices.SortFunc(sorted, func(a, b unawaitedPVC) int {
if byNamespace := strings.Compare(a.namespace, b.namespace); byNamespace != 0 {
return byNamespace
}

return strings.Compare(a.name, b.name)
})

identityWidth := 0

for _, claim := range sorted {
if width := len(claim.namespace) + len("/") + len(claim.name); width > identityWidth {
identityWidth = width
}
}

var block strings.Builder

block.WriteString("\nThe following restored PersistentVolumeClaims are not awaited:\n\n")

for _, claim := range sorted {
fmt.Fprintf(
&block,
" %-*s StorageClass %q volumeBindingMode %s\n",
identityWidth,
claim.namespace+"/"+claim.name,
claim.storageClass.name,
claim.storageClass.bindingMode,
)
}

block.WriteString(
"\nThese claims use StorageClasses with volumeBindingMode WaitForFirstConsumer.\n" +
"They are not expected to become Bound until a consumer is scheduled, so this\n" +
"restore does not wait for them.\n\n",
)

if _, err := io.WriteString(out, block.String()); err != nil {
return fmt.Errorf("writing the list of %d PVCs that are not awaited: %w", len(sorted), err)
}

return nil
}

// resolveStorageClass returns the StorageClass a PVC actually binds through: the resolved
// object's name and its effective volumeBindingMode. The cluster's default StorageClass is
// resolved when className is empty (spec.storageClassName can be legitimately unset), so
// the returned name is the class that was really read, not the claim's field. Results are
// cached per StorageClass name so a restore with many PVCs on the same class issues one API
// call per class, not one per PVC; the empty-name case is cached under a distinct key since
// it requires a List rather than a Get.
func resolveStorageClass(
ctx context.Context,
cfg Config,
scGVR schema.GroupVersionResource,
className string,
cache map[string]resolvedStorageClass,
) (resolvedStorageClass, error) {
cacheKey := className
if cacheKey == "" {
cacheKey = "\x00default"
}

if mode, ok := cache[cacheKey]; ok {
return mode, nil
if resolved, ok := cache[cacheKey]; ok {
return resolved, nil
}

var (
Expand All @@ -2409,20 +2510,21 @@ func resolveVolumeBindingMode(ctx context.Context, cfg Config, scGVR schema.Grou
err = errors.Join(err, ctxErr)
}

return "", fmt.Errorf("get StorageClass %q: %w", className, err)
return resolvedStorageClass{}, fmt.Errorf("get StorageClass %q: %w", className, err)
}
} else {
sc, err = findDefaultStorageClass(ctx, cfg, scGVR)
if err != nil {
return "", err
return resolvedStorageClass{}, err
}

if sc == nil {
cfg.Log.Info("no default StorageClass is annotated; assuming Immediate binding for PVCs with an empty storageClassName")

cache[cacheKey] = volumeBindingModeImmediate
resolved := resolvedStorageClass{bindingMode: volumeBindingModeImmediate}
cache[cacheKey] = resolved

return volumeBindingModeImmediate, nil
return resolved, nil
}
}

Expand All @@ -2431,9 +2533,10 @@ func resolveVolumeBindingMode(ctx context.Context, cfg Config, scGVR schema.Grou
mode = volumeBindingModeImmediate
}

cache[cacheKey] = mode
resolved := resolvedStorageClass{name: sc.GetName(), bindingMode: mode}
cache[cacheKey] = resolved

return mode, nil
return resolved, nil
}

// findDefaultStorageClass returns the cluster's default StorageClass (annotated
Expand Down
Loading
Loading