Skip to content

Commit 15c5767

Browse files
authored
Delete ONTAP FlexVols by UUID to avoid REST name-index orphans
ONTAP's REST name index is updated asynchronously, so a just-created FlexVol can be reported as already gone by a name-based lookup. The destroy path's VolumeExists pre-check then skipped the delete and left the volume orphaned.
1 parent 408c4fc commit 15c5767

27 files changed

Lines changed: 917 additions & 240 deletions

core/concurrent_core.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3132,6 +3132,8 @@ func (o *ConcurrentTridentOrchestrator) cloneVolume(
31323132
// Clear these values as they were copied from the source volume Config
31333133
cloneConfig.SubordinateVolumes = make(map[string]interface{})
31343134
cloneConfig.ShareSourceVolume = ""
3135+
// The clone gets its own backing volume, whose ID the driver records when it creates it.
3136+
cloneConfig.BackendVolumeID = ""
31353137
cloneConfig.LUKSPassphraseNames = sourceVolume.Config.LUKSPassphraseNames
31363138
// Override this value only if SplitOnClone has been defined in clone volume's config
31373139
if volConfig.SplitOnClone != "" {

core/orchestrator_core.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2652,6 +2652,8 @@ func (o *TridentOrchestrator) cloneVolumeInitial(
26522652
// Clear these values as they were copied from the source volume Config
26532653
cloneConfig.SubordinateVolumes = make(map[string]interface{})
26542654
cloneConfig.ShareSourceVolume = ""
2655+
// The clone gets its own backing volume, whose ID the driver records when it creates it.
2656+
cloneConfig.BackendVolumeID = ""
26552657
cloneConfig.LUKSPassphraseNames = sourceVolume.Config.LUKSPassphraseNames
26562658
// Override this value only if SplitOnClone has been defined in clone volume's config
26572659
if volumeConfig.SplitOnClone != "" {

mocks/mock_storage_drivers/mock_ontap/mock_api.go

Lines changed: 23 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

mocks/mock_storage_drivers/mock_ontap/mock_ontap_rest_interface.go

Lines changed: 23 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

storage/volume.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ type VolumeConfig struct {
7171
// PeerVolumeHandle is the internal volume handle for the source volume if this volume is a mirror destination
7272
PeerVolumeHandle string `json:"requiredPeerVolumeHandle,omitempty"`
7373
// InternalID is an optional, backend-specific identifier to help find an object
74-
InternalID string `json:"internalID,omitempty"`
74+
InternalID string `json:"internalID,omitempty"`
75+
// BackendVolumeID is a stable, backend-assigned identifier for the volume's backing object.
76+
BackendVolumeID string `json:"backendVolumeID,omitempty"`
7577
ShareSourceVolume string `json:"shareSourceVolume"`
7678
SubordinateVolumes map[string]interface{} `json:"-"`
7779
Namespace string `json:"namespace"`

storage_drivers/ontap/api/abstraction.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,23 @@ type OntapAPI interface {
212212
VolumeCloneCreate(ctx context.Context, cloneName, sourceName, snapshot string, async bool) error
213213
VolumeCloneSplitStart(ctx context.Context, cloneName string) error
214214

215-
VolumeCreate(ctx context.Context, volume Volume) error
216-
VolumeCreateBalanced(ctx context.Context, volume Volume) error
215+
// VolumeCreate creates a volume and returns the UUID the backend assigned to it, which is empty
216+
// when the backend does not report one (for example on the ZAPI path).
217+
VolumeCreate(ctx context.Context, volume Volume) (string, error)
218+
VolumeCreateBalanced(ctx context.Context, volume Volume) (string, error)
217219
VolumeModify(ctx context.Context, volume Volume) error
218220
// VolumeDestroy keeps both flags in the shared abstraction because ZAPI and REST expose
219221
// delete semantics differently. ZAPI consumes force directly and may need a follow-up
220222
// recovery-queue purge when skipRecoveryQueue is true. REST only exposes one delete flag at
221223
// the transport layer, so the REST adapter maps skipRecoveryQueue onto that flag while keeping
222224
// this caller-facing signature stable.
223225
VolumeDestroy(ctx context.Context, volumeName string, force, skipRecoveryQueue bool) error
226+
// VolumeDestroyByUUID deletes the volume identified by volumeUUID. Deleting by UUID avoids the
227+
// name index, which ONTAP updates asynchronously and which can therefore report a newly created
228+
// volume as missing. It returns a NotFoundError once the UUID no longer resolves, and an
229+
// UnsupportedError when the caller must fall back to deleting by name (no UUID provided, or
230+
// the ZAPI path which cannot delete by UUID).
231+
VolumeDestroyByUUID(ctx context.Context, volumeUUID, volumeName string, force, skipRecoveryQueue bool) error
224232
VolumeMove(
225233
ctx context.Context, volumeName, destinationAggregateName string, dryRun bool,
226234
) (string, error)

storage_drivers/ontap/api/abstraction_rest.go

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ func (d OntapAPIREST) ValidateAPIVersion(ctx context.Context) error {
174174
return nil
175175
}
176176

177-
func (d OntapAPIREST) VolumeCreate(ctx context.Context, volume Volume) error {
177+
func (d OntapAPIREST) VolumeCreate(ctx context.Context, volume Volume) (string, error) {
178178
fields := LogFields{
179179
"Method": "VolumeCreate",
180180
"Type": "OntapAPIREST",
@@ -190,17 +190,17 @@ func (d OntapAPIREST) VolumeCreate(ctx context.Context, volume Volume) error {
190190
aggregateName = volume.Aggregates[0]
191191
}
192192

193-
creationErr := d.api.VolumeCreate(ctx, volume.Name, aggregateName, volume.Size, volume.SpaceReserve,
193+
volumeUUID, creationErr := d.api.VolumeCreate(ctx, volume.Name, aggregateName, volume.Size, volume.SpaceReserve,
194194
volume.SnapshotPolicy, volume.UnixPermissions, volume.ExportPolicy, volume.SecurityStyle,
195195
volume.TieringPolicy, volume.Comment, volume.Qos, volume.Encrypt, volume.SnapshotReserve, volume.DPVolume)
196196
if creationErr != nil {
197-
return fmt.Errorf("error creating volume: %v", creationErr)
197+
return "", fmt.Errorf("error creating volume: %v", creationErr)
198198
}
199199

200-
return nil
200+
return volumeUUID, nil
201201
}
202202

203-
func (d OntapAPIREST) VolumeCreateBalanced(ctx context.Context, volume Volume) error {
203+
func (d OntapAPIREST) VolumeCreateBalanced(ctx context.Context, volume Volume) (string, error) {
204204
fields := LogFields{
205205
"Method": "VolumeCreateBalanced",
206206
"Type": "OntapAPIREST",
@@ -213,17 +213,17 @@ func (d OntapAPIREST) VolumeCreateBalanced(ctx context.Context, volume Volume) e
213213

214214
// Double-check this API is supported
215215
if !d.SupportsFeature(ctx, BalancedPlacement) {
216-
return errors.UnsupportedError("ONTAP version does not support balanced placement")
216+
return "", errors.UnsupportedError("ONTAP version does not support balanced placement")
217217
}
218218

219-
creationErr := d.api.VolumeCreateBalanced(ctx, volume.Name, volume.Size, volume.SpaceReserve,
219+
volumeUUID, creationErr := d.api.VolumeCreateBalanced(ctx, volume.Name, volume.Size, volume.SpaceReserve,
220220
volume.SnapshotPolicy, volume.UnixPermissions, volume.ExportPolicy, volume.SecurityStyle,
221221
volume.TieringPolicy, volume.Comment, volume.Qos, volume.Encrypt, volume.SnapshotReserve, volume.DPVolume)
222222
if creationErr != nil {
223-
return fmt.Errorf("error creating volume: %v", creationErr)
223+
return "", fmt.Errorf("error creating volume: %v", creationErr)
224224
}
225225

226-
return nil
226+
return volumeUUID, nil
227227
}
228228

229229
func (d OntapAPIREST) VolumeModify(ctx context.Context, volume Volume) error {
@@ -233,14 +233,61 @@ func (d OntapAPIREST) VolumeModify(ctx context.Context, volume Volume) error {
233233
func (d OntapAPIREST) VolumeDestroy(ctx context.Context, name string, force, skipRecoveryQueue bool) error {
234234
destroyVolume := func() error {
235235
deletionErr := d.api.VolumeDestroy(ctx, name, skipRecoveryQueue)
236-
if deletionErr != nil && !IsVolumeBusyRESTError(deletionErr) {
237-
return backoff.Permanent(fmt.Errorf("error destroying volume %v: %v", name, deletionErr))
238-
}
239-
if deletionErr != nil {
240-
return fmt.Errorf("error destroying volume %v: %v", name, deletionErr)
236+
return classifyVolumeDeleteError(name, deletionErr)
237+
}
238+
return d.retryVolumeDestroy(ctx, name, destroyVolume)
239+
}
240+
241+
// VolumeDestroyByUUID deletes the volume identified by volumeUUID. Addressing the volume by UUID
242+
// avoids the lag in ONTAP's name index, which can report a just-created volume as missing. A UUID
243+
// that no longer resolves yields a NotFoundError, meaning the delete already completed.
244+
func (d OntapAPIREST) VolumeDestroyByUUID(
245+
ctx context.Context, volumeUUID, volumeName string, force, skipRecoveryQueue bool,
246+
) error {
247+
fields := LogFields{
248+
"Method": "VolumeDestroyByUUID",
249+
"Type": "OntapAPIREST",
250+
"volume": volumeName,
251+
"uuid": volumeUUID,
252+
}
253+
Logd(ctx, d.driverName,
254+
d.api.ClientConfig().DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> VolumeDestroyByUUID")
255+
defer Logd(ctx, d.driverName,
256+
d.api.ClientConfig().DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< VolumeDestroyByUUID")
257+
258+
if volumeUUID == "" {
259+
return errors.UnsupportedError("no volume UUID provided; delete by name")
260+
}
261+
262+
destroyVolume := func() error {
263+
deletionErr := d.api.VolumeDestroyByUUID(ctx, volumeUUID, skipRecoveryQueue)
264+
if errors.IsNotFoundError(deletionErr) {
265+
return backoff.Permanent(errors.NotFoundError("volume %s (UUID %s) does not exist", volumeName, volumeUUID))
241266
}
267+
return classifyVolumeDeleteError(volumeName, deletionErr)
268+
}
269+
return d.retryVolumeDestroy(ctx, volumeName, destroyVolume)
270+
}
271+
272+
// classifyVolumeDeleteError wraps a raw delete error so that a busy volume is retried while any
273+
// other failure aborts the retry loop immediately. A NotFoundError passes through with its type
274+
// intact, since callers rely on it to tell an already-deleted volume from a real failure.
275+
func classifyVolumeDeleteError(name string, deletionErr error) error {
276+
if deletionErr == nil {
242277
return nil
243278
}
279+
if errors.IsNotFoundError(deletionErr) {
280+
return backoff.Permanent(deletionErr)
281+
}
282+
if !IsVolumeBusyRESTError(deletionErr) {
283+
return backoff.Permanent(fmt.Errorf("error destroying volume %v: %v", name, deletionErr))
284+
}
285+
return fmt.Errorf("error destroying volume %v: %v", name, deletionErr)
286+
}
287+
288+
// retryVolumeDestroy runs a delete operation with the shared busy-volume backoff, unwrapping the
289+
// permanent error (which may be a NotFoundError the caller treats as success) when the loop stops.
290+
func (d OntapAPIREST) retryVolumeDestroy(ctx context.Context, name string, destroyVolume func() error) error {
244291
destroyNotify := func(err error, duration time.Duration) {
245292
Logc(ctx).WithField("increment", duration).Debug("Volume busy, waiting.")
246293
}

0 commit comments

Comments
 (0)