Make check-db-ready initContainers optional #26284
Labels
Comments
garethbrickman
added
the
deployment: k8s
This was referenced Dec 9, 2024
gibsondan
pushed a commit
that referenced
this issue
Dec 14, 2024
…#26343) ## Summary & Motivation Address #26311 (partially) and #26284 (duplicate issues) tl;dr: the `check-db-ready` container can be part of a race condition with other initContainers that are responsible for enabling the DB connection. Externally managed DBs (i.e. not the postgres container optionally included in this chart) can be validated before Helm installation so this step is not strictly required. ## How I Tested These Changes 1. Wrote some tests 2. Installed the chart to minikube with ``` helm install dagster . --set dagsterWebserver.image.tag=latest --set dagsterDaemon.image.tag=latest --set pipelineRun.image.tag=latest --set dagsterWebserver.checkDbReadyInitContainer=true ``` And observed the initContainers present. Then ran it again with `false` ``` helm install dagster . --set dagsterWebserver.image.tag=latest --set dagsterDaemon.image.tag=latest --set pipelineRun.image.tag=latest --set dagsterWebserver.checkDbReadyInitContainer=false ``` And observed the initContainers absent. ## Changelog - Added flag `checkDbReadyInitContainer` to optionally disable db check initContainer. --------- Co-authored-by: Tyler Eason <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What's the use case?
In my k8s infra,
istiocontainers can be applied as sidecars and initContainers. One of the initContainer functions would allow me to connect to the database instance backing Dagster. However, because theistioinitContainer is appended to the list, it occurs aftercheck-db-ready. It results in a cyclic dependency where the DB check can't succeed becauseistiohasn't set up the connection, and theistiocontainer can't succeed because it's not first in line.Ideas of implementation
Add a flag to the Helm values files which disables
check-db-readyinit containers.Additional information
No response
Message from the maintainers
Impacted by this issue? Give it a 👍! We factor engagement into prioritization.
The text was updated successfully, but these errors were encountered: