search

gluster / gluster-containers

Dockerfiles (CentOS, Fedora, Red Hat) for GlusterFS
https://github.com/gluster/gluster-containers/pkgs/container/gluster-containers
223 stars 135 forks source link

Set GB_CLI_TIMEOUT value as part of container startup. #146

Closed SaravanaStorageNetwork closed 5 years ago

SaravanaStorageNetwork commented 5 years ago

Signed-off-by: Saravanakumar Arumugam sarumuga@redhat.com

SaravanaStorageNetwork commented 5 years ago

Please describe the need for this change in the commit message. Point to public bugs if they exist.

Done

nixpanic commented 5 years ago

Now this confuses me. The commit message mentions that gluster-block uses /etc/sysconfig/gluster-blockd, but the environment is changed/added to /usr/lib/systemd/system/gluster-blockd.service. Changing the option in /etc/sysconfig/gluster-blockd would be much cleaner.

What is the reason to change the default of 900 to 600? Or is the 900 in the commit message just an example?

SaravanaStorageNetwork commented 5 years ago

Now this confuses me. The commit message mentions that gluster-block uses /etc/sysconfig/gluster-blockd, but the environment is changed/added to /usr/lib/systemd/system/gluster-blockd.service. Changing the option in /etc/sysconfig/gluster-blockd would be much cleaner.

Nope, the value is updated in gluster-blockd.service file only. I have referred the bz for updating this comment. and hence the confusion. I will reword it.

What is the reason to change the default of 900 to 600? Or is the 900 in the commit message just an example?

Yes, 900 is just and example. I will better reword this one too to avoid confusion.

SaravanaStorageNetwork commented 5 years ago

Changing the option in /etc/sysconfig/gluster-blockd would be much cleaner.

Agree. But even another variable is updated in gluster-blockd.service only. I will take up this cleanup of moving to /etc/sysconfig/gluster-blockd separately.

Ref: https://github.com/gluster/gluster-containers/blob/master/CentOS/update-params.sh#L37

humblec commented 5 years ago

@SaravanaStorageNetwork anything pending here ?

SaravanaStorageNetwork commented 5 years ago

@SaravanaStorageNetwork anything pending here ?

All done.

humblec commented 5 years ago

LGTM. Merging! Thanks all!