From c1cd483d28648e6e5885b5f96396767dd2bb1a87 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Tue, 19 Aug 2025 11:12:10 +0300 Subject: [PATCH 1/4] add webhook Signed-off-by: Valeriy Khorunzhin --- .../pkg/builder/cvi/cvi.go | 47 +++++ .../pkg/builder/cvi/option.go | 41 ++++ .../pkg/builder/vd/option.go | 30 +++ .../pkg/builder/vd/vd.go | 48 +++++ .../pkg/builder/vi/option.go | 42 ++++ .../pkg/builder/vi/vi.go | 48 +++++ .../pkg/builder/vm/option.go | 7 + .../first_block_device_validator.go | 135 +++++++++++++ .../first_block_device_validator_test.go | 187 ++++++++++++++++++ .../pkg/controller/vm/vm_webhook.go | 1 + 10 files changed, 586 insertions(+) create mode 100644 images/virtualization-artifact/pkg/builder/cvi/cvi.go create mode 100644 images/virtualization-artifact/pkg/builder/cvi/option.go create mode 100644 images/virtualization-artifact/pkg/builder/vd/option.go create mode 100644 images/virtualization-artifact/pkg/builder/vd/vd.go create mode 100644 images/virtualization-artifact/pkg/builder/vi/option.go create mode 100644 images/virtualization-artifact/pkg/builder/vi/vi.go create mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go create mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go diff --git a/images/virtualization-artifact/pkg/builder/cvi/cvi.go b/images/virtualization-artifact/pkg/builder/cvi/cvi.go new file mode 100644 index 0000000000..e490f3e3c4 --- /dev/null +++ b/images/virtualization-artifact/pkg/builder/cvi/cvi.go @@ -0,0 +1,47 @@ +/* +Copyright 2025 Flant JSC +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cvi + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +func New(options ...Option) *v1alpha2.ClusterVirtualImage { + cvi := NewEmpty("") + ApplyOptions(cvi, options) + return cvi +} + +func ApplyOptions(cvi *v1alpha2.ClusterVirtualImage, opts []Option) { + if cvi == nil { + return + } + for _, opt := range opts { + opt(cvi) + } +} + +func NewEmpty(name string) *v1alpha2.ClusterVirtualImage { + return &v1alpha2.ClusterVirtualImage{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha2.SchemeGroupVersion.String(), + Kind: v1alpha2.ClusterVirtualImageKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } +} diff --git a/images/virtualization-artifact/pkg/builder/cvi/option.go b/images/virtualization-artifact/pkg/builder/cvi/option.go new file mode 100644 index 0000000000..f4c5f2ce1c --- /dev/null +++ b/images/virtualization-artifact/pkg/builder/cvi/option.go @@ -0,0 +1,41 @@ +/* +Copyright 2025 Flant JSC +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cvi + +import ( + "github.com/deckhouse/virtualization-controller/pkg/builder/meta" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +type Option func(cvi *v1alpha2.ClusterVirtualImage) + +var ( + WithName = meta.WithName[*v1alpha2.ClusterVirtualImage] + WithLabel = meta.WithLabel[*v1alpha2.ClusterVirtualImage] + WithLabels = meta.WithLabels[*v1alpha2.ClusterVirtualImage] + WithAnnotation = meta.WithAnnotation[*v1alpha2.ClusterVirtualImage] + WithAnnotations = meta.WithAnnotations[*v1alpha2.ClusterVirtualImage] +) + +func WithPhase(phase v1alpha2.ImagePhase) func(vi *v1alpha2.ClusterVirtualImage) { + return func(vi *v1alpha2.ClusterVirtualImage) { + vi.Status.Phase = phase + } +} + +func WithCDROM(cdrom bool) func(vi *v1alpha2.ClusterVirtualImage) { + return func(vi *v1alpha2.ClusterVirtualImage) { + vi.Status.CDROM = cdrom + } +} diff --git a/images/virtualization-artifact/pkg/builder/vd/option.go b/images/virtualization-artifact/pkg/builder/vd/option.go new file mode 100644 index 0000000000..e3c58b28e4 --- /dev/null +++ b/images/virtualization-artifact/pkg/builder/vd/option.go @@ -0,0 +1,30 @@ +/* +Copyright 2025 Flant JSC +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vd + +import ( + "github.com/deckhouse/virtualization-controller/pkg/builder/meta" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +type Option func(vd *v1alpha2.VirtualDisk) + +var ( + WithName = meta.WithName[*v1alpha2.VirtualDisk] + WithNamespace = meta.WithNamespace[*v1alpha2.VirtualDisk] + WithLabel = meta.WithLabel[*v1alpha2.VirtualDisk] + WithLabels = meta.WithLabels[*v1alpha2.VirtualDisk] + WithAnnotation = meta.WithAnnotation[*v1alpha2.VirtualDisk] + WithAnnotations = meta.WithAnnotations[*v1alpha2.VirtualDisk] +) diff --git a/images/virtualization-artifact/pkg/builder/vd/vd.go b/images/virtualization-artifact/pkg/builder/vd/vd.go new file mode 100644 index 0000000000..c0102d321a --- /dev/null +++ b/images/virtualization-artifact/pkg/builder/vd/vd.go @@ -0,0 +1,48 @@ +/* +Copyright 2025 Flant JSC +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vd + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +func New(options ...Option) *v1alpha2.VirtualDisk { + vd := NewEmpty("", "") + ApplyOptions(vd, options) + return vd +} + +func ApplyOptions(vd *v1alpha2.VirtualDisk, opts []Option) { + if vd == nil { + return + } + for _, opt := range opts { + opt(vd) + } +} + +func NewEmpty(name, namespace string) *v1alpha2.VirtualDisk { + return &v1alpha2.VirtualDisk{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha2.SchemeGroupVersion.String(), + Kind: v1alpha2.VirtualDiskKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } +} diff --git a/images/virtualization-artifact/pkg/builder/vi/option.go b/images/virtualization-artifact/pkg/builder/vi/option.go new file mode 100644 index 0000000000..26c0e8d1de --- /dev/null +++ b/images/virtualization-artifact/pkg/builder/vi/option.go @@ -0,0 +1,42 @@ +/* +Copyright 2025 Flant JSC +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vi + +import ( + "github.com/deckhouse/virtualization-controller/pkg/builder/meta" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +type Option func(vi *v1alpha2.VirtualImage) + +var ( + WithName = meta.WithName[*v1alpha2.VirtualImage] + WithNamespace = meta.WithNamespace[*v1alpha2.VirtualImage] + WithLabel = meta.WithLabel[*v1alpha2.VirtualImage] + WithLabels = meta.WithLabels[*v1alpha2.VirtualImage] + WithAnnotation = meta.WithAnnotation[*v1alpha2.VirtualImage] + WithAnnotations = meta.WithAnnotations[*v1alpha2.VirtualImage] +) + +func WithPhase(phase v1alpha2.ImagePhase) func(vi *v1alpha2.VirtualImage) { + return func(vi *v1alpha2.VirtualImage) { + vi.Status.Phase = phase + } +} + +func WithCDROM(cdrom bool) func(vi *v1alpha2.VirtualImage) { + return func(vi *v1alpha2.VirtualImage) { + vi.Status.CDROM = cdrom + } +} diff --git a/images/virtualization-artifact/pkg/builder/vi/vi.go b/images/virtualization-artifact/pkg/builder/vi/vi.go new file mode 100644 index 0000000000..e78f2179bb --- /dev/null +++ b/images/virtualization-artifact/pkg/builder/vi/vi.go @@ -0,0 +1,48 @@ +/* +Copyright 2025 Flant JSC +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package vi + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +func New(options ...Option) *v1alpha2.VirtualImage { + vi := NewEmpty("", "") + ApplyOptions(vi, options) + return vi +} + +func ApplyOptions(vi *v1alpha2.VirtualImage, opts []Option) { + if vi == nil { + return + } + for _, opt := range opts { + opt(vi) + } +} + +func NewEmpty(name, namespace string) *v1alpha2.VirtualImage { + return &v1alpha2.VirtualImage{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha2.SchemeGroupVersion.String(), + Kind: v1alpha2.VirtualImageKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + } +} diff --git a/images/virtualization-artifact/pkg/builder/vm/option.go b/images/virtualization-artifact/pkg/builder/vm/option.go index f250e32f0e..c99bcc7847 100644 --- a/images/virtualization-artifact/pkg/builder/vm/option.go +++ b/images/virtualization-artifact/pkg/builder/vm/option.go @@ -49,3 +49,10 @@ func WithMemory(size resource.Quantity) Option { vm.Spec.Memory.Size = size } } + +func WithBlockDeviceRefs(bdRefs ...v1alpha2.BlockDeviceSpecRef) Option { + return func(vm *v1alpha2.VirtualMachine) { + vm.Spec.BlockDeviceRefs = []v1alpha2.BlockDeviceSpecRef{} + vm.Spec.BlockDeviceRefs = append(vm.Spec.BlockDeviceRefs, bdRefs...) + } +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go new file mode 100644 index 0000000000..dde76e114f --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go @@ -0,0 +1,135 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validators + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/deckhouse/virtualization-controller/pkg/common/object" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +const ( + viMainErrorMessage = "A non-CDROM VirtualImage cannot occupy the first position in block devices" + cviMainErrorMessage = "A non-CDROM ClusterVirtualImage cannot occupy the first position in block devices" + cannotCheckViMessage = "Unable to verify if the specified VirtualImage is a CDROM" + cannotCheckCviMessage = "Unable to verify if the specified ClusterVirtualImage is a CDROM" +) + +type FirstBlockDeviceValidator struct { + client client.Client +} + +func NewFirstDiskValidator(client client.Client) *FirstBlockDeviceValidator { + return &FirstBlockDeviceValidator{client: client} +} + +func (v *FirstBlockDeviceValidator) ValidateCreate(ctx context.Context, vm *v1alpha2.VirtualMachine) (admission.Warnings, error) { + return v.Validate(ctx, vm) +} + +func (v *FirstBlockDeviceValidator) ValidateUpdate(ctx context.Context, _, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) { + return v.Validate(ctx, newVM) +} + +func (v *FirstBlockDeviceValidator) Validate(ctx context.Context, vm *v1alpha2.VirtualMachine) (admission.Warnings, error) { + if len(vm.Spec.BlockDeviceRefs) == 0 { + return nil, nil + } + + switch vm.Spec.BlockDeviceRefs[0].Kind { + case v1alpha2.ImageDevice: + return nil, v.ValidateVI(ctx, vm.Spec.BlockDeviceRefs[0].Name, vm.GetNamespace()) + case v1alpha2.ClusterImageDevice: + return nil, v.ValidateCVI(ctx, vm.Spec.BlockDeviceRefs[0].Name) + } + + return nil, nil +} + +func (v *FirstBlockDeviceValidator) ValidateCVI(ctx context.Context, name string) error { + cvi, err := object.FetchObject(ctx, types.NamespacedName{Name: name}, v.client, &v1alpha2.ClusterVirtualImage{}) + if err != nil { + return err + } + if cvi == nil { + return fmt.Errorf( + "%s: %s: ClusterVirtualImage %s does not exist", + cviMainErrorMessage, + cannotCheckCviMessage, + name, + ) + } + + if !cvi.Status.CDROM { + if cvi.Status.Phase == v1alpha2.ImageReady { + return fmt.Errorf( + "%s: ClusterVirtualImage %s is not CDROM", + cviMainErrorMessage, + name, + ) + } else { + return fmt.Errorf( + "%s: %s: ClusterVirtualImage %s is not ready", + cviMainErrorMessage, + cannotCheckCviMessage, + name, + ) + } + } + + return nil +} + +func (v *FirstBlockDeviceValidator) ValidateVI(ctx context.Context, name, namespace string) error { + vi, err := object.FetchObject(ctx, types.NamespacedName{Name: name, Namespace: namespace}, v.client, &v1alpha2.VirtualImage{}) + if err != nil { + return err + } + if vi == nil { + return fmt.Errorf( + "%s: %s: VirtualImage %s does not exist", + viMainErrorMessage, + cannotCheckViMessage, + name, + ) + } + + if !vi.Status.CDROM { + if vi.Status.Phase == v1alpha2.ImageReady { + return fmt.Errorf( + "%s: VirtualImage %s is not CDROM", + viMainErrorMessage, + name, + ) + } else { + return fmt.Errorf( + "%s: %s: VirtualImage %s is not ready", + viMainErrorMessage, + cannotCheckViMessage, + name, + ) + } + } + + return nil +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go new file mode 100644 index 0000000000..9466fc1849 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go @@ -0,0 +1,187 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validators_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "errors" + + "sigs.k8s.io/controller-runtime/pkg/client" + + cvibuilder "github.com/deckhouse/virtualization-controller/pkg/builder/cvi" + vdbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vd" + vibuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vi" + vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" + "github.com/deckhouse/virtualization-controller/pkg/common/testutil" + "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/validators" + "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +var _ = DescribeTable("TestFirstBlockDeviceValidator", func(args firstBlockDeviceValidatorTestArgs) { + objs := []client.Object{args.VM} + objs = append(objs, args.Objects...) + fakeClient := setupEnvironment(objs...) + + validator := validators.NewFirstDiskValidator(fakeClient) + _, err := validator.Validate(testutil.ContextBackgroundWithNoOpLogger(), args.VM) + if args.ExpectedError == nil { + Expect(err).Should(BeNil()) + } else { + Expect(err.Error()).Should(Equal(args.ExpectedError.Error())) + } +}, + Entry("Has no block devices", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns"), + ExpectedError: nil, + }), + Entry("Has VD as first device", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.DiskDevice, + Name: "vd1", + }), + Objects: []client.Object{ + generateVD("vd1", "ns"), + }, + ExpectedError: nil, + }), + Entry("Has VI CDROM as first device", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.ImageDevice, + Name: "vi1", + }), + Objects: []client.Object{ + generateVI("vi1", "ns", v1alpha2.ImageReady, true), + }, + ExpectedError: nil, + }), + Entry("Has not CDROM VI as first device", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.ImageDevice, + Name: "vi1", + }), + Objects: []client.Object{ + generateVI("vi1", "ns", v1alpha2.ImageReady, false), + }, + ExpectedError: errors.New("A non-CDROM VirtualImage cannot occupy the first position in block devices: VirtualImage vi1 is not CDROM"), + }), + Entry("Has not ready VI as first device", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.ImageDevice, + Name: "vi1", + }), + Objects: []client.Object{ + generateVI("vi1", "ns", v1alpha2.ImagePending, false), + }, + ExpectedError: errors.New("A non-CDROM VirtualImage cannot occupy the first position in block devices: Unable to verify if the specified VirtualImage is a CDROM: VirtualImage vi1 is not ready"), + }), + Entry("Has not exists vi as first device", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.ImageDevice, + Name: "vi1", + }), + ExpectedError: errors.New("A non-CDROM VirtualImage cannot occupy the first position in block devices: Unable to verify if the specified VirtualImage is a CDROM: VirtualImage vi1 does not exist"), + }), + Entry("Has CRDOM CVI as first device", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.ClusterImageDevice, + Name: "cvi1", + }), + Objects: []client.Object{ + generateCVI("cvi1", v1alpha2.ImageReady, true), + }, + ExpectedError: nil, + }), + Entry("Has not CDROM CVI as first device", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.ClusterImageDevice, + Name: "cvi1", + }), + Objects: []client.Object{ + generateCVI("cvi1", v1alpha2.ImageReady, false), + }, + ExpectedError: errors.New("A non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: ClusterVirtualImage cvi1 is not CDROM"), + }), + Entry("Has not ready CVI as first device", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.ClusterImageDevice, + Name: "cvi1", + }), + Objects: []client.Object{ + generateCVI("cvi1", v1alpha2.ImagePending, false), + }, + ExpectedError: errors.New("A non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: Unable to verify if the specified ClusterVirtualImage is a CDROM: ClusterVirtualImage cvi1 is not ready"), + }), + Entry("Has not exists CVI as first device", firstBlockDeviceValidatorTestArgs{ + VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + Kind: v1alpha2.ClusterImageDevice, + Name: "cvi1", + }), + ExpectedError: errors.New("A non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: Unable to verify if the specified ClusterVirtualImage is a CDROM: ClusterVirtualImage cvi1 does not exist"), + }), +) + +func setupEnvironment(objs ...client.Object) client.Client { + GinkgoHelper() + + var allObjects []client.Object = []client.Object{} + allObjects = append(allObjects, objs...) + + fakeClient, err := testutil.NewFakeClientWithObjects(allObjects...) + Expect(err).NotTo(HaveOccurred()) + + return fakeClient +} + +type firstBlockDeviceValidatorTestArgs struct { + VM *v1alpha2.VirtualMachine + Objects []client.Object + ExpectedError error +} + +func generateVM(name, namespace string, bdRefs ...v1alpha2.BlockDeviceSpecRef) *v1alpha2.VirtualMachine { + return vmbuilder.New( + vmbuilder.WithName(name), + vmbuilder.WithNamespace(namespace), + vmbuilder.WithBlockDeviceRefs(bdRefs...), + ) +} + +func generateVI(name, namespace string, phase v1alpha2.ImagePhase, isCDROM bool) *v1alpha2.VirtualImage { + return vibuilder.New( + vibuilder.WithName(name), + vibuilder.WithNamespace(namespace), + vibuilder.WithPhase(phase), + vibuilder.WithCDROM(isCDROM), + ) +} + +func generateCVI(name string, phase v1alpha2.ImagePhase, isCDROM bool) *v1alpha2.ClusterVirtualImage { + return cvibuilder.New( + cvibuilder.WithName(name), + cvibuilder.WithPhase(phase), + cvibuilder.WithCDROM(isCDROM), + ) +} + +func generateVD(name, namespace string) *v1alpha2.VirtualDisk { + return vdbuilder.New( + vdbuilder.WithName(name), + vdbuilder.WithNamespace(namespace), + ) +} diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go b/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go index e7bbf4bf6b..2fba1720bc 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_webhook.go @@ -53,6 +53,7 @@ func NewValidator(ipam internal.IPAM, client client.Client, service *service.Blo validators.NewTopologySpreadConstraintValidator(), validators.NewCPUCountValidator(), validators.NewNetworksValidator(), + validators.NewFirstDiskValidator(client), }, log: log.With("webhook", "validation"), } From ade91c8ff878d63978772cf143a8d21ffe375ee0 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Tue, 19 Aug 2025 12:17:44 +0300 Subject: [PATCH 2/4] fix linter Signed-off-by: Valeriy Khorunzhin --- .../first_block_device_validator.go | 8 +++---- .../first_block_device_validator_test.go | 23 +++++++++---------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go index dde76e114f..f50e8ae9fe 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go @@ -29,10 +29,10 @@ import ( ) const ( - viMainErrorMessage = "A non-CDROM VirtualImage cannot occupy the first position in block devices" - cviMainErrorMessage = "A non-CDROM ClusterVirtualImage cannot occupy the first position in block devices" - cannotCheckViMessage = "Unable to verify if the specified VirtualImage is a CDROM" - cannotCheckCviMessage = "Unable to verify if the specified ClusterVirtualImage is a CDROM" + viMainErrorMessage = "a non-CDROM VirtualImage cannot occupy the first position in block devices" + cviMainErrorMessage = "a non-CDROM ClusterVirtualImage cannot occupy the first position in block devices" + cannotCheckViMessage = "unable to verify if the specified VirtualImage is a CDROM" + cannotCheckCviMessage = "unable to verify if the specified ClusterVirtualImage is a CDROM" ) type FirstBlockDeviceValidator struct { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go index 9466fc1849..30cbef547a 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go @@ -17,11 +17,10 @@ limitations under the License. package validators_test import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "errors" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "sigs.k8s.io/controller-runtime/pkg/client" cvibuilder "github.com/deckhouse/virtualization-controller/pkg/builder/cvi" @@ -78,7 +77,7 @@ var _ = DescribeTable("TestFirstBlockDeviceValidator", func(args firstBlockDevic Objects: []client.Object{ generateVI("vi1", "ns", v1alpha2.ImageReady, false), }, - ExpectedError: errors.New("A non-CDROM VirtualImage cannot occupy the first position in block devices: VirtualImage vi1 is not CDROM"), + ExpectedError: errors.New("a non-CDROM VirtualImage cannot occupy the first position in block devices: VirtualImage vi1 is not CDROM"), }), Entry("Has not ready VI as first device", firstBlockDeviceValidatorTestArgs{ VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ @@ -88,14 +87,14 @@ var _ = DescribeTable("TestFirstBlockDeviceValidator", func(args firstBlockDevic Objects: []client.Object{ generateVI("vi1", "ns", v1alpha2.ImagePending, false), }, - ExpectedError: errors.New("A non-CDROM VirtualImage cannot occupy the first position in block devices: Unable to verify if the specified VirtualImage is a CDROM: VirtualImage vi1 is not ready"), + ExpectedError: errors.New("a non-CDROM VirtualImage cannot occupy the first position in block devices: unable to verify if the specified VirtualImage is a CDROM: VirtualImage vi1 is not ready"), }), Entry("Has not exists vi as first device", firstBlockDeviceValidatorTestArgs{ VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ Kind: v1alpha2.ImageDevice, Name: "vi1", }), - ExpectedError: errors.New("A non-CDROM VirtualImage cannot occupy the first position in block devices: Unable to verify if the specified VirtualImage is a CDROM: VirtualImage vi1 does not exist"), + ExpectedError: errors.New("a non-CDROM VirtualImage cannot occupy the first position in block devices: unable to verify if the specified VirtualImage is a CDROM: VirtualImage vi1 does not exist"), }), Entry("Has CRDOM CVI as first device", firstBlockDeviceValidatorTestArgs{ VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ @@ -115,31 +114,31 @@ var _ = DescribeTable("TestFirstBlockDeviceValidator", func(args firstBlockDevic Objects: []client.Object{ generateCVI("cvi1", v1alpha2.ImageReady, false), }, - ExpectedError: errors.New("A non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: ClusterVirtualImage cvi1 is not CDROM"), + ExpectedError: errors.New("a non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: ClusterVirtualImage cvi1 is not CDROM"), }), Entry("Has not ready CVI as first device", firstBlockDeviceValidatorTestArgs{ - VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + VM: generateVM("vm1", "ns1", v1alpha2.BlockDeviceSpecRef{ Kind: v1alpha2.ClusterImageDevice, Name: "cvi1", }), Objects: []client.Object{ generateCVI("cvi1", v1alpha2.ImagePending, false), }, - ExpectedError: errors.New("A non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: Unable to verify if the specified ClusterVirtualImage is a CDROM: ClusterVirtualImage cvi1 is not ready"), + ExpectedError: errors.New("a non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: unable to verify if the specified ClusterVirtualImage is a CDROM: ClusterVirtualImage cvi1 is not ready"), }), Entry("Has not exists CVI as first device", firstBlockDeviceValidatorTestArgs{ - VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ + VM: generateVM("vm", "ns", v1alpha2.BlockDeviceSpecRef{ Kind: v1alpha2.ClusterImageDevice, Name: "cvi1", }), - ExpectedError: errors.New("A non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: Unable to verify if the specified ClusterVirtualImage is a CDROM: ClusterVirtualImage cvi1 does not exist"), + ExpectedError: errors.New("a non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: unable to verify if the specified ClusterVirtualImage is a CDROM: ClusterVirtualImage cvi1 does not exist"), }), ) func setupEnvironment(objs ...client.Object) client.Client { GinkgoHelper() - var allObjects []client.Object = []client.Object{} + var allObjects []client.Object allObjects = append(allObjects, objs...) fakeClient, err := testutil.NewFakeClientWithObjects(allObjects...) From bcfee9ddfacdca77b3d9dd6e68be41a7fc8ee961 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Tue, 19 Aug 2025 17:03:19 +0300 Subject: [PATCH 3/4] change e2e for this PR Signed-off-by: Valeriy Khorunzhin --- tests/e2e/testdata/vm-versions/kustomization.yaml | 2 +- .../testdata/vm-versions/{vi => vd}/kustomization.yaml | 2 +- .../{vi/vi-alpine-http.yaml => vd/vd-root.yaml} | 7 ++++--- tests/e2e/testdata/vm-versions/vm/vm.yaml | 4 ++-- tests/e2e/vm_version_test.go | 8 ++++---- 5 files changed, 12 insertions(+), 11 deletions(-) rename tests/e2e/testdata/vm-versions/{vi => vd}/kustomization.yaml (75%) rename tests/e2e/testdata/vm-versions/{vi/vi-alpine-http.yaml => vd/vd-root.yaml} (73%) diff --git a/tests/e2e/testdata/vm-versions/kustomization.yaml b/tests/e2e/testdata/vm-versions/kustomization.yaml index 7938421590..fa6e4b5a8b 100644 --- a/tests/e2e/testdata/vm-versions/kustomization.yaml +++ b/tests/e2e/testdata/vm-versions/kustomization.yaml @@ -5,7 +5,7 @@ namePrefix: pr-number-or-commit-hash- resources: - ns.yaml - vm - - vi + - vd configurations: - transformer.yaml labels: diff --git a/tests/e2e/testdata/vm-versions/vi/kustomization.yaml b/tests/e2e/testdata/vm-versions/vd/kustomization.yaml similarity index 75% rename from tests/e2e/testdata/vm-versions/vi/kustomization.yaml rename to tests/e2e/testdata/vm-versions/vd/kustomization.yaml index b807d8e2d2..0e2842b120 100644 --- a/tests/e2e/testdata/vm-versions/vi/kustomization.yaml +++ b/tests/e2e/testdata/vm-versions/vd/kustomization.yaml @@ -1,4 +1,4 @@ apiVersion: kustomize.config.k8s.io/v1beta1 kind: Kustomization resources: - - vi-alpine-http.yaml + - vd-root.yaml diff --git a/tests/e2e/testdata/vm-versions/vi/vi-alpine-http.yaml b/tests/e2e/testdata/vm-versions/vd/vd-root.yaml similarity index 73% rename from tests/e2e/testdata/vm-versions/vi/vi-alpine-http.yaml rename to tests/e2e/testdata/vm-versions/vd/vd-root.yaml index abac06b48c..7b782f506c 100644 --- a/tests/e2e/testdata/vm-versions/vi/vi-alpine-http.yaml +++ b/tests/e2e/testdata/vm-versions/vd/vd-root.yaml @@ -1,10 +1,11 @@ --- apiVersion: virtualization.deckhouse.io/v1alpha2 -kind: VirtualImage +kind: VirtualDisk metadata: - name: vi-alpine-http + name: vd-root spec: - storage: ContainerRegistry + persistentVolumeClaim: + size: 512Mi dataSource: type: HTTP http: diff --git a/tests/e2e/testdata/vm-versions/vm/vm.yaml b/tests/e2e/testdata/vm-versions/vm/vm.yaml index a62cdcfa21..6e923ed906 100644 --- a/tests/e2e/testdata/vm-versions/vm/vm.yaml +++ b/tests/e2e/testdata/vm-versions/vm/vm.yaml @@ -13,5 +13,5 @@ spec: disruptions: restartApprovalMode: Manual blockDeviceRefs: - - kind: VirtualImage - name: vi-alpine-http + - kind: VirtualDisk + name: vd-root diff --git a/tests/e2e/vm_version_test.go b/tests/e2e/vm_version_test.go index f3868e76a2..fffa96bf76 100644 --- a/tests/e2e/vm_version_test.go +++ b/tests/e2e/vm_version_test.go @@ -63,10 +63,10 @@ var _ = Describe("VirtualMachineVersions", ginkgoutil.CommonE2ETestDecorators(), }) }) - Context("When virtual images are applied:", func() { - It("checks VIs phases", func() { - By(fmt.Sprintf("VIs should be in %s phase", PhaseReady)) - WaitPhaseByLabel(kc.ResourceVI, PhaseReady, kc.WaitOptions{ + Context("When virtual disks are applied:", func() { + It("checks VDs phases", func() { + By(fmt.Sprintf("VDs should be in %s phase", PhaseReady)) + WaitPhaseByLabel(kc.ResourceVD, PhaseReady, kc.WaitOptions{ Labels: testCaseLabel, Namespace: ns, Timeout: MaxWaitTimeout, From 6168e09d517cb2c0aa00773573727fbd5581fc51 Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Fri, 22 Aug 2025 16:13:46 +0300 Subject: [PATCH 4/4] update logic Signed-off-by: Valeriy Khorunzhin --- .../first_block_device_validator.go | 61 ++++++------------- .../first_block_device_validator_test.go | 8 +-- 2 files changed, 22 insertions(+), 47 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go index f50e8ae9fe..3407d4f154 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator.go @@ -19,6 +19,7 @@ package validators import ( "context" "fmt" + "reflect" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -29,10 +30,8 @@ import ( ) const ( - viMainErrorMessage = "a non-CDROM VirtualImage cannot occupy the first position in block devices" - cviMainErrorMessage = "a non-CDROM ClusterVirtualImage cannot occupy the first position in block devices" - cannotCheckViMessage = "unable to verify if the specified VirtualImage is a CDROM" - cannotCheckCviMessage = "unable to verify if the specified ClusterVirtualImage is a CDROM" + viMainErrorMessage = "a non-CDROM VirtualImage cannot occupy the first position in block devices" + cviMainErrorMessage = "a non-CDROM ClusterVirtualImage cannot occupy the first position in block devices" ) type FirstBlockDeviceValidator struct { @@ -47,7 +46,11 @@ func (v *FirstBlockDeviceValidator) ValidateCreate(ctx context.Context, vm *v1al return v.Validate(ctx, vm) } -func (v *FirstBlockDeviceValidator) ValidateUpdate(ctx context.Context, _, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) { +func (v *FirstBlockDeviceValidator) ValidateUpdate(ctx context.Context, oldVM, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) { + if reflect.DeepEqual(oldVM.Spec.BlockDeviceRefs, newVM.Spec.BlockDeviceRefs) { + return nil, nil + } + return v.Validate(ctx, newVM) } @@ -72,31 +75,17 @@ func (v *FirstBlockDeviceValidator) ValidateCVI(ctx context.Context, name string return err } if cvi == nil { + return nil + } + + if !cvi.Status.CDROM && cvi.Status.Phase == v1alpha2.ImageReady { return fmt.Errorf( - "%s: %s: ClusterVirtualImage %s does not exist", + "%s: ClusterVirtualImage %s is not CDROM", cviMainErrorMessage, - cannotCheckCviMessage, name, ) } - if !cvi.Status.CDROM { - if cvi.Status.Phase == v1alpha2.ImageReady { - return fmt.Errorf( - "%s: ClusterVirtualImage %s is not CDROM", - cviMainErrorMessage, - name, - ) - } else { - return fmt.Errorf( - "%s: %s: ClusterVirtualImage %s is not ready", - cviMainErrorMessage, - cannotCheckCviMessage, - name, - ) - } - } - return nil } @@ -106,30 +95,16 @@ func (v *FirstBlockDeviceValidator) ValidateVI(ctx context.Context, name, namesp return err } if vi == nil { + return nil + } + + if !vi.Status.CDROM && vi.Status.Phase == v1alpha2.ImageReady { return fmt.Errorf( - "%s: %s: VirtualImage %s does not exist", + "%s: VirtualImage %s is not CDROM", viMainErrorMessage, - cannotCheckViMessage, name, ) } - if !vi.Status.CDROM { - if vi.Status.Phase == v1alpha2.ImageReady { - return fmt.Errorf( - "%s: VirtualImage %s is not CDROM", - viMainErrorMessage, - name, - ) - } else { - return fmt.Errorf( - "%s: %s: VirtualImage %s is not ready", - viMainErrorMessage, - cannotCheckViMessage, - name, - ) - } - } - return nil } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go index 30cbef547a..4eee5f1747 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/validators/first_block_device_validator_test.go @@ -87,14 +87,14 @@ var _ = DescribeTable("TestFirstBlockDeviceValidator", func(args firstBlockDevic Objects: []client.Object{ generateVI("vi1", "ns", v1alpha2.ImagePending, false), }, - ExpectedError: errors.New("a non-CDROM VirtualImage cannot occupy the first position in block devices: unable to verify if the specified VirtualImage is a CDROM: VirtualImage vi1 is not ready"), + ExpectedError: nil, }), Entry("Has not exists vi as first device", firstBlockDeviceValidatorTestArgs{ VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ Kind: v1alpha2.ImageDevice, Name: "vi1", }), - ExpectedError: errors.New("a non-CDROM VirtualImage cannot occupy the first position in block devices: unable to verify if the specified VirtualImage is a CDROM: VirtualImage vi1 does not exist"), + ExpectedError: nil, }), Entry("Has CRDOM CVI as first device", firstBlockDeviceValidatorTestArgs{ VM: generateVM("vm1", "ns", v1alpha2.BlockDeviceSpecRef{ @@ -124,14 +124,14 @@ var _ = DescribeTable("TestFirstBlockDeviceValidator", func(args firstBlockDevic Objects: []client.Object{ generateCVI("cvi1", v1alpha2.ImagePending, false), }, - ExpectedError: errors.New("a non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: unable to verify if the specified ClusterVirtualImage is a CDROM: ClusterVirtualImage cvi1 is not ready"), + ExpectedError: nil, }), Entry("Has not exists CVI as first device", firstBlockDeviceValidatorTestArgs{ VM: generateVM("vm", "ns", v1alpha2.BlockDeviceSpecRef{ Kind: v1alpha2.ClusterImageDevice, Name: "cvi1", }), - ExpectedError: errors.New("a non-CDROM ClusterVirtualImage cannot occupy the first position in block devices: unable to verify if the specified ClusterVirtualImage is a CDROM: ClusterVirtualImage cvi1 does not exist"), + ExpectedError: nil, }), )