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

Use env. variable GLUSTER_BLOCK_ENABLED to enable/disable gluster-block-service #122

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

Conversation

nixpanic
Copy link
Member

@nixpanic nixpanic commented Jan 15, 2019

Enable gluster-block services if GLUSTER_BLOCK_ENABLED is "1" and disable otherwise.

By default, GLUSTER_BLOCK_ENABLED is set as "1".

This change was recently reverted with commit f5409d9. With a small correction to the change, this should now function as expected. The status-probe.sh script checks the value for GLUSTER_BLOCK_ENABLED and compares it to 1. However the update-params.sh script set the variable by default to TRUE. gluster-kubernetes and openshift-ansible both set the variable to "1" by default, so should update-params.sh.

Updates: #120

@SaravanaStorageNetwork
Copy link
Member

@nixpanic But status-probe refers the variable GLUSTER_BLOCKD_STATUS_PROBE_ENABLE( which is for whether to check gluster blockd status or not)
Here, it is GLUSTER_BLOCK_ENABLED which is for whether enable/disable GLUSTER_BLOCK ?

Note:
It is good idea to merge them both, but currently they are different variables(with different meaning).

@nixpanic
Copy link
Member Author

nixpanic commented Jan 15, 2019 via email

…ck service.

Enable gluster-block services if GLUSTER_BLOCK_ENABLED is "1" and
disable otherwise.

By default, GLUSTER_BLOCK_ENABLED is set as "1".

When gluster-block is disabled, the status-probe.sh script should not
check for its status.

Updates: gluster#120
Signed-off-by: Saravanakumar Arumugam <[email protected]>
Signed-off-by: Niels de Vos <[email protected]>
@nixpanic nixpanic force-pushed the gluster-block/env-var-to-disable branch from 9b1aeba to 35a40b8 Compare January 16, 2019 07:39
@nixpanic
Copy link
Member Author

I have modified the probe-status.sh script to also check the GLUSTER_BLOCK_ENABLED variable now. By default gluster-block is enabled, and so is the probe. GLUSTER_BLOCKD_STATUS_PROBE_ENABLE can still be used to disable the probe, there could be users that have configured it that way.

Copy link

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

Looks reasonable enough to me.

#FIXME To update in environment file
sed -i '/GB_GLFS_LRU_COUNT=/s/GB_GLFS_LRU_COUNT=.*/'GB_GLFS_LRU_COUNT="$GB_GLFS_LRU_COUNT"\"'/' /usr/lib/systemd/system/gluster-blockd.service
sed -i '/EnvironmentFile/i Environment="GB_LOGDIR='$GB_LOGDIR'"' /usr/lib/systemd/system/gluster-blockd.service
if [ "$GLUSTER_BLOCK_ENABLED" == 1 ]; then

Choose a reason for hiding this comment

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

Here, 1 is numeric value => eq is better.

Choose a reason for hiding this comment

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

< This change was recently reverted with commit f5409d9. With a small correction to the change, this should now function as expected. The status-probe.sh script checks the value for GLUSTER_BLOCK_ENABLED and compares it to 1. However the update-params.sh script set the variable by default to TRUE. gluster-kubernetes and openshift-ansible both set the variable to "1" by default, so should update-params.sh.

This needs an update too

if [ "$GLUSTER_BLOCK_ENABLED" == 1 ]; then
echo "Enabling gluster-block service and updating env. variables"
systemctl enable gluster-block-setup.service
systemctl enable gluster-blockd.service

Choose a reason for hiding this comment

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

Is the actual issue mentioned here issue #120 resolved here? Please reboot the node and see whether this specific line executes fine.

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