fix(backup): customize the timeout of backup binary execution

Add a default setting `backupEngineBinaryTimeout` ref: longhorn/longhorn 8954,8319 Signed-off-by: James Lu <james.lu@suse.com>
longhorn · Aug 2, 2024 · 81244f7 · 81244f7
1 parent b10fa4a
commit 81244f7
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 10 deletions.
diff --git a/controller/backup_target_controller.go b/controller/backup_target_controller.go
@@ -24,6 +24,7 @@ import (
 	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
 
 	systembackupstore "github.com/longhorn/backupstore/systembackup"
+	lhtypes "github.com/longhorn/go-common-libs/types"
 
 	"github.com/longhorn/longhorn-manager/datastore"
 	"github.com/longhorn/longhorn-manager/engineapi"
@@ -276,7 +277,17 @@ func newBackupTargetClient(ds *datastore.DataStore, backupTarget *longhorn.Backu
 			return nil, err
 		}
 	}
-	return engineapi.NewBackupTargetClient(engineImage, backupTarget.Spec.BackupTargetURL, credential), nil
+
+	executeTimeout, err := ds.GetSettingAsInt(types.SettingNameBackupExecuteTimeout)
+	if err != nil {
+		return nil, err
+	}
+	timeout := time.Duration(executeTimeout) * time.Minute
+	if executeTimeout <= 0 {
+		timeout = lhtypes.ExecuteNoTimeout
+	}
+
+	return engineapi.NewBackupTargetClient(engineImage, backupTarget.Spec.BackupTargetURL, credential, timeout), nil
 }
 
 func newBackupTargetClientFromDefaultEngineImage(ds *datastore.DataStore, backupTarget *longhorn.BackupTarget) (*engineapi.BackupTargetClient, error) {

diff --git a/engineapi/backups.go b/engineapi/backups.go
@@ -31,17 +31,19 @@ const (
 )
 
 type BackupTargetClient struct {
-	Image      string
-	URL        string
-	Credential map[string]string
+	Image          string
+	URL            string
+	Credential     map[string]string
+	ExecuteTimeout time.Duration
 }
 
 // NewBackupTargetClient returns the backup target client
-func NewBackupTargetClient(engineImage, url string, credential map[string]string) *BackupTargetClient {
+func NewBackupTargetClient(engineImage, url string, credential map[string]string, executeTimeout time.Duration) *BackupTargetClient {
 	return &BackupTargetClient{
-		Image:      engineImage,
-		URL:        url,
-		Credential: credential,
+		Image:          engineImage,
+		URL:            url,
+		Credential:     credential,
+		ExecuteTimeout: executeTimeout,
 	}
 }
 
@@ -68,7 +70,16 @@ func NewBackupTargetClientFromBackupTarget(backupTarget *longhorn.BackupTarget,
 		}
 	}
 
-	return NewBackupTargetClient(defaultEngineImage, backupTarget.Spec.BackupTargetURL, credential), nil
+	executeTimeout, err := ds.GetSettingAsInt(types.SettingNameBackupExecuteTimeout)
+	if err != nil {
+		return nil, err
+	}
+	timeout := time.Duration(executeTimeout) * time.Minute
+	if executeTimeout <= 0 {
+		timeout = lhtypes.ExecuteNoTimeout
+	}
+
+	return NewBackupTargetClient(defaultEngineImage, backupTarget.Spec.BackupTargetURL, credential, timeout), nil
 }
 
 func (btc *BackupTargetClient) LonghornEngineBinary() string {
@@ -130,7 +141,7 @@ func (btc *BackupTargetClient) ExecuteEngineBinary(args ...string) (string, erro
 	if err != nil {
 		return "", err
 	}
-	return lhexec.NewExecutor().Execute(envs, btc.LonghornEngineBinary(), args, lhtypes.ExecuteDefaultTimeout)
+	return lhexec.NewExecutor().Execute(envs, btc.LonghornEngineBinary(), args, btc.ExecuteTimeout)
 }
 
 func (btc *BackupTargetClient) ExecuteEngineBinaryWithTimeout(timeout time.Duration, args ...string) (string, error) {

diff --git a/types/setting.go b/types/setting.go
@@ -135,6 +135,7 @@ const (
 	SettingNameFreezeFilesystemForSnapshot                              = SettingName("freeze-filesystem-for-snapshot")
 	SettingNameAutoCleanupSnapshotWhenDeleteBackup                      = SettingName("auto-cleanup-when-delete-backup")
 	SettingNameDefaultMinNumberOfBackingImageCopies                     = SettingName("default-min-number-of-backing-image-copies")
+	SettingNameBackupExecuteTimeout                                     = SettingName("backup-execute-timeout")
 	SettingNameRWXVolumeFastFailover                                    = SettingName("rwx-volume-fast-failover")
 )
 
@@ -227,6 +228,7 @@ var (
 		SettingNameFreezeFilesystemForSnapshot,
 		SettingNameAutoCleanupSnapshotWhenDeleteBackup,
 		SettingNameDefaultMinNumberOfBackingImageCopies,
+		SettingNameBackupExecuteTimeout,
 		SettingNameRWXVolumeFastFailover,
 	}
 )
@@ -347,6 +349,7 @@ var (
 		SettingNameFreezeFilesystemForSnapshot:                              SettingDefinitionFreezeFilesystemForSnapshot,
 		SettingNameAutoCleanupSnapshotWhenDeleteBackup:                      SettingDefinitionAutoCleanupSnapshotWhenDeleteBackup,
 		SettingNameDefaultMinNumberOfBackingImageCopies:                     SettingDefinitionDefaultMinNumberOfBackingImageCopies,
+		SettingNameBackupExecuteTimeout:                                     SettingDefinitionBackupExecuteTimeout,
 		SettingNameRWXVolumeFastFailover:                                    SettingDefinitionRWXVolumeFastFailover,
 	}
 
@@ -409,6 +412,19 @@ var (
 		},
 	}
 
+	SettingDefinitionBackupExecuteTimeout = SettingDefinition{
+		DisplayName: "Backup Execute Timeout",
+		Description: "In minutes. This setting determines the timeout for the backup engine execution. Set to 0 to disable the timeout.",
+		Category:    SettingCategoryBackup,
+		Type:        SettingTypeInt,
+		Required:    true,
+		ReadOnly:    false,
+		Default:     "1",
+		ValueIntRange: map[string]int{
+			ValueIntRangeMinimum: 0,
+		},
+	}
+
 	SettingDefinitionRestoreVolumeRecurringJobs = SettingDefinition{
 		DisplayName: "Restore Volume Recurring Jobs",
 		Description: "Restore recurring jobs from the backup volume on the backup target and create recurring jobs if not exist during a backup restoration.\n\n" +