From 0be6fdfa2d28a78347075c06975911f74e9a87fe Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Wed, 24 Jun 2020 15:25:44 -0600 Subject: [PATCH 1/2] ceph: save mon config even if failed to remove dead mon During mon failover if the mon fails to be removed, the new mon config was not being saved. The mon removal anyway is not retried if it fails, so it should make a best effort to remove all the resources related to the mon, and then always update the mon config after the failover. Signed-off-by: Travis Nielsen --- pkg/operator/ceph/cluster/mon/health.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/operator/ceph/cluster/mon/health.go b/pkg/operator/ceph/cluster/mon/health.go index f48f7d511..ea9446294 100644 --- a/pkg/operator/ceph/cluster/mon/health.go +++ b/pkg/operator/ceph/cluster/mon/health.go @@ -275,6 +275,7 @@ func (c *Cluster) failoverMon(name string) error { return c.removeMon(name) } +// make a best effort to remove the mon and all its resources func (c *Cluster) removeMon(daemonName string) error { logger.Infof("ensuring removal of unhealthy monitor %s", daemonName) @@ -288,13 +289,13 @@ func (c *Cluster) removeMon(daemonName string) error { if kerrors.IsNotFound(err) { logger.Infof("dead mon %s was already gone", resourceName) } else { - return errors.Wrapf(err, "failed to remove dead mon deployment %s", resourceName) + logger.Errorf("failed to remove dead mon deployment %q. %v", resourceName, err) } } // Remove the bad monitor from quorum if err := removeMonitorFromQuorum(c.context, c.ClusterInfo.Name, daemonName); err != nil { - return errors.Wrapf(err, "failed to remove mon %s from quorum", daemonName) + logger.Errorf("failed to remove mon %q from quorum. %v", daemonName, err) } delete(c.ClusterInfo.Monitors, daemonName) // check if a mapping exists for the mon @@ -307,7 +308,7 @@ func (c *Cluster) removeMon(daemonName string) error { if kerrors.IsNotFound(err) { logger.Infof("dead mon service %s was already gone", resourceName) } else { - return errors.Wrapf(err, "failed to remove dead mon service %s", resourceName) + logger.Errorf("failed to remove dead mon service %q. %v", resourceName, err) } } From 1d370b1314a1b9c28ec90b788b136c03bd9bdc40 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Wed, 24 Jun 2020 15:27:08 -0600 Subject: [PATCH 2/2] ceph: during mon failover remove the backing pvc If the mon is backed by a pvc, ensure the pvc is deleted when the mon is failed over. The pvc is being left in a bound state even though it will no longer be used in the cluster. Signed-off-by: Travis Nielsen --- pkg/operator/ceph/cluster/mon/health.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/operator/ceph/cluster/mon/health.go b/pkg/operator/ceph/cluster/mon/health.go index ea9446294..d24a565b7 100644 --- a/pkg/operator/ceph/cluster/mon/health.go +++ b/pkg/operator/ceph/cluster/mon/health.go @@ -312,6 +312,15 @@ func (c *Cluster) removeMon(daemonName string) error { } } + // Remove the PVC backing the mon if it existed + if err := c.context.Clientset.CoreV1().PersistentVolumeClaims(c.Namespace).Delete(resourceName, &metav1.DeleteOptions{}); err != nil { + if kerrors.IsNotFound(err) { + logger.Infof("mon pvc did not exist %q", resourceName) + } else { + logger.Errorf("failed to remove dead mon pvc %q. %v", resourceName, err) + } + } + if err := c.saveMonConfig(); err != nil { return errors.Wrapf(err, "failed to save mon config after failing over mon %s", daemonName) }