Skip to content

Commit

Permalink
fix flaky unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
ak017151 committed Nov 21, 2024
1 parent ab34f35 commit 918f5c4
Show file tree
Hide file tree
Showing 13 changed files with 71 additions and 43 deletions.
7 changes: 5 additions & 2 deletions pkg/common/cns-lib/volume/listview.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ var ErrListViewTaskAddition = errors.New("failure to add task to listview")
var ErrSessionNotAuthenticated = errors.New("session is not authenticated")

// NewListViewImpl creates a new listView object and starts a goroutine to listen to property collector task updates
func NewListViewImpl(ctx context.Context, virtualCenter *cnsvsphere.VirtualCenter) (*ListViewImpl, error) {
func NewListViewImpl(ctx context.Context, virtualCenter *cnsvsphere.VirtualCenter, isUnitTestRun bool) (*ListViewImpl,
error) {
log := logger.GetLogger(ctx)
t := &ListViewImpl{
taskMap: NewTaskMap(),
Expand All @@ -84,7 +85,9 @@ func NewListViewImpl(ctx context.Context, virtualCenter *cnsvsphere.VirtualCente
return nil, logger.LogNewErrorf(log, "failed to create a ListView. error: %+v", err)
}
go t.listenToTaskUpdates()
go t.restartContainer()
if !isUnitTestRun {
go t.restartContainer()
}
return t, nil
}

Expand Down
13 changes: 6 additions & 7 deletions pkg/common/cns-lib/volume/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,8 @@ type createVolumeTaskDetails struct {
// GetManager returns the Manager instance.
func GetManager(ctx context.Context, vc *cnsvsphere.VirtualCenter,
operationStore cnsvolumeoperationrequest.VolumeOperationRequest,
idempotencyHandlingEnabled, multivCenterEnabled,
multivCenterTopologyDeployment bool,
clusterFlavor cnstypes.CnsClusterFlavor) (Manager, error) {
idempotencyHandlingEnabled, multivCenterEnabled, multivCenterTopologyDeployment bool,
clusterFlavor cnstypes.CnsClusterFlavor, isUnitTestRun bool) (Manager, error) {
log := logger.GetLogger(ctx)
managerInstanceLock.Lock()
defer managerInstanceLock.Unlock()
Expand Down Expand Up @@ -273,7 +272,7 @@ func GetManager(ctx context.Context, vc *cnsvsphere.VirtualCenter,
}
managerInstanceMap[vc.Config.Host] = managerInstance
}
err := managerInstance.initListView(ctx)
err := managerInstance.initListView(ctx, isUnitTestRun)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -675,7 +674,7 @@ func (m *defaultManager) waitOnTask(csiOpContext context.Context,
taskMoRef vim25types.ManagedObjectReference) (*vim25types.TaskInfo, error) {
log := logger.GetLogger(csiOpContext)
if m.listViewIf == nil {
err := m.initListView(context.Background())
err := m.initListView(context.Background(), false)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -723,10 +722,10 @@ func waitForResultOrTimeout(csiOpContext context.Context, taskMoRef vim25types.M
return taskInfo, err
}

func (m *defaultManager) initListView(ctx context.Context) error {
func (m *defaultManager) initListView(ctx context.Context, isUnitTestRun bool) error {
log := logger.GetLogger(ctx)
var err error
m.listViewIf, err = NewListViewImpl(ctx, m.virtualCenter)
m.listViewIf, err = NewListViewImpl(ctx, m.virtualCenter, isUnitTestRun)
if err != nil {
return logger.LogNewErrorf(log, "failed to initialize listView object. err: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/common/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,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, "", true)
if err != nil {
t.Fatalf("failed to create an instance of volume manager. err=%v", err)
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/csi/service/vanilla/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ func (c *controller) Init(config *cnsconfig.Config, version string) error {
return err
}
vc.Config = vcenterconfig
volumeManager, err := cnsvolume.GetManager(ctx, vc, operationStore,
true, false,
false, cnstypes.CnsClusterFlavorVanilla)
volumeManager, err := cnsvolume.GetManager(ctx, vc, operationStore, true,
false, false,
cnstypes.CnsClusterFlavorVanilla, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down Expand Up @@ -218,9 +218,9 @@ func (c *controller) Init(config *cnsconfig.Config, version string) error {
"err=%v", vcenterconfig.Host, err)
}
c.managers.VcenterConfigs[vcenterconfig.Host] = vcenterconfig
volumeManager, err := cnsvolume.GetManager(ctx, vcenter,
operationStore, true, true,
multivCenterTopologyDeployment, cnstypes.CnsClusterFlavorVanilla)
volumeManager, err := cnsvolume.GetManager(ctx, vcenter, operationStore,
true, true, multivCenterTopologyDeployment,
cnstypes.CnsClusterFlavorVanilla, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down
17 changes: 14 additions & 3 deletions pkg/csi/service/vanilla/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"strings"
"sync"
"testing"
"time"

"github.com/google/uuid"
"github.com/vmware/govmomi/cns"
Expand All @@ -33,6 +34,7 @@ import (
"github.com/vmware/govmomi/pbm/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"k8s.io/apimachinery/pkg/util/wait"

"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/vmware/govmomi/simulator"
Expand Down Expand Up @@ -216,13 +218,22 @@ func getControllerTest(t *testing.T) *controllerTest {
t.Fatal(err)
}

volumeManager, err := cnsvolume.GetManager(ctx, vcenter,
fakeOpStore, true, false,
false, cnstypes.CnsClusterFlavorVanilla)
volumeManager, err := cnsvolume.GetManager(ctx, vcenter, fakeOpStore,
true, false, false,
cnstypes.CnsClusterFlavorVanilla, true)
if err != nil {
t.Fatalf("failed to create an instance of volume manager. err=%v", err)
}

// wait till property collector has been started
err = wait.PollUntilContextTimeout(ctx, 1*time.Second, 10*time.Second, false,
func(ctx context.Context) (done bool, err error) {
return volumeManager.IsListViewReady(), nil
})
if err != nil {
t.Fatalf("listview not ready. err=%v", err)
}

manager := &common.Manager{
VcenterConfig: vcenterconfig,
CnsConfig: config,
Expand Down
5 changes: 2 additions & 3 deletions pkg/csi/service/vanilla/controller_topology_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,8 @@ func getControllerTestWithTopology(t *testing.T) *controllerTestTopology {
t.Fatal(err)
}

volumeManager, err := cnsvolume.GetManager(ctxtopology, vcenter,
fakeOpStore, true, false,
false, cnstypes.CnsClusterFlavorVanilla)
volumeManager, err := cnsvolume.GetManager(ctxtopology, vcenter, fakeOpStore, true, false, false,
cnstypes.CnsClusterFlavorVanilla, true)
if err != nil {
t.Fatalf("failed to create an instance of volume manager. err=%v", err)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/csi/service/wcp/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ func (c *controller) Init(config *cnsconfig.Config, version string) error {
}

volumeManager, err := cnsvolume.GetManager(ctx, vcenter, operationStore,
idempotencyHandlingEnabled, false,
false, cnstypes.CnsClusterFlavorWorkload)
idempotencyHandlingEnabled, false, false,
cnstypes.CnsClusterFlavorWorkload, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down Expand Up @@ -397,8 +397,8 @@ func (c *controller) ReloadConfiguration(reconnectToVCFromNewConfig bool) error
c.manager.VcenterConfig = newVCConfig

volumeManager, err := cnsvolume.GetManager(ctx, vcenter, operationStore,
idempotencyHandlingEnabled, false,
false, cnstypes.CnsClusterFlavorWorkload)
idempotencyHandlingEnabled, false, false,
cnstypes.CnsClusterFlavorWorkload, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down
18 changes: 15 additions & 3 deletions pkg/csi/service/wcp/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ import (
"strings"
"sync"
"testing"
"time"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"k8s.io/apimachinery/pkg/util/wait"

"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/google/uuid"
Expand All @@ -33,6 +35,7 @@ import (
v1 "k8s.io/api/core/v1"

cnstypes "github.com/vmware/govmomi/cns/types"

cnsvolume "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume"
cnsvsphere "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/vsphere"
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
Expand Down Expand Up @@ -112,13 +115,22 @@ func getControllerTest(t *testing.T) *controllerTest {
t.Fatalf("Failed to create co agnostic interface. err=%v", err)
}

volumeManager, err := cnsvolume.GetManager(ctx, vcenter,
fakeOpStore, true, false,
false, cnstypes.CnsClusterFlavorWorkload)
volumeManager, err := cnsvolume.GetManager(ctx, vcenter, fakeOpStore,
true, false, false,
cnstypes.CnsClusterFlavorWorkload, false)
if err != nil {
t.Fatalf("failed to create an instance of volume manager. err=%v", err)
}

// wait till property collector has been started
err = wait.PollUntilContextTimeout(ctx, 1*time.Second, 10*time.Second, false,
func(ctx context.Context) (done bool, err error) {
return volumeManager.IsListViewReady(), nil
})
if err != nil {
t.Fatalf("listview not ready. err=%v", err)
}

manager := &common.Manager{
VcenterConfig: vcenterconfig,
CnsConfig: config,
Expand Down
2 changes: 1 addition & 1 deletion pkg/syncer/cnsoperator/manager/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func InitCnsOperator(ctx context.Context, clusterFlavor cnstypes.CnsClusterFlavo
}
}

volumeManager, err = volumes.GetManager(ctx, vCenter, nil, false, false, false, clusterFlavor)
volumeManager, err = volumes.GetManager(ctx, vCenter, nil, false, false, false, clusterFlavor, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down
24 changes: 14 additions & 10 deletions pkg/syncer/metadatasyncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (

"github.com/go-logr/zapr"
cr_log "sigs.k8s.io/controller-runtime/pkg/log"

cnsoperatorv1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator"
storagepolicyv1alpha2 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/storagepolicy/v1alpha2"
sqperiodicsyncv1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/storagequotaperiodicsync/v1alpha1"
Expand Down Expand Up @@ -98,7 +99,7 @@ var (
// isMultiVCenterFssEnabled is true if the Multi VC support FSS is enabled, false otherwise.
isMultiVCenterFssEnabled bool

//IsMigrationEnabled is true when in-tree to CSI Migration FSS is enabled for the driver, false otherwise.
// IsMigrationEnabled is true when in-tree to CSI Migration FSS is enabled for the driver, false otherwise.
IsMigrationEnabled bool
// nodeMgr stores the manager to interact with nodeVMs.
nodeMgr node.Manager
Expand Down Expand Up @@ -463,8 +464,9 @@ func InitMetadataSyncer(ctx context.Context, clusterFlavor cnstypes.CnsClusterFl
volumeInfoCrDeletionMap[metadataSyncer.host] = make(map[string]bool)
volumeOperationsLock[metadataSyncer.host] = &sync.Mutex{}

volumeManager, err := volumes.GetManager(ctx, vCenter,
nil, false, false, false, metadataSyncer.clusterFlavor)
volumeManager, err := volumes.GetManager(ctx, vCenter, nil,
false, false,
false, metadataSyncer.clusterFlavor, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down Expand Up @@ -538,8 +540,9 @@ func InitMetadataSyncer(ctx context.Context, clusterFlavor cnstypes.CnsClusterFl
volumeInfoCrDeletionMap[metadataSyncer.host] = make(map[string]bool)
volumeOperationsLock[metadataSyncer.host] = &sync.Mutex{}

volumeManager, err := volumes.GetManager(ctx, vCenter, nil, false, false, false,
metadataSyncer.clusterFlavor)
volumeManager, err := volumes.GetManager(ctx, vCenter, nil,
false, false,
false, metadataSyncer.clusterFlavor, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand All @@ -561,9 +564,9 @@ func InitMetadataSyncer(ctx context.Context, clusterFlavor cnstypes.CnsClusterFl
return logger.LogNewErrorf(log, "failed to get vCenterInstance for vCenter Host: %q, err: %v",
vcconfig.Host, err)
}
volumeManager, err := volumes.GetManager(ctx, vCenter,
nil, false, true,
multivCenterTopologyDeployment, metadataSyncer.clusterFlavor)
volumeManager, err := volumes.GetManager(ctx, vCenter, nil,
false, true,
multivCenterTopologyDeployment, metadataSyncer.clusterFlavor, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down Expand Up @@ -2222,8 +2225,9 @@ func ReloadConfiguration(metadataSyncer *metadataSyncInformer, reconnectToVCFrom
if err != nil {
return logger.LogNewErrorf(log, "failed to reset volume manager. err=%v", err)
}
volumeManager, err := volumes.GetManager(ctx, vcenter, nil, false, false, false,
metadataSyncer.clusterFlavor)
volumeManager, err := volumes.GetManager(ctx, vcenter, nil,
false, false,
false, metadataSyncer.clusterFlavor, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/syncer/storagepool/diskDecommissionController.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (w *DiskDecommController) detachVolumes(ctx context.Context, storagePoolNam
if err != nil {
return logger.LogNewErrorf(log, "failed to get cluster flavor. Error: %v", err)
}
volManager, err := volume.GetManager(ctx, &vc, nil, false, false, false, clusterFlavor)
volManager, err := volume.GetManager(ctx, &vc, nil, false, false, false, clusterFlavor, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/syncer/storagepool/migrationController.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (m *migrationController) relocateCNSVolume(ctx context.Context, volumeID st
if err != nil {
return logger.LogNewErrorf(log, "failed to get cluster flavor. Error: %v", err)
}
volManager, err := volume.GetManager(ctx, m.vc, nil, false, false, false, clusterFlavor)
volManager, err := volume.GetManager(ctx, m.vc, nil, false, false, false, clusterFlavor, false)
if err != nil {
return logger.LogNewErrorf(log, "failed to create an instance of volume manager. err=%v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/syncer/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func TestSyncerWorkflows(t *testing.T) {
}
}()

volumeManager, err = cnsvolumes.GetManager(ctx, virtualCenter, nil, false, false, false, "")
volumeManager, err = cnsvolumes.GetManager(ctx, virtualCenter, nil, false, false, false, "", false)
if err != nil {
t.Fatalf("failed to create an instance of volume manager. err=%v", err)
}
Expand Down

0 comments on commit 918f5c4

Please sign in to comment.