Skip to content

Commit

Permalink
server: allow Bidirectional mounts that contain storage root
Browse files Browse the repository at this point in the history
previously we would never allow a container to get anything other than HostToContainer
for something that contains the storage root. This breaks situations where the container wishes to update
mount points within the container and see them propegated to the host.

Instead, we should only override this if the mount is Private.

Signed-off-by: Peter Hunt <pehunt@redhat.com>
  • Loading branch information
haircommander authored and kwilczynski committed Oct 31, 2023
1 parent 49307f1 commit dde9ff5
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
2 changes: 1 addition & 1 deletion server/container_create_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ func addOCIBindMounts(ctx context.Context, ctr ctrfactory.Container, mountLabel,
log.Warnf(ctx, "Configuration specifies mounting host root to the container root. This is dangerous (especially with privileged containers) and should be avoided.")
}

if isSubDirectoryOf(storageRoot, m.HostPath) {
if isSubDirectoryOf(storageRoot, m.HostPath) && m.Propagation == types.MountPropagation_PROPAGATION_PRIVATE {
log.Infof(ctx, "Mount propogration for the host path %s will be set to HostToContainer as it includes the container storage root", m.HostPath)
m.Propagation = types.MountPropagation_PROPAGATION_HOST_TO_CONTAINER

Check warning on line 984 in server/container_create_linux.go

View check run for this annotation

Codecov / codecov/patch

server/container_create_linux.go#L983-L984

Added lines #L983 - L984 were not covered by tests
}
Expand Down
36 changes: 36 additions & 0 deletions test/ctr.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,42 @@ function check_oci_annotation() {
! crictl create "$pod_id" "$TESTDIR/config" "$TESTDATA"/sandbox_config.json
}

@test "ctr that mounts container storage as shared should keep shared" {
# parent of `--root`, keep in sync with test/helpers.bash
PARENT_DIR="$TESTDIR"
CTR_DIR="/host"
jq --arg path "$PARENT_DIR" --arg ctr_dir "$CTR_DIR" \
' .mounts = [ {
host_path: $path,
container_path: $ctr_dir,
propagation: 2
} ]' \
"$TESTDATA"/container_redis.json > "$TESTDIR/config"

start_crio

ctr_id=$(crictl run "$TESTDIR/config" "$TESTDATA"/sandbox_config.json)
crictl exec --sync "$ctr_id" findmnt -no TARGET,PROPAGATION "$CTR_DIR" | grep shared
}

@test "ctr that mounts container storage as private should not be private" {
# parent of `--root`, keep in sync with test/helpers.bash
PARENT_DIR="$TESTDIR"
CTR_DIR="/host"
jq --arg path "$PARENT_DIR" --arg ctr_dir "$CTR_DIR" \
' .mounts = [ {
host_path: $path,
container_path: $ctr_dir,
propagation: 1
} ]' \
"$TESTDATA"/container_redis.json > "$TESTDIR/config"

start_crio

ctr_id=$(crictl run "$TESTDIR/config" "$TESTDATA"/sandbox_config.json)
crictl exec --sync "$ctr_id" findmnt -no TARGET,PROPAGATION "$CTR_DIR" | grep -v private
}

@test "ctr has containerenv" {
start_crio
pod_id=$(crictl runp "$TESTDATA"/sandbox_config.json)
Expand Down

0 comments on commit dde9ff5

Please sign in to comment.