[DRE-commits] [librarian-puppet] 58/153: Adding the ability to use rsync to converge cached and installed modules
Stig Sandbeck Mathisen
ssm at debian.org
Wed Jun 1 20:30:42 UTC 2016
This is an automated email from the git hooks/post-receive script.
ssm pushed a commit to branch master
in repository librarian-puppet.
commit 188acb78c6ee717197cf8a7acfc130bd51e3650a
Author: David Danzilio <david.danzilio at mac.com>
Date: Wed Sep 17 21:30:38 2014 -0400
Adding the ability to use rsync to converge cached and installed modules
Need to use the parent of dest for rsync
Reverting gemfile
Adding a test. Going to check with maintainers to see if this is a valid testing strategy.
Adding ctime to the mix to make sure that the file isn't just assigned the same inode
Adding tests for the rsync option using the install command
Only compare ctime if inodes match
Adding test case to make sure update works properly with git
---
README.md | 22 +++++++
features/install.feature | 72 +++++++++++++++++++++
features/step_definitions/convergence_steps.rb | 28 +++++++++
features/update.feature | 77 +++++++++++++++++++++++
lib/librarian/puppet/source/forge/repo.rb | 2 +-
lib/librarian/puppet/source/githubtarball/repo.rb | 2 +-
lib/librarian/puppet/source/local.rb | 2 +-
lib/librarian/puppet/util.rb | 24 +++++--
librarian-puppet.gemspec | 1 +
9 files changed, 221 insertions(+), 9 deletions(-)
diff --git a/README.md b/README.md
index b4df856..113009e 100644
--- a/README.md
+++ b/README.md
@@ -256,6 +256,28 @@ Configuration can be set by passing specific options to other commands.
the environment or global config will be used.
+## Rsync Option
+
+The default convergence strategy between the cache and the module directory is
+to execute an `rm -r` on the module directory and just `cp -r` from the cache.
+This causes the module to be removed from the module path every time librarian
+puppet updates, regardless of whether the content has changed. This can cause
+some problems in environments with lots of change. The problem arises when the
+module directory gets removed while Puppet is trying to read files inside it.
+The `puppet master` process will lose its CWD and the catalog will fail to
+compile. To avoid this, you can use `rsync` to implement a more conservative
+convergence strategy. This will use `rsync` with the `-avz` and `--delete`
+flags instead of a `rm -r` and `cp -r`. To use this feature, just set the
+`rsync` configuration setting to `true`.
+
+ $ librarian-puppet config rsync true --global
+
+Alternatively, using an environment variable:
+
+ LIBRARIAN_PUPPET_RSYNC='true'
+
+Note that the directories will still be purged if you run librarian-puppet with
+the --clean or --destructive flags.
## How to Contribute
diff --git a/features/install.feature b/features/install.feature
index 77fe34d..d42efda 100644
--- a/features/install.feature
+++ b/features/install.feature
@@ -40,3 +40,75 @@ Feature: cli/install
"""
Unable to parse .*/bad_modulefile/Modulefile, ignoring: Missing version
"""
+
+ Scenario: Install a module with the rsync configuration using the --clean flag
+ Given a file named "Puppetfile" with:
+ """
+ forge "http://forge.puppetlabs.com"
+
+ mod 'maestrodev/test'
+ """
+ And a file named ".librarian/puppet/config" with:
+ """
+ ---
+ LIBRARIAN_PUPPET_RSYNC: 'true'
+ """
+ When I run `librarian-puppet config`
+ Then the exit status should be 0
+ And the output should contain "rsync: true"
+ When I run `librarian-puppet install`
+ Then the exit status should be 0
+ And a directory named "modules/test" should exist
+ And the file "modules/test" should have an inode and ctime
+ When I run `librarian-puppet install --clean`
+ Then the exit status should be 0
+ And a directory named "modules/test" should exist
+ And the file "modules/test" should not have the same inode or ctime as before
+
+ Scenario: Install a module with the rsync configuration using the --destructive flag
+ Given a file named "Puppetfile" with:
+ """
+ forge "http://forge.puppetlabs.com"
+
+ mod 'maestrodev/test'
+ """
+ And a file named ".librarian/puppet/config" with:
+ """
+ ---
+ LIBRARIAN_PUPPET_RSYNC: 'true'
+ """
+ When I run `librarian-puppet config`
+ Then the exit status should be 0
+ And the output should contain "rsync: true"
+ When I run `librarian-puppet install`
+ Then the exit status should be 0
+ And a directory named "modules/test" should exist
+ And the file "modules/test" should have an inode and ctime
+ When I run `librarian-puppet install --destructive`
+ Then the exit status should be 0
+ And a directory named "modules/test" should exist
+ And the file "modules/test" should not have the same inode or ctime as before
+
+ Scenario: Install a module with the rsync configuration
+ Given a file named "Puppetfile" with:
+ """
+ forge "http://forge.puppetlabs.com"
+
+ mod 'maestrodev/test'
+ """
+ And a file named ".librarian/puppet/config" with:
+ """
+ ---
+ LIBRARIAN_PUPPET_RSYNC: 'true'
+ """
+ When I run `librarian-puppet config`
+ Then the exit status should be 0
+ And the output should contain "rsync: true"
+ When I run `librarian-puppet install`
+ Then the exit status should be 0
+ And a directory named "modules/test" should exist
+ And the file "modules/test" should have an inode and ctime
+ When I run `librarian-puppet install`
+ Then the exit status should be 0
+ And a directory named "modules/test" should exist
+ And the file "modules/test" should have the same inode and ctime as before
diff --git a/features/step_definitions/convergence_steps.rb b/features/step_definitions/convergence_steps.rb
new file mode 100644
index 0000000..b7cf031
--- /dev/null
+++ b/features/step_definitions/convergence_steps.rb
@@ -0,0 +1,28 @@
+Then /^the file "([^"]*)" should have an inode and ctime$/ do |file|
+ prep_for_fs_check do
+ stat = File.stat(File.expand_path(file))
+ @before_inode = { 'ino' => stat.ino, 'ctime' => stat.ctime }
+ expect(@before_inode['ino']).not_to eq nil
+ expect(@before_inode['ctime']).not_to eq nil
+ end
+end
+
+Then /^the file "([^"]*)" should have the same inode and ctime as before$/ do |file|
+ prep_for_fs_check do
+ stat = File.stat(File.expand_path(file))
+ expect(stat.ino).to eq @before_inode['ino']
+ expect(stat.ctime).to eq @before_inode['ctime']
+ end
+end
+
+Then /^the file "([^"]*)" should not have the same inode or ctime as before$/ do |file|
+ prep_for_fs_check do
+ stat = File.stat(File.expand_path(file))
+
+ begin
+ expect(stat.ino).not_to eq @before_inode['ino']
+ rescue RSpec::Expectations::ExpectationNotMetError
+ expect(stat.ctime).not_to eq @before_inode['ctime']
+ end
+ end
+end
diff --git a/features/update.feature b/features/update.feature
index 5dcbf85..d49cd05 100644
--- a/features/update.feature
+++ b/features/update.feature
@@ -204,3 +204,80 @@ Feature: cli/update
And the file "Puppetfile.lock" should match /maestrodev.test \(1\.0\.[1-9][0-9]\)/
And the file "modules/test/Modulefile" should contain "name 'maestrodev-test'"
And the file "modules/test/Modulefile" should match /version '1\.0\.[1-9][0-9]'/
+
+ Scenario: Updating a forge module with the rsync configuration
+ Given a file named "Puppetfile" with:
+ """
+ forge "http://forge.puppetlabs.com"
+
+ mod 'maestrodev/test'
+ """
+ And a file named "Puppetfile.lock" with:
+ """
+ FORGE
+ remote: http://forge.puppetlabs.com
+ specs:
+ maestrodev/test (1.0.2)
+
+ DEPENDENCIES
+ maestrodev/test (>= 0)
+ """
+ And a file named ".librarian/puppet/config" with:
+ """
+ ---
+ LIBRARIAN_PUPPET_RSYNC: 'true'
+ """
+ When I run `librarian-puppet config`
+ Then the exit status should be 0
+ And the output should contain "rsync: true"
+ When I run `librarian-puppet update --verbose`
+ Then the exit status should be 0
+ And a directory named "modules/test" should exist
+ And the file "modules/test" should have an inode and ctime
+ When I run `librarian-puppet update --verbose`
+ Then the exit status should be 0
+ And a directory named "modules/test" should exist
+ And the file "modules/test" should have the same inode and ctime as before
+
+ Scenario: Updating a git module with the rsync configuration
+ Given a file named "Puppetfile" with:
+ """
+ forge "http://forge.puppetlabs.com"
+
+ mod "stdlib",
+ :git => "https://github.com/puppetlabs/puppetlabs-stdlib.git", :ref => "3.1.x"
+ """
+ And a file named "Puppetfile.lock" with:
+ """
+ GIT
+ remote: https://github.com/puppetlabs/puppetlabs-stdlib.git
+ ref: 3.1.x
+ sha: 614b3fbf6c15893e89ed8654fb85596223b5b7c5
+ specs:
+ stdlib (3.1.1)
+
+ DEPENDENCIES
+ stdlib (>= 0)
+ """
+ And a file named ".librarian/puppet/config" with:
+ """
+ ---
+ LIBRARIAN_PUPPET_RSYNC: 'true'
+ """
+ When I run `librarian-puppet config`
+ Then the exit status should be 0
+ And the output should contain "rsync: true"
+ When I run `librarian-puppet install`
+ Then the exit status should be 0
+ And the file "modules/stdlib/.git/HEAD" should match /614b3fbf6c15893e89ed8654fb85596223b5b7c5/
+ And a directory named "modules/stdlib" should exist
+ When I run `librarian-puppet update`
+ Then the exit status should be 0
+ And a directory named "modules/stdlib" should exist
+ And the file "modules/stdlib" should have an inode and ctime
+ And the file "modules/stdlib/.git/HEAD" should match /a3c600d5f277f0c9d91c98ef67daf7efc9eed3c5/
+ When I run `librarian-puppet update`
+ Then the exit status should be 0
+ And a directory named "modules/stdlib" should exist
+ And the file "modules/stdlib" should have the same inode and ctime as before
+ And the file "modules/stdlib/.git/HEAD" should match /a3c600d5f277f0c9d91c98ef67daf7efc9eed3c5/
diff --git a/lib/librarian/puppet/source/forge/repo.rb b/lib/librarian/puppet/source/forge/repo.rb
index 06364c6..c16a878 100644
--- a/lib/librarian/puppet/source/forge/repo.rb
+++ b/lib/librarian/puppet/source/forge/repo.rb
@@ -55,7 +55,7 @@ module Librarian
cache_version_unpacked! version
- if install_path.exist?
+ if install_path.exist? && rsync? != true
install_path.rmtree
end
diff --git a/lib/librarian/puppet/source/githubtarball/repo.rb b/lib/librarian/puppet/source/githubtarball/repo.rb
index 1aa8b08..c53bf18 100644
--- a/lib/librarian/puppet/source/githubtarball/repo.rb
+++ b/lib/librarian/puppet/source/githubtarball/repo.rb
@@ -48,7 +48,7 @@ module Librarian
cache_version_unpacked! version
- if install_path.exist?
+ if install_path.exist? && rsync? != true
install_path.rmtree
end
diff --git a/lib/librarian/puppet/source/local.rb b/lib/librarian/puppet/source/local.rb
index 78c3bbd..52e411c 100644
--- a/lib/librarian/puppet/source/local.rb
+++ b/lib/librarian/puppet/source/local.rb
@@ -20,7 +20,7 @@ module Librarian
end
install_path = environment.install_path.join(organization_name(name))
- if install_path.exist?
+ if install_path.exist? && rsync? != true
debug { "Deleting #{relative_path_to(install_path)}" }
install_path.rmtree
end
diff --git a/lib/librarian/puppet/util.rb b/lib/librarian/puppet/util.rb
index 9c1f566..7031d58 100644
--- a/lib/librarian/puppet/util.rb
+++ b/lib/librarian/puppet/util.rb
@@ -1,3 +1,5 @@
+require 'rsync'
+
module Librarian
module Puppet
@@ -13,16 +15,26 @@ module Librarian
environment.logger.warn(*args, &block)
end
+ def rsync?
+ environment.config_db.local['rsync'] == 'true'
+ end
+
# workaround Issue #173 FileUtils.cp_r will fail if there is a symlink that points to a missing file
# or when the symlink is copied before the target file when preserve is true
# see also https://tickets.opscode.com/browse/CHEF-833
+ #
+ # If the rsync configuration parameter is set, use rsync instead of FileUtils
def cp_r(src, dest)
- begin
- FileUtils.cp_r(src, dest, :preserve => true)
- rescue Errno::ENOENT
- debug { "Failed to copy from #{src} to #{dest} preserving file types, trying again without preserving them" }
- FileUtils.rm_rf(dest)
- FileUtils.cp_r(src, dest)
+ if rsync?
+ Rsync.run(File.join(src, "/"), dest, ['-avz', '--delete'])
+ else
+ begin
+ FileUtils.cp_r(src, dest, :preserve => true)
+ rescue Errno::ENOENT
+ debug { "Failed to copy from #{src} to #{dest} preserving file types, trying again without preserving them" }
+ FileUtils.rm_rf(dest)
+ FileUtils.cp_r(src, dest)
+ end
end
end
diff --git a/librarian-puppet.gemspec b/librarian-puppet.gemspec
index 6299c62..ed7240e 100644
--- a/librarian-puppet.gemspec
+++ b/librarian-puppet.gemspec
@@ -27,6 +27,7 @@ Gem::Specification.new do |s|
s.executables = ['librarian-puppet']
s.add_dependency "librarian", ">=0.1.2"
+ s.add_dependency "rsync"
s.add_dependency "puppet_forge"
s.add_development_dependency "rake"
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-ruby-extras/librarian-puppet.git
More information about the Pkg-ruby-extras-commits
mailing list