Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(wip) fix flaky unit tests #3118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
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