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

fix: VFS Disk Cache Group Permissions, Merged Manifests Folder, is_mount checks #235

Merged
merged 6 commits into from
Mar 25, 2024

Conversation

baxeaz
Copy link
Contributor

@baxeaz baxeaz commented Mar 24, 2024

  • Adding group owner permissions to vfs_disk_cache/cas_prefix.
  • Writing merged manifests to new .vfs_manifests subfolder under session folder rather than /tmp.
  • Using new VFSProcessManager.is_mount rather than os.path.ismount to propery check for mounts owned by job-user

What was the problem/requirement? (What/Why)

  • The asset disk cache created under the session folder (session_dir/.vfs_object_cache) created a corresponding cas prefix when used (session_dir/.vfs_object_cache/my/cas/prefix) subfolder but did not apply group permissions to the entire folder structure before launching the vfs, preventing job-user access
  • VFS Merged manifests were written out to a temporary file in /tmp and stuck around after session completion
  • When merging manifests the os.path.ismount check was still used which does not properly return true for mounts owned by other users (such as when deadline-worker is checking for the mount owned by job-user)

What was the solution? (How)

  • Add group owner permissions to the folder structure from the beginning of the asset disk cache to the leaf of the cas prefix
  • Write merged manifests to a new session_dir/.vfs_manifests folder
  • Switch os.path.ismount to VFSProcessManager.is_mount

What is the impact of this change?

  • The on disk cache feature should work properly for deadline_vfs instances started by job-user
  • Merged manifests should be properly cleaned up when the session folder is cleaned
  • Manifests for existing mounts should merge properly

How was this change tested?

New test added which pass. Manual testing on CMF instances.

Was this change documented?

No

Is this a breaking change?

No

…g merged manifests to new merged_manifests subfolder under session folder rather than /tmp. Using new VFSProcessManager.is_mount rather than os.path.ismount to propery check for mounts owned by job-user

Signed-off-by: Brian Axelson <[email protected]>
@baxeaz baxeaz requested a review from a team as a code owner March 24, 2024 15:36
baxeaz added 3 commits March 24, 2024 15:57
Signed-off-by: Brian Axelson <[email protected]>
…g merged manifests to new merged_manifests subfolder under session folder rather than /tmp. Using new VFSProcessManager.is_mount rather than os.path.ismount to propery check for mounts owned by job-user

Signed-off-by: Brian Axelson <[email protected]>
src/deadline/job_attachments/download.py Outdated Show resolved Hide resolved
src/deadline/job_attachments/download.py Outdated Show resolved Hide resolved
src/deadline/job_attachments/download.py Outdated Show resolved Hide resolved
Signed-off-by: Brian Axelson <[email protected]>
@baxeaz baxeaz requested a review from mwiebe March 25, 2024 01:16
@baxeaz baxeaz enabled auto-merge (squash) March 25, 2024 12:10
@baxeaz baxeaz merged commit 30dac3d into mainline Mar 25, 2024
18 checks passed
@baxeaz baxeaz deleted the baxelson/vfscachemanifest branch March 25, 2024 12:19
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.

3 participants