Skip to content

Add missing keyword_init to Struct.new in Gitaly server.rb

What does this MR do and why?

After !163430 (merged) got merged. We had some master:broken failures like: #482724 (comment 2087872420) and #482725 (comment 2087872586). The assumption here is that the Gitlab::GitalyClient::ServerService gets called multiple times and it caches an unexpected response in Rails.cache.

There was an attempt to fix some of those errors: !164887 (merged), but the clean_gitlab_redis_cache did not solve the original problem.

  1) API::WebCommits GET /web_commits/public_key when Gitaly is unavailable does not cache the public key
     Failure/Error:
       Rails.cache.fetch(GITALY_PUBLIC_KEY_CACHE_KEY, expires_in: 1.hour.to_i, skip_nil: true) do
         { public_key: server_signature_public_key }
       end

       (<ActiveSupport::Cache::NullStore>).fetch("gitaly_public_key", {:expires_in=>3600, :skip_nil=>true})
           expected: 0 times with arguments: ("gitaly_public_key", {:expires_in=>3600, :skip_nil=>true})
           received: 1 time with arguments: ("gitaly_public_key", {:expires_in=>3600, :skip_nil=>true})
     # ./lib/api/web_commits.rb:35:in `cache_public_key'
     # ./lib/api/web_commits.rb:54:in `block in <class:WebCommits>'
     # ./ee/lib/gitlab/middleware/ip_restrictor.rb:14:in `block in call'
     # ./lib/gitlab/ip_address_state.rb:11:in `with'
     # ./ee/lib/gitlab/middleware/ip_restrictor.rb:13:in `call'
     # ./lib/api/api_guard.rb:219:in `call'
     # ./lib/gitlab/middleware/sidekiq_shard_awareness_validation.rb:20:in `block in call'
     # ./lib/gitlab/sidekiq_sharding/validator.rb:42:in `enabled'
     # ./lib/gitlab/middleware/sidekiq_shard_awareness_validation.rb:20:in `call'
     # ./lib/gitlab/middleware/memory_report.rb:13:in `call'
     # ./lib/gitlab/middleware/speedscope.rb:13:in `call'
     # ./lib/gitlab/query_limiting/middleware.rb:17:in `block in call'
     # ./lib/gitlab/query_limiting/transaction.rb:48:in `run'
     # ./lib/gitlab/query_limiting/middleware.rb:16:in `call'
     # ./lib/gitlab/database/load_balancing/rack_middleware.rb:23:in `call'
     # ./lib/gitlab/middleware/go.rb:22:in `call'
     # ./lib/gitlab/etag_caching/middleware.rb:21:in `call'
     # ./lib/gitlab/middleware/query_analyzer.rb:11:in `block in call'
     # ./lib/gitlab/database/query_analyzer.rb:40:in `within'
     # ./lib/gitlab/middleware/query_analyzer.rb:11:in `call'
     # ./lib/gitlab/middleware/multipart.rb:173:in `call'
     # ./lib/gitlab/middleware/read_only/controller.rb:50:in `call'
     # ./lib/gitlab/middleware/read_only.rb:18:in `call'
     # ./lib/gitlab/middleware/unauthenticated_session_expiry.rb:18:in `call'
     # ./lib/gitlab/middleware/strip_cookies.rb:29:in `call'
     # ./lib/gitlab/middleware/same_site_cookies.rb:27:in `call'
     # ./lib/gitlab/middleware/path_traversal_check.rb:37:in `call'
     # ./lib/gitlab/middleware/handle_malformed_strings.rb:21:in `call'
     # ./lib/gitlab/middleware/basic_health_check.rb:25:in `call'
     # ./lib/gitlab/middleware/handle_ip_spoof_attack_error.rb:25:in `call'
     # ./lib/gitlab/middleware/request_context.rb:15:in `call'
     # ./lib/gitlab/middleware/webhook_recursion_detection.rb:15:in `call'
     # ./config/initializers/fix_local_cache_middleware.rb:11:in `call'
     # ./lib/gitlab/middleware/compressed_json.rb:44:in `call'
     # ./lib/gitlab/middleware/static.rb:11:in `call'
     # ./lib/gitlab/testing/clear_process_memory_cache_middleware.rb:13:in `call'
     # ./lib/gitlab/testing/request_inspector_middleware.rb:35:in `call'
     # ./lib/gitlab/testing/robots_blocker_middleware.rb:30:in `call'
     # ./lib/gitlab/testing/request_blocker_middleware.rb:47:in `call'
     # ./lib/gitlab/middleware/rack_multipart_tempfile_factory.rb:19:in `call'
     # ./lib/gitlab/middleware/sidekiq_web_static.rb:20:in `call'
     # ./lib/gitlab/metrics/requests_rack_middleware.rb:79:in `call'
     # ./spec/requests/api/web_commits_spec.rb:65:in `block (4 levels) in <main>'
     # ./spec/spec_helper.rb:471:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/sidekiq_sharding/validator.rb:42:in `enabled'
     # ./spec/spec_helper.rb:470:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:465:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:456:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:452:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:92:in `with_raw_context'
     # ./spec/spec_helper.rb:452:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:268:in `block (2 levels) in <top (required)>'
     # ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
     # ./spec/support/redis.rb:17:in `block (3 levels) in <main>'
     # ./spec/support/fast_quarantine.rb:22:in `block (2 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'

  2) API::WebCommits GET /web_commits/public_key when Gitaly is unavailable returns service unavailable
     Failure/Error: expect(response).to have_gitlab_http_status(:service_unavailable)
       expected the response to have status code :service_unavailable but it was 200. The response was: {"public_key":{"public_key":null,"error":true}}
     # ./spec/requests/api/web_commits_spec.rb:71:in `block (4 levels) in <main>'
     # ./spec/spec_helper.rb:471:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/sidekiq_sharding/validator.rb:42:in `enabled'
     # ./spec/spec_helper.rb:470:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:465:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:456:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:452:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:92:in `with_raw_context'
     # ./spec/spec_helper.rb:452:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:268:in `block (2 levels) in <top (required)>'
     # ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
     # ./spec/support/redis.rb:17:in `block (3 levels) in <main>'
     # ./spec/support/fast_quarantine.rb:22:in `block (2 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'

Finished in 59.5 seconds (files took 1 minute 11.71 seconds to load)
2 examples, 2 failures

Failed examples:

rspec ./spec/requests/api/web_commits_spec.rb:60 # API::WebCommits GET /web_commits/public_key when Gitaly is unavailable does not cache the public key
rspec ./spec/requests/api/web_commits_spec.rb:68 # API::WebCommits GET /web_commits/public_key when Gitaly is unavailable returns service unavailable
  1) Gitaly::Server#server_signature_error? when the server signature raises a GRPC error returns an error and no public_key
     Failure/Error: expect(server.server_signature_public_key).to be_nil

       expected: nil
            got: {:error=>true, :public_key=>nil}
     # ./spec/lib/gitaly/server_spec.rb:165:in `block (4 levels) in <main>'
     # ./spec/spec_helper.rb:471:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/sidekiq_sharding/validator.rb:42:in `enabled'
     # ./spec/spec_helper.rb:470:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:465:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:456:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:452:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:92:in `with_raw_context'
     # ./spec/spec_helper.rb:452:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:268:in `block (2 levels) in <top (required)>'
     # ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
     # ./spec/support/fast_quarantine.rb:22:in `block (2 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'

Finished in 50.53 seconds (files took 1 minute 11.92 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/lib/gitaly/server_spec.rb:164 # Gitaly::Server#server_signature_error? when the server signature raises a GRPC error returns an error and no public_key

Resolves: gitlab-org/quality/engineering-productivity/master-broken-incidents#8259 (closed)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Javiera Tapia

Merge request reports

Loading