From 303a200080242cdd58d0af02f610951d4f29c20b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakob=20M=C3=B6ller?= Date: Tue, 24 Oct 2023 14:09:26 +0200 Subject: [PATCH] test: various reconcile failure branch tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jakob Möller --- internal/controllers/vgmanager/controller.go | 2 +- .../controllers/vgmanager/controller_test.go | 162 +++++++++++++++++- internal/controllers/vgmanager/devices.go | 5 +- 3 files changed, 164 insertions(+), 5 deletions(-) diff --git a/internal/controllers/vgmanager/controller.go b/internal/controllers/vgmanager/controller.go index 875a78337..9d12ebaee 100644 --- a/internal/controllers/vgmanager/controller.go +++ b/internal/controllers/vgmanager/controller.go @@ -164,7 +164,7 @@ func (r *Reconciler) reconcile( if wiped { blockDevices, err = r.LSBLK.ListBlockDevices() if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to list block devices: %w", err) + return ctrl.Result{}, fmt.Errorf("failed to list block devices after wiping devices: %w", err) } } diff --git a/internal/controllers/vgmanager/controller_test.go b/internal/controllers/vgmanager/controller_test.go index ecf25f9d1..c3c312964 100644 --- a/internal/controllers/vgmanager/controller_test.go +++ b/internal/controllers/vgmanager/controller_test.go @@ -24,12 +24,14 @@ import ( lvmmocks "github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvm/mocks" "github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvmd" lvmdmocks "github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvmd/mocks" + wipefsmocks "github.com/openshift/lvm-operator/internal/controllers/vgmanager/wipefs/mocks" "github.com/stretchr/testify/mock" topolvmv1 "github.com/topolvm/topolvm/api/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" @@ -47,6 +49,9 @@ var _ = Describe("vgmanager controller", func() { Context("verifying standard behavior with node selector", func() { It("should be reconciled successfully with a mocked block device", testMockedBlockDeviceOnHost) Context("edge cases during reconciliation", func() { + Context("failure in LVM or LSBLK", func() { + It("reconcile failure because of external errors", testReconcileFailure) + }) It("should fail on error while fetching LVMVolumeGroup", testErrorOnGetLVMVolumeGroup) It("should correctly handle node selector", testNodeSelector) It("should handle LVMD edge cases correctly", testLVMD) @@ -78,9 +83,10 @@ func init() { } type testInstances struct { - LVM *lvmmocks.MockLVM - LSBLK *lsblkmocks.MockLSBLK - LVMD lvmd.Configurator + LVM *lvmmocks.MockLVM + LSBLK *lsblkmocks.MockLSBLK + LVMD lvmd.Configurator + Wipefs *wipefsmocks.MockWipefs host string namespace *corev1.Namespace @@ -101,6 +107,7 @@ func setupInstances() testInstances { mockLSBLK := lsblkmocks.NewMockLSBLK(t) mockLVM := lvmmocks.NewMockLVM(t) + mockWipefs := wipefsmocks.NewMockWipefs(t) testLVMD := lvmd.NewFileConfigurator(filepath.Join(t.TempDir(), "lvmd.yaml")) hostname := "test-host.vgmanager.test.io" @@ -121,6 +128,7 @@ func setupInstances() testInstances { LVM: mockLVM, LSBLK: mockLSBLK, LVMD: testLVMD, + Wipefs: mockWipefs, namespace: namespace, node: node, host: hostname, @@ -140,6 +148,7 @@ func setupInstances() testInstances { LVMD: testLVMD, LVM: mockLVM, LSBLK: mockLSBLK, + Wipefs: mockWipefs, NodeName: node.GetName(), Namespace: namespace.GetName(), Filters: filter.DefaultFilters, @@ -669,3 +678,150 @@ func testThinPoolCreation(ctx context.Context) { err = r.addThinPoolToVG(ctx, "vg1", thinPool) Expect(err).ToNot(HaveOccurred(), "should not error if thin pool already exists, extension should work") } + +func testReconcileFailure(ctx context.Context) { + logger := zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)) + ctx = log.IntoContext(ctx, logger) + + By("injecting mocked LVM and LSBLK") + instances := setupInstances() + mockLVMD := lvmdmocks.NewMockConfigurator(GinkgoT()) + instances.LVMD = mockLVMD + + vg := &lvmv1alpha1.LVMVolumeGroup{} + By("creating the LVMVolumeGroup with the mocked device", func() { + vg.SetName("vg1") + vg.SetNamespace(instances.namespace.GetName()) + vg.Spec.NodeSelector = instances.nodeSelector.DeepCopy() + vg.Spec.DeviceSelector = &lvmv1alpha1.DeviceSelector{Paths: []string{"/dev/sda"}} + vg.Spec.DeviceSelector.ForceWipeDevicesAndDestroyAllData = ptr.To(true) + vg.Spec.ThinPoolConfig = &lvmv1alpha1.ThinPoolConfig{ + Name: "thin-pool-1", + SizePercent: 90, + OverprovisionRatio: 10, + } + Expect(instances.client.Create(ctx, vg)).To(Succeed()) + // First reconcile adds finalizer + _, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)}) + Expect(err).ToNot(HaveOccurred()) + }) + + By("triggering listblockdevices failure", func() { + instances.LSBLK.EXPECT().ListBlockDevices().Once().Return(nil, fmt.Errorf("mocked error")) + _, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to list block devices")) + }) + + By("triggering wipefs failure", func() { + instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{ + {Name: "/dev/sda", KName: "/dev/sda", FSType: "ext4"}, + }, nil) + instances.Wipefs.EXPECT().Wipe("/dev/sda").Once().Return(fmt.Errorf("mocked error")) + _, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to wipe devices")) + }) + + By("triggering lsblk failure after wipefs", func() { + instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{ + {Name: "/dev/sda", KName: "/dev/sda", FSType: "ext4"}, + }, nil) + instances.Wipefs.EXPECT().Wipe("/dev/sda").Once().Return(nil) + instances.LSBLK.EXPECT().ListBlockDevices().Once().Return(nil, fmt.Errorf("mocked error")) + _, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to list block devices after wiping devices")) + }) + + By("triggering failure on fetching new devices to add to volume group", func() { + blockDevices := []lsblk.BlockDevice{ + {Name: "/dev/xxx", KName: "/dev/xxx", FSType: "ext4"}, + } + instances.LSBLK.EXPECT().ListBlockDevices().Once().Return(blockDevices, nil) + _, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unable to validate device")) + }) + + Expect(instances.client.Get(ctx, client.ObjectKeyFromObject(vg), vg)).To(Succeed()) + vg.Spec.DeviceSelector.ForceWipeDevicesAndDestroyAllData = ptr.To(false) + Expect(instances.client.Update(ctx, vg)).To(Succeed()) + + By("triggering failure because vg is not found even though there are no devices to be added", func() { + evalSymlinks = func(path string) (string, error) { + return path, nil + } + defer func() { + evalSymlinks = filepath.EvalSymlinks + }() + instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{ + {Name: "/dev/sda", KName: "/dev/sda", FSType: "xfs", PartLabel: "reserved"}, + }, nil) + instances.LVM.EXPECT().ListVGs().Twice().Return(nil, nil) + instances.LSBLK.EXPECT().HasBindMounts(mock.Anything).Once().Return(false, "", nil) + _, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)}) + Expect(err).To(HaveOccurred()) + + Expect(err.Error()).To(ContainSubstring("the volume group vg1 does not exist and there were no available devices to create it")) + }) + + By("triggering failure because vg is not found and the status update fails as well", func() { + evalSymlinks = func(path string) (string, error) { + return path, nil + } + defer func() { + evalSymlinks = filepath.EvalSymlinks + }() + instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{ + {Name: "/dev/sda", KName: "/dev/sda", FSType: "xfs", PartLabel: "reserved"}, + }, nil) + instances.LVM.EXPECT().ListVGs().Once().Return(nil, nil) + instances.LVM.EXPECT().ListVGs().Once().Return(nil, fmt.Errorf("mocked error")) + instances.LSBLK.EXPECT().HasBindMounts(mock.Anything).Once().Return(false, "", nil) + _, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("the volume group vg1 does not exist and there were no available devices to create it")) + }) + + By("triggering failure because vg is found but thin-pool validation failed", func() { + evalSymlinks = func(path string) (string, error) { + return path, nil + } + defer func() { + evalSymlinks = filepath.EvalSymlinks + }() + instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{ + {Name: "/dev/sda", KName: "/dev/sda", FSType: "xfs", PartLabel: "reserved"}, + }, nil) + vgs := []lvm.VolumeGroup{ + {Name: "vg1", VgSize: "1g"}, + } + instances.LVM.EXPECT().ListVGs().Twice().Return(vgs, nil) + instances.LSBLK.EXPECT().HasBindMounts(mock.Anything).Once().Return(false, "", nil) + instances.LVM.EXPECT().ListLVs("vg1").Once().Return(nil, fmt.Errorf("mocked error")) + _, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)}) + Expect(err).To(HaveOccurred()) + }) + + By("triggering failure because vg is found but thin-pool validation failed and the status update as well", func() { + evalSymlinks = func(path string) (string, error) { + return path, nil + } + defer func() { + evalSymlinks = filepath.EvalSymlinks + }() + instances.LSBLK.EXPECT().ListBlockDevices().Once().Return([]lsblk.BlockDevice{ + {Name: "/dev/sda", KName: "/dev/sda", FSType: "xfs", PartLabel: "reserved"}, + }, nil) + vgs := []lvm.VolumeGroup{ + {Name: "vg1", VgSize: "1g"}, + } + instances.LVM.EXPECT().ListVGs().Once().Return(vgs, nil) + instances.LVM.EXPECT().ListVGs().Once().Return(nil, fmt.Errorf("mocked error")) + instances.LSBLK.EXPECT().HasBindMounts(mock.Anything).Once().Return(false, "", nil) + instances.LVM.EXPECT().ListLVs("vg1").Once().Return(nil, fmt.Errorf("mocked error")) + _, err := instances.Reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(vg)}) + Expect(err).To(HaveOccurred()) + }) +} diff --git a/internal/controllers/vgmanager/devices.go b/internal/controllers/vgmanager/devices.go index 84b6729cd..d26bc8b9f 100644 --- a/internal/controllers/vgmanager/devices.go +++ b/internal/controllers/vgmanager/devices.go @@ -231,13 +231,16 @@ func hasExactDisk(blockDevices []lsblk.BlockDevice, deviceName string) (lsblk.Bl return lsblk.BlockDevice{}, false } +// evalSymlinks redefined to be able to override in tests +var evalSymlinks = filepath.EvalSymlinks + // getValidDevice will do various checks on a device path to make sure it is a valid device // // An error will be returned if the device is invalid // No error and an empty BlockDevice object will be returned if this device should be skipped (ex: duplicate device) func getValidDevice(devicePath string, blockDevices []lsblk.BlockDevice, nodeStatus *lvmv1alpha1.LVMVolumeGroupNodeStatus, volumeGroup *lvmv1alpha1.LVMVolumeGroup) (lsblk.BlockDevice, error) { // Make sure the symlink exists - diskName, err := filepath.EvalSymlinks(devicePath) + diskName, err := evalSymlinks(devicePath) if err != nil { return lsblk.BlockDevice{}, fmt.Errorf("unable to find symlink for disk path %s: %v", devicePath, err) }