diff --git a/CHANGELOG.md b/CHANGELOG.md index 69c42ef10..4fba0e46d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ## Changes since v18.04.0 **Fixes:** +- Fixed cleanup of the transaction object upon failed deletion (Issue [#126](https://github.com/NetApp/trident/issues/126)). - **Kubernetes:** Fixed an issue where Trident would provision a ReadWriteMany PVC to an iSCSI backend. - **Kubernetes:** Fixed an installer issue where the Trident PV could be bound to the wrong PVC. - **Kubernetes:** Fixed an issue where the Trident installer could fail if a default storage class is set. diff --git a/config/config.go b/config/config.go index 4a2bd33ea..c41c4b5ed 100644 --- a/config/config.go +++ b/config/config.go @@ -25,6 +25,7 @@ type Telemetry struct { const ( /* Misc. orchestrator constants */ OrchestratorName = "trident" + OrchestratorClientName = OrchestratorName + "ctl" orchestratorVersion = "18.07.0" OrchestratorAPIVersion = "1" PersistentStoreBootstrapAttempts = 30 diff --git a/core/orchestrator_core.go b/core/orchestrator_core.go index aabb006fd..6eb778d37 100644 --- a/core/orchestrator_core.go +++ b/core/orchestrator_core.go @@ -306,14 +306,16 @@ func (o *TridentOrchestrator) rollBackTransaction(v *persistentstore.VolumeTrans // the volume is still in etcd. In this case, it will have been // loaded into memory when previously bootstrapping. if _, ok := o.volumes[v.Config.Name]; ok { - // Ignore errors, since the volume may no longer exist on the - // backend log.WithFields(log.Fields{ "name": v.Config.Name, }).Info("Volume for delete transaction found.") err := o.deleteVolume(v.Config.Name) if err != nil { - return fmt.Errorf("unable to clean up deleted volume %s: %v", v.Config.Name, err) + log.WithFields(log.Fields{ + "volume": v.Config.Name, + "error": err, + }).Errorf("Unable to finalize deletion of the volume! ", + "Repeat deleting the volume using %s.", config.OrchestratorClientName) } } else { log.WithFields(log.Fields{ @@ -890,6 +892,9 @@ func (o *TridentOrchestrator) addVolumeCleanup( } } err = fmt.Errorf("%s", strings.Join(errList, "\n\t")) + log.Warnf("Unable to clean up artifacts of volume creation: %v. ", + "Repeat creating the volume or restart %v.", + err, config.OrchestratorName) } return } @@ -979,7 +984,7 @@ func (o *TridentOrchestrator) deleteVolume(volumeName string) error { // Note that this call will only return an error if the backend actually // fails to delete the volume. If the volume does not exist on the backend, - // the nDVP will not return an error. Thus, we're fine. + // the driver will not return an error. Thus, we're fine. if err := volumeBackend.RemoveVolume(volume); err != nil { log.WithFields(log.Fields{ "volume": volumeName, @@ -1017,14 +1022,11 @@ func (o *TridentOrchestrator) deleteVolume(volumeName string) error { // DeleteVolume does the necessary set up to delete a volume during the course // of normal operation, verifying that the volume is present in Trident and -// creating a transaction to ensure that the delete eventually completes. It -// only resolves the transaction if all stages of deletion complete -// successfully, ensuring that the deletion will complete either upon retrying -// the delete or upon reboot of Trident. -// Returns true if the volume is found and false otherwise. -func (o *TridentOrchestrator) DeleteVolume(volumeName string) error { +// creating a transaction to ensure that the delete eventually completes. +func (o *TridentOrchestrator) DeleteVolume(volumeName string) (err error) { if o.bootstrapError != nil { - return o.bootstrapError + err = o.bootstrapError + return } o.mutex.Lock() @@ -1042,19 +1044,25 @@ func (o *TridentOrchestrator) DeleteVolume(volumeName string) error { if err := o.storeClient.AddVolumeTransaction(volTxn); err != nil { return err } - if err := o.deleteVolume(volumeName); err != nil { + + defer func() { + if errTxn := o.storeClient.DeleteVolumeTransaction(volTxn); errTxn != nil { + log.WithFields(log.Fields{ + "volume": volume, + "error": errTxn, + }).Warnf("Unable to delete volume transaction. ", + "Repeat deletion using %s or restart %v.", + config.OrchestratorClientName, config.OrchestratorName) + } + }() + + if err = o.deleteVolume(volumeName); err != nil { // Do not try to delete the volume transaction here; instead, if we // fail, leave the transaction around and let the deletion be attempted // again. return err } - if err := o.storeClient.DeleteVolumeTransaction(volTxn); err != nil { - log.WithFields(log.Fields{ - "volume": volume, - }).Warn("Unable to delete volume transaction. Repeat deletion to finalize.") - // Reinsert the volume so that it can be deleted again - o.volumes[volumeName] = volume - } + return nil } diff --git a/docs/docker/install/ndvp_ontap_config.rst b/docs/docker/install/ndvp_ontap_config.rst index ac4c16334..64c529b07 100644 --- a/docs/docker/install/ndvp_ontap_config.rst +++ b/docs/docker/install/ndvp_ontap_config.rst @@ -39,7 +39,7 @@ In addition to the global configuration values above, when using ONTAP these top +-----------------------+----------------------------------------------------------------------------+------------+ | Option | Description | Example | -+=======================+===========================================================================-+============+ ++=======================+============================================================================+============+ | ``managementLIF`` | IP address of ONTAP management LIF | 10.0.0.1 | +-----------------------+----------------------------------------------------------------------------+------------+ | ``dataLIF`` | IP address of protocol LIF; will be derived if not specified | 10.0.0.2 |