diff --git a/.sync.yml b/.sync.yml index c645ca804..35775e974 100644 --- a/.sync.yml +++ b/.sync.yml @@ -1,7 +1,3 @@ --- -Gemfile: - optional: - ':test': - - gem: 'retries' .rubocop.yml: unmanaged: true diff --git a/Gemfile b/Gemfile index 3174d3c09..a4a3b2043 100644 --- a/Gemfile +++ b/Gemfile @@ -8,7 +8,6 @@ group :test do gem 'coveralls', :require => false gem 'simplecov-console', :require => false gem 'puppet_metadata', '~> 3.5', :require => false - gem 'retries', :require => false end group :development do diff --git a/NATIVE_TYPES_AND_PROVIDERS.md b/NATIVE_TYPES_AND_PROVIDERS.md index d9dd05deb..083546f4c 100644 --- a/NATIVE_TYPES_AND_PROVIDERS.md +++ b/NATIVE_TYPES_AND_PROVIDERS.md @@ -121,8 +121,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 @@ -141,13 +139,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 -- diff --git a/lib/puppet/feature/retries.rb b/lib/puppet/feature/retries.rb deleted file mode 100644 index 426565f6a..000000000 --- a/lib/puppet/feature/retries.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -require 'puppet/util/feature' - -Puppet.features.add(:retries, libs: 'retries') diff --git a/lib/puppet/x/jenkins/config.rb b/lib/puppet/x/jenkins/config.rb index 1a0556d89..e32c8092a 100644 --- a/lib/puppet/x/jenkins/config.rb +++ b/lib/puppet/x/jenkins/config.rb @@ -18,7 +18,6 @@ class UnknownConfig < ArgumentError; end ssh_private_key: nil, puppet_helper: '/usr/share/java/puppet_helper.groovy', cli_tries: 30, - cli_try_sleep: 2, cli_username: nil, cli_password: nil, cli_password_file: '/tmp/jenkins_credentials_for_puppet', diff --git a/lib/puppet/x/jenkins/provider/cli.rb b/lib/puppet/x/jenkins/provider/cli.rb index 6057cdc48..c979940ea 100644 --- a/lib/puppet/x/jenkins/provider/cli.rb +++ b/lib/puppet/x/jenkins/provider/cli.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'puppet/provider' +require 'puppet/util/retry_action' require 'facter' require 'json' @@ -31,7 +32,6 @@ def self.inherited(subclass) # rubocop:todo Lint/MissingSuper 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 @@ -170,7 +170,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] @@ -202,16 +201,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, 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 @@ -262,7 +252,8 @@ def self.execute_exceptionify(cmd, options) 'SEVERE: I/O error in channel CLI connection', 'java.net.SocketException: Connection reset', 'java.net.ConnectException: Connection refused', - 'java.io.IOException: Failed to connect' + 'java.io.IOException: Failed to connect', + 'java.io.IOException: Server returned HTTP response code: 503' ] if options.key?(:tmpfile_as_param) diff --git a/manifests/cli/config.pp b/manifests/cli/config.pp index 1c815ba67..0b1e71ec6 100644 --- a/manifests/cli/config.pp +++ b/manifests/cli/config.pp @@ -17,28 +17,10 @@ Boolean $cli_password_file_exists = false, Optional[String] $ssh_private_key_content = undef, ) { - if str2bool($facts['is_pe']) { - $gem_provider = 'pe_gem' - # lint:ignore:legacy_facts - } elsif $facts['rubysitedir'] and ('/opt/puppetlabs/puppet/lib/ruby' in $facts['rubysitedir']) { - # lint:endignore - # AIO puppet - $gem_provider = 'puppet_gem' - } else { - $gem_provider = 'gem' - } - # lint:ignore:legacy_facts $is_root = $facts['id'] == 'root' # lint:endignore - # 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', diff --git a/manifests/init.pp b/manifests/init.pp index 424b665ab..c59aa76e9 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -301,7 +301,7 @@ Optional[String] $cli_username = undef, Optional[String] $cli_password = undef, Optional[String] $cli_password_file = undef, - Integer $cli_tries = 10, + Integer $cli_tries = 9, Integer $cli_try_sleep = 10, Integer $port = 8080, Stdlib::Absolutepath $libdir = '/usr/share/java', diff --git a/spec/classes/cli/config_spec.rb b/spec/classes/cli/config_spec.rb index 9069004ba..4b998bf2a 100644 --- a/spec/classes/cli/config_spec.rb +++ b/spec/classes/cli/config_spec.rb @@ -126,26 +126,6 @@ end end end - - 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 - end end end end diff --git a/spec/unit/puppet/x/jenkins/config_spec.rb b/spec/unit/puppet/x/jenkins/config_spec.rb index 02221d7d1..583c47bb0 100644 --- a/spec/unit/puppet/x/jenkins/config_spec.rb +++ b/spec/unit/puppet/x/jenkins/config_spec.rb @@ -10,8 +10,7 @@ url: 'http://localhost:8080', ssh_private_key: nil, puppet_helper: '/usr/share/java/puppet_helper.groovy', - cli_tries: 30, - cli_try_sleep: 2 + cli_tries: 30 }.freeze shared_context 'facts' do @@ -21,7 +20,6 @@ Facter.add(:jenkins_ssh_private_key) { setcode { 'fact.id_rsa' } } Facter.add(:jenkins_puppet_helper) { setcode { 'fact.groovy' } } Facter.add(:jenkins_cli_tries) { setcode { 22 } } - Facter.add(:jenkins_cli_try_sleep) { setcode { 33 } } end end @@ -126,8 +124,7 @@ url: 'http://localhost:111', ssh_private_key: 'cat.id_rsa', puppet_helper: 'cat.groovy', - cli_tries: 222, - cli_try_sleep: 333 + cli_tries: 222 ) catalog.add_resource jenkins diff --git a/spec/unit/puppet/x/jenkins/provider/cli_spec.rb b/spec/unit/puppet/x/jenkins/provider/cli_spec.rb index b74e067ec..2a702269c 100644 --- a/spec/unit/puppet/x/jenkins/provider/cli_spec.rb +++ b/spec/unit/puppet/x/jenkins/provider/cli_spec.rb @@ -3,9 +3,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 @@ -38,7 +35,6 @@ Facter.add(:jenkins_puppet_helper) { setcode { 'fact.groovy' } } Facter.add(:jenkins_cli_username) { setcode { 'myuser' } } Facter.add(:jenkins_cli_tries) { setcode { 22 } } - Facter.add(:jenkins_cli_try_sleep) { setcode { 33 } } end end @@ -316,13 +312,6 @@ end 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( @@ -400,8 +389,7 @@ url: 'http://localhost:111', ssh_private_key: 'cat.id_rsa', cli_username: 'myuser', - cli_tries: 222, - cli_try_sleep: 333 + cli_tries: 222 ) catalog.add_resource jenkins @@ -423,10 +411,8 @@ context 'without ssh_private_key' do CLI_AUTH_ERRORS.each do |error| it 'does not retry cli on AuthError exception' do - expect(described_class.superclass).to receive(:execute).with( - 'java -jar /usr/share/java/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo', - failonfail: true, combine: true - ).and_raise(AuthError, error) + expect(described_class).to receive(:execute_with_auth).once.and_raise(AuthError, error) + expect(Puppet::Util::RetryAction).not_to receive(:sleep) expect { described_class.cli('foo') }. to raise_error(AuthError) @@ -490,14 +476,12 @@ context 'network failure' do context 'without ssh_private_key' do CLI_NET_ERRORS.each do |error| - it 'does not retry cli on AuthError exception' do - expect(described_class.superclass).to receive(:execute).with( - 'java -jar /usr/share/java/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo', - failonfail: true, combine: true - ).exactly(30).times.and_raise(NetError, error) + it 'retries cli on NetError exception' do + expect(described_class).to receive(:execute_with_auth).exactly(31).times.and_raise(NetError, error) + expect(Puppet::Util::RetryAction).to receive(:sleep).exactly(30).times expect { described_class.cli('foo') }. - to raise_error(NetError) + to raise_error(Puppet::Util::RetryAction::RetryException::RetriesExceeded) end end end @@ -514,10 +498,7 @@ ) catalog.add_resource jenkins - expect(described_class.superclass).to receive(:execute).with( - 'java -jar /usr/share/java/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo', - failonfail: true, combine: true - ).exactly(30).times.and_raise(UnknownError, 'foo') + expect(Puppet::Util::RetryAction).to receive(:retry_action).with(retries: 30, retry_exceptions: [UnknownError, NetError]).and_raise(UnknownError, 'foo') expect { described_class.cli('foo', catalog: catalog) }. to raise_error(UnknownError, 'foo') @@ -530,10 +511,7 @@ ) catalog.add_resource jenkins - expect(described_class.superclass).to receive(:execute).with( - 'java -jar /usr/share/java/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo', - failonfail: true, combine: true - ).twice.and_raise(UnknownError, 'foo') + expect(Puppet::Util::RetryAction).to receive(:retry_action).with(retries: 2, retry_exceptions: [UnknownError, NetError]).and_raise(UnknownError, 'foo') expect { described_class.cli('foo', catalog: catalog) }. to raise_error(UnknownError, 'foo') @@ -547,10 +525,7 @@ ) catalog.add_resource jenkins - expect(described_class.superclass).to receive(:execute).with( - 'java -jar /usr/share/java/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo', - failonfail: true, combine: true - ).exactly(3).times.and_raise(UnknownError, 'foo') + expect(Puppet::Util::RetryAction).to receive(:retry_action).with(retries: 3, retry_exceptions: [UnknownError, NetError]).and_raise(UnknownError, 'foo') expect { described_class.cli('foo', catalog: catalog) }. to raise_error(UnknownError, 'foo') @@ -565,70 +540,13 @@ ) catalog.add_resource jenkins - expect(described_class.superclass).to receive(:execute).with( - 'java -jar /usr/share/java/jenkins-cli.jar -s http://localhost:8080 -logger WARNING foo', - failonfail: true, combine: true - ).twice.and_raise(UnknownError, 'foo') + expect(Puppet::Util::RetryAction).to receive(:retry_action).with(retries: 2, retry_exceptions: [UnknownError, NetError]).and_raise(UnknownError, 'foo') expect { described_class.cli('foo', catalog: catalog) }. to raise_error(UnknownError, 'foo') end - end - - 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 + end # n times + end # should retry cli on UnknownError context 'options with :stdinjson' do RSpec::Matchers.define :a_json_doc do |x| diff --git a/spec/unit/puppet/x/spec_jenkins_providers.rb b/spec/unit/puppet/x/spec_jenkins_providers.rb index 4e144433d..51a2701cf 100644 --- a/spec/unit/puppet/x/spec_jenkins_providers.rb +++ b/spec/unit/puppet/x/spec_jenkins_providers.rb @@ -8,19 +8,6 @@ described_class.confine_collection.instance_variable_get(:@confines) end - it 'has no matched confines' do - expect(described_class.confine_collection.summary).to eq({}) - end - - context 'feature :retries' do - it do - expect(confines).to include( - be_a(Puppet::Confine::Feature). - and(have_attributes(values: [:retries])) - ) - end - end - context 'commands :java' do it do expect(confines).to include(