Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/components/versions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ firmware:
libvirt: v10.9.0
edk2: stable202411
core:
3p-kubevirt: v1.6.2-v12n.53
3p-kubevirt: feat/virt-controller/provisioning-volume-removal-no-restart
3p-containerized-data-importer: v1.60.3-v12n.21
distribution: 2.8.3
package:
Expand Down
2 changes: 2 additions & 0 deletions images/virt-artifact/werf.inc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down Expand Up @@ -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:
Expand Down
16 changes: 16 additions & 0 deletions images/virtualization-artifact/pkg/controller/kvbuilder/kvvm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,15 @@ func compareProvisioning(current, desired *v1alpha2.VirtualMachineSpec) []FieldC
ActionRestart,
)
if len(changes) > 0 {
// 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
}

Expand Down Expand Up @@ -228,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
}
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ provisioning:
),
},
{
"restart on provisioning remove",
"apply immediate on provisioning remove",
`
provisioning:
type: UserDataRef
Expand All @@ -438,7 +438,7 @@ provisioning:
"",
nil,
assertChanges(
actionRequired(ActionRestart),
actionRequired(ActionApplyImmediate),
requirePathOperation("provisioning", ChangeRemove),
),
},
Expand Down
151 changes: 151 additions & 0 deletions test/e2e/vm/provisioning_removal.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading