Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ require (
github.com/stretchr/testify v1.11.1
github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20250923172217-bf5a74e51c65
github.com/vmware-tanzu/vm-operator/external/byok v0.0.0-20250509154507-b93e51fc90fa
github.com/vmware/govmomi v0.53.0-alpha.0.0.20251203163802-5ce652387dac
github.com/vmware/govmomi v0.53.0-alpha.0.0.20251203213634-99f18b71ea8e
go.uber.org/zap v1.27.1
golang.org/x/crypto v0.46.0
golang.org/x/sync v0.19.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,8 @@ github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20250923172217-bf5a74e51c65 h1:
github.com/vmware-tanzu/vm-operator/api v1.9.1-0.20250923172217-bf5a74e51c65/go.mod h1:nWTPpxfe4gHuuYuFcrs86+NMxfkqPk3a3IlvI8TCWak=
github.com/vmware-tanzu/vm-operator/external/byok v0.0.0-20250509154507-b93e51fc90fa h1:4MKu14YJ7J54O6QKmT4ds5EUpysWLLtQRMff73cVkmU=
github.com/vmware-tanzu/vm-operator/external/byok v0.0.0-20250509154507-b93e51fc90fa/go.mod h1:8tiuyYslzjLIUmOlXZuGKQdQP2ZgWGCVhVeyptmZYnk=
github.com/vmware/govmomi v0.53.0-alpha.0.0.20251203163802-5ce652387dac h1:E3W+2J1I0B5LyIillKYVQHIb6CpslGcogt7Q+8FHT3c=
github.com/vmware/govmomi v0.53.0-alpha.0.0.20251203163802-5ce652387dac/go.mod h1:FM3GTg002dFFN7l2/hNS0YWC4f78HTw4kvgUwAE52cM=
github.com/vmware/govmomi v0.53.0-alpha.0.0.20251203213634-99f18b71ea8e h1:TG9xuPu9N29Ak1gNs85VsMImNv1bd2l0yNfAMc3imOU=
github.com/vmware/govmomi v0.53.0-alpha.0.0.20251203213634-99f18b71ea8e/go.mod h1:FM3GTg002dFFN7l2/hNS0YWC4f78HTw4kvgUwAE52cM=
github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM=
github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg=
github.com/xiang90/probing v0.0.0-20221125231312-a49e3df8f510 h1:S2dVYn90KE98chqDkyE9Z4N61UnQd+KOfgp5Iu53llk=
Expand Down
316 changes: 298 additions & 18 deletions pkg/common/cns-lib/volume/manager.go

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions pkg/common/cns-lib/volume/manager_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,11 @@ func (m MockManager) SyncVolume(ctx context.Context, syncVolumeSpecs []cnstypes.
//TODO implement me
panic("implement me")
}

func (m MockManager) ReRegisterVolume(ctx context.Context, volumeID string) error {
if m.failRequest {
return m.err
}

return nil
}
246 changes: 246 additions & 0 deletions pkg/common/cns-lib/volume/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/assert"
cnstypes "github.com/vmware/govmomi/cns/types"
vim25types "github.com/vmware/govmomi/vim25/types"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/logger"
)

const createVolumeTaskTimeout = 3 * time.Second
Expand Down Expand Up @@ -284,3 +285,248 @@ func TestAttachVolumeFaultHandling(t *testing.T) {
})
}
}

// TestIsCnsNotRegisteredFault tests the IsCnsNotRegisteredFault helper function
// that detects CnsNotRegisteredFault from CNS operations.
func TestIsCnsNotRegisteredFault(t *testing.T) {
ctx := logger.NewContextWithLogger(context.Background())

tests := []struct {
name string
fault *vim25types.LocalizedMethodFault
expected bool
}{
{
name: "nil fault",
fault: nil,
expected: false,
},
{
name: "fault with nil Fault field",
fault: &vim25types.LocalizedMethodFault{
LocalizedMessage: "Some error",
Fault: nil,
},
expected: false,
},
{
name: "CnsNotRegisteredFault",
fault: &vim25types.LocalizedMethodFault{
LocalizedMessage: "The input volume is not registered as a CNS volume",
Fault: &cnstypes.CnsNotRegisteredFault{},
},
expected: true,
},
{
name: "CnsFault (not CnsNotRegisteredFault)",
fault: &vim25types.LocalizedMethodFault{
LocalizedMessage: "Generic CNS fault",
Fault: &cnstypes.CnsFault{
Reason: "Generic error",
},
},
expected: false,
},
{
name: "ResourceInUse fault",
fault: &vim25types.LocalizedMethodFault{
LocalizedMessage: "Resource is in use",
Fault: &vim25types.ResourceInUse{
Type: "Disk",
Name: "test-disk",
},
},
expected: false,
},
{
name: "NotSupported fault",
fault: &vim25types.LocalizedMethodFault{
LocalizedMessage: "Operation not supported",
Fault: &vim25types.NotSupported{},
},
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := IsCnsNotRegisteredFault(ctx, tt.fault)
assert.Equal(t, tt.expected, result,
"IsCnsNotRegisteredFault(%v) = %v, expected %v", tt.fault, result, tt.expected)
})
}
}

// TestIsCnsVolumeAlreadyExistsFault tests the IsCnsVolumeAlreadyExistsFault helper function
// that checks if a fault type indicates the volume already exists in CNS.
func TestIsCnsVolumeAlreadyExistsFault(t *testing.T) {
ctx := logger.NewContextWithLogger(context.Background())

tests := []struct {
name string
faultType string
expected bool
}{
{
name: "CnsVolumeAlreadyExistsFault",
faultType: "vim.fault.CnsVolumeAlreadyExistsFault",
expected: true,
},
{
name: "empty fault type",
faultType: "",
expected: false,
},
{
name: "CnsFault",
faultType: "vim.fault.CnsFault",
expected: false,
},
{
name: "NotSupported",
faultType: "vim25:NotSupported",
expected: false,
},
{
name: "partial match - should not match",
faultType: "CnsVolumeAlreadyExistsFault",
expected: false,
},
{
name: "case sensitive - lowercase should not match",
faultType: "vim.fault.cnsvolumealreadyexistsfault",
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := IsCnsVolumeAlreadyExistsFault(ctx, tt.faultType)
assert.Equal(t, tt.expected, result,
"IsCnsVolumeAlreadyExistsFault(%q) = %v, expected %v", tt.faultType, result, tt.expected)
})
}
}

// TestCnsNotRegisteredFaultHandlingScenarios tests the scenarios where
// CnsNotRegisteredFault should trigger re-registration of the volume.
func TestCnsNotRegisteredFaultHandlingScenarios(t *testing.T) {
ctx := logger.NewContextWithLogger(context.Background())

tests := []struct {
name string
volumeOperationResult *cnstypes.CnsVolumeOperationResult
expectReRegistrationNeeded bool
description string
}{
{
name: "CnsNotRegisteredFault should trigger re-registration",
volumeOperationResult: &cnstypes.CnsVolumeOperationResult{
VolumeId: cnstypes.CnsVolumeId{Id: "test-volume-1"},
Fault: &vim25types.LocalizedMethodFault{
LocalizedMessage: "The input volume is not registered as a CNS volume",
Fault: &cnstypes.CnsNotRegisteredFault{},
},
},
expectReRegistrationNeeded: true,
description: "Volume not registered in CNS should trigger re-registration",
},
{
name: "CnsFault should not trigger re-registration",
volumeOperationResult: &cnstypes.CnsVolumeOperationResult{
VolumeId: cnstypes.CnsVolumeId{Id: "test-volume-2"},
Fault: &vim25types.LocalizedMethodFault{
LocalizedMessage: "Generic CNS error",
Fault: &cnstypes.CnsFault{
Reason: "Some other error",
},
},
},
expectReRegistrationNeeded: false,
description: "Generic CnsFault should not trigger re-registration",
},
{
name: "No fault should not trigger re-registration",
volumeOperationResult: &cnstypes.CnsVolumeOperationResult{
VolumeId: cnstypes.CnsVolumeId{Id: "test-volume-3"},
Fault: nil,
},
expectReRegistrationNeeded: false,
description: "Successful operation should not trigger re-registration",
},
{
name: "ResourceInUse should not trigger re-registration",
volumeOperationResult: &cnstypes.CnsVolumeOperationResult{
VolumeId: cnstypes.CnsVolumeId{Id: "test-volume-4"},
Fault: &vim25types.LocalizedMethodFault{
LocalizedMessage: "Resource is in use",
Fault: &vim25types.ResourceInUse{
Type: "Disk",
Name: "test-disk",
},
},
},
expectReRegistrationNeeded: false,
description: "ResourceInUse should not trigger re-registration",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var reRegistrationNeeded bool
if tt.volumeOperationResult.Fault != nil {
reRegistrationNeeded = IsCnsNotRegisteredFault(ctx, tt.volumeOperationResult.Fault)
}
assert.Equal(t, tt.expectReRegistrationNeeded, reRegistrationNeeded,
"Test: %s - %s", tt.name, tt.description)
})
}
}

// TestReRegistrationIdempotency tests that re-registration handles
// CnsVolumeAlreadyExistsFault gracefully (idempotent behavior).
func TestReRegistrationIdempotency(t *testing.T) {
ctx := logger.NewContextWithLogger(context.Background())

tests := []struct {
name string
faultTypeAfterReRegister string
expectSuccess bool
description string
}{
{
name: "Re-registration succeeds",
faultTypeAfterReRegister: "",
expectSuccess: true,
description: "Re-registration with no fault should succeed",
},
{
name: "Volume already exists (idempotent)",
faultTypeAfterReRegister: "vim.fault.CnsVolumeAlreadyExistsFault",
expectSuccess: true,
description: "CnsVolumeAlreadyExistsFault should be treated as success",
},
{
name: "Other fault during re-registration",
faultTypeAfterReRegister: "vim.fault.CnsFault",
expectSuccess: false,
description: "Other faults should be treated as failure",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var success bool
if tt.faultTypeAfterReRegister == "" {
success = true
} else if IsCnsVolumeAlreadyExistsFault(ctx, tt.faultTypeAfterReRegister) {
// CnsVolumeAlreadyExistsFault means volume is already registered
success = true
} else {
success = false
}
assert.Equal(t, tt.expectSuccess, success,
"Test: %s - %s", tt.name, tt.description)
})
}
}
14 changes: 14 additions & 0 deletions pkg/common/cns-lib/volume/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -632,3 +632,17 @@ func IsCnsVolumeAlreadyExistsFault(ctx context.Context, faultType string) bool {
log.Infof("Checking fault type: %q is vim.fault.CnsVolumeAlreadyExistsFault", faultType)
return faultType == "vim.fault.CnsVolumeAlreadyExistsFault"
}

// IsCnsNotRegisteredFault checks if the fault is CnsNotRegisteredFault
func IsCnsNotRegisteredFault(ctx context.Context, fault *types.LocalizedMethodFault) bool {
log := logger.GetLogger(ctx)
if fault == nil || fault.Fault == nil {
log.Infof("fault is nil or fault.Fault is nil. Not a CnsNotRegisteredFault")
return false
}
if _, ok := fault.Fault.(*cnstypes.CnsNotRegisteredFault); ok {
log.Infof("observed CnsNotRegisteredFault")
return true
}
return false
}
4 changes: 4 additions & 0 deletions pkg/common/unittestcommon/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,7 @@ func (m *MockVolumeManager) SyncVolume(ctx context.Context,
syncVolumeSpecs []cnstypes.CnsSyncVolumeSpec) (string, error) {
return "", nil
}

func (m *MockVolumeManager) ReRegisterVolume(ctx context.Context, volumeID string) error {
return nil
}
2 changes: 1 addition & 1 deletion pkg/common/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func getCommonUtilsTest(t *testing.T) *commonUtilsTest {
t.Fatal(err)
}

volumeManager, err := cnsvolumes.GetManager(ctx, virtualCenter, nil, false, false, false, "")
volumeManager, err := cnsvolumes.GetManager(ctx, virtualCenter, nil, false, false, false, "", "", "")
if err != nil {
t.Fatalf("failed to create an instance of volume manager. err=%v", err)
}
Expand Down
31 changes: 10 additions & 21 deletions pkg/csi/service/common/vsphereutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -1226,31 +1226,20 @@ func DeleteSnapshotUtil(ctx context.Context, volumeManager cnsvolume.Manager, cs
}

// GetCnsVolumeType is the helper function that determines the volume type based on the volume-id
func GetCnsVolumeType(ctx context.Context, volumeManager cnsvolume.Manager, volumeId string) (string, error) {
// GetCnsVolumeType is the helper function that determines the volume type based on the volume-id prefix.
// If volume ID begins with "file:", it is a file volume, otherwise it is a block volume.
func GetCnsVolumeType(ctx context.Context, volumeId string) string {
log := logger.GetLogger(ctx)
var volumeType string
queryFilter := cnstypes.CnsQueryFilter{
VolumeIds: []cnstypes.CnsVolumeId{{Id: volumeId}},
}
querySelection := cnstypes.CnsQuerySelection{
Names: []string{
string(cnstypes.QuerySelectionNameTypeVolumeType),
},
}
// Select only the volume type.
queryResult, err := volumeManager.QueryAllVolume(ctx, queryFilter, querySelection)
if err != nil {
return "", logger.LogNewErrorCodef(log, codes.Internal,
"queryVolume failed for volumeID: %q with err=%+v", volumeId, err)
// Determine volume type based on volume ID prefix
if strings.HasPrefix(volumeId, "file:") {
volumeType = FileVolumeType
} else {
volumeType = BlockVolumeType
}

if len(queryResult.Volumes) == 0 {
log.Infof("volume: %s not found during query while determining CNS volume type", volumeId)
return "", ErrNotFound
}
volumeType = queryResult.Volumes[0].VolumeType
log.Infof("volume: %s is of type: %s", volumeId, volumeType)
return volumeType, nil
log.Infof("volume: %s is of type: %s (determined from volume ID prefix)", volumeId, volumeType)
return volumeType
}

// GetNodeVMsWithAccessToDatastore finds out NodeVMs which have access to the given
Expand Down
5 changes: 5 additions & 0 deletions pkg/csi/service/common/vsphereutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@ func (m *mockVolumeManager) SyncVolume(ctx context.Context,
syncVolumeSpecs []cnstypes.CnsSyncVolumeSpec) (string, error) {
return "", nil
}

func (m *mockVolumeManager) ReRegisterVolume(ctx context.Context, volumeID string) error {
return nil
}

func TestQueryVolumeSnapshotsByVolumeIDWithQuerySnapshotsCnsVolumeNotFoundFault(t *testing.T) {
// Skip test on ARM64 due to gomonkey limitations
if runtime.GOARCH == "arm64" {
Expand Down
Loading