Ensure reconcilation is requeued for restore controller if the restore is not completed

controllers/admin_client_test.go

@@ -25,15 +25,12 @@ import (
 	"fmt"
 	"net"
 
-	"github.com/FoundationDB/fdb-kubernetes-operator/pkg/fdbadminclient/mock"
-
+	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/api/v1beta2"
 	"github.com/FoundationDB/fdb-kubernetes-operator/internal"
-	"k8s.io/utils/pointer"
-
+	"github.com/FoundationDB/fdb-kubernetes-operator/pkg/fdbadminclient/mock"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-
-	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/api/v1beta2"
+	"k8s.io/utils/pointer"
 )
 
 var _ = Describe("admin_client_test", func() {
@@ -372,30 +369,29 @@ var _ = Describe("admin_client_test", func() {
 	})
 
 	Describe("restore status", func() {
-		var status string
+		var restoreStatus string
 
 		Context("with no restore running", func() {
 			BeforeEach(func() {
-				status, err = mockAdminClient.GetRestoreStatus()
+				restoreStatus, err = mockAdminClient.GetRestoreStatus()
 				Expect(err).NotTo(HaveOccurred())
 			})
 
 			It("should be empty", func() {
-				Expect(status).To(Equal("\n"))
+				Expect(restoreStatus).To(BeEmpty())
 			})
 		})
 
 		Context("with a restore running", func() {
 			BeforeEach(func() {
-				err = mockAdminClient.StartRestore("blobstore://test@test-service/test-backup", nil)
-				Expect(err).NotTo(HaveOccurred())
+				Expect(mockAdminClient.StartRestore("blobstore://test@test-service/test-backup", nil)).To(Succeed())
 
-				status, err = mockAdminClient.GetRestoreStatus()
+				restoreStatus, err = mockAdminClient.GetRestoreStatus()
 				Expect(err).NotTo(HaveOccurred())
 			})
 
 			It("should contain the backup URL", func() {
-				Expect(status).To(Equal("blobstore://test@test-service/test-backup\n"))
+				Expect(restoreStatus).To(ContainSubstring("blobstore://test@test-service/test-backup"))
 			})
 		})
 	})

controllers/restore_controller.go

@@ -22,21 +22,20 @@ package controllers
 
 import (
 	"context"
-
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	"github.com/FoundationDB/fdb-kubernetes-operator/pkg/fdbadminclient"
-
-	"sigs.k8s.io/controller-runtime/pkg/controller"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
+	"time"
 
 	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/api/v1beta2"
+	"github.com/FoundationDB/fdb-kubernetes-operator/pkg/fdbadminclient"
 	"github.com/go-logr/logr"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
 
 // FoundationDBRestoreReconciler reconciles a FoundationDBRestore object
@@ -67,7 +66,7 @@ func (r *FoundationDBRestoreReconciler) Reconcile(ctx context.Context, request c
 		return ctrl.Result{}, err
 	}
 
-	restoreLog := globalControllerLogger.WithValues("namespace", restore.Namespace, "restore", restore.Name)
+	restoreLog := globalControllerLogger.WithValues("namespace", restore.Namespace, "restore", restore.Name, "traceID", uuid.NewUUID())
 
 	subReconcilers := []restoreSubReconciler{
 		updateRestoreStatus{},
@@ -76,12 +75,24 @@ func (r *FoundationDBRestoreReconciler) Reconcile(ctx context.Context, request c
 	}
 
 	for _, subReconciler := range subReconcilers {
-		requeue := subReconciler.reconcile(ctx, r, restore)
-		if requeue == nil {
+		req := subReconciler.reconcile(ctx, r, restore)
+		if req == nil {
 			continue
 		}
+		return processRequeue(req, subReconciler, restore, r.Recorder, restoreLog)
+	}
+
+	if restore.Status.State != fdbv1beta2.CompletedFoundationDBRestoreState {
+		restoreLog.Info("Restore has not yet completed",
+			"Status", restore.Status.State,
+			"Running", restore.Status.Running)
+
+		// Check the status of the restore every 5 minutes, otherwise check the status every minute.
+		if restore.Status.State != fdbv1beta2.RunningFoundationDBRestoreState {
+			return ctrl.Result{Requeue: true, RequeueAfter: 5 * time.Minute}, nil
+		}
 
-		return processRequeue(requeue, subReconciler, restore, r.Recorder, restoreLog)
+		return ctrl.Result{Requeue: true, RequeueAfter: 1 * time.Minute}, nil
 	}
 
 	restoreLog.Info("Reconciliation complete")

controllers/restore_controller_test.go

@@ -83,18 +83,15 @@ var _ = Describe("restore_controller", func() {
 			result, err := reconcileRestore(restore)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(result.Requeue).To(BeFalse())
-
-			err = reloadRestore(restore)
-			Expect(err).NotTo(HaveOccurred())
-			err = k8sClient.Get(context.TODO(), types.NamespacedName{Namespace: restore.Namespace, Name: restore.Name}, cluster)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(reloadRestore(restore)).To(Succeed())
+			Expect(k8sClient.Get(context.TODO(), types.NamespacedName{Namespace: restore.Namespace, Name: restore.Name}, cluster)).To(Succeed())
 		})
 
-		Context("when reconciling a new restore", func() {
+		When("reconciling a new restore", func() {
 			It("should start a restore", func() {
 				status, err := adminClient.GetRestoreStatus()
 				Expect(err).NotTo(HaveOccurred())
-				Expect(status).To(Equal("blobstore://test@test-service:443/test-backup?bucket=fdb-backups\n"))
+				Expect(status).To(ContainSubstring("blobstore://test@test-service:443/test-backup?bucket=fdb-backups"))
 			})
 		})
 
@@ -103,8 +100,7 @@ var _ = Describe("restore_controller", func() {
 				restore.Spec.CustomParameters = fdbv1beta2.FoundationDBCustomParameters{
 					"knob_http_verbose_level=3",
 				}
-				err = k8sClient.Update(context.TODO(), restore)
-				Expect(err).NotTo(HaveOccurred())
+				Expect(k8sClient.Update(context.TODO(), restore)).To(Succeed())
 			})
 
 			It("should append the custom parameters to the command", func() {

e2e/fixtures/fdb_restore.go

@@ -23,10 +23,11 @@ package fixtures
 import (
 	"context"
 	"log"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"strconv"
 	"time"
 
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
 	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/api/v1beta2"
 	"github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -84,6 +85,16 @@ func waitForRestoreToComplete(backup *FdbBackup) {
 				context.Background(),
 				restore,
 				patch)).To(gomega.Succeed())
+
+			out, _, err := backup.fdbCluster.ExecuteCmdOnPod(
+				*backup.GetBackupPod(),
+				fdbv1beta2.MainContainerName,
+				"fdbrestore status --dest_cluster_file $FDB_CLUSTER_FILE",
+				false,
+			)
+
+			log.Println(out)
+			g.Expect(err).To(gomega.Succeed())
 		}
 
 		return restore.Status.State

pkg/fdbadminclient/mock/admin_client_mock.go

@@ -905,7 +905,11 @@ func (client *AdminClient) GetRestoreStatus() (string, error) {
 		return "", client.mockError
 	}
 
-	return fmt.Sprintf("%s\n", client.restoreURL), nil
+	if client.restoreURL == "" {
+		return "", nil
+	}
+
+	return fmt.Sprintf("%s, State: completed\n", client.restoreURL), nil
 }
 
 // MockClientVersion returns a mocked client version