diff --git a/api/v1alpha1/lvmcluster_test.go b/api/v1alpha1/lvmcluster_test.go index 7e7315176..c8e2af229 100644 --- a/api/v1alpha1/lvmcluster_test.go +++ b/api/v1alpha1/lvmcluster_test.go @@ -150,6 +150,73 @@ var _ = Describe("webhook acceptance tests", func() { Expect(statusError.Status().Message).To(ContainSubstring(ErrAtLeastOneDeviceClassRequired.Error())) }) + It("device classes cannot be removed in update", func(ctx SpecContext) { + resource := defaultLVMClusterInUniqueNamespace(ctx) + resource.Spec.Storage.DeviceClasses = []DeviceClass{ + { + Name: "test-device-class-1", + ThinPoolConfig: &ThinPoolConfig{ + Name: "thin-pool-1", + SizePercent: 90, + OverprovisionRatio: 10, + }, + DeviceSelector: &DeviceSelector{Paths: []string{ + "/dev/test1", + }}, + FilesystemType: "xfs", + }, + { + Name: "test-device-class-2", + ThinPoolConfig: &ThinPoolConfig{ + Name: "thin-pool-1", + SizePercent: 90, + OverprovisionRatio: 10, + }, + DeviceSelector: &DeviceSelector{Paths: []string{ + "/dev/test2", + }}, + FilesystemType: "xfs", + }, + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + + updated := resource.DeepCopy() + updated.Spec.Storage.DeviceClasses = []DeviceClass{ + { + Name: "test-device-class-1", + ThinPoolConfig: &ThinPoolConfig{ + Name: "thin-pool-1", + SizePercent: 90, + OverprovisionRatio: 10, + }, + DeviceSelector: &DeviceSelector{Paths: []string{ + "/dev/test1", + }}, + FilesystemType: "xfs", + }, + { + Name: "test-device-class-3", + ThinPoolConfig: &ThinPoolConfig{ + Name: "thin-pool-1", + SizePercent: 90, + OverprovisionRatio: 10, + }, + DeviceSelector: &DeviceSelector{Paths: []string{ + "/dev/test3", + }}, + FilesystemType: "xfs", + }, + } + err := k8sClient.Update(ctx, updated) + Expect(err).To(HaveOccurred()) + Expect(err).To(Satisfy(k8serrors.IsForbidden)) + statusError := &k8serrors.StatusError{} + Expect(errors.As(err, &statusError)).To(BeTrue()) + Expect(statusError.Status().Message).To(ContainSubstring("device classes were deleted from the LVMCluster")) + + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + }) + It("two default device classes are not allowed", func(ctx SpecContext) { resource := defaultLVMClusterInUniqueNamespace(ctx) resource.Spec.Storage.DeviceClasses = append(resource.Spec.Storage.DeviceClasses, DeviceClass{ diff --git a/api/v1alpha1/lvmcluster_webhook.go b/api/v1alpha1/lvmcluster_webhook.go index d7f57654a..ddbbcb5d7 100644 --- a/api/v1alpha1/lvmcluster_webhook.go +++ b/api/v1alpha1/lvmcluster_webhook.go @@ -151,7 +151,13 @@ func (v *lvmClusterValidator) ValidateUpdate(_ context.Context, old, new runtime oldLVMCluster, ok := old.(*LVMCluster) if !ok { - return warnings, fmt.Errorf("Failed to parse LVMCluster.") + return warnings, fmt.Errorf("failed to parse LVMCluster") + } + + // Validate all the old device classes still exist + err = validateDeviceClassesStillExist(oldLVMCluster.Spec.Storage.DeviceClasses, l.Spec.Storage.DeviceClasses) + if err != nil { + return warnings, fmt.Errorf("invalid: device classes were deleted from the LVMCluster: %w", err) } for _, deviceClass := range l.Spec.Storage.DeviceClasses { @@ -224,6 +230,25 @@ func (v *lvmClusterValidator) ValidateUpdate(_ context.Context, old, new runtime return warnings, nil } +func validateDeviceClassesStillExist(old, new []DeviceClass) error { + deviceClassMap := make(map[string]bool) + + for _, deviceClass := range old { + deviceClassMap[deviceClass.Name] = true + } + + for _, deviceClass := range new { + delete(deviceClassMap, deviceClass.Name) + } + + // if any old device class is removed now + if len(deviceClassMap) != 0 { + return fmt.Errorf("device classes can not be removed from the LVMCluster once added oldDeviceClasses:%v, newDeviceClasses:%v", old, new) + } + + return nil +} + func validateDevicePathsStillExist(old, new []string) error { deviceMap := make(map[string]bool)