Skip to content

fix: preserve backend volume associations during reconciliation#1167

Open
s-starostin wants to merge 1 commit into
NetApp:masterfrom
s-starostin:fix-concurrent-backend-volume-associations
Open

fix: preserve backend volume associations during reconciliation#1167
s-starostin wants to merge 1 commit into
NetApp:masterfrom
s-starostin:fix-concurrent-backend-volume-associations

Conversation

@s-starostin

Copy link
Copy Markdown

Change description

Preserve backend volume associations during concurrent backend reconciliation.

When controller concurrency is enabled, periodic backend reconciliation may replace an existing backend object with a newly initialized backend object. The replacement backend starts with an empty backend-local volume map.

The global volume cache remains intact, so tridentctl get volume continues to return all managed volumes with the correct backend UUIDs. However, tridentctl get backend reports VOLUMES=0 because the replacement backend no longer contains the existing volume associations.

The empty backend-local volume map may also affect node access reconciliation because backend volume associations are consulted before the replacement backend is written to the concurrent cache.

This change copies the existing backend-local volume associations from the original backend to the replacement backend before node access reconciliation and cache replacement.

The copy is performed entirely in memory and does not call the storage driver for every volume. Explicit backend update workflows continue to use the existing volume validation path where storage-side validation is required.

Fixes #1166

Project tracking

  • This PR does not require a JIRA ticket (explain below why)

External community bug fix tracked by GitHub issue #1166 .

Do any added TODOs have an issue in the backlog?

No TODOs added.

Did you add unit tests? Why not?

Yes.

Added focused unit coverage for copying backend volume associations, including:

  • matching backend UUIDs;
  • different backend UUIDs;
  • nil backend inputs;
  • preservation of the original backend volume map.

Extended concurrent backend reconciliation coverage to verify that a backend replacement caused by a backend pools change preserves all existing backend volume associations.

The regression test verifies that both the global volume cache and the replacement backend-local volume map remain populated after reconcileBackendState.

Does this code need functional testing?

Yes.

Targeted functional validation is recommended because the issue is triggered by periodic backend reconciliation with controller concurrency enabled.

The functional validation should:

  1. Enable controller concurrency.
  2. Confirm that tridentctl get backend reports the expected volume counts after bootstrap.
  3. Wait for or trigger periodic backend reconciliation.
  4. Confirm that backend volume counts remain unchanged.
  5. Confirm that tridentctl get volume continues to return the same volumes and backend UUIDs.
  6. Validate volume create, publish, unpublish, and delete operations after reconciliation.

Is a code review walkthrough needed? why or why not?

Yes, a brief walkthrough is recommended.

The code change is small, but its placement is important. Existing volume associations must be copied to the replacement backend before node access reconciliation and before the backend is replaced in the concurrent cache.

The walkthrough should also confirm that the existing concurrent cache locking guarantees prevent volume association changes while the backend replacement is in progress.

Should additional test coverage be executed in addition to pre-merge?

Yes.

In addition to the standard pre-merge test suite, run the focused concurrent-core tests with the Go race detector and repeat the reconciliation regression test multiple times.

Recommended validation:

go test ./core \
  -run 'TestCopyBackendVolumes|TestReconcileBackendStateConcurrentCore' \
  -count=1

go test -race ./core \
  -run 'TestCopyBackendVolumes|TestReconcileBackendStateConcurrentCore' \
  -count=20

go test -race ./core \
  -run 'TestReconcileBackendStateConcurrentCore/ReconcileBackendPoolsChangePreservesVolumes' \
  -count=100

Does this code need a note in the changelog?

/needs-changelog

This affects backend state reporting and backend-to-volume associations when controller concurrency is enabled.

Changelog

Fixed concurrent backend reconciliation to preserve existing backend volume associations when replacing a backend object.

Does this code require documentation changes?

No.

Additional Information

The issue was reproduced with controller concurrency enabled.

Observed behavior:

  • all backends remained online and normal;
  • tridentctl get backend reported VOLUMES=0;
  • the backend JSON contained empty volumes arrays;
  • tridentctl get volume continued to return all managed volumes with valid backend UUIDs;
  • disabling controller concurrency and restarting the controller restored the backend volume counts.

The behavior was reproduced on released versions v26.02.1 and v26.06.0.

Example observed state on v26.06.0:

$ tridentctl -n trident get backend -o json | \
    jq -r '.items[] | [.name, .backendUUID, ((.volumes // []) | length)] | @tsv'

netapp-qos-01-02  6d32238f-fbc2-4f3e-9e35-b9d79850d867  0
netapp-qos-04-02  08415f87-5c6d-44ad-aada-bead9e5d644d  0
netapp-qos-03-01  26973da6-5606-464a-a61c-3768cf9ca675  0
netapp-qos-03-02  3fa59e32-c0cf-4061-9cdc-e3f8c3858682  0
netapp-qos-04-01  3c42334b-4ca0-41bb-9033-190d6be11e0b  0
netapp-qos-01-01  89e43b0a-e8b0-4829-9442-a682ba4a39d3  0

At the same time, the global volume cache remained populated:

$ tridentctl -n trident get volume -o json | \
    jq -r '.items[] | .backendUUID' | sort | uniq -c

364 08415f87-5c6d-44ad-aada-bead9e5d644d
683 26973da6-5606-464a-a61c-3768cf9ca675
346 3c42334b-4ca0-41bb-9033-190d6be11e0b
153 3fa59e32-c0cf-4061-9cdc-e3f8c3858682
 34 6d32238f-fbc2-4f3e-9e35-b9d79850d867
 24 89e43b0a-e8b0-4829-9442-a682ba4a39d3

@torirevilla

Copy link
Copy Markdown
Contributor

If you wish to contribute to Trident please complete the necessary steps outlined in the CONTRIBUTING.md file, including the signed CCLA.

Sign and submit NetApp's Contributor License Agreement. You must sign and submit the Corporate Contributor License Agreement (CCLA) in order to contribute.

@s-starostin

Copy link
Copy Markdown
Author

Thanks. The CCLA has been submitted for VI.Tech and includes my GitHub username, @s-starostin.

Please let me know if any additional information is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concurrent backend reconciliation clears backend volume associations

2 participants