carverauto/serviceradar

Fork 0

Bug/agent crashing k8s #2696

Merged

mfreeman451 merged 2 commits from refs/pull/2696/head into staging

2026-01-18 09:53:12 +00:00

mfreeman451 commented

2026-01-18 09:45:03 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #2349
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2349
Original created: 2026-01-18T09:45:03Z
Original updated: 2026-01-18T09:53:14Z
Original head: carverauto/serviceradar:bug/agent-crashing-k8s
Original base: staging
Original merged: 2026-01-18T09:53:12Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Bug fix

Description

Made CONFIG_SOURCE=kv idempotent instead of crashing
- Logs deprecation warning and falls back to file config
- Removes fatal error that caused agent pod CrashLoopBackOff
Updated OpenSpec proposal and specs to reflect graceful fallback behavior
Updated task completion status for agent crash resolution

Diagram Walkthrough

flowchart LR
  A["CONFIG_SOURCE=kv set"] -->|Previously| B["Fatal error crash"]
  A -->|Now| C["Log deprecation warning"]
  C --> D["Fall back to file config"]
  D --> E["Agent starts successfully"]

File Walkthrough

Relevant files

Bug fix

config.go

Make KV config source idempotent with fallback

pkg/config/config.go

Removed errKVConfigRemoved error variable declaration
Changed case "kv" handler from returning fatal error to logging
deprecation warning
Set loader = c.defaultLoader to fall back to file-based configuration

+3/-2

Documentation

proposal.md `Update proposal with idempotent KV config behavior` openspec/changes/fix-agent-kv-removal-crash/proposal.md Updated Issue 1 resolution from "force ArgoCD sync" to "made CONFIG_SOURCE=kv idempotent" Changed code path description from present tense to past tense Reordered code changes section to prioritize config.go fix Updated deployment changes to reflect actual implementation	+15/-14
spec.md `Update spec for graceful KV config deprecation` openspec/changes/fix-agent-kv-removal-crash/specs/agent-configuration/spec.md Renamed scenario from "Agent rejects KV configuration source" to "Agent handles deprecated KV configuration source gracefully" Changed expected behavior from failure to graceful fallback with deprecation warning Updated assertions to expect successful agent startup with file config fallback	+4/-3
tasks.md `Mark agent crash fix tasks as completed` openspec/changes/fix-agent-kv-removal-crash/tasks.md Marked section 1 tasks as completed with actual implementation details Updated task 3.2 and 3.3 to reflect module deletion instead of deprecation Updated task 4.1 status to completed	+7/-7

Imported from GitHub pull request. Original GitHub pull request: #2349 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2349 Original created: 2026-01-18T09:45:03Z Original updated: 2026-01-18T09:53:14Z Original head: carverauto/serviceradar:bug/agent-crashing-k8s Original base: staging Original merged: 2026-01-18T09:53:12Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Bug fix ___ ### **Description** - Made CONFIG_SOURCE=kv idempotent instead of crashing - Logs deprecation warning and falls back to file config - Removes fatal error that caused agent pod CrashLoopBackOff - Updated OpenSpec proposal and specs to reflect graceful fallback behavior - Updated task completion status for agent crash resolution ___ ### Diagram Walkthrough ```mermaid flowchart LR A["CONFIG_SOURCE=kv set"] -->|Previously| B["Fatal error crash"] A -->|Now| C["Log deprecation warning"] C --> D["Fall back to file config"] D --> E["Agent starts successfully"] ``` <details><summary><h3>File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>config.go</strong><dd><code>Make KV config source idempotent with fallback</code>                      </dd></summary> <hr> pkg/config/config.go <ul><li>Removed <code>errKVConfigRemoved</code> error variable declaration<br> <li> Changed <code>case "kv"</code> handler from returning fatal error to logging <br>deprecation warning<br> <li> Set <code>loader = c.defaultLoader</code> to fall back to file-based configuration</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2349/files#diff-a3d824da3c42420cd5cbb0a4a2c0e7b5bfddd819652788a0596d195dc6e31fa5">+3/-2</a>      </td> </tr> </table></td></tr><tr><td><strong>Documentation</strong></td><td><table> <tr> <td> <details> <summary><strong>proposal.md</strong><dd><code>Update proposal with idempotent KV config behavior</code>              </dd></summary> <hr> openspec/changes/fix-agent-kv-removal-crash/proposal.md <ul><li>Updated Issue 1 resolution from "force ArgoCD sync" to "made <br>CONFIG_SOURCE=kv idempotent"<br> <li> Changed code path description from present tense to past tense<br> <li> Reordered code changes section to prioritize config.go fix<br> <li> Updated deployment changes to reflect actual implementation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2349/files#diff-1eb96a33b075ff8a9734ead4853f20914d3b9ca35de85b9d5c4ef35d2d97b7b4">+15/-14</a>  </td> </tr> <tr> <td> <details> <summary><strong>spec.md</strong><dd><code>Update spec for graceful KV config deprecation</code>                      </dd></summary> <hr> openspec/changes/fix-agent-kv-removal-crash/specs/agent-configuration/spec.md <ul><li>Renamed scenario from "Agent rejects KV configuration source" to <br>"Agent handles deprecated KV configuration source gracefully"<br> <li> Changed expected behavior from failure to graceful fallback with <br>deprecation warning<br> <li> Updated assertions to expect successful agent startup with file config <br>fallback</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2349/files#diff-56b407d5c0fd7c0d2ef554abf0c359cd033ce04195fcc3af34637b5324d540a2">+4/-3</a>      </td> </tr> <tr> <td> <details> <summary><strong>tasks.md</strong><dd><code>Mark agent crash fix tasks as completed</code>                                    </dd></summary> <hr> openspec/changes/fix-agent-kv-removal-crash/tasks.md <ul><li>Marked section 1 tasks as completed with actual implementation details<br> <li> Updated task 3.2 and 3.3 to reflect module deletion instead of <br>deprecation<br> <li> Updated task 4.1 status to completed</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/2349/files#diff-0253693147c3e954ed11efc9238c012f87c33803523e194e26a94090a1c647ea">+7/-7</a>      </td> </tr> </table></td></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2026-01-18 09:45:31 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2349#issuecomment-3765111365
Original created: 2026-01-18T09:45:31Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2349#issuecomment-3765111365 Original created: 2026-01-18T09:45:31Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </strong></summary> - [ ] Create ticket/issue  </details></td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=6>🟢</td><td> <details><summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2">   </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2026-01-18 09:46:24 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2349#issuecomment-3765112237
Original created: 2026-01-18T09:46:24Z

ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Log deprecation warning only once Use `sync.Once` to ensure the `kv` deprecation warning is logged only once per process lifecycle, preventing log spam on configuration reloads. pkg/config/config.go [239-242] `+var kvDeprecationOnce sync.Once + +// inside loadAndValidateWithSource switch: case "kv": - // KV config source removed - treat as no-op and use file config instead - c.logger.Warn().Msg("CONFIG_SOURCE=kv is deprecated and no longer supported; using file configuration instead") + kvDeprecationOnce.Do(func() { + c.logger.Warn().Msg("CONFIG_SOURCE=kv is deprecated and no longer supported; using file configuration instead") + }) loader = c.defaultLoader` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 5 __ Why: This suggestion correctly identifies a potential for log spam if the configuration is reloaded and proposes a standard Go pattern (`sync.Once`) to prevent it, which improves code quality and log readability.	Low
General	Use constant for KV source Replace the hard-coded string `"kv"` with a `configSourceKV` constant for better maintainability and consistency with other configuration source definitions. pkg/config/config.go [239-242] `-case "kv": +const configSourceKV = "kv" + +// inside loadAndValidateWithSource switch: +case configSourceKV: // KV config source removed - treat as no-op and use file config instead - c.logger.Warn().Msg("CONFIG_SOURCE=kv is deprecated and no longer supported; using file configuration instead") + c.logger.Warn().Msgf("CONFIG_SOURCE=%s is deprecated and no longer supported; using file configuration instead", configSourceKV) loader = c.defaultLoader` `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 4 __ Why: The suggestion improves code maintainability and consistency by replacing a magic string with a constant, which aligns with existing patterns in the file for `configSourceFile` and `configSourceEnv`.	Low
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2349#issuecomment-3765112237 Original created: 2026-01-18T09:46:24Z --- <pre>ⓘ Your approaching your monthly quota for Qodo. <a href="https://www.qodo.ai/pricing">Upgrade your plan</a></pre> ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=2>General</td> <td> <details><summary>Log deprecation warning only once</summary> ___ **Use <code>sync.Once</code> to ensure the <code>kv</code> deprecation warning is logged only once per <br>process lifecycle, preventing log spam on configuration reloads.** [pkg/config/config.go [239-242]](https://github.com/carverauto/serviceradar/pull/2349/files#diff-a3d824da3c42420cd5cbb0a4a2c0e7b5bfddd819652788a0596d195dc6e31fa5R239-R242) ```diff +var kvDeprecationOnce sync.Once + +// inside loadAndValidateWithSource switch: case "kv": - // KV config source removed - treat as no-op and use file config instead - c.logger.Warn().Msg("CONFIG_SOURCE=kv is deprecated and no longer supported; using file configuration instead") + kvDeprecationOnce.Do(func() { + c.logger.Warn().Msg("CONFIG_SOURCE=kv is deprecated and no longer supported; using file configuration instead") + }) loader = c.defaultLoader ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: This suggestion correctly identifies a potential for log spam if the configuration is reloaded and proposes a standard Go pattern (`sync.Once`) to prevent it, which improves code quality and log readability. </details></details></td><td align=center>Low </td></tr><tr><td> <details><summary>Use constant for KV source</summary> ___ **Replace the hard-coded string <code>"kv"</code> with a <code>configSourceKV</code> constant for better <br>maintainability and consistency with other configuration source definitions.** [pkg/config/config.go [239-242]](https://github.com/carverauto/serviceradar/pull/2349/files#diff-a3d824da3c42420cd5cbb0a4a2c0e7b5bfddd819652788a0596d195dc6e31fa5R239-R242) ```diff -case "kv": +const configSourceKV = "kv" + +// inside loadAndValidateWithSource switch: +case configSourceKV: // KV config source removed - treat as no-op and use file config instead - c.logger.Warn().Msg("CONFIG_SOURCE=kv is deprecated and no longer supported; using file configuration instead") + c.logger.Warn().Msgf("CONFIG_SOURCE=%s is deprecated and no longer supported; using file configuration instead", configSourceKV) loader = c.defaultLoader ``` `[To ensure code accuracy, apply this suggestion manually]` <details><summary>Suggestion importance[1-10]: 4</summary> __ Why: The suggestion improves code maintainability and consistency by replacing a magic string with a constant, which aligns with existing patterns in the file for `configSourceFile` and `configSourceEnv`. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>