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: fixing dashboard configuration handling #13604

Merged
merged 2 commits into from Jan 26, 2024

Conversation

rkachach
Copy link
Contributor

@rkachach rkachach commented Jan 22, 2024

previously, the dashboard parameters supported by Rook were stored in the daemon configuration section (mgr.X, for example). This differs from Cephadm-based deployments, where all configurations are stored in the global mgr configuration section. This variance could result in configuration mismatches between the active and standby dashboards. Furthermore, all Ceph dashboard documentation exclusively points to the global mgr configuration section and makes no use of individual daemons sections.

Fixes: #13577

Upgrade test

Starting from previous vesion (image: rook/ceph:master) and using the following configuration:

  mgr:
    count: 2
    allowMultiplePerNode: true
  dashboard:
    enabled: true
    port: 7777
    ssl: false
    prometheusEndpoint: "http://192.168.39.54:30900"
    prometheusEndpointSSLVerify: false

Let's get the status of the config once the cluster is up&running:

bash-4.4$ ceph config get mgr.a
WHO     MASK  LEVEL     OPTION                                   VALUE                       RO
mgr.a         advanced  mgr/dashboard/PROMETHEUS_API_HOST        http://192.168.39.54:30900  * 
mgr.a         advanced  mgr/dashboard/PROMETHEUS_API_SSL_VERIFY  false                       * 
mgr.a         advanced  mgr/dashboard/server_port                7777                        * 
mgr.a         advanced  mgr/dashboard/ssl                        false                       * 
mgr           advanced  mgr/orchestrator/orchestrator            rook                          
mgr           advanced  mgr/prometheus/rbd_stats_pools                                       * 

bash-4.4$ ceph config get mgr.b
WHO     MASK  LEVEL     OPTION                                   VALUE                       RO
mgr.b         advanced  mgr/dashboard/PROMETHEUS_API_HOST        http://192.168.39.54:30900  * 
mgr.b         advanced  mgr/dashboard/PROMETHEUS_API_SSL_VERIFY  false                       * 
mgr.b         advanced  mgr/dashboard/server_port                7777                        * 
mgr.b         advanced  mgr/dashboard/ssl                        false                       * 
mgr           advanced  mgr/orchestrator/orchestrator            rook                          
mgr           advanced  mgr/prometheus/rbd_stats_pools                                       * 

bash-4.4$ ceph config get mgr  
WHO     MASK  LEVEL     OPTION                             VALUE                        RO
mgr           advanced  mgr/dashboard/PROMETHEUS_API_HOST  http://192.168.39.215:30900  * 
mgr           advanced  mgr/orchestrator/orchestrator      rook                           
mgr           advanced  mgr/prometheus/rbd_stats_pools                                  * 
bash-4.4$ 

Mgr daemons configuration after the upgrade (no specific per-daemon configuration anymore). The configuration related to ssl now is part of the main mgr section.

bash-4.4$ ceph config get mgr.a
WHO     MASK  LEVEL     OPTION                                   VALUE                       RO
mgr           advanced  mgr/dashboard/PROMETHEUS_API_HOST        http://192.168.39.54:30900  * 
mgr           advanced  mgr/dashboard/PROMETHEUS_API_SSL_VERIFY  false                       * 
mgr           advanced  mgr/dashboard/server_port                7777                        * 
mgr           advanced  mgr/dashboard/ssl                        false                       * 
mgr           advanced  mgr/orchestrator/orchestrator            rook                          
mgr           advanced  mgr/prometheus/rbd_stats_pools                                       * 

bash-4.4$ 
bash-4.4$ ceph config get mgr.b
WHO     MASK  LEVEL     OPTION                                   VALUE                       RO
mgr           advanced  mgr/dashboard/PROMETHEUS_API_HOST        http://192.168.39.54:30900  * 
mgr           advanced  mgr/dashboard/PROMETHEUS_API_SSL_VERIFY  false                       * 
mgr           advanced  mgr/dashboard/server_port                7777                        * 
mgr           advanced  mgr/dashboard/ssl                        false                       * 
mgr           advanced  mgr/orchestrator/orchestrator            rook                          
mgr           advanced  mgr/prometheus/rbd_stats_pools                                       * 
bash-4.4$ 

