diff --git a/pkg/operator/ceph/cluster/mon/health.go b/pkg/operator/ceph/cluster/mon/health.go index fe1042ae8..44142ac55 100644 --- a/pkg/operator/ceph/cluster/mon/health.go +++ b/pkg/operator/ceph/cluster/mon/health.go @@ -372,10 +372,10 @@ func (c *Cluster) checkHealth(ctx context.Context) error { // failover any mons present in the mon fail over list for _, mon := range c.ClusterInfo.InternalMonitors { - if c.monsToFailover.Has(mon.Name) { + if _, ok := c.monsToFailover[mon.Name]; ok { logger.Infof("fail over mon %q from the mon fail over list", mon.Name) c.failMon(len(c.ClusterInfo.InternalMonitors), desiredMonCount, mon.Name) - c.monsToFailover.Delete(mon.Name) + delete(c.monsToFailover, mon.Name) return nil } } @@ -710,9 +710,11 @@ func (c *Cluster) failoverMon(name string) error { logger.Infof("starting new mon: %+v", m) // Scale down the failed mon to allow a new one to start - if err := c.updateMonDeploymentReplica(name, false); err != nil { - // attempt to continue with the failover even if the bad mon could not be stopped - logger.Warningf("failed to stop mon %q for failover. %v", name, err) + if c.stopMonDuringFailover(name) { + if err := c.updateMonDeploymentReplica(name, false); err != nil { + // attempt to continue with the failover even if the bad mon could not be stopped + logger.Warningf("failed to stop mon %q for failover. %v", name, err) + } } // If the mon failover is not successful, revert the failover @@ -794,6 +796,24 @@ func (c *Cluster) failoverMon(name string) error { return c.removeMon(name) } +func (c *Cluster) stopMonDuringFailover(name string) bool { + if !c.spec.Network.IsHost() { + return true + } + + // If the mon is using host networking, we don't want to stop the mon + // since we don't want to schedule a new mon with the same host ip. + // With host networking, when failing over the mon, we either require + // another eligible node without a mon, or we expect the down mon to come back online. + // But if the host networking settings are being enabled, we can allow the failover from a + // non-host networking mon. + if mon, ok := c.monsToFailover[name]; ok && !mon.UseHostNetwork { + return true + } + logger.Infof("skipping stopping mon %q during failover, since with host networking a new mon cannot be started on the same node", name) + return false +} + // make a best effort to remove the mon and all its resources func (c *Cluster) removeMon(daemonName string) error { return c.removeMonWithOptionalQuorum(daemonName, true) diff --git a/pkg/operator/ceph/cluster/mon/health_test.go b/pkg/operator/ceph/cluster/mon/health_test.go index 10196d129..3e462a225 100644 --- a/pkg/operator/ceph/cluster/mon/health_test.go +++ b/pkg/operator/ceph/cluster/mon/health_test.go @@ -300,6 +300,34 @@ func TestEvictMonOnSameNode(t *testing.T) { assert.Equal(t, 3, c.maxMonID) } +func TestHostNetworkFailover(t *testing.T) { + ctx := context.TODO() + context := &clusterd.Context{} + ownerInfo := cephclient.NewMinimumOwnerInfoWithOwnerRef() + c := New(ctx, context, "ns", cephv1.ClusterSpec{}, ownerInfo) + + t.Run("should stop mon on default network", func(t *testing.T) { + assert.True(t, c.stopMonDuringFailover("a")) + }) + + t.Run("should not stop mon on host network", func(t *testing.T) { + c.spec.Network.Provider = "host" + assert.False(t, c.stopMonDuringFailover("a")) + }) + + t.Run("should stop mon converting to host network", func(t *testing.T) { + c.spec.Network.Provider = "host" + c.monsToFailover["a"] = &monConfig{UseHostNetwork: false} + assert.True(t, c.stopMonDuringFailover("a")) + }) + + t.Run("should stop mon converting from host network", func(t *testing.T) { + c.spec.Network.Provider = "" + c.monsToFailover["a"] = &monConfig{UseHostNetwork: true} + assert.True(t, c.stopMonDuringFailover("a")) + }) +} + func createTestMonPod(t *testing.T, clientset kubernetes.Interface, c *Cluster, name, node string) { m := &monConfig{ResourceName: resourceName(name), DaemonName: name, DataPathMap: &config.DataPathMap{}} d, err := c.makeDeployment(m, false) diff --git a/pkg/operator/ceph/cluster/mon/mon.go b/pkg/operator/ceph/cluster/mon/mon.go index b16941627..bc43bb8d2 100644 --- a/pkg/operator/ceph/cluster/mon/mon.go +++ b/pkg/operator/ceph/cluster/mon/mon.go @@ -132,7 +132,7 @@ type Cluster struct { isUpgrade bool arbiterMon string // list of mons to be failed over - monsToFailover sets.Set[string] + monsToFailover map[string]*monConfig } // monConfig for a single monitor @@ -180,7 +180,7 @@ func New(ctx context.Context, clusterdContext *clusterd.Context, namespace strin ClusterInfo: &cephclient.ClusterInfo{ Context: ctx, }, - monsToFailover: sets.New[string](), + monsToFailover: map[string]*monConfig{}, } } @@ -1557,13 +1557,13 @@ func (c *Cluster) startMon(m *monConfig, schedule *controller.MonScheduleInfo) e if deploymentExists { // skip update if mon path has changed if hasMonPathChanged(existingDeployment, c.spec.Mon.VolumeClaimTemplate.ToPVC()) { - c.monsToFailover.Insert(m.DaemonName) + c.monsToFailover[m.DaemonName] = m return nil } // skip update if mon fail over is required due to change in hostnetwork settings if isMonIPUpdateRequiredForHostNetwork(m.DaemonName, m.UseHostNetwork, &c.spec.Network) { - c.monsToFailover.Insert(m.DaemonName) + c.monsToFailover[m.DaemonName] = m return nil } diff --git a/pkg/operator/ceph/cluster/mon/mon_test.go b/pkg/operator/ceph/cluster/mon/mon_test.go index 98b6d8d77..6484d2211 100644 --- a/pkg/operator/ceph/cluster/mon/mon_test.go +++ b/pkg/operator/ceph/cluster/mon/mon_test.go @@ -45,7 +45,6 @@ import ( discoveryv1 "k8s.io/api/discovery/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/ptr" ) @@ -124,7 +123,7 @@ func newCluster(context *clusterd.Context, namespace string, allowMultiplePerNod Schedule: map[string]*opcontroller.MonScheduleInfo{}, }, ownerInfo: ownerInfo, - monsToFailover: sets.New[string](), + monsToFailover: map[string]*monConfig{}, } }