Trident fails to start after rebooting a cluster when failed trident …

…transactions are present

Trident fails to start after rebooting a cluster when failed trident transactions are present

core/orchestrator_core.go

@@ -650,13 +650,6 @@ func (o *TridentOrchestrator) handleFailedTransaction(ctx context.Context, v *st
  "snapshot": v.SnapshotConfig.Name,
  "op": v.Op,
  }).Info("Processed snapshot transaction log.")
- case storage.UpgradeVolume:
- Logc(ctx).WithFields(LogFields{
- "volume": v.Config.Name,
- "PVC": v.PVUpgradeConfig.PVCConfig.Name,
- "PV": v.PVUpgradeConfig.PVConfig.Name,
- "op": v.Op,
- }).Info("Processed volume upgrade transaction log.")
  case storage.VolumeCreating:
  Logc(ctx).WithFields(LogFields{
  "volume": v.VolumeCreatingConfig.Name,
@@ -758,8 +751,9 @@ func (o *TridentOrchestrator) handleFailedTransaction(ctx context.Context, v *st
  }
  // Snapshot deletion is an idempotent operation, so it's safe to
  // delete an already deleted snapshot.
+ // If the volume gets deleted before the snapshot, the snapshot deletion returns "NotFoundError".
  err := backend.DeleteSnapshot(ctx, v.SnapshotConfig, v.Config)
- if err != nil && !errors.IsUnsupportedError(err) {
+ if err != nil && !errors.IsUnsupportedError(err) && !errors.IsNotFoundError(err) {
  return fmt.Errorf("error attempting to clean up snapshot %s from backend %s: %v",
  v.SnapshotConfig.Name, backend.Name(), err)
  }
@@ -864,7 +858,7 @@ func (o *TridentOrchestrator) handleFailedTransaction(ctx context.Context, v *st
  return fmt.Errorf("failed to clean up volume addition transaction: %v", err)
  }
 
- case storage.UpgradeVolume, storage.VolumeCreating:
+ case storage.VolumeCreating:
  // Do nothing
  }
 
@@ -2709,7 +2703,7 @@ func (o *TridentOrchestrator) AddVolumeTransaction(ctx context.Context, volTxn *
  return err
  }
  if oldTxn != nil {
- if oldTxn.Op != storage.UpgradeVolume && oldTxn.Op != storage.VolumeCreating {
+ if oldTxn.Op != storage.VolumeCreating {
  err = o.handleFailedTransaction(ctx, oldTxn)
  if err != nil {
  return fmt.Errorf("unable to process the preexisting transaction for volume %s: %v",

core/orchestrator_core_test.go

@@ -7422,6 +7422,26 @@ func TestHandleFailedSnapshot(t *testing.T) {
  err = o.handleFailedTransaction(ctx(), vt)
  assert.Error(t, err, "Delete snapshot error")
 
+ // DeleteSnapshot returns UnSupported error
+ mockBackend2.EXPECT().State().Return(storage.Unknown).AnyTimes()
+ mockBackend.EXPECT().State().Return(storage.Online)
+ mockBackend.EXPECT().DeleteSnapshot(coreCtx, gomock.Any(),
+ gomock.Any()).Return(errors.UnsupportedError("failed to delete snapshot"))
+ mockStoreClient.EXPECT().DeleteVolumeTransaction(coreCtx,
+ gomock.Any()).Return(errors.New("failed to delete transaction"))
+ err = o.handleFailedTransaction(ctx(), vt)
+ assert.Error(t, err, "Delete snapshot error")
+
+ // DeleteSnapshot returns NotFound error
+ mockBackend2.EXPECT().State().Return(storage.Unknown).AnyTimes()
+ mockBackend.EXPECT().State().Return(storage.Online)
+ mockBackend.EXPECT().DeleteSnapshot(coreCtx, gomock.Any(),
+ gomock.Any()).Return(errors.NotFoundError("failed to delete snapshot"))
+ mockStoreClient.EXPECT().DeleteVolumeTransaction(coreCtx,
+ gomock.Any()).Return(errors.New("failed to delete transaction"))
+ err = o.handleFailedTransaction(ctx(), vt)
+ assert.Error(t, err, "Delete snapshot error")
+
  delete(o.backends, "xyz")
  mockBackend.EXPECT().State().Return(storage.Online)
  mockBackend.EXPECT().DeleteSnapshot(coreCtx, gomock.Any(), gomock.Any()).Return(nil)

persistent_store/crd/apis/netapp/v1/transaction_test.go

@@ -7,7 +7,6 @@ import (
  "reflect"
  "testing"
 
- v1 "k8s.io/api/core/v1"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
  "k8s.io/apimachinery/pkg/runtime"
 
@@ -103,56 +102,6 @@ func TestNewSnapshotTransaction(t *testing.T) {
  }
 }
 
-func TestNewUpgradeTransaction(t *testing.T) {
- // Build volume transaction
- volConfig := &storage.VolumeConfig{
- Version: string(config.OrchestratorAPIVersion),
- Name: "volumeTransaction",
- Size: "1GB",
- Protocol: config.File,
- StorageClass: "gold",
- }
- pvConfig := &v1.PersistentVolume{}
- pvcConfig := &v1.PersistentVolumeClaim{}
-
- upgradeConfig := &storage.PVUpgradeConfig{
- PVConfig: pvConfig,
- PVCConfig: pvcConfig,
- OwnedPodsForPVC: []string{"myPod1", "myPod2"},
- }
- txn := &storage.VolumeTransaction{
- Op: "upgradeVolume",
- Config: volConfig,
- PVUpgradeConfig: upgradeConfig,
- }
-
- // Convert to Kubernetes Object using NewTridentTransaction
- volumeTransaction, err := NewTridentTransaction(txn)
- if err != nil {
- t.Fatal("Unable to construct TridentTransaction CRD: ", err)
- }
-
- // Build expected Kubernetes Object
- expected := &TridentTransaction{
- TypeMeta: metav1.TypeMeta{
- APIVersion: "trident.netapp.io/v1",
- Kind: "TridentTransaction",
- },
- ObjectMeta: metav1.ObjectMeta{
- Name: NameFix(volConfig.Name),
- Finalizers: GetTridentFinalizers(),
- },
- Transaction: runtime.RawExtension{
- Raw: MustEncode(json.Marshal(txn)),
- },
- }
-
- // Compare
- if !reflect.DeepEqual(volumeTransaction, expected) {
- t.Fatalf("TridentTransaction does not match expected result, got %v expected %v", volumeTransaction, expected)
- }
-}
-
 func TestTransaction_Persistent(t *testing.T) {
  // Build volume transaction
  volConfig := &storage.VolumeConfig{
@@ -268,73 +217,3 @@ func TestSnapshotTransaction_Persistent(t *testing.T) {
  persistentSnapshotConfig, expectedSnapshotConfig)
  }
 }
-
-func TestUpgradeTransaction_Persistent(t *testing.T) {
- // Build volume transaction
- volConfig := &storage.VolumeConfig{
- Version: string(config.OrchestratorAPIVersion),
- Name: "volumeTransaction",
- Size: "1GB",
- Protocol: config.File,
- StorageClass: "gold",
- }
-
- pvConfig := &v1.PersistentVolume{}
- pvcConfig := &v1.PersistentVolumeClaim{}
-
- upgradeConfig := &storage.PVUpgradeConfig{
- PVConfig: pvConfig,
- PVCConfig: pvcConfig,
- OwnedPodsForPVC: []string{"myPod1", "myPod2"},
- }
-
- txn := &storage.VolumeTransaction{
- Op: "upgradeVolume",
- Config: volConfig,
- PVUpgradeConfig: upgradeConfig,
- }
-
- // Build Kubernetes Object
- volumeTransaction := &TridentTransaction{
- TypeMeta: metav1.TypeMeta{
- APIVersion: "trident.netapp.io/v1",
- Kind: "TridentTransaction",
- },
- ObjectMeta: metav1.ObjectMeta{
- Name: NameFix(volConfig.Name),
- Finalizers: GetTridentFinalizers(),
- },
- Transaction: runtime.RawExtension{
- Raw: MustEncode(json.Marshal(txn)),
- },
- }
-
- // Build persistent object by calling TridentBackend.Persistent
- txn, err := volumeTransaction.Persistent()
- if err != nil {
- t.Fatal("Unable to construct TridentTransaction persistent object: ", err)
- }
-
- persistentOp := txn.Op
- persistentVolumeConfig := txn.Config
- persistentUpgradeConfig := txn.PVUpgradeConfig
-
- // Build expected persistent object
- expectedOp := "upgradeVolume"
- expectedVolumeConfig := volConfig
- expectedUpgradeConfig := upgradeConfig
-
- // Compare
- if string(persistentOp) != expectedOp {
- t.Fatalf("TridentTransaction does not match expected result, got %v expected %v",
- persistentOp, expectedOp)
- }
- if !reflect.DeepEqual(persistentVolumeConfig, expectedVolumeConfig) {
- t.Fatalf("TridentTransaction does not match expected result, got %v expected %v",
- persistentVolumeConfig, expectedVolumeConfig)
- }
- if !reflect.DeepEqual(persistentUpgradeConfig, expectedUpgradeConfig) {
- t.Fatalf("TridentTransaction does not match expected result, got %v expected %v",
- persistentUpgradeConfig, expectedUpgradeConfig)
- }
-}

persistent_store/passthrough_test.go

@@ -63,13 +63,10 @@ func getFakeVolumeTransactionWithName(name string) *storage.VolumeTransaction {
  }
  snapshotConfig := &storage.SnapshotConfig{}
 
- upgradeConfig := &storage.PVUpgradeConfig{}
-
  return &storage.VolumeTransaction{
- Config: volumeConfig,
- SnapshotConfig: snapshotConfig,
- PVUpgradeConfig: upgradeConfig,
- Op: storage.AddVolume,
+ Config: volumeConfig,
+ SnapshotConfig: snapshotConfig,
+ Op: storage.AddVolume,
  }
 }
 

storage/volume_transaction.go

@@ -2,10 +2,6 @@
 
 package storage
 
-import (
- v1 "k8s.io/api/core/v1"
-)
-
 type VolumeOperation string
 
 const (
@@ -14,7 +10,6 @@ const (
  DeleteVolume VolumeOperation = "deleteVolume"
  ImportVolume VolumeOperation = "importVolume"
  ResizeVolume VolumeOperation = "resizeVolume"
- UpgradeVolume VolumeOperation = "upgradeVolume"
  AddSnapshot VolumeOperation = "addSnapshot"
  DeleteSnapshot VolumeOperation = "deleteSnapshot"
 
@@ -26,16 +21,9 @@ type VolumeTransaction struct {
  Config *VolumeConfig
  VolumeCreatingConfig *VolumeCreatingConfig
  SnapshotConfig *SnapshotConfig
- PVUpgradeConfig *PVUpgradeConfig
  Op VolumeOperation
 }
 
-type PVUpgradeConfig struct {
- PVCConfig *v1.PersistentVolumeClaim `json:"pvcConfig,omitempty"`
- PVConfig *v1.PersistentVolume `json:"pvConfig,omitempty"`
- OwnedPodsForPVC []string `json:"ownedPodsForPVC,omitempty"`
-}
-
 // Name returns a unique identifier for the VolumeTransaction. Volume transactions should only
 // be identified by their name, while snapshot transactions should be identified by their name as
 // well as their volume name. It's possible that some situations will leave a delete transaction

storage_drivers/azure/azure_anf_subvolume.go

@@ -1191,7 +1191,7 @@ func (d *NASBlockStorageDriver) GetSnapshot(
  return nil, fmt.Errorf("could not find source subvolume '%s'; %v", volConfig.InternalID, err)
  }
  if !sourceSubvolumeExists {
- return nil, fmt.Errorf("source subvolume '%s' does not exist", volConfig.Name)
+ return nil, errors.NotFoundError(fmt.Sprintf("source subvolume '%s' does not exist", volConfig.Name))
  }
 
  // For snapshot imports, creation token should be the internal name for the backend snapshot.

storage_drivers/ontap/api/abstraction_zapi.go

@@ -455,8 +455,8 @@ func (d OntapAPIZAPI) LunGetByName(ctx context.Context, name string) (*Lun, erro
  d.api.ClientConfig().DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< LunGetByName")
 
  lunResponse, err := d.api.LunGet(name)
- if err != nil || lunResponse == nil {
- return nil, fmt.Errorf("could not get LUN by name %v, error: %v", name, err)
+ if err != nil {
+ return nil, err
  }
 
  lun, err := lunInfoFromZapiAttrsHelper(*lunResponse)
@@ -2501,7 +2501,7 @@ func (d OntapAPIZAPI) NVMeEnsureNamespaceUnmapped(ctx context.Context, hostNQN,
 }
 
 func (d OntapAPIZAPI) NVMeNamespaceGetSize(ctx context.Context, subsystemName string) (int, error) {
- return 0, fmt.Errorf("ZAPI call is not supported yet")
+ return 0, errors.UnsupportedError("ZAPI call is not supported yet")
 }
 
 func (d OntapAPIZAPI) VolumeWaitForStates(ctx context.Context, volumeName string, desiredStates,