Skip to content

feat: add livenessProbe to Package Manager#137

Merged
stevenolen merged 3 commits into
mainfrom
ppm-liveness-probe
May 12, 2026
Merged

feat: add livenessProbe to Package Manager#137
stevenolen merged 3 commits into
mainfrom
ppm-liveness-probe

Conversation

@stevenolen
Copy link
Copy Markdown
Contributor

Summary

  • Add HTTP /__ping__ livenessProbe to the Package Manager container, alongside the existing readinessProbe.
  • Values match upstream PPM Helm chart defaults: initialDelay=10s, period=5s, timeout=2s, failureThreshold=10. Uses the named http port to stay consistent with the readinessProbe.

Test plan

  • go build ./...
  • Package Manager unit tests pass
  • Deploy via adhoc image to a test cluster and confirm probe shows healthy

@claude
Copy link
Copy Markdown

claude Bot commented May 12, 2026

Claude finished @stevenolen's task —— View job


Review Complete

  • Read review guidelines
  • Analyze diff and surrounding code
  • Submit review

Submitted a non-blocking review. The change looks good — follows existing patterns and uses appropriate upstream defaults. Left two minor suggestions around field consistency and test coverage.

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — straightforward addition that mirrors the existing readinessProbe structure and uses sensible values from upstream defaults.

Minor observations:

  1. Consistency with readinessProbe fields (package_manager.go:588-599): The readinessProbe explicitly sets SuccessThreshold: 1 and TerminationGracePeriodSeconds: nil. While those are defaults for liveness probes too, including them would keep the two definitions visually symmetric and make it clear nothing was accidentally omitted. Not a blocker.

  2. No unit test coverage for the new probe: The existing package_manager_controller_test.go doesn't assert on probes. The flightdeck controller tests do verify their livenessProbe (flightdeck_test.go:197-200). Consider adding a similar assertion for PPM to catch accidental regressions. Also not a blocker for this PR given the test plan includes manual validation.

Overall: clean change, follows existing patterns, no correctness or security concerns.

@stevenolen stevenolen marked this pull request as ready for review May 12, 2026 19:17
@stevenolen stevenolen added this pull request to the merge queue May 12, 2026
Merged via the queue into main with commit ac44f4c May 12, 2026
8 checks passed
@stevenolen stevenolen deleted the ppm-liveness-probe branch May 12, 2026 20:20
ian-flores pushed a commit that referenced this pull request May 12, 2026
# [1.25.0](v1.24.1...v1.25.0) (2026-05-12)

### Features

* add livenessProbe to Package Manager ([#137](#137)) ([ac44f4c](ac44f4c))
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