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

vstart: fix initial start when there is no ceph.conf #21019

Merged
merged 1 commit into from Mar 23, 2018

Conversation

Projects
None yet
2 participants
@majianpeng
Copy link
Member

commented Mar 23, 2018

This partly revert commit:f437598cfcabbd66c372bc8.
Before firstly creating a ceph cluster, there is no ceph.conf.
If specify a non-exist ceph.conf, ceph-conf will meet error.

Signed-off-by: Jianpeng Ma jianpeng.ma@intel.com

@majianpeng

This comment has been minimized.

Copy link
Member Author

commented Mar 23, 2018

Refer #20888

@@ -349,14 +349,18 @@ if [ "$overwrite_conf" -eq 0 ]; then
else
if [ "$new" -ne 0 ]; then
# only delete if -n

This comment has been minimized.

Copy link
@trociny

trociny Mar 23, 2018

Contributor

So the purpose of this if [ "$new" -ne 0 ] section is to cleanup remnants (asok dir) from the previous vstart run. I think it makes sense to try to remove asok_dir only if $conf_fn exists, otherwise (when ceph-conf is called without -c $conf_fn we may delete someone other dir specified in the global ceph.conf.

Thus I think you should check if $conf_fn exists end skip the removal if it does not. I.e. the condition could be:

     if [ "$new" -ne 0 -a -e "$conf_fn" ]; then

@trociny trociny changed the title Partly revert commit f437598cfcabbd66c372bc8 vstart: fix initial start when there is no ceph.conf Mar 23, 2018

rm -- "$conf_fn"
else
asok_dir=`dirname $($CEPH_BIN/ceph-conf --show-config-value admin_socket)`
fi
if [ $asok_dir != /var/run/ceph ]; then
[ -d $asok_dir ] && rm -f $asok_dir/* && rmdir $asok_dir
fi
if [ -z "$CEPH_ASOK_DIR" ]; then

This comment has been minimized.

Copy link
@trociny

trociny Mar 23, 2018

Contributor

Ah, sorry, this part should be called for if [ "$new" -ne 0 ] condition. So I think you need:

if [ "$new" -ne 0  ]; then
  if [ -e "$conf_fn" ]; then
    try to remove $asok_dir
  fi
  if [ -z "$CEPH_ASOK_DIR" ]; then
    ...

This comment has been minimized.

Copy link
@majianpeng

majianpeng Mar 23, 2018

Author Member

If previous we created a cluster used vstart.sh. And rm -rf build and rebuild. Although there is no ceph.conf but it can still remove asok_dir. That it ceph-conf can do default value if no ceph.conf.

This comment has been minimized.

Copy link
@trociny

trociny Mar 23, 2018

Contributor

If there is no ceph.conf, it will use /etc/ceph/ceph.conf. Let's suppose I use /etc/ceph/ceph.conf to connect to a remote cluster and have in that file admin_socket = /my/secure/socket/location/admin.asok. Why when deploying a local cluster with vstart will need to delete /my/secure/socket/location/ directory?

The purpose of this $asok_dir removal is to remove the directory created by the previous vstart.sh -n invocation. And the location is specified in the local (build directory) ceph.conf. If this ceph.conf file does not exist we don't have information where the old $asok_dir could be. Deleting someone else directory in this case (just because we can find it in the global config) is not correct.

This comment has been minimized.

Copy link
@majianpeng

majianpeng Mar 23, 2018

Author Member

If ceph.conf didn't exist.
./bin/ceph-conf --show-config-value admin_socket
did not load config file, using default settings.
/var/run/ceph/ceph-client.admin.asok

./bin/ceph-conf -c ceph.conf --show-config-value admin_socket
global_init: unable to open config file from search list ceph.conf

This comment has been minimized.

Copy link
@trociny

trociny Mar 23, 2018

Contributor
zhuzha:~% echo -e "[global]\nadmin socket = $HOME/admin.asok" | sudo tee /etc/ceph/ceph.conf
[global]
admin socket = /home/mgolub/admin.asok
zhuzha:~% ceph-conf  --show-config-value admin_socket
/home/mgolub/admin.asok

What should I expect when I run vstart.sh -n after this?

This comment has been minimized.

Copy link
@majianpeng

majianpeng Mar 23, 2018

Author Member

Ok, i see. Unless specify ceph.conf, we don't remove $asok_dir(not equal /var/run/ceph).

@majianpeng majianpeng force-pushed the majianpeng:fix-vstart branch from 777e88c to 5de320b Mar 23, 2018

@trociny
Copy link
Contributor

left a comment

LGTM

@trociny trociny merged commit f0b780f into ceph:master Mar 23, 2018

4 of 5 checks passed

make check (arm64) make check failed
Details
Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
vstart.sh: Avoiding ceph-conf error when create a new cluster.
This partly revert commit:f437598cfcabbd66c372bc8.
Before firstly creating a ceph cluster, there is no ceph.conf.
If specify a non-exist ceph.conf, ceph-conf will meet error.

Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.