Skip to content

feat(router): hot config reloading #1746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 35 commits into from
May 19, 2025
Merged

Conversation

endigma
Copy link
Member

@endigma endigma commented Apr 1, 2025

Motivation and Context

As a user who deploys the router with a dynamic config from the filesystem, I'd like it such that when the file changes on disk, there is the option to have the router automatically reload itself when it detects this change.

Changes

  • Better responsibility boundary between cmd/router/main.go and router/pkg/config/config.go
    • config.LoadConfig no longer loads dotenv into the environment or decides the config path under any circumstance
    • These responsibilities fall to main.Main now
  • The log level is now atomic and mutable at runtime if you hold a reference
  • The router is now capable of executing a full reload when either the config file changes or SIGHUP signal is sent to the process
  • Moved randomDuration utility out of controlplane package into internal/timex for reuse
  • Added configuration for watching the config file and triggering a reload when a change is detected (example below)

Config responsibility change

As a side effect of drawing a clearer boundary between the responsibilities of main.go and config.go for determining the config file to read, direct calls to LoadConfig will no longer check CONFIG_FILE environment variable. This is intentional, as this responsibility now falls to the caller.

LoadConfig continues to default to config.DefaultConfigPath when the provided path is "".

Limitations

The config path loading and the file watcher are outside the reload loop, so:

  • Configuration hot-reloading will not detect changes to the config path
  • Configuration for the file watching behaviour itself will not be updated during reloads

Configuration Example

watch_config:
  enabled: true
  interval: "10s"

  startup_delay:
    enabled: false
    maximum: "10s"

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.

@github-actions github-actions bot added the router label Apr 1, 2025
@endigma endigma force-pushed the jesse/eng-6609-hot-config-reloading branch from 8a15b9e to 7348dd5 Compare April 1, 2025 20:33
Copy link

github-actions bot commented Apr 1, 2025

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-665fa6436a60b33e7252eb76891d029ebc54a9c6

@endigma endigma force-pushed the jesse/eng-6609-hot-config-reloading branch 4 times, most recently from 0259b29 to 2b9f831 Compare April 3, 2025 10:09
@endigma endigma marked this pull request as ready for review April 3, 2025 18:44
@endigma endigma force-pushed the jesse/eng-6609-hot-config-reloading branch from 15fa6b5 to e3c5c15 Compare April 3, 2025 18:44
@endigma endigma requested review from StarpTech and Copilot April 3, 2025 18:44
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces hot configuration reloading for the router by adding file watching and SIGHUP trigger support, while also refining the config loading responsibilities between main.go and config.go.

  • Updated configuration to include hot-reload settings
  • Modified LoadConfig signature and updated tests accordingly
  • Enhanced shutdown error handling with a new concurrent error joiner and integrated config watcher in the main command

Reviewed Changes

Copilot reviewed 21 out of 26 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
router/pkg/config/fixtures/full.yaml Added watch_config section for enabling hot config reloading
router/pkg/config/config_test.go Updated tests to use the new LoadConfig signature and added tests for watch configuration via environment variables
router/core/router.go Introduced a concurrent error joiner and updated router shutdown logic with improvements
router/internal/timex/duration_test.go Added tests for RandomDuration function; contains an error in the loop iteration
Other files Minor adjustments (e.g., quoting in YAML/compose files, dotenv usage, test env helpers) to support the overall changes
Files not reviewed (5)
  • docker/nats/Dockerfile: Language not supported
  • docker/nats/build-push.sh: Language not supported
  • docker/nats/nats-server.conf: Language not supported
  • router-tests/go.mod: Language not supported
  • router/pkg/config/config.schema.json: Language not supported

@endigma endigma force-pushed the jesse/eng-6609-hot-config-reloading branch from 049cc58 to 90a6c00 Compare April 7, 2025 12:08
@endigma endigma force-pushed the jesse/eng-6609-hot-config-reloading branch 5 times, most recently from 7a82e01 to 812ab58 Compare April 15, 2025 15:48
@endigma endigma requested a review from StarpTech April 15, 2025 15:50
@endigma endigma force-pushed the jesse/eng-6609-hot-config-reloading branch 5 times, most recently from f257a58 to 10c406b Compare April 22, 2025 12:43
@endigma endigma force-pushed the jesse/eng-6609-hot-config-reloading branch 4 times, most recently from 3654e1c to 6215c28 Compare April 29, 2025 21:17
@endigma endigma force-pushed the jesse/eng-6609-hot-config-reloading branch from 3dd9987 to d6611cb Compare May 19, 2025 10:16
@endigma endigma force-pushed the jesse/eng-6609-hot-config-reloading branch from d6611cb to aad72f2 Compare May 19, 2025 10:16
@endigma endigma requested a review from StarpTech May 19, 2025 10:37
Copy link
Contributor

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

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

LGTM

@endigma endigma merged commit e87d727 into main May 19, 2025
24 checks passed
@endigma endigma deleted the jesse/eng-6609-hot-config-reloading branch May 19, 2025 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants