OCPBUGS-19990: Do not allow removing device classes after initial deployment

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{

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)