feat: remove default backup target settings (backport #3414)

api/forwarder.go

@@ -42,6 +42,20 @@ func OwnerIDFromVolume(m *manager.VolumeManager) func(req *http.Request) (string
 	}
 }
 
+func OwnerIDFromBackupTarget(m *manager.VolumeManager) func(req *http.Request) (string, error) {
+	return func(req *http.Request) (string, error) {
+		backupTargetName := mux.Vars(req)["backupTargetName"]
+		backupTarget, err := m.GetBackupTarget(backupTargetName)
+		if err != nil {
+			return "", errors.Wrapf(err, "failed to get backup target '%s'", backupTargetName)
+		}
+		if backupTarget == nil {
+			return "", nil
+		}
+		return backupTarget.Status.OwnerID, nil
+	}
+}
+
 // NodeHasDefaultEngineImage picks a node that is ready and has default engine image deployed.
 // To prevent the repeatedly forwarding the request around, prioritize the current node if it meets the requirement.
 func NodeHasDefaultEngineImage(m *manager.VolumeManager) func(req *http.Request) (string, error) {

api/router.go

@@ -126,8 +126,8 @@ func NewRouter(s *Server) *mux.Router {
 	r.Methods("PUT").Path("/v1/backuptargets").Handler(f(schemas, s.BackupTargetSyncAll))
 	r.Methods("DELETE").Path("/v1/backuptargets/{backupTargetName}").Handler(f(schemas, s.BackupTargetDelete))
 	backupTargetActions := map[string]func(http.ResponseWriter, *http.Request) error{
-		"backupTargetSync":   s.fwd.Handler(s.fwd.HandleProxyRequestByNodeID, s.fwd.GetHTTPAddressByNodeID(NodeHasDefaultEngineImage(s.m)), s.BackupTargetSync),
-		"backupTargetUpdate": s.fwd.Handler(s.fwd.HandleProxyRequestByNodeID, s.fwd.GetHTTPAddressByNodeID(NodeHasDefaultEngineImage(s.m)), s.BackupTargetUpdate),
+		"backupTargetSync":   s.fwd.Handler(s.fwd.HandleProxyRequestByNodeID, s.fwd.GetHTTPAddressByNodeID(OwnerIDFromBackupTarget(s.m)), s.BackupTargetSync),
+		"backupTargetUpdate": s.fwd.Handler(s.fwd.HandleProxyRequestByNodeID, s.fwd.GetHTTPAddressByNodeID(OwnerIDFromBackupTarget(s.m)), s.BackupTargetUpdate),
 	}
 	for name, action := range backupTargetActions {
 		r.Methods("POST").Path("/v1/backuptargets/{backupTargetName}").Queries("action", name).Handler(f(schemas, action))

controller/setting_controller.go

@@ -241,10 +241,6 @@ func (sc *SettingController) syncNonDangerZoneSettingsForManagedComponents(setti
 		if err := sc.syncUpgradeChecker(); err != nil {
 			return err
 		}
-	case types.SettingNameBackupTarget, types.SettingNameBackupTargetCredentialSecret, types.SettingNameBackupstorePollInterval:
-		if err := sc.syncDefaultBackupTarget(); err != nil {
-			return err
-		}
 	case types.SettingNameKubernetesClusterAutoscalerEnabled:
 		if err := sc.updateKubernetesClusterAutoscalerEnabled(); err != nil {
 			return err
@@ -401,65 +397,6 @@ func getResponsibleNodeID(ds *datastore.DataStore) (string, error) {
 	return responsibleNodes[0], nil
 }
 
-func (sc *SettingController) syncDefaultBackupTarget() (err error) {
-	defer func() {
-		err = errors.Wrap(err, "failed to sync backup target")
-	}()
-
-	// Get default backup target settings
-	targetSetting, err := sc.ds.GetSettingWithAutoFillingRO(types.SettingNameBackupTarget)
-	if err != nil {
-		return err
-	}
-
-	secretSetting, err := sc.ds.GetSettingWithAutoFillingRO(types.SettingNameBackupTargetCredentialSecret)
-	if err != nil {
-		return err
-	}
-
-	interval, err := sc.ds.GetSettingAsInt(types.SettingNameBackupstorePollInterval)
-	if err != nil {
-		return err
-	}
-	pollInterval := time.Duration(interval) * time.Second
-
-	backupTarget, err := sc.ds.GetBackupTarget(types.DefaultBackupTargetName)
-	if err != nil {
-		if !datastore.ErrorIsNotFound(err) {
-			return err
-		}
-
-		// Create the default BackupTarget CR if not present
-		backupTarget, err = sc.ds.CreateBackupTarget(&longhorn.BackupTarget{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: types.DefaultBackupTargetName,
-			},
-			Spec: longhorn.BackupTargetSpec{
-				BackupTargetURL:  targetSetting.Value,
-				CredentialSecret: secretSetting.Value,
-				PollInterval:     metav1.Duration{Duration: pollInterval},
-			},
-		})
-		if err != nil {
-			return errors.Wrap(err, "failed to create backup target")
-		}
-	}
-
-	existingBackupTarget := backupTarget.DeepCopy()
-	backupTarget.Spec.BackupTargetURL = targetSetting.Value
-	backupTarget.Spec.CredentialSecret = secretSetting.Value
-	backupTarget.Spec.PollInterval = metav1.Duration{Duration: pollInterval}
-	if !reflect.DeepEqual(existingBackupTarget.Spec, backupTarget.Spec) {
-		// Force sync backup target once the BackupTarget spec be updated
-		backupTarget.Spec.SyncRequestedAt = metav1.Time{Time: time.Now().UTC()}
-		if _, err = sc.ds.UpdateBackupTarget(backupTarget); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // updateTaintToleration deletes all user-deployed and system-managed components immediately with the updated taint toleration.
 func (sc *SettingController) updateTaintToleration() error {
 	setting, err := sc.ds.GetSettingWithAutoFillingRO(types.SettingNameTaintToleration)
@@ -1605,7 +1542,6 @@ func (info *ClusterInfo) collectSettings() error {
 		types.SettingNameBackingImageCleanupWaitInterval:                          true,
 		types.SettingNameBackingImageRecoveryWaitInterval:                         true,
 		types.SettingNameBackupCompressionMethod:                                  true,
-		types.SettingNameBackupstorePollInterval:                                  true,
 		types.SettingNameBackupConcurrentLimit:                                    true,
 		types.SettingNameConcurrentAutomaticEngineUpgradePerNodeLimit:             true,
 		types.SettingNameConcurrentBackupRestorePerNodeLimit:                      true,
@@ -1663,10 +1599,6 @@ func (info *ClusterInfo) collectSettings() error {
 		settingName := types.SettingName(setting.Name)
 
 		switch {
-		// Setting that require extra processing to identify their general purpose
-		case settingName == types.SettingNameBackupTarget:
-			settingMap[setting.Name] = types.GetBackupTargetSchemeFromURL(setting.Value)
-
 		// Setting that should be collected as boolean (true if configured, false if not)
 		case includeAsBoolean[settingName]:
 			definition, ok := types.GetSettingDefinition(types.SettingName(setting.Name))

datastore/longhorn.go

@@ -6,6 +6,7 @@ import (
 	"math/bits"
 	"math/rand"
 	"net/url"
+	"reflect"
 	"regexp"
 	"runtime"
 	"strconv"
@@ -55,7 +56,7 @@ func (s *DataStore) UpdateCustomizedSettings(defaultImages map[types.SettingName
 		return err
 	}
 
-	customizedDefaultSettings, err := types.GetCustomizedDefaultSettings(defaultSettingCM)
+	customizedDefaultSettings, customizedDefaultBackupTargetSettings, err := types.GetCustomizedDefaultSettings(defaultSettingCM)
 	if err != nil {
 		return err
 	}
@@ -74,6 +75,10 @@ func (s *DataStore) UpdateCustomizedSettings(defaultImages map[types.SettingName
 		return err
 	}
 
+	if err := s.syncDefaultBackupTargetResourceWithCustomizedSettings(customizedDefaultBackupTargetSettings); err != nil {
+		return err
+	}
+
 	return s.syncSettingCRsWithCustomizedDefaultSettings(availableCustomizedDefaultSettings, defaultSettingCM.ResourceVersion)
 }
 
@@ -210,9 +215,71 @@ func (s *DataStore) applyCustomizedDefaultSettingsToDefinitions(customizedDefaul
 	return nil
 }
 
+func (s *DataStore) syncDefaultBackupTargetResourceWithCustomizedSettings(customizedDefaultBackupTargetSettings map[string]string) error {
+	backupTarget := &longhorn.BackupTarget{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: types.DefaultBackupTargetName,
+		},
+		Spec: longhorn.BackupTargetSpec{
+			PollInterval: metav1.Duration{Duration: types.DefaultBackupstorePollInterval},
+		},
+	}
+	backupTargetURL, urlExists := customizedDefaultBackupTargetSettings[string(types.SettingNameBackupTarget)]
+	backupTargetCredentialSecret, secretExists := customizedDefaultBackupTargetSettings[string(types.SettingNameBackupTargetCredentialSecret)]
+	pollIntervalStr, pollInterExists := customizedDefaultBackupTargetSettings[string(types.SettingNameBackupstorePollInterval)]
+
+	if backupTargetURL != "" {
+		backupTarget.Spec.BackupTargetURL = backupTargetURL
+	}
+	if backupTargetCredentialSecret != "" {
+		backupTarget.Spec.CredentialSecret = backupTargetCredentialSecret
+	}
+
+	if pollIntervalStr != "" {
+		pollIntervalAsInt, err := strconv.ParseInt(pollIntervalStr, 10, 64)
+		if err != nil {
+			return err
+		}
+		backupTarget.Spec.PollInterval = metav1.Duration{Duration: time.Duration(pollIntervalAsInt) * time.Second}
+	}
+
+	existingBackupTarget, err := s.GetBackupTarget(types.DefaultBackupTargetName)
+	if err != nil {
+		if !ErrorIsNotFound(err) {
+			return err
+		}
+		_, err = s.CreateBackupTarget(backupTarget)
+		if apierrors.IsAlreadyExists(err) {
+			return nil // Already exists, no need to return error
+		}
+		return err
+	}
+
+	// If the settings are not in the longhorn-default-setting ConfigMap, use the existing values.
+	if !urlExists {
+		backupTarget.Spec.BackupTargetURL = existingBackupTarget.Spec.BackupTargetURL
+	}
+	if !secretExists {
+		backupTarget.Spec.CredentialSecret = existingBackupTarget.Spec.CredentialSecret
+	}
+	if !pollInterExists {
+		backupTarget.Spec.PollInterval = existingBackupTarget.Spec.PollInterval
+	}
+	syncTime := metav1.Time{Time: time.Now().UTC()}
+	backupTarget.Spec.SyncRequestedAt = syncTime
+	existingBackupTarget.Spec.SyncRequestedAt = syncTime
+
+	if reflect.DeepEqual(existingBackupTarget.Spec, backupTarget.Spec) {
+		return nil // No changes, no need to update
+	}
+
+	existingBackupTarget.Spec = backupTarget.Spec
+	_, err = s.UpdateBackupTarget(existingBackupTarget)
+	return err
+}
+
 func (s *DataStore) syncSettingCRsWithCustomizedDefaultSettings(customizedDefaultSettings map[string]string, defaultSettingCMResourceVersion string) error {
 	for _, sName := range types.SettingNameList {
-
 		definition, ok := types.GetSettingDefinition(sName)
 		if !ok {
 			return fmt.Errorf("BUG: setting %v is not defined", sName)
@@ -289,65 +356,6 @@ func (s *DataStore) ValidateSetting(name, value string) (err error) {
 	}
 
 	switch sName {
-	case types.SettingNameBackupTarget:
-		vs, err := s.ListDRVolumesRO()
-		if err != nil {
-			return errors.Wrapf(err, "failed to list standby volume when modifying BackupTarget")
-		}
-		if len(vs) != 0 {
-			standbyVolumeNames := make([]string, len(vs))
-			for k := range vs {
-				standbyVolumeNames = append(standbyVolumeNames, k)
-			}
-			return fmt.Errorf("cannot modify BackupTarget since there are existing standby volumes: %v", standbyVolumeNames)
-		}
-		if err := s.ValidateBackupTargetURL(types.DefaultBackupTargetName, value); err != nil {
-			return err
-		}
-	case types.SettingNameBackupTargetCredentialSecret:
-		secret, err := s.GetSecretRO(s.namespace, value)
-		if err != nil {
-			if !apierrors.IsNotFound(err) {
-				return errors.Wrapf(err, "failed to get the secret before modifying backup target credential secret setting")
-			}
-			return nil
-		}
-		checkKeyList := []string{
-			types.AWSAccessKey,
-			types.AWSIAMRoleAnnotation,
-			types.AWSIAMRoleArn,
-			types.AWSAccessKey,
-			types.AWSSecretKey,
-			types.AWSEndPoint,
-			types.AWSCert,
-			types.CIFSUsername,
-			types.CIFSPassword,
-			types.AZBlobAccountName,
-			types.AZBlobAccountKey,
-			types.AZBlobEndpoint,
-			types.AZBlobCert,
-			types.HTTPSProxy,
-			types.HTTPProxy,
-			types.NOProxy,
-			types.VirtualHostedStyle,
-		}
-		for _, checkKey := range checkKeyList {
-			if value, ok := secret.Data[checkKey]; ok {
-				if strings.TrimSpace(string(value)) != string(value) {
-					switch {
-					case strings.TrimLeft(string(value), " ") != string(value):
-						return fmt.Errorf("invalid leading white space in %s", checkKey)
-					case strings.TrimRight(string(value), " ") != string(value):
-						return fmt.Errorf("invalid trailing white space in %s", checkKey)
-					case strings.TrimLeft(string(value), "\n") != string(value):
-						return fmt.Errorf("invalid leading new line in %s", checkKey)
-					case strings.TrimRight(string(value), "\n") != string(value):
-						return fmt.Errorf("invalid trailing new line in %s", checkKey)
-					}
-					return fmt.Errorf("invalid white space or new line in %s", checkKey)
-				}
-			}
-		}
 	case types.SettingNamePriorityClass:
 		if value != "" {
 			if _, err := s.GetPriorityClass(value); err != nil {