ceph: always apply config flags for mds and rgw

We now always set the config flags on reconcile. Previously we were
looking for non-existing rgw or mds which means that on upgrade we would
never set those flags. However, we want to set those flags on "old"
cluster that got upgraded.

Signed-off-by: Sébastien Han <seb@redhat.com>

pkg/operator/ceph/file/filesystem_test.go

@@ -256,8 +256,11 @@ func TestCreateFilesystem(t *testing.T) {
 				return "", nil
 			} else if reflect.DeepEqual(args[0:6], []string{"osd", "pool", "set", fsName + "-data1", "size", "1"}) {
 				return "", nil
+				// } else if reflect.DeepEqual(args[0:1], []string{"config", "set"}) {
+			} else if args[0] == "config" && args[1] == "set" {
+				return "", nil
 			}
-			assert.Fail(t, "Unexpected command")
+			assert.Fail(t, fmt.Sprintf("Unexpected command: %v", args))
 			return "", nil
 		},
 	}

pkg/operator/ceph/file/mds/mds.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/banzaicloud/k8s-objectmatcher/patch"
@@ -34,6 +35,7 @@ import (
 	"github.com/rook/rook/pkg/operator/ceph/config"
 	"github.com/rook/rook/pkg/operator/ceph/controller"
 	"github.com/rook/rook/pkg/operator/k8sutil"
+	"github.com/rook/rook/pkg/util/exec"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -136,17 +138,19 @@ func (c *Cluster) Start() error {
 			return errors.Wrapf(err, "failed to generate keyring for %q", resourceName)
 		}
 
-		// Check for existing deployment and set the daemon config flags
-		_, err = c.context.Clientset.AppsV1().Deployments(c.fs.Namespace).Get(ctx, mdsConfig.ResourceName, metav1.GetOptions{})
-		// We don't need to handle any error here
+		// Set the mds config flags
+		// Previously we were checking if the deployment was present, if not we would set the config flags
+		// Which means that we would only set the flag on newly created CephFilesystem CR
+		// 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 MDS is running
+		logger.Info("setting mds config flags")
+		err = c.setDefaultFlagsMonConfigStore(mdsConfig.DaemonID)
 		if err != nil {
-			// Apply the flag only when the deployment is not found
-			if kerrors.IsNotFound(err) {
-				logger.Info("setting mds config flags")
-				err = c.setDefaultFlagsMonConfigStore(mdsConfig.DaemonID)
-				if err != nil {
-					return errors.Wrap(err, "failed to set default mds config options")
-				}
+			// Getting EPERM typically happens when the flag may not be modified at runtime
+			// This is fine to ignore
+			code, ok := exec.ExitStatus(err)
+			if ok && code != int(syscall.EPERM) {
+				return errors.Wrap(err, "failed to set default rgw config options")
 			}
 		}
 

pkg/operator/ceph/object/rgw.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"syscall"
 
 	"github.com/banzaicloud/k8s-objectmatcher/patch"
 	"github.com/pkg/errors"
@@ -31,6 +32,7 @@ import (
 	"github.com/rook/rook/pkg/operator/ceph/config"
 	"github.com/rook/rook/pkg/operator/ceph/pool"
 	"github.com/rook/rook/pkg/operator/k8sutil"
+	"github.com/rook/rook/pkg/util/exec"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -113,17 +115,19 @@ func (c *clusterConfig) startRGWPods(realmName, zoneGroupName, zoneName string)
 			return errors.Wrap(err, "failed to create rgw keyring")
 		}
 
-		// Check for existing deployment and set the daemon config flags
-		_, err = c.context.Clientset.AppsV1().Deployments(c.store.Namespace).Get(ctx, rgwConfig.ResourceName, metav1.GetOptions{})
-		// We don't need to handle any error here
+		// Set the rgw config flags
+		// Previously we were checking if the deployment was present, if not we would set the config flags
+		// Which means that we would only set the flag on newly created CephObjectStore CR
+		// 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.ResourceName)
 		if err != nil {
-			// Apply the flag only when the deployment is not found
-			if kerrors.IsNotFound(err) {
-				logger.Info("setting rgw config flags")
-				err = c.setDefaultFlagsMonConfigStore(rgwConfig.ResourceName)
-				if err != nil {
-					return errors.Wrap(err, "failed to set default rgw config options")
-				}
+			// Getting EPERM typically happens when the flag may not be modified at runtime
+			// This is fine to ignore
+			code, ok := exec.ExitStatus(err)
+			if ok && code != int(syscall.EPERM) {
+				return errors.Wrap(err, "failed to set default rgw config options")
 			}
 		}