From 57e3937d405b9c4ed16bd571e228d0bc09bf6487 Mon Sep 17 00:00:00 2001 From: Redouane Kachach Date: Mon, 22 Jan 2024 13:21:25 +0100 Subject: [PATCH 1/2] mgr: fixing dashboard configuration handling 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: https://github.com/rook/rook/issues/13577 Signed-off-by: Redouane Kachach --- pkg/operator/ceph/cluster/mgr/dashboard.go | 47 +++++++++++++--------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/pkg/operator/ceph/cluster/mgr/dashboard.go b/pkg/operator/ceph/cluster/mgr/dashboard.go index ead1a152b..d34f5a79b 100644 --- a/pkg/operator/ceph/cluster/mgr/dashboard.go +++ b/pkg/operator/ceph/cluster/mgr/dashboard.go @@ -51,7 +51,8 @@ const ( ) var ( - dashboardInitWaitTime = 5 * time.Second + dashboardInitWaitTime = 5 * time.Second + removeMgrDaemonConfiguration = true ) func (c *Cluster) configureDashboardService() error { @@ -93,15 +94,9 @@ func (c *Cluster) configureDashboardModules() error { return errors.Wrap(err, "failed to initialize dashboard") } - var hasChanged bool - for _, daemonID := range c.getDaemonIDs() { - changed, err := c.configureDashboardModuleSettings(daemonID) - if err != nil { - return err - } - if changed { - hasChanged = true - } + hasChanged, err := c.configureDashboardModuleSettings() + if err != nil { + return err } if hasChanged { logger.Info("dashboard config has changed. restarting the dashboard module") @@ -110,20 +105,19 @@ func (c *Cluster) configureDashboardModules() error { return nil } -func (c *Cluster) configureDashboardModuleSettings(daemonID string) (bool, error) { +func (c *Cluster) configureDashboardModuleSettings() (bool, error) { + monStore := config.GetMonStore(c.context, c.clusterInfo) - daemonID = fmt.Sprintf("mgr.%s", daemonID) - // url prefix - hasChanged, err := monStore.SetIfChanged(daemonID, "mgr/dashboard/url_prefix", c.spec.Dashboard.URLPrefix) + hasChanged, err := monStore.SetIfChanged(config.MgrType, "mgr/dashboard/url_prefix", c.spec.Dashboard.URLPrefix) if err != nil { return false, err } // ssl support ssl := strconv.FormatBool(c.spec.Dashboard.SSL) - changed, err := monStore.SetIfChanged(daemonID, "mgr/dashboard/ssl", ssl) + changed, err := monStore.SetIfChanged(config.MgrType, "mgr/dashboard/ssl", ssl) if err != nil { return false, err } @@ -131,7 +125,7 @@ func (c *Cluster) configureDashboardModuleSettings(daemonID string) (bool, error // Prometheus host end point prometheusEndpoint := c.spec.Dashboard.PrometheusEndpoint - changed, err = monStore.SetIfChanged(daemonID, "mgr/dashboard/PROMETHEUS_API_HOST", prometheusEndpoint) + changed, err = monStore.SetIfChanged(config.MgrType, "mgr/dashboard/PROMETHEUS_API_HOST", prometheusEndpoint) if err != nil { return false, err } @@ -139,7 +133,7 @@ func (c *Cluster) configureDashboardModuleSettings(daemonID string) (bool, error // Prometheus host end point ssl verify prometheusEndpointSSLVerify := strconv.FormatBool(c.spec.Dashboard.PrometheusEndpointSSLVerify) - changed, err = monStore.SetIfChanged(daemonID, "mgr/dashboard/PROMETHEUS_API_SSL_VERIFY", prometheusEndpointSSLVerify) + changed, err = monStore.SetIfChanged(config.MgrType, "mgr/dashboard/PROMETHEUS_API_SSL_VERIFY", prometheusEndpointSSLVerify) if err != nil { return false, err } @@ -147,7 +141,7 @@ func (c *Cluster) configureDashboardModuleSettings(daemonID string) (bool, error // server port port := strconv.Itoa(c.dashboardInternalPort()) - changed, err = monStore.SetIfChanged(daemonID, "mgr/dashboard/server_port", port) + changed, err = monStore.SetIfChanged(config.MgrType, "mgr/dashboard/server_port", port) if err != nil { return false, err } @@ -155,13 +149,28 @@ func (c *Cluster) configureDashboardModuleSettings(daemonID string) (bool, error // SSL enabled. Needed to set specifically the ssl port setting if c.spec.Dashboard.SSL { - changed, err = monStore.SetIfChanged(daemonID, "mgr/dashboard/ssl_server_port", port) + changed, err = monStore.SetIfChanged(config.MgrType, "mgr/dashboard/ssl_server_port", port) if err != nil { return false, err } hasChanged = hasChanged || changed } + // Remove any existing per mgr-daemon configuration + if removeMgrDaemonConfiguration { + logger.Info("Removing any previous per mgr-daemon configuration") + for _, daemonID := range c.getDaemonIDs() { + mgrDaemonID := fmt.Sprintf("%s.%s", config.MgrType, daemonID) + monStore.Delete(mgrDaemonID, "mgr/dashboard/url_prefix") + monStore.Delete(mgrDaemonID, "mgr/dashboard/ssl") + monStore.Delete(mgrDaemonID, "mgr/dashboard/PROMETHEUS_API_HOST") + monStore.Delete(mgrDaemonID, "mgr/dashboard/PROMETHEUS_API_SSL_VERIFY") + monStore.Delete(mgrDaemonID, "mgr/dashboard/server_port") + monStore.Delete(mgrDaemonID, "mgr/dashboard/ssl_server_port") + } + removeMgrDaemonConfiguration = false + } + return hasChanged, nil } From 39c06d2fa5ba98b5515cbff26206143deff70007 Mon Sep 17 00:00:00 2001 From: Redouane Kachach Date: Wed, 24 Jan 2024 13:01:35 +0100 Subject: [PATCH 2/2] mgr: adding logic to keep retrying in case mgr config delete fails we dont set removeMgrDaemonConfiguration to true unless all the delete operations went successfull. Signed-off-by: Redouane Kachach --- pkg/operator/ceph/cluster/mgr/dashboard.go | 47 +++++++++++++++++----- pkg/operator/ceph/config/monstore.go | 2 +- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/pkg/operator/ceph/cluster/mgr/dashboard.go b/pkg/operator/ceph/cluster/mgr/dashboard.go index d34f5a79b..a5c90120c 100644 --- a/pkg/operator/ceph/cluster/mgr/dashboard.go +++ b/pkg/operator/ceph/cluster/mgr/dashboard.go @@ -105,6 +105,41 @@ func (c *Cluster) configureDashboardModules() error { return nil } +// Delete the manager per-daemon configuration. Returns true +// if all the configuration entries have been delete successfully. +func (c *Cluster) deleteManagerDaemonConfiguration() bool { + + mgrKeysToDelete := []string{ + "mgr/dashboard/url_prefix", + "mgr/dashboard/ssl", + "mgr/dashboard/PROMETHEUS_API_HOST", + "mgr/dashboard/PROMETHEUS_API_SSL_VERIFY", + "mgr/dashboard/server_port", + "mgr/dashboard/ssl_server_port", + } + + success := true + monStore := config.GetMonStore(c.context, c.clusterInfo) + for _, daemonID := range c.getDaemonIDs() { + mgrDaemonID := fmt.Sprintf("%s.%s", config.MgrType, daemonID) + for _, key := range mgrKeysToDelete { + err := monStore.Delete(mgrDaemonID, key) + if err != nil { + logger.Errorf("failed to delete configuration entry %q %q, err: %v", mgrDaemonID, key, err) + success = false + } + } + } + + if success { + logger.Info("All per-daemon mgr configuration has been deleted successfully.") + } else { + logger.Error("At least one delete operation failed while trying to delete per-daemon mgr configuration.") + } + + return success +} + func (c *Cluster) configureDashboardModuleSettings() (bool, error) { monStore := config.GetMonStore(c.context, c.clusterInfo) @@ -158,17 +193,7 @@ func (c *Cluster) configureDashboardModuleSettings() (bool, error) { // Remove any existing per mgr-daemon configuration if removeMgrDaemonConfiguration { - logger.Info("Removing any previous per mgr-daemon configuration") - for _, daemonID := range c.getDaemonIDs() { - mgrDaemonID := fmt.Sprintf("%s.%s", config.MgrType, daemonID) - monStore.Delete(mgrDaemonID, "mgr/dashboard/url_prefix") - monStore.Delete(mgrDaemonID, "mgr/dashboard/ssl") - monStore.Delete(mgrDaemonID, "mgr/dashboard/PROMETHEUS_API_HOST") - monStore.Delete(mgrDaemonID, "mgr/dashboard/PROMETHEUS_API_SSL_VERIFY") - monStore.Delete(mgrDaemonID, "mgr/dashboard/server_port") - monStore.Delete(mgrDaemonID, "mgr/dashboard/ssl_server_port") - } - removeMgrDaemonConfiguration = false + removeMgrDaemonConfiguration = !c.deleteManagerDaemonConfiguration() } return hasChanged, nil diff --git a/pkg/operator/ceph/config/monstore.go b/pkg/operator/ceph/config/monstore.go index 63c475e13..01c4eac44 100644 --- a/pkg/operator/ceph/config/monstore.go +++ b/pkg/operator/ceph/config/monstore.go @@ -108,7 +108,7 @@ func (m *MonStore) Set(who, option, value string) error { // Delete a config in the centralized mon configuration database. func (m *MonStore) Delete(who, option string) error { - logger.Infof("deleting %q option from the mon configuration database", option) + logger.Infof("deleting %q %q option from the mon configuration database", who, option) args := []string{"config", "rm", who, normalizeKey(option)} cephCmd := client.NewCephCommand(m.context, m.clusterInfo, args) out, err := cephCmd.RunWithTimeout(exec.CephCommandsTimeout)