rgw: always update period, even if realm exists

Rook should always update the RGW object store's period, even if the
zone/zonegroup/realm haven't changed. This is because the
'radosgw-admin period update' command can fail, and we must retry it in
that case.

Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>

pkg/operator/ceph/object/controller_test.go

@@ -442,24 +442,29 @@ func TestCephObjectStoreController(t *testing.T) {
 						{"poolnum":9,"poolname":"my-store.rgw.buckets.data"}
 					]`, nil
 				}
+				t.Logf("returning default success for command: %s %v", command, args)
 				return "", nil
 			},
 			MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) {
-				if args[0] == "realm" && args[1] == "list" {
-					return realmListJSON, nil
-				}
-				if args[0] == "realm" && args[1] == "get" {
-					return realmGetJSON, nil
-				}
-				if args[0] == "zonegroup" && args[1] == "get" {
-					return zoneGroupGetJSON, nil
-				}
-				if args[0] == "zone" && args[1] == "get" {
-					return zoneGetJSON, nil
-				}
-				if args[0] == "user" {
-					return userCreateJSON, nil
+				// this assumes the realm/zone/zonegroup have already been created
+				if command == "radosgw-admin" {
+					if args[0] == "realm" && args[1] == "list" {
+						return realmListJSON, nil
+					}
+					if args[0] == "realm" && args[1] == "get" {
+						return realmGetJSON, nil
+					}
+					if args[0] == "zonegroup" && args[1] == "get" {
+						return zoneGroupGetJSON, nil
+					}
+					if args[0] == "zone" && args[1] == "get" {
+						return zoneGetJSON, nil
+					}
+					if args[0] == "user" {
+						return userCreateJSON, nil
+					}
 				}
+				t.Logf("returning default success for command: %s %v", command, args)
 				return "", nil
 			},
 		}
@@ -500,6 +505,37 @@ func TestCephObjectStoreController(t *testing.T) {
 		assert.Equal(t, "http://rook-ceph-rgw-my-store.rook-ceph.svc:80", objectStore.Status.Info["endpoint"], objectStore)
 	})
 
+	t.Run("regression check - period update command should always be called", func(t *testing.T) {
+		r := setupEnvironmentWithReadyCephCluster()
+
+		// do a little hack to get the mock executor that was set up in the setup function, and make
+		// a wrapper around it that will let us verify that the period update command got issued
+		periodUpdateCount := 0
+		mockExec, ok := r.context.Executor.(*exectest.MockExecutor)
+		if !ok {
+			t.Fatalf("failed to set up test mock executor")
+		}
+		origMockExecWithTimeout := mockExec.MockExecuteCommandWithTimeout
+		mockExec.MockExecuteCommandWithTimeout = func(timeout time.Duration, command string, arg ...string) (string, error) {
+			// count how many times period is updated
+			if command == "radosgw-admin" && arg[0] == "period" && arg[1] == "update" {
+				periodUpdateCount++
+			}
+			if command == "radosgw-admin" && arg[0] == "zone" && arg[1] == "create" {
+				t.Fatal("this test assumes that the zone (and zonegroup and realm) already exist and should not be created, " +
+					"but 'create' was called! The test setup must have changed! " +
+					"Please fix the setup or this test to compensate!")
+			}
+			// pass through all commands to the original function
+			return origMockExecWithTimeout(timeout, command, arg...)
+		}
+
+		res, err := r.Reconcile(ctx, req)
+		assert.NoError(t, err)
+		assert.False(t, res.Requeue)
+		assert.Equal(t, periodUpdateCount, 1)
+	})
+
 }
 
 func TestCephObjectStoreControllerMultisite(t *testing.T) {

pkg/operator/ceph/object/objectstore.go

@@ -362,13 +362,11 @@ func createMultisite(objContext *Context, endpointArg string) error {
 	realmArg := fmt.Sprintf("--rgw-realm=%s", objContext.Realm)
 	zoneGroupArg := fmt.Sprintf("--rgw-zonegroup=%s", objContext.ZoneGroup)
 
-	updatePeriod := false
 	// create the realm if it doesn't exist yet
 	output, err := RunAdminCommandNoMultisite(objContext, true, "realm", "get", realmArg)
 	if err != nil {
 		// ENOENT means “No such file or directory”
 		if code, err := exec.ExtractExitCode(err); err == nil && code == int(syscall.ENOENT) {
-			updatePeriod = true
 			output, err = RunAdminCommandNoMultisite(objContext, false, "realm", "create", realmArg)
 			if err != nil {
 				return errorOrIsNotFound(err, "failed to create ceph realm %q, for reason %q", objContext.ZoneGroup, output)
@@ -384,7 +382,6 @@ func createMultisite(objContext *Context, endpointArg string) error {
 	if err != nil {
 		// ENOENT means “No such file or directory”
 		if code, err := exec.ExtractExitCode(err); err == nil && code == int(syscall.ENOENT) {
-			updatePeriod = true
 			output, err = RunAdminCommandNoMultisite(objContext, false, "zonegroup", "create", "--master", realmArg, zoneGroupArg, endpointArg)
 			if err != nil {
 				return errorOrIsNotFound(err, "failed to create ceph zone group %q, for reason %q", objContext.ZoneGroup, output)
@@ -400,7 +397,6 @@ func createMultisite(objContext *Context, endpointArg string) error {
 	if err != nil {
 		// ENOENT means “No such file or directory”
 		if code, err := exec.ExtractExitCode(err); err == nil && code == int(syscall.ENOENT) {
-			updatePeriod = true
 			output, err = runAdminCommand(objContext, false, "zone", "create", "--master", endpointArg)
 			if err != nil {
 				return errorOrIsNotFound(err, "failed to create ceph zone %q, for reason %q", objContext.Zone, output)
@@ -411,14 +407,12 @@ func createMultisite(objContext *Context, endpointArg string) error {
 		}
 	}
 
-	if updatePeriod {
-		// the period will help notify other zones of changes if there are multi-zones
-		_, err := runAdminCommand(objContext, false, "period", "update", "--commit")
-		if err != nil {
-			return errorOrIsNotFound(err, "failed to update period")
-		}
-		logger.Debugf("updated period for realm %v", objContext.Realm)
+	// the period will help notify other zones of changes if there are multi-zones
+	_, err = runAdminCommand(objContext, false, "period", "update", "--commit")
+	if err != nil {
+		return errorOrIsNotFound(err, "failed to update period")
 	}
+	logger.Debugf("updated period for realm %v", objContext.Realm)
 
 	logger.Infof("Multisite for object-store: realm=%s, zonegroup=%s, zone=%s", objContext.Realm, objContext.ZoneGroup, objContext.Zone)