Update ruby version + fix CI

- CI is failing due to lib compatibility versions
- Update code to ruby 3.1
- Fix rubocop on Style/HashSyntax
- Update rubocop config namespace
- Replace foodcritic by cookstyle in the CI run
- Update kitchen docker:
  - update OS to latest
  - update chef client to 18.3.0 so that ruby 3.1 is used

.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.1.6
           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.1.6
           bundler-cache: true
       - run: KITCHEN_LOCAL_YAML=.kitchen.docker.yml bundle exec kitchen verify ${{ matrix.instance }}
   supermarket:

.kitchen.docker.yml

@@ -2,7 +2,7 @@
 ---
 driver:
   name: dokken
-  chef_version: 17.9.26
+  chef_version: 18.3.0
   privileged: true # because Docker and SystemD/Upstart
 
 transport:
@@ -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:

.rubocop.yml

@@ -4,12 +4,12 @@
 # - submit the change in the code_generator
 
 AllCops:
-  TargetRubyVersion: 2.7
+  TargetRubyVersion: 3.1
 
 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:

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'

cookstyle.yml

@@ -0,0 +1,48 @@
+AllCops:
+  TargetRubyVersion: 3.1.6
+
+# 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

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 }

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:, 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: {} })
+      value = Mash.new({ version: 1, 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:, token:)
       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
+        Diplomat::Kv.put(path, value.to_json, cas: 0, dc:, 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:, 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']

libraries/primitive_consul_maintenance.rb

@@ -47,9 +47,9 @@ def maintenance_reason
     def maintenance(enable = true) # rubocop:disable Style/OptionalBooleanParameter
       token = @options[:consul_token]
       if @options.key?(:service_id)
-        Diplomat::Service.maintenance(@options[:service_id], { enable: enable, reason: @options[:reason], token: token })
+        Diplomat::Service.maintenance(@options[:service_id], { enable:, reason: @options[:reason], token: })
       else
-        Diplomat::Maintenance.enable(enable, @options[:reason], { token: token })
+        Diplomat::Maintenance.enable(enable, @options[:reason], { token: })
       end
     end
 

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
 

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

spec/unit/primitive_consul_maintenance_spec.rb

@@ -41,7 +41,7 @@
     let(:service_id) { 'service_test' }
     let(:choregraphie) do
       Choregraphie::Choregraphie.new('test') do
-        consul_maintenance service_id: service_id, reason: 'Testing', consul_token: 'foo', check_interval: 0
+        consul_maintenance service_id:, reason: 'Testing', consul_token: 'foo', check_interval: 0
       end
     end
 

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