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

pybind/mgr/restful: improve cert handling; work with vstart #15405

Merged
merged 17 commits into from Jun 8, 2017

Conversation

Projects
None yet
4 participants
@liewegas
Member

liewegas commented Jun 1, 2017

No description provided.

@liewegas liewegas requested a review from b-ranto Jun 1, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jun 1, 2017

retest this please. see #15406

cert_fname = None
pkey_fname = None
cert = self.get_config("cert")

This comment has been minimized.

@b-ranto

b-ranto Jun 1, 2017

Contributor

This should be get_config_json, not get_config. get_config returns the cluster config as in /config/cluster, not the config-keys. (the same should go for other get_config calls).

This comment has been minimized.

@liewegas

liewegas Jun 1, 2017

Member

i'm not sure what you mean. see the config-key sets in vstart.sh; this is pulling those same values (and working). same thing that dashboard is doing...

This comment has been minimized.

@b-ranto

b-ranto Jun 1, 2017

Contributor

Ah, sorry, I have misread that. I thought it was self.get('config') which is a completely different thing. I suppose this should work. The only question is whether we want to do the json serialization or not but I guess for the keys, it does make sense not to serialize.

server_addr, server_port)
cert_fname = None
pkey_fname = None

This comment has been minimized.

@b-ranto

b-ranto Jun 1, 2017

Contributor

There is no need to initialize variables like this in python, they will be overwritten (re-initialized) by next assignment anyway (that holds even for if/else like you use later in the code). The same goes for cert_tmp and pkey_tmp.

This comment has been minimized.

@liewegas

liewegas Jun 1, 2017

Member

ah, fixed

@b-ranto

b-ranto approved these changes Jun 1, 2017

@liewegas liewegas added the mgr label Jun 1, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 1, 2017

@b-ranto can you take another quick look at the test change too?

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Jun 1, 2017

lgtm as well

@liewegas liewegas added the needs-qa label Jun 1, 2017

@liewegas liewegas changed the title from WIP: pybind/mgr/restful: improve cert handling; work with vstart to pybind/mgr/restful: improve cert handling; work with vstart Jun 1, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 1, 2017

retest this please

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 2, 2017

@b-ranto @jcsp: several changes

  • changed commands to have 'restful' prefix
  • added get_mgr_id
  • added get_config_prefix
  • get_localized_config helper in restful
  • used 'crt' and 'key' names instead of 'cert' and 'pkey' (to match file extensions)
@liewegas

This comment has been minimized.

Member

liewegas commented Jun 5, 2017

retest this please

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 6, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 6, 2017

retest this please

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 7, 2017

retest this please

1 similar comment
@liewegas

This comment has been minimized.

Member

liewegas commented Jun 7, 2017

retest this please

liewegas added some commits May 23, 2017

vstart.sh: start mgr dashboard, echo URL(s)
Signed-off-by: Sage Weil <sage@redhat.com>
vstart.sh: load 'restful' (not 'rest') mgr module
Signed-off-by: Sage Weil <sage@redhat.com>
ceph.spec.in: add python-pecan as BuildDepends too
For install-deps.sh's benefit.

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

liewegas added some commits Jun 1, 2017

pybind/mgr/restful: all crt/key or filename in config-key
Signed-off-by: Sage Weil <sage@redhat.com>
test/vstart_wrapper.sh: fix MGR_PYTHON_PATH
Signed-off-by: Sage Weil <sage@redhat.com>
vstart.sh: start up mgr restful API
We try to stagger the ports that mgr modules bind to so that
concurrent vstart instances can run with consecutive ports but
the services will not collide.  Yes, this is awkward.

Signed-off-by: Sage Weil <sage@redhat.com>
qa/workunits/rest/test_mgr_rest_api.py: improvements
Signed-off-by: Sage Weil <sage@redhat.com>
mon/ConfigKeyService: more useful status message
Signed-off-by: Sage Weil <sage@redhat.com>
mgr/PyModules: prefix by mgr/, not mgr/$id/
If modules want per-instance state, they can include
the mgr id in their portion of the key name.

Signed-off-by: Sage Weil <sage@redhat.com>
mgr/PyState: add get_mgr_id() to module interface
Signed-off-by: Sage Weil <sage@redhat.com>
pybind/mgr/restful: localize key/crt keys
Signed-off-by: Sage Weil <sage@redhat.com>
mgr: add get_config_prefix
Fetch a dict of all config options with a given prefix.

Signed-off-by: Sage Weil <sage@redhat.com>
pybind/mgr/restful: store each key+pass in a separate key
Signed-off-by: Sage Weil <sage@redhat.com>
pybind/mgr/restful: prefix commands with 'restful'
And use - instead of _, following the ceph convention.

Signed-off-by: Sage Weil <sage@redhat.com>
pybind/mgr/dashboard: get_localized_config for server_{addr,port}
Signed-off-by: Sage Weil <sage@redhat.com>
mgr/Mgr: fix deadlock in load_config
Signed-off-by: Sage Weil <sage@redhat.com>
debian/control: Build-Depends on python-pecan
This is needed for make check (so that restful mgr module can load).

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

@liewegas liewegas merged commit 1101661 into ceph:master Jun 8, 2017

1 of 3 checks passed

Unmodifed Submodules checking if PR has modified submodules
Details
default Build started sha1 is merged.
Details
Signed-off-by all commits in this PR are signed
Details

@liewegas liewegas deleted the liewegas:wip-rest-test branch Jun 8, 2017

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