bash-4.4$ ceph config get mgr  
WHO     MASK  LEVEL     OPTION                                   VALUE                       RO
mgr           advanced  mgr/dashboard/PROMETHEUS_API_HOST        http://192.168.39.54:30900  * 
mgr           advanced  mgr/dashboard/PROMETHEUS_API_SSL_VERIFY  false                       * 
mgr           advanced  mgr/dashboard/server_port                7777                        * 
mgr           advanced  mgr/dashboard/ssl                        false                       * 
mgr           advanced  mgr/orchestrator/orchestrator            rook                          
mgr           advanced  mgr/prometheus/rbd_stats_pools                                       * 

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

Copy link
Contributor

@jmolmo jmolmo left a comment

Choose a reason for hiding this comment

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

Good catch!!! LGTM.
Just review we are not "getting" dashboard options in other operator places.

@rkachach rkachach requested a review from travisn January 23, 2024 09:25
@rkachach rkachach changed the title [WIP] mgr: fixing dashboard configuration handling mgr: fixing dashboard configuration handling Jan 23, 2024
@rkachach rkachach marked this pull request as ready for review January 23, 2024 09:56
pkg/operator/ceph/cluster/mgr/dashboard.go Outdated Show resolved Hide resolved
pkg/operator/ceph/cluster/mgr/dashboard.go Outdated Show resolved Hide resolved
@rkachach rkachach force-pushed the fix_issue_13577 branch 2 times, most recently from 167c5f3 to f0d638b Compare January 23, 2024 15:26
@rkachach rkachach requested a review from travisn January 23, 2024 15:27
@parth-gr
Copy link
Member

bash-4.4$ ceph config get mgr  
WHO     MASK  LEVEL     OPTION                                   VALUE                       RO
mgr           advanced  mgr/dashboard/PROMETHEUS_API_HOST        http://192.168.39.54:30900  * 
mgr           advanced  mgr/dashboard/PROMETHEUS_API_SSL_VERIFY  false                       * 
mgr           advanced  mgr/dashboard/server_port                7777                        * 
mgr           advanced  mgr/dashboard/ssl                        false                       * 
mgr           advanced  mgr/orchestrator/orchestrator            rook                          
mgr           advanced  mgr/prometheus/rbd_stats_pools                                       * 

This is the active mgr configuration?

@rkachach
Copy link
Contributor Author

bash-4.4$ ceph config get mgr  
WHO     MASK  LEVEL     OPTION                                   VALUE                       RO
mgr           advanced  mgr/dashboard/PROMETHEUS_API_HOST        http://192.168.39.54:30900  * 
mgr           advanced  mgr/dashboard/PROMETHEUS_API_SSL_VERIFY  false                       * 
mgr           advanced  mgr/dashboard/server_port                7777                        * 
mgr           advanced  mgr/dashboard/ssl                        false                       * 
mgr           advanced  mgr/orchestrator/orchestrator            rook                          
mgr           advanced  mgr/prometheus/rbd_stats_pools                                       * 

This is the active mgr configuration?

This the general mgr configuration and it affects to ALL the mgr daemons. Previously we were having the config per-daemon which could lead to some inconsistencies in the conf and is different from what cephadm based deployments (where all the config is at manager level)

@rkachach rkachach requested a review from travisn January 24, 2024 17:57
previously, the dashboard parameters supported by Rook were stored in the
daemon configuration section (mgr.X, for example). This differs from
Cephadm-based deployments, where all configurations are stored in the
global mgr configuration section. This variance could result in
configuration mismatches between the active and standby dashboards.
Furthermore, all Ceph dashboard documentation exclusively points to
the global mgr configuration section and makes no use of individual
daemons sections.

Fixes: rook#13577

Signed-off-by: Redouane Kachach <rkachach@redhat.com>
we dont set removeMgrDaemonConfiguration to true unless all the
delete operations went successfull.

Signed-off-by: Redouane Kachach <rkachach@redhat.com>
@travisn travisn merged commit f85ba88 into rook:master Jan 26, 2024
49 of 50 checks passed
travisn added a commit that referenced this pull request Jan 26, 2024
mgr: fixing dashboard configuration handling (backport #13604)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants