Skip to content

mon: have mon-specific commands under 'mon' module#4595

Closed
jecluis wants to merge 17 commits intomasterfrom
wip-11545
Closed

mon: have mon-specific commands under 'mon' module#4595
jecluis wants to merge 17 commits intomasterfrom
wip-11545

Conversation

@jecluis
Copy link
Copy Markdown
Member

@jecluis jecluis commented May 6, 2015

e.g., instead of having 'ceph scrub' or 'ceph sync force', have instead
'ceph mon scrub' and 'ceph mon sync force'. Makes more sense and avoids
confusing the user about what component of ceph is being synchronized or
scrubbed.

Fixes: #11545

Signed-off-by: Joao Eduardo Luis joao@suse.de

@jecluis
Copy link
Copy Markdown
Member Author

jecluis commented May 6, 2015

note before: this has not gone through any teuthology run yet. I have not run a manual upgrade test yet, and I'm a bit concerned it will not make it on the first try -- although I expect it to work if the peons are the first to be upgraded.

@ghost
Copy link
Copy Markdown

ghost commented May 6, 2015

is this backward compatible ?

@loic-bot
Copy link
Copy Markdown

loic-bot commented May 7, 2015

@jecluis
Copy link
Copy Markdown
Member Author

jecluis commented May 7, 2015

@dachary I think so but not in a way I'm absolutely comfortable with. I'll rework this patch later today.

@jecluis
Copy link
Copy Markdown
Member Author

jecluis commented May 8, 2015

repushed a saner approach

@ghost
Copy link
Copy Markdown

ghost commented May 11, 2015

@jecluis LGTM

@ghost
Copy link
Copy Markdown

ghost commented May 11, 2015

@jecluis could you link the successfull mon suite run you scheduled from here for archival purposes ?

@jecluis
Copy link
Copy Markdown
Member Author

jecluis commented May 11, 2015

@dachary Haven't scheduled a run on this yet. Trying to avoid wasting resources on something that seems a bit controversial -- and thus has higher probability of being scrapped.

@tchaikov
Copy link
Copy Markdown
Contributor

lgtm. looks like i need to revise my "mon_metadata" to move it into the herd of mon commands.

@jecluis
Copy link
Copy Markdown
Member Author

jecluis commented May 11, 2015

@tchaikov I would think that if both end up being merged in the same dev release, and your 'mon_metadata' pull request is merged before this one, we could simply change your command without the need to deprecate it. I may be wrong here, but I think that such a change would not leak outside the dev release.

@tchaikov
Copy link
Copy Markdown
Contributor

oh, thanks. that's a relief. =)

@jecluis
Copy link
Copy Markdown
Member Author

jecluis commented May 12, 2015

rebased on top of current master and changed @tchaikov's 'mon_metadata' to 'mon metadata' (see last patch of the series)

@loic-bot
Copy link
Copy Markdown

@liewegas
Copy link
Copy Markdown
Member

Reviewed-by:

@liewegas
Copy link
Copy Markdown
Member

mds cluster_down
Error EINVAL: invalid command
in the test..

@liewegas liewegas removed their assignment May 14, 2015
@jecluis
Copy link
Copy Markdown
Member Author

jecluis commented May 26, 2015

rebase on top of current master

jecluis added 2 commits May 29, 2015 12:20
This allows us to do nifty stuff like 'FLAG(foo) | FLAG(bar)' and expand
it to (MonCommand::FLAG_foo | MonCommand::FLAG_bar), instead of being
bound by a single flag on macro expansion.

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Makes much more sense if we're OR'ing flags.  Or not as weird.

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
jecluis added 15 commits May 29, 2015 12:20
Instead of passing '0' for commands without flags, pass FLAG_NONE
instead.  It's prettier and more obvious -- and it only costs 64 bits.

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Otherwise it's virtually impossible to change a command's help string
without triggering a mismatch with the leader's command set.

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Makes it easier to identify the command as being related with the
monitor instead of cluster-wide.

This entails adding an exception to module 'mon' in order to have this
command handled by the Monitor class instead of MonmapMonitor (which is
the one traditionally handling 'mon' module commands).

Fixes: #11545

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Makes it easier to identify the command as being related with the
monitor instead of cluster-wide.

This entails adding an exception to module 'mon' in order to have this
command handled by the Monitor class instead of MonmapMonitor (which is
the one traditionally handling 'mon' module commands).

Fixes: #11545

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
Makes it easier to identify the command as being related with the
monitor instead of cluster-wide.

This entails adding an exception to module 'mon' in order to have this
command handled by the Monitor class instead of MonmapMonitor (which is
the one traditionally handling 'mon' module commands).

Fixes: #11545

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
'ceph mon_metadata' was added still during this dev cycle, so there is
no need to deprecate it first.

Fixes: #11545

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
jecluis added a commit that referenced this pull request Jun 1, 2015
mon: have mon-specific commands under 'mon' module
@jecluis
Copy link
Copy Markdown
Member Author

jecluis commented Jul 16, 2015

merged manually

@jecluis jecluis closed this Jul 16, 2015
@liewegas liewegas deleted the wip-11545 branch November 23, 2016 23:25
tchaikov added a commit to tchaikov/ceph that referenced this pull request Dec 20, 2019
quote from Sage's reply

> This is a mon-specific command--it doesn't make sense as a CLI command
> for the entire cluster--it only makes sense as a command to tell a
> specific monitor.  Like ``ceph tell mon.a compact``.  Back when Joao
> did ceph#4595 these commands were all mixed together and putting it under
> 'ceph mon ...' made sense, but now you're specifially sending it to a
> mon, so the 'mon' part of the command is redundant.

so let's drop "mon compact" in favor of "compact" command

Signed-off-by: Kefu Chai <kchai@redhat.com>
ronen-fr pushed a commit to ronen-fr/ceph that referenced this pull request Dec 29, 2019
quote from Sage's reply

> This is a mon-specific command--it doesn't make sense as a CLI command
> for the entire cluster--it only makes sense as a command to tell a
> specific monitor.  Like ``ceph tell mon.a compact``.  Back when Joao
> did ceph#4595 these commands were all mixed together and putting it under
> 'ceph mon ...' made sense, but now you're specifially sending it to a
> mon, so the 'mon' part of the command is redundant.

so let's drop "mon compact" in favor of "compact" command

Signed-off-by: Kefu Chai <kchai@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants