From b8b1d3c6ac6e4cb5c7d7e97f01a357ebd6f14491 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Thu, 2 Jul 2026 18:23:31 +0300 Subject: [PATCH 1/5] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/kvbuilder/kvvm.go | 16 ++++ .../pkg/controller/kvbuilder/kvvm_test.go | 79 +++++++++++++++++++ .../pkg/controller/vmchange/comparators.go | 8 ++ .../pkg/controller/vmchange/compare_test.go | 4 +- 4 files changed, 105 insertions(+), 2 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go index c03765de14..fea082c65b 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go @@ -651,6 +651,18 @@ func (b *KVVM) SetDisk(name string, opts SetDiskOptions) error { return nil } +// RemoveDisk deletes a disk and its volume from the KVVM spec by name. +func (b *KVVM) RemoveDisk(name string) { + b.Resource.Spec.Template.Spec.Domain.Devices.Disks = slices.DeleteFunc( + b.Resource.Spec.Template.Spec.Domain.Devices.Disks, + func(d virtv1.Disk) bool { return d.Name == name }, + ) + b.Resource.Spec.Template.Spec.Volumes = slices.DeleteFunc( + b.Resource.Spec.Template.Spec.Volumes, + func(v virtv1.Volume) bool { return v.Name == name }, + ) +} + func (b *KVVM) SetTablet(name string) { i := virtv1.Input{ Name: name, @@ -678,13 +690,17 @@ func (b *KVVM) HasTablet(name string) bool { func (b *KVVM) SetProvisioning(p *v1alpha2.Provisioning) error { if p == nil { + b.RemoveDisk(CloudInitDiskName) + b.RemoveDisk(SysprepDiskName) return nil } switch p.Type { case v1alpha2.ProvisioningTypeSysprepRef: + b.RemoveDisk(CloudInitDiskName) return b.SetDisk(SysprepDiskName, SetDiskOptions{Provisioning: p, IsCdrom: true}) case v1alpha2.ProvisioningTypeUserData, v1alpha2.ProvisioningTypeUserDataRef: + b.RemoveDisk(SysprepDiskName) return b.SetDisk(CloudInitDiskName, SetDiskOptions{Provisioning: p}) default: return fmt.Errorf("unexpected provisioning type %s. %w", p.Type, common.ErrUnknownType) diff --git a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go index 4c13924c6d..0f5d092cab 100644 --- a/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go +++ b/images/virtualization-artifact/pkg/controller/kvbuilder/kvvm_test.go @@ -253,6 +253,85 @@ func TestSetOsType(t *testing.T) { }) } +func TestSetProvisioning(t *testing.T) { + hasDisk := func(b *KVVM, name string) bool { + for _, d := range b.Resource.Spec.Template.Spec.Domain.Devices.Disks { + if d.Name == name { + return true + } + } + return false + } + hasVolume := func(b *KVVM, name string) bool { + for _, v := range b.Resource.Spec.Template.Spec.Volumes { + if v.Name == name { + return true + } + } + return false + } + + cloudInit := &v1alpha2.Provisioning{ + Type: v1alpha2.ProvisioningTypeUserData, + UserData: "#cloud-config", + } + sysprep := &v1alpha2.Provisioning{ + Type: v1alpha2.ProvisioningTypeSysprepRef, + SysprepRef: &v1alpha2.SysprepRef{Kind: v1alpha2.SysprepRefKindSecret, Name: "sysprep-secret"}, + } + + t.Run("removes cloudinit disk and volume when provisioning is removed", func(t *testing.T) { + b := newTestKVVM() + if err := b.SetProvisioning(cloudInit); err != nil { + t.Fatalf("SetProvisioning(cloudInit) failed: %v", err) + } + if !hasDisk(b, CloudInitDiskName) || !hasVolume(b, CloudInitDiskName) { + t.Fatal("cloudinit disk and volume should be present after setting provisioning") + } + + if err := b.SetProvisioning(nil); err != nil { + t.Fatalf("SetProvisioning(nil) failed: %v", err) + } + if hasDisk(b, CloudInitDiskName) || hasVolume(b, CloudInitDiskName) { + t.Error("cloudinit disk and volume should be removed after removing provisioning") + } + }) + + t.Run("removes sysprep disk and volume when provisioning is removed", func(t *testing.T) { + b := newTestKVVM() + if err := b.SetProvisioning(sysprep); err != nil { + t.Fatalf("SetProvisioning(sysprep) failed: %v", err) + } + if !hasDisk(b, SysprepDiskName) || !hasVolume(b, SysprepDiskName) { + t.Fatal("sysprep disk and volume should be present after setting provisioning") + } + + if err := b.SetProvisioning(nil); err != nil { + t.Fatalf("SetProvisioning(nil) failed: %v", err) + } + if hasDisk(b, SysprepDiskName) || hasVolume(b, SysprepDiskName) { + t.Error("sysprep disk and volume should be removed after removing provisioning") + } + }) + + t.Run("removes stale disk when provisioning type changes", func(t *testing.T) { + b := newTestKVVM() + if err := b.SetProvisioning(cloudInit); err != nil { + t.Fatalf("SetProvisioning(cloudInit) failed: %v", err) + } + if err := b.SetProvisioning(sysprep); err != nil { + t.Fatalf("SetProvisioning(sysprep) failed: %v", err) + } + + if hasDisk(b, CloudInitDiskName) || hasVolume(b, CloudInitDiskName) { + t.Error("cloudinit disk and volume should be removed after switching to sysprep") + } + if !hasDisk(b, SysprepDiskName) || !hasVolume(b, SysprepDiskName) { + t.Error("sysprep disk and volume should be present after switching to sysprep") + } + }) +} + func newTestKVVM() *KVVM { return NewEmptyKVVM(types.NamespacedName{Name: "test", Namespace: "default"}, KVVMOptions{ EnableParavirtualization: true, diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparators.go b/images/virtualization-artifact/pkg/controller/vmchange/comparators.go index 11b707dd2b..6259533e41 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparators.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparators.go @@ -130,6 +130,14 @@ func compareProvisioning(current, desired *v1alpha2.VirtualMachineSpec) []FieldC ActionRestart, ) if len(changes) > 0 { + // Provisioning data is consumed at boot only, so its removal can be + // applied to the underlying VM immediately: it allows deleting the + // referenced secret without restarting the VM. + for i := range changes { + if changes[i].Operation == ChangeRemove { + changes[i].ActionRequired = ActionApplyImmediate + } + } return changes } diff --git a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go index 8377ac10fa..e3e3f75eee 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/compare_test.go @@ -427,7 +427,7 @@ provisioning: ), }, { - "restart on provisioning remove", + "apply immediate on provisioning remove", ` provisioning: type: UserDataRef @@ -438,7 +438,7 @@ provisioning: "", nil, assertChanges( - actionRequired(ActionRestart), + actionRequired(ActionApplyImmediate), requirePathOperation("provisioning", ChangeRemove), ), }, From 704620744bf7953c3632ba7467783fdd8b335311 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Thu, 2 Jul 2026 18:27:37 +0300 Subject: [PATCH 2/5] wip Signed-off-by: Daniil Loktev --- build/components/versions.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/components/versions.yml b/build/components/versions.yml index 73b02b11e0..a75501e206 100644 --- a/build/components/versions.yml +++ b/build/components/versions.yml @@ -3,7 +3,7 @@ firmware: libvirt: v10.9.0 edk2: stable202411 core: - 3p-kubevirt: v1.6.2-v12n.50 + 3p-kubevirt: feat/virt-controller/provisioning-volume-removal-no-restart 3p-containerized-data-importer: v1.60.3-v12n.20 distribution: 2.8.3 package: From 480add562b564d15c7a75238dfd8d1bb94534fbf Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Thu, 2 Jul 2026 18:28:47 +0300 Subject: [PATCH 3/5] wip Signed-off-by: Daniil Loktev --- images/virt-artifact/werf.inc.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/images/virt-artifact/werf.inc.yaml b/images/virt-artifact/werf.inc.yaml index 0d75ebb629..8594685e95 100644 --- a/images/virt-artifact/werf.inc.yaml +++ b/images/virt-artifact/werf.inc.yaml @@ -9,6 +9,7 @@ image: {{ .ModuleNamePrefix }}{{ .ImageName }}-src-artifact final: false fromImage: builder/src +fromCacheVersion: "{{ now | date "Mon Jan 2 15:04:05 MST 2006" }}" secrets: - id: SOURCE_REPO value: {{ $.SOURCE_REPO }} @@ -44,6 +45,7 @@ packages: image: {{ .ModuleNamePrefix }}{{ .ImageName }} final: false fromImage: {{ eq $.SVACE_ENABLED "false" | ternary "builder/golang-alt-1.25" "builder/golang-alt-1.25" }} +fromCacheVersion: "{{ now | date "Mon Jan 2 15:04:05 MST 2006" }}" mount: {{- include "mount points for golang builds" . }} secrets: From 903262588823f30defa8248ab11254f41778aa6b Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 6 Jul 2026 11:29:46 +0300 Subject: [PATCH 4/5] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/vmchange/comparators.go | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmchange/comparators.go b/images/virtualization-artifact/pkg/controller/vmchange/comparators.go index 6259533e41..acc61d8caa 100644 --- a/images/virtualization-artifact/pkg/controller/vmchange/comparators.go +++ b/images/virtualization-artifact/pkg/controller/vmchange/comparators.go @@ -130,12 +130,13 @@ func compareProvisioning(current, desired *v1alpha2.VirtualMachineSpec) []FieldC ActionRestart, ) if len(changes) > 0 { - // Provisioning data is consumed at boot only, so its removal can be - // applied to the underlying VM immediately: it allows deleting the - // referenced secret without restarting the VM. - for i := range changes { - if changes[i].Operation == ChangeRemove { - changes[i].ActionRequired = ActionApplyImmediate + // Cloud-init data is consumed at boot only, so its removal can be + // applied to the underlying VM immediately + if isCloudInitProvisioning(current.Provisioning) { + for i := range changes { + if changes[i].Operation == ChangeRemove { + changes[i].ActionRequired = ActionApplyImmediate + } } } return changes @@ -236,3 +237,10 @@ func compareProvisioning(current, desired *v1alpha2.VirtualMachineSpec) []FieldC return nil } + +func isCloudInitProvisioning(p *v1alpha2.Provisioning) bool { + if p == nil { + return false + } + return p.Type == v1alpha2.ProvisioningTypeUserData || p.Type == v1alpha2.ProvisioningTypeUserDataRef +} From 170383c0112f34e3f8b1371bc009c93b79169657 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 6 Jul 2026 13:53:51 +0300 Subject: [PATCH 5/5] add e2e Signed-off-by: Daniil Loktev --- test/e2e/vm/provisioning_removal.go | 151 ++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 test/e2e/vm/provisioning_removal.go diff --git a/test/e2e/vm/provisioning_removal.go b/test/e2e/vm/provisioning_removal.go new file mode 100644 index 0000000000..81f77ab07d --- /dev/null +++ b/test/e2e/vm/provisioning_removal.go @@ -0,0 +1,151 @@ +/* +Copyright 2026 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 vm + +import ( + "context" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + virtv1 "kubevirt.io/api/core/v1" + crclient "sigs.k8s.io/controller-runtime/pkg/client" + + vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm" + "github.com/deckhouse/virtualization-controller/pkg/common/patch" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" + "github.com/deckhouse/virtualization/test/e2e/internal/framework" + "github.com/deckhouse/virtualization/test/e2e/internal/object" + "github.com/deckhouse/virtualization/test/e2e/internal/precheck" + "github.com/deckhouse/virtualization/test/e2e/internal/util" +) + +// cloudInitVolumeName is the name of the cloud-init volume and disk in the +// underlying KubeVirt VMI (see kvbuilder.CloudInitDiskName). +const cloudInitVolumeName = "cloudinit" + +var _ = Describe("VirtualMachineProvisioningRemoval", Label(precheck.NoPrecheck), func() { + It("should remove cloud-init provisioning from a running VM without a restart", func() { + ctx := context.Background() + f := framework.NewFramework("vm-provisioning-removal") + DeferCleanup(f.After) + f.Before() + + t := newProvisioningRemovalTest(f) + + By("Environment preparation") + t.GenerateResources() + err := f.CreateWithDeferredDeletion(ctx, t.VM, t.VDRoot) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting until VM agent is ready") + util.UntilVMAgentReady(ctx, crclient.ObjectKeyFromObject(t.VM), framework.LongTimeout) + + By("Checking the running VMI carries the cloud-init disk") + kvvmi, err := util.GetInternalVirtualMachineInstance(ctx, t.VM) + Expect(err).NotTo(HaveOccurred()) + Expect(kvvmi).NotTo(BeNil()) + Expect(vmiHasVolume(kvvmi, cloudInitVolumeName)).To(BeTrue(), "cloud-init volume should be present before removal") + Expect(vmiHasDisk(kvvmi, cloudInitVolumeName)).To(BeTrue(), "cloud-init disk should be present before removal") + vmiUID := kvvmi.UID + + initialNode, err := util.GetVMNode(ctx, f, t.VM) + Expect(err).NotTo(HaveOccurred()) + + By("Removing provisioning from the VM spec") + patchset := patch.NewJSONPatch(patch.WithRemove("/spec/provisioning")) + patchBytes, err := patchset.Bytes() + Expect(err).NotTo(HaveOccurred()) + t.VM, err = f.VirtClient().VirtualMachines(t.VM.Namespace).Patch(ctx, t.VM.Name, types.JSONPatchType, patchBytes, metav1.PatchOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting until the cloud-init disk is hot-detached from the running VMI") + Eventually(func(g Gomega) { + kvvmi, err := util.GetInternalVirtualMachineInstance(ctx, t.VM) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(kvvmi).NotTo(BeNil()) + g.Expect(vmiHasVolume(kvvmi, cloudInitVolumeName)).To(BeFalse(), "cloud-init volume should be detached") + g.Expect(vmiHasDisk(kvvmi, cloudInitVolumeName)).To(BeFalse(), "cloud-init disk should be detached") + g.Expect(kvvmi.UID).To(Equal(vmiUID), "VMI must not be recreated (no restart)") + }).WithTimeout(framework.MiddleTimeout).WithPolling(time.Second).Should(Succeed()) + + By("Checking that no restart is required and the VM was not restarted") + Consistently(func(g Gomega) { + err := f.GenericClient().Get(ctx, crclient.ObjectKeyFromObject(t.VM), t.VM) + g.Expect(err).NotTo(HaveOccurred()) + _, exists := conditions.GetCondition(vmcondition.TypeAwaitingRestartToApplyConfiguration, t.VM.Status.Conditions) + g.Expect(exists).To(BeFalse(), "removing cloud-init must not require a restart") + g.Expect(t.VM.Status.RestartAwaitingChanges).To(BeNil()) + + kvvmi, err := util.GetInternalVirtualMachineInstance(ctx, t.VM) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(kvvmi).NotTo(BeNil()) + g.Expect(kvvmi.UID).To(Equal(vmiUID), "VMI must not be recreated (no restart)") + }).WithTimeout(framework.ShortTimeout).WithPolling(time.Second).Should(Succeed()) + + util.ExpectNoVMOperationsForVirtualMachine(ctx, f, t.VM) + util.ExpectVMOnNode(ctx, f, t.VM, initialNode) + + By("Checking the VM is still reachable over SSH") + util.UntilSSHReady(f, t.VM, framework.ShortTimeout) + }) +}) + +type provisioningRemovalTest struct { + Framework *framework.Framework + + VM *v1alpha2.VirtualMachine + VDRoot *v1alpha2.VirtualDisk +} + +func newProvisioningRemovalTest(f *framework.Framework) *provisioningRemovalTest { + return &provisioningRemovalTest{Framework: f} +} + +func (t *provisioningRemovalTest) GenerateResources() { + t.VDRoot = object.NewVDFromCVI("vd-root", t.Framework.Namespace().Name, object.PrecreatedCVIAlpineBIOS) + + // NewMinimalVM sets cloud-init provisioning (UserData) by default. Manual + // restart approval makes a wrongly-required restart observable as the + // AwaitingRestartToApplyConfiguration condition instead of an auto-restart. + t.VM = object.NewMinimalVM("vm", t.Framework.Namespace().Name, + vmbuilder.WithDisks(t.VDRoot), + vmbuilder.WithRestartApprovalMode(v1alpha2.Manual), + ) +} + +func vmiHasVolume(vmi *virtv1.VirtualMachineInstance, name string) bool { + for _, v := range vmi.Spec.Volumes { + if v.Name == name { + return true + } + } + return false +} + +func vmiHasDisk(vmi *virtv1.VirtualMachineInstance, name string) bool { + for _, d := range vmi.Spec.Domain.Devices.Disks { + if d.Name == name { + return true + } + } + return false +}