viash-io / viash

script + metadata = standalone component
https://viash.io
GNU General Public License v3.0
39 stars 2 forks source link

Docker chown fails with multiple outputs defined #21

Closed Grifs closed 3 years ago

Grifs commented 3 years ago

As soon as a file parameter with multilpe outputs is defined chown fails.

The wrapper script seems similar between single and multiple: single:

  if [ ! -z "$VIASH_PAR_OUTPUT" ]; then
    eval docker run --entrypoint=chown -i --rm $VIASH_EXTRA_MOUNTS bash:3.2 "$(id -u):$(id -g)" -R "$VIASH_PAR_OUTPUT"
  fi

multiple:

  if [ ! -z "$VIASH_PAR_OUTPUT" ]; then
    IFS=":"
    for var in $VIASH_PAR_OUTPUT; do
      eval docker run --entrypoint=chown -i --rm $VIASH_EXTRA_MOUNTS bash:3.2 "$(id -u):$(id -g)" -R "$var"
    done
    unset IFS
    VIASH_PAR_OUTPUT="$VIASH_TEST_OUTPUT"
  fi

However, when manually adding an echo with the same parameters to inspect what is passed to eval, I get different results: single:

docker run --entrypoint=chown -i --rm -v "/tmp/viash_tester11849352422085712247:/viash_automount \
  /tmp/viash_tester11849352422085712247" -v "/tmp/viash_tester11849352422085712247:/viash_automount \
  /tmp/viash_tester11849352422085712247" -v "/tmp/viash_tester11849352422085712247:/viash_automount \
  /tmp/viash_tester11849352422085712247" -v "/tmp:/viash_automount/tmp" \
  bash:3.2 1000:1000 -R /viash_automount/tmp/viash_tester11849352422085712247/out.txt

multiple:

docker run --entrypoint=chown -i --rm  -v "/tmp/viash_tester14994981997667224524 /viash_automount \
  /tmp/viash_tester14994981997667224524" -v "/tmp/viash_tester14994981997667224524 /viash_automount \
  /tmp/viash_tester14994981997667224524" -v "/tmp/viash_tester14994981997667224524 /viash_automount \
  /tmp/viash_tester14994981997667224524" -v "/tmp /viash_automount/tmp" \
  bash:3.2 1000:1000 -R /viash_automount/tmp/viash_tester14994981997667224524/out.txt
chown: /viash_automount/tmp/viash_tester14994981997667224524/out.txt: No such file or directory

(note the missing : in the arguments)

Side question; why is the -v part repeated 3 times with the same parameters?

Grifs commented 3 years ago

When adding debug code to check where what goes wrong, I'm starting to believe it starts to go wrong here: (only echo statements added)

if [ ! -z "$VIASH_PAR_OUTPUT" ]; then
  echo VIASH_PAR_OUTPUT $VIASH_PAR_OUTPUT
  echo VIASH_EXTRA_MOUNTS $VIASH_EXTRA_MOUNTS
  IFS=":"
  for var in $VIASH_PAR_OUTPUT; do
    echo VIASH_EXTRA_MOUNTS $VIASH_EXTRA_MOUNTS
    echo var $var
    VIASH_EXTRA_MOUNTS="$VIASH_EXTRA_MOUNTS $(ViashAutodetectMountArg "$var")"
    echo VIASH_EXTRA_MOUNTS $VIASH_EXTRA_MOUNTS
    if [ -z "$VIASH_TEST_OUTPUT" ]; then
      VIASH_TEST_OUTPUT="$(ViashAutodetectMount "$var")"
    else
      VIASH_TEST_OUTPUT="$VIASH_TEST_OUTPUT:""$(ViashAutodetectMount "$var")"
    fi
  done
  unset IFS
  VIASH_PAR_OUTPUT="$VIASH_TEST_OUTPUT"
fi

returns

/tmp/viash_tester14994981997667224524 >>> ./testbash testbash --real_number 1 --whole_number 1 -s abc --output out.txt              [1]
VIASH_PAR_OUTPUT out.txt
VIASH_EXTRA_MOUNTS -v "/tmp/viash_tester14994981997667224524:/viash_automount/tmp/viash_tester14994981997667224524"
VIASH_EXTRA_MOUNTS  -v "/tmp/viash_tester14994981997667224524 /viash_automount/tmp/viash_tester14994981997667224524"
var out.txt
VIASH_EXTRA_MOUNTS  -v "/tmp/viash_tester14994981997667224524 /viash_automount/tmp/viash_tester14994981997667224524" -v "/tmp/viash_tester14994981997667224524 /viash_automount/tmp/viash_tester14994981997667224524"
INFO: Parsed input arguments.
INFO: Writing output to file
docker run --entrypoint=chown -i --rm  -v "/tmp/viash_tester14994981997667224524 /viash_automount/tmp/viash_tester14994981997667224524" -v "/tmp/viash_tester14994981997667224524 /viash_automount/tmp/viash_tester14994981997667224524" -v "/tmp/viash_tester14994981997667224524 /viash_automount/tmp/viash_tester14994981997667224524" -v "/tmp /viash_automount/tmp" bash:3.2 1000:1000 -R /viash_automount/tmp/viash_tester14994981997667224524/out.txt
chown: /viash_automount/tmp/viash_tester14994981997667224524/out.txt: No such file or directory

It can be seen that the echo of VIASH_EXTRA_MOUNTS is different before and after setting the IFS variable.

rcannood commented 3 years ago

Interesting find! Indeed, setting an IFS in order to iterate over $VIASH_PAR_OUTPUT will cause $VIASH_EXTRA_MOUNTS to be interpreted differently. This is illustrated by the following example, where we would want the '-v foo bar' to remain -v foo:bar instead of -v foo bar.

VIASH_EXTRA_MOUNTS="-v foo:bar -v foo2:bar2"
VIASH_PAR_OUTPUT="file1:file2:file3"
IFS=":"
for var in $VIASH_PAR_OUTPUT; do
  echo command $VIASH_EXTRA_MOUNTS $var
done
unset IFS

returns:

command -v foo bar -v foo2 bar2 file1
command -v foo bar -v foo2 bar2 file2
command -v foo bar -v foo2 bar2 file3

A solution seems to be to simply unset the IFS in the for loop straight away:


```bash
VIASH_EXTRA_MOUNTS="-v foo:bar -v foo2:bar2"
VIASH_PAR_OUTPUT="file1:file2:file3"
IFS=":"
for var in $VIASH_PAR_OUTPUT; do
  unset IFS
  echo command $VIASH_EXTRA_MOUNTS $var
done

returns:

command -v foo:bar -v foo2:bar2 file1
command -v foo:bar -v foo2:bar2 file2
command -v foo:bar -v foo2:bar2 file3
rcannood commented 3 years ago

Side question; why is the -v part repeated 3 times with the same parameters?

Probably because you have multiple inputs and outputs in the same folder. When processing the arguments, mounts are added iteratively without checking for duplicates. This could be improved upon :)

rcannood commented 3 years ago

Fixed with ebcf79052a034fd43d5a185e63b2ab0afacbd9a6