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

de-duplicate fuse mounting/unmounting code #2199

Conversation

preminger
Copy link
Contributor

@preminger preminger commented Sep 15, 2023

Description of the Pull Request (PR):

Transition as much FUSE-related code as is currently feasible to using internal/pkg/util/fs/fuse package.

This PR addresses FUSE-related code in the following source files:

  • internal/pkg/runtime/launcher/native/launcher_linux.go, and dependencies thereof
  • internal/pkg/runtime/engine/singularity/cleanup_host_linux.go, and dependencies thereof

(These two are related to use of the --sif-fuse flag in native mode.)

This PR does not address FUSE-related code in the following source files (see #2201):

  • internal/pkg/runtime/engine/singularity/container_linux.go
  • internal/pkg/runtime/engine/singularity/prepare_linux.go
  • internal/pkg/runtime/engine/singularity/process_linux.go

(These three are related to passing FUSE mounts by file-descriptor from host to container.)

This fixes or addresses the following GitHub issues:

@preminger preminger force-pushed the 2025-chore-de-duplicate-fuse-mountingunmounting-code branch 2 times, most recently from a550c50 to 21f2008 Compare September 15, 2023 18:28
@preminger preminger marked this pull request as ready for review September 15, 2023 19:02
@preminger preminger changed the title WIP: de-duplicate fuse mounting/unmounting code de-duplicate fuse mounting/unmounting code Sep 15, 2023
Copy link
Member

@dtrudg dtrudg left a comment

Choose a reason for hiding this comment

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

Taking shape nicely. I've left some comments. My chief concern is that extra options must not be allowed to potentially override things that are set from specific attributes of ImageMount.

internal/pkg/runtime/launcher/native/launcher_linux.go Outdated Show resolved Hide resolved
internal/pkg/util/fs/fuse/fuse_mount_linux.go Outdated Show resolved Hide resolved
internal/pkg/util/fs/fuse/fuse_mount_linux.go Outdated Show resolved Hide resolved
@preminger preminger force-pushed the 2025-chore-de-duplicate-fuse-mountingunmounting-code branch 2 times, most recently from b67f8e8 to 1781b79 Compare September 18, 2023 15:53
@preminger preminger force-pushed the 2025-chore-de-duplicate-fuse-mountingunmounting-code branch from 5666227 to a54a580 Compare September 18, 2023 15:58
Copy link
Member

@dtrudg dtrudg 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 LGTM - but I think I missed something r.e. uid / gid handling which should probably at this point mean that they are handled as attributes?

@preminger preminger force-pushed the 2025-chore-de-duplicate-fuse-mountingunmounting-code branch from 831a664 to 4f53e79 Compare September 18, 2023 17:03
Copy link
Member

@dtrudg dtrudg left a comment

Choose a reason for hiding this comment

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

LGTM - noted some typos in comments

internal/pkg/util/fs/fuse/fuse_mount_linux.go Outdated Show resolved Hide resolved
internal/pkg/util/fs/fuse/fuse_mount_linux.go Outdated Show resolved Hide resolved
@preminger preminger force-pushed the 2025-chore-de-duplicate-fuse-mountingunmounting-code branch from aecd206 to 8ced169 Compare September 18, 2023 19:04
@preminger preminger merged commit bfb7a87 into sylabs:main Sep 18, 2023
1 check passed
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.

chore: de-duplicate fuse mounting/unmounting code
2 participants