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.sh: do not support positional arguments of mon,osd,mds #13974

Merged
merged 5 commits into from Mar 24, 2017

Conversation

Projects
None yet
3 participants
@tchaikov
Contributor

tchaikov commented Mar 15, 2017

No description provided.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 21, 2017

@liewegas @alimaredia mind taking a look?

@liewegas

This comment has been minimized.

Member

liewegas commented Mar 21, 2017

looks fine to me, although i'd still probably lean toward eliminating start_* entirely so that there is just one way to control what is started (MON, MGR, etc. vars). Either way, though!

@alimaredia

This comment has been minimized.

Contributor

alimaredia commented Mar 21, 2017

@liewegas I agree on having just one way to control what is started. This could make the usage much clearer as well. That's out of the scope of this PR though, perhaps I shoud take it to the mailing list?

@liewegas

This comment has been minimized.

Member

liewegas commented Mar 21, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 22, 2017

@liewegas could you take another look?

changelog

  • rebased against master.
  • and removed the start_*

tchaikov added some commits Mar 15, 2017

vstart.sh: extract start_{osd,mon,mgr,mds} into functions
Signed-off-by: Kefu Chai <kchai@redhat.com>
vstart.sh: do nothing if $CEPH_NUM_* is 0
Signed-off-by: Kefu Chai <kchai@redhat.com>
[ -z "$CEPH_NUM_MDS" ] && CEPH_NUM_MDS=0
[ -z "$CEPH_NUM_MGR" ] && CEPH_NUM_MGR=0
kill_all=0
fi

This comment has been minimized.

@liewegas

liewegas Mar 22, 2017

Member

what's the motivation for this? is this for running without -n?

FWIW I usually run vstart by override one or two of $MON or $MDS (e.g., MON=1 MDS=0) depending on what i'm testing. not getting the default values if any of them are specified seems weird.

This comment has been minimized.

@tchaikov

tchaikov Mar 22, 2017

Contributor

i usually just start MON and OSD, without MDS or MGR, this saves the time to ready and launch these daemons, and also i don't compile MDS by default.

This comment has been minimized.

@tchaikov

tchaikov Mar 22, 2017

Contributor

no, "-k" is the default behavior. the CEPH_NUM_* will be set using the stored number in ceph.conf afterwards if not "-n".

if you feel strong about this, i will instead make them override the default values.

@liewegas

This comment has been minimized.

Member

liewegas commented Mar 22, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 22, 2017

@liewegas, please see e8e83ae, this behavior is more backward compatible with existing tests. otherwise we need to specify the MDS=0 MGR=0 where they are not used. we will add "MGR=1" once the mon=>mgr work is done, for sure.

@liewegas

This comment has been minimized.

Member

liewegas commented Mar 22, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 23, 2017

okay, i will incorporate 47cc059 and make the numbers override the default settings. actually, much simpler in vstart.sh that way.

tchaikov added some commits Mar 22, 2017

vstart.sh: remove start_*
so there are only two ways to override the number of daemons to start
- using the env var CEPH_NUM_{MON|OSD|MGR|MDS} or {MON|OSD|MGR|MDS}
- command line options: --{mon,osd,mds}_num

do prevent a daemon from running, set the corrresponding env var to 0.

Signed-off-by: Kefu Chai <kchai@redhat.com>
vstart.sh: do not init fsmap if "$new == 0"
we cannot create a new cephfs using a non-empty pool without '--force'
option now, so the "ceph fs new" command fails with "vstart.sh -k".

Signed-off-by: Kefu Chai <kchai@redhat.com>
tests: remove mds,osd,mon args passed to vstart.sh
Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 23, 2017

retest this please

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 24, 2017

@liewegas fixed and repushed.

@tchaikov tchaikov changed the title from vstart: do not start mgr if not start_all to vstart.sh: do not support positional arguments of mon,osd,mds Mar 24, 2017

@liewegas liewegas added the build/ops label Mar 24, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Mar 24, 2017

lgtm.. ok to merge?

@tchaikov tchaikov merged commit 21d8a97 into ceph:master Mar 24, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Mar 24, 2017

@liewegas thanks for the review, merging!

@tchaikov tchaikov deleted the tchaikov:wip-vstart-start-mgr branch Mar 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment