Skip to content

fix(helm): add backstop livenessProbe to DHCP config-sync sidecar#65

Merged
dmathur0 merged 2 commits into
mainfrom
davesh/dhcp-config-sync-liveness-backstop
Jun 25, 2026
Merged

fix(helm): add backstop livenessProbe to DHCP config-sync sidecar#65
dmathur0 merged 2 commits into
mainfrom
davesh/dhcp-config-sync-liveness-backstop

Conversation

@dmathur0

@dmathur0 dmathur0 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Description

Split out of #23 so this low-risk, Helm-only change can ship independently while the KEA bootstrap-drift detection/re-apply application logic in #23 waits for QA.

Adds a conservative backstop livenessProbe to the config-sync-v4 container in deploy/helm/templates/dhcp.yaml. If the in-loop KEA divergence check ever wedges (asyncio loop stalls, or the kea control socket stays unresponsive long enough that get_config() keeps raising), kubelet recycles the sidecar. On restart the container re-runs the unconditional "Run once" set_config() and heals the deadlock — even if the in-loop check itself is broken.

  • Probes the healthcheck endpoint (/healthcheck on port 9090) in the same pod/network namespace, matching the pattern used by the kea container's probe.
  • Thresholds are deliberately looser than the other probes (failureThreshold * periodSeconds = 300s of failure) so the in-loop recovery (~10s) wins under normal conditions and we only recycle the sidecar when the loop itself is the problem.

This contains no application code — it's purely the Kubernetes safety net, so it carries minimal risk and does not need to wait on QA of the drift-recovery logic.

Validation

Checklist

  • I am familiar with the contributing guidelines in CONTRIBUTING.md.
  • Commits are signed off for DCO compliance.
  • New or existing tests cover these changes, or the PR explains why tests are not needed. (Helm-only probe addition; covered by Kind integration rendering.)
  • Documentation is updated for user-facing behavior changes.
  • Generated artifacts are updated when applicable.

Summary by CodeRabbit

  • Bug Fixes
    • Added an additional health check to improve recovery for DHCP config synchronization.
    • Helps the service restart more reliably if the sync process becomes unresponsive.

Adds a conservative livenessProbe to the config-sync-v4 container in
deploy/helm/templates/dhcp.yaml so kubelet can recycle the sidecar if the
in-loop KEA divergence check ever wedges (asyncio loop stalls or the kea
control socket stays unresponsive). On restart the container re-runs the
unconditional "Run once" set_config() and heals the deadlock.

Thresholds are intentionally looser than the other probes
(failureThreshold * periodSeconds = 300s) so the in-loop recovery wins
under normal conditions and we only recycle when the loop itself is broken.

Split out of #23 so this low-risk, Helm-only safety net can ship now while
the KEA bootstrap-drift detection/re-apply application logic waits for QA.

Signed-off-by: Davesh Mathur <daveshm@nvidia.com>
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds an HTTP liveness probe to the config-sync-v4 container in the DHCP Helm template, checking /healthcheck on port 9090 with separate timing and failure thresholds.

Changes

DHCP config-sync backstop probe

Layer / File(s) Summary
config-sync-v4 liveness probe
deploy/helm/templates/dhcp.yaml
Adds an HTTP GET livenessProbe for config-sync-v4 against /healthcheck on port 9090 with its own delay, period, timeout, and failure settings.

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 I hop by the helm, so neat and bright,
/healthcheck now watches through the night.
If config-sync naps, the probe will say “boo,”
then back to startup the pod will chew.
Ears up, wheels turn, the cluster sings too.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main Helm change: adding a backstop liveness probe to the DHCP config-sync sidecar.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch davesh/dhcp-config-sync-liveness-backstop

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

Comment thread deploy/helm/templates/dhcp.yaml
@dmathur0 dmathur0 merged commit 3730d88 into main Jun 25, 2026
14 of 15 checks passed
@dmathur0 dmathur0 deleted the davesh/dhcp-config-sync-liveness-backstop branch June 25, 2026 21:48
spidercensus pushed a commit that referenced this pull request Jun 25, 2026
Adds a conservative livenessProbe to the config-sync-v4 container in
deploy/helm/templates/dhcp.yaml so kubelet can recycle the sidecar if the
in-loop KEA divergence check ever wedges (asyncio loop stalls or the kea
control socket stays unresponsive). On restart the container re-runs the
unconditional "Run once" set_config() and heals the deadlock.

Thresholds are intentionally looser than the other probes
(failureThreshold * periodSeconds = 300s) so the in-loop recovery wins
under normal conditions and we only recycle when the loop itself is broken.

Split out of #23 so this low-risk, Helm-only safety net can ship now while
the KEA bootstrap-drift detection/re-apply application logic waits for QA.

Signed-off-by: Davesh Mathur <daveshm@nvidia.com>
dmathur0 added a commit that referenced this pull request Jun 26, 2026
The config-sync-v4 backstop livenessProbe was split out of this PR and merged
to main independently in #65. Remove it here so this branch carries only the
KEA bootstrap-drift detection / re-apply application logic (and tests), which
is being held for QA, and the two changes no longer overlap.

Signed-off-by: Davesh Mathur <daveshm@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants