25.06 Address volume creations failures caused by stale transactions from Trident restarts

Co-authored-by: Joe Webster <31218426+jwebster7@users.noreply.github.com>

Author: jharrod

cli/k8s_client/k8s_client.go

@@ -1,4 +1,4 @@
-// Copyright 2023 NetApp, Inc. All Rights Reserved.
+// Copyright 2025 NetApp, Inc. All Rights Reserved.
 
 package k8sclient
 

core/orchestrator_core.go

@@ -21,6 +21,7 @@ import (
 	"github.com/netapp/trident/core/cache"
 	"github.com/netapp/trident/frontend"
 	controllerhelpers "github.com/netapp/trident/frontend/csi/controller_helpers"
+	"github.com/netapp/trident/internal/fiji"
 	. "github.com/netapp/trident/logging"
 	persistentstore "github.com/netapp/trident/persistent_store"
 	"github.com/netapp/trident/pkg/capacity"
@@ -47,6 +48,8 @@ const (
 	AttachISCSIVolumeTimeoutLong   = time.Second * 90
 )
 
+var addVolumeAfterAddVolumeTxn = fiji.Register("addVolumeAfterAddVolumeTransaction", "orchestrator_core")
+
 type TridentOrchestrator struct {
 	backends                 map[string]storage.Backend // key is UUID, not name
 	volumes                  map[string]*storage.Volume
@@ -1932,6 +1935,11 @@ func (o *TridentOrchestrator) addVolumeInitial(
 		return nil, err
 	}
 
+	// addVolumeAfterAddVolumeTxn allows fault injection for automated testing.
+	if err := addVolumeAfterAddVolumeTxn.Inject(); err != nil {
+		return nil, err
+	}
+
 	// Copy the volume config into a working copy should any backend mutate the config but fail to create the volume.
 	mutableConfig := volumeConfig.ConstructClone()
 

frontend/crd/crd_controller_test.go

@@ -974,13 +974,12 @@ func TestCrdControllerTransactionFinalizerRemoval(t *testing.T) {
 		"txn": savedTxn,
 	}).Debug("Got transaction.")
 
-	// Ensure the CRD was saved with a Trident finalizer
-	if !savedTxn.HasTridentFinalizers() {
-		t.Fatalf("expected transaction CRD to have Trident finalizer")
+	// Ensure the CRD was saved with no Trident finalizer
+	if savedTxn.HasTridentFinalizers() {
+		t.Fatalf("expected transaction CRD to not have Trident finalizer")
+		Logc(ctx()).Debug("Deleting transaction.")
 	}
 
-	Logc(ctx()).Debug("Deleting transaction.")
-
 	// Delete the CRD
 	deleteErr := crdClient.TridentV1().TridentTransactions(tridentNamespace).Delete(ctx(), "vol1", deleteOptions)
 	if deleteErr != nil {

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

@@ -19,8 +19,7 @@ func NewTridentTransaction(txn *storage.VolumeTransaction) (*TridentTransaction,
 			Kind:       "TridentTransaction",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:       NameFix(txn.Name()),
-			Finalizers: GetTridentFinalizers(),
+			Name: NameFix(txn.Name()),
 		},
 	}
 

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

@@ -1,4 +1,4 @@
-// Copyright 2022 NetApp, Inc. All Rights Reserved.
+// Copyright 2025 NetApp, Inc. All Rights Reserved.
 
 package v1
 
@@ -7,6 +7,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 
@@ -41,8 +42,7 @@ func TestNewTransaction(t *testing.T) {
 			Kind:       "TridentTransaction",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:       NameFix(volConfig.Name),
-			Finalizers: GetTridentFinalizers(),
+			Name: NameFix(volConfig.Name),
 		},
 		Transaction: runtime.RawExtension{
 			Raw: MustEncode(json.Marshal(txn)),
@@ -53,6 +53,7 @@ func TestNewTransaction(t *testing.T) {
 	if !reflect.DeepEqual(volumeTransaction, expected) {
 		t.Fatalf("TridentTransaction does not match expected result, got %v expected %v", volumeTransaction, expected)
 	}
+	assert.Empty(t, volumeTransaction.Finalizers) // Transactions should never have finalizers.
 }
 
 func TestNewSnapshotTransaction(t *testing.T) {
@@ -88,8 +89,7 @@ func TestNewSnapshotTransaction(t *testing.T) {
 			Kind:       "TridentTransaction",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:       NameFix(volConfig.Name),
-			Finalizers: GetTridentFinalizers(),
+			Name: NameFix(volConfig.Name),
 		},
 		Transaction: runtime.RawExtension{
 			Raw: MustEncode(json.Marshal(txn)),
@@ -100,6 +100,7 @@ func TestNewSnapshotTransaction(t *testing.T) {
 	if !reflect.DeepEqual(volumeTransaction, expected) {
 		t.Fatalf("TridentTransaction does not match expected result, got %v expected %v", volumeTransaction, expected)
 	}
+	assert.Empty(t, volumeTransaction.Finalizers) // Transactions should never have finalizers.
 }
 
 func TestTransaction_Persistent(t *testing.T) {
@@ -123,8 +124,7 @@ func TestTransaction_Persistent(t *testing.T) {
 			Kind:       "TridentTransaction",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:       NameFix(volConfig.Name),
-			Finalizers: GetTridentFinalizers(),
+			Name: NameFix(volConfig.Name),
 		},
 		Transaction: runtime.RawExtension{
 			Raw: MustEncode(json.Marshal(txn)),
@@ -180,8 +180,7 @@ func TestSnapshotTransaction_Persistent(t *testing.T) {
 			Kind:       "TridentTransaction",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:       NameFix(volConfig.Name),
-			Finalizers: GetTridentFinalizers(),
+			Name: NameFix(volConfig.Name),
 		},
 		Transaction: runtime.RawExtension{
 			Raw: MustEncode(json.Marshal(txn)),

persistent_store/crdv1.go

@@ -1157,8 +1157,7 @@ func (k *CRDClientV1) GetVolumeTransaction(
 }
 
 func (k *CRDClientV1) DeleteVolumeTransaction(ctx context.Context, volTxn *storage.VolumeTransaction) error {
-	err := k.crdClient.TridentV1().TridentTransactions(k.namespace).Delete(ctx, v1.NameFix(volTxn.Name()),
-		k.deleteOpts())
+	err := k.crdClient.TridentV1().TridentTransactions(k.namespace).Delete(ctx, v1.NameFix(volTxn.Name()), k.deleteOpts())
 
 	if k8sapierrors.IsNotFound(err) {
 		return nil