Skip to content
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.

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

csi-wrapper: pass PublishContext from ControllerPublishVolume to NodeStageVolume #2108

Merged

Conversation

daniel-weisse
Copy link
Contributor

A CSI driver may rely on the output of ControllerPublishVolume during the NodeStageVolume call.
Since the csi-wrapper simply creates a fake ControllerPublishVolumeResponse, information may be lost between the two calls.
Since the peerpod volume caches the real response, we can use this during the wrapper's NodeStageVolume to supply the driver with the real values.

@daniel-weisse daniel-weisse requested a review from a team as a code owner October 11, 2024 07:00
@stevenhorsman
Copy link
Member

@yoheiueda - are you able to review this when you get a chance as I think you worked on the csi-wrapper feature?

Copy link
Member

@yoheiueda yoheiueda left a comment

Choose a reason for hiding this comment

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

This is a good catch! Thank you very much.

src/csi-wrapper/pkg/wrapper/podvmservice.go Outdated Show resolved Hide resolved
Copy link
Member

@yoheiueda yoheiueda left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you very much!

@daniel-weisse daniel-weisse force-pushed the keep-publish-context branch 2 times, most recently from abfdb50 to 1557eb4 Compare October 17, 2024 07:45
When reproducing `NodeStageVolume` for the pod VM,
pass the `PublishContext` of the reproduced `ControllerPublishVolume`
to the `NodeStageVolumeRequest` of the Pod VM.

Signed-off-by: Daniel Weiße <dw@edgeless.systems>
…lume

The fake information should never be needed by the CSI driver, but may
potentially cause issues if the driver performs strict checking on its
input values

Signed-off-by: Daniel Weiße <dw@edgeless.systems>
@katexochen katexochen merged commit 488681f into confidential-containers:main Oct 18, 2024
19 checks passed
@daniel-weisse daniel-weisse deleted the keep-publish-context branch October 18, 2024 07:39
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