Skip to content

OLS-3221 Reduce app server liveness probe failureThreshold from 15 to 3.#1757

Open
sriroopar wants to merge 1 commit into
openshift:mainfrom
sriroopar:postgres-auto-recovery
Open

OLS-3221 Reduce app server liveness probe failureThreshold from 15 to 3.#1757
sriroopar wants to merge 1 commit into
openshift:mainfrom
sriroopar:postgres-auto-recovery

Conversation

@sriroopar

@sriroopar sriroopar commented Jun 24, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Reduce app server liveness probe failureThreshold from 15 to 3, giving the pod 90 seconds to self-heal via the background
health-check loop before Kubernetes restarts it

  • Add test assertions for failureThreshold: 3 and periodSeconds: 30 to document the 90-second recovery contract

Description

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up dependent library

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Bug Fixes
    • Updated the app server container liveness probe to mark pods unhealthy sooner by setting FailureThreshold to 3 (from 15).
  • Tests
    • Expanded deployment tests to validate the liveness probe timing and failure limits (including PeriodSeconds 30 and FailureThreshold 3).

@coderabbitai

coderabbitai Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: dff1e4a1-cbd4-4559-b005-ddfeea623ac9

📥 Commits

Reviewing files that changed from the base of the PR and between a04e330 and 4b5b8de.

📒 Files selected for processing (3)
  • internal/controller/appserver/assets_test.go
  • internal/controller/appserver/deployment.go
  • internal/controller/appserver/deployment_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • internal/controller/appserver/assets_test.go
  • internal/controller/appserver/deployment.go
📝 Walkthrough

Walkthrough

The app server deployment lowers the liveness probe failure threshold from 15 to 3. Related tests assert the updated failure threshold and period values.

Changes

App server liveness probe update

Layer / File(s) Summary
Probe settings and validation
internal/controller/appserver/deployment.go, internal/controller/appserver/deployment_test.go, internal/controller/appserver/assets_test.go		 The deployment sets FailureThreshold to 3, while tests assert FailureThreshold: 3 and PeriodSeconds: 30.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: blublinsky, xrajesh

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: lowering the app server liveness probe failure threshold from 15 to 3.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@openshift-ci openshift-ci Bot requested review from blublinsky and xrajesh June 24, 2026 18:24
@sriroopar sriroopar force-pushed the postgres-auto-recovery branch from a04e330 to ac7df2d Compare June 24, 2026 19:26
@xrajesh

xrajesh commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 1, 2026
@openshift-ci

openshift-ci Bot commented Jul 1, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xrajesh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 1, 2026
@openshift-merge-bot

openshift-merge-bot Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 7ed4504 and 2 for PR HEAD ac7df2d in total

@openshift-merge-bot

openshift-merge-bot Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD a1bbf9c and 1 for PR HEAD ac7df2d in total

@openshift-merge-bot

openshift-merge-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 9184fd8 and 0 for PR HEAD ac7df2d in total

@openshift-merge-bot

openshift-merge-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

/hold

Revision ac7df2d was retested 3 times: holding

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 6, 2026
@sriroopar sriroopar force-pushed the postgres-auto-recovery branch from ac7df2d to 4e019a0 Compare July 13, 2026 16:30
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2026
@openshift-ci

openshift-ci Bot commented Jul 13, 2026

Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@sriroopar @claude
OLS-3221 Reduce app server liveness probe failureThreshold to 3
4b5b8de 
Lowers the liveness probe failureThreshold from 15 to 3, giving the pod
90 seconds to self-heal before Kubernetes restarts it. This enables
auto-recovery after PostgreSQL restart once the service-side liveness
endpoint checks DB health.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sriroopar sriroopar force-pushed the postgres-auto-recovery branch from 4e019a0 to 4b5b8de Compare July 14, 2026 16:03
@openshift-ci

openshift-ci Bot commented Jul 14, 2026

Copy link
Copy Markdown

@sriroopar: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blublinsky blublinsky Awaiting requested review from blublinsky
@xrajesh xrajesh Awaiting requested review from xrajesh

Assignees

@xrajesh xrajesh

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sriroopar @xrajesh