Skip to content

Conversation

@AndrewChubatiuk
Copy link
Contributor

@AndrewChubatiuk AndrewChubatiuk commented Jul 26, 2025

currently it's only possible to set external PVC for vlsingle and vmsingle only with crutches: storageDataPath should be set using spec.extraArgs, since default or one, which is controlled by spec.storageDataPath creates PVC from spec.storage or mounts emptyDir if spec.storage is not set. any volume mounts that are mounted to spec.storageDataPath lead to collision. Even with this crutch pod has unneeded volume mount, also in vmsingle backup functionality becomes unavailable, since it also relies on spec.storageDataPath value.

this PR moves method, which mounts storage volume to build package and does the following:

  • adds volume mount that points to storageDataPath only if volume with expected name data is not already present in spec.volumeMounts
  • adds volume with managed PVC if spec.storage is defined
  • adds emptyDir volume only storage volume is not found in spec.volumes
  • fails if any volume with other name, than data mount path intersects with storageDataPath
  • fails if volume with name data doesn't intersect with storageDataPath

fixes #784

@AndrewChubatiuk AndrewChubatiuk force-pushed the mount-existing-volume-as-storage branch 2 times, most recently from 2bc9689 to 96465ef Compare July 27, 2025 07:37
@f41gh7 f41gh7 self-assigned this Jul 29, 2025
@AndrewChubatiuk AndrewChubatiuk force-pushed the mount-existing-volume-as-storage branch from 96465ef to c3b6a72 Compare December 2, 2025 13:43
@AndrewChubatiuk AndrewChubatiuk force-pushed the mount-existing-volume-as-storage branch 6 times, most recently from 73fe37c to 80f8959 Compare December 3, 2025 00:34
},
RetentionPeriod: "1",
RemovePvcAfterDelete: true,
StorageDataPath: "/tmp/",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be checking that StorageDataPath is listed in one of the volumes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated PR

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking about extending tests too:

Expect(ts.Volumes[0].EmptyDir).ToNot(BeNil())
Expect(ts.Containers[0].VolumeMounts[0].MountPath).To(Equal("/tmp/"))

and so on in other testcases too

@AndrewChubatiuk AndrewChubatiuk force-pushed the mount-existing-volume-as-storage branch 3 times, most recently from 846d359 to de2f69b Compare December 3, 2025 10:48
@f41gh7
Copy link
Collaborator

f41gh7 commented Dec 3, 2025

Lets be careful with this change. It potentially may break exist deployments of VMSingle instances, I'd like to keep it backward-compatible.

@AndrewChubatiuk AndrewChubatiuk force-pushed the mount-existing-volume-as-storage branch from de2f69b to 90f6e48 Compare December 5, 2025 14:23
@AndrewChubatiuk
Copy link
Contributor Author

@f41gh7 @vrutkovs
updated implementation and description

@AndrewChubatiuk AndrewChubatiuk force-pushed the mount-existing-volume-as-storage branch 2 times, most recently from 088ef81 to f9bc48f Compare December 5, 2025 15:42
@AndrewChubatiuk AndrewChubatiuk changed the title vlsingle, vmsingle: do not mount emptyDir if data volume is present in spec.volumes vlsingle, vmsingle, vtsingle: do not mount emptyDir if data volume is present in spec.volumes Dec 8, 2025
@AndrewChubatiuk AndrewChubatiuk force-pushed the mount-existing-volume-as-storage branch from f9bc48f to 3130702 Compare December 8, 2025 14:05
@lexfrei
Copy link

lexfrei commented Dec 8, 2025

Related to #1677

@AndrewChubatiuk AndrewChubatiuk force-pushed the mount-existing-volume-as-storage branch from 3130702 to 352c9d7 Compare December 9, 2025 12:03
@AndrewChubatiuk AndrewChubatiuk force-pushed the mount-existing-volume-as-storage branch from 352c9d7 to e6fd498 Compare December 9, 2025 12:26
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.

VMStroage pod can't start when an additional volume mount to the storageDataPath

4 participants