Testing help for new Augeas Puppet provider which only executes if required

[Bryan Kearney]

I apologize for the cross posting, but I would like to hit up folks to
test and comment on a new version of the Augeas Puppet provider. You can
get it at [1] which is a few day old version of the Puppet code. Added
on top of this is a fix to [2] which states that the provider always
executes, even if it does not need to.

Augeas version 0.3.6 added some changes which allow Augeas to be run in
a noop mode, and report what "would" have happend to the file system.
So, the new logic for the provider is

1 Check if the provider needs to run
2 Apply the filter. If it matches, continue. If not, return no.
3 if Augeas version 0.3.6 is installed
3.1 Load up the contents of the file
3.2 Make the changes in noop mode
3.3 If the files would have changed, return yes. if not, return no.

4 Execute the provider
5 if Augeas version 0.3.6 is installed
5.1 actually save the files.
6 If Augeas version 0.3.6 is not installed
6.1 Load up the contents of the file
6.2 Make the changes
6.3 Write the new files.

The issue is, will any changes made to the file by other types between
steps 3.3 and 5.1 be lost, and will that cause issues? There may be
other issues I have not thought of yet (perhaps a dependent type
actually lays down a file).

Anyways.. comments on testing with the code would be greatly appreciated.

-- bk

[1] http://github.com/bkearney/puppet/tree/1826-0.24.x
[2] http://projects.reductivelabs.com/issues/1826

[David Lutterkort]

On Wed, 2009-02-18 at 13:07 -0500, Bryan Kearney wrote:
> The issue is, will any changes made to the file by other types between
> steps 3.3 and 5.1 be lost, and will that cause issues?

Yes, they will. If you want to avoid that, you'd have to load the files
again, make your changes and then save, i.e. the only differencce
between using 0.3.6 and earlier is whether you check for actual changes
or always assume there are some and tell Augeas to save (it'll not
modify unchanged files anyway, you just can't find that out pre-0.3.6)

David

[Bryan Kearney]

so.. the next question this is if we attempt the model of not executing
unless changes are made at the initial scan, it is possible that some
other change between steps 3.3 and 5.1 would have caused a change to
occur. So... even if I reload there could be issues.

I think I am left with 3 issues:

1) Always have the Augeas Plugin execute (current state)
2) Always have it short circuit, and reload the files to limit data loss
(but not eliminate it)
3) Add a "force" or a "dont short circuit" option which makes it behave
like (1).

Opinions?

-- bk

[David Lutterkort]

On Thu, 2009-02-19 at 16:26 -0500, Bryan Kearney wrote:
> so.. the next question this is if we attempt the model of not executing
> unless changes are made at the initial scan, it is possible that some
> other change between steps 3.3 and 5.1 would have caused a change to
> occur. So... even if I reload there could be issues.

Yes, though that's a general issue with puppet, and ultimately, there's
no good way to do check-and-change type operations on files, since they
can not be done atomically. I wouldn't sweat it; if you have a false
negative, the next puppet run will catch it ;)

David

[Ben]

I agree with David.

Ben

[Bryan Kearney]

One question.... If I have a file with 10 nodes, does augeas re-write
the whole file, or just scan for the nodes which changed?

-- bk

[David Lutterkort]

On Fri, 2009-02-20 at 07:53 -0500, Bryan Kearney wrote:
> One question.... If I have a file with 10 nodes, does augeas re-write
> the whole file, or just scan for the nodes which changed?

It will write a new file (into a tmp location) and then replace the old
file with the new one. Since rename(2) is the only way to modify files
atomically, it's generally a bad idea to modify files that could be in
the process of being read/written by others in place[1].

David

[1] Augeas will fall back to modifying in place under certain
circumstances, most importantly when the file is bindmounted, since you
can't use rename in that case.

[Bryan Kearney]

David Lutterkort wrote:
> On Fri, 2009-02-20 at 07:53 -0500, Bryan Kearney wrote:
>> One question.... If I have a file with 10 nodes, does augeas re-write
>> the whole file, or just scan for the nodes which changed?
>
> It will write a new file (into a tmp location) and then replace the old
> file with the new one. Since rename(2) is the only way to modify files
> atomically, it's generally a bad idea to modify files that could be in
> the process of being read/written by others in place[1].

I pushed a new version up, if folks would not mind testing it. I made 2
changes to the previous version:

1) I call Augeas twice. Once during the check to see if it should run,
and once when executing the changes. This is a bit in-efficient, but
removes the chance for data loss from the file changing between the 2 runs.

2) I also added a force parameter, which will disable this check. If
force is set to true, then augeas will be called only being constrained
by the onlyif parameter.

So.. the new logic to determine is the type should execute is:

<pseudo code>
needs_to_run = true

If only_if is set
needs_to_run = false if get or match not found
end

if force is not set to true
if Augeas version >= 0.3.6
needs_to_run = false if executing the commands
would not effect any files.
end
end

return needs_to_run
</pseudo code>

Again, if folks could take a look and see if it meets your needs before
I ask James to push, that would be great.

-- bk