From 9b77bcb2cba8e272799fa21e2d484e9f6e7c34d0 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Fri, 2 Jun 2017 15:24:30 -0400 Subject: [PATCH] Use systemd mount unit file for extra volume mount in compat mode We were using systemd mount unit file for extra volume mount. Then we wanted to extend container-storage-setup for use by other run times and we started doing mounts inline and got rid of dependency on systemd mount unit. But this does not work for compatibility mode. And reason being that now it is possible that volume device is not up by the time container-storage-setup runs and that means we will not mount it and either try to create new or return. We can't even wait for device to come up as in comaptibility mode we don't save any metadata. So we don't know if we are running for first time and we should create volume or we are restarting and we should wait for volume. So go back to creating a systemd mount unit file for mounting extra volumes. Only for compatibility mode. In non-compatibility mode, we need to explicitly activate configuration and that means by that time volume has already been created and that means we can wait for volume. Signed-off-by: Vivek Goyal Closes: #245 Approved by: cgwalters --- container-storage-setup.sh | 86 +++++++++++++++++++++++++++++++++++++- 1 file changed, 84 insertions(+), 2 deletions(-) diff --git a/container-storage-setup.sh b/container-storage-setup.sh index ed98c60..5048ffa 100755 --- a/container-storage-setup.sh +++ b/container-storage-setup.sh @@ -1054,7 +1054,7 @@ extra_lv_mountpoint() { local vg=$1 local lv_name=$2 local mount_dir=$3 - mounts=$(findmnt -n -o TARGET --source /dev/$vg/$lv_name | grep $mount_dir) + mounts=$(findmnt -n -o TARGET --source /dev/$vg/$lv_name | grep "^$mount_dir") echo $mounts } @@ -1109,6 +1109,88 @@ setup_extra_volume() { fi } +# This is used only in compatibility mode. We are still using systemd +# mount unit only for compatibility mode. Reason being that upon service +# restart, we run into races and device might not yet be up when we try +# to mount it. And we don't know if this is first time start and we need +# to create volume or this is restart and we need to wait for device. So +# continue to use systemd mount unit for compatibility mode. +setup_systemd_mount_unit_compat() { + local filename + local vg=$1 + local lv_name=$2 + local mount_dir=$3 + local unit_file_path + + # filename must match the path ${mount_dir}. + # e.g if ${mount_dir} is /var/lib/containers + # then filename will be var-lib-containers.mount + filename=$(systemd_escaped_filename ${mount_dir}) + unit_file_path="/etc/systemd/system/$filename" + + # If unit file already exists, nothing to do. + [ -f "$unit_file_path" ] && return 0 + + cat < "${unit_file_path}.tmp" +# WARNING: This file was auto generated by container-storage-setup. Do not +# edit it. In the future, this file might be moved to a different location. +[Unit] +Description=Mount $lv_name on $mount_dir directory. +Before=docker-storage-setup.service + +[Mount] +What=/dev/$vg/$lv_name +Where=${mount_dir} +Type=xfs +Options=defaults + +[Install] +WantedBy=docker-storage-setup.service +EOF + mv "${unit_file_path}.tmp" "$unit_file_path" + systemctl daemon-reload + systemctl enable $filename >/dev/null 2>&1 + systemctl start $filename +} + +setup_extra_lv_fs_compat() { + [ -z "$_RESOLVED_MOUNT_DIR_PATH" ] && return 0 + if ! setup_extra_dir $_RESOLVED_MOUNT_DIR_PATH; then + return 1 + fi + # If we are restarting, then extra volume should exist. This unit + # file is dependent on extra volume mount unit file. That means this + # code should run after mount unit has activated successfully. That + # means after extra volume has come up. + + # We had got rid of this logic and reintroducing it back. That means + # there can be configurations out there which have extra volume but + # don't have unit file. So in such case, drop a unit file now. This + # is still racy though. There is no guarantee that volume will be + # up by the time this code runs when unit file is not present already. + if extra_volume_exists $CONTAINER_ROOT_LV_NAME $VG; then + if ! setup_systemd_mount_unit_compat "$VG" "$CONTAINER_ROOT_LV_NAME" "$_RESOLVED_MOUNT_DIR_PATH"; then + Fatal "Failed to setup systemd mount unit for extra volume $CONTAINER_ROOT_LV_NAME." + fi + return 0 + fi + + if [ -z "$CONTAINER_ROOT_LV_SIZE" ]; then + Fatal "Specify a valid value for CONTAINER_ROOT_LV_SIZE." + fi + if ! check_data_size_syntax $CONTAINER_ROOT_LV_SIZE; then + Fatal "CONTAINER_ROOT_LV_SIZE value $CONTAINER_ROOT_LV_SIZE is invalid." + fi + # Container runtime extra volume does not exist. Create one. + if ! setup_extra_volume $CONTAINER_ROOT_LV_NAME $_RESOLVED_MOUNT_DIR_PATH $CONTAINER_ROOT_LV_SIZE; then + Fatal "Failed to setup extra volume $CONTAINER_ROOT_LV_NAME." + fi + + if ! setup_systemd_mount_unit_compat "$VG" "$CONTAINER_ROOT_LV_NAME" "$_RESOLVED_MOUNT_DIR_PATH"; then + Fatal "Failed to setup systemd mount unit for extra volume $CONTAINER_ROOT_LV_NAME." + fi +} + setup_extra_lv_fs() { [ -z "$_RESOLVED_MOUNT_DIR_PATH" ] && return 0 if ! setup_extra_dir $_RESOLVED_MOUNT_DIR_PATH; then @@ -1246,7 +1328,7 @@ setup_storage_compat() { # Set up a separate named ($CONTAINER_ROOT_LV_NAME) volume # for $CONTAINER_ROOT_LV_MOUNT_PATH. - if ! setup_extra_lv_fs; then + if ! setup_extra_lv_fs_compat; then Error "Failed to setup logical volume for $CONTAINER_ROOT_LV_MOUNT_PATH." return 1 fi