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

mgr,mon: enable/disable mgr modules via 'ceph mgr module ...' commands #15958

Merged
merged 4 commits into from Jul 4, 2017

Conversation

Projects
None yet
3 participants
@liewegas
Member

liewegas commented Jun 27, 2017

This doesn't include any magic for auto-enabling modules that are installed. I'm
not sure that's such a good idea anyway.

@liewegas liewegas requested a review from jcsp Jun 27, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 27, 2017

@jcsp

This comment has been minimized.

Contributor

jcsp commented Jun 27, 2017

It's not essential, but I think this will be friendlier if we have the mgr daemons report their available plugins to the mons, so that we can warn (if not forbid) the user if they are enabling a nonexistent module -- typos etc.

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 27, 2017

I don't really like the MgrMap structure of having the top level contain the active mgr fields and standbys map contain the others. Maybe we can move to a more MDSMap-like structure where we have a map of all daemons, and a single top-level field indicating which one is the active one?

That ugliness aside, this should work. If we're going to change it though we should flip around the structure and compat encoding first before adding this.

void PyModules::list_modules(std::set<std::string> *modules)
{
// FIXME search system paths too?

This comment has been minimized.

@jcsp

jcsp Jun 27, 2017

Contributor

Restricting to mgr_module_path is by design -- if we used system paths then our mgr module names would potentially collide with any ordinary python module names on the system.

struct stat st;
int r = ::stat(fn.c_str(), &st);
if (r == 0 && S_ISDIR(st.st_mode)) {
string initfn = fn + "/__init__.py";

This comment has been minimized.

@jcsp

jcsp Jun 27, 2017

Contributor

Should check for "module.py" -- we might have an __init__.py in a directory that was just e.g. shared code in a module.

The fixed module.py name is a bit hacky but probably actually helps in this case.

:Description: List of python modules to load
:Type: String
:Default: ``"rest"`` (Load the REST API module only)

This comment has been minimized.

@tserong

tserong Jun 28, 2017

Contributor

Should be "restful"

This comment has been minimized.

@liewegas

liewegas Jun 28, 2017

Member

Removed now anyway :)

@@ -36,17 +38,21 @@ class StandbyInfo
void encode(bufferlist& bl) const
{
ENCODE_START(1, 1, bl);
ENCODE_START(2, 1, bl);

This comment has been minimized.

@tserong

tserong Jun 28, 2017

Contributor

Is this inconsistent with the HEAD_VERSION in MMgrBeacon.h? Or do I just not understand this piece?

This comment has been minimized.

@liewegas

liewegas Jun 28, 2017

Member

Unrelated... the 1 and 2 here are the version for this struct only (StandbyInfo)

::decode(gid, p);
::decode(name, p);
if (struct_v >= 2) {

This comment has been minimized.

@tserong

tserong Jun 28, 2017

Contributor

Same question as above (there's more instances of this below, too)

liewegas added some commits Jun 27, 2017

mgr: include MgrMap in ClusterState
Signed-off-by: Sage Weil <sage@redhat.com>
mon,mgr: manage mgr module list in mgrmap
Kill old mgr_modules option.

Add new mgr_initial_modules option, on the mon, for the initial cluster
mgrmap.

Add ls, enable, disable commands.

Respawn mgr if the module list changes.  In the future we could enable
new modules without a full restart, but disabling probably requires (and
is best handled by) a respawn.

Signed-off-by: Sage Weil <sage@redhat.com>
doc: update docs for enabling mgr modules
Signed-off-by: Sage Weil <sage@redhat.com>
mon,mgr: track available modules in MgrMap too
Require --force to enable a module that isn't reported as available.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas

This comment has been minimized.

Member

liewegas commented Jun 30, 2017

@jcsp ok to merge do you think?

@jcsp

This comment has been minimized.

Contributor

jcsp commented Jul 4, 2017

👍 no more fumbling around with mgr_modules!

@jcsp

jcsp approved these changes Jul 4, 2017

@jcsp jcsp merged commit b363e58 into ceph:master Jul 4, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
default Build finished.
Details
make check make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment