Skip to content

Fixes #39229 - Add foreman_request_timeout setting to control Net::HTTP read timeout#936

Open
pablomh wants to merge 1 commit intotheforeman:developfrom
pablomh:fix-foreman-request-timeout
Open

Fixes #39229 - Add foreman_request_timeout setting to control Net::HTTP read timeout#936
pablomh wants to merge 1 commit intotheforeman:developfrom
pablomh:fix-foreman-request-timeout

Conversation

@pablomh
Copy link
Copy Markdown

@pablomh pablomh commented Apr 13, 2026

Problem

ForemanRequest uses a bare Net::HTTP instance whose read_timeout defaults to Ruby's hardcoded 60s. Under high-concurrency registration load this causes 500 errors when Foreman takes longer than 60s to process POST /register.

Fix

Introduces :foreman_request_timeout and documents it in settings.yml.example, then applies it to http.read_timeout in ForemanRequest#http_init when configured and greater than zero. The respond_to? guard preserves backward compatibility when the setting is absent.

subscription-manager's default server_timeout is 180s, so values above that are recommended to avoid cutting connections the client is still waiting on.

Fixes https://projects.theforeman.org/issues/39229

@pablomh
Copy link
Copy Markdown
Author

pablomh commented Apr 17, 2026

CI failure: rake 13.4.2 regression (not related to this PR)

All test jobs fail with rake_test_loader: version unknown and exit code 1. The test runner never actually runs any tests.

Root cause: gem 'rake' in bundler.d/test.rb has no version constraint and no Gemfile.lock is committed, so CI resolves the latest version from rubygems.org. Rake 13.4.2 (released April 16) changed Rake::TestTask#option_list to append -v to the test command when verbose = true — previously this flag was not passed to the test runner. The new -v causes test-unit 3.7.7 to fail silently.

This affects all new PRs on smart-proxy, not just this one. PR #935 passed because its bundle cache still had rake 13.3.1.

Diff in lib/rake/testtask.rb (13.3.1 → 13.4.2):

# 13.3.1 — option_list never adds -v
def option_list
  (ENV["TESTOPTS"] || ENV["TESTOPT"] || ENV["TEST_OPTS"] || ENV["TEST_OPT"] || @options || "")
end

# 13.4.2 — now adds -v when verbose is true
def option_list(verbose: @verbose)
  opts = ENV["TESTOPTS"] || ENV["TESTOPT"] || ENV["TEST_OPTS"] || ENV["TEST_OPT"] || @options || ""
  if verbose && !opts.split.include?("-v")
    opts = opts.empty? ? "-v" : "#{opts} -v"
  end
  opts
end

Fix options:

  • Pin gem 'rake', '< 13.4' in bundler.d/test.rb (workaround)
  • Report the regression upstream to ruby/rake
  • Or fix the test-unit interaction with -v if the issue is on that side

Copy link
Copy Markdown
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, but 1 minor comment inline.

Comment thread lib/proxy/request.rb Outdated
…TP read timeout

ForemanRequest uses a bare Net::HTTP instance whose read_timeout defaults to
Ruby's hardcoded 60s. Under high-concurrency registration load this causes
500 errors when Foreman takes longer than 60s to process POST /register.

Introduces :foreman_request_timeout (documented in settings.yml.example) and
applies it to http.read_timeout in ForemanRequest#http_init when configured.
The respond_to? guard preserves backward compatibility when the setting is absent.

subscription-manager's default server_timeout is 180s, so values above that
are recommended to avoid cutting connections the client is still waiting on.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants