reload: preserve configuration path during hot reload - #12336
Conversation
Relative service file references resolve against the directory containing the main configuration. Reload created a new context without preserving that directory, so relative files could no longer be resolved. Copy the canonical path before loading the new configuration. Parsers, external plugins, upstream HA files, and stream processor files all use this path when resolving relative references. Fixes fluent#12333 Signed-off-by: Antonio Santos <antonio@santosvelasco.com>
Signed-off-by: Antonio Santos <antonio@santosvelasco.com>
Signed-off-by: Antonio Santos <antonio@santosvelasco.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughHot reload now preserves the main configuration directory when creating a new configuration. Integration and internal tests verify relative parser files after startup and HTTP-triggered reloads. ChangesHot reload relative path handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Hot reload now preserves the configuration directory so relative parser and plugin references resolve consistently with startup; the replacement context is independently owned and cleaned up on pre-replacement failures, and no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The implementation copies old_config->conf_path into the replacement context. The integration and internal tests cover relative parser files in classic and YAML configurations, which addresses the relative-path resolution failure described in issue [
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a1ed7d9de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
cosmo0920
left a comment
There was a problem hiding this comment.
Thanks for your work.
It's a really good first step to mitigate handling relative paths during hot-reloading.
I found an issue on a testing assertion.
Could you address on it?
Signed-off-by: Antonio Santos <antonio@santosvelasco.com>
Hot reload now preserves the directory of the main configuration when it creates the replacement context.
Fluent Bit uses this path to resolve relative service file references. Startup sets the path correctly, but reload creates a fresh context without carrying it over. As a result, relative
Parsers_FileandPlugins_Filereferences that work at startup fail during reload.Copying the path before loading the new configuration allows the existing path resolution to work unchanged. This also adds regression coverage for relative parser paths in both classic and YAML configurations.
Disclosure: I developed this change with AI assistance. I have not written C code in more than 20 years, so I would appreciate particular scrutiny of the memory ownership, cleanup paths, and conformity with Fluent Bit’s conventions.
Fixes #12333
Summary by CodeRabbit
Bug Fixes
Tests