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_v2: Fix test_cluster_configuration test #20782

Conversation

Projects
None yet
5 participants
@sebastian-philipp
Copy link
Member

commented Mar 7, 2018

This PR consist of two fixes in mgr.dashboard_v2.test_cluster_configuration:

1. common/config: Fix dumping of md_config_t::config_options.

Caused by 49a5e77

2. mgr/dashboard_v2: Fix another teuthology test failure.

Removed unused code in the dashboard_v2 backend, but the frontend is still broken.
This was caused by a rename of "developer" to "dev".
Caused by: 1717d4c


This PR fixes the dashboard_v2 cluster_conf endpoint.

Fixes: http://tracker.ceph.com/issues/23265

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

@sebastian-philipp sebastian-philipp force-pushed the sebastian-philipp:dashboard_v2_fix_config_options_endpoint branch from 76d1331 to f0cce81 Mar 7, 2018

@sebastian-philipp sebastian-philipp changed the title [WIP] common/config: Fix dumping of `options` array. [WIP] common/config: Fix dumping of md_config_t::config_options. Mar 7, 2018

@sebastian-philipp sebastian-philipp force-pushed the sebastian-philipp:dashboard_v2_fix_config_options_endpoint branch 2 times, most recently from 615ca18 to 9865320 Mar 7, 2018

common/config: Fix dumping of `md_config_t::config_options`.
Caused by 49a5e77

This fixes the dashboard_v2 `cluster_conf` endpoint.

Fixes: http://tracker.ceph.com/issues/23265

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

@sebastian-philipp sebastian-philipp force-pushed the sebastian-philipp:dashboard_v2_fix_config_options_endpoint branch from 9865320 to 0ad029f Mar 7, 2018

@rjfd rjfd added the needs-qa label Mar 7, 2018

@jecluis jecluis requested a review from liewegas Mar 7, 2018

@jecluis
Copy link
Member

left a comment

lgtm, although it's not clear in the ticket how this is fixing the behavior observed by the reporter.

Can you please add that info to the tracker for posterity? Otherwise, if this is not actually addressing the behavior in the ticket, but something else, mind dropping the Fixes tag?

@sebastian-philipp sebastian-philipp changed the title [WIP] common/config: Fix dumping of md_config_t::config_options. common/config: Fix dumping of md_config_t::config_options. Mar 7, 2018

@sebastian-philipp

This comment has been minimized.

Copy link
Member Author

commented Mar 7, 2018

@jecluis, added a comment to the ticket explaining the problem.

mgr/dashboard_v2: removed unused code that broke teuthology
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>

@sebastian-philipp sebastian-philipp changed the title common/config: Fix dumping of md_config_t::config_options. mgr/dashboard_v2: Fix mgr.dashboard_v2.test_cluster_configuration test Mar 7, 2018

@sebastian-philipp sebastian-philipp removed the DNM label Mar 7, 2018

@sebastian-philipp sebastian-philipp requested review from adamemerson and p-na Mar 7, 2018

@sebastian-philipp sebastian-philipp changed the title mgr/dashboard_v2: Fix mgr.dashboard_v2.test_cluster_configuration test mgr/dashboard_v2: Fix test_cluster_configuration test Mar 7, 2018

@@ -10,22 +10,8 @@
@ApiController('cluster_conf')
@AuthRequired()
class ClusterConfiguration(RESTController):
def list(self, service=None, level=None):

This comment has been minimized.

Copy link
@rjfd

rjfd Mar 8, 2018

Contributor

Is this being removed because it's not being used by the dashboard frontend?

This comment has been minimized.

Copy link
@sebastian-philipp

sebastian-philipp Mar 8, 2018

Author Member

yes.

@rjfd

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2018

@sebastian-philipp the rename "developer -> dev " will not break the dashboard frontend as well?

@sebastian-philipp

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2018

@rjfd the dashboard_v2 frontend is broken. @tspmelo already knows that. I haven't looked at the dashboard frontend, though.

@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2018

http://pulpito.ceph.com/kchai-2018-03-08_12:59:37-rados-wip-kefu-testing-2018-03-08-1932-distro-basic-smithi/

  • the swift failure was caused by #20419
  • and i reran osd-pool-create.sh multiple times locally, and was not able to reproduce that failure.
@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2018

the src/common/config.cc change looks sane to me.

@rjfd

rjfd approved these changes Mar 8, 2018

Copy link
Contributor

left a comment

lgtm

@s0nea

s0nea approved these changes Mar 8, 2018

@tchaikov tchaikov merged commit 0f93d68 into ceph:master Mar 9, 2018

5 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

epuertat added a commit to rhcs-dashboard/ceph that referenced this pull request Jul 17, 2018

Merge pull request ceph#20782 from sebastian-philipp/dashboard_v2_fix…
…_config_options_endpoint

mgr/dashboard_v2: Fix test_cluster_configuration test

Reviewed-by: Ricardo Dias <rdias@suse.com>
Reviewed-by: Tatjana Dehler <tdehler@suse.com>
Reviewed-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit 0f93d68)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>

epuertat added a commit to rhcs-dashboard/ceph that referenced this pull request Jul 24, 2018

Merge pull request ceph#20782 from sebastian-philipp/dashboard_v2_fix…
…_config_options_endpoint

mgr/dashboard_v2: Fix test_cluster_configuration test

Reviewed-by: Ricardo Dias <rdias@suse.com>
Reviewed-by: Tatjana Dehler <tdehler@suse.com>
Reviewed-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit 0f93d68)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>

epuertat added a commit to rhcs-dashboard/ceph that referenced this pull request Jul 26, 2018

Merge pull request ceph#20782 from sebastian-philipp/dashboard_v2_fix…
…_config_options_endpoint

mgr/dashboard_v2: Fix test_cluster_configuration test

Reviewed-by: Ricardo Dias <rdias@suse.com>
Reviewed-by: Tatjana Dehler <tdehler@suse.com>
Reviewed-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit 0f93d68)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>

epuertat added a commit to rhcs-dashboard/ceph that referenced this pull request Jul 26, 2018

Merge pull request ceph#20782 from sebastian-philipp/dashboard_v2_fix…
…_config_options_endpoint

mgr/dashboard_v2: Fix test_cluster_configuration test

Reviewed-by: Ricardo Dias <rdias@suse.com>
Reviewed-by: Tatjana Dehler <tdehler@suse.com>
Reviewed-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit 0f93d68)
Signed-off-by: Ernesto Puerta <epuertat@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.