Skip to content
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/dashboard: Add ErasureCodeProfile controller #20920

Merged

Conversation

sebastian-philipp
Copy link
Contributor

@sebastian-philipp sebastian-philipp commented Mar 15, 2018

This PR is part of backend implementation of the create-pool dialog. It is related to PR #20865 but not a dependency of it. Part of the Create-Pool-Dialog is a sub-dialog to manage ECPs.

TODO:

Signed-off-by: Sebastian Wagner sebastian.wagner@suse.com

I've also added a commit made by @Devp00l to fix an issue with the pool creation dialog.

Copy link
Member

@jecluis jecluis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR depends on a yet-unmerged PR (#20865), and that should be mentioned on the PR description.

return ret

@RESTController.args_from_json
def create(self, name, k, m, ruleset_failure_domain=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about allowing the caller to specify the plugin to the be used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this was something that was brought up in a conversation I had today. Being able to provide the plugin would be nice, but then we would also need a method that returns a list of available plugins, so we don't have to maintain a hard-coded list. Not sure if that exists, though. We had similar issues with providing a selection list for compression plugins in openATTIC where we maintained a static list, but the actual build of Ceph may have been compiled with different defaults.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

if ruleset_failure_domain:
profile.append('crush-failure-domain={}'.format(ruleset_failure_domain))

CephService.send_command('mon', 'osd erasure-code-profile set', name=name,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is the potential errors of this call being handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error handling will change as soon as the task manager form @rjfd is merged. Right now, an exception is thrown and cached by the general exception handler which will render a JSON error containing an error message.

@rjfd
Copy link
Contributor

rjfd commented Mar 15, 2018

@sebastian-philipp can you please add a bit more detail in the PR's description about what this PR implements?

@LenzGr LenzGr changed the title [WIP] mgr/dashboard_v2: Add ErasureCodeProfile controller [WIP] mgr/dashboard: Add ErasureCodeProfile controller Mar 16, 2018
@sebastian-philipp sebastian-philipp force-pushed the dashboard_v2-erasure-code-profile branch from fec1dd1 to eedc1f1 Compare March 20, 2018 15:53
@sebastian-philipp sebastian-philipp changed the title [WIP] mgr/dashboard: Add ErasureCodeProfile controller mgr/dashboard: Add ErasureCodeProfile controller Mar 23, 2018
@sebastian-philipp sebastian-philipp force-pushed the dashboard_v2-erasure-code-profile branch 2 times, most recently from cd6f732 to 1ed3bcd Compare March 26, 2018 15:00
@LenzGr
Copy link
Contributor

LenzGr commented Apr 6, 2018

retest this please

sebastian-philipp and others added 2 commits April 6, 2018 17:40
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
As needed by the UI.

Signed-off-by: Stephan Müller <smueller@suse.com>
@sebastian-philipp
Copy link
Contributor Author

Just updated this PR to the latest master. @Devp00l please retest.

@sebastian-philipp
Copy link
Contributor Author

jenkins retest this please

Copy link

@Devp00l Devp00l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@rjfd rjfd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rjfd
Copy link
Contributor

rjfd commented Apr 11, 2018

@LenzGr LenzGr merged commit f0614a8 into ceph:master Apr 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants