OCPBUGS-66983: Fix race condition in gather_core_dumps pod name retrieval #517
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: anuragthehatter 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 |
Author
|
@sferich888 @chrisdolphy can you help review? |
Author
|
/assign @sferich888 if i may. Thnaks |
sferich888
suggested changes
Dec 8, 2025
Contributor
sferich888
left a comment
sferich888
left a comment
There was a problem hiding this comment.
I think if we merge this it is likely to break some of our 'threading' that backgrounds processes and tracks their completion.
collection-scripts/gather_core_dumps
Outdated
|
|
||
| #Mimic a normal oc call, i.e pause between two successive calls to allow pod to register | ||
| #Wait for the debug pod to be created and extract its name | ||
| sleep 2 |
Contributor
There was a problem hiding this comment.
This shouldn't be a sleep but a retry loop checking if the pod was started; using an exponential backoff (for the check); with a limited number of attempts 3-5; 10 max.
Author
There was a problem hiding this comment.
Agree. I thought to keep it untouched from original code.
Author
There was a problem hiding this comment.
Moved to exponential backoff retry logic
Author
|
cc @TrilokGeer PTAL as well |
anuragthehatter
changed the title
|
@anuragthehatter: An error was encountered adding this pull request to the external tracker bugs for bug OCPBUGS-66983 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details. Full error message.
failed to add remote link: failed to add link: No Link Issue Permission for issue 'OCPBUGS-66983'.: request failed. Please analyze the request body for more details. Status code: 403:
Please contact an administrator to resolve this issue, then request a bug refresh with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Author
|
/jira refresh |
openshift-ci-robot
added
jira/valid-reference
|
@anuragthehatter: This pull request references Jira Issue OCPBUGS-66983, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
TrilokGeer
reviewed
Dec 20, 2025
collection-scripts/gather_core_dumps
Outdated
| #Start Debug pod in background and capture output to get pod name | ||
| local tmpfile=$(mktemp) | ||
| oc debug --to-namespace="default" node/"$1" -- /bin/bash -c 'sleep 300' > "$tmpfile" 2>&1 & | ||
| local debug_pid=$! |
collection-scripts/gather_core_dumps
Outdated
| local max_delay=2.0 # Cap the maximum delay | ||
|
|
||
| while [ -z "$debugPod" ] && [ $attempt -lt $max_attempts ]; do | ||
| debugPod=$(grep -oP "(?<=pod/)[^ ]*" "$tmpfile" 2>/dev/null | head -1) |
There was a problem hiding this comment.
POSIX grep does not gurantee -P option and also, it is affects portability for other variants. For example, the script fails to run BSD/Linux (mac os) for local development. Would it be possible to update to accomodate the posix-compliant options?
collection-scripts/gather_core_dumps
Outdated
| #Copy Core Dumps out of Nodes suppress Stdout | ||
| echo "Copying core dumps on node ""$1""" | ||
| oc cp --loglevel 1 -n "default" "$debugPod":/host/var/lib/systemd/coredump "${CORE_DUMP_PATH}"/"$1"_core_dump > /dev/null 2>&1 && PIDS+=($!) | ||
| oc cp --loglevel 1 -n "default" "$debugPod":/host/var/lib/systemd/coredump "${CORE_DUMP_PATH}"/"$1"_core_dump > /dev/null 2>&1 |
There was a problem hiding this comment.
What happens if the oc fails to copy the core dump?
Author
There was a problem hiding this comment.
Added error handling for oc cp failures
- Now checks exit status and prints warning if copy fails
- Provides visibility into failures instead of silently ignoring them
|
Thanks for the PR @anuragthehatter, dropped in some reviews, hope it helps. Borrowing more help from @praveencodes @shivprakashmuley @swghosh to get attention on the PR on priority. |
5 tasks
Author
|
Addressed above comments and re-ran script on an 18 node cluster @TrilokGeer PTAL again .Thanks |
Contributor
|
@anuragthehatter: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Current logic found failed on one of prow CI run where node indeed had a core dump present but dump was not collected by
oc adm must-gather -- gather_core_dumpscommand underneath a prow chain. It seems we may have missed core dumps collection since long time due to this race issue.Failed logs on prod builds here
Saw a race condition where dump was tried to be copied but debug pod was already removed.
Analysis:
Old approach (main branch):
debugPod=$(oc debug --to-namespace="default" node/"$1" -o jsonpath='{.metadata.name}')
oc debug --to-namespace="default" node/"$1" -- /bin/bash -c 'sleep 300' > /dev/null 2>&1 &
sleep 2
oc wait -n "default" --for=condition=Ready pod/"$debugPod" --timeout=30s
New approach:
Point to note:
FAIL_ON_CORE_DUMP: "true"under prow CI workflows and they seemed to be passing as dumps were never collected resulted in successful prow CI flows executions. The flow logic seems ok but seems like oc adm must-gather never collected dumps if there were any?Reproduced error locally with help of a simulated core dump on an OCP cluster node
using default must-gather image
Ran test image on same cluster