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: allow to start multiple radosgw when RGW=x #15632

Merged
merged 2 commits into from Jul 11, 2017

Conversation

Projects
None yet
5 participants
@aclamk
Contributor

aclamk commented Jun 12, 2017

This fulfills feature request #20235

Signed-off-by: Adam Kupczyk akupczyk@mirantis.com

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 12, 2017

retest this please.

@theanalyst

This comment has been minimized.

Member

theanalyst commented Jun 12, 2017

mstart also actually increments by number of clusters, can you verify the mstart scripts also work with say RGW=2

@liewegas liewegas added the build/ops label Jun 12, 2017

@liewegas liewegas changed the title from feature, vstart: allow to start multiple radosgw when RGW=x in vstart.sh invocation to vstart: allow to start multiple radosgw when RGW=x in vstart.sh invocation Jun 12, 2017

@liewegas liewegas changed the title from vstart: allow to start multiple radosgw when RGW=x in vstart.sh invocation to vstart: allow to start multiple radosgw when RGW=x Jun 12, 2017

@theanalyst theanalyst added the tools label Jun 13, 2017

@aclamk

This comment has been minimized.

Contributor

aclamk commented Jun 13, 2017

@theanalyst You were right. Addresses would have overlapped.
Now first cluster gets radosgws on: 8001, 8011, 8021
Second: 8002, 8012, 8022.
And so on.....
The reasoning is that existing tests assume that 8001 and 8002 are used by different clusters.

@theanalyst

Looks good, I know this isn't a part of the changeset as such, but can we wrap the user create in a if ["$new" -eq 1] block, currently the behaviour of vstart is that if there is a new cluster it works fine with RGW, but in case of reusing an old vstart cluster RGW=x fails because it fails at creating the subuser again (which was already created) .. this isn't introduced as a part of this changeset but would be good to have

@aclamk

This comment has been minimized.

Contributor

aclamk commented Jun 13, 2017

@theanalyst Done. Added creation of users only when cluster is created.

@theanalyst theanalyst requested a review from cbodley Jun 13, 2017

[client.rgw.$rgw]
rgw frontends = $rgw_frontend port=$((CEPH_RGW_PORT + rgw * 10))
EOF

This comment has been minimized.

@cbodley

cbodley Jun 13, 2017

Contributor

a similar approach came up last year, and i suggested passing rgw_frontends on the command line in do_rgw(), instead of setting it in the config. that would allow you to create a vstart cluster with -n, and then later increase CEPH_NUM_RGW without having to rewrite ceph.conf. see #8241 (comment). do you think that would work here?

@aclamk

This comment has been minimized.

Contributor

aclamk commented Jun 14, 2017

@cbodley Done.
Now you can create cluster without specifying RGW=, and then give RGW=n on cluster start. Works for up to n=10.
@theanalyst Please review again.

@@ -975,8 +989,13 @@ do_rgw()
RGWSUDO=
[ $CEPH_RGW_PORT -lt 1024 ] && RGWSUDO=sudo
n=$(($CEPH_NUM_RGW - 1))
#extract rgw_frontends
local rgw_fe=`$CEPH_BIN/ceph-conf -c $conf_fn --name $VSTART_SEC rgw_frontends 2>/dev/null`

This comment has been minimized.

@cbodley

cbodley Jun 14, 2017

Contributor

by relying on ceph.conf for the frontend type and base port here, that means you'd have to recreate the cluster in order to change either. it seems more flexible to just use the values of $rgw_frontend and $CEPH_RGW_PORT from the current invocation?

--cap osd 'allow *' \
--cap mds 'allow *' \
--cap mgr 'allow *' \
"$keyring_fn"

This comment has been minimized.

@theanalyst

theanalyst Jun 14, 2017

Member

since we're not appending in ceph.conf anymore the keyring isn't strictly needed anymore,while there is an advantage that we could have rgw specific section under that config. the disadvantage is that we have to specify a --name when start rgw if I'm not mistaken... no strong opinions but we could do away with not creating a keyring I suppose? @cbodley thoughts?

This comment has been minimized.

@aclamk

aclamk Jun 14, 2017

Contributor

@theanalyst If we do not supply --name then we use "client.admin", so it will still work.
You are right, if we want to use "client.admin", then both [client.rgw] in ceph.conf and creating keyring for client.rgw are unnecessary.
But I followed existing logic of having separate entries for radosgw.

This comment has been minimized.

@theanalyst

