Skip to content

feat(dir): add wait check to dependencies for apiserver - #1747

Open
arpad-csepi wants to merge 2 commits into
mainfrom
fix/helm-chart-healthcheck
Open

feat(dir): add wait check to dependencies for apiserver#1747
arpad-csepi wants to merge 2 commits into
mainfrom
fix/helm-chart-healthcheck

Conversation

@arpad-csepi

@arpad-csepi arpad-csepi commented Jul 4, 2026

Copy link
Copy Markdown
Member

Both the API server and the reconciler previously assumed their dependencies (PostgreSQL and the OCI registry) were already reachable the moment the process started. In Kubernetes environments where pod scheduling is not strictly ordered, this caused crash-loops during initial deploy/rolling restarts: the service failed on first connection attempt and the pod restarted before dependencies came up.

This PR introduces a configurable startup-wait mechanism. Before establishing any connections, each process probes its required dependencies and retries with exponential backoff until they become ready or a configurable timeout is reached. The behavior is opt-in via Helm values now and can be enabled per-dependency but should not broke any existing deployment workflow if opt-out (needs feedback, my preference opt-out).

An alternative solution can be implemented with Kubernetes init-containers but this approach is environment independent.

@arpad-csepi arpad-csepi self-assigned this Jul 4, 2026
@github-actions github-actions Bot added the size/M Denotes a PR that changes 200-999 lines label Jul 4, 2026
@arpad-csepi
arpad-csepi force-pushed the fix/helm-chart-healthcheck branch 3 times, most recently from 8292400 to d0511f0 Compare July 4, 2026 20:40
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.85075% with 27 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
server/startup/dependencies.go 81.1% 9 Missing and 8 partials ⚠️
reconciler/main.go 0.0% 6 Missing ⚠️
server/server.go 0.0% 2 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@arpad-csepi
arpad-csepi marked this pull request as ready for review July 4, 2026 20:51
@arpad-csepi
arpad-csepi requested a review from a team as a code owner July 4, 2026 20:51
@arpad-csepi arpad-csepi changed the title feat(dir): add wait check for dependencies for apiserver feat(dir): add wait check to dependencies for apiserver Jul 5, 2026
Comment thread install/charts/dir/apiserver/values.yaml Outdated
@ramizpolic

Copy link
Copy Markdown
Member

Although I understand the rationale, but is this really an issue? Kubernetes will sometimes deploy services not in order and will cause API server to crash, but it will be restarted until it's deployed and actually ready.
So even if we have initial crashes, this is resolved after some restarts. I do not see this as an issue or problematic behavior since it is eventually consistent behavior.

@arpad-csepi

Copy link
Copy Markdown
Member Author

Although I understand the rationale, but is this really an issue? Kubernetes will sometimes deploy services not in order and will cause API server to crash, but it will be restarted until it's deployed and actually ready. So even if we have initial crashes, this is resolved after some restarts. I do not see this as an issue or problematic behavior since it is eventually consistent behavior.

In this case when Kubernetes deployment is ongoing, it cause unnecessary noise in form of log spamming and restart count increase. It is not ideal specially on version upgrade to have these kind of distractions, but I can understand the eventually consistent concept behind Kubernetes too.

However this solution covers deployment outside of Kubernetes where this consistent concept are not there, like docker, native, etc...

Do you think this "standardized" health check is worth it to have (in every environment) even if not aligned with Kubernetes consistent concept? Do you favor to handle the health checks for every type of environment instead?

@tkircsi

tkircsi commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

I think a "retry" mechanism could be useful, not only during initialization but also while the program is running. That's why I would make it much more generic. The logic/process itself is almost the same for any resource.

  1. input parameters: timeouts, attempts, probeFn
  2. run probeFn
  3. ok/nok (retry)

@arpad-csepi
arpad-csepi force-pushed the fix/helm-chart-healthcheck branch 5 times, most recently from cadbfdc to a2dd6d1 Compare July 24, 2026 11:35
Signed-off-by: Árpád Csepi <csepi.arpad@outlook.com>
@arpad-csepi
arpad-csepi force-pushed the fix/helm-chart-healthcheck branch from a2dd6d1 to 5d69a22 Compare July 24, 2026 14:18
@arpad-csepi

Copy link
Copy Markdown
Member Author

I think a "retry" mechanism could be useful, not only during initialization but also while the program is running. That's why I would make it much more generic. The logic/process itself is almost the same for any resource.

1. input parameters: timeouts, attempts, probeFn

2. run probeFn

3. ok/nok (retry)

@tkircsi
Make sense to want to be more generic solution. Is it a good solution if a healthcheck interface is provided, the probeFn is in the directory code for each module where needed and the rest of parameters are coming from config/helm values?

@ramizpolic
Would this work for you as well?

Signed-off-by: Árpád Csepi <csepi.arpad@outlook.com>
@arpad-csepi
arpad-csepi force-pushed the fix/helm-chart-healthcheck branch from 5d69a22 to 07c88b2 Compare July 24, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 200-999 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants