Bug 623774

Summary: rhncfg-client: Creation of symbolic links fail on non-root partition
Product: [Community] Spacewalk Reporter: Ryan Falkenberg <ryan.falkenberg>
Component: ClientsAssignee: Milan Zázrivec <mzazrivec>
Status: CLOSED WORKSFORME QA Contact: Red Hat Satellite QA List <satqe-list>
Severity: medium Docs Contact:
Priority: low    
Version: 1.0CC: jpazdziora, mmraka
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-08-24 12:14:28 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 723481    

Description Ryan Falkenberg 2010-08-12 18:20:19 UTC
Description of problem:
Deploying a symbolic link to a spacewalk-managed system fails if the symlink location is not on the root (/) filesystem

Version-Release number of selected component (if applicable):
rhncfg-5.9.19-1.el5.noarch


How reproducible:
Always

Steps to Reproduce:
1. Add a locally managed symbolic link to a spacewalk managed system, set the symbolic link location to a filesystem other than / (I am using /opt), the target of the symlink does not matter
2. deploy the symlink to the system
  
Actual results:
Deployment fails with:
Client execution returned "Failed deployment, rolled back: [Errno 18] Invalid cross-device link" (code 49)

Expected results:
Symlink to be properly deployed


Additional info:
- Deploying a symbolic link to a directory on the root (/) file system works

- I've traced the problem back to an os.rename in /usr/share/rhn/config_common/transactions.py and attached a prof of concept fix.  Calling `mv` is a hack and probably fragile but hopefully it will help someone with better python skills than me write a better solution

--- /usr/share/rhn/config_common/transactions.py.orig   2010-08-11 17:04:33.000000000 -0600
+++ /usr/share/rhn/config_common/transactions.py        2010-08-12 10:51:13.000000000 -0600
@@ -350,7 +351,21 @@
                     self.deployment_cb(path)

                 log_debug(6, "deploying %s ..." % path)
-                os.rename(self.newtemp_by_path[path], path)
+                # os.renames will fail if the path and the new_path are on different partitions
+                # need to make sure to handle it if we catch a 'OSError: [Errno 18] Invalid cross-device link'
+                try:
+                    log_debug(9, "trying to use os.rename to move the temp file into place")
+                    os.rename(self.newtemp_by_path[path], path)
+                except OSError, e:
+                    if e.errno == 18:
+                        #log_debug(9, "os.rename failed, using shutil functions")
+                        #shutil.copy(self.newtemp_by_path[path], path)
+                        # This is kind of hacky but i don't know a better way that properly handles dangling symlinks
+                        log_debug(9, "os.rename failed, using the system `mv` command to copy %s to %s" % (self.newtemp_by_path[path], path))
+                        os.system("mv %s %s" % (self.newtemp_by_path[path], path))
+                    else:
+                        raise
                 # race
                 del self.newtemp_by_path[path]
                 log_debug(9, "new version of %s deployed" % path)



- In my testing this fix works regardless of where the symbolic link points to (directory, file, or dangling symlink).
- With this fix symlink deployments will fail if the destination directory doesn't exist (i.e. deploying /nonresistant/directory/symlink will fail).  And deploying the same symlink twice will fail if the symlink target is a directory

Comment 1 Ryan Falkenberg 2010-08-12 18:23:37 UTC
/usr/share/rhn/config_common/transactions.py needs to be patched on the client systems, not the spacewalk server

Comment 2 Jan Pazdziora 2010-09-01 12:16:37 UTC
(In reply to comment #0)
> +                        os.system("mv %s %s" % (self.newtemp_by_path[path],
> path))

I don't like forking the mv, not mentioning the fact that the comemnt will fail if the path is strange (starts with a dash or has a space in it or something).

Why can't we create the temporary entries in the same directory as the final location, with some dot at start and some random suffix?

Comment 3 Jan Pazdziora 2010-11-19 16:03:26 UTC
Mass-moving to space13.

Comment 4 Miroslav Suchý 2011-04-11 07:32:09 UTC
We did not have time for this one during Spacewalk 1.4 time frame. Mass moving to Spacewalk 1.5.

Comment 5 Miroslav Suchý 2011-04-11 07:36:37 UTC
We did not have time for this one during Spacewalk 1.4 time frame. Mass moving to Spacewalk 1.5.

Comment 6 Jan Pazdziora 2011-07-20 11:49:56 UTC
Aligning under space16.

Comment 7 Michael Mráka 2011-08-24 12:14:28 UTC
I was not able to reproduce this issue with the latest rhncfg-client.

#  rhncfg-client get
Deploying //mnt/dir
Deploying /mnt/dir/link
Deploying /mnt/dir/file
Deploying /mnt/file
# ll /mnt/dir/
-rw-r--r-- 1 root root 6 Aug 24 13:53 file
lrwxrwxrwx 1 root root 9 Aug 24 13:53 link -> /bin/bash
# df /bin /mnt
Filesystem           1K-blocks      Used Available Use% Mounted on
/dev/xvda1            40629568   1870084  36662856   5% /
/root/filesys.img        39663      4610     33005  13% /mnt
# rpm -q rhncfg-client
rhncfg-client-5.10.13-1.el6.noarch

So I'm closing it as WORKSFORME.

If you disagree please reopen and specify exact version of rhncfg-client.