Skip to content

Commit

Permalink
Remove the retries gem
Browse files Browse the repository at this point in the history
Retries is only used by the CLI providers. Puppet itself also provides
retry functionality. This avoids the need to ensure a gem is installed
in the right environment, which may not even work if it's a disconnected
environment.
  • Loading branch information
ekohl committed Apr 15, 2019
1 parent 5d292f1 commit 31e388d
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 174 deletions.
1 change: 0 additions & 1 deletion .sync.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ Gemfile:
optional:
':test':
- gem: 'rspec-its'
- gem: 'retries'
spec/spec_helper.rb:
spec_overrides: "require 'rspec/its'"
mock_with: ':rspec'
1 change: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ group :test do
gem 'parallel_tests', '2.24.0', :require => false if RUBY_VERSION < '2.2.0'
gem 'parallel_tests', :require => false if RUBY_VERSION >= '2.2.0'
gem 'rspec-its', :require => false
gem 'retries', :require => false
end

group :development do
Expand Down
9 changes: 0 additions & 9 deletions NATIVE_TYPES_AND_PROVIDERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,6 @@ class { '::jenkins':
include ::jenkins::cli_helper
```

The ruby gem `retries` is presently required by all providers.

### `puppetserver`

There is a known issue with `puppetserver` being unable to load code from
Expand All @@ -144,13 +142,6 @@ jruby-puppet: {

See [SERVER-973](https://tickets.puppetlabs.com/browse/SERVER-973)

Additionally, the `retries` gem is required. This may be installed on the master by running:

```
/opt/puppetlabs/bin/puppetserver gem install retries
```


Types
--

Expand Down
3 changes: 0 additions & 3 deletions lib/puppet/feature/retries.rb

This file was deleted.

14 changes: 2 additions & 12 deletions lib/puppet/x/jenkins/provider/cli.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'puppet/provider'
require 'puppet/util/retry_action'
require 'facter'

require 'json'
Expand Down Expand Up @@ -27,7 +28,6 @@ def self.inherited(subclass)
initvars

commands java: 'java'
confine feature: :retries

# subclasses should inherit this value once it has been determined that
# jenkins requires authorization, it shortens the run time be elemating the
Expand Down Expand Up @@ -172,7 +172,6 @@ def self.execute_with_retry(command, options = {}, cli_pre_cmd = [])
url = config[:url]
ssh_private_key = config[:ssh_private_key]
cli_tries = config[:cli_tries]
cli_try_sleep = config[:cli_try_sleep]
cli_username = config[:cli_username]
cli_password = config[:cli_password]
cli_password_file = config[:cli_password_file]
Expand Down Expand Up @@ -220,16 +219,7 @@ def self.execute_with_retry(command, options = {}, cli_pre_cmd = [])
# retry on "unknown" execution errors but don't catch AuthErrors. If an
# AuthError has bubbled up to this level it means either an ssh_private_key
# is required and we don't have one or that one we have was rejected.
handler = proc do |exception, attempt_number, total_delay|
Puppet.debug("#{sname} caught #{exception.class.to_s.match(%r{::([^:]+)$})[1]}; retry attempt #{attempt_number}; #{total_delay.round(3)} seconds have passed")
end
with_retries(
max_tries: cli_tries,
base_sleep_seconds: 1,
max_sleep_seconds: cli_try_sleep,
rescue: [UnknownError, NetError],
handler: handler
) do
Puppet::Util::RetryAction.retry_action :retries => cli_tries - 1, :retry_exceptions => [UnknownError, NetError] do
result = execute_with_auth(cli_cmd, auth_cmd, options)
Puppet.debug("#{sname} command stdout:\n#{result}") unless result == ''
return result
Expand Down
19 changes: 0 additions & 19 deletions manifests/cli/config.pp
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,6 @@
Optional[String] $ssh_private_key_content = undef,
) {

if str2bool($::is_pe) {
$gem_provider = 'pe_gem'
} elsif $::puppetversion
and (versioncmp($::puppetversion, '4.0.0') >= 0)
and $::rubysitedir
and ('/opt/puppetlabs/puppet/lib/ruby' in $::rubysitedir) {
# AIO puppet
$gem_provider = 'puppet_gem'
} else {
$gem_provider = 'gem'
}

# required by PuppetX::Jenkins::Provider::Clihelper base
if ! defined(Package['retries']) {
package { 'retries':
provider => $gem_provider,
}
}

if $ssh_private_key and $ssh_private_key_content {
file { $ssh_private_key:
ensure => 'file',
Expand Down
38 changes: 0 additions & 38 deletions spec/classes/cli/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,44 +146,6 @@
end # when ssh_private_key is also set
end # ssh_private_key_content
end # parameters

describe 'package gem provider' do
context 'is_pe fact' do
context 'true' do
let :facts do
super().merge(is_pe: true)
end

it { is_expected.to contain_package('retries').with(provider: 'pe_gem') }
end

context 'false' do
let :facts do
super().merge(is_pe: false)
end

it { is_expected.to contain_package('retries').with(provider: 'gem') }
end
end # 'is_pe fact' do

context 'puppetversion facts' do
context '=> 3.8.4' do
let :facts do
super().merge(puppetversion: '3.8.4')
end

it { is_expected.to contain_package('retries').with(provider: 'gem') }
end

context '=> 4.0.0' do
let :facts do
super().merge(puppetversion: '4.0.0')
end

it { is_expected.to contain_package('retries').with(provider: 'gem') }
end
end # 'puppetversion facts' do
end # 'package gem provider' do
end
end
end
111 changes: 29 additions & 82 deletions spec/unit/puppet/x/jenkins/provider/cli_spec.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
require 'spec_helper'
require 'unit/puppet/x/spec_jenkins_providers'

# we need to make sure retries is always loaded or random test ordering can
# cause failures when a side effect hasn't yet caused the lib to be loaded
require 'retries'
require 'puppet/x/jenkins/provider/cli'

describe Puppet::X::Jenkins::Provider::Cli do
Expand Down Expand Up @@ -307,13 +304,6 @@
end # ::clihelper

describe '::cli' do
before do
# disable with_retries sleeping to [vastly] speed up testing
#
# we are relying the side effects of ::suitable? from a previous example
Retries.sleep_enabled = false
end

shared_examples 'uses default values' do
it 'uses default values' do
expect(described_class.superclass).to receive(:execute).with(
Expand Down Expand Up @@ -391,7 +381,6 @@
url: 'http://localhost:111',
ssh_private_key: 'cat.id_rsa',
cli_tries: 222,
cli_try_sleep: 333
)

catalog.add_resource jenkins
Expand Down Expand Up @@ -478,15 +467,15 @@

context 'network failure' do
context 'without ssh_private_key' do
CLI_NET_ERRORS.each do |error|
CLI_AUTH_ERRORS.each do |error|
it 'does not retry cli on AuthError exception' do
expect(described_class.superclass).to receive(:execute).with(
allow(described_class.superclass).to receive(:execute).with(
'java -jar /usr/lib/jenkins/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo',
failonfail: true, combine: true
).exactly(30).times.and_raise(NetError, error)
).and_raise(AuthError, error)

expect { described_class.cli('foo') }.
to raise_error(NetError)
expect { described_class.cli('foo') }.to raise_error(AuthError)
expect(described_class.superclass).to have_received(:execute).once
end
end
end
Expand All @@ -503,13 +492,16 @@
)
catalog.add_resource jenkins

expect(described_class.superclass).to receive(:execute).with(
allow(Puppet::Util::RetryAction).to receive(:sleep)
allow(described_class.superclass).to receive(:execute).with(
'java -jar /usr/lib/jenkins/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo',
failonfail: true, combine: true
).exactly(30).times.and_raise(UnknownError, 'foo')
).and_raise(UnknownError, 'foo')

expect { described_class.cli('foo', catalog: catalog) }.
to raise_error(UnknownError, 'foo')
to raise_error(Puppet::Util::RetryAction::RetryException::RetriesExceeded)
expect(Puppet::Util::RetryAction).to have_received(:sleep).exactly(29).times
expect(described_class.superclass).to have_received(:execute).exactly(30).times
end

it 'from catalog value' do
Expand All @@ -519,13 +511,16 @@
)
catalog.add_resource jenkins

expect(described_class.superclass).to receive(:execute).with(
allow(Puppet::Util::RetryAction).to receive(:sleep)
allow(described_class.superclass).to receive(:execute).with(
'java -jar /usr/lib/jenkins/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo',
failonfail: true, combine: true
).exactly(2).times.and_raise(UnknownError, 'foo')
).and_raise(UnknownError, 'foo')

expect { described_class.cli('foo', catalog: catalog) }.
to raise_error(UnknownError, 'foo')
to raise_error(Puppet::Util::RetryAction::RetryException::RetriesExceeded)
expect(Puppet::Util::RetryAction).to have_received(:sleep).exactly(1).times
expect(described_class.superclass).to have_received(:execute).exactly(2).times
end

it 'from fact' do
Expand All @@ -536,13 +531,16 @@
)
catalog.add_resource jenkins

expect(described_class.superclass).to receive(:execute).with(
allow(Puppet::Util::RetryAction).to receive(:sleep)
allow(described_class.superclass).to receive(:execute).with(
'java -jar /usr/lib/jenkins/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo',
failonfail: true, combine: true
).exactly(3).times.and_raise(UnknownError, 'foo')
).and_raise(UnknownError, 'foo')

expect { described_class.cli('foo', catalog: catalog) }.
to raise_error(UnknownError, 'foo')
to raise_error(Puppet::Util::RetryAction::RetryException::RetriesExceeded)
expect(Puppet::Util::RetryAction).to have_received(:sleep).exactly(2).times
expect(described_class.superclass).to have_received(:execute).exactly(3).times
end

it 'from catalog overriding fact' do
Expand All @@ -554,69 +552,18 @@
)
catalog.add_resource jenkins

expect(described_class.superclass).to receive(:execute).with(
allow(Puppet::Util::RetryAction).to receive(:sleep)
allow(described_class.superclass).to receive(:execute).with(
'java -jar /usr/lib/jenkins/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo',
failonfail: true, combine: true
).exactly(2).times.and_raise(UnknownError, 'foo')
).and_raise(UnknownError, 'foo')

expect { described_class.cli('foo', catalog: catalog) }.
to raise_error(UnknownError, 'foo')
to raise_error(Puppet::Util::RetryAction::RetryException::RetriesExceeded)
expect(Puppet::Util::RetryAction).to have_received(:sleep).exactly(1).times
expect(described_class.superclass).to have_received(:execute).exactly(2).times
end
end # n times

context 'waiting up to n seconds' do
# this isn't behavioral testing because we don't want to either wait
# for the wallclock delay timeout or attempt to accurate time examples
it 'by default' do
jenkins = Puppet::Type.type(:component).new(
name: 'jenkins::cli::config'
)
catalog.add_resource jenkins

expect(described_class).to receive(:with_retries).with(hash_including(max_sleep_seconds: 2))

described_class.cli('foo', catalog: catalog)
end

it 'from catalog value' do
jenkins = Puppet::Type.type(:component).new(
name: 'jenkins::cli::config',
cli_try_sleep: 3
)
catalog.add_resource jenkins

expect(described_class).to receive(:with_retries).with(hash_including(max_sleep_seconds: 3))

described_class.cli('foo', catalog: catalog)
end

it 'from fact' do
Facter.add(:jenkins_cli_try_sleep) { setcode { 4 } }

jenkins = Puppet::Type.type(:component).new(
name: 'jenkins::cli::config'
)
catalog.add_resource jenkins

expect(described_class).to receive(:with_retries).with(hash_including(max_sleep_seconds: 4))

described_class.cli('foo', catalog: catalog)
end

it 'from catalog overriding fact' do
Facter.add(:jenkins_cli_try_sleep) { setcode { 4 } }

jenkins = Puppet::Type.type(:component).new(
name: 'jenkins::cli::config',
cli_try_sleep: 3
)
catalog.add_resource jenkins

expect(described_class).to receive(:with_retries).with(hash_including(max_sleep_seconds: 3))

described_class.cli('foo', catalog: catalog)
end
end
end # should retry cli on UnknownError

context 'options with :stdinjson' do
Expand Down
9 changes: 0 additions & 9 deletions spec/unit/puppet/x/spec_jenkins_providers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,6 @@
described_class.confine_collection.instance_variable_get(:@confines)
end

context 'feature :retries' do
it do
expect(confines).to include(
be_kind_of(Puppet::Confine::Feature).
and(have_attributes(values: [:retries]))
)
end
end

context 'commands :java' do
it do
expect(confines).to include(
Expand Down

0 comments on commit 31e388d

Please sign in to comment.