From dbe3da97a1d5d91edf9d2b1ffa554f5701731a43 Mon Sep 17 00:00:00 2001 From: travisn Date: Wed, 8 May 2019 00:33:23 -0400 Subject: [PATCH 1/2] ceph: preserve the non-default port for hostnetworking Clusters using host networking will not work if the non-default port is being used by the mons. Previous to rook 1.0 the mons were all using the non-default port 6790 and now they are using the default port of 6789 from 1.0. Now host networking will preserve the non-default port to allow upgrades to continue working. Signed-off-by: travisn --- pkg/operator/ceph/cluster/mon/node_test.go | 30 ++++++++++++++++++++-- pkg/operator/ceph/cluster/mon/spec.go | 14 ++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/pkg/operator/ceph/cluster/mon/node_test.go b/pkg/operator/ceph/cluster/mon/node_test.go index 765550003..27b96bdfc 100644 --- a/pkg/operator/ceph/cluster/mon/node_test.go +++ b/pkg/operator/ceph/cluster/mon/node_test.go @@ -17,6 +17,7 @@ limitations under the License. package mon import ( + "strings" "testing" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" @@ -271,11 +272,36 @@ func TestHostNetwork(t *testing.T) { assert.Nil(t, err) assert.Equal(t, 3, len(nodes)) - pod := c.makeMonPod(testGenMonConfig("c"), nodes[2].Name) + monConfig := testGenMonConfig("c") + pod := c.makeMonPod(monConfig, nodes[2].Name) assert.NotNil(t, pod) - assert.Equal(t, true, pod.Spec.HostNetwork) assert.Equal(t, v1.DNSClusterFirstWithHostNet, pod.Spec.DNSPolicy) + val, message := extractArgValue(pod.Spec.Containers[0].Args, "--public-addr") + assert.Equal(t, "2.4.6.3", val, message) + val, message = extractArgValue(pod.Spec.Containers[0].Args, "--public-bind-addr") + assert.Equal(t, "$(ROOK_POD_IP)", val, message) + + monConfig.Port = 6790 + pod = c.makeMonPod(monConfig, nodes[2].Name) + val, message = extractArgValue(pod.Spec.Containers[0].Args, "--public-addr") + assert.Equal(t, "2.4.6.3:6790", val, message) + val, message = extractArgValue(pod.Spec.Containers[0].Args, "--public-bind-addr") + assert.Equal(t, "$(ROOK_POD_IP):6790", val, message) + assert.NotNil(t, pod) +} + +func extractArgValue(args []string, name string) (string, string) { + for _, arg := range args { + if strings.Contains(arg, name) { + vals := strings.Split(arg, "=") + if len(vals) != 2 { + return "", "cannot split arg: " + arg + } + return vals[1], "value: " + vals[1] + } + } + return "", "arg not found: " + name } func TestGetNodeInfoFromNode(t *testing.T) { diff --git a/pkg/operator/ceph/cluster/mon/spec.go b/pkg/operator/ceph/cluster/mon/spec.go index f26849186..394910df0 100644 --- a/pkg/operator/ceph/cluster/mon/spec.go +++ b/pkg/operator/ceph/cluster/mon/spec.go @@ -17,6 +17,7 @@ limitations under the License. package mon import ( + "fmt" "os" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" @@ -158,6 +159,15 @@ func (c *Cluster) makeMonFSInitContainer(monConfig *monConfig) v1.Container { func (c *Cluster) makeMonDaemonContainer(monConfig *monConfig) v1.Container { podIPEnvVar := "ROOK_POD_IP" + publicAddr := monConfig.PublicIP + publicBindAddr := opspec.ContainerEnvVarReference(podIPEnvVar) + if c.HostNetwork && monConfig.Port != DefaultMsgr1Port { + logger.Warningf("Starting mon %s with host networking on a non-default port %d. The mon must be failed over before enabling msgr2.", + monConfig.DaemonName, monConfig.Port) + publicAddr = fmt.Sprintf("%s:%d", publicAddr, monConfig.Port) + publicBindAddr = fmt.Sprintf("%s:%d", publicBindAddr, monConfig.Port) + } + container := v1.Container{ Name: "mon", Command: []string{ @@ -168,10 +178,10 @@ func (c *Cluster) makeMonDaemonContainer(monConfig *monConfig) v1.Container { "--foreground", // If the mon is already in the monmap, when the port is left off of --public-addr, // it will still advertise on the previous port b/c monmap is saved to mon database. - config.NewFlag("public-addr", monConfig.PublicIP), + config.NewFlag("public-addr", publicAddr), // Opposite of the above, --public-bind-addr will *not* still advertise on the previous // port, which makes sense because this is the pod IP, which changes with every new pod. - config.NewFlag("public-bind-addr", opspec.ContainerEnvVarReference(podIPEnvVar)), + config.NewFlag("public-bind-addr", publicBindAddr), ), Image: c.spec.CephVersion.Image, VolumeMounts: opspec.DaemonVolumeMounts(monConfig.DataPathMap, keyringStoreName), From cb82f36eba4f718f7b9f197760c2b0a4da2905de Mon Sep 17 00:00:00 2001 From: travisn Date: Fri, 10 May 2019 15:07:46 -0600 Subject: [PATCH 2/2] ceph: no need for --public-bind-addr with host networking The mons require the --public-bind-addr when host networking is not in use since the pod ip will be different from the service ip that will be advertised to clients. When host networking is enabled, there is no need for this argument since the endpoints will be the same to bind inside the pod as what is advertised to clients. Signed-off-by: travisn --- pkg/operator/ceph/cluster/mon/node_test.go | 5 ++--- pkg/operator/ceph/cluster/mon/spec.go | 16 +++++++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/pkg/operator/ceph/cluster/mon/node_test.go b/pkg/operator/ceph/cluster/mon/node_test.go index 27b96bdfc..601786fdc 100644 --- a/pkg/operator/ceph/cluster/mon/node_test.go +++ b/pkg/operator/ceph/cluster/mon/node_test.go @@ -280,14 +280,13 @@ func TestHostNetwork(t *testing.T) { val, message := extractArgValue(pod.Spec.Containers[0].Args, "--public-addr") assert.Equal(t, "2.4.6.3", val, message) val, message = extractArgValue(pod.Spec.Containers[0].Args, "--public-bind-addr") - assert.Equal(t, "$(ROOK_POD_IP)", val, message) + assert.Equal(t, "", val) + assert.Equal(t, "arg not found: --public-bind-addr", message) monConfig.Port = 6790 pod = c.makeMonPod(monConfig, nodes[2].Name) val, message = extractArgValue(pod.Spec.Containers[0].Args, "--public-addr") assert.Equal(t, "2.4.6.3:6790", val, message) - val, message = extractArgValue(pod.Spec.Containers[0].Args, "--public-bind-addr") - assert.Equal(t, "$(ROOK_POD_IP):6790", val, message) assert.NotNil(t, pod) } diff --git a/pkg/operator/ceph/cluster/mon/spec.go b/pkg/operator/ceph/cluster/mon/spec.go index 394910df0..e12fc8b40 100644 --- a/pkg/operator/ceph/cluster/mon/spec.go +++ b/pkg/operator/ceph/cluster/mon/spec.go @@ -160,12 +160,13 @@ func (c *Cluster) makeMonFSInitContainer(monConfig *monConfig) v1.Container { func (c *Cluster) makeMonDaemonContainer(monConfig *monConfig) v1.Container { podIPEnvVar := "ROOK_POD_IP" publicAddr := monConfig.PublicIP - publicBindAddr := opspec.ContainerEnvVarReference(podIPEnvVar) + + // Handle the non-default port for host networking. If host networking is not being used, + // the service created elsewhere will handle the non-default port redirection to the default port inside the container. if c.HostNetwork && monConfig.Port != DefaultMsgr1Port { logger.Warningf("Starting mon %s with host networking on a non-default port %d. The mon must be failed over before enabling msgr2.", monConfig.DaemonName, monConfig.Port) publicAddr = fmt.Sprintf("%s:%d", publicAddr, monConfig.Port) - publicBindAddr = fmt.Sprintf("%s:%d", publicBindAddr, monConfig.Port) } container := v1.Container{ @@ -179,9 +180,6 @@ func (c *Cluster) makeMonDaemonContainer(monConfig *monConfig) v1.Container { // If the mon is already in the monmap, when the port is left off of --public-addr, // it will still advertise on the previous port b/c monmap is saved to mon database. config.NewFlag("public-addr", publicAddr), - // Opposite of the above, --public-bind-addr will *not* still advertise on the previous - // port, which makes sense because this is the pod IP, which changes with every new pod. - config.NewFlag("public-bind-addr", publicBindAddr), ), Image: c.spec.CephVersion.Image, VolumeMounts: opspec.DaemonVolumeMounts(monConfig.DataPathMap, keyringStoreName), @@ -200,6 +198,14 @@ func (c *Cluster) makeMonDaemonContainer(monConfig *monConfig) v1.Container { Resources: cephv1.GetMonResources(c.spec.Resources), } + // If host networking is enabled, we don't need a bind addr that is different from the public addr + if !c.HostNetwork { + // Opposite of the above, --public-bind-addr will *not* still advertise on the previous + // port, which makes sense because this is the pod IP, which changes with every new pod. + container.Args = append(container.Args, + config.NewFlag("public-bind-addr", opspec.ContainerEnvVarReference(podIPEnvVar))) + } + // If deploying Nautilus and newer we need a new port of the monitor container if c.clusterInfo.CephVersion.IsAtLeastNautilus() { addContainerPort(container, "msgr2", 3300)