Bug #1564

Saving File#checksum to state.yaml broken

Added by David Schmitt over 3 years ago. Updated over 3 years ago.

Status:Closed Start date:09/07/2008
Priority:Normal Due date:
Assignee:Paul Nasrat % Done:

0%
Category:file
Target version:0.24.6
Affected Puppet version:0.24.5 Branch:
Keywords:
Votes: 0

Description

When recursively copying a directory with checksum=>mtime, puppetd doesn’t notice that it already has the correct local version and triggers updates on every run.

Puppet manifest:

file {
  "/var/lib/puppet/modules/${name}":
    source => [ "puppet:///${name}/modules_dir", "puppet:///common/empty"],
    checksum => mtime,
    # ignore the placeholder
    ignore => '\.ignore',
    recurse => true, purge => true, force => true,
    mode => $mode, owner => $owner, group => $group;
}

state.yaml:

---
File[/var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list]:
  :checked: 2008-09-07 14:10:04.590123 +00:00
  :checksums:
    :mtime: "{md5}13f98c2ba6787ca5e7dbf74e99ff677c"
  :synced: 2008-09-07 14:10:04.806034 +00:00

puppetd —test —debug

debug: Calling fileserver.list
debug: Calling fileserver.describe
debug: Calling fileserver.retrieve
debug: //Modules_dir[virtual]/File[/var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list]/source: Executing 'diff /var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list /tmp/puppet-diffing.10425.0'
debug: //Modules_dir[virtual]/File[/var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list]: Changing checksum,source
debug: //Modules_dir[virtual]/File[/var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list]: 2 change(s)
debug: //Modules_dir[virtual]/File[/var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list]/checksum: Replacing /var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list checksum {md5}13f98c2ba6787ca5e7dbf74e99ff677c with {mtime}Sun Sep 07 14:04:45 +0000 2008
notice: //Modules_dir[virtual]/File[/var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list]/checksum: checksum changed '{md5}13f98c2ba6787ca5e7dbf74e99ff677c' to '{mtime}Sun Sep 07 14:04:45 +0000 2008'
debug: Calling puppetbucket.addfile
info: //Modules_dir[virtual]/File[/var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list]: Filebucketed to server with sum 13f98c2ba6787ca5e7dbf74e99ff677c
debug: //Modules_dir[virtual]/File[/var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list]/checksum: Replacing /var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list checksum {mtime}Sun Sep 07 14:04:45 +0000 2008 with {md5}13f98c2ba6787ca5e7dbf74e99ff677c
notice: //Modules_dir[virtual]/File[/var/lib/puppet/modules/virtual/build_vserver/skel_etch/apt/sources.list]/source: replacing from source puppet:///virtual/modules_dir/build_vserver/skel_etch/apt/sources.list with contents {md5}13f98c2ba6787ca5e7dbf74e99ff677c

Related issues

related to Puppet - Bug #264: file checksum option always uses md5-sums Closed

History

#1
Updated by James Turnbull over 3 years ago

  • Category set to file
  • Status changed from Unreviewed to Accepted
  • Target version set to 0.24.6

#2
Updated by Paul Nasrat over 3 years ago

  • Assignee set to Paul Nasrat

Looking at it in ruby-debug I see when we call File::Source.insync? we get:

(rdb:1) pp @stats {:checksum=>“{md5}d3b07384d113edec49eaa6238ad5ff00”, :type=>“file”, :mode=>420, :owner=>0, :group=>0} (rdb:1) pp parentchecksum “{mtime}Thu Sep 18 17:40:30 +0100 2008”

comparing different checksums isn’t going to work here, I guess source description needs to be aware of sum type.

pp self.describe(self.source) {:checksum=>“{md5}d3b07384d113edec49eaa6238ad5ff00”, :type=>“file”, :mode=>420, :owner=>0, :group=>0}

#3
Updated by Paul Nasrat over 3 years ago

  • Status changed from Accepted to Needs Decision

This is going to require a change to the xmlrpc protocol, whilst Puppet::FileServing::Metadata in theory supports multiple checksum_type (it doesn’t work correctly at the moment but I’ve fixed that in my branch for this ticket).

When we are describe the source file in type/file/source.rb we call the server:

  desc = server.describe(path, @resource[:links])

Which is served by the interface in fileserver.rb:

  iface.add_method("string describe(string, string)")

This in turn calls FileServing::Metadata to get the information. However we need to communicate the desired checksum type, and ensure that is used throughout (including when we update the checksum in our final write()).

#4
Updated by Luke Kanies over 3 years ago

Note that this is related to #264, which points out that we always use md5 rather than using the specified checksum type.

Given that I’m (nearly) positive this is working in the master branch, I think the “right” approach for now is to just throw a warning and revert to md5 if someone tries to use something other than md5 sums when doing remote copies.

It doesn’t seem worth an API change, given that it’d be pretty short-lived and would introduce backward compatibility issues.

#5
Updated by Paul Nasrat over 3 years ago

  • Status changed from Needs Decision to Code Insufficient

#6
Updated by Paul Nasrat over 3 years ago

  • Status changed from Code Insufficient to In Topic Branch Pending Review

As discussed on list patch to fix checksum for merge into 0.24.x and HEAD, plus warning and resetting of checksum to md5.

Patches to list and at git://github.com/pnasrat/puppet.git tickets/0.24.x/1564

#7
Updated by James Turnbull over 3 years ago

  • Status changed from In Topic Branch Pending Review to Closed

Pushed in commit:e6698c2b8624fe2c2bbeef594318e3e8d932d345 in branch 0.24.x

Also available in: Atom PDF