Since part #1 of my three-part plan is proving troublesome I moved
ahead on #2.
Although part #3 of my original plan borrows some code from #1 it does
not require #1. If this patch is accepted then I will move ahead on #3.
This patch changes copyfile in two ways:
1. Copyfile now opens a tempfile on the minion and appends data to this
tempfile, then moves the tempfile over the target file after all of the data
has been appended.
The advantage of this is that updating the target file is atomic, or
as close to it as possible in python. This minimizes the chance of some
program accessing an empty or half-complete file. If the file in question
were a server config file... surely you can imagine how this could be a
problem.
The tempfiles are created in the same directory as the target file
(for security) and have the same name as the target file but with the
extension ".functmp". Permissions are set on this file during the
"open"
stage, while it is still zero-size.
2. Copyfile has a new option called "retainperms" ("--retainperms" at
the
command line). It is "False" by default.
Currently func tries to give the new file on the minion the same
attributes (permissions and uid/gid) as the file on the overlord. The SELinux
context of the new file is not set (and does not change). This overrides this
behavior to make the permissions, flags, uid, gid and SELinux contexts of the
new file identical to the existing file (if any) on the minion.
This is intended to deal with the situation in which the file on the
overlord has different permissions than the files on the minions and the user
does not want to overwrite them. Perhaps the overlord computer does not have
the same users and groups as the minions? I figured this might be useful for
someone.
On a related note, does anyone think it would be worthwhile to have
copyfile copy the SELinux contexts of the source file to the target? How
about making command-line options to allow the user to specify the desired
mode, uid, gid, contexts, etc.?
--
Marcus
On Saturday, August 25, 2012 04:55:09 PM Marcus Lauer wrote:
Attached are my original patch and a git-patch which contains
the
same code. These implement changes I called #1 in my previous e-mail.
#2 doesn't seem too hard. CopyFile.open will have to open a
tempfile rather than the target file. It already returns the filepath on
success. Append already uses the return value of CopyFile.open as the file
to append to. After appending has finished there needs to be a
CopyFile.move function called by the overlord which takes both paths and
moves the tempfile over the source file. Making sure the
ownership/permissions/SELinux attributes of the target file do not change
could be is important so had better be careful about that.
Once this is in place #3 is almost trivial. The overlord needs to
calculate the MD5 of the source file. The code for doing this is also
needed by #1 and is included in my patches. That MD5 should be passed to
the CopyFile.move function. It should calculate the MD5 of the tempfile
and if they don't match just delete the tempfile and return an error
instead of moving it. I figure that this MD5 check should be optional so I
will also create a flag to enable it.
Can anyone see any issues with this general design? If not I'll get
working on it when I have the time.
--
Marcus
On Friday, August 24, 2012 07:32:14 PM you wrote:
> Copyfile could most certainly use some TLC, so let me know if I can
> help,
> that would certainly be of help if you'd like to work on that. The list
> has indeed changed to
lists.fedorahosted.com, so definitely use that
> one.
>
> Would you mind shooting me your original patch as well? Thanks very
> much!>
> On Fri, Aug 24, 2012 at 7:22 PM, Marcus Lauer
<marcuslauer(a)nyc.rr.com>wrote:
> > It seems to me that there are at least three
> > improvements which
> >
> > should be made to copyfile:
> >
> > 1. If the file on the overlord and minion are the same, that file
> > should not
> > be copied. This saves bandwidth/processing time.
> >
> > 2. Overwriting the file on the minion should be atomic, e.g. the
> > file on the
> > overlord should be copied to a tempfile on the minion then moved to
> > overwrite
> > the target file as a final step. Right now if funcd dies or
> > something
> > goes wrong mid-copy you end up with half a file on the minion.
> >
> > 3. There should be an option (off by default) such that if the file
> > which
> > has
> > been copied over to the minion is not identical to the file on the
> > overlord it
> > is re-copied, or at least does not overwrite the file on the minion.
> > This won't work for some files (busy logfiles) but for most files it
> > seems like an
> > extra bit of reliability which most people would want in most
> > situations.
> >
> > I submitted a patch (though not a git-patch... my
> > bad!)
> > back in
> >
> > January 2011 which implements #1, but imperfectly. The overlord
> > still
> > has to
> > send a (small) packet to the minion for each 60KB chunk of target
> > file.
> >
> > Would anyone be bothered terribly if I worked on #2
> > and
> > #3? While
> >
> > I am
> > at it I will try to find a way to fix up my patch for #1 as well.
> > There is some overlap between these patches. Both will involve
> > calculating an MD5 hash
> > of the file on the overlord, for example.
> >
> > P.S. Is the new mailing list on
fedorahosted.org
> > live
> > yet? I am
> >
> > sending this message to both just in case.
> >
> > --
> > Marcus
> > _______________________________________________
> > func mailing list
> > func(a)lists.fedorahosted.org
> >
https://lists.fedorahosted.org/mailman/listinfo/func
>
> --
> Steve Salevan
> steve(a)tumblr.com