Skip to content

Fix race conditions on unmount, and verify volume is actually attached to the correct node#7

Merged
shahedex merged 4 commits intoNexGenCloud:mainfrom
Thunder-Compute:jack/update-volume-unmount-verification
Oct 3, 2025
Merged

Fix race conditions on unmount, and verify volume is actually attached to the correct node#7
shahedex merged 4 commits intoNexGenCloud:mainfrom
Thunder-Compute:jack/update-volume-unmount-verification

Conversation

@jackowfish
Copy link
Copy Markdown
Contributor

@jackowfish jackowfish commented Oct 2, 2025

Right now, there isn't any polling on volume unpublished, or any verification that a disk is actually attached to the correct node in the volume publish. Adding both checks here to avoid mounting in another containers volume. Could be missing the mark here on root cause - this is a WIP as I test this on our staging cluster

@nqngo nqngo requested a review from shafinhasnat October 3, 2025 01:09
@brianmodel
Copy link
Copy Markdown
Contributor

Adding here, I don't think this was actually the issue we were running into. I believe the source of the issue is that the Hyperstack Volumes API returns the wrong device mount point. Specifically, the where the volume is actually mounted on the node after a AttachVolumeToNode call to the API and *(*getVolume.Attachments)[0].Device are different. I think that after mounts are removed these values get out of sync in the backend.

Copy link
Copy Markdown
Contributor

@shafinhasnat shafinhasnat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

},
}, nil
attachedInstanceIdStr := *(*getVolume.Attachments)[0].InstanceId
attachedInstanceId, err := strconv.Atoi(attachedInstanceIdStr)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int type in strconv

}
if *v.Status == "in-use" && len(*v.Attachments) > 0 {
attachment := (*v.Attachments)[0]
attachedId, err := strconv.Atoi(*attachment.InstanceId)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Int type set to strconv

@shahedex shahedex merged commit 2733dc4 into NexGenCloud:main Oct 3, 2025
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.

4 participants