Skip to content

Remove target check when server clustered #2195

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

Jump to bottom
Merged
merged 1 commit into from
Jun 16, 2025
Merged

Remove target check when server clustered #2195

merged 1 commit into from
Jun 16, 2025

Conversation

presztak
Copy link
Collaborator

@presztak presztak commented Jun 13, 2025 •
edited
Loading

When the server is part of a cluster and the source volume is on different target, we don't need to check the target parameter. In such cases, clusterCopyCustomVolumeInternal should always be used.

That said, when executing the command:
incus storage volume copy local/bug-test local/bug-test-copy
if the source volume is located on a different cluster node, the system should always call clusterCopyCustomVolumeInternal.

Closes: #2178

@presztak
incusd: Remove target check when server clustered
8375a12 
When the server is part of a cluster and the source volume is
on different target, we no longer need to check the 'target'
parameter. In such cases, clusterCopyCustomVolumeInternal
should always be used.

Signed-off-by: Piotr Resztak <piotr.resztak@futurfusion.io>
@presztak presztak marked this pull request as ready for review June 13, 2025 10:34
@presztak presztak requested a review from stgraber as a code owner June 13, 2025 10:34
@stgraber stgraber enabled auto-merge June 13, 2025 13:05
@stgraber stgraber disabled auto-merge June 16, 2025 03:57
@stgraber stgraber merged commit 896ee71 into lxc:main Jun 16, 2025
168 of 174 checks passed
@stgraber
Copy link
Member

Merging despite the Linstor failure as it's already affecting main.

@bensmrs any idea what's up with Linstor?

@bensmrs
Copy link
Contributor

bensmrs commented Jun 16, 2025

I’m afraid I can’t reproduce locally… and I’m having trouble understanding how this PR could in any way influence the standalone check.

@stgraber
Copy link
Member

@bensmrs yeah, this PR doesn't that's why I merged it anyway. We're seeing the same failure for main builds prior to this being merged so there's something weird going on with Linstor, at least in our test environment.

@bensmrs
Copy link
Contributor

bensmrs commented Jun 16, 2025 •
edited
Loading

@luissimas, could you see if the standalone tests run on Ubuntu? Works on my Debian™
I’m quite confused by the error though, as I don’t see how snapshot names could possibly get so long (we’re normally only using our prefix + the UUID without dashes). The actual error in all the messy error messages is Name "linstor_linstor--incus-incus--volume--178c8981584e4a52ad9d8923cfe821c2_00000_safety--snap--394e5244--04f9--480b--a2f8--7fb3791b692c" too long.

@presztak presztak deleted the copy_volume_cluster branch July 28, 2025 17:05
@luissimas
Copy link
Contributor

Hey @bensmrs! Sorry for the late reply on this one. Is this still an issue?

This safety--snap component of the snapshot name seems like something Linstor specific.

https://github.com/LINBIT/linstor-server/blob/2680d60d0059b44842d60e805b771081c0c656b8/controller/src/main/java/com/linbit/linstor/core/apicallhandler/controller/CtrlSnapshotRollbackApiCallHandler.java#L238-L243

Since this is an internal snapshot that Linstor itself creates, it seems weird that it would hit the 48 characters limit. But IIRC the constraint for the name length is at the database level, so it is possible that this is what's causing the issue.

@bensmrs
Copy link
Contributor

bensmrs commented Jul 28, 2025

Hi!
The issue appeared, then disappeared. It’s one of these transient things with LINSTOR that I like to keep hidden under the rug. I’ll re-ping you when it comes back :D

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

@stgraber stgraber stgraber approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Copying a storage volume in a cluster fails on local storage if destination is a different cluster member
4 participants
@presztak @stgraber @bensmrs @luissimas