diff --git a/controller/setting_controller.go b/controller/setting_controller.go index fb67870017..9607e1e265 100644 --- a/controller/setting_controller.go +++ b/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)) diff --git a/datastore/longhorn.go b/datastore/longhorn.go index b048152a01..c88e9ae659 100644 --- a/datastore/longhorn.go +++ b/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,57 @@ 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 := customizedDefaultBackupTargetSettings[string(types.SettingNameBackupTarget)] + backupTargetCredentialSecret := customizedDefaultBackupTargetSettings[string(types.SettingNameBackupTargetCredentialSecret)] + pollIntervalStr := 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 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 +342,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 { diff --git a/types/setting.go b/types/setting.go index 62c8274405..98b5c3b125 100644 --- a/types/setting.go +++ b/types/setting.go @@ -2,8 +2,6 @@ package types import ( "fmt" - "net/url" - "regexp" "strconv" "strings" "sync" @@ -33,6 +31,8 @@ const ( MaxSnapshotNum = 250 DefaultMinNumberOfCopies = 3 + + DefaultBackupstorePollInterval = 300 * time.Second ) type SettingType string @@ -50,8 +50,6 @@ const ( type SettingName string const ( - SettingNameBackupTarget = SettingName("backup-target") - SettingNameBackupTargetCredentialSecret = SettingName("backup-target-credential-secret") SettingNameAllowRecurringJobWhileVolumeDetached = SettingName("allow-recurring-job-while-volume-detached") SettingNameCreateDefaultDiskLabeledNodes = SettingName("create-default-disk-labeled-nodes") SettingNameDefaultDataPath = SettingName("default-data-path") @@ -73,7 +71,6 @@ const ( SettingNameDefaultReplicaCount = SettingName("default-replica-count") SettingNameDefaultDataLocality = SettingName("default-data-locality") SettingNameDefaultLonghornStaticStorageClass = SettingName("default-longhorn-static-storage-class") - SettingNameBackupstorePollInterval = SettingName("backupstore-poll-interval") SettingNameTaintToleration = SettingName("taint-toleration") SettingNameSystemManagedComponentsNodeSelector = SettingName("system-managed-components-node-selector") SettingNameCRDAPIVersion = SettingName("crd-api-version") @@ -140,12 +137,16 @@ const ( SettingNameDefaultMinNumberOfBackingImageCopies = SettingName("default-min-number-of-backing-image-copies") SettingNameBackupExecutionTimeout = SettingName("backup-execution-timeout") SettingNameRWXVolumeFastFailover = SettingName("rwx-volume-fast-failover") + // These three backup target settings are used in the "longhorn-default-setting" ConfigMap + // to update the default BackupTarget resource. + // Longhorn won't create the Setting resources for the three settings. + SettingNameBackupTarget = SettingName("backup-target") + SettingNameBackupTargetCredentialSecret = SettingName("backup-target-credential-secret") + SettingNameBackupstorePollInterval = SettingName("backupstore-poll-interval") ) var ( SettingNameList = []SettingName{ - SettingNameBackupTarget, - SettingNameBackupTargetCredentialSecret, SettingNameAllowRecurringJobWhileVolumeDetached, SettingNameCreateDefaultDiskLabeledNodes, SettingNameDefaultDataPath, @@ -167,7 +168,6 @@ var ( SettingNameDefaultReplicaCount, SettingNameDefaultDataLocality, SettingNameDefaultLonghornStaticStorageClass, - SettingNameBackupstorePollInterval, SettingNameTaintToleration, SettingNameSystemManagedComponentsNodeSelector, SettingNameCRDAPIVersion, @@ -266,8 +266,6 @@ var settingDefinitionsLock sync.RWMutex var ( settingDefinitions = map[SettingName]SettingDefinition{ - SettingNameBackupTarget: SettingDefinitionBackupTarget, - SettingNameBackupTargetCredentialSecret: SettingDefinitionBackupTargetCredentialSecret, SettingNameAllowRecurringJobWhileVolumeDetached: SettingDefinitionAllowRecurringJobWhileVolumeDetached, SettingNameCreateDefaultDiskLabeledNodes: SettingDefinitionCreateDefaultDiskLabeledNodes, SettingNameDefaultDataPath: SettingDefinitionDefaultDataPath, @@ -289,7 +287,6 @@ var ( SettingNameDefaultReplicaCount: SettingDefinitionDefaultReplicaCount, SettingNameDefaultDataLocality: SettingDefinitionDefaultDataLocality, SettingNameDefaultLonghornStaticStorageClass: SettingDefinitionDefaultLonghornStaticStorageClass, - SettingNameBackupstorePollInterval: SettingDefinitionBackupstorePollInterval, SettingNameTaintToleration: SettingDefinitionTaintToleration, SettingNameSystemManagedComponentsNodeSelector: SettingDefinitionSystemManagedComponentsNodeSelector, SettingNameCRDAPIVersion: SettingDefinitionCRDAPIVersion, @@ -358,24 +355,6 @@ var ( SettingNameRWXVolumeFastFailover: SettingDefinitionRWXVolumeFastFailover, } - SettingDefinitionBackupTarget = SettingDefinition{ - DisplayName: "Default Backup Target", - Description: "The endpoint used to access the default backupstore. NFS, CIFS and S3 are supported.", - Category: SettingCategoryBackup, - Type: SettingTypeString, - Required: false, - ReadOnly: false, - } - - SettingDefinitionBackupTargetCredentialSecret = SettingDefinition{ - DisplayName: "Default Backup Target Credential Secret", - Description: "The name of the Kubernetes secret associated with the default backup target.", - Category: SettingCategoryBackup, - Type: SettingTypeString, - Required: false, - ReadOnly: false, - } - SettingDefinitionAllowRecurringJobWhileVolumeDetached = SettingDefinition{ DisplayName: "Allow Recurring Job While Volume Is Detached", Description: "If this setting is enabled, Longhorn will automatically attaches the volume and takes snapshot/backup when it is the time to do recurring snapshot/backup. \n\n" + @@ -388,19 +367,6 @@ var ( Default: "false", } - SettingDefinitionBackupstorePollInterval = SettingDefinition{ - DisplayName: "Default Backupstore Poll Interval", - Description: "In seconds. The default backupstore poll interval determines how often Longhorn checks the backupstore for new backups. Set to 0 to disable the polling.", - Category: SettingCategoryBackup, - Type: SettingTypeInt, - Required: true, - ReadOnly: false, - Default: "300", - ValueIntRange: map[string]int{ - ValueIntRangeMinimum: 0, - }, - } - SettingDefinitionFailedBackupTTL = SettingDefinition{ DisplayName: "Failed Backup Time to Live", Description: "In minutes. This setting determines how long Longhorn will keep the backup resource that was failed. Set to 0 to disable the auto-deletion.\n" + @@ -1590,16 +1556,25 @@ func isValidChoice(choices []string, value string) bool { return len(choices) == 0 } -func GetCustomizedDefaultSettings(defaultSettingCM *corev1.ConfigMap) (defaultSettings map[string]string, err error) { +func GetCustomizedDefaultSettings(defaultSettingCM *corev1.ConfigMap) (defaultSettings, defaultBackupTargetSettings map[string]string, err error) { defaultSettingYAMLData := []byte(defaultSettingCM.Data[DefaultSettingYAMLFileName]) + defaultBackupTargetSettings = map[string]string{} + defaultSettings, err = getDefaultSettingFromYAML(defaultSettingYAMLData) if err != nil { - return nil, err + return nil, nil, err } // won't accept partially valid result for name, value := range defaultSettings { + if name == string(SettingNameBackupTarget) || + name == string(SettingNameBackupTargetCredentialSecret) || + name == string(SettingNameBackupstorePollInterval) { + defaultBackupTargetSettings[name] = value + continue + } + value = strings.Trim(value, " ") definition, exist := GetSettingDefinition(SettingName(name)) if !exist { @@ -1649,7 +1624,7 @@ func GetCustomizedDefaultSettings(defaultSettingCM *corev1.ConfigMap) (defaultSe defaultSettings = map[string]string{} } - return defaultSettings, nil + return defaultSettings, defaultBackupTargetSettings, nil } func getDefaultSettingFromYAML(defaultSettingYAMLData []byte) (map[string]string, error) { @@ -1836,25 +1811,6 @@ func validateString(sName SettingName, definition SettingDefinition, value strin return errors.Wrapf(err, "the value of %v is invalid", sName) } - case SettingNameBackupTarget: - u, err := url.Parse(value) - if err != nil { - return errors.Wrapf(err, "failed to parse %v as url", value) - } - - // Check whether have $ or , have been set in BackupTarget path - regStr := `[\$\,]` - if u.Scheme == "cifs" { - // The $ in SMB/CIFS URIs means that the share is hidden. - regStr = `[\,]` - } - - reg := regexp.MustCompile(regStr) - findStr := reg.FindAllString(u.Path, -1) - if len(findStr) != 0 { - return fmt.Errorf("value %s, contains %v", value, strings.Join(findStr, " or ")) - } - case SettingNameStorageNetwork: if err := ValidateStorageNetwork(value); err != nil { return errors.Wrapf(err, "the value of %v is invalid", sName) diff --git a/webhook/resources/systembackup/validator.go b/webhook/resources/systembackup/validator.go index 65f8093949..2a8100405d 100644 --- a/webhook/resources/systembackup/validator.go +++ b/webhook/resources/systembackup/validator.go @@ -39,11 +39,6 @@ func (v *systemBackupValidator) Resource() admission.Resource { } func (v *systemBackupValidator) Create(request *admission.Request, newObj runtime.Object) error { - _, err := v.ds.GetSettingValueExisted(types.SettingNameBackupTarget) - if err != nil { - return werror.NewBadRequest(err.Error()) - } - backupTarget, err := v.ds.GetBackupTargetRO(types.DefaultBackupTargetName) if err != nil { return werror.NewBadRequest(err.Error())