Repository navigation
Conversation
CreateSnapshotResponse and ListSnapshots entries never set SizeBytes, so VolumeSnapshot.status.restoreSize was always 0. Kubernetes uses this value to show the minimum restore size and to reject PVCs that are too small. Use the snapshot virtualsize from CloudStack, falling back to the source volume size. Also populate Size/CreatedAt in GetSnapshotByID and Size in GetSnapshotByName so callers see the real snapshot size.
CloudStack creates a volume from a snapshot at the snapshot's size and ignores a larger requested size. The driver returned that smaller capacity, so external-provisioner rejected it with 'created volume capacity X less than requested capacity Y', deleted the volume and retried forever. After creating the volume, resize it to the requested size when it is smaller. If the resize fails, delete the undersized volume and return an error. The node plugin already grows the filesystem on NodeStage.
CreateSnapshot always called CloudStack createSnapshot, so a retry from csi-snapshotter (e.g. after a timeout) created a duplicate snapshot. The CSI spec requires CreateSnapshot to return the existing snapshot when one with the same name and source volume already exists. Look up the snapshot by name first; return it if it belongs to the same volume, return AlreadyExists if it belongs to a different volume, and only create a new snapshot when none exists.
… state A timed-out CreateSnapshot keeps running against CloudStack, so a retry could miss the in-progress snapshot and create a duplicate. Hold a per-name lock for the call and return Aborted to concurrent calls. CreateSnapshot and ListSnapshots always returned ReadyToUse: true, even while CloudStack was still creating or backing up the snapshot. Report ReadyToUse=false until the snapshot leaves Allocated/Creating/ CreatedOnPrimary/BackingUp/Copying, and an error for the Error state.
If a snapshot was destroyed outside the driver, or an earlier delete completed after the RPC timed out, CloudStack returns errorcode 431 '... is already destroyed'. That was not mapped to ErrNotFound, so DeleteSnapshot returned Internal and the VolumeSnapshotContent (and its VolumeSnapshot) could never be deleted. The CSI spec requires DeleteSnapshot to succeed when the snapshot no longer exists.
…elete as done CloudStack returns error 4350 (InvalidParameterValueException) both for an unknown ID and for refusals such as deleting a volume that is still attached. DeleteVolume mapped every 4350 to ErrNotFound, so a refused delete was reported as success: Kubernetes removed the PV and the CloudStack volume was left behind with nothing tracking it. DeleteSnapshot matched error text instead, which missed messages such as 'unable to find a snapshot with id N', leaving VolumeSnapshots stuck. The cloud client now returns delete errors unchanged. After a failed delete, the controller looks the volume or snapshot up by ID and only reports success if it is gone or already destroyed. A volume that is still attached returns FailedPrecondition; anything else returns the CloudStack error.
…he maximum size before restoring
When a restore from snapshot took longer than the provisioner's timeout,
the retry found the volume by name and returned it without a content
source and after comparing it with the StorageClass's disk offering. The
restored volume keeps the snapshot's offering, and external-provisioner
deletes a restored volume without a content source ('volume content
source missing'), so every retry copied the snapshot from secondary
storage again and the PVC never bound.
A retry for a restore now reuses the existing volume: it is grown if an
earlier attempt stopped before the resize, checked for size and topology
only, and returned with its content source.
A restore larger than the snapshot is now checked against CloudStack's
maximum custom disk size (listCapabilities) before anything is created.
Previously each retry restored the whole snapshot only for the resize to
fail; now the request fails at once with OutOfRange.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #: Fixes #46, Fixes #47
Description of changes:
Depends on #43; only the last commit is new. Independent of the sidecar update in #44.
Retries reuse the restored volume (#46): skip the disk-offering check, grow the volume if needed, check size and topology, and return it with its ContentSource.
Maximum size is checked first (#47): a restore larger than the snapshot is checked against customdiskofferingmaxsize (listCapabilities) and fails with OutOfRange before anything is created.
Unit tests for both, plus a small refactor of the existing-volume handling to stay within lint limits.
Testing performed: lint, make test and make test-sanity pass. Lab (Kubernetes 1.37.0, CloudStack 4.23, KVM/NFS)
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.