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") +}