Skip to content

[MOSIP-43648] Configure graceful pod termination with lifecycle hooks#229

Merged
dhanendra06 merged 5 commits into
mosip:developfrom
Ivanmeneges:develop
Nov 16, 2025
Merged

[MOSIP-43648] Configure graceful pod termination with lifecycle hooks#229
dhanendra06 merged 5 commits into
mosip:developfrom
Ivanmeneges:develop

Conversation

@Ivanmeneges

@Ivanmeneges Ivanmeneges commented Nov 16, 2025

Copy link
Copy Markdown

Summary by CodeRabbit

  • Chores
    • Optimized resource allocation with increased CPU and memory limits.
    • Enhanced pod graceful shutdown with improved lifecycle management.
    • Updated container image dependency to a newer version.
    • Adjusted readiness probe timing for better health checks.

Signed-off-by: Ivanmeneges <ivan.anil016@gmail.com>
Signed-off-by: Ivanmeneges <ivan.anil016@gmail.com>
Signed-off-by: Ivanmeneges <ivan.anil016@gmail.com>
@coderabbitai

coderabbitai Bot commented Nov 16, 2025

Copy link
Copy Markdown

Warning

Rate limit exceeded

@Ivanmeneges has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 57 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 4b3e463 and f8e1baa.

📒 Files selected for processing (1)
  • kernel/kernel-auditmanager-service/pom.xml (1 hunks)

Walkthrough

This PR updates the auditmanager Helm chart with multiple configuration refinements: corrects a .gitignore entry capitalization, adds pod termination grace period configuration to the Deployment template, and updates readiness probe timings, resource limits, Java memory options, lifecycle hooks, and container image references in values.yaml.

Changes

Cohort / File(s) Summary
Gitignore Configuration
helm/auditmanager/.gitignore
Corrected lockfile entry capitalization from Charts.lock to Chart.lock
Deployment Template
helm/auditmanager/templates/deployment.yaml
Added terminationGracePeriodSeconds field to pod spec, sourcing from Values with a default of 60 seconds
Helm Values
helm/auditmanager/values.yaml
Updated readiness probe timings (initialDelaySeconds: 0→30, periodSeconds: 10→15, timeoutSeconds: 5→10), increased resource limits (cpu: 1000m→2000m, memory: 2500Mi→8000Mi), adjusted Java heap options (1750M→3050M), added preStop lifecycle hook with 30-second sleep, added terminationGracePeriodSeconds (60), and changed volumePermissions image from bitnami/bitnami-shell:10 to mosipid/os-shell:12-debian-12-r46

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Straightforward configuration updates without complex logic changes
  • Primary focus: verify resource and timing adjustments are intentional and well-suited for production workloads
  • Note: Image repository change from bitnami to mosipid/os-shell should be verified for availability and compatibility

Poem

🐰 With whiskers twitching and tail held high,
We've tuned the timings way up high,
Grace periods and probes aligned,
Resources doubled—oh so kind!
A lockfile renamed, clean and neat,
The chart's now ready—victory sweet! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding graceful pod termination configuration with lifecycle hooks, which is reflected in the values.yaml (lifecycleHooks preStop added) and deployment.yaml (terminationGracePeriodSeconds added) changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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 and usage tips.

Signed-off-by: Ivanmeneges <ivan.anil016@gmail.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
helm/auditmanager/values.yaml (1)

227-227: Minor: Fix typo in comment.

Line 227 has a typo: "perios" should be "periods".

-## Termination grace perios : the maximum amount of time (in seconds) Kubernetes will wait for a container to gracefully shut down
+## Termination grace period: the maximum amount of time (in seconds) Kubernetes will wait for a container to gracefully shut down
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 85bb08a and 4b3e463.

📒 Files selected for processing (3)
  • helm/auditmanager/.gitignore (1 hunks)
  • helm/auditmanager/templates/deployment.yaml (1 hunks)
  • helm/auditmanager/values.yaml (4 hunks)
🔇 Additional comments (6)
helm/auditmanager/.gitignore (1)

2-2: LGTM—correct Helm lock file name.

The capitalization fix from "Charts.lock" to "Chart.lock" aligns with Helm's standard lock file naming convention.

helm/auditmanager/templates/deployment.yaml (1)

37-37: Graceful termination config correctly templated.

The terminationGracePeriodSeconds setting enables a configurable grace period for pods to shut down gracefully, preventing forcible termination after a timeout. The template correctly references the values with a sensible default that matches the values.yaml configuration.

helm/auditmanager/values.yaml (4)

102-104: Readiness probe timings adjusted for graceful termination.

The increased initialDelaySeconds (0→30), periodSeconds (10→15), and timeoutSeconds (5→10) are reasonable adjustments given the significant resource limit increases. These longer tolerances help the service stay marked as ready during graceful shutdown scenarios.


129-130: Verify heap-to-memory ratio is intentional.

The container memory limit increased 3.2× (2500Mi→8000Mi) while Java heap increased 1.74× (1750M→3050M). This results in a ~38% heap-to-total-memory ratio. Best practice recommends keeping the JVM heap to container memory ratio no higher than 0.5 (50%), as higher ratios are known to be unstable and may cause unexpected restarts due to JVM off-heap usage. The current ratio is within safe bounds (~4950Mi available for non-heap overhead), but please confirm this resource scaling aligns with performance testing and actual application needs.

Also applies to: 138-138


219-228: LGTM—lifecycle hooks and grace period well-coordinated.

The preStop hook executes at the container level before SIGTERM is sent, allowing the deregistration process to complete before the container begins shutdown. The 30-second sleep in preStop combined with 60-second terminationGracePeriodSeconds provides appropriate time windows: 30s for endpoint removal propagation, then 30s for application shutdown. Research suggests 5–10 seconds is often sufficient for most cases, so 30 seconds is conservative and well-suited for a Java service.


357-358: Verify volumePermissions image availability and maintenance.

The image reference changed from the community-maintained bitnami/bitnami-shell:10 to an internal registry image mosipid/os-shell:12-debian-12-r46. Confirm that:

  • The mosipid registry image is available and accessible to the cluster
  • The image is actively maintained and receives security updates
  • The Debian 12 base is compatible with existing initialization scripts (if any)
  • Image pull credentials are properly configured (if the registry requires authentication)

Signed-off-by: Ivanmeneges <ivan.anil016@gmail.com>
@dhanendra06 dhanendra06 merged commit 688cedf into mosip:develop Nov 16, 2025
9 checks passed
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