diff --git a/pkg/operator/ceph/cluster/mon/spec.go b/pkg/operator/ceph/cluster/mon/spec.go index 7d5f00e77..140657f4d 100644 --- a/pkg/operator/ceph/cluster/mon/spec.go +++ b/pkg/operator/ceph/cluster/mon/spec.go @@ -327,8 +327,22 @@ func (c *Cluster) makeMonDaemonContainer(monConfig *monConfig) corev1.Container bindaddr := controller.ContainerEnvVarReference(podIPEnvVar) if monConfig.Port == DefaultMsgr2Port { container.Args = append(container.Args, config.NewFlag("ms_bind_msgr1", "false")) - // To avoid binding to the msgr1 port, ceph expects to find the msgr2 port on the bind address - bindaddr = fmt.Sprintf("%s:%d", bindaddr, DefaultMsgr2Port) + + // mons don't use --ms-bind-msgr1 to control whether they bind to v1 port or not. + // in order to force use of only v2 port, Rook must include the port in the bind addr + if c.spec.Network.DualStack { + // in a dual stack environment, Rook can't know whether IPv4 or IPv6 will be used. + // in order to be safe, don't add the port to the bind addr. this will mean that mons + // might listen on both msgr1 and msgr2 ports, but it is more critical to make sure mons + // don't crash than to forcefully disable msgr1 + } else if c.spec.Network.IPFamily == cephv1.IPv6 { + // IPv6 addrs have to be surrounded in square brackets when a port is given + bindaddr = fmt.Sprintf("[%s]:%d", bindaddr, DefaultMsgr2Port) + } else if c.spec.Network.IPFamily == cephv1.IPv4 || c.spec.Network.IPFamily == "" { + // IPv4 addrs must have the port added without any special syntax + // if the IP family is unset, IPv4 is a safe assumption + bindaddr = fmt.Sprintf("%s:%d", bindaddr, DefaultMsgr2Port) + } } else { // Add messenger 1 port container.Ports = append(container.Ports, corev1.ContainerPort{ diff --git a/pkg/operator/ceph/cluster/mon/spec_test.go b/pkg/operator/ceph/cluster/mon/spec_test.go index 5ab9d35ac..a4d666429 100644 --- a/pkg/operator/ceph/cluster/mon/spec_test.go +++ b/pkg/operator/ceph/cluster/mon/spec_test.go @@ -18,6 +18,7 @@ package mon import ( "context" + "strings" "testing" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" @@ -105,29 +106,72 @@ func testPodSpec(t *testing.T, monID string, pvc bool) { t.Run(("msgr2 not required"), func(t *testing.T) { container := c.makeMonDaemonContainer(monConfig) - checkMsgr2Required(t, container, false) + checkMsgr2Required(t, container, false, false, false) }) t.Run(("require msgr2"), func(t *testing.T) { monConfig.Port = DefaultMsgr2Port container := c.makeMonDaemonContainer(monConfig) - checkMsgr2Required(t, container, true) + checkMsgr2Required(t, container, true, true, false) + }) + + t.Run(("require msgr2 -- dual stack"), func(t *testing.T) { + monConfig.Port = DefaultMsgr2Port + c.spec.Network = cephv1.NetworkSpec{ + DualStack: true, + } + monConfig.Port = DefaultMsgr2Port + container := c.makeMonDaemonContainer(monConfig) + checkMsgr2Required(t, container, true, false, false) + }) + + t.Run(("require msgr2 -- IPv4"), func(t *testing.T) { + monConfig.Port = DefaultMsgr2Port + c.spec.Network = cephv1.NetworkSpec{ + DualStack: false, + IPFamily: cephv1.IPv4, + } + monConfig.Port = DefaultMsgr2Port + container := c.makeMonDaemonContainer(monConfig) + checkMsgr2Required(t, container, true, true, false) + }) + + t.Run(("require msgr2 -- IPv6"), func(t *testing.T) { + monConfig.Port = DefaultMsgr2Port + c.spec.Network = cephv1.NetworkSpec{ + DualStack: false, + IPFamily: cephv1.IPv6, + } + monConfig.Port = DefaultMsgr2Port + container := c.makeMonDaemonContainer(monConfig) + checkMsgr2Required(t, container, true, true, true) }) } -func checkMsgr2Required(t *testing.T, container v1.Container, expectedRequireMsgr2 bool) { +func checkMsgr2Required(t *testing.T, container v1.Container, expectedRequireMsgr2, expectedPort, expectedBrackets bool) { foundDisabledMsgr1 := false foundMsgr2Port := false + foundBrackets := false + for _, arg := range container.Args { if arg == "--ms-bind-msgr1=false" { foundDisabledMsgr1 = true } - if arg == "--public-bind-addr=$(ROOK_POD_IP):3300" { - foundMsgr2Port = true + if strings.HasPrefix(arg, "--public-bind-addr=") { + // flag should always refer to the env var, no matter what + assert.Contains(t, arg, "$(ROOK_POD_IP)") + if strings.HasSuffix(arg, ":3300") { + foundMsgr2Port = true + } + // brackets are expected for IPv6 addrs + if strings.Contains(arg, "[$(ROOK_POD_IP)]") { + foundBrackets = true + } } } assert.Equal(t, expectedRequireMsgr2, foundDisabledMsgr1) - assert.Equal(t, expectedRequireMsgr2, foundMsgr2Port) + assert.Equal(t, expectedPort, foundMsgr2Port) + assert.Equal(t, expectedBrackets, foundBrackets) } func TestDeploymentPVCSpec(t *testing.T) {