diff --git a/.github/workflows/criteo-cookbooks-ci.yml b/.github/workflows/criteo-cookbooks-ci.yml index e01e4c3..0f983a8 100644 --- a/.github/workflows/criteo-cookbooks-ci.yml +++ b/.github/workflows/criteo-cookbooks-ci.yml @@ -15,24 +15,23 @@ jobs: - uses: actions/checkout@v2 - uses: ruby/setup-ruby@v1 with: - ruby-version: 2.7 + ruby-version: 3.0.3 bundler-cache: true - run: bundle exec rubocop --version - run: bundle exec rubocop - - run: bundle exec foodcritic --version - - run: bundle exec foodcritic . --exclude spec -f any + - run: bundle exec cookstyle -c cookstyle.yml . - run: bundle exec rspec kitchen: needs: [rspec] runs-on: ubuntu-latest strategy: matrix: - instance: ['default-centos-7', 'default-ubuntu-1604', 'default-fedora-29'] + instance: ['default-centos-8', 'default-ubuntu-2404', 'default-fedora-40'] steps: - uses: actions/checkout@v2 - uses: ruby/setup-ruby@v1 with: - ruby-version: 2.7 + ruby-version: 3.0.3 bundler-cache: true - run: KITCHEN_LOCAL_YAML=.kitchen.docker.yml bundle exec kitchen verify ${{ matrix.instance }} supermarket: diff --git a/.kitchen.docker.yml b/.kitchen.docker.yml index 1d744a7..8cf56bf 100644 --- a/.kitchen.docker.yml +++ b/.kitchen.docker.yml @@ -16,28 +16,31 @@ verifier: name: inspec platforms: -- name: centos-7 +- name: centos-8 driver: - image: centos:7 + image: centos:8 intermediate_instructions: - - RUN yum clean all - - RUN yum install -y sudo + - RUN dnf clean all + # - RUN dnf install -y sudo # Centos 8 deprecated see https://blog.centos.org/2023/04/end-dates-are-coming-for-centos-stream-8-and-centos-linux-7/ pid_one_command: /usr/lib/systemd/systemd -- name: fedora-29 +- name: fedora-40 driver: - image: fedora:29 + image: fedora:40 intermediate_instructions: - - RUN yum clean all - - RUN yum install -y sudo - pid_one_command: /usr/lib/systemd/systemd + - RUN dnf clean all + - RUN dnf install -y sudo + - RUN dnf install -y libxcrypt-compat + - RUN dnf install -y systemd + pid_one_command: /lib/systemd/systemd -- name: ubuntu-16.04 +- name: ubuntu-24.04 driver: - image: ubuntu:16.04 - pid_one_command: /bin/systemd + image: ubuntu:24.04 + pid_one_command: /usr/bin/systemd intermediate_instructions: - - RUN /usr/bin/apt-get update + - RUN /usr/bin/apt-get -y update + - RUN /usr/bin/apt install -y systemd - RUN /usr/bin/apt-get install sudo -y suites: diff --git a/.rubocop.yml b/.rubocop.yml index 1e539a0..11bcf09 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -4,12 +4,12 @@ # - submit the change in the code_generator AllCops: - TargetRubyVersion: 2.7 + TargetRubyVersion: 3.0.3 Style/FrozenStringLiteralComment: Enabled: false -Metrics/LineLength: +Layout/LineLength: Max: 180 @@ -19,7 +19,7 @@ Metrics/ModuleLength: Metrics/MethodLength: Max: 40 -Style/VariableNumber: +Naming/VariableNumber: Enabled: false Metrics/AbcSize: @@ -43,7 +43,7 @@ Style/MultilineBlockChain: Enabled: false -Style/FileName: +Naming/FileName: Enabled: false Lint/ConstantDefinitionInBlock: @@ -71,3 +71,6 @@ Metrics/ClassLength: Style/ClassVars: Enabled: false + +Style/HashSyntax: + EnforcedShorthandSyntax: never diff --git a/Gemfile b/Gemfile index d7a9091..7088bc8 100644 --- a/Gemfile +++ b/Gemfile @@ -4,6 +4,7 @@ gem 'chef', '>= 17.9.26' gem 'chefspec', '>= 9.3.1' gem 'berkshelf' +gem 'cookstyle' gem 'foodcritic' gem 'kitchen-dokken' gem 'kitchen-inspec' diff --git a/cookstyle.yml b/cookstyle.yml new file mode 100644 index 0000000..a2db76d --- /dev/null +++ b/cookstyle.yml @@ -0,0 +1,49 @@ +AllCops: + TargetChefVersion: 17.9.46 + TargetRubyVersion: 3.0.3 # Matches Chef client 17.9.46 + +# Chef +Chef/Correctness/IncorrectLibraryInjection: + Enabled: false +Chef/Deprecations/FoodcriticTesting: + Enabled: false +Chef/RedundantCode/LongDescriptionMetadata: + Enabled: false +Chef/RedundantCode/NamePropertyIsRequired: + Enabled: false +Chef/Deprecations/ResourceWithoutUnifiedTrue: + Enabled: false +Chef/Deprecations/UseInlineResourcesDefined: + Enabled: false +Chef/Modernize/RespondToInMetadata: + Enabled: false +Chef/Modernize/ShellOutHelper: + Enabled: false +Chef/Modernize/WhyRunSupportedTrue: + Enabled: false +Chef/Sharing/EmptyMetadataField: + Enabled: false +Chef/Sharing/InvalidLicenseString: + Enabled: false +Chef/Style/CommentFormat: + Enabled: false + +# Layout +Layout/RescueEnsureAlignment: + Enabled: false + +# Style +Style/ClassVars: + Enabled: false +Style/FormatString: + Enabled: false +Style/PercentLiteralDelimiters: + Enabled: false +Style/StringLiterals: + Enabled: false +Style/TrailingCommaInArguments: + Enabled: false +Style/TrailingCommaInArrayLiteral: + Enabled: false +Style/TrailingCommaInHashLiteral: + Enabled: false diff --git a/libraries/choregraphie.rb b/libraries/choregraphie.rb index 7a751a2..af28bb1 100644 --- a/libraries/choregraphie.rb +++ b/libraries/choregraphie.rb @@ -129,11 +129,11 @@ def self.ensure_whyrun_supported(run_context, resource_name, ignore_missing_reso begin resource = run_context.resource_collection.find(resource_name) rescue Chef::Exceptions::ResourceNotFound - if ignore_missing_resource - # some resources are defined only when used - # so we ignore them - return - elsif resource_name =~ /,/ + # some resources are defined only when used + # so we ignore them + return if ignore_missing_resource + + if resource_name =~ /,/ # rubocop: disable Style/GuardClause Chef::Log.warn "#{resource_name} contains a comma which triggers " \ "https://github.com/criteo-cookbooks/choregraphie/issues/43, we can't check if resource supports why run or not" resource = run_context.resource_collection.map(&:itself).find { |r| r.declared_key == resource_name } diff --git a/libraries/primitive_consul_lock.rb b/libraries/primitive_consul_lock.rb index 37df8c7..530d664 100644 --- a/libraries/primitive_consul_lock.rb +++ b/libraries/primitive_consul_lock.rb @@ -51,7 +51,7 @@ def semaphore_class def semaphore # this object cannot be reused after enter/exit - semaphore_class.get_or_create(path, concurrency, dc: @options[:datacenter], token: @options[:token]) + semaphore_class.get_or_create(path, concurrency: concurrency, dc: @options[:datacenter], token: @options[:token]) end def backoff(start_time, current_try) @@ -113,20 +113,22 @@ def path end class Semaphore - def self.get_or_create(path, concurrency, dc: nil, token: nil) + def self.get_or_create(path, concurrency:, **kwargs) + dc = kwargs[:dc] + token = kwargs[:token] require 'diplomat' retry_left = 5 value = Mash.new({ version: 1, concurrency: concurrency, holders: {} }) current_lock = begin Chef::Log.info "Fetch lock state for #{path}" - Diplomat::Kv.get(path, decode_values: true, dc: dc, token: token) + Diplomat::Kv.get(path, decode_values: true, dc: dc, token: dc) rescue Diplomat::KeyNotFound Chef::Log.info "Lock for #{path} did not exist, creating with value #{value}" Diplomat::Kv.put(path, value.to_json, cas: 0, dc: dc, token: token) # we ignore success/failure of CaS (retry_left -= 1).positive? ? retry : raise end.first desired_lock = bootstrap_lock(value, current_lock) - new(path, desired_lock, dc, token: token) + new(path, new_lock: desired_lock, dc: dc, token: token) end def self.bootstrap_lock(desired_value, current_lock) @@ -138,7 +140,9 @@ def self.bootstrap_lock(desired_value, current_lock) desired_lock end - def initialize(path, new_lock, dc = nil, token: nil) + def initialize(path, new_lock:, **kwargs) + dc = kwargs[:dc] + token = kwargs[:token] @path = path @h = JSON.parse(new_lock['Value']) @cas = new_lock['ModifyIndex'] diff --git a/spec/unit/lock_spec.rb b/spec/unit/lock_spec.rb index 8a7ca43..ad19314 100644 --- a/spec/unit/lock_spec.rb +++ b/spec/unit/lock_spec.rb @@ -37,7 +37,7 @@ def self.debug(_); end stub_request(:get, 'http://localhost:8500/v1/kv/check-lock/my_lock') .to_return(existing_response) - expect(Semaphore.get_or_create('check-lock/my_lock', 1, dc: nil, token: nil)).to be_a(Semaphore) + expect(Semaphore.get_or_create('check-lock/my_lock', concurrency: 1, dc: nil, token: nil)).to be_a(Semaphore) end end @@ -50,7 +50,7 @@ def self.debug(_); end .with(body: '{"version":1,"concurrency":2,"holders":{}}') .to_return(status: 200, body: 'true') - expect(Semaphore.get_or_create('check-lock/my_lock', 2, dc: nil, token: nil)).to be_a(Semaphore) + expect(Semaphore.get_or_create('check-lock/my_lock', concurrency: 2, dc: nil, token: nil)).to be_a(Semaphore) end end end @@ -59,14 +59,14 @@ def self.debug(_); end value = { version: 1, concurrency: 5, holders: { another_node: 12_345 } }.to_json Semaphore.new( 'check-lock/my_lock', - { 'Value' => value, 'ModifyIndex' => 42 }, + new_lock: { 'Value' => value, 'ModifyIndex' => 42 }, ) end let(:full_lock) do value = { version: 1, concurrency: 1, holders: { another_node: 12_345 } }.to_json Semaphore.new( 'check-lock/my_lock', - { 'Value' => value, 'ModifyIndex' => 42 }, + new_lock: { 'Value' => value, 'ModifyIndex' => 42 }, ) end diff --git a/spec/unit/primitive_consul_lock_spec.rb b/spec/unit/primitive_consul_lock_spec.rb index 7469756..35c85a1 100644 --- a/spec/unit/primitive_consul_lock_spec.rb +++ b/spec/unit/primitive_consul_lock_spec.rb @@ -62,7 +62,7 @@ lock = double('lock') expect(lock).to receive(:enter).with(name: 'my_node').and_return(true) - expect(Semaphore).to receive(:get_or_create).with('chef_lock/test', 3, dc: nil, token: nil).and_return(lock) + expect(Semaphore).to receive(:get_or_create).with('chef_lock/test', concurrency: 3, dc: nil, token: nil).and_return(lock) choregraphie_service.before.each(&:call) end diff --git a/spec/unit/rack_lock_spec.rb b/spec/unit/rack_lock_spec.rb index a93bb8b..157ca70 100644 --- a/spec/unit/rack_lock_spec.rb +++ b/spec/unit/rack_lock_spec.rb @@ -37,7 +37,7 @@ def self.debug(_); end stub_request(:get, 'http://localhost:8500/v1/kv/check-lock/my_lock') .to_return(existing_response) - expect(SemaphoreByRack.get_or_create('check-lock/my_lock', 2, dc: nil, token: nil)).to be_a(SemaphoreByRack) + expect(SemaphoreByRack.get_or_create('check-lock/my_lock', concurrency: 2, dc: nil, token: nil)).to be_a(SemaphoreByRack) end end @@ -50,7 +50,7 @@ def self.debug(_); end .with(body: '{"version":1,"concurrency":2,"holders":{}}') .to_return(status: 200, body: 'true') - expect(SemaphoreByRack.get_or_create('check-lock/my_lock', 2, dc: nil, token: nil)).to be_a(SemaphoreByRack) + expect(SemaphoreByRack.get_or_create('check-lock/my_lock', concurrency: 2, dc: nil, token: nil)).to be_a(SemaphoreByRack) end end end @@ -59,21 +59,21 @@ def self.debug(_); end value = { version: 1, concurrency: 5, holders: { another_rack: { another_node: 12_345 } } }.to_json SemaphoreByRack.new( 'check-lock/my_lock', - 'Value' => value, 'ModifyIndex' => 42, + new_lock: { 'Value' => value, 'ModifyIndex' => 42 }, ) end let(:full_lock) do value = { version: 1, concurrency: 1, holders: { another_rack: { another_node: 12_345 } } }.to_json SemaphoreByRack.new( 'check-lock/my_lock', - 'Value' => value, 'ModifyIndex' => 42, + new_lock: { 'Value' => value, 'ModifyIndex' => 42 }, ) end let(:taken_lock) do value = { version: 1, concurrency: 5, holders: { my_rack: { myself: 12_345, another_node: 123_456 } } }.to_json SemaphoreByRack.new( 'check-lock/my_lock', - 'Value' => value, 'ModifyIndex' => 42, + new_lock: { 'Value' => value, 'ModifyIndex' => 42 }, ) end