Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add readiness probe #1047

Merged
merged 4 commits into from
Sep 15, 2023
Merged

Add readiness probe #1047

merged 4 commits into from
Sep 15, 2023

Conversation

sjberman
Copy link
Contributor

@sjberman sjberman commented Sep 8, 2023

Problem: The NKG Pod would report Ready as soon as it started, which could lead to traffic being sent too soon before nginx was actually configured.

Solution: Add a readiness probe that will report Ready once the controller has written its first config to nginx (or has nothing to do on startup).

Also changed the metrics "disable" helm flag to "enable" to be consistent and easier to read. (This is a pre-1.0 breaking change).

Testing: Manually verified that an upgrade has no downtime for connections.

Closes #542

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@sjberman sjberman requested a review from a team as a code owner September 8, 2023 18:27
@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request labels Sep 8, 2023
deploy/helm-chart/README.md Show resolved Hide resolved
cmd/gateway/commands.go Show resolved Hide resolved
internal/mode/static/health_test.go Outdated Show resolved Hide resolved
cmd/gateway/commands.go Show resolved Hide resolved
cmd/gateway/commands.go Show resolved Hide resolved
deploy/helm-chart/README.md Outdated Show resolved Hide resolved
internal/mode/static/handler.go Outdated Show resolved Hide resolved
internal/mode/static/health.go Outdated Show resolved Hide resolved
internal/mode/static/health.go Outdated Show resolved Hide resolved
internal/mode/static/handler.go Outdated Show resolved Hide resolved
Problem: The NKG Pod would report Ready as soon as it started, which could lead to traffic being sent too soon before nginx was actually configured.

Solution: Add a readiness probe that will report Ready once the controller has written its first config to nginx (or has nothing to do on startup).

Also changed the metrics "disable" helm flag to "enable" to be consistent and easier to read.
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

👍

@sjberman sjberman merged commit 8b2900e into nginxinc:main Sep 15, 2023
23 checks passed
@sjberman sjberman deleted the feat/health-probes branch September 15, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Ecosystem: Support liveness and readiness probes
5 participants