Skip to content

Glusterfs #249

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

Merged
merged 31 commits into from
Apr 29, 2020
Merged

Glusterfs #249

merged 31 commits into from
Apr 29, 2020

Conversation

garvct
Copy link
Collaborator

@garvct garvct commented Apr 28, 2020

Initial support for glusterfs in azurehpc.

Copy link
Collaborator

@xpillons xpillons left a comment

Choose a reason for hiding this comment

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

I wouldn't named the example directory with the SKU type as it may change in the future. maybe something like glusterfs_ephemeral would be better ?

mkdir -p /mnt/brick1/$GLUSTERFS_VOL_NAME

if [ "$PSSH_NODENUM" = "0" ]; then
for host in `cat ~hpcadmin/azhpc_install_config/hostlists/$HOSTLIST`
Copy link
Collaborator

Choose a reason for hiding this comment

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

config file name is hardcoded as well ad the username, it's fine right now, but this can easily be broken by using another config file name or user

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Xavier, I changed the example dir name to be glusterfs_ephemeral.


GLUSTERFS_VOL_NAME=gv0

mkdir -p /mnt/brick1/$GLUSTERFS_VOL_NAME
Copy link
Collaborator

Choose a reason for hiding this comment

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

/mnt/brick1 is hardcoded, can you add it as a param and default to that value ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added BRICK_MOUNT_PT variable with default /mnt/brick1

systemctl status glusterd

if [ "$PSSH_NODENUM" = "0" ]; then
for host in `cat ~hpcadmin/azhpc_install_config/hostlists/$HOSTLIST`
Copy link
Collaborator

Choose a reason for hiding this comment

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

username and config file name hardcoded, maybe the hostlist full path should be provided instead ? @edwardsp what do you think ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

the scripts are run from inside the temp directory - you could just use "cat hostlists/$HOSTLIST". But, better still, you could just set the arg in the config as "$(<hostlists/glusterfs)" and then you would get it as the first arg. You would only then need to iterate as follows:
for host in $HOSTLIST; do ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Paul and Xavier, I made the changes you suggested, looks cleaner now.

@garvct garvct requested a review from xpillons April 28, 2020 17:51
@garvct garvct merged commit c9c640e into master Apr 29, 2020
@garvct garvct deleted the glusterfs branch April 29, 2020 16:43
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