From c6c608f6b499523388ef576ed502571701790a9a Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Wed, 8 Nov 2017 11:39:13 -0600 Subject: [PATCH] ontap-nas-economy fixes for 17.10.1 Making qtree delete idempotent as needed by Trident Allow deleting qtrees with really long names --- CHANGELOG.md | 12 ++++++- storage_drivers/ontap_nas_qtree.go | 56 +++++++++++++++++++++++++----- storage_drivers/storage_drivers.go | 2 +- 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6e32ad..625c9d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,17 @@ [Releases](https://github.com/NetApp/netappdvp/releases) -## Changes since 17.07.0 +## Changes since 17.10.0 + +**Fixes:** +- Added delete idempotency to the ontap-nas-economy driver as needed by Trident. +- Fixed an issue where qtrees with names near the 64-character limit could not + be deleted. + +**Enhancements:** + + +## 17.10.0 **Fixes:** - Changed the SolidFire driver to tear down the iSCSI connection as part diff --git a/storage_drivers/ontap_nas_qtree.go b/storage_drivers/ontap_nas_qtree.go index 0e597d3..90435ec 100644 --- a/storage_drivers/ontap_nas_qtree.go +++ b/storage_drivers/ontap_nas_qtree.go @@ -21,6 +21,7 @@ import ( // OntapNASQtreeStorageDriverName is the constant name for this Ontap qtree-based NAS storage driver const OntapNASQtreeStorageDriverName = "ontap-nas-economy" const deletedQtreeNamePrefix = "deleted_" +const maxQtreeNameLength = 64 const maxQtreesPerFlexvol = 200 const defaultPruneFlexvolsPeriodSecs = uint64(600) // default to 10 minutes const defaultResizeQuotasPeriodSecs = uint64(60) // default to 1 minute @@ -171,11 +172,12 @@ func (d *OntapNASQtreeStorageDriver) startHousekeepingTasks() { // Keep the system devoid of Flexvols with no qtrees d.pruneUnusedFlexvols() + d.reapDeletedQtrees() pruneTicker := time.NewTicker(time.Duration(pruneFlexvolsPeriodSecs) * time.Second) go func() { - for t := range pruneTicker.C { - log.WithField("tick", t).Debug("Running housekeeping task: prune unused Flexvols.") + for range pruneTicker.C { d.pruneUnusedFlexvols() + d.reapDeletedQtrees() } }() @@ -183,8 +185,7 @@ func (d *OntapNASQtreeStorageDriver) startHousekeepingTasks() { d.resizeQuotas() resizeTicker := time.NewTicker(time.Duration(resizeQuotasPeriodSecs) * time.Second) go func() { - for t := range resizeTicker.C { - log.WithField("tick", t).Debug("Running housekeeping task: resize quotas.") + for range resizeTicker.C { d.resizeQuotas() } }() @@ -219,6 +220,11 @@ func (d *OntapNASQtreeStorageDriver) Create(name string, sizeBytes uint64, opts return fmt.Errorf("Volume %s already exists.", name) } + // Ensure qtree name isn't too long + if len(name) > maxQtreeNameLength { + return fmt.Errorf("Volume %s name exceeds the limit of %d characters.", name, maxQtreeNameLength) + } + // Get Flexvol options with default fallback values // see also: ontap_common.go#PopulateConfigurationDefaults size := strconv.FormatUint(sizeBytes, 10) @@ -336,11 +342,19 @@ func (d *OntapNASQtreeStorageDriver) Destroy(name string) error { } if !exists { log.WithField("qtree", name).Warn("Qtree not found.") + return nil } - // Rename qtree so it doesn't show up in lists while ONTAP is deleting it in the background + // Rename qtree so it doesn't show up in lists while ONTAP is deleting it in the background. + // Ensure the deleted name doesn't exceed the qtree name length limit of 64 characters. path := fmt.Sprintf("/vol/%s/%s", flexvol, name) - deletedPath := fmt.Sprintf("/vol/%s/%s", flexvol, deletedQtreeNamePrefix+name+"_"+utils.RandomString(5)) + deletedName := deletedQtreeNamePrefix + name + "_" + utils.RandomString(5) + if len(deletedName) > maxQtreeNameLength { + trimLength := len(deletedQtreeNamePrefix) + 10 + deletedName = deletedQtreeNamePrefix + name[trimLength:] + "_" + utils.RandomString(5) + } + deletedPath := fmt.Sprintf("/vol/%s/%s", flexvol, deletedName) + renameResponse, err := d.API.QtreeRename(path, deletedPath) if err = ontap.GetError(renameResponse, err); err != nil { log.Errorf("Qtree rename failed. %v", err) @@ -801,6 +815,8 @@ func (d *OntapNASQtreeStorageDriver) resizeQuotas() { d.provMutex.Lock() defer d.provMutex.Unlock() + log.Debug("Housekeeping, resizing quotas.") + for flexvol, resize := range d.quotaResizeMap { if resize { @@ -872,6 +888,8 @@ func (d *OntapNASQtreeStorageDriver) pruneUnusedFlexvols() { d.provMutex.Lock() defer d.provMutex.Unlock() + log.Debug("Housekeeping, checking for managed Flexvols with no qtrees.") + // Get list of Flexvols managed by this driver volumeListResponse, err := d.API.VolumeList(d.FlexvolNamePrefix()) if err = ontap.GetError(volumeListResponse, err); err != nil { @@ -885,8 +903,6 @@ func (d *OntapNASQtreeStorageDriver) pruneUnusedFlexvols() { flexvols = append(flexvols, volName) } - log.WithField("flexvols", len(flexvols)).Debug("Housekeeping, checking for managed Flexvols with qtrees.") - // Destroy any Flexvol if it is devoid of qtrees for _, flexvol := range flexvols { qtreeCount, err := d.API.QtreeCount(flexvol) @@ -897,6 +913,30 @@ func (d *OntapNASQtreeStorageDriver) pruneUnusedFlexvols() { } } +// reapDeletedQtrees is called periodically by a background task. Any qtrees +// that have been deleted (discovered by virtue of having a well-known hardcoded +// prefix on their names) are destroyed. This is only needed for the exceptional case +// in which a qtree was renamed (prior to being destroyed) but the subsequent +// destroy call failed or was never made due to a process interruption. +func (d *OntapNASQtreeStorageDriver) reapDeletedQtrees() { + + log.Debug("Housekeeping, checking for deleted qtrees.") + + // Get all deleted qtrees in all FlexVols managed by this driver + prefix := deletedQtreeNamePrefix + *d.Config.StoragePrefix + listResponse, err := d.API.QtreeList(prefix, d.FlexvolNamePrefix()) + if err = ontap.GetError(listResponse, err); err != nil { + log.Errorf("Error listing deleted qtrees. %v", err) + } + + // AttributesList() returns []QtreeInfoType + for _, qtree := range listResponse.Result.AttributesList() { + qtreePath := fmt.Sprintf("/vol/%s/%s", qtree.Volume(), qtree.Qtree()) + log.WithField("qtree", qtreePath).Debug("Housekeeping, reaping deleted qtree.") + d.API.QtreeDestroyAsync(qtreePath, true) + } +} + // ensureDefaultExportPolicy checks for an export policy with a well-known name that will be suitable // for setting on a Flexvol and will enable access to all qtrees therein. If the policy exists, the // method assumes it created the policy itself and that all is good. If the policy does not exist, diff --git a/storage_drivers/storage_drivers.go b/storage_drivers/storage_drivers.go index 263ee04..ceee368 100644 --- a/storage_drivers/storage_drivers.go +++ b/storage_drivers/storage_drivers.go @@ -24,7 +24,7 @@ const ( const ConfigVersion = 1 // DriverVersion is the actual release version number -const DriverVersion = "17.10.0" +const DriverVersion = "17.10.1" // FullDriverVersion is the DriverVersion as well as any pre-release tags var FullDriverVersion = DriverVersion