Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(spdk): Support changing volume size #2646

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions controller/engine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1279,18 +1279,20 @@ func IsValidForExpansion(engine *longhorn.Engine, cliAPIVersion, imAPIVersion in
return false, fmt.Errorf("the expected size %v of engine %v should not be smaller than the current size %v", engine.Spec.VolumeSize, engine.Name, engine.Status.CurrentSize)
}

if cliAPIVersion < engineapi.CLIAPIMinVersionForExistingEngineBeforeUpgrade {
return false, nil
}
if datastore.IsDataEngineV1(engine.Spec.DataEngine) {
if cliAPIVersion < engineapi.CLIAPIMinVersionForExistingEngineBeforeUpgrade {
return false, nil
}

if !engineapi.IsEndpointTGTBlockDev(engine.Status.Endpoint) {
return true, nil
}
if cliAPIVersion < 7 {
return false, fmt.Errorf("failed to do online expansion for the old engine %v with cli API version %v", engine.Name, cliAPIVersion)
}
if imAPIVersion < 3 {
return false, fmt.Errorf("failed do online expansion for the engine %v in the instance manager with API version %v", engine.Name, imAPIVersion)
if !engineapi.IsEndpointTGTBlockDev(engine.Status.Endpoint) {
return true, nil
}
if cliAPIVersion < 7 {
return false, fmt.Errorf("failed to do online expansion for the old engine %v with cli API version %v", engine.Name, cliAPIVersion)
}
if imAPIVersion < 3 {
return false, fmt.Errorf("failed do online expansion for the engine %v in the instance manager with API version %v", engine.Name, imAPIVersion)
}
}

return true, nil
Expand Down
6 changes: 0 additions & 6 deletions webhook/resources/volume/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,6 @@

if datastore.IsDataEngineV2(newVolume.Spec.DataEngine) {
// TODO: remove this check when we support the following features for SPDK volumes
if oldVolume.Spec.Size != newVolume.Spec.Size {
err := fmt.Errorf("changing volume size for volume %v is not supported for data engine %v",
newVolume.Name, newVolume.Spec.DataEngine)
return werror.NewInvalidError(err.Error(), "")
}

if oldVolume.Spec.NumberOfReplicas != newVolume.Spec.NumberOfReplicas {
err := fmt.Errorf("changing number of replicas for volume %v is not supported for data engine %v",
newVolume.Name, newVolume.Spec.DataEngine)
Expand Down Expand Up @@ -498,51 +492,51 @@
return nil
}

func (v *volumeValidator) validateUpdatingSnapshotMaxCountAndSize(oldVolume, 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) == 0 {
return nil
}

// It is dangerous to update snapshotMaxCount and snapshotMaxSize while migrating. However, when upgrading to a
// Longhorn version that includes these fields from one that does not, we automatically set snapshotMaxCount = 250,
// and we do not want a validation failure here to stop the upgrade. We accept the change, but do not propagate it
// to engines or replicas until the migration is complete.
if len(engines) >= 2 {
if oldVolume.Spec.SnapshotMaxCount == 0 &&
newVolume.Spec.SnapshotMaxCount == types.MaxSnapshotNum &&
oldVolume.Spec.SnapshotMaxSize == newVolume.Spec.SnapshotMaxSize {
logrus.WithField("volumeName", newVolume.Name).Debugf("Allowing snapshotMaxCount of a migrating volume to change from 0 to %v during upgrade", types.MaxSnapshotNum)
} else {
return werror.NewInvalidError("can't update snapshotMaxCount or snapshotMaxSize during migration", "")
}
}

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

Check notice on line 541 in webhook/resources/volume/validator.go

View check run for this annotation

codefactor.io / CodeFactor

webhook/resources/volume/validator.go#L495-L541

Complex Method
}