theanalyst Jun 14, 2017

Member

ok, I'm fine with the keyring actually it does allow us to have seperate debug levels and config sections

This comment has been minimized.

@cbodley

cbodley Jun 19, 2017

Contributor

is there still a need for this and the [client.rgw] section?

This comment has been minimized.

@cbodley

cbodley Jun 22, 2017

Contributor

is there still a need for this and the [client.rgw] section?

@aclamk latest changes look good, but i'm still curious about this part

@aclamk

This comment has been minimized.

Contributor

aclamk commented Jun 19, 2017

@cbodley Now rgw_frontend and CEPH_RGW_PORT are guessed from ceph.conf only if not set in command line.

@@ -107,7 +107,7 @@ cephx=1 #turn cephx on by default
cache=""
memstore=0
bluestore=0
rgw_frontend="civetweb"
rgw_frontend=${rgw_frontend:-"civetweb"}

This comment has been minimized.

@cbodley

cbodley Jun 19, 2017

Contributor

this is the first use of rgw_frontend, so i don't think this is any different than rgw_frontend="civetweb"

--cap osd 'allow *' \
--cap mds 'allow *' \
--cap mgr 'allow *' \
"$keyring_fn"

This comment has been minimized.

@cbodley

cbodley Jun 19, 2017

Contributor

is there still a need for this and the [client.rgw] section?

@@ -975,8 +989,16 @@ do_rgw()
RGWSUDO=
[ $CEPH_RGW_PORT -lt 1024 ] && RGWSUDO=sudo
n=$(($CEPH_NUM_RGW - 1))
#extract rgw_frontends
local rgw_fe_tmp=`$CEPH_BIN/ceph-conf -c $conf_fn --name $VSTART_SEC rgw_frontends 2>/dev/null`

This comment has been minimized.

@cbodley

cbodley Jun 19, 2017

Contributor

i thought we agreed to pass $rgw_frontend and $CEPH_RGW_PORT directly instead of rereading them from ceph.conf. it looks like this would prevent you from changing the frontend type after running with -n

@aclamk

This comment has been minimized.

Contributor

aclamk commented Jun 22, 2017

@cbodley @theanalyst Simplified. Now no more quessing params from ceph.conf.

@cbodley

This comment has been minimized.

Contributor

cbodley commented Jul 10, 2017

ping @aclamk, needs rebase and i still had a question

vstart: allow to start multiple radosgw on consecutive ports
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
@aclamk

This comment has been minimized.

Contributor

aclamk commented Jul 11, 2017

@cbodley rebased

@@ -977,7 +991,8 @@ do_rgw()
i=0
for rgw in j k l m n o p q r s t u v; do
ceph_adm auth get-or-create client.rgw.$rgw mon 'allow rw' osd 'allow rwx' mgr 'allow rw' -o $CEPH_DEV_DIR/rgw.$rgw.keyring
run 'rgw' $RGWSUDO $CEPH_BIN/radosgw -c $conf_fn --log-file=${CEPH_OUT_DIR}/rgw.$rgw.log ${RGWDEBUG} --debug-ms=1 -n client.rgw.$rgw -k $CEPH_DEV_DIR/rgw.$rgw.keyring
echo start rgw on http://localhost:$((CEPH_RGW_PORT + i))
run 'rgw' $RGWSUDO $CEPH_BIN/radosgw -c $conf_fn --log-file=${CEPH_OUT_DIR}/rgw.$rgw.log ${RGWDEBUG} --debug-ms=1 -n client.rgw "--rgw_frontends=${rgw_frontend} port=$((CEPH_RGW_PORT + i))"

This comment has been minimized.

@cbodley

cbodley Jul 11, 2017

Contributor

this changes the instance name from client.rgw.$rgw to client.rgw, but we're still creating keys for both. we should probably remove the ceph_adm auth get-or-create client.rgw.$rgw line above. also, the keyring created for client.rgw uses allow * for everything (even mds), whereas these client.rgw.$rgw keys are more restrictive

This comment has been minimized.

@aclamk

aclamk Jul 11, 2017

Contributor

@cbodley I removed keys for client.rgw.$rgw and also tuned down permissions for client.rgw.

Adam Kupczyk
Fixed too big privileges for client.rgw.
Signed-off-by: Adam Kupczyk <akucpzyk@redhat.com>

@cbodley cbodley merged commit e531270 into ceph:master Jul 11, 2017

4 checks passed

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
make check (arm64) make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment