[manoj-1407]

Overview

walkVarsDir in sensor_kernelvars.go opened each entry of a /proc/sys/kernel directory and did defer varFile.Close() inside the loop. A defer runs when the function returns, not at the end of the iteration, so every file in a directory stayed open until the whole directory (and everything nested in it) had been read. Open file descriptors grew with the number of entries in a directory instead of staying constant.

The per-entry work is now in readVarsEntry, where the defer is correct, so each handle is closed as soon as that entry is done. Behaviour is the same otherwise: same skip rules for "permission denied" and "operation not permitted", same errors, same recursion. Open descriptors now depend on directory depth instead of directory size.

Additional Information

On a normal node you probably won't hit this: a real /proc/sys/kernel doesn't have that many entries and the usual fd limit is high. It shows up with a low RLIMIT_NOFILE or when the agent is already using a lot of descriptors. Any error return halfway through the walk also left all those handles pending.

The regression test fakes a host root with 300 variables, lowers RLIMIT_NOFILE to the current open fds + 64, and walks it. On the old code it fails with failed to open file (.../proc/sys/kernel/var204): ... too many open files. It restores the limit afterwards and skips if the limit can't be lowered. Since the limit is process-wide it's worth knowing about if parallel tests ever get added to this package (none use t.Parallel today).

How to Test

go test ./pkg/hostsensormanager/ -run KernelVariables -count=1 -v

There are also tests for collecting files and nested directories, an empty directory, and a missing /proc/sys/kernel.

Summary by CodeRabbit

• Bug Fixes
  • Improved kernel-variable collection across nested directories, including more consistent handling of inaccessible files and read errors.
  • Reduced the chance of collection failures when directories contain many files by releasing file resources as entries are processed.
• Tests
  • Added coverage for nested, empty, and missing kernel-variable directories, as well as large directories under limited system resources.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 017badfa-62c6-487a-a18c-5f766112da76

📥 Commits

Reviewing files that changed from the base of the PR and between 02f0bf7 and 02d2e58.

📒 Files selected for processing: 2

• pkg/hostsensormanager/sensor_kernelvars.go
• pkg/hostsensormanager/sensor_kernelvars_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The kernel-variable directory walk now processes and closes each entry individually, propagates selected errors, and recursively collects regular-file variables. Linux-only tests cover normal, empty, missing-directory, and high-file-count cases.

Changes: Kernel Variable Sensing

Layer / File(s)	Summary
Entry processing and sensing coverage pkg/hostsensormanager/sensor_kernelvars.go, pkg/hostsensormanager/sensor_kernelvars_test.go	The directory walk delegates entry processing to a helper that closes file handles, recurses into directories, and returns variables or errors. Linux-only tests cover normal collection, empty and missing directories, and reading 303 variables with a reduced open-file limit.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Suggested reviewers: bezbran

Merge Risk: ⚪ Minimal · up to 02d2e

This change closes kernel variable files after each entry instead of at the end of the directory walk, which avoids "too many open files" failures on large directories. No merge-blocking risk was found.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: closing kernel variable files after each entry instead of after the directory walk.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[matthyx]

Approve — no material blockers found in 02d2e583437831c14b039a0534f1306c7ad8ee83, targeting main (PR-reported base 02f0bf7b21fd98da0d6f6935d3c49503d6978738). Independently checked current main ef6ef6a1adc4d6b823e043ac8ba56bb3135a2e29: subsequent changes do not alter the host-sensor package or this fix.

The fix is needed: the existing loop retains processed sibling descriptors until the walk returns. readVarsEntry gives each descriptor one owner and closes it before the next sibling, including skip/error paths, while keeping directory handles alive during recursion. Values, source paths, ordering, permission handling, and error wrapping remain compatible. No public API, configuration, schema, or dependency changes. Independent correctness and architecture reviews found no blockers.

Validation in a credential-free, capability-dropped Go 1.27.1 container:

• go test -p 3 ./pkg/hostsensormanager/ -run KernelVariables -count=1 -v: all four tests passed, without skips.
• go vet -p 3 ./pkg/hostsensormanager/: passed.
• Replaced only sensor_kernelvars.go with the target-base implementation and repeated the tests: the descriptor regression failed with too many open files; the other three passed.
• Formatting and diff whitespace checks passed. Initial container attempts ran out of disk space; rerunning with temporary build storage succeeded. Full-suite, race, and live-node checks were not run. Permission/read-error branches were reviewed statically.

History searches across open/closed/merged PRs and issues used kernel-variable, hostsensormanager, proc/sys/kernel, descriptor, RLIMIT_NOFILE, affected-symbol/file, and too-many-open-files terms (100-result cap per query; search indexing is not exhaustive). The original #681 discussion already identified this exact deferred-close problem; this PR addresses it. #998 concerns procfs event-path resolution, and #984 adds sensor exclusions: neither fixes descriptor accumulation. Excluding this sensor avoids collection rather than preserving it. No competing fix or explicit rejection of this approach was found. Related host-identity #982 was deferred (“closing for now”), not a rejection of this fix.

No existing review threads or outstanding reviews were present on final recheck. DCO, GitGuardian, and CodeRabbit passed; test workflows await approval. That unrun CI does not change this approval.