From 93890cec68c27bde22744260aa1cf7089e8e590f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20M=C3=B6ller?= Date: Tue, 12 Sep 2023 17:30:23 +0200 Subject: [PATCH] fix: attempt race fix --- pkg/vgmanager/devices_test.go | 6 +-- pkg/vgmanager/kname_test.go | 17 ++++++++ pkg/vgmanager/suite_test.go | 50 +++++++++++++++------- pkg/vgmanager/vgmanager_controller_test.go | 8 ++-- 4 files changed, 56 insertions(+), 25 deletions(-) create mode 100644 pkg/vgmanager/kname_test.go diff --git a/pkg/vgmanager/devices_test.go b/pkg/vgmanager/devices_test.go index f94b8be68..2ef1acbf9 100644 --- a/pkg/vgmanager/devices_test.go +++ b/pkg/vgmanager/devices_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "runtime" "testing" "github.com/go-logr/logr/testr" @@ -587,8 +586,5 @@ func TestAvailableDevicesForVG(t *testing.T) { // it has /private in the beginning because /tmp symlinks are evaluated as with /private in the beginning on darwin. func calculateDevicePath(t *testing.T, deviceName string) string { t.Helper() - if runtime.GOOS == "darwin" { - return fmt.Sprintf("/private%s", devicePaths[deviceName]) - } - return devicePaths[deviceName] + return getKNameFromDevice(devicePaths[deviceName]) } diff --git a/pkg/vgmanager/kname_test.go b/pkg/vgmanager/kname_test.go new file mode 100644 index 000000000..b8c1dc758 --- /dev/null +++ b/pkg/vgmanager/kname_test.go @@ -0,0 +1,17 @@ +package vgmanager + +import ( + "path/filepath" + "runtime" +) + +func getKNameFromDevice(device string) string { + // HACK: if we are on unix, we can simply use the "/tmp" path. + // if we are on darwin, then the symlink of the temp file + // will resolve to /private/var from /var, so we have to adjust + // the block device name + if runtime.GOOS == "darwin" { + return filepath.Join("/", "private", device) + } + return device +} diff --git a/pkg/vgmanager/suite_test.go b/pkg/vgmanager/suite_test.go index 141c9e9b4..0ff6ea150 100644 --- a/pkg/vgmanager/suite_test.go +++ b/pkg/vgmanager/suite_test.go @@ -24,9 +24,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - configv1 "github.com/openshift/api/config/v1" secv1 "github.com/openshift/api/security/v1" + lsblkmocks "github.com/openshift/lvm-operator/pkg/lsblk/mocks" + lvmmocks "github.com/openshift/lvm-operator/pkg/lvm/mocks" + "github.com/openshift/lvm-operator/pkg/lvmd" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes/scheme" @@ -40,10 +42,6 @@ import ( lvmv1alpha1 "github.com/openshift/lvm-operator/api/v1alpha1" "github.com/openshift/lvm-operator/pkg/filter" - lsblkmocks "github.com/openshift/lvm-operator/pkg/lsblk/mocks" - lvmmocks "github.com/openshift/lvm-operator/pkg/lvm/mocks" - "github.com/openshift/lvm-operator/pkg/lvmd" - topolvmv1 "github.com/topolvm/topolvm/api/v1" //+kubebuilder:scaffold:imports ) @@ -57,8 +55,7 @@ var ( ctx context.Context cancel context.CancelFunc testNodeSelector corev1.NodeSelector - mockLSBLK *lsblkmocks.MockLSBLK - mockLVM *lvmmocks.MockLVM + testVGReconciler *VGReconciler ) func TestAPIs(t *testing.T) { @@ -141,20 +138,15 @@ var _ = BeforeSuite(func() { }}} Expect(k8sClient.Create(ctx, testNode)).Should(Succeed()) - testLVMD := lvmd.NewFileConfigurator(filepath.Join(GinkgoT().TempDir(), "lvmd.yaml")) - mockLSBLK = lsblkmocks.NewMockLSBLK(GinkgoT()) - mockLVM = lvmmocks.NewMockLVM(GinkgoT()) - err = (&VGReconciler{ + testVGReconciler = &VGReconciler{ Client: k8sManager.GetClient(), Scheme: k8sManager.GetScheme(), EventRecorder: k8sManager.GetEventRecorderFor(ControllerName), - LVM: mockLVM, - LSBLK: mockLSBLK, - LVMD: testLVMD, Namespace: testNamespaceName, NodeName: testNodeName, Filters: filter.DefaultFilters, - }).SetupWithManager(k8sManager) + } + err = (testVGReconciler).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) go func() { @@ -170,3 +162,31 @@ var _ = AfterSuite(func() { By("tearing down the test environment") Expect(testEnv.Stop()).To(Succeed()) }) + +func setupMocks() (*lvmmocks.MockLVM, *lsblkmocks.MockLSBLK) { + t := GinkgoT() + t.Helper() + + mockLSBLK := &lsblkmocks.MockLSBLK{} + mockLSBLK.Mock.Test(t) + DeferCleanup(func() { + mockLSBLK.AssertExpectations(t) + }) + mockLVM := &lvmmocks.MockLVM{} + mockLVM.Mock.Test(t) + DeferCleanup(func() { + mockLVM.AssertExpectations(t) + }) + testLVMD := lvmd.NewFileConfigurator(filepath.Join(t.TempDir(), "lvmd.yaml")) + + testVGReconciler.LSBLK = mockLSBLK + testVGReconciler.LVM = mockLVM + testVGReconciler.LVMD = testLVMD + DeferCleanup(func() { + testVGReconciler.LVMD = nil + testVGReconciler.LSBLK = nil + testVGReconciler.LVM = nil + }) + + return mockLVM, mockLSBLK +} diff --git a/pkg/vgmanager/vgmanager_controller_test.go b/pkg/vgmanager/vgmanager_controller_test.go index fd30fb078..4b40af4b0 100644 --- a/pkg/vgmanager/vgmanager_controller_test.go +++ b/pkg/vgmanager/vgmanager_controller_test.go @@ -25,10 +25,8 @@ var _ = Describe("vgmanager controller", func() { }) func testMockedBlockDeviceOnHost(ctx context.Context) { - DeferCleanup(func() { - mockLVM.AssertExpectations(GinkgoT()) - mockLSBLK.AssertExpectations(GinkgoT()) - }) + By("injecting mocked LVM and LSBLK") + mockLVM, mockLSBLK := setupMocks() By("setting up the disk as a block device with losetup") device := filepath.Join(GinkgoT().TempDir(), "mock0") @@ -52,7 +50,7 @@ func testMockedBlockDeviceOnHost(ctx context.Context) { mockLSBLK.EXPECT().ListBlockDevices().Return([]lsblk.BlockDevice{ { Name: "mock0", - KName: device, + KName: getKNameFromDevice(device), Type: "mocked", Model: "mocked", Vendor: "mocked",