From 30befe511aec4c809ea4111c71811642ba8401d2 Mon Sep 17 00:00:00 2001 From: PoAn Yang Date: Wed, 3 Jan 2024 23:15:27 +0800 Subject: [PATCH] feat: support snapshot max count and size Signed-off-by: PoAn Yang --- api/model.go | 8 +++ api/router.go | 2 + api/volume.go | 44 ++++++++++++++++ controller/engine_controller.go | 18 +++++++ controller/volume_controller.go | 25 ++++++++++ datastore/longhorn.go | 8 +++ engineapi/engine.go | 20 ++++++++ engineapi/enginesim.go | 8 +++ engineapi/instance_manager.go | 4 ++ engineapi/proxy_volume.go | 10 ++++ engineapi/types.go | 4 ++ manager/volume.go | 52 +++++++++++++++++++ types/setting.go | 19 +++++-- upgrade/v15xto160/upgrade.go | 2 + webhook/resources/volume/mutator.go | 37 ++++++++++++++ webhook/resources/volume/validator.go | 72 +++++++++++++++++++++++++++ 16 files changed, 330 insertions(+), 3 deletions(-) diff --git a/api/model.go b/api/model.go index 1d92778c2b..04ea8289dc 100644 --- a/api/model.go +++ b/api/model.go @@ -330,6 +330,14 @@ type UpdateReplicaDiskSoftAntiAffinityInput struct { ReplicaDiskSoftAntiAffinity string `json:"replicaDiskSoftAntiAffinity"` } +type UpdateSnapshotMaxCount struct { + SnapshotMaxCount int `json:"snapshotMaxCount"` +} + +type UpdateSnapshotMaxSize struct { + SnapshotMaxSize int64 `json:"snapshotMaxSize"` +} + type PVCreateInput struct { PVName string `json:"pvName"` FSType string `json:"fsType"` diff --git a/api/router.go b/api/router.go index 65f6ae1239..3ad6ba9827 100644 --- a/api/router.go +++ b/api/router.go @@ -75,6 +75,8 @@ func NewRouter(s *Server) *mux.Router { "updateDataLocality": s.VolumeUpdateDataLocality, "updateAccessMode": s.VolumeUpdateAccessMode, "updateUnmapMarkSnapChainRemoved": s.VolumeUpdateUnmapMarkSnapChainRemoved, + "updateSnapshotMaxCount": s.VolumeUpdateSnapshotMaxCount, + "updateSnapshotMaxSize": s.VolumeUpdateSnapshotMaxSize, "updateReplicaSoftAntiAffinity": s.VolumeUpdateReplicaSoftAntiAffinity, "updateReplicaZoneSoftAntiAffinity": s.VolumeUpdateReplicaZoneSoftAntiAffinity, "updateReplicaDiskSoftAntiAffinity": s.VolumeUpdateReplicaDiskSoftAntiAffinity, diff --git a/api/volume.go b/api/volume.go index 9789769cb0..8073bed79a 100644 --- a/api/volume.go +++ b/api/volume.go @@ -779,3 +779,47 @@ func (s *Server) EngineUpgrade(rw http.ResponseWriter, req *http.Request) error return s.responseWithVolume(rw, req, id, v) } + +func (s *Server) VolumeUpdateSnapshotMaxCount(rw http.ResponseWriter, req *http.Request) error { + var input UpdateSnapshotMaxCount + id := mux.Vars(req)["name"] + + apiContext := api.GetApiContext(req) + if err := apiContext.Read(&input); err != nil { + return errors.Wrap(err, "failed to read SnapshotMaxCount input") + } + + obj, err := util.RetryOnConflictCause(func() (interface{}, error) { + return s.m.UpdateSnapshotMaxCount(id, input.SnapshotMaxCount) + }) + if err != nil { + return err + } + v, ok := obj.(*longhorn.Volume) + if !ok { + return fmt.Errorf("failed to convert to volume %v object", id) + } + return s.responseWithVolume(rw, req, "", v) +} + +func (s *Server) VolumeUpdateSnapshotMaxSize(rw http.ResponseWriter, req *http.Request) error { + var input UpdateSnapshotMaxSize + id := mux.Vars(req)["name"] + + apiContext := api.GetApiContext(req) + if err := apiContext.Read(&input); err != nil { + return errors.Wrap(err, "failed to read SnapshotMaxSize input") + } + + obj, err := util.RetryOnConflictCause(func() (interface{}, error) { + return s.m.UpdateSnapshotMaxSize(id, input.SnapshotMaxSize) + }) + if err != nil { + return err + } + v, ok := obj.(*longhorn.Volume) + if !ok { + return fmt.Errorf("failed to convert to volume %v object", id) + } + return s.responseWithVolume(rw, req, "", v) +} diff --git a/controller/engine_controller.go b/controller/engine_controller.go index 9cd7faf563..761c4028fd 100644 --- a/controller/engine_controller.go +++ b/controller/engine_controller.go @@ -986,6 +986,24 @@ func (m *EngineMonitor) refresh(engine *longhorn.Engine) error { volumeInfo.UnmapMarkSnapChainRemoved, engine.Spec.UnmapMarkSnapChainRemovedEnabled) } } + + engine.Status.SnapshotMaxCount = volumeInfo.SnapshotMaxCount + if engine.Spec.SnapshotMaxCount != volumeInfo.SnapshotMaxCount { + logrus.Infof("Correcting flag SnapshotMaxCount from %d to %d", volumeInfo.SnapshotMaxCount, engine.Spec.SnapshotMaxCount) + if err := engineClientProxy.VolumeSnapshotMaxCountSet(engine); err != nil { + return errors.Wrapf(err, "failed to correct flag SnapshotMaxCount from %d to %d", + volumeInfo.SnapshotMaxCount, engine.Spec.SnapshotMaxCount) + } + } + + engine.Status.SnapshotMaxSize = volumeInfo.SnapshotMaxSize + if engine.Spec.SnapshotMaxSize != volumeInfo.SnapshotMaxSize { + logrus.Infof("Correcting flag SnapshotMaxSize from %d to %d", volumeInfo.SnapshotMaxSize, engine.Spec.SnapshotMaxSize) + if err := engineClientProxy.VolumeSnapshotMaxSizeSet(engine); err != nil { + return errors.Wrapf(err, "failed to correct flag SnapshotMaxSize from %d to %d", + volumeInfo.SnapshotMaxSize, engine.Spec.SnapshotMaxSize) + } + } } } else { // For incompatible running engine, the current size is always `engine.Spec.VolumeSize`. diff --git a/controller/volume_controller.go b/controller/volume_controller.go index 83d57e77c9..35022834ba 100644 --- a/controller/volume_controller.go +++ b/controller/volume_controller.go @@ -459,6 +459,10 @@ func (c *VolumeController) syncVolume(key string) (err error) { return err } + if err := c.syncVolumeSnapshotSetting(volume, engines, replicas); err != nil { + return err + } + if err := c.updateRecurringJobs(volume); err != nil { return err } @@ -1188,6 +1192,23 @@ func (c *VolumeController) syncVolumeUnmapMarkSnapChainRemovedSetting(v *longhor return nil } +func (c *VolumeController) syncVolumeSnapshotSetting(v *longhorn.Volume, es map[string]*longhorn.Engine, rs map[string]*longhorn.Replica) error { + if es == nil && rs == nil { + return nil + } + + for _, e := range es { + e.Spec.SnapshotMaxCount = v.Spec.SnapshotMaxCount + e.Spec.SnapshotMaxSize = v.Spec.SnapshotMaxSize + } + for _, r := range rs { + r.Spec.SnapshotMaxCount = v.Spec.SnapshotMaxCount + r.Spec.SnapshotMaxSize = v.Spec.SnapshotMaxSize + } + + return nil +} + // ReconcileVolumeState handles the attaching and detaching of volume func (c *VolumeController) ReconcileVolumeState(v *longhorn.Volume, es map[string]*longhorn.Engine, rs map[string]*longhorn.Replica) (err error) { defer func() { @@ -3191,6 +3212,8 @@ func (c *VolumeController) createEngine(v *longhorn.Volume, currentEngineName st ReplicaAddressMap: map[string]string{}, UpgradedReplicaAddressMap: map[string]string{}, RevisionCounterDisabled: v.Spec.RevisionCounterDisabled, + SnapshotMaxCount: v.Spec.SnapshotMaxCount, + SnapshotMaxSize: v.Spec.SnapshotMaxSize, }, } @@ -3242,6 +3265,8 @@ func (c *VolumeController) createReplica(v *longhorn.Volume, e *longhorn.Engine, HardNodeAffinity: hardNodeAffinity, RevisionCounterDisabled: v.Spec.RevisionCounterDisabled, UnmapMarkDiskChainRemovedEnabled: e.Spec.UnmapMarkSnapChainRemovedEnabled, + SnapshotMaxCount: v.Spec.SnapshotMaxCount, + SnapshotMaxSize: v.Spec.SnapshotMaxSize, }, } if isRebuildingReplica { diff --git a/datastore/longhorn.go b/datastore/longhorn.go index c74362a1bd..4fa09c6f8e 100644 --- a/datastore/longhorn.go +++ b/datastore/longhorn.go @@ -411,6 +411,14 @@ func (s *DataStore) ValidateSetting(name, value string) (err error) { if value == "true" && autoCleanupValue { return errors.Errorf("cannot set %v setting to true when %v setting is true", name, types.SettingNameAutoCleanupSystemGeneratedSnapshot) } + case types.SettingNameSnapshotMaxCount: + v, err := strconv.Atoi(value) + if err != nil { + return err + } + if v < 2 || v > 250 { + return fmt.Errorf("%s should be between 2 and 250", name) + } } return nil } diff --git a/engineapi/engine.go b/engineapi/engine.go index 5c22e7f586..062970f601 100644 --- a/engineapi/engine.go +++ b/engineapi/engine.go @@ -290,6 +290,26 @@ func (e *EngineBinary) VolumeUnmapMarkSnapChainRemovedSet(engine *longhorn.Engin return nil } +// VolumeSnapshotMaxCountSet calls engine binary +// TODO: Deprecated, replaced by gRPC proxy +func (e *EngineBinary) VolumeSnapshotMaxCountSet(engine *longhorn.Engine) error { + cmdline := []string{"snapshot-max-count", strconv.Itoa(engine.Spec.SnapshotMaxCount)} + if _, err := e.ExecuteEngineBinary(cmdline...); err != nil { + return errors.Wrapf(err, "error setting volume flag SnapshotMaxCount to %d", engine.Spec.SnapshotMaxCount) + } + return nil +} + +// VolumeSnapshotMaxSizeSet calls engine binary +// TODO: Deprecated, replaced by gRPC proxy +func (e *EngineBinary) VolumeSnapshotMaxSizeSet(engine *longhorn.Engine) error { + cmdline := []string{"snapshot-max-size", strconv.FormatInt(engine.Spec.SnapshotMaxSize, 10)} + if _, err := e.ExecuteEngineBinary(cmdline...); err != nil { + return errors.Wrapf(err, "error setting volume flag SnapshotMaxSize to %d", engine.Spec.SnapshotMaxSize) + } + return nil +} + // ReplicaRebuildVerify calls engine binary // TODO: Deprecated, replaced by gRPC proxy func (e *EngineBinary) ReplicaRebuildVerify(engine *longhorn.Engine, replicaName, url string) error { diff --git a/engineapi/enginesim.go b/engineapi/enginesim.go index 5d75eea3c1..36ab0d06bc 100644 --- a/engineapi/enginesim.go +++ b/engineapi/enginesim.go @@ -245,6 +245,14 @@ func (e *EngineSimulator) VolumeUnmapMarkSnapChainRemovedSet(*longhorn.Engine) e return fmt.Errorf(ErrNotImplement) } +func (e *EngineSimulator) VolumeSnapshotMaxCountSet(*longhorn.Engine) error { + return fmt.Errorf(ErrNotImplement) +} + +func (e *EngineSimulator) VolumeSnapshotMaxSizeSet(*longhorn.Engine) error { + return fmt.Errorf(ErrNotImplement) +} + func (e *EngineSimulator) ReplicaRebuildVerify(engine *longhorn.Engine, replicaName, url string) error { return fmt.Errorf(ErrNotImplement) } diff --git a/engineapi/instance_manager.go b/engineapi/instance_manager.go index 70d2fc2619..ef63d6c5ae 100644 --- a/engineapi/instance_manager.go +++ b/engineapi/instance_manager.go @@ -313,6 +313,8 @@ func getBinaryAndArgsForEngineProcessCreation(e *longhorn.Engine, if engineCLIAPIVersion >= 9 { args = append([]string{"--engine-instance-name", e.Name}, args...) + args = append(args, "--snapshot-max-count", strconv.Itoa(e.Spec.SnapshotMaxCount)) + args = append(args, "--snapshot-max-size", strconv.FormatInt(e.Spec.SnapshotMaxSize, 10)) } for _, addr := range e.Status.CurrentReplicaAddressMap { @@ -354,6 +356,8 @@ func getBinaryAndArgsForReplicaProcessCreation(r *longhorn.Replica, if engineCLIAPIVersion >= 9 { args = append(args, "--replica-instance-name", r.Name) args = append([]string{"--volume-name", r.Spec.VolumeName}, args...) + args = append(args, "--snapshot-max-count", strconv.Itoa(r.Spec.SnapshotMaxCount)) + args = append(args, "--snapshot-max-size", strconv.FormatInt(r.Spec.SnapshotMaxSize, 10)) } // 3 ports are already used by replica server, data server and syncagent server diff --git a/engineapi/proxy_volume.go b/engineapi/proxy_volume.go index 5367ed8983..1aec825081 100644 --- a/engineapi/proxy_volume.go +++ b/engineapi/proxy_volume.go @@ -43,3 +43,13 @@ func (p *Proxy) VolumeUnmapMarkSnapChainRemovedSet(e *longhorn.Engine) error { return p.grpcClient.VolumeUnmapMarkSnapChainRemovedSet(string(e.Spec.DataEngine), e.Name, e.Spec.VolumeName, p.DirectToURL(e), e.Spec.UnmapMarkSnapChainRemovedEnabled) } + +func (p *Proxy) VolumeSnapshotMaxCountSet(e *longhorn.Engine) error { + return p.grpcClient.VolumeSnapshotMaxCountSet(string(e.Spec.BackendStoreDriver), e.Name, e.Spec.VolumeName, + p.DirectToURL(e), e.Spec.SnapshotMaxCount) +} + +func (p *Proxy) VolumeSnapshotMaxSizeSet(e *longhorn.Engine) error { + return p.grpcClient.VolumeSnapshotMaxSizeSet(string(e.Spec.BackendStoreDriver), e.Name, e.Spec.VolumeName, + p.DirectToURL(e), e.Spec.SnapshotMaxSize) +} diff --git a/engineapi/types.go b/engineapi/types.go index 355975dd94..1ef7cac926 100644 --- a/engineapi/types.go +++ b/engineapi/types.go @@ -75,6 +75,8 @@ type EngineClient interface { VolumeFrontendStart(*longhorn.Engine) error VolumeFrontendShutdown(*longhorn.Engine) error VolumeUnmapMarkSnapChainRemovedSet(engine *longhorn.Engine) error + VolumeSnapshotMaxCountSet(engine *longhorn.Engine) error + VolumeSnapshotMaxSizeSet(engine *longhorn.Engine) error ReplicaList(*longhorn.Engine) (map[string]*Replica, error) ReplicaAdd(engine *longhorn.Engine, replicaName, url string, isRestoreVolume, fastSync bool, replicaFileSyncHTTPClientTimeout int64) error @@ -128,6 +130,8 @@ type Volume struct { LastExpansionError string `json:"lastExpansionError"` LastExpansionFailedAt string `json:"lastExpansionFailedAt"` UnmapMarkSnapChainRemoved bool `json:"unmapMarkSnapChainRemoved"` + SnapshotMaxCount int `json:"snapshotMaxCount"` + SnapshotMaxSize int64 `json:"SnapshotMaxSize"` } type BackupTarget struct { diff --git a/manager/volume.go b/manager/volume.go index 7b7b78c0af..193aa4e803 100644 --- a/manager/volume.go +++ b/manager/volume.go @@ -1143,3 +1143,55 @@ func (m *VolumeManager) verifyDataSourceForVolumeCreation(dataSource longhorn.Vo } return nil } + +func (m *VolumeManager) UpdateSnapshotMaxCount(name string, snapshotMaxCount int) (v *longhorn.Volume, err error) { + defer func() { + err = errors.Wrapf(err, "unable to update field SnapshotMaxCount for volume %s", name) + }() + + v, err = m.ds.GetVolume(name) + if err != nil { + return nil, err + } + + if v.Spec.SnapshotMaxCount == snapshotMaxCount { + logrus.Debugf("Volume %s already set field SnapshotMaxCount to %d", v.Name, snapshotMaxCount) + return v, nil + } + + oldSnapshotMaxCount := v.Spec.SnapshotMaxCount + v.Spec.SnapshotMaxCount = snapshotMaxCount + v, err = m.ds.UpdateVolume(v) + if err != nil { + return nil, err + } + + logrus.Infof("Updated volume %s field SnapshotMaxCount from %d to %d", v.Name, oldSnapshotMaxCount, snapshotMaxCount) + return v, nil +} + +func (m *VolumeManager) UpdateSnapshotMaxSize(name string, snapshotMaxSize int64) (v *longhorn.Volume, err error) { + defer func() { + err = errors.Wrapf(err, "unable to update field SnapshotMaxSize for volume %s", name) + }() + + v, err = m.ds.GetVolume(name) + if err != nil { + return nil, err + } + + if v.Spec.SnapshotMaxSize == snapshotMaxSize { + logrus.Debugf("Volume %s already set field SnapshotMaxSize to %d", v.Name, snapshotMaxSize) + return v, nil + } + + oldSnapshotMaxSize := v.Spec.SnapshotMaxSize + v.Spec.SnapshotMaxSize = snapshotMaxSize + v, err = m.ds.UpdateVolume(v) + if err != nil { + return nil, err + } + + logrus.Infof("Updated volume %s field SnapshotMaxSize from %d to %d", v.Name, oldSnapshotMaxSize, snapshotMaxSize) + return v, nil +} diff --git a/types/setting.go b/types/setting.go index 6d25ba2a3e..b8ecbd9d7d 100644 --- a/types/setting.go +++ b/types/setting.go @@ -29,7 +29,7 @@ const ( ValueUnknown = "unknown" // From `maximumChainLength` in longhorn-engine/pkg/replica/replica.go - maxSnapshotNum = 250 + MaxSnapshotNum = 250 ) type SettingType string @@ -104,6 +104,7 @@ const ( SettingNameSnapshotDataIntegrity = SettingName("snapshot-data-integrity") SettingNameSnapshotDataIntegrityImmediateCheckAfterSnapshotCreation = SettingName("snapshot-data-integrity-immediate-check-after-snapshot-creation") SettingNameSnapshotDataIntegrityCronJob = SettingName("snapshot-data-integrity-cronjob") + SettingNameSnapshotMaxCount = SettingName("snapshot-max-count") SettingNameRestoreVolumeRecurringJobs = SettingName("restore-volume-recurring-jobs") SettingNameRemoveSnapshotsDuringFilesystemTrim = SettingName("remove-snapshots-during-filesystem-trim") SettingNameFastReplicaRebuildEnabled = SettingName("fast-replica-rebuild-enabled") @@ -185,6 +186,7 @@ var ( SettingNameSnapshotDataIntegrity, SettingNameSnapshotDataIntegrityCronJob, SettingNameSnapshotDataIntegrityImmediateCheckAfterSnapshotCreation, + SettingNameSnapshotMaxCount, SettingNameRestoreVolumeRecurringJobs, SettingNameRemoveSnapshotsDuringFilesystemTrim, SettingNameFastReplicaRebuildEnabled, @@ -292,6 +294,7 @@ var ( SettingNameSnapshotDataIntegrity: SettingDefinitionSnapshotDataIntegrity, SettingNameSnapshotDataIntegrityImmediateCheckAfterSnapshotCreation: SettingDefinitionSnapshotDataIntegrityImmediateCheckAfterSnapshotCreation, SettingNameSnapshotDataIntegrityCronJob: SettingDefinitionSnapshotDataIntegrityCronJob, + SettingNameSnapshotMaxCount: SettingDefinitionSnapshotMaxCount, SettingNameRestoreVolumeRecurringJobs: SettingDefinitionRestoreVolumeRecurringJobs, SettingNameRemoveSnapshotsDuringFilesystemTrim: SettingDefinitionRemoveSnapshotsDuringFilesystemTrim, SettingNameFastReplicaRebuildEnabled: SettingDefinitionFastReplicaRebuildEnabled, @@ -1052,6 +1055,16 @@ var ( Default: "0 0 */7 * *", } + SettingDefinitionSnapshotMaxCount = SettingDefinition{ + DisplayName: "Snapshot Maximum Count", + Description: "Maximum snapshot count for a volume. The value should be between 2 to 250", + Category: SettingCategorySnapshot, + Type: SettingTypeInt, + Required: true, + ReadOnly: false, + Default: strconv.Itoa(MaxSnapshotNum), + } + SettingDefinitionRemoveSnapshotsDuringFilesystemTrim = SettingDefinition{ DisplayName: "Remove Snapshots During Filesystem Trim", Description: "This setting allows Longhorn filesystem trim feature to automatically mark the latest snapshot and its ancestors as removed and stops at the snapshot containing multiple children.\n\n" + @@ -1455,8 +1468,8 @@ func ValidateSetting(name, value string) (err error) { if err != nil { return errors.Wrapf(err, "value %v is not a number", value) } - if maxNumber < 1 || maxNumber > maxSnapshotNum { - return fmt.Errorf("the value %v should be between 1 and %v", maxNumber, maxSnapshotNum) + if maxNumber < 1 || maxNumber > MaxSnapshotNum { + return fmt.Errorf("the value %v should be between 1 and %v", maxNumber, MaxSnapshotNum) } case SettingNameEngineReplicaTimeout: timeout, err := strconv.Atoi(value) diff --git a/upgrade/v15xto160/upgrade.go b/upgrade/v15xto160/upgrade.go index c024d99016..670537d92c 100644 --- a/upgrade/v15xto160/upgrade.go +++ b/upgrade/v15xto160/upgrade.go @@ -75,6 +75,8 @@ func upgradeVolumes(namespace string, lhClient *lhclientset.Clientset, resourceM } else { v.Spec.DataEngine = longhorn.DataEngineType(v.Spec.BackendStoreDriver) } + + v.Spec.SnapshotMaxCount = types.MaxSnapshotNum } return nil diff --git a/webhook/resources/volume/mutator.go b/webhook/resources/volume/mutator.go index 3c340f2d6f..9f1892a818 100644 --- a/webhook/resources/volume/mutator.go +++ b/webhook/resources/volume/mutator.go @@ -180,6 +180,16 @@ func (v *volumeMutator) Create(request *admission.Request, newObj runtime.Object patchOps = append(patchOps, fmt.Sprintf(`{"op": "replace", "path": "/spec/backupCompressionMethod", "value": "%s"}`, defaultCompressionMethod)) } + if volume.Spec.SnapshotMaxCount == 0 { + snapshotMaxCount, err := v.getSnapshotMaxCount() + if err != nil { + err = errors.Wrap(err, "BUG: cannot get valid number for setting snapshot max count") + return nil, werror.NewInvalidError(err.Error(), "") + } + logrus.Infof("Use the default snapshot max count %v", snapshotMaxCount) + patchOps = append(patchOps, fmt.Sprintf(`{"op": "replace", "path": "/spec/snapshotMaxCount", "value": %v}`, snapshotMaxCount)) + } + // TODO: Remove the mutations below after they are implemented for SPDK volumes if datastore.IsDataEngineV2(volume.Spec.DataEngine) { if volume.Spec.DataLocality != longhorn.DataLocalityDisabled { @@ -219,6 +229,10 @@ func (v *volumeMutator) Create(request *admission.Request, newObj runtime.Object } func (v *volumeMutator) Update(request *admission.Request, oldObj runtime.Object, newObj runtime.Object) (admission.PatchOps, error) { + oldVolume, ok := oldObj.(*longhorn.Volume) + if !ok { + return nil, werror.NewInvalidError(fmt.Sprintf("%v is not a *longhorn.Volume", oldObj), "") + } volume, ok := newObj.(*longhorn.Volume) if !ok { return nil, werror.NewInvalidError(fmt.Sprintf("%v is not a *longhorn.Volume", newObj), "") @@ -239,6 +253,21 @@ func (v *volumeMutator) Update(request *admission.Request, oldObj runtime.Object patchOps = append(patchOps, fmt.Sprintf(`{"op": "replace", "path": "/spec/backupCompressionMethod", "value": "%s"}`, longhorn.BackupCompressionMethodGzip)) } + if volume.Spec.SnapshotMaxCount == 0 { + snapshotMaxCount, err := v.getSnapshotMaxCount() + if err != nil { + err = errors.Wrap(err, "BUG: cannot get valid number for setting snapshot max count") + return nil, werror.NewInvalidError(err.Error(), "") + } + logrus.Infof("Use the default snapshot max count %v", snapshotMaxCount) + patchOps = append(patchOps, fmt.Sprintf(`{"op": "replace", "path": "/spec/snapshotMaxCount", "value": %v}`, snapshotMaxCount)) + } + + // if user expand volume size, we don't want snapshotMaxSize < size*2 blocks the change + if oldVolume.Spec.Size != volume.Spec.Size && volume.Spec.SnapshotMaxSize != 0 && volume.Spec.SnapshotMaxSize < volume.Spec.Size*2 { + patchOps = append(patchOps, fmt.Sprintf(`{"op": "replace", "path": "/spec/snapshotMaxSize", "value": "%s"}`, strconv.FormatInt(volume.Spec.Size*2, 10))) + } + var patchOpsInCommon admission.PatchOps var err error if patchOpsInCommon, err = mutate(newObj, nil); err != nil { @@ -330,3 +359,11 @@ func (v *volumeMutator) getDefaultReplicaCount() (int, error) { } return int(c), nil } + +func (v *volumeMutator) getSnapshotMaxCount() (int, error) { + c, err := v.ds.GetSettingAsInt(types.SettingNameSnapshotMaxCount) + if err != nil { + return 0, err + } + return int(c), nil +} diff --git a/webhook/resources/volume/validator.go b/webhook/resources/volume/validator.go index acd47f3c78..4c665c0932 100644 --- a/webhook/resources/volume/validator.go +++ b/webhook/resources/volume/validator.go @@ -132,6 +132,14 @@ func (v *volumeValidator) Create(request *admission.Request, newObj runtime.Obje return err } + if err := validateSnapshotMaxCount(volume.Spec.SnapshotMaxCount); err != nil { + return werror.NewInvalidError(err.Error(), "spec.snapshotMaxCount") + } + + if err := validateSnapshotMaxSize(volume.Spec.Size, volume.Spec.SnapshotMaxSize); err != nil { + return werror.NewInvalidError(err.Error(), "spec.snapshotMaxSize") + } + if err := v.ds.CheckDataEngineImageCompatiblityByImage(volume.Spec.Image, volume.Spec.DataEngine); err != nil { return werror.NewInvalidError(err.Error(), "volume.spec.image") } @@ -315,6 +323,19 @@ func (v *volumeValidator) Update(request *admission.Request, oldObj runtime.Obje return werror.NewInvalidError(err.Error(), "") } + if err := validateSnapshotMaxCount(newVolume.Spec.SnapshotMaxCount); err != nil { + return werror.NewInvalidError(err.Error(), "spec.snapshotMaxCount") + } + + if err := validateSnapshotMaxSize(newVolume.Spec.Size, newVolume.Spec.SnapshotMaxSize); err != nil { + return werror.NewInvalidError(err.Error(), "spec.snapshotMaxSize") + } + + if (oldVolume.Spec.SnapshotMaxCount != newVolume.Spec.SnapshotMaxCount) || (oldVolume.Spec.SnapshotMaxSize != newVolume.Spec.SnapshotMaxSize) { + if err := v.validateUpdatingSnapshotMaxCountAndSize(newVolume); err != nil { + return err + } + } return nil } @@ -452,3 +473,54 @@ func (v *volumeValidator) canDisableRevisionCounter(image string, dataEngine lon return true, nil } + +func validateSnapshotMaxCount(snapshotMaxCount int) error { + if snapshotMaxCount < 2 || snapshotMaxCount > 250 { + return fmt.Errorf("snapshot max count should be between 2 to 250") + } + return nil +} + +func validateSnapshotMaxSize(size, snapshotMaxSize int64) error { + if snapshotMaxSize != 0 && snapshotMaxSize < size*2 { + return fmt.Errorf("snapshot max size can not be 0 and at least twice of volume size") + } + return nil +} + +func (v *volumeValidator) validateUpdatingSnapshotMaxCountAndSize(newVolume *longhorn.Volume) error { + var ( + currentSnapshotCount int + currentTotalSnapshotSize int64 + engine *longhorn.Engine + ) + engines, err := v.ds.ListVolumeEngines(newVolume.Name) + if err != nil && !datastore.ErrorIsNotFound(err) { + return werror.NewInternalError(fmt.Sprintf("can't list engines for volume %s, err %v", newVolume.Name, err)) + } else if len(engines) >= 2 { + return werror.NewInvalidError("can't update snapshotMaxCount or snapshotMaxSize during migration", "") + } else if len(engines) == 0 { + return nil + } + + for _, e := range engines { + engine = e + } + + for _, snapshotInfo := range engine.Status.Snapshots { + if snapshotInfo == nil || snapshotInfo.Removed || snapshotInfo.Name == "volume-head" { + continue + } + currentSnapshotCount++ + snapshotSize, err := strconv.ParseInt(snapshotInfo.Size, 10, 64) + if err != nil { + return werror.NewInternalError(fmt.Sprintf("can't parse size %s from snapshot %s in volume %s, err %v", snapshotInfo.Size, snapshotInfo.Name, newVolume.Name, err)) + } + currentTotalSnapshotSize += snapshotSize + } + + if currentSnapshotCount > newVolume.Spec.SnapshotMaxCount || (newVolume.Spec.SnapshotMaxSize != 0 && currentTotalSnapshotSize > newVolume.Spec.SnapshotMaxSize) { + return werror.NewInvalidError("can't make snapshotMaxCount or snapshotMaxSize be smaller than current usage, please remove snapshots first", "") + } + return nil +}