From d29216c4c00fc04a2a5e8220caf1a0741812fd57 Mon Sep 17 00:00:00 2001 From: "Neumoin, Konstantin" Date: Thu, 6 Aug 2026 10:44:16 +0300 Subject: [PATCH] [snapshot] restore --wait: name the PVCs it does not await --wait already finishes without polling a dormant WaitForFirstConsumer claim, but it only mentioned each one in a progress line and summed them up as a counter, so an operator could not tell which restored claims the command stopped tracking. Collect those claims during the existing classification pass and write one list, before the remaining claims are polled: namespace/name, the StorageClass that was actually resolved (the cluster default is reported by its own name, not as an empty field), and its volumeBindingMode. The list is part of the command result, so it goes to an injectable writer wired to cmd.OutOrStdout() and a failed write fails the restore instead of exiting successfully with incomplete output. The classification itself is unchanged: which claims are awaited still follows selected node, live consumer, and provisioning events. A wait timeout now names the claim, its last observed status.phase, and states that the restored objects were already applied and are not rolled back. Help no longer implies that every restored PVC is awaited until Bound. Signed-off-by: Neumoin, Konstantin --- internal/snapshot/cmd/restore/restore.go | 25 +- internal/snapshot/restore/restore.go | 149 ++++- internal/snapshot/restore/wait_report_test.go | 606 ++++++++++++++++++ 3 files changed, 747 insertions(+), 33 deletions(-) create mode 100644 internal/snapshot/restore/wait_report_test.go diff --git a/internal/snapshot/cmd/restore/restore.go b/internal/snapshot/cmd/restore/restore.go index 1869c731..404a7ddf 100644 --- a/internal/snapshot/cmd/restore/restore.go +++ b/internal/snapshot/cmd/restore/restore.go @@ -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 @@ -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 } @@ -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", diff --git a/internal/snapshot/restore/restore.go b/internal/snapshot/restore/restore.go index 542a7ef6..25c20b78 100644 --- a/internal/snapshot/restore/restore.go +++ b/internal/snapshot/restore/restore.go @@ -39,6 +39,7 @@ import ( "io" "log/slog" "os" + "slices" "strings" "time" @@ -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. @@ -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 } @@ -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 @@ -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 @@ -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 { @@ -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 } @@ -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)) @@ -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], @@ -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 ( @@ -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 } } @@ -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 diff --git a/internal/snapshot/restore/wait_report_test.go b/internal/snapshot/restore/wait_report_test.go new file mode 100644 index 00000000..f6613441 --- /dev/null +++ b/internal/snapshot/restore/wait_report_test.go @@ -0,0 +1,606 @@ +/* +Copyright 2026 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restore + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + "time" + + "k8s.io/apimachinery/pkg/runtime" + dynamicfake "k8s.io/client-go/dynamic/fake" + clienttesting "k8s.io/client-go/testing" +) + +const unawaitedReportHeader = "The following restored PersistentVolumeClaims are not awaited:" + +func wffcClass(name string) resolvedStorageClass { + return resolvedStorageClass{name: name, bindingMode: volumeBindingModeWFC} +} + +// TestReportUnawaitedPVCs renders the list directly, including the cross-namespace +// ordering that a single-namespace Run cannot produce. +func TestReportUnawaitedPVCs(t *testing.T) { + t.Parallel() + + t.Run("no claims writes nothing", func(t *testing.T) { + t.Parallel() + + var out bytes.Buffer + + if err := reportUnawaitedPVCs(&out, nil); err != nil { + t.Fatalf("reportUnawaitedPVCs with no claims: %v", err) + } + + if out.Len() != 0 { + t.Errorf("empty claim list wrote %q, want no output", out.String()) + } + }) + + t.Run("orders by namespace then name", func(t *testing.T) { + t.Parallel() + + var out bytes.Buffer + + claims := []unawaitedPVC{ + {namespace: "beta", name: "pvc-b", storageClass: wffcClass("sc")}, + {namespace: "alpha", name: "pvc-z", storageClass: wffcClass("sc")}, + {namespace: "beta", name: "pvc-a", storageClass: wffcClass("sc")}, + {namespace: "alpha", name: "pvc-a", storageClass: wffcClass("sc")}, + } + + if err := reportUnawaitedPVCs(&out, claims); err != nil { + t.Fatalf("reportUnawaitedPVCs: %v", err) + } + + assertOrderedIdentities(t, out.String(), []string{ + "alpha/pvc-a", + "alpha/pvc-z", + "beta/pvc-a", + "beta/pvc-b", + }) + }) + + t.Run("failed write is reported to the caller", func(t *testing.T) { + t.Parallel() + + claims := []unawaitedPVC{ + {namespace: testNS, name: "pvc-1", storageClass: wffcClass("sc")}, + } + + err := reportUnawaitedPVCs(failingWriter{}, claims) + if err == nil { + t.Fatal("expected a write failure to be returned") + } + + if !strings.Contains(err.Error(), "pipe") { + t.Errorf("error %q does not preserve the write failure", err.Error()) + } + }) +} + +// TestRun_Wait_UnawaitedReport_NotWrittenWithoutWait proves the report is a property of +// the wait: without --wait no StorageClass is read for it and nothing is written. +func TestRun_Wait_UnawaitedReport_NotWrittenWithoutWait(t *testing.T) { + t.Parallel() + + src := &stubSource{body: mustArray(t, pvcManifestSC("pvc-1", "Pending", "wffc-sc"))} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + storageClassObj("wffc-sc", volumeBindingModeWFC, false), + restoredPVCObject("pvc-1", pvcPhasePending, "wffc-sc", false), + ) + + var out bytes.Buffer + + cfg := baseConfig(src, dyn) + cfg.Out = &out + + if err := Run(context.Background(), cfg); err != nil { + t.Fatalf("Run without --wait: %v", err) + } + + if out.Len() != 0 { + t.Errorf("run without --wait wrote %q, want no output", out.String()) + } + + if got := countStorageClassCalls(dyn); got != 0 { + t.Errorf("run without --wait made %d StorageClass calls, want none", got) + } +} + +// TestRun_Wait_UnawaitedReport_NoPVCsWritesNothing covers a restore whose manifest set +// holds no PersistentVolumeClaim at all. +func TestRun_Wait_UnawaitedReport_NoPVCsWritesNothing(t *testing.T) { + t.Parallel() + + src := &stubSource{body: mustArray(t, configMapManifest("cm-1"))} + dyn := newFakeDynamic(readySnapshot()) + + var out bytes.Buffer + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = time.Second + cfg.Out = &out + + if err := Run(context.Background(), cfg); err != nil { + t.Fatalf("Run with --wait and no PVCs: %v", err) + } + + if out.Len() != 0 { + t.Errorf("restore without PVCs wrote %q, want no output", out.String()) + } +} + +// TestRun_Wait_UnawaitedReport_SingleDormantWFFC is the reported case: one dormant WFFC +// claim is named once, the restore succeeds, and that claim is never polled. +func TestRun_Wait_UnawaitedReport_SingleDormantWFFC(t *testing.T) { + t.Parallel() + + src := &stubSource{body: mustArray(t, pvcManifestSC("pvc-1", "Pending", "wffc-sc"))} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + storageClassObj("wffc-sc", volumeBindingModeWFC, false), + restoredPVCObject("pvc-1", pvcPhasePending, "wffc-sc", false), + ) + + var out bytes.Buffer + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = time.Second + cfg.Out = &out + + if err := Run(context.Background(), cfg); err != nil { + t.Fatalf("Run with one dormant WFFC PVC: %v", err) + } + + report := out.String() + + if got := strings.Count(report, unawaitedReportHeader); got != 1 { + t.Fatalf("report header appears %d times, want exactly one block: %q", got, report) + } + + for _, want := range []string{ + testNS + "/pvc-1", + `StorageClass "wffc-sc"`, + volumeBindingModeWFC, + } { + if !strings.Contains(report, want) { + t.Errorf("report %q does not contain %q", report, want) + } + } + + // Two preflight reads plus the one classification read: a polled claim would add more. + if got := countPVCGets(dyn, "pvc-1"); got != 3 { + t.Errorf("PVC GET count = %d, want two preflights and one classification read", got) + } +} + +// TestRun_Wait_UnawaitedReport_NamesTheResolvedDefaultStorageClass covers a claim with no +// spec.storageClassName: the report must name the default StorageClass the wait actually +// read, because that object is what the caller has to inspect. +func TestRun_Wait_UnawaitedReport_NamesTheResolvedDefaultStorageClass(t *testing.T) { + t.Parallel() + + src := &stubSource{body: mustArray(t, pvcManifestSC("pvc-1", "Pending", ""))} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + storageClassObj("cluster-default-sc", volumeBindingModeWFC, true), + storageClassObj("other-sc", volumeBindingModeImmediate, false), + restoredPVCObject("pvc-1", pvcPhasePending, "", false), + ) + + var out bytes.Buffer + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = time.Second + cfg.Out = &out + + if err := Run(context.Background(), cfg); err != nil { + t.Fatalf("Run with an empty storageClassName on a WFFC default: %v", err) + } + + report := out.String() + + if !strings.Contains(report, `StorageClass "cluster-default-sc"`) { + t.Errorf("report %q does not name the resolved default StorageClass", report) + } + + if strings.Contains(report, `StorageClass ""`) { + t.Errorf("report %q leaves the StorageClass name empty", report) + } +} + +// TestRun_Wait_UnawaitedReport_WriteFailureFailsTheRestore pins that the list is a +// required part of the result: if it cannot be written, the command reports that instead +// of exiting successfully with incomplete output. +func TestRun_Wait_UnawaitedReport_WriteFailureFailsTheRestore(t *testing.T) { + t.Parallel() + + src := &stubSource{body: mustArray(t, pvcManifestSC("pvc-1", "Pending", "wffc-sc"))} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + storageClassObj("wffc-sc", volumeBindingModeWFC, false), + restoredPVCObject("pvc-1", pvcPhasePending, "wffc-sc", false), + ) + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = time.Second + cfg.Out = failingWriter{} + + err := Run(context.Background(), cfg) + if err == nil { + t.Fatal("expected an unwritable result to fail the restore") + } + + for _, want := range []string{"restored objects were applied", "broken pipe"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not contain %q", err.Error(), want) + } + } +} + +// TestRun_Wait_UnawaitedReport_MultipleClaimsOneBlock verifies several dormant WFFC +// claims share one sorted block and do not re-read their common StorageClass. +func TestRun_Wait_UnawaitedReport_MultipleClaimsOneBlock(t *testing.T) { + t.Parallel() + + src := &stubSource{body: mustArray(t, + pvcManifestSC("pvc-c", "Pending", "wffc-sc"), + pvcManifestSC("pvc-a", "Pending", "wffc-sc"), + pvcManifestSC("pvc-b", "Pending", "wffc-sc"), + )} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + storageClassObj("wffc-sc", volumeBindingModeWFC, false), + restoredPVCObject("pvc-a", pvcPhasePending, "wffc-sc", false), + restoredPVCObject("pvc-b", pvcPhasePending, "wffc-sc", false), + restoredPVCObject("pvc-c", pvcPhasePending, "wffc-sc", false), + ) + + var out bytes.Buffer + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = time.Second + cfg.Out = &out + + if err := Run(context.Background(), cfg); err != nil { + t.Fatalf("Run with three dormant WFFC PVCs: %v", err) + } + + report := out.String() + + if got := strings.Count(report, unawaitedReportHeader); got != 1 { + t.Fatalf("report header appears %d times, want one block for all claims: %q", got, report) + } + + assertOrderedIdentities(t, report, []string{ + testNS + "/pvc-a", + testNS + "/pvc-b", + testNS + "/pvc-c", + }) + + if got := countStorageClassCalls(dyn); got != 1 { + t.Errorf("StorageClass calls = %d, want one cached lookup for the shared class", got) + } + + for _, name := range []string{"pvc-a", "pvc-b", "pvc-c"} { + if got := countPVCGets(dyn, name); got != 3 { + t.Errorf("PVC %s GET count = %d, want no polling round", name, got) + } + } +} + +// TestRun_Wait_UnawaitedReport_MixedSetListsOnlyUnawaited keeps the existing wait +// semantics for an Immediate claim while naming only the dormant WFFC one. +func TestRun_Wait_UnawaitedReport_MixedSetListsOnlyUnawaited(t *testing.T) { + t.Parallel() + + src := &stubSource{body: mustArray(t, + pvcManifestSC("pvc-wffc", "Pending", "wffc-sc"), + pvcManifestSC("pvc-immediate", "", "immediate-sc"), + )} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + storageClassObj("wffc-sc", volumeBindingModeWFC, false), + storageClassObj("immediate-sc", volumeBindingModeImmediate, false), + restoredPVCObject("pvc-wffc", pvcPhasePending, "wffc-sc", false), + restoredPVCObject("pvc-immediate", pvcPhasePending, "immediate-sc", false), + boundPVObject("pvc-immediate"), + ) + + immediateGets := 0 + dyn.PrependReactor("get", "persistentvolumeclaims", func(action clienttesting.Action) (bool, runtime.Object, error) { + getAction, ok := action.(clienttesting.GetAction) + if !ok || getAction.GetName() != "pvc-immediate" { + return false, nil, nil + } + + immediateGets++ + if immediateGets <= 2 { + return false, nil, nil + } + + return true, restoredPVCObject("pvc-immediate", pvcPhaseBound, "immediate-sc", false), nil + }) + + var out bytes.Buffer + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = time.Second + cfg.Out = &out + + if err := Run(context.Background(), cfg); err != nil { + t.Fatalf("Run with a mixed WFFC/Immediate set: %v", err) + } + + report := out.String() + + if !strings.Contains(report, testNS+"/pvc-wffc") { + t.Errorf("report %q does not name the unawaited WFFC claim", report) + } + + if strings.Contains(report, "pvc-immediate") { + t.Errorf("report %q names the Immediate claim, which is still awaited", report) + } + + if immediateGets <= 2 { + t.Errorf("Immediate PVC GET count = %d, want it to be awaited past the preflights", immediateGets) + } +} + +// TestRun_Wait_UnawaitedReport_ActiveWFFCIsStillAwaited pins that the classification is +// unchanged: a WFFC claim with a selected node is polled as before and is not listed. +func TestRun_Wait_UnawaitedReport_ActiveWFFCIsStillAwaited(t *testing.T) { + t.Parallel() + + pvc := restoredPVCObject("pvc-active", pvcPhasePending, "wffc-sc", false) + pvc.SetAnnotations(map[string]string{selectedNodeAnnotation: "node-a"}) + + src := &stubSource{body: mustArray(t, pvcManifestSC("pvc-active", "", "wffc-sc"))} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + storageClassObj("wffc-sc", volumeBindingModeWFC, false), + pvc, + boundPVObject("pvc-active"), + ) + + pvcGets := 0 + dyn.PrependReactor("get", "persistentvolumeclaims", func(clienttesting.Action) (bool, runtime.Object, error) { + pvcGets++ + if pvcGets <= 3 { + return false, nil, nil + } + + return true, restoredPVCObject("pvc-active", pvcPhaseBound, "wffc-sc", false), nil + }) + + var out bytes.Buffer + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = time.Second + cfg.Out = &out + + if err := Run(context.Background(), cfg); err != nil { + t.Fatalf("Run with an active WFFC PVC: %v", err) + } + + if out.Len() != 0 { + t.Errorf("an awaited WFFC claim was reported as unawaited: %q", out.String()) + } + + if pvcGets != 4 { + t.Errorf("PVC GET count = %d, want two preflights, inspection, and Bound poll", pvcGets) + } +} + +// TestRun_Wait_UnawaitedReport_WrittenBeforeAWaitFailure proves the block survives a +// later failure of an awaited claim: it is written before the wait, and the error is +// returned unchanged. +func TestRun_Wait_UnawaitedReport_WrittenBeforeAWaitFailure(t *testing.T) { + t.Parallel() + + src := &stubSource{body: mustArray(t, + pvcManifestSC("pvc-wffc", "Pending", "wffc-sc"), + pvcManifestSC("pvc-immediate", "Pending", "immediate-sc"), + )} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + storageClassObj("wffc-sc", volumeBindingModeWFC, false), + storageClassObj("immediate-sc", volumeBindingModeImmediate, false), + restoredPVCObject("pvc-wffc", pvcPhasePending, "wffc-sc", false), + restoredPVCObject("pvc-immediate", pvcPhasePending, "immediate-sc", false), + ) + + var out bytes.Buffer + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = 20 * time.Millisecond + cfg.Out = &out + + err := Run(context.Background(), cfg) + if err == nil { + t.Fatal("expected the never-binding Immediate PVC to time out") + } + + if !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("error %q does not wrap context.DeadlineExceeded", err.Error()) + } + + if !strings.Contains(out.String(), testNS+"/pvc-wffc") { + t.Errorf("report %q was not written before the wait failure", out.String()) + } +} + +// TestRun_Wait_TimeoutNamesTheClaimAndAppliedObjects checks the timeout states the claim, +// its last observed phase, and that the apply already happened -- and claims no cause. +func TestRun_Wait_TimeoutNamesTheClaimAndAppliedObjects(t *testing.T) { + t.Parallel() + + src := &stubSource{body: mustArray(t, pvcManifest("pvc-1", "Pending"))} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + restoredPVCObject("pvc-1", pvcPhasePending, "", false), + ) + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = time.Millisecond + + err := Run(context.Background(), cfg) + if err == nil { + t.Fatal("expected a wait timeout, got nil") + } + + for _, want := range []string{ + testNS + "/pvc-1", + "Bound", + `status.phase was "Pending"`, + "already applied", + "not rolled back", + } { + if !strings.Contains(err.Error(), want) { + t.Errorf("timeout error %q does not contain %q", err.Error(), want) + } + } + + if !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("error %q does not wrap context.DeadlineExceeded", err.Error()) + } +} + +// TestRun_Wait_UnawaitedReport_WriterIsInjectedAndDeterministic proves the report goes +// only to the injected writer and that the restore result does not depend on it: the same +// run with a discarding default (no terminal, no stdin) succeeds identically. +func TestRun_Wait_UnawaitedReport_WriterIsInjectedAndDeterministic(t *testing.T) { + t.Parallel() + + run := func(t *testing.T, out *bytes.Buffer) { + t.Helper() + + src := &stubSource{body: mustArray(t, pvcManifestSC("pvc-1", "Pending", "wffc-sc"))} + dyn := newFakeDynamic( + readySnapshot(), + readyVolumeSnapshot("vs-1"), + storageClassObj("wffc-sc", volumeBindingModeWFC, false), + restoredPVCObject("pvc-1", pvcPhasePending, "wffc-sc", false), + ) + + cfg := baseConfig(src, dyn) + cfg.Wait = true + cfg.Timeout = time.Second + + if out != nil { + cfg.Out = out + } + + if err := Run(context.Background(), cfg); err != nil { + t.Fatalf("Run with a dormant WFFC PVC: %v", err) + } + + if got := countPVCGets(dyn, "pvc-1"); got != 3 { + t.Errorf("PVC GET count = %d, want the same classification regardless of the writer", got) + } + } + + var captured bytes.Buffer + + run(t, &captured) + + if !strings.Contains(captured.String(), unawaitedReportHeader) { + t.Errorf("injected writer received %q, want the report block", captured.String()) + } + + // No writer configured: the default discards instead of reaching the process stdout, + // and the restore behaves identically. + run(t, nil) +} + +func assertOrderedIdentities(t *testing.T, report string, identities []string) { + t.Helper() + + previous := -1 + + for _, identity := range identities { + index := strings.Index(report, identity) + if index < 0 { + t.Fatalf("report %q does not contain %q", report, identity) + } + + if index < previous { + t.Errorf("report %q does not list %q in namespace/name order", report, identity) + } + + previous = index + } +} + +func countPVCGets(dyn *dynamicfake.FakeDynamicClient, name string) int { + count := 0 + + for _, action := range dyn.Actions() { + if action.GetVerb() != "get" || action.GetResource() != pvcGVR { + continue + } + + if getAction, ok := action.(clienttesting.GetAction); ok && getAction.GetName() == name { + count++ + } + } + + return count +} + +func countStorageClassCalls(dyn *dynamicfake.FakeDynamicClient) int { + count := 0 + + for _, action := range dyn.Actions() { + if action.GetResource() == scGVR { + count++ + } + } + + return count +} + +// failingWriter stands in for a closed result stream, such as a piped stdout whose reader +// exited. +type failingWriter struct{} + +func (failingWriter) Write([]byte) (int, error) { + return 0, errors.New("broken pipe") +}