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

backwards compatibility for file:///abs-path #44

Merged
merged 8 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.8.0
0.8.1
33 changes: 23 additions & 10 deletions ci/tests/test.bats
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ EOF
run_srun sh -c 'findmnt /user-environment && findmnt /user-profilers && findmnt /user-tools'
}

@test "uenv_mount_list_environment_backward_compat" {
# older versions of squashfs-mount used `file://` prefix for <abs-path> in UENV_MOUNT_LIST
# check that if images have been mounted `uenv --uenv=...`, the slurm plugin recogines UENV_MOUNT_LIST and mounts the same images
export UENV_MOUNT_LIST="file://${SQFSDIR}/binaries.sqfs,${SQFSDIR}/profilers.sqfs:/user-profilers,file://${SQFSDIR}/tools.sqfs:/user-tools"
run_srun sh -c 'findmnt /user-environment && findmnt /user-profilers && findmnt /user-tools'
}

@test "sbatch_override_in_srun" {
# check that images mounted via sbatch --uenv are overriden when `--uenv` flag is given to srun
run_sbatch <<EOF
Expand All @@ -95,10 +102,21 @@ srun --uenv=${SQFSDIR}/binaries.sqfs:/user-tools bash -c '! findmnt /user-enviro
EOF
}

@test "plain_sbatch" {
# check that images mounted via sbatch --uenv are overriden when `--uenv` flag is given to srun
run_sbatch <<EOF
#!/bin/bash
srun true
EOF
}

@test "empty_uenv_mount_list" {
UENV_MOUNT_LIST= srun true
}

@test "duplicate_mountpoints_fail" {
run_srun_unchecked --uenv ${SQFSDIR}/binaries.sqfs,${SQFSDIR}/tools.sqfs true
assert_output --partial 'Duplicate mountpoints found'

}

@test "duplicate_image_fails" {
Expand All @@ -112,12 +130,7 @@ EOF
assert_output --partial 'Invalid syntax for --uenv'
}

@test "empty_argument1" {
run_srun_unchecked --uenv='' true
assert_output --partial 'No mountpoints given.'
}

@test "empty_argument2" {
run_srun_unchecked --uenv=,,, true
assert_output --partial 'No mountpoints given.'
}
# @test "empty_argument1" {
# run_srun_unchecked --uenv='' true
# assert_output --partial 'No mountpoints given.'
# }
3 changes: 2 additions & 1 deletion src/lib/mount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ namespace util {

util::expected<std::string, std::string>
do_mount(const std::vector<mount_entry> &mount_entries) {
if (mount_entries.size() == 0)
if (mount_entries.size() == 0) {
return "nothing to mount";
}
if (unshare(CLONE_NEWNS) != 0) {
return util::unexpected("Failed to unshare the mount namespace");
}
Expand Down
9 changes: 5 additions & 4 deletions src/lib/parse_args.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@

namespace util {

const std::regex default_pattern("(" LINUX_ABS_FPATH ")"
const std::regex default_pattern("^(?:(file:\\/\\/))?"
"(" LINUX_ABS_FPATH ")"
"(:" LINUX_ABS_FPATH ")?",
std::regex::ECMAScript);
// match <image-name>/?<version>:?<tag>:?<abs-mount-path>
Expand Down Expand Up @@ -72,7 +73,7 @@ parse_arg(const std::string &arg, std::optional<std::string> uenv_repo_path,
std::vector<std::string> arguments = util::split(arg, ',', true);

if (arguments.empty()) {
return util::unexpected("No mountpoints given.");
return {};
}

auto get_mount_point = [](std::ssub_match sub_match) -> std::string {
Expand All @@ -85,8 +86,8 @@ parse_arg(const std::string &arg, std::optional<std::string> uenv_repo_path,
std::vector<util::mount_entry> mount_entries;
for (auto &entry : arguments) {
if (std::smatch match; std::regex_match(entry, match, default_pattern)) {
std::string image_path = match[1];
std::string mount_point = get_mount_point(match[2]);
std::string image_path = match[2];
std::string mount_point = get_mount_point(match[3]);
mount_entries.emplace_back(util::mount_entry{image_path, mount_point});
} else if (std::smatch match;
std::regex_match(entry, match, repo_pattern)) {
Expand Down
4 changes: 3 additions & 1 deletion src/plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ int init_post_opt_remote(spank_t sp,
auto abs_mount = *util::realpath(entry.mount_point);
env_var += "file://" + abs_image + ":" + abs_mount + ",";
}
spank_setenv(sp, UENV_MOUNT_LIST, env_var.c_str(), 1);
if (mount_entries.size() > 0) {
spank_setenv(sp, UENV_MOUNT_LIST, env_var.c_str(), 1);
}

return ESPANK_SUCCESS;
}
Expand Down
Loading