fluent-bit icon indicating copy to clipboard operation
fluent-bit copied to clipboard
verified publisher icon fluent

in_kubernetes_events: add missing config parameter descriptions. Fixes #11243.

Open eschabell opened this issue 3 weeks ago • 7 comments

Add config_map entries for struct fields that were previously defined but not exposed as configuration options:

  • db.locking: set exclusive locking mode for the database
  • db.journal_mode: set the journal mode (DELETE, TRUNCATE,PERSIST, MEMORY, WAL, OFF)
  • dns_retries: dns lookup retries until network starts working
  • dns_wait_time: interval between network status checks
  • Also add the missing db.sync property reading code and journal_mode validation in kubernetes_events_conf.c to properly handle these database configuration options.

Fixes issue #11243

Testing Before we can approve your change; please submit the following in a comment:

  • [N/A ] Example configuration file for the change
  • [X ] Debug log output from testing the change

Added tests in_kubernetes_events: Add test coverage for the new configuration parameters:

  • db.sync: test all valid values (extra, full, normal, off)
  • db.journal_mode: test all valid values (DELETE, TRUNCATE, PERSIST, MEMORY, WAL, OFF)
  • db.locking: test boolean values (true, false)
  • dns_retries and dns_wait_time: test custom values

Add test_ctx_create_with_config() helper function to support testing optional configuration parameters without modifying the existing test context creator.

Did full Fluent Bit build with tests and ran:

$ ./bin/flb-rt-in_kubernetes_events 

...
[ OK ]
SUCCESS: All unit tests have passed.
  • [N/A ] Attached Valgrind output that shows no leaks or memory corruption was found
  • [N/A ] Run local packaging test showing all targets (including any new ones) build.
  • [N/A ] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [X ] Documentation required for this feature

Docs PR: https://github.com/fluent/fluent-bit-docs/pull/2273

Backporting

  • [N/A ] Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

  • New Features
    • Added runtime-configurable DB options for the Kubernetes events plugin: db.locking and db.journal_mode (with validation and defaults); db.sync is validated/normalized before DB open.
  • Changes
    • Removed DNS retry-related configuration (dns_retries and dns_wait_time) from the Kubernetes events plugin.
  • Tests
    • Added tests covering DB config permutations and validation.

✏️ Tip: You can customize this high-level summary in your review settings.

eschabell avatar Dec 02 '25 19:12 eschabell

Walkthrough

Added SQLDB-related runtime config options and pre-open validation for the Kubernetes events input, extended runtime tests covering DB config permutations (including a duplicated helper), and removed two DNS-related fields from the plugin's public struct.

Changes

Cohort / File(s) Summary
Plugin config additions
plugins/in_kubernetes_events/kubernetes_events.c		 Added SQLDB config entries db.locking (BOOL) and db.journal_mode (STR) under FLB_HAVE_SQLDB, mapped to new public struct fields.
Configuration validation
plugins/in_kubernetes_events/kubernetes_events_conf.c		 Added defaulting/validation for db.sync (maps extra→3, full→2, normal→1, off→0; invalid values log error and abort init) and validation for db.journal_mode (accepts DELETE, TRUNCATE, PERSIST, MEMORY, WAL, OFF; invalid values log error and abort init). Performed before DB open.
Public struct change
plugins/in_kubernetes_events/kubernetes_events.h		 Added fields to struct k8s_events: db_locking (BOOL) and db_journal_mode (STR). Removed two fields from struct k8s_events: dns_retries and dns_wait_time.
Tests
tests/runtime/in_kubernetes_events.c		 Added test_ctx_create_with_config helper (duplicated in file) and tests: flb_test_config_db_sync_values, flb_test_config_db_journal_mode_values, flb_test_config_db_locking_values. Tests create contexts with config overrides, start the pipeline, and clean up.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Verify duplicated test_ctx_create_with_config is consolidated.
  • Confirm db_locking and db_journal_mode fields are initialized/defaulted and correctly mapped in config_map.
  • Validate that invalid db.sync and db.journal_mode values intentionally abort initialization and produce clear error messages.
  • Search for remaining references to removed dns_retries and dns_wait_time.

Suggested reviewers

  • edsiper
  • cosmo0920

Poem

🐰 In a burrow of code I found a new key,
Journals and locks now waltz merrily,
Tests planted carrots in orderly rows,
Two DNS bits hopped off where the cold wind blows,
Hooray — I nibble and dance as the pipeline grows!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change—adding missing config parameter descriptions to the in_kubernetes_events plugin—and is clear and concise.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f88c4c74119f174e12521ef7cac7f3c7a9ff85d and ff81e2e71a605eac1d990030e575ee287b10a364.

📒 Files selected for processing (4)
  • plugins/in_kubernetes_events/kubernetes_events.c (1 hunks)
  • plugins/in_kubernetes_events/kubernetes_events.h (0 hunks)
  • plugins/in_kubernetes_events/kubernetes_events_conf.c (1 hunks)
  • tests/runtime/in_kubernetes_events.c (2 hunks)
💤 Files with no reviewable changes (1)
  • plugins/in_kubernetes_events/kubernetes_events.h
🚧 Files skipped from review as they are similar to previous changes (3)
  • plugins/in_kubernetes_events/kubernetes_events.c
  • tests/runtime/in_kubernetes_events.c
  • plugins/in_kubernetes_events/kubernetes_events_conf.c
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
  • GitHub Check: PR - fuzzing test
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-without-cxx (3.31.6)
  • GitHub Check: pr-compile-centos-7

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Dec 02 '25 19:12 coderabbitai[bot]

thanks for this PR.

I noticed that while Database options have a proper code, there is no code handling for dns_wait_time and dns_retries, so the options are not being used, so they are a placeholder and should not be exposed to the user in the configuration.

There might be cases where in addition to the config map registration, we need to check if that option/flag is actually used.

edsiper avatar Dec 03 '25 16:12 edsiper

@edsiper pushed changes for requested removal of dns_wait_time and dns_retries, including cleaning up of their testing code.

eschabell avatar Dec 04 '25 09:12 eschabell

@eschabell thanks for the updates, as a last steps we just need to clean the history of commits

image

edsiper avatar Dec 05 '25 23:12 edsiper

@edsiper squashing here too.

eschabell avatar Dec 09 '25 12:12 eschabell

Can't pass the commit linter as there are two areas of work, one in_kubernetes_events: plugin and the other in tests:, so if I commit with the following they all fail:

commit message with tests:

tests: missing db config options

Subject prefix 'tests:' does not match files changed.
Expected one of: in_kubernetes_events:, tests:

commit message with in_kubernetes_events:

in_kubernetes_events: missing db config options

Subject prefix 'in_kubernetes_events:' does not match files changed.
Expected one of: in_kubernetes_events:, tests:

commit message with in_kubernetes_events: tests:

in_kubernetes_events: tests: missing db config options

Subject prefix 'in_kubernetes_events:' does not match files changed.
Expected one of: in_kubernetes_events:, tests:

@edsiper @cosmo0920 @lecaros What is the way to get past this?

eschabell avatar Dec 10 '25 09:12 eschabell

commit message with in_kubernetes_events: tests:

in_kubernetes_events: tests: missing db config options

Subject prefix 'in_kubernetes_events:' does not match files changed.
Expected one of: in_kubernetes_events:, tests:

in_kubernetes_events: tests: missing db config options should be passed against the linter of commit message. If not passing, you have to rebase off master. The current master version of this linter could loosen and rigidly handle such type of commit messages.

cosmo0920 avatar Dec 11 '25 10:12 cosmo0920

@cosmo0920 pushed amended commit message for the commit linter like you suggested, see what the results are? Same error, so maybe someone can use this to adjust the linter?

eschabell avatar Dec 15 '25 15:12 eschabell

@cosmo0920 pushed amended commit message for the commit linter like you suggested, see what the results are? Same error, so maybe someone can use this to adjust the linter?

❌ Commit 6f20aa7a1b failed: Subject prefix 'in_kubernetes_events:' does not match files changed. Expected one of: in_kubernetes_events:, tests:

This linter error indicates that src itself and contaminated with tests directory. We usually commit src itself and their tests separately.

cosmo0920 avatar Dec 16 '25 06:12 cosmo0920

@cosmo0920 adjusted back to two commit messages, one for in_kubernetes_events: and one for tests:, fingers crossed this works.

eschabell avatar Dec 16 '25 08:12 eschabell

@cosmo0920 adjusted back to two commit messages, one for in_kubernetes_events: and one for tests:, fingers crossed this works.

Yeah, we did! Nice work. 😄

Run python .github/scripts/commit_prefix_check.py ✅ Commit prefix validation passed.

cosmo0920 avatar Dec 16 '25 08:12 cosmo0920

@edsiper can you refresh your review?

eschabell avatar Dec 16 '25 11:12 eschabell

tests commit must be prefixed with tests: runtime: in_kubernetes_events: ...

edsiper avatar Dec 17 '25 14:12 edsiper