Merge "Fix and refactor apt_fuel provided"

deployment/puppet/osnailyfacter/lib/puppet/provider/package/apt_fuel.rb

@@ -8,20 +8,43 @@ Puppet::Type.type(:package).provide :apt_fuel, :parent => :apt, :source => :apt
 
   defaultfor :operatingsystem => [:ubuntu]
 
-  def locked?(file)
-    """
-    Check whether ``apt-get`` or ``dpkg`` is currently active.
+  def default_lock_timeout
+    300
+  end
 
-    This works by checking whether the lock file ``/var/lib/dpkg/lock`` is
-    locked by an ``apt-get`` or ``dpkg`` process, which in turn is done by
-    momentarily trying to acquire the lock. This means that the current process
-    needs to have sufficient privileges.
-    """
-    f = open(file, 'w')
-    flockstruct = [Fcntl::F_RDLCK, 0, 0, 0, 0].pack("ssqqi")
-    f.fcntl Fcntl::F_GETLK, flockstruct
-    status =  flockstruct.unpack("ssqqi")[0]
-    f.close()
+  def lock_file
+    '/var/lib/dpkg/lock'
+  end
+
+  def lock_sleep
+    2
+  end
+
+  def retry_count
+    3
+  end
+
+  def retry_sleep
+    5
+  end
+
+  def timeout
+    @property_hash.fetch(:timeout, default_lock_timeout)
+  end
+
+  # Check whether ``apt-get`` or ``dpkg`` is currently active.
+  #
+  # This works by checking whether the lock file ``/var/lib/dpkg/lock`` is
+  # locked by an ``apt-get`` or ``dpkg`` process, which in turn is done by
+  # momentarily trying to acquire the lock. This means that the current process
+  # needs to have sufficient privileges.
+  # @return [true,false]
+  def locked?
+    status = open(lock_file, 'w') do |lock|
+      flock_struct = [Fcntl::F_RDLCK, 0, 0, 0, 0].pack('ssqqi')
+      lock.fcntl Fcntl::F_GETLK, flock_struct
+      flock_struct.unpack('ssqqi').first
+    end
     case status
       when Fcntl::F_UNLCK
         return false
@@ -32,41 +55,47 @@ Puppet::Type.type(:package).provide :apt_fuel, :parent => :apt, :source => :apt
     end
   end
 
+  # Wait for the lock file to be unlocked
+  # and the ``dpkg`` not being currently active.
   def wait_for_lock
-    default_timeout = 300
-    lock_file = '/var/lib/dpkg/lock'
-
-    Timeout::timeout(@property_hash.fetch(:timeout, default_timeout), Puppet::Error) do
-      while self.locked?(lock_file) do
-        debug("#{lock_file} is locked, retrying")
-        sleep 2
+    Timeout::timeout(timeout, Puppet::Error) do
+      while locked? do
+        debug "#{lock_file} is locked, retrying..."
+        sleep lock_sleep
       end
     end
   end
 
   def install
-    tries = 3
-    tries.times do |try|
+    (1..retry_count).each do |try|
       begin
-        self.wait_for_lock()
+        wait_for_lock
         super
-        info("Attempt #{try+1} was successful") if try > 0
+        info "Attempt #{try} of #{retry_count} was successful!" if try > 1
         break
-      rescue Puppet::ExecutionFailure => e
-        warn("Attempt #{try+1} of #{tries} failed: #{e.message}")
-        raise if try == tries-1
-        sleep 5
+      rescue Puppet::ExecutionFailure => exception
+        warning "Attempt #{try} of #{retry_count} have failed: #{exception.message}"
+        raise exception if try == retry_count
+        sleep retry_sleep
+        update
       end
     end
   end
 
+  def update
+    wait_for_lock
+    Timeout::timeout(timeout, Puppet::Error) do
+      aptget '-q', '-y', :update
+    end
+  end
+
   def uninstall
-    self.wait_for_lock()
+    wait_for_lock
     super
   end
 
   def purge
-    self.wait_for_lock()
+    wait_for_lock
     super
   end
 end

deployment/puppet/osnailyfacter/spec/unit/provider/package/apt_fuel__spec.rb

@@ -1,18 +0,0 @@
-require 'spec_helper'
-
-describe Puppet::Type.type(:package).provider(:apt_fuel) do
-
-  it 'should exist' do
-    expect(Puppet::Type.type(:package).provider(:apt_fuel).nil?).to eq false
-  end
-
-  it 'should use apt_fuel provider on Ubuntu' do
-   if Facter.fact(:operatingsystem) == "Ubuntu"
-      resource  = Puppet::Type.type(:package).new(
-        :ensure    => :present,
-        :name      => 'test'
-      )
-     expect(resource.provider).to be_kind_of(Puppet::Type.type(:package).provider(:apt_fuel))
-   end
-  end
-end

deployment/puppet/osnailyfacter/spec/unit/provider/package/apt_fuel_spec.rb

@@ -0,0 +1,70 @@
+require 'spec_helper'
+
+describe Puppet::Type.type(:package).provider(:apt_fuel) do
+  let(:resource) do
+    Puppet::Type.type(:package).new(
+        :ensure => :present,
+        :name => 'test',
+        :provider => :apt_fuel,
+    )
+  end
+
+  let(:provider) do
+    resource.provider
+  end
+
+  subject { provider }
+
+  before(:each) do
+    puppet_debug_override
+    subject.stubs(:lock_sleep).returns(0)
+    subject.stubs(:retry_sleep).returns(0)
+    subject.stubs(:default_lock_timeout).returns(1)
+  end
+
+  it 'should exist' do
+    is_expected.not_to be_nil
+  end
+
+  it 'should check for lock file to be free before installing a package' do
+    subject.expects(:locked?).returns(false)
+    subject.expects(:aptget).returns(true)
+    subject.install
+  end
+
+  it 'should wait unless lock file is free' do
+    subject.expects(:locked?).returns(true, false).twice
+    subject.expects(:aptget).returns(true)
+    subject.install
+  end
+
+  it 'should fail if lock timeout is exceeded' do
+    subject.stubs(:lock_sleep).returns(2)
+    subject.stubs(:locked?).returns(true, false)
+    subject.stubs(:aptget).returns(true)
+    expect do
+      subject.install
+    end.to raise_error Puppet::Error
+  end
+
+  it 'should retry the failed installation attempts' do
+    subject.stubs(:locked?).returns(false)
+    subject.expects(:aptget).
+        with('-q', '-y', '-o', 'DPkg::Options::=--force-confold', :install, 'test').
+        raises(Puppet::ExecutionFailure, 'installation failed').times(3)
+    subject.expects(:aptget).with('-q', '-y', :update).times(2)
+    expect do
+      subject.install
+    end.to raise_error Puppet::ExecutionFailure, 'installation failed'
+  end
+
+  it 'should be able to succeed after failing' do
+    subject.stubs(:locked?).returns(false)
+    subject.expects(:aptget).
+        with('-q', '-y', '-o', 'DPkg::Options::=--force-confold', :install, 'test').
+        raises(Puppet::ExecutionFailure, 'installation failed').then.returns(true).times(2)
+    subject.expects(:aptget).with('-q', '-y', :update).times(1)
+    subject.install
+  end
+
+end