diff --git a/go.mod b/go.mod index 0b33dcbc8c..62f4ce95a5 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/go-co-op/gocron v1.37.0 github.com/go-logr/zapr v1.3.0 github.com/golang/protobuf v1.5.4 + github.com/google/go-cmp v0.7.0 github.com/google/uuid v1.6.0 github.com/hashicorp/go-version v1.6.0 github.com/kubernetes-csi/csi-proxy/v2 v2.0.0-alpha.1 @@ -100,7 +101,6 @@ require ( github.com/google/btree v1.1.3 // indirect github.com/google/cadvisor v0.52.1 // indirect github.com/google/cel-go v0.26.0 // indirect - github.com/google/go-cmp v0.7.0 // indirect github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db // indirect github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect diff --git a/pkg/common/cns-lib/volume/manager.go b/pkg/common/cns-lib/volume/manager.go index 94850028e4..119fff27e3 100644 --- a/pkg/common/cns-lib/volume/manager.go +++ b/pkg/common/cns-lib/volume/manager.go @@ -160,6 +160,9 @@ type Manager interface { UnregisterVolume(ctx context.Context, volumeID string, unregisterDisk bool) (string, error) // SyncVolume returns the aggregated capacity for volumes SyncVolume(ctx context.Context, syncVolumeSpecs []cnstypes.CnsSyncVolumeSpec) (string, error) + // QueryBackingTypeFromVirtualDiskInfo queries the backing type of a volume using + // VirtualDiskManager's QueryVirtualDiskInfo API. + QueryBackingTypeFromVirtualDiskInfo(ctx context.Context, volumeID string) (string, error) } // CnsVolumeInfo hold information related to volume created by CNS. @@ -3830,3 +3833,81 @@ func (m *defaultManager) unregisterVolume(ctx context.Context, volumeID string, log.Infof("volume %q unregistered successfully", volumeID) return "", nil } + +// QueryBackingTypeFromVirtualDiskInfo queries the backing type of a volume using +// VirtualDiskManager's QueryVirtualDiskInfo API. +// It first queries the volume to get the BackingDiskPath, then uses that path to call +// QueryVirtualDiskInfo which returns the disk type information. +func (m *defaultManager) QueryBackingTypeFromVirtualDiskInfo(ctx context.Context, + volumeID string) (string, error) { + log := logger.GetLogger(ctx) + + // Get the datacenters + dcs, err := m.virtualCenter.GetDatacenters(ctx) + if err != nil { + return "", fmt.Errorf("failed to get datacenters: %w", err) + } + if len(dcs) == 0 { + return "", fmt.Errorf("no datacenters found") + } + + // Query volume to get the backing disk path + queryFilter := cnstypes.CnsQueryFilter{ + VolumeIds: []cnstypes.CnsVolumeId{{Id: volumeID}}, + } + querySelection := cnstypes.CnsQuerySelection{ + Names: []string{string(cnstypes.QuerySelectionNameTypeBackingObjectDetails)}, + } + + queryResult, err := m.QueryVolumeAsync(ctx, queryFilter, &querySelection) + if err != nil { + return "", fmt.Errorf("failed to query volume for backing details %s: %w", volumeID, err) + } + if queryResult == nil || len(queryResult.Volumes) == 0 { + return "", fmt.Errorf("no volume found for volumeID %s", volumeID) + } + + backingObjectDetails := queryResult.Volumes[0].BackingObjectDetails + if backingObjectDetails == nil { + return "", fmt.Errorf("backing object details not found for volumeID %s", volumeID) + } + + blockBackingDetails, ok := backingObjectDetails.(*cnstypes.CnsBlockBackingDetails) + if !ok { + return "", fmt.Errorf("backing object details is not of type CnsBlockBackingDetails for volumeID %s", volumeID) + } + + backingFilePath := blockBackingDetails.BackingDiskPath + if backingFilePath == "" { + return "", fmt.Errorf("backing disk path not found for volumeID %s", volumeID) + } + + // Get the vim25 client that uses standard vim25 namespace. + // This is required because QueryVirtualDiskInfo uses internal vim25 namespace + // which doesn't work with the vsan service version used by the main client. + vim25Client, err := m.virtualCenter.GetVim25Client(ctx) + if err != nil { + return "", fmt.Errorf("failed to get vim25 client: %w", err) + } + + // Query virtual disk info using the datacenter we already have + virtualDiskManager := object.NewVirtualDiskManager(vim25Client) + diskInfoList, err := virtualDiskManager.QueryVirtualDiskInfo(ctx, backingFilePath, dcs[0].Datacenter, false) + if err != nil { + return "", fmt.Errorf("failed to query virtual disk info for %s: %w", backingFilePath, err) + } + if len(diskInfoList) == 0 { + return "", fmt.Errorf("no disk info returned for %s", backingFilePath) + } + + diskType := diskInfoList[0].DiskType + log.Debugf("Retrieved diskType %s for volumeID %s", diskType, volumeID) + + // Convert diskType to backing type + backingType := ConvertDiskTypeToBackingType(diskType) + if backingType == "" { + return "", fmt.Errorf("unable to find backingType for diskType:%s for the volume %s", + diskType, volumeID) + } + return backingType, nil +} diff --git a/pkg/common/cns-lib/volume/manager_mock.go b/pkg/common/cns-lib/volume/manager_mock.go index b5ef7939bf..c9627574d9 100644 --- a/pkg/common/cns-lib/volume/manager_mock.go +++ b/pkg/common/cns-lib/volume/manager_mock.go @@ -199,3 +199,9 @@ func (m MockManager) SyncVolume(ctx context.Context, syncVolumeSpecs []cnstypes. //TODO implement me panic("implement me") } + +func (m MockManager) QueryBackingTypeFromVirtualDiskInfo(ctx context.Context, + volumeID string) (string, error) { + //TODO implement me + panic("implement me") +} diff --git a/pkg/common/cns-lib/volume/util.go b/pkg/common/cns-lib/volume/util.go index 620336963a..df98c102e2 100644 --- a/pkg/common/cns-lib/volume/util.go +++ b/pkg/common/cns-lib/volume/util.go @@ -632,3 +632,32 @@ func IsCnsVolumeAlreadyExistsFault(ctx context.Context, faultType string) bool { log.Infof("Checking fault type: %q is vim.fault.CnsVolumeAlreadyExistsFault", faultType) return faultType == "vim.fault.CnsVolumeAlreadyExistsFault" } + +// ConvertDiskTypeToBackingType converts the diskType returned by QueryVirtualDiskInfo +// to the appropriate CnsVolumeBackingType string used for batch attach operations. +// The returned values correspond to VirtualDevice.FileBackingInfo subclasses as defined in +// github.com/vmware/govmomi/cns/types (CnsVolumeBackingType constants). +func ConvertDiskTypeToBackingType(diskType string) string { + switch diskType { + case "thin", "preallocated", "thick", "eagerZeroedThick", "thick2Gb", "flatMonolithic": + // All flat/thick disk types use FlatVer2BackingInfo + // thin -> FlatVer2BackingInfo (thinProvisioned=true) + // preallocated/thick -> FlatVer2BackingInfo (thinProvisioned=false, eagerlyScrub=false) + // eagerZeroedThick -> FlatVer2BackingInfo (thinProvisioned=false, eagerlyScrub=true) + // thick2Gb/flatMonolithic -> FlatVer2BackingInfo (split variations) + return string(cnstypes.CnsVolumeBackingTypeFlatVer2BackingInfo) + case "sparse2Gb", "sparseMonolithic", "delta", "vmfsSparse": + // sparse types -> SparseVer2BackingInfo + return string(cnstypes.CnsVolumeBackingTypeSparseVer2BackingInfo) + case "seSparse": + // seSparse -> SeSparseBackingInfo + return string(cnstypes.CnsVolumeBackingTypeSeSparseBackingInfo) + case "rdm", "rdmp": + // rdm -> RawDiskMappingVer1BackingInfo (compatibilityMode="virtualMode") + // rdmp -> RawDiskMappingVer1BackingInfo (compatibilityMode="physicalMode") + return string(cnstypes.CnsVolumeBackingTypeRawDiskMappingVer1BackingInfo) + default: + // Unknown disk type, return empty string + return "" + } +} diff --git a/pkg/common/cns-lib/volume/util_test.go b/pkg/common/cns-lib/volume/util_test.go new file mode 100644 index 0000000000..794507f861 --- /dev/null +++ b/pkg/common/cns-lib/volume/util_test.go @@ -0,0 +1,114 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package volume + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestConvertDiskTypeToBackingType(t *testing.T) { + tests := []struct { + name string + diskType string + want string + }{ + { + name: "thin disk type", + diskType: "thin", + want: "FlatVer2BackingInfo", + }, + { + name: "preallocated disk type", + diskType: "preallocated", + want: "FlatVer2BackingInfo", + }, + { + name: "thick disk type", + diskType: "thick", + want: "FlatVer2BackingInfo", + }, + { + name: "eagerZeroedThick disk type", + diskType: "eagerZeroedThick", + want: "FlatVer2BackingInfo", + }, + { + name: "sparse2Gb disk type", + diskType: "sparse2Gb", + want: "SparseVer2BackingInfo", + }, + { + name: "sparseMonolithic disk type", + diskType: "sparseMonolithic", + want: "SparseVer2BackingInfo", + }, + { + name: "delta disk type", + diskType: "delta", + want: "SparseVer2BackingInfo", + }, + { + name: "vmfsSparse disk type", + diskType: "vmfsSparse", + want: "SparseVer2BackingInfo", + }, + { + name: "thick2Gb disk type", + diskType: "thick2Gb", + want: "FlatVer2BackingInfo", + }, + { + name: "flatMonolithic disk type", + diskType: "flatMonolithic", + want: "FlatVer2BackingInfo", + }, + { + name: "seSparse disk type", + diskType: "seSparse", + want: "SeSparseBackingInfo", + }, + { + name: "rdm disk type", + diskType: "rdm", + want: "RawDiskMappingVer1BackingInfo", + }, + { + name: "rdmp disk type", + diskType: "rdmp", + want: "RawDiskMappingVer1BackingInfo", + }, + { + name: "unknown disk type", + diskType: "unknown", + want: "", + }, + { + name: "empty disk type", + diskType: "", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ConvertDiskTypeToBackingType(tt.diskType) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/common/cns-lib/vsphere/virtualcenter.go b/pkg/common/cns-lib/vsphere/virtualcenter.go index 32ecb13d40..a6b65cc121 100644 --- a/pkg/common/cns-lib/vsphere/virtualcenter.go +++ b/pkg/common/cns-lib/vsphere/virtualcenter.go @@ -76,6 +76,11 @@ type VirtualCenter struct { VsanClient *vsan.Client // VslmClient represents the Vslm client instance. VslmClient *vslm.Client + // Vim25Client is a vim25.Client that uses the standard vim25 namespace. + // This is needed for APIs like VirtualDiskManager.QueryVirtualDiskInfo that + // use internal vim25 namespaces and don't work with the vsan service version. + // It shares the same http.Transport as the main Client but uses vim25.Path and vim25.Namespace. + Vim25Client *vim25.Client // ClientMutex is used for exclusive connection creation. ClientMutex *sync.Mutex } @@ -386,9 +391,50 @@ func (vc *VirtualCenter) connect(ctx context.Context) error { } vc.VsanClient.RoundTripper = &MetricRoundTripper{"vsan", vc.VsanClient.RoundTripper} } + // Recreate Vim25Client if created using timed out VC Client. + if vc.Vim25Client != nil { + vc.Vim25Client = vc.newVim25Client() + } return nil } +// newVim25Client creates a vim25.Client that uses the standard vim25 namespace. +// This is needed for APIs like VirtualDiskManager.QueryVirtualDiskInfo that use +// internal vim25 namespaces and don't work with the vsan service version. +func (vc *VirtualCenter) newVim25Client() *vim25.Client { + soapClient := vc.Client.Client.Client + vimSoapClient := soapClient.NewServiceClient(vim25.Path, vim25.Namespace) + vimSoapClient.Version = vc.Client.Client.ServiceContent.About.ApiVersion + return &vim25.Client{ + Client: vimSoapClient, + ServiceContent: vc.Client.Client.ServiceContent, + RoundTripper: vimSoapClient, + } +} + +// GetVim25Client returns a vim25.Client that uses the standard vim25 namespace. +// This client is needed for APIs like VirtualDiskManager.QueryVirtualDiskInfo that +// use internal vim25 namespaces and don't work with the vsan service version. +// The client is created lazily on first call and reused for subsequent calls. +// It shares the same http.Transport as the main Client. +func (vc *VirtualCenter) GetVim25Client(ctx context.Context) (*vim25.Client, error) { + log := logger.GetLogger(ctx) + + // Ensure connection is established + if err := vc.Connect(ctx); err != nil { + return nil, err + } + + vc.ClientMutex.Lock() + defer vc.ClientMutex.Unlock() + + if vc.Vim25Client == nil { + log.Info("Creating Vim25Client for standard vim25 namespace operations") + vc.Vim25Client = vc.newVim25Client() + } + return vc.Vim25Client, nil +} + // ReadVCConfigs will ensure we are always reading the latest config // before attempting to create a new govmomi client. // It works in case of both vanilla (including multi-vc) and wcp diff --git a/pkg/common/unittestcommon/types.go b/pkg/common/unittestcommon/types.go index ed6639a717..7f5be8409f 100644 --- a/pkg/common/unittestcommon/types.go +++ b/pkg/common/unittestcommon/types.go @@ -224,3 +224,8 @@ func (m *MockVolumeManager) SyncVolume(ctx context.Context, syncVolumeSpecs []cnstypes.CnsSyncVolumeSpec) (string, error) { return "", nil } + +func (m *MockVolumeManager) QueryBackingTypeFromVirtualDiskInfo(ctx context.Context, + volumeID string) (string, error) { + return "", nil +} diff --git a/pkg/csi/service/common/vsphereutil_test.go b/pkg/csi/service/common/vsphereutil_test.go index 8a82133cb1..9440a0d6f4 100644 --- a/pkg/csi/service/common/vsphereutil_test.go +++ b/pkg/csi/service/common/vsphereutil_test.go @@ -131,6 +131,12 @@ func (m *mockVolumeManager) SyncVolume(ctx context.Context, syncVolumeSpecs []cnstypes.CnsSyncVolumeSpec) (string, error) { return "", nil } + +func (m *mockVolumeManager) QueryBackingTypeFromVirtualDiskInfo(ctx context.Context, + volumeID string) (string, error) { + return "", nil +} + func TestQueryVolumeSnapshotsByVolumeIDWithQuerySnapshotsCnsVolumeNotFoundFault(t *testing.T) { // Skip test on ARM64 due to gomonkey limitations if runtime.GOARCH == "arm64" { diff --git a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go index 94f25a3947..42fc1d5c41 100644 --- a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go +++ b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_controller.go @@ -538,7 +538,7 @@ func (r *Reconciler) processBatchAttach(ctx context.Context, k8sClient kubernete // Construct batch attach request pvcsInAttachList, volumeIdsInAttachList, batchAttachRequest, err := constructBatchAttachRequest(ctx, - volumesToAttach, instance) + volumesToAttach, instance, r.volumeManager, k8sClient) if err != nil { log.Errorf("failed to construct batch attach request. Err: %s", err) return err diff --git a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go index aa84282e9d..ac8917781d 100644 --- a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go +++ b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_helper.go @@ -517,7 +517,9 @@ func listAttachedFcdsForVM(ctx context.Context, // It also validates each of the requests to make sure user input is correct. func constructBatchAttachRequest(ctx context.Context, volumesToAttach map[string]string, - instance *v1alpha1.CnsNodeVMBatchAttachment) (pvcsInSpec map[string]string, + instance *v1alpha1.CnsNodeVMBatchAttachment, + volumeManager volumes.Manager, + k8sClient kubernetes.Interface) (pvcsInSpec map[string]string, volumeIdsInSpec map[string]string, batchAttachRequest []volumes.BatchAttachRequest, err error) { log := logger.GetLogger(ctx) @@ -552,6 +554,26 @@ func constructBatchAttachRequest(ctx context.Context, isPvcEncrypted := isPvcEncrypted(pvcObj.Annotations) log.Infof("PVC %s has encryption enabled: %t", pvcName, isPvcEncrypted) + // Get BackingType from PVC annotation, if not available query from VirtualDiskManager + backingType := pvcObj.GetAnnotations()[common.AnnKeyBackingDiskType] + if backingType == "" { + log.Infof("BackingType annotation not found on PVC %s, querying from VirtualDiskManager", pvcName) + queriedBackingType, queryErr := volumeManager.QueryBackingTypeFromVirtualDiskInfo(ctx, volumeID) + if queryErr != nil { + log.With("pvc", pvcName).With("namespace", instance.Namespace).Error(queryErr) + return pvcsInSpec, volumeIdsInSpec, batchAttachRequest, queryErr + } + backingType = queriedBackingType + log.Infof("Successfully retrieved BackingType %s for PVC %s from VirtualDiskManager", + backingType, pvcName) + // Update the PVC annotation with the queried BackingType so it can be reused in future attach operations + patchErr := patchPVCBackingTypeAnnotation(ctx, k8sClient, pvcObj, backingType) + if patchErr != nil { + log.With("pvc", pvcName).With("namespace", instance.Namespace).Error(patchErr) + return pvcsInSpec, volumeIdsInSpec, batchAttachRequest, patchErr + } + } + // Populate values for attach request. currentBatchAttachRequest := volumes.BatchAttachRequest{ VolumeID: volumeID, @@ -559,7 +581,7 @@ func constructBatchAttachRequest(ctx context.Context, DiskMode: string(volume.PersistentVolumeClaim.DiskMode), ControllerKey: volume.PersistentVolumeClaim.ControllerKey, UnitNumber: volume.PersistentVolumeClaim.UnitNumber, - BackingType: pvcObj.GetAnnotations()[common.AnnKeyBackingDiskType], + BackingType: backingType, VolumeEncrypted: &isPvcEncrypted, } batchAttachRequest = append(batchAttachRequest, currentBatchAttachRequest) @@ -763,6 +785,54 @@ func patchPVCAnnotations(ctx context.Context, k8sClient kubernetes.Interface, return nil } +// patchPVCBackingTypeAnnotation updates the BackingType annotation on the PVC. +// This is used to cache the backing type so it doesn't need to be queried again on future attach operations. +func patchPVCBackingTypeAnnotation(ctx context.Context, k8sClient kubernetes.Interface, + pvc *v1.PersistentVolumeClaim, backingType string) error { + log := logger.GetLogger(ctx) + + patchAnnotations := make(map[string]interface{}) + if pvc.Annotations != nil { + for k, v := range pvc.Annotations { + patchAnnotations[k] = v + } + } + + log.Infof("Setting BackingType annotation %s=%s on PVC %s", + common.AnnKeyBackingDiskType, backingType, pvc.Name) + patchAnnotations[common.AnnKeyBackingDiskType] = backingType + + // Build patch structure + patch := map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": patchAnnotations, + }, + } + + patchBytes, err := json.Marshal(patch) + if err != nil { + log.Errorf("failed to marshal BackingType annotation for PVC %s. Err: %s", pvc.Name, err) + return fmt.Errorf("failed to marshal patch: %v", err) + } + + log.Infof("Patching PVC %s with BackingType annotation", pvc.Name) + + // Apply the patch + updatedpvc, err := k8sClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Patch( + ctx, + pvc.Name, + types.MergePatchType, + patchBytes, + metav1.PatchOptions{}, + ) + if err != nil { + log.Errorf("failed to patch PVC %s with BackingType annotation. Err: %s", pvc.Name, err) + return fmt.Errorf("failed to patch PVC %s: %v", pvc.Name, err) + } + log.Infof("Successfully patched PVC: %s with BackingType annotation %+v", pvc.Name, updatedpvc.Annotations) + return nil +} + // pvcHasUsedByAnnotaion goes through all annotations on the PVC to find out if the PVC is used by any VM or not. func pvcHasUsedByAnnotaion(ctx context.Context, pvc *v1.PersistentVolumeClaim) bool { log := logger.GetLogger(ctx) diff --git a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_test.go b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_test.go index 546fbc3288..e7a91dc695 100644 --- a/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_test.go +++ b/pkg/syncer/cnsoperator/controller/cnsnodevmbatchattachment/cnsnodevmbatchattachment_test.go @@ -208,8 +208,26 @@ func getClientSetWithPvc() *k8sFake.Clientset { }, } + // Define fail-attach-pvc-3 for attach failure tests + pvc3 := &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fail-attach-pvc-3", + Namespace: "test-ns", + }, + Spec: v1.PersistentVolumeClaimSpec{ + AccessModes: []v1.PersistentVolumeAccessMode{ + v1.ReadWriteOnce, + }, + Resources: v1.VolumeResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("3Gi"), + }, + }, + }, + } + // Initialize fake clientset with the PVC - clientset := k8sFake.NewSimpleClientset(pvc1, pvc2) + clientset := k8sFake.NewSimpleClientset(pvc1, pvc2, pvc3) return clientset } @@ -1611,6 +1629,98 @@ func TestUpdateInstanceVolume_WhenVolumeNameIsEmpty(t *testing.T) { assert.Len(t, instance.Status.VolumeStatus, 0) } +func TestPatchPVCBackingTypeAnnotation(t *testing.T) { + ctx := context.TODO() + backingType := "thin" + + tests := []struct { + name string + initialAnnotations map[string]string + expectError bool + }{ + { + name: "PVC with no annotations", + initialAnnotations: nil, + expectError: false, + }, + { + name: "PVC with existing annotations", + initialAnnotations: map[string]string{"existing": "value"}, + expectError: false, + }, + { + name: "PVC with existing BackingType annotation", + initialAnnotations: map[string]string{ + "cns.vmware.com.protected/disk-backing": "thick", + }, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := k8sFake.NewSimpleClientset() + + // Create PVC in fake cluster + pvc := &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "default", + Annotations: tt.initialAnnotations, + }, + } + + _, err := client.CoreV1().PersistentVolumeClaims("default").Create(ctx, pvc, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("failed to create pvc in fake client: %v", err) + } + + // Get PVC from fake client + pvcFromClient, err := client.CoreV1().PersistentVolumeClaims("default").Get(ctx, "test-pvc", metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get pvc: %v", err) + } + + err = patchPVCBackingTypeAnnotation(ctx, client, pvcFromClient, backingType) + + if tt.expectError { + if err == nil { + t.Fatalf("expected error, got nil") + } + return + } else if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify annotation is added/updated + updatedPVC, err := client.CoreV1().PersistentVolumeClaims("default").Get(ctx, "test-pvc", metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get updated pvc: %v", err) + } + + val, ok := updatedPVC.Annotations["cns.vmware.com.protected/disk-backing"] + if !ok { + t.Errorf("expected BackingType annotation not found") + } + if val != backingType { + t.Errorf("expected annotation value %q, got %q", backingType, val) + } + + // Verify existing annotations are preserved + if tt.initialAnnotations != nil { + for k, v := range tt.initialAnnotations { + if k == "cns.vmware.com.protected/disk-backing" { + continue // Skip BackingType as it's expected to be updated + } + if updatedPVC.Annotations[k] != v { + t.Errorf("annotation %q changed: got %q, want %q", k, updatedPVC.Annotations[k], v) + } + } + } + }) + } +} + func TestUpdateInstanceVolumeStatusByPvc_SetsSuccessCondition_WhenNoError(t *testing.T) { instance := &v1alpha1.CnsNodeVMBatchAttachment{ Status: v1alpha1.CnsNodeVMBatchAttachmentStatus{ diff --git a/pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go b/pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go index 1702842d02..d698fde5a8 100644 --- a/pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go +++ b/pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller_test.go @@ -224,6 +224,11 @@ func (m *mockVolumeManager) SyncVolume(ctx context.Context, return "", nil } +func (m *mockVolumeManager) QueryBackingTypeFromVirtualDiskInfo(ctx context.Context, + volumeID string) (string, error) { + return "", nil +} + type mockCOCommon struct{} func (m *mockCOCommon) GetPVCNamespacedNameByUID(uid string) (types.NamespacedName, bool) {