Fix gather_sos debug pod race condition and add timeouts - #153
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
80dbaaf to
15f1ebd
Compare
| local dump="${3:-openstack_databases}" | ||
| # skip the service db dump if it does not exist on the current $dbpod | ||
| if ! /usr/bin/oc exec -n "$ns" "$dbpod" -- mysql -u root -e "USE $dbname;" 2>/dev/null; then | ||
| if ! run_with_timeout "$CMD_TIMEOUT" /usr/bin/oc exec -n "$ns" "$dbpod" -- mysql -u root -e "USE $dbname;" 2>/dev/null; then |
There was a problem hiding this comment.
would it be good to retry of we hit a CMD_TIMEOUT?
fmount
left a comment
There was a problem hiding this comment.
+1 from me about wrapping with a global timeout and a CMD_TIMEOUT for sos which usually takes a lot of time.
We could improve with a retry mechanism but let's see how this patch behaves in CI first.
| # Wait until we know for sure the debug pod is gone, this is to try to | ||
| # workaround the following failure: | ||
| # Error from server (BadRequest): container "container-00" in pod debug is waiting to start: ContainerCreating | ||
| while oc get pod "${node}-debug" 1>/dev/null 2>&1 1>/dev/; do |
There was a problem hiding this comment.
+1 thanks, this definitely makes sense
| export RABBITMQ_SELECTOR="app.kubernetes.io/component=rabbitmq" | ||
| # Timeouts (seconds) to prevent commands from hanging indefinitely | ||
| export CMD_TIMEOUT=${CMD_TIMEOUT:-120} | ||
| export SOS_CMD_TIMEOUT=${SOS_CMD_TIMEOUT:-600} |
There was a problem hiding this comment.
do we have some metric from customer setups on how much time it takes to collect sos reports on both cases i.e ocp workers and edpm nodes, just wanted to ensure this default covers most of the deployments.
There was a problem hiding this comment.
A fair point, for sure. I do know some customers complained about sos taking a very long time for large clouds.
There was a problem hiding this comment.
I'm not sure about that, but I think in any case we need to document these variables in the main README [1].
sos from edpm nodes are disabled by default, so we need to update the example and explicitly mention the timeout, while for ocp master/worker nodes it runs by default, and we could use a unijob to estimate a value that can be used. I see the most of the time is spent during tar xJvf of the collected gathering, and for that part we have [2] that we should consider to speedup the process.
[1] https://github.com/openstack-k8s-operators/openstack-must-gather#customize-gathered-data
[2] https://redhat.atlassian.net/browse/OSPRH-18808
There was a problem hiding this comment.
@fmount actually CI jobs don't provide real value that's why asked for Customer metrics as that's where it will reflect. Yes document is fine but we should also ensure sane defaults, and avoid collecting/uploading must gathers multiple times just to know that data is missing
There was a problem hiding this comment.
I added the missing env vars to the README.
The while loop in gather_sos that waits for a previous debug pod to terminate had an erroneous redirect (1>/dev/;) that caused the loop to never execute, creating a race condition where the next oc debug call could fail with "container in pod debug is waiting to start: ContainerCreating". More critically, no oc debug/rsh/exec/cp, SSH, or openstack CLI command anywhere in the must-gather had a timeout. A single hanging command (e.g. stalled image pull, unresponsive pod, unreachable service) would block the entire must-gather until the external oc adm must-gather --timeout killed it — collecting nothing. Add two configurable timeout tiers via environment variables: - CMD_TIMEOUT (default 120s): oc rsh, oc exec, oc cp, openstack CLI - SOS_CMD_TIMEOUT (default 600s): SOS report oc debug and SSH ops Wrap all remote-execution commands with run_with_timeout in gather_sos, gather_edpm_sos, gather_trigger_gmr, gather_services_status, and gather_db. The wrapper logs the timed-out command to stderr for diagnostic visibility. Fast local commands (oc get, oc describe) are unchanged. Co-Authored-By: Andrew Bays <abays@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
15f1ebd to
a13acb8
Compare
The while loop in
gather_sosthat waits for a previous debug pod to terminate had an erroneous redirect(1>/dev/;)that caused the loop to never execute, creating a race condition where the nextoc debugcall could fail with "container in pod debug is waiting to start: ContainerCreating".More critically, no
oc debug/rsh/exec/cp, SSH, oropenstackCLI command anywhere in the must-gather had a timeout. A single hanging command (e.g. stalled image pull, unresponsive pod, unreachable service) would block the entire must-gather until the externaloc adm must-gather --timeoutkilled it — collecting nothing.Add two configurable timeout tiers via environment variables:
CMD_TIMEOUT(default 120s):oc rsh,oc exec,oc cp,openstackCLISOS_CMD_TIMEOUT(default 600s): SOS reportoc debugand SSH opsWrap all remote-execution commands with
timeoutingather_sos,gather_edpm_sos,gather_trigger_gmr,gather_services_status, andgather_db. Fast local commands (oc get,oc describe) are unchanged.Co-Authored-By: Andrew Bays abays@redhat.com
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com