Petr, this is you code, would you look on this?
Sure, seems the problem has been introduced by the following commit: http://git.fedorahosted.org/git/?p=beakerlib.git;a=commitdiff;h=ab63f048 Where "readlink -f" is used to get the full path of the file to be backed up. It expands to the real file name instead to keep the symlink. I'll revert the change, which purpose seems to be only "get rid of that long ugly regexp" (which worked better).
Created attachment 456421 [details] Patch: backing up symlinks + test suite adjustment Adding patch. Includes a small adjustment to the assertRun() to be able to expect multiple values (as rlRun() does). * fixed symlink backup * added test coverage for the bug * tiny test suite adjustment [multiple expected values] Please, review.
Thank you, Petr, for digging into this. Yes, it seems that replacing the readlink command with that ugly regexp solves the issue :-)
Great, thanks for verifying! Any objections to the subtle test suite enhancement from anybody else? Otherwise I'll push.
(In reply to comment #5) >Any objections to the subtle test > suite enhancement from anybody else? Otherwise I'll push. > - if [ "$status" -eq "$expected" ]; then > + if [[ "$status" =~ ^$expected$ ]]; then no objections
Thanks. Pushed to git: http://git.fedorahosted.org/git/?p=beakerlib.git;a=commitdiff;h=63a93da
I have experienced the issue too. It would be cool if it is resolved.
Please note that symlink support is crucial in RHEL7. Adjusting bug summary
increasing priority/severity, I have several tests failing on RHEL7 because of deleted items in /var/run, rlFileRestore breaks the environment for other tests.
Pasting reproducer. Imagine /var/run instead of /tmp/run and you see why we are f****d up on RHEL7. # mkdir -p /run/a /run/b # ln -s /run /tmp/run You have mail in /var/spool/mail/root # ls -l /tmp/run lrwxrwxrwx 1 root root 4 Jul 19 04:10 /tmp/run -> /run # ls -l /tmp/run/ total 16 drwxr-xr-x 2 root root 4096 Jul 19 04:00 a drwxr-xr-x 2 root root 4096 Jul 19 04:00 b # . /usr/lib/beakerlib/beakerlib.sh # rlJournalStart # rlFileBackup /tmp/run/a # rmdir /tmp/run/a # rlFileRestore # ls -l /tmp/run total 8 drwxr-xr-x 2 root root 4096 Jul 19 04:00 a # ls -l /run total 8 drwxr-xr-x 2 root root 4096 Jul 19 04:00 b
No I can see that the proposed patch is applied in beakerlib-1.5-1.el5 but the rlFileRestore is broken anyway
Following change fixed it for me, but I didn't do any detailed testing. --- /usr/share/beakerlib/infrastructure.sh 2012-05-15 11:45:38.000000000 -0400 +++ /usr/share/beakerlib/infrastructure.sh.new 2012-07-19 04:33:42.000000000 -0400 @@ -302,6 +302,8 @@ for file in "$@"; do # convert relative path to absolute, remove trailing slash file=$(echo "$file" | sed "s|^\([^/]\)|$PWD/\1|" | sed "s|/$||") + # follow symlinks + file=$( readlink -n -f $file ) path=$(dirname "$file") # bail out if the file does not exist
Previous fix breaks the behavior expected in #c0. We had a discussion with Petr and proper symlink handling seems to be a challenge (backup of symlinks in the directory tree and also backup of their targets). Anyway, this is a better quick fix for "symlink in parent" issue, preserving the symlink backup behavior. This should be enough to fix bad behavior on RHEL7 and should not break anything. :-) Could you please review it ASAP? --- /usr/share/beakerlib/infrastructure.sh 2012-05-15 11:45:38.000000000 -0400 +++ /usr/share/beakerlib/infrastructure.sh.new 2012-07-19 06:02:19.000000000 -0400 @@ -302,7 +302,10 @@ for file in "$@"; do # convert relative path to absolute, remove trailing slash file=$(echo "$file" | sed "s|^\([^/]\)|$PWD/\1|" | sed "s|/$||") + # follow symlinks path=$(dirname "$file") + path=$(readlink -n -f $path) + file=$path/$(basename $file) # bail out if the file does not exist if ! [ -e "$file" ]; then check that we do not rewrite symlink in the parent # . /usr/lib/beakerlib/beakerlib.sh # ls -l /tmp/run lrwxrwxrwx 1 root root 4 Jul 19 04:31 /tmp/run -> /run # ls -l /tmp/run/ total 16 drwxr-xr-x 2 root root 4096 Jul 19 04:31 a drwxr-xr-x 2 root root 4096 Jul 19 04:30 b # rlJournalStart # rlFileBackup /tmp/run/a # rmdir /tmp/run/a # rlFileRestore # ls -l /tmp/run lrwxrwxrwx 1 root root 4 Jul 19 04:31 /tmp/run -> /run # ls -l /tmp/run/ total 16 drwxr-xr-x 2 root root 4096 Jul 19 04:31 a drwxr-xr-x 2 root root 4096 Jul 19 04:30 b check that we are able to backup symlink itself # rlJournalStart # ls -l /tmp/run lrwxrwxrwx 1 root root 4 Jul 19 04:31 /tmp/run -> /run # ls -l /tmp/run/ total 16 drwxr-xr-x 2 root root 4096 Jul 19 04:31 a drwxr-xr-x 2 root root 4096 Jul 19 04:30 b # rlFileBackup /tmp/run # rm /tmp/run rm: remove symbolic link `/tmp/run'? y # rlFileRestore # ls -l /tmp/run/ total 16 drwxr-xr-x 2 root root 4096 Jul 19 04:31 a drwxr-xr-x 2 root root 4096 Jul 19 04:30 b # ls -l /tmp/run lrwxrwxrwx 1 root root 4 Jul 19 06:07 /tmp/run -> /run This is just to illustrate the no-follow symlink backup problem: # rlJournalStart # ls -l /tmp/test total 8 lrwxrwxrwx 1 root root 4 Jul 19 06:12 run -> /run -rw-r--r-- 1 root root 0 Jul 19 06:11 x # ls -l /tmp/test/ total 8 lrwxrwxrwx 1 root root 4 Jul 19 06:12 run -> /run -rw-r--r-- 1 root root 0 Jul 19 06:11 x # cat /tmp/test/run/a/y 1 # rlFileBackup /tmp/test # echo 2 > /tmp/test/run/a/y # rm -rf /tmp/test # rlFileRestore # ls -l /tmp/test total 8 lrwxrwxrwx 1 root root 4 Jul 19 06:15 run -> /run -rw-r--r-- 1 root root 0 Jul 19 06:11 x # ls -l /tmp/test/ total 8 lrwxrwxrwx 1 root root 4 Jul 19 06:15 run -> /run -rw-r--r-- 1 root root 0 Jul 19 06:11 x # cat /tmp/test/run/a/y 2 This is bad! I want the file to be restored with content 1!!!
Karel, can you please included a test for the fix as well? Thanks
Sure, I will prepare a git patch with both.
Created attachment 599125 [details] proposed fix for issue with symlink in parent dir, including test case Attached proposed patch. I hope this is my last comment in this bug today. :-)
The tests FAIL for me after applying: [ INFO ] Running test_rlFileBackup_SymlinkInParent [ PASS ] Preparing tmp directory [ PASS ] Preparing target directory [ PASS ] Preparing linked directory [ FAIL ] Backing up the link/file1 [ PASS ] Removing link/file1 [ FAIL ] Restoring the file1 [ PASS ] Testing that link points to target [ PASS ] Testing that link/file1 was restored [ PASS ] Testing that link/file2 is still present [ PASS ] Clean up
(In reply to comment #20) > The tests FAIL for me after applying: Please disregard. rlBackup* & friends apparently need root. Mea culpa.
Created attachment 599321 [details] proposed fix for issue with symlink in parent dir, including test case (updated) Apparently other test cases are ignoring rlFileBackup/Restore errors if the test is executed as non-root. I have updated the patch accordingly.
http://git.fedorahosted.org/git?p=beakerlib.git;a=commit;h=2de1becf573531d449701491d505abf6c9474dce
*** Bug 839716 has been marked as a duplicate of this bug. ***
Thanks for the fix, Karel.
beakerlib-1.6-1 is already in fedora
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle. Changing version to '19'. (As we did not run this process for some time, it could affect also pre-Fedora 19 development cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.) More information and reason for this action is here: https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19