feat: add check replica command to identify broken snapshot chains by Hassanzadeh-sd · Pull Request #561 · longhorn/cli

Hassanzadeh-sd · 2026-07-20T14:24:49Z

Which issue(s) this PR fixes:

What this PR does / why we need it:

Adds a new longhornctl check replica command that identifies broken snapshot chains in the Longhorn replica data directories, for the v1 data engine.

The remote command follows the existing DaemonSet pattern (same as longhornctl get replica): it deploys privileged pods that run longhornctl-local check replica against the host data directory on each node, then aggregates the per-node JSON results into YAML.

For every replica directory (filterable with --volume-name and --name), the checker:

reads volume.meta and every volume-head-*.img.meta / volume-snap-*.img.meta disk metadata file
verifies every disk file has a metadata file and vice versa
verifies every Parent reference resolves to an existing disk, including snapshot tree branches that are not part of the volume head chain
verifies the volume head declared in volume.meta exists, and warns about unexpected extra volume-head-*.img files
detects loops in the chain and metadata declaring a mismatching disk name
reports the reachable snapshot chain from the volume head to the root for context

A warning is emitted when the replica is in use, since findings may be transient while the engine is modifying the chain (for example during snapshot creation or purge).

Example output for a replica with a missing parent snapshot:

replicas:
  pvc-48a6457d-585e-423b-b530-bbc68a5f948a-0e2603a7:
    - node: ip-10-0-2-123
      directory: /var/lib/longhorn/replicas/pvc-48a6457d-585e-423b-b530-bbc68a5f948a-0e2603a7
      volumeName: pvc-48a6457d-585e-423b-b530-bbc68a5f948a
      snapshotChain:
        - volume-head-001.img
        - volume-snap-40b3b028-b3b3-4a35-a806-8bea77f27c00.img
      errors:
        - 'broken snapshot chain: disk volume-snap-40b3b028-b3b3-4a35-a806-8bea77f27c00.img references parent volume-snap-6f244bbe-2857-46e4-92e2-eb1e16a63ba1.img, but the parent metadata file is missing'

Special notes for your reviewer:

The snapshot chain validation logic lives in pkg/local/replica/checker.go (validateSnapshotChain) and is covered by unit tests using synthetic replica directories: healthy chain, snapshot tree branch after revert, missing parent, missing disk file, missing metadata file, missing volume head, chain loop, extra head file, corrupted metadata, and name mismatch.
pkg/local/replica/getter.go helper methods getReplicaNamesInDirectory and isReplicaInUse were refactored into package-level functions so the checker can reuse them; no behavior change.
This targets the v1 data engine (sparse-file replica directories). The v2 (SPDK) data engine stores snapshots differently and would need a separate implementation.

Additional documentation or context

docs/longhornctl_check_replica.md is generated with the doc command, following the repo's doc generation flow.
A manual test plan is documented in [IMPROVEMENT] longhornctl supports identifying the broken snapshot chain longhorn#9102.

Hassanzadeh-sd · 2026-07-28T07:23:01Z

Hi @derekbit, this PR is complete from my side and ready for review:

CI is green (DCO, commit-lint, build)
The branch is rebased on master
The snapshot chain validation logic is covered by unit tests (healthy chain, snapshot tree branch after revert, missing parent, missing disk or metadata files, missing volume head, chain loop, extra head file, corrupted metadata, name mismatch)
A manual test plan is documented in [IMPROVEMENT] longhornctl supports identifying the broken snapshot chain longhorn#9102, and I am happy to adjust it based on QA feedback

I would appreciate a review when you have time, and if everything looks good, consideration for merging toward the v1.13.0 milestone. I will address any feedback promptly.

Also, as discussed in the issue, feel free to assign longhorn/longhorn#9102 to me. Thanks!

Add the new check replica command (longhorn/cli#561, Issue longhorn/longhorn#9102) to the longhornctl troubleshooting command list for v1.13.0. Signed-off-by: Sajjad Hassanzadeh <hassanzadeh.sd@gmail.com>

Copilot

Pull request overview

Adds a new longhornctl check replica command (and its longhornctl-local counterpart) to validate v1-engine replica snapshot chains across nodes via a DaemonSet, aggregating per-node JSON into YAML for presentation.

Changes:

Introduces replica-check result types (ReplicaCheckCollection, ReplicaCheckInfo) for JSON/YAML serialization.
Adds remote DaemonSet-based replica checker plus local snapshot-chain validation logic with unit tests.
Wires the new check replica subcommand into both remote and local CLIs and updates generated docs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/types/replica.go	Adds types for replica-check result aggregation/serialization.
pkg/remote/replica/checker.go	Implements remote DaemonSet runner that aggregates per-node JSON into YAML output.
pkg/local/replica/getter.go	Refactors shared helper functions so the checker can reuse replica discovery and “in use” detection.
pkg/local/replica/checker.go	Implements local snapshot-chain integrity validation and JSON output.
pkg/local/replica/checker_test.go	Adds unit tests covering common broken-chain scenarios and edge cases.
pkg/consts/replica.go	Adds DaemonSet app name constant for the replica checker.
cmd/remote/subcmd/check.go	Registers `longhornctl check replica` remote command.
cmd/local/subcmd/check.go	Registers `longhornctl-local check replica` local command.
docs/longhornctl_check.md	Updates generated command index to include `check replica`.
docs/longhornctl_check_replica.md	Adds generated documentation for the new `check replica` command.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Add 'longhornctl check replica' which inspects the snapshot chains in the Longhorn replica data directories on each node and reports integrity issues: - snapshots referencing a missing parent (broken chain) - disk files without metadata files, and metadata files without disk files - missing or corrupted volume.meta and disk metadata files - a missing volume head, unexpected extra volume head files, and metadata declaring a mismatching disk name - loops in the snapshot chain The command follows the existing DaemonSet pattern: the remote command deploys privileged pods that run 'longhornctl-local check replica' against the host data directory, then aggregates the per-node JSON results into YAML. Results can be filtered with --volume-name and --name, and include the reachable snapshot chain from the volume head for context. A warning is emitted when the replica is in use, since findings may be transient while the engine is modifying the chain. Longhorn 9102 Signed-off-by: Sajjad Hassanzadeh <hassanzadeh.sd@gmail.com>

Derive the volume name through a helper that validates the expected <volume-name>-<suffix> pattern instead of slicing on strings.LastIndex directly, which panics with a slice bounds error when a directory under the replicas root does not contain a hyphen (for example a stray manually created directory). Unexpected directory names are now skipped with a warning during discovery, and the getter and checker leave the volume name empty instead of crashing. Longhorn 9102 Signed-off-by: Sajjad Hassanzadeh <hassanzadeh.sd@gmail.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

davidcheng0922 · 2026-08-31T06:14:06Z

+// getVolumeNameFromReplicaDirectoryName derives the volume name from a replica
+// data directory name (<volume-name>-<8-character suffix>). It returns false
+// when the name does not follow the expected pattern.
+func getVolumeNameFromReplicaDirectoryName(replicaDirectoryName string) (string, bool) {
+	index := strings.LastIndex(replicaDirectoryName, "-")
+	if index <= 0 {
+		return "", false
+	}
+	return replicaDirectoryName[:index], true
+}


+		{"pvc-48a6457d-585e-423b-b530-bbc68a5f948a-0e2603a7", "pvc-48a6457d-585e-423b-b530-bbc68a5f948a", true},
+		{"vol-1a2b3c4d", "vol", true},
+		{"no-hyphen-suffix-", "no-hyphen-suffix", true},
+		{"nohyphen", "", false},
+		{"-1a2b3c4d", "", false},


davidcheng0922 · 2026-08-31T05:50:55Z

+	localreplica "github.com/longhorn/cli/pkg/local/replica"
 	"github.com/longhorn/cli/pkg/types"
 	"github.com/longhorn/cli/pkg/utils"
 )


NIT: Re-organize the import package

davidcheng0922 · 2026-08-31T05:56:36Z

+	return cmd
+}
+
+func newCmdCheckReplica(globalOpts *types.GlobalCmdOptions) *cobra.Command {


This seems to only work with v1. We should specify the engine explicitly.

davidcheng0922 · 2026-08-31T05:57:35Z

+	lhmgrutil "github.com/longhorn/longhorn-manager/util"
+
+	commonio "github.com/longhorn/go-common-libs/io"
+
+	"github.com/longhorn/cli/pkg/consts"
+	remote "github.com/longhorn/cli/pkg/remote/replica"
+	"github.com/longhorn/cli/pkg/types"
+	"github.com/longhorn/cli/pkg/utils"
+	utilslonghorn "github.com/longhorn/cli/pkg/utils/longhorn"


NIT: Same, re-organize the import package

davidcheng0922 · 2026-08-31T06:04:54Z

+func (local *Checker) checkReplica(replicaName string) (*types.ReplicaCheckInfo, error) {
+	log := local.logger
+
+	log.Infof("Checking snapshot chain for replica %s", replicaName)
+
+	replicaCheckInfo := &types.ReplicaCheckInfo{}
+	replicaCheckInfo.Node = local.CurrentNodeID
+	if volumeName, ok := getVolumeNameFromReplicaDirectoryName(replicaName); ok {
+		replicaCheckInfo.VolumeName = volumeName
+	}
+
+	replicaDirectory := filepath.Join(local.replicasDirectory, replicaName)
+	replicaCheckInfo.Directory = strings.TrimPrefix(replicaDirectory, consts.VolumeMountHostDirectory)
+
+	isEmpty, err := commonio.IsDirectoryEmpty(replicaDirectory)
+	if err != nil {
+		replicaCheckInfo.Errors = append(replicaCheckInfo.Errors, errors.Wrapf(err, "failed to check if directory %s is empty", replicaCheckInfo.Directory).Error())
+		return replicaCheckInfo, nil
+	}
+
+	if isEmpty {
+		log.Warnf("Replica directory %s is empty", replicaCheckInfo.Directory)
+		replicaCheckInfo.Warnings = append(replicaCheckInfo.Warnings, "replica directory is empty")
+		return replicaCheckInfo, nil
+	}
+
+	isReplicaInUse, err := isReplicaDirectoryInUse(replicaDirectory)
+	if err != nil {
+		replicaCheckInfo.Warnings = append(replicaCheckInfo.Warnings, errors.Wrapf(err, "failed to check if replica %s is in use", replicaName).Error())
+	} else if isReplicaInUse {
+		replicaCheckInfo.Warnings = append(replicaCheckInfo.Warnings, "replica is in use; findings may be transient while the engine is modifying the snapshot chain")
+	}
+
+	chain, checkErrors, warnings := validateSnapshotChain(replicaDirectory)
+	replicaCheckInfo.SnapshotChain = chain
+	replicaCheckInfo.Errors = append(replicaCheckInfo.Errors, checkErrors...)
+	replicaCheckInfo.Warnings = append(replicaCheckInfo.Warnings, warnings...)
+
+	return replicaCheckInfo, nil
+}


The returned error is always nil. If it's not needed, consider removing it.

davidcheng0922 · 2026-08-31T06:09:44Z

+func (remote *Checker) Init() error {
+	kubeClient, err := kubeutils.NewKubeClient("", remote.KubeConfigPath)
+	if err != nil {
+		return err
+	}
+	remote.kubeClient = kubeClient
+
+	remote.appName = consts.AppNameReplicaChecker
+
+	return nil
+}


It looks like this method only supports v1 volumes, as v2 cannot fetch the replica chain here. We should add a validation check for the volume version.

davidcheng0922 · 2026-08-31T06:14:06Z

+// getVolumeNameFromReplicaDirectoryName derives the volume name from a replica
+// data directory name (<volume-name>-<8-character suffix>). It returns false
+// when the name does not follow the expected pattern.
+func getVolumeNameFromReplicaDirectoryName(replicaDirectoryName string) (string, bool) {
+	index := strings.LastIndex(replicaDirectoryName, "-")
+	if index <= 0 {
+		return "", false
+	}
+	return replicaDirectoryName[:index], true
+}


davidcheng0922 · 2026-08-31T06:25:06Z

+
+		chain = append(chain, current)
+		current = diskMeta.Parent
+	}


// Every parent reference must resolve, including the ones on snapshot tree

Do we need to check all snapshot tree branches, not just the head chain?

davidcheng0922 · 2026-08-31T06:25:20Z

+		if metaName := disks[diskName].Name; metaName != "" && metaName != diskName {
+			checkErrors = append(checkErrors, fmt.Sprintf("disk metadata %s%s declares mismatching disk name %s", diskName, ".meta", metaName))
+		}


Do we need an additional check for disks[diskName].Name == ""?

Hassanzadeh-sd force-pushed the feat-check-replica-snapshot-chain branch from d3312fc to ebe7bbc Compare July 26, 2026 06:55

Hassanzadeh-sd mentioned this pull request Jul 26, 2026

[IMPROVEMENT] longhornctl supports identifying the broken snapshot chain longhorn/longhorn#9102

Open

Hassanzadeh-sd mentioned this pull request Aug 9, 2026

docs(1.13.0): add longhornctl check replica to command overview longhorn/website#1414

Open

derekbit force-pushed the feat-check-replica-snapshot-chain branch from ebe7bbc to d5794b3 Compare August 24, 2026 07:21

derekbit requested a lite review from Copilot August 26, 2026 00:49

Copilot started reviewing on behalf of derekbit August 26, 2026 00:50 View session

Copilot AI reviewed Aug 26, 2026

View reviewed changes

Comment thread pkg/local/replica/checker.go

Comment thread pkg/local/replica/getter.go Outdated

derekbit requested a lite review from Copilot August 26, 2026 16:59

Copilot started reviewing on behalf of derekbit August 26, 2026 17:00 View session

Hassanzadeh-sd added 2 commits August 27, 2026 01:01

derekbit force-pushed the feat-check-replica-snapshot-chain branch from 9f10609 to 39f8a6e Compare August 26, 2026 17:01

Copilot AI reviewed Aug 26, 2026

View reviewed changes

davidcheng0922 reviewed Aug 31, 2026

View reviewed changes

Navigation Menu

feat: add check replica command to identify broken snapshot chains - #561

feat: add check replica command to identify broken snapshot chains#561
Hassanzadeh-sd wants to merge 2 commits into
longhorn:masterfrom
Hassanzadeh-sd:feat-check-replica-snapshot-chain

Hassanzadeh-sd commented Jul 20, 2026 •

edited

Loading

Uh oh!

Hassanzadeh-sd commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

davidcheng0922 Aug 31, 2026

Uh oh!

davidcheng0922 Aug 31, 2026

Uh oh!

davidcheng0922 Aug 31, 2026

Uh oh!

davidcheng0922 Aug 31, 2026

Uh oh!

davidcheng0922 Aug 31, 2026

Uh oh!

davidcheng0922 Aug 31, 2026

Uh oh!

davidcheng0922 Aug 31, 2026

Uh oh!

davidcheng0922 Aug 31, 2026

Uh oh!

davidcheng0922 Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Hassanzadeh-sd commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue(s) this PR fixes:

What this PR does / why we need it:

Special notes for your reviewer:

Additional documentation or context

Uh oh!

Hassanzadeh-sd commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Hassanzadeh-sd commented Jul 20, 2026 •

edited

Loading