From 6be7ae46fc3d1349743aa186b400119c80bf4e20 Mon Sep 17 00:00:00 2001
From: Shinya Hayashi <shinya-hayashi@cybozu.co.jp>
Date: Wed, 16 Oct 2024 08:13:59 +0000
Subject: [PATCH] WIP: review fix1

Signed-off-by: Shinya Hayashi <shinya-hayashi@cybozu.co.jp>
---
 .../controller/mantlebackup_controller.go     | 40 +++++++++++--------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/internal/controller/mantlebackup_controller.go b/internal/controller/mantlebackup_controller.go
index 9f653c23..b7213d5a 100644
--- a/internal/controller/mantlebackup_controller.go
+++ b/internal/controller/mantlebackup_controller.go
@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"sort"
 	"time"
 
 	mantlev1 "github.com/cybozu-go/mantle/api/v1"
@@ -612,9 +611,9 @@ func (r *MantleBackupReconciler) finalize(
 }
 
 type dataSyncPrepareResult struct {
-	isIncremental                     bool
+	isIncremental                     bool // This field is valid iff isSecondaryMantleBackupReadyToUse is false.
 	isSecondaryMantleBackupReadyToUse bool
-	diffFrom                          *mantlev1.MantleBackup // non-nil value iff isIncremental is true.
+	diffFrom                          *mantlev1.MantleBackup // non-nil value iff isIncremental or isSecondaryMantleBackupReadyToUse is true.
 }
 
 func (r *MantleBackupReconciler) prepareForDataSynchronization(
@@ -622,7 +621,10 @@ func (r *MantleBackupReconciler) prepareForDataSynchronization(
 	backup *mantlev1.MantleBackup,
 	msc proto.MantleServiceClient,
 ) (*dataSyncPrepareResult, error) {
-	exportTargetPVCUID := backup.GetLabels()[labelLocalBackupTargetPVCUID]
+	exportTargetPVCUID, ok := backup.GetLabels()[labelLocalBackupTargetPVCUID]
+	if !ok {
+		return nil, fmt.Errorf(`"%s" label is missing`, labelLocalBackupTargetPVCUID)
+	}
 	resp, err := msc.ListMantleBackup(
 		ctx,
 		&proto.ListMantleBackupRequest{
@@ -638,10 +640,10 @@ func (r *MantleBackupReconciler) prepareForDataSynchronization(
 	if err != nil {
 		return nil, err
 	}
-	secondaryBackupSet := convertToMap(secondaryBackups)
+	secondaryBackupMap := convertToMap(secondaryBackups)
 
 	isSecondaryMantleBackupReadyToUse := false
-	secondaryBackup, ok := secondaryBackupSet[backup.GetName()]
+	secondaryBackup, ok := secondaryBackupMap[backup.GetName()]
 	if !ok {
 		return nil, fmt.Errorf("secondary MantleBackup not found: %s, %s",
 			backup.GetName(), backup.GetNamespace())
@@ -651,6 +653,14 @@ func (r *MantleBackupReconciler) prepareForDataSynchronization(
 		mantlev1.BackupConditionReadyToUse,
 	)
 
+	if isSecondaryMantleBackupReadyToUse {
+		return &dataSyncPrepareResult{
+			isIncremental:                     false,
+			isSecondaryMantleBackupReadyToUse: true,
+			diffFrom:                          nil,
+		}, nil
+	}
+
 	if syncMode, ok := backup.GetAnnotations()[annotSyncMode]; ok {
 		switch syncMode {
 		case syncModeFull:
@@ -694,7 +704,7 @@ func (r *MantleBackupReconciler) prepareForDataSynchronization(
 		return nil, err
 	}
 
-	diffFrom := searchForDiffOriginMantleBackup(backup, primaryBackupList.Items, secondaryBackupSet)
+	diffFrom := searchForDiffOriginMantleBackup(backup, primaryBackupList.Items, secondaryBackupMap)
 	isIncremental := (diffFrom != nil)
 
 	return &dataSyncPrepareResult{
@@ -718,7 +728,7 @@ func searchForDiffOriginMantleBackup(
 	primaryBackups []mantlev1.MantleBackup,
 	secondaryBackupSet map[string]*mantlev1.MantleBackup,
 ) *mantlev1.MantleBackup {
-	candidates := make([]*mantlev1.MantleBackup, 0)
+	var diffOrigin *mantlev1.MantleBackup
 	for _, primaryBackup := range primaryBackups {
 		primaryBackup := primaryBackup
 		secondaryBackup, ok := secondaryBackupSet[primaryBackup.Name]
@@ -735,16 +745,12 @@ func searchForDiffOriginMantleBackup(
 		if *backup.Status.SnapID <= *primaryBackup.Status.SnapID {
 			continue
 		}
-		candidates = append(candidates, &primaryBackup)
-	}
-	if len(candidates) == 0 {
-		return nil
+		if diffOrigin == nil || *diffOrigin.Status.SnapID < *primaryBackup.Status.SnapID {
+			diffOrigin = &primaryBackup
+		}
 	}
-	// Sort by SnapID in descending order.
-	sort.Slice(candidates, func(i, j int) bool {
-		return *candidates[i].Status.SnapID > *candidates[j].Status.SnapID
-	})
-	return candidates[0]
+
+	return diffOrigin
 }
 
 func (r *MantleBackupReconciler) export(