Skip to content
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

Resolves #117 Attempts to retry failed mounts of bricks #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gonoph
Copy link

@gonoph gonoph commented Dec 24, 2018

Resolves Issue #117

After the update in commit 2e2e284, gluster-setup.sh no longer attempts to mount failed bricks.

  • checks for the return codes 1, 2, and 4 which is fatal
  • logs successful mount
  • if unsuccessful, allows script to continue to attempt the retry logic below.

According to the man page RETURN CODE 1 is incorrect permissions or invocation (bad parameters). The previous commit attempted to only test for that error, which if continuing in the script logic allows the glusterd pod/container to attempt to identify the failed bricks, and try various tricks to mount them.

This change tests for RETURN CODE 1, 2, and 4 - which should cover most fatal errors, and still allow the script to attempt to mount failed bricks using the logic below that test.

Also, the test at the end:

        if [ "$((cat $GLUSTERFS_LOG_CONT_DIR/failed_bricks | wc -l) )" -gt 1 ]

works better than the wc -l $GLUSTERFS_LOG_CONT_DIR/failed_bricks it replaces, as the wc command appends the filename, which breaks the number test -gt. Therefore, the cat | wc syntax works better as wc does not print the filename when used on stdin, and thus it does not break the -gt test.

After the update in commit 2e2e284, gluster-setup.sh no longer attempts to mount failed bricks.

* checks for the return codes 1, 2, and 4 which is fatal
* logs successful mount
* if unsuccessful, allows script to continue to attempt the retry logic below.
@nixpanic
Copy link
Member

Can you have a look at #114? It should address the same problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants