-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix: support retries for redirecting check-mount script execution res… #5297
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
Fix: support retries for redirecting check-mount script execution res… #5297
Conversation
…ult to stdout. Signed-off-by: 玖宇 <guotongyu.gty@alibaba-inc.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modifies the postStartHook script generation to improve reliability of output redirection by implementing a retry mechanism. The changes move output redirection from the command invocation level into the script itself with retry logic.
Key Changes:
- Added a
redirect_output_with_retry()function that attempts output redirection up to 5 times with 1-second delays between attempts - Removed the
>> /proc/1/fd/1redirection from the command string, as redirection now happens within the script
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/application/inject/fuse/poststart/check_fuse_default.go | Added retry logic for output redirection in the privileged sidecar script and removed redirection from command invocation |
| pkg/application/inject/fuse/poststart/check_fuse_app.go | Added retry logic for output redirection in the app mount check script and removed redirection from command invocation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4b34e04 to
d45d173
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5297 +/- ##
==========================================
+ Coverage 55.77% 57.24% +1.46%
==========================================
Files 413 412 -1
Lines 28087 27465 -622
==========================================
+ Hits 15666 15722 +56
+ Misses 11006 10322 -684
- Partials 1415 1421 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cheyang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve



In the serverless environment, Fluid injects Fuse as a sidecar into the application pod and sets a check-mount.sh script as a postStart hook to check the mount point status inside the sidecar. For easier troubleshooting, the execution output of check-mount.sh is redirected to the container’s standard output, as shown below.
However, because postStart and the container process startup are not serialized, the redirection may fail, causing the Fuse container to crash and restart. This PR moves the redirection into the check-mount.sh script itself and adds a retry mechanism.