From 332bac5ca2ed2003b20d224921cb02c5c5fa589a Mon Sep 17 00:00:00 2001
From: Lucas Henry <polyedre@disroot.org>
Date: Mon, 5 Jun 2023 15:44:24 +0200
Subject: [PATCH] rgw: add option to disable synchronization traffic

Some users want to deploy two CephObjectStores for a single Zone. The first
configures RGWs to process the synchronization of the data, while the second
CephObjectStore configures the client RGWs.

Currently, this can be implemented by setting the RGW option
'rgw_run_sync_thread' in the 'rook-config-override' ConfigMap, though it is not
really user friendly.

Ref: https://docs.ceph.com/en/latest/radosgw/config-ref/#confval-rgw_run_sync_thread

This commit adds a new option in the CephObjectStore CRD as defined in issue
https://github.com/rook/rook/issues/12272. The new option
'disableMultisiteSyncTraffic' determine whether the operator should disable the
multisite sync threads for the RGWs.

If the option is set to 'false', or if the option is not specified, the operator
does nothing. This ensures that the multisite sync threads will not be enabled
for users that disabled explicitely the multisite sync threads either manually
or with the 'rook-config-override' ConfigMap.

This commit also recommends to use two objectstore when scaling Ceph Objectstore
Multisite replication, with one objectstore configured with disabled replication
traffic.

Signed-off-by: Lucas Henry <polyedre@disroot.org>
(cherry picked from commit 393d09347e2ef28b83c91b5f5b8786f20933fb85)
---
 Documentation/CRDs/specification.md           | 22 ++++++++
 .../ceph-object-multisite.md                  | 40 ++++++++++++++
 .../charts/rook-ceph/templates/resources.yaml |  5 +-
 deploy/examples/crds.yaml                     |  5 +-
 pkg/apis/ceph.rook.io/v1/types.go             | 12 +++++
 pkg/operator/ceph/object/config.go            |  7 ++-
 pkg/operator/ceph/object/rgw.go               |  2 +-
 pkg/operator/ceph/object/rgw_test.go          | 54 +++++++++++++++++--
 8 files changed, 139 insertions(+), 8 deletions(-)

diff --git a/Documentation/CRDs/specification.md b/Documentation/CRDs/specification.md
index 849b9a79f..3617cd2ed 100644
--- a/Documentation/CRDs/specification.md
+++ b/Documentation/CRDs/specification.md
@@ -1992,6 +1992,9 @@ include the port in the definition. For example: &ldquo;<a href="https://my-obje
 In many cases, you should set this to the endpoint of the ingress resource that makes the
 CephObjectStore associated with this CephObjectStoreZone reachable to peer clusters.
 The list can have one or more endpoints pointing to different RGW servers in the zone.</p>
+<p>If a CephObjectStore endpoint is omitted from this list, that object store&rsquo;s gateways will
+not receive multisite replication data
+(see CephObjectStore.spec.gateway.disableMultisiteSyncTraffic).</p>
 </td>
 </tr>
 <tr>
@@ -5988,6 +5991,22 @@ Placement
 </tr>
 <tr>
 <td>
+<code>disableMultisiteSyncTraffic</code><br/>
+<em>
+bool
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>DisableMultisiteSyncTraffic, when true, prevents this object store&rsquo;s gateways from
+transmitting multisite replication data. Note that this value does not affect whether
+gateways receive multisite replication traffic: see ObjectZone.spec.customEndpoints for that.
+If false or unset, this object store&rsquo;s gateways will be able to transmit multisite
+replication data.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>annotations</code><br/>
 <em>
 <a href="#ceph.rook.io/v1.Annotations">
@@ -8741,6 +8760,9 @@ include the port in the definition. For example: &ldquo;<a href="https://my-obje
 In many cases, you should set this to the endpoint of the ingress resource that makes the
 CephObjectStore associated with this CephObjectStoreZone reachable to peer clusters.
 The list can have one or more endpoints pointing to different RGW servers in the zone.</p>
+<p>If a CephObjectStore endpoint is omitted from this list, that object store&rsquo;s gateways will
+not receive multisite replication data
+(see CephObjectStore.spec.gateway.disableMultisiteSyncTraffic).</p>
 </td>
 </tr>
 <tr>
diff --git a/Documentation/Storage-Configuration/Object-Storage-RGW/ceph-object-multisite.md b/Documentation/Storage-Configuration/Object-Storage-RGW/ceph-object-multisite.md
index 15e5f37d9..3a4e0990c 100644
--- a/Documentation/Storage-Configuration/Object-Storage-RGW/ceph-object-multisite.md
+++ b/Documentation/Storage-Configuration/Object-Storage-RGW/ceph-object-multisite.md
@@ -186,6 +186,46 @@ object-multisite-pull-realm.yaml (with changes) in the [examples](https://github
 kubectl create -f object-multisite-pull-realm.yaml
 ```
 
+## Scaling a Multisite
+
+Scaling the number of gateways that run the synchronization thread to 2 or more can increase the latency of the
+replication of each S3 object. The recommended way to scale a mutisite configuration is to dissociate the gateway dedicated
+to the synchronization from gateways that serve clients.
+
+The two types of gateways can be deployed by creating two CephObjectStores associated with the same CephObjectZone. The
+objectstore that deploys the gateway dedicated to the synchronization must have `spec.gateway.instances` set to `1`,
+while the objectstore that deploys the client gateways have multiple replicas and should disable the synchronization
+thread on the gateways by setting `spec.gateway.disableMultisiteSyncTraffic` to `true`.
+
+```yaml
+---
+apiVersion: ceph.rook.io/v1
+kind: CephObjectStore
+metadata:
+  name: replication
+  namespace: rook-ceph
+spec:
+  gateway:
+    port: 80
+    instances: 1
+    disableMultisiteSyncTraffic: false
+  zone:
+    name: zone-a
+---
+apiVersion: ceph.rook.io/v1
+kind: CephObjectStore
+metadata:
+  name: clients
+  namespace: rook-ceph
+spec:
+  gateway:
+    port: 80
+    instances: 5
+    disableMultisiteSyncTraffic: true
+  zone:
+    name: zone-a
+```
+
 ## Multisite Cleanup
 
 Multisite configuration must be cleaned up by hand. Deleting a realm/zone group/zone CR will not delete the underlying Ceph realm, zone group, zone, or the pools associated with a zone.
diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml
index 3f290047c..af52dbb16 100644
--- a/deploy/charts/rook-ceph/templates/resources.yaml
+++ b/deploy/charts/rook-ceph/templates/resources.yaml
@@ -9157,6 +9157,9 @@ spec:
                       nullable: true
                       type: boolean
                       x-kubernetes-preserve-unknown-fields: true
+                    disableMultisiteSyncTraffic:
+                      description: 'DisableMultisiteSyncTraffic, when true, prevents this object store''s gateways from transmitting multisite replication data. Note that this value does not affect whether gateways receive multisite replication traffic: see ObjectZone.spec.customEndpoints for that. If false or unset, this object store''s gateways will be able to transmit multisite replication data.'
+                      type: boolean
                     externalRgwEndpoints:
                       description: ExternalRgwEndpoints points to external RGW endpoint(s). Multiple endpoints can be given, but for stability of ObjectBucketClaims, we highly recommend that users give only a single external RGW endpoint that is a load balancer that sends requests to the multiple RGWs.
                       items:
@@ -10667,7 +10670,7 @@ spec:
               description: ObjectZoneSpec represent the spec of an ObjectZone
               properties:
                 customEndpoints:
-                  description: 'If this zone cannot be accessed from other peer Ceph clusters via the ClusterIP Service endpoint created by Rook, you must set this to the externally reachable endpoint(s). You may include the port in the definition. For example: "https://my-object-store.my-domain.net:443". In many cases, you should set this to the endpoint of the ingress resource that makes the CephObjectStore associated with this CephObjectStoreZone reachable to peer clusters. The list can have one or more endpoints pointing to different RGW servers in the zone.'
+                  description: "If this zone cannot be accessed from other peer Ceph clusters via the ClusterIP Service endpoint created by Rook, you must set this to the externally reachable endpoint(s). You may include the port in the definition. For example: \"https://my-object-store.my-domain.net:443\". In many cases, you should set this to the endpoint of the ingress resource that makes the CephObjectStore associated with this CephObjectStoreZone reachable to peer clusters. The list can have one or more endpoints pointing to different RGW servers in the zone. \n If a CephObjectStore endpoint is omitted from this list, that object store's gateways will not receive multisite replication data (see CephObjectStore.spec.gateway.disableMultisiteSyncTraffic)."
                   items:
                     type: string
                   nullable: true
diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml
index 2fbca1617..eb5c13ac4 100644
--- a/deploy/examples/crds.yaml
+++ b/deploy/examples/crds.yaml
@@ -9149,6 +9149,9 @@ spec:
                       nullable: true
                       type: boolean
                       x-kubernetes-preserve-unknown-fields: true
+                    disableMultisiteSyncTraffic:
+                      description: 'DisableMultisiteSyncTraffic, when true, prevents this object store''s gateways from transmitting multisite replication data. Note that this value does not affect whether gateways receive multisite replication traffic: see ObjectZone.spec.customEndpoints for that. If false or unset, this object store''s gateways will be able to transmit multisite replication data.'
+                      type: boolean
                     externalRgwEndpoints:
                       description: ExternalRgwEndpoints points to external RGW endpoint(s). Multiple endpoints can be given, but for stability of ObjectBucketClaims, we highly recommend that users give only a single external RGW endpoint that is a load balancer that sends requests to the multiple RGWs.
                       items:
@@ -10656,7 +10659,7 @@ spec:
               description: ObjectZoneSpec represent the spec of an ObjectZone
               properties:
                 customEndpoints:
-                  description: 'If this zone cannot be accessed from other peer Ceph clusters via the ClusterIP Service endpoint created by Rook, you must set this to the externally reachable endpoint(s). You may include the port in the definition. For example: "https://my-object-store.my-domain.net:443". In many cases, you should set this to the endpoint of the ingress resource that makes the CephObjectStore associated with this CephObjectStoreZone reachable to peer clusters. The list can have one or more endpoints pointing to different RGW servers in the zone.'
+                  description: "If this zone cannot be accessed from other peer Ceph clusters via the ClusterIP Service endpoint created by Rook, you must set this to the externally reachable endpoint(s). You may include the port in the definition. For example: \"https://my-object-store.my-domain.net:443\". In many cases, you should set this to the endpoint of the ingress resource that makes the CephObjectStore associated with this CephObjectStoreZone reachable to peer clusters. The list can have one or more endpoints pointing to different RGW servers in the zone. \n If a CephObjectStore endpoint is omitted from this list, that object store's gateways will not receive multisite replication data (see CephObjectStore.spec.gateway.disableMultisiteSyncTraffic)."
                   items:
                     type: string
                   nullable: true
diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go
index f1b107357..b92f1d088 100755
--- a/pkg/apis/ceph.rook.io/v1/types.go
+++ b/pkg/apis/ceph.rook.io/v1/types.go
@@ -1433,6 +1433,14 @@ type GatewaySpec struct {
 	// +optional
 	Placement Placement `json:"placement,omitempty"`
 
+	// DisableMultisiteSyncTraffic, when true, prevents this object store's gateways from
+	// transmitting multisite replication data. Note that this value does not affect whether
+	// gateways receive multisite replication traffic: see ObjectZone.spec.customEndpoints for that.
+	// If false or unset, this object store's gateways will be able to transmit multisite
+	// replication data.
+	// +optional
+	DisableMultisiteSyncTraffic bool `json:"disableMultisiteSyncTraffic,omitempty"`
+
 	// The annotations-related configuration to add/set on each Pod related object.
 	// +kubebuilder:pruning:PreserveUnknownFields
 	// +nullable
@@ -1765,6 +1773,10 @@ type ObjectZoneSpec struct {
 	// In many cases, you should set this to the endpoint of the ingress resource that makes the
 	// CephObjectStore associated with this CephObjectStoreZone reachable to peer clusters.
 	// The list can have one or more endpoints pointing to different RGW servers in the zone.
+	//
+	// If a CephObjectStore endpoint is omitted from this list, that object store's gateways will
+	// not receive multisite replication data
+	// (see CephObjectStore.spec.gateway.disableMultisiteSyncTraffic).
 	// +nullable
 	// +optional
 	CustomEndpoints []string `json:"customEndpoints,omitempty"`
diff --git a/pkg/operator/ceph/object/config.go b/pkg/operator/ceph/object/config.go
index c34c453d2..8ebe1237a 100644
--- a/pkg/operator/ceph/object/config.go
+++ b/pkg/operator/ceph/object/config.go
@@ -110,11 +110,16 @@ func (c *clusterConfig) generateKeyring(rgwConfig *rgwConfig) (string, error) {
 	return keyring, s.CreateOrUpdate(rgwConfig.ResourceName, keyring)
 }
 
-func (c *clusterConfig) setDefaultFlagsMonConfigStore(rgwConfig *rgwConfig) error {
+func (c *clusterConfig) setFlagsMonConfigStore(rgwConfig *rgwConfig) error {
 	monStore := cephconfig.GetMonStore(c.context, c.clusterInfo)
 	who := generateCephXUser(rgwConfig.ResourceName)
 	configOptions := make(map[string]string)
 
+	configOptions["rgw_run_sync_thread"] = "true"
+	if c.store.Spec.Gateway.DisableMultisiteSyncTraffic {
+		configOptions["rgw_run_sync_thread"] = "false"
+	}
+
 	configOptions["rgw_log_nonexistent_bucket"] = "true"
 	configOptions["rgw_log_object_name_utc"] = "true"
 	configOptions["rgw_enable_usage_log"] = "true"
diff --git a/pkg/operator/ceph/object/rgw.go b/pkg/operator/ceph/object/rgw.go
index 889b7494a..06252e52c 100644
--- a/pkg/operator/ceph/object/rgw.go
+++ b/pkg/operator/ceph/object/rgw.go
@@ -135,7 +135,7 @@ func (c *clusterConfig) startRGWPods(realmName, zoneGroupName, zoneName string)
 		// Unfortunately, on upgrade we would not set the flags which is not ideal for old clusters where we were no setting those flags
 		// The KV supports setting those flags even if the RGW is running
 		logger.Info("setting rgw config flags")
-		err = c.setDefaultFlagsMonConfigStore(rgwConfig)
+		err = c.setFlagsMonConfigStore(rgwConfig)
 		if err != nil {
 			// Getting EPERM typically happens when the flag may not be modified at runtime
 			// This is fine to ignore
diff --git a/pkg/operator/ceph/object/rgw_test.go b/pkg/operator/ceph/object/rgw_test.go
index 907d5e527..2723ca5a3 100644
--- a/pkg/operator/ceph/object/rgw_test.go
+++ b/pkg/operator/ceph/object/rgw_test.go
@@ -19,6 +19,7 @@ package object
 import (
 	"context"
 	"testing"
+	"time"
 
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 	"github.com/rook/rook/pkg/client/clientset/versioned/scheme"
@@ -40,6 +41,10 @@ import (
 func TestStartRGW(t *testing.T) {
 	ctx := context.TODO()
 	clientset := test.New(t, 3)
+
+	// Store the configuration options applied to gateways through the MockExecutor
+	appliedRgwConfigurations := make(map[string]string)
+
 	executor := &exectest.MockExecutor{
 		MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) {
 			if args[0] == "auth" && args[1] == "get-or-create-key" {
@@ -47,13 +52,22 @@ func TestStartRGW(t *testing.T) {
 			}
 			return `{"id":"test-id"}`, nil
 		},
+		MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) {
+			// Answer to `ceph config set ...`
+			if args[0] == "config" && args[1] == "set" {
+				config_option := args[3]
+				value := args[4]
+				appliedRgwConfigurations[config_option] = value
+			}
+			return "", nil
+		},
 	}
 
 	configDir := t.TempDir()
 	info := clienttest.CreateTestClusterInfo(1)
 	context := &clusterd.Context{Clientset: clientset, Executor: executor, ConfigDir: configDir}
 	store := simpleStore()
-	store.Spec.Gateway.Instances = 1
+
 	version := "v1.1.0"
 	data := config.NewStatelessDaemonDataPathMap(config.RgwType, "my-fs", "rook-ceph", "/var/lib/rook/")
 
@@ -65,10 +79,42 @@ func TestStartRGW(t *testing.T) {
 	// start a basic cluster
 	ownerInfo := client.NewMinimumOwnerInfoWithOwnerRef()
 	c := &clusterConfig{context, info, store, version, &cephv1.ClusterSpec{}, ownerInfo, data, r.client}
-	err := c.startRGWPods(store.Name, store.Name, store.Name)
-	assert.Nil(t, err)
 
-	validateStart(ctx, t, c, clientset)
+	t.Run("Deployment is created", func(t *testing.T) {
+		store.Spec.Gateway.Instances = 1
+		err := c.startRGWPods(store.Name, store.Name, store.Name)
+		assert.Nil(t, err)
+
+		validateStart(ctx, t, c, clientset)
+	})
+
+	t.Run("Multisite sync traffic disabled", func(t *testing.T) {
+		store.ObjectMeta.Name = "client-rgw-sync-disabled"
+		store.Spec.Gateway.DisableMultisiteSyncTraffic = true
+
+		// Purge store of configurations applied to gateways
+		appliedRgwConfigurations = make(map[string]string)
+
+		err := c.startRGWPods(store.Name, store.Name, store.Name)
+		assert.Nil(t, err)
+
+		assert.Contains(t, appliedRgwConfigurations, "rgw_run_sync_thread")
+		assert.Equal(t, appliedRgwConfigurations["rgw_run_sync_thread"], "false")
+	})
+
+	t.Run("Multisite sync traffic enabled", func(t *testing.T) {
+		store.ObjectMeta.Name = "client-rgw-sync-enabled"
+		store.Spec.Gateway.DisableMultisiteSyncTraffic = false
+
+		// Purge store of configurations applied to gateways
+		appliedRgwConfigurations = make(map[string]string)
+
+		err := c.startRGWPods(store.Name, store.Name, store.Name)
+		assert.Nil(t, err)
+
+		assert.Contains(t, appliedRgwConfigurations, "rgw_run_sync_thread")
+		assert.Equal(t, appliedRgwConfigurations["rgw_run_sync_thread"], "true")
+	})
 }
 
 func validateStart(ctx context.Context, t *testing.T, c *clusterConfig, clientset *fclient.Clientset) {