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

portblock: create tickle_dir if it doesnt exist #845

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

Conversation

oalbrigt
Copy link
Contributor

No description provided.

@krig
Copy link
Contributor

krig commented Sep 16, 2016

Hmm, wouldn't it be better to check if the tickle_dir exists in the validate-all action?

Creating the directory if it doesn't exist masks typos and configuration errors, where someone mistyped the directory they meant or something like that. Checking in validate-all and setting exit-reason would be clearer, I think...

@oalbrigt oalbrigt force-pushed the portblock-create-tickle_dir branch from 60690fa to 8ac0598 Compare September 19, 2016 12:57
@oalbrigt
Copy link
Contributor Author

Updated to create it in the validate-all action.

@@ -466,8 +466,7 @@ IptablesValidateAll()
exit $OCF_ERR_CONFIGURED
fi
if [ ! -d "$OCF_RESKEY_tickle_dir" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also get rid of this if statement. mkdir -p will just exit if the directory exists.

@oalbrigt oalbrigt force-pushed the portblock-create-tickle_dir branch from 8ac0598 to 408b178 Compare November 8, 2016 09:44
@dmuhamedagic
Copy link
Contributor

Kristoffer's note is also a valid concern and, even though we do have occasional RA that creates a directory automatically, in this case I lean more to reporting an error instead (as it is now). This directory must anyway be either shared or synced, hence it is necessary to create it on only one node.

@oalbrigt
Copy link
Contributor Author

I have updated the patch based on his concern.

I think it's an easy fix to avoid annoyances when you forget to create the directory.

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.

3 participants