Skip to content

Revert "fix(vm): add removing VM's conditions by design" #1022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
universal-itengineer merged 1 commit into from
May 7, 2025
Merged

Revert "fix(vm): add removing VM's conditions by design" #1022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion api/core/v1alpha2/vmcondition/condition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const (
TypeRunning Type = "Running"
TypeMigrating Type = "Migrating"
TypeMigratable Type = "Migratable"
TypePodStarted Type = "PodStarted"
TypeProvisioningReady Type = "ProvisioningReady"
TypeAgentReady Type = "AgentReady"
TypeAgentVersionNotSupported Type = "AgentVersionNotSupported"
Expand Down Expand Up @@ -80,6 +81,10 @@ const (
ReasonRestartAwaitingVMClassChangesExist Reason = "RestartAwaitingVMClassChangesExist"
ReasonRestartNoNeed Reason = "NoNeedRestart"

ReasonPodStarted Reason = "PodStarted"
ReasonPodNotFound Reason = "PodNotFound"
ReasonPodNotStarted Reason = "PodNotStarted"

ReasonMigratable Reason = "VirtualMachineMigratable"
ReasonNotMigratable Reason = "VirtualMachineNotMigratable"

Expand All @@ -89,14 +94,14 @@ const (
ReasonVmIsNotRunning Reason = "VirtualMachineNotRunning"
ReasonVmIsRunning Reason = "VirtualMachineRunning"
ReasonInternalVirtualMachineError Reason = "InternalVirtualMachineError"
ReasonPodNotStarted Reason = "PodNotStarted"

// ReasonFilesystemFrozen indicates that virtual machine's filesystem has been successfully frozen.
ReasonFilesystemFrozen Reason = "Frozen"

WaitingForTheSnapshotToStart Reason = "WaitingForTheSnapshotToStart"
ReasonSnapshottingInProgress Reason = "SnapshottingInProgress"

ReasonSizingPolicyMatched Reason = "SizingPolicyMatched"
ReasonSizingPolicyNotMatched Reason = "SizingPolicyNotMatched"
ReasonVirtualMachineClassTerminating Reason = "VirtualMachineClassTerminating"
ReasonVirtualMachineClassNotExists Reason = "VirtalMachineClassNotExists"
Expand Down
32 changes: 11 additions & 21 deletions images/virtualization-artifact/pkg/controller/vm/internal/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ import (

const nameAgentHandler = "AgentHandler"

var agentConditions = []vmcondition.Type{
vmcondition.TypeAgentReady,
vmcondition.TypeAgentVersionNotSupported,
}

func NewAgentHandler() *AgentHandler {
return &AgentHandler{}
}
Expand All @@ -44,6 +49,10 @@ func (h *AgentHandler) Handle(ctx context.Context, s state.VirtualMachineState)
current := s.VirtualMachine().Current()
changed := s.VirtualMachine().Changed()

if update := addAllUnknown(changed, agentConditions...); update {
return reconcile.Result{Requeue: true}, nil
}

if isDeletion(current) {
return reconcile.Result{}, nil
}
Expand Down Expand Up @@ -71,14 +80,7 @@ func (h *AgentHandler) syncAgentReady(vm *virtv2.VirtualMachine, kvvmi *virtv1.V

cb := conditions.NewConditionBuilder(vmcondition.TypeAgentReady).Generation(vm.GetGeneration())

defer func() {
phase := vm.Status.Phase
if phase == virtv2.MachinePending || phase == virtv2.MachineStarting || phase == virtv2.MachineStopped {
conditions.RemoveCondition(vmcondition.TypeAgentReady, &vm.Status.Conditions)
} else {
conditions.SetCondition(cb, &vm.Status.Conditions)
}
}()
defer func() { conditions.SetCondition(cb, &vm.Status.Conditions) }()

if kvvmi == nil {
cb.Status(metav1.ConditionFalse).
Expand Down Expand Up @@ -114,19 +116,7 @@ func (h *AgentHandler) syncAgentVersionNotSupport(vm *virtv2.VirtualMachine, kvv

cb := conditions.NewConditionBuilder(vmcondition.TypeAgentVersionNotSupported).Generation(vm.GetGeneration())

defer func() {
switch vm.Status.Phase {
case virtv2.MachinePending, virtv2.MachineStarting, virtv2.MachineStopped:
conditions.RemoveCondition(vmcondition.TypeAgentVersionNotSupported, &vm.Status.Conditions)

default:
if cb.Condition().Status == metav1.ConditionTrue {
conditions.SetCondition(cb, &vm.Status.Conditions)
} else {
conditions.RemoveCondition(vmcondition.TypeAgentVersionNotSupported, &vm.Status.Conditions)
}
}
}()
defer func() { conditions.SetCondition(cb, &vm.Status.Conditions) }()

if kvvmi == nil {
cb.Status(metav1.ConditionFalse).
Expand Down
170 changes: 0 additions & 170 deletions images/virtualization-artifact/pkg/controller/vm/internal/agent_test.go
View file
Open in desktop

This file was deleted.

28 changes: 10 additions & 18 deletions images/virtualization-artifact/pkg/controller/vm/internal/firmware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,16 @@ func (h *FirmwareHandler) syncFirmwareUpToDate(vm *virtv2.VirtualMachine, kvvmi
upToDate := kvvmi == nil || kvvmi.Status.LauncherContainerImageVersion == "" || kvvmi.Status.LauncherContainerImageVersion == h.image

cb := conditions.NewConditionBuilder(vmcondition.TypeFirmwareUpToDate).Generation(vm.GetGeneration())
defer func() {
switch vm.Status.Phase {
case virtv2.MachinePending, virtv2.MachineStarting, virtv2.MachineStopped:
conditions.RemoveCondition(vmcondition.TypeFirmwareUpToDate, &vm.Status.Conditions)

default:
if cb.Condition().Status == metav1.ConditionFalse {
conditions.SetCondition(cb, &vm.Status.Conditions)
} else {
conditions.RemoveCondition(vmcondition.TypeFirmwareUpToDate, &vm.Status.Conditions)
}
}
}()

if !upToDate {
cb.Status(metav1.ConditionFalse).
Reason(vmcondition.ReasonFirmwareOutOfDate).
Message("The VM firmware version is outdated and not recommended for use with the current version of the virtualization module, please migrate or reboot the VM to upgrade its firmware version.")
if upToDate {
cb.Status(metav1.ConditionTrue).
Reason(vmcondition.ReasonFirmwareUpToDate).
Message("")
conditions.SetCondition(cb, &vm.Status.Conditions)
return
}

cb.Status(metav1.ConditionFalse).
Reason(vmcondition.ReasonFirmwareOutOfDate).
Message("The VM firmware version is outdated and not recommended for use with the current version of the virtualization module, please migrate or reboot the VM to upgrade its firmware version.")
conditions.SetCondition(cb, &vm.Status.Conditions)
}
57 changes: 11 additions & 46 deletions images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ var _ = Describe("TestFirmwareHandler", func() {
}

DescribeTable("Condition TypeFirmwareUpToDate should be in expected state",
func(vm *virtv2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, expectedStatus metav1.ConditionStatus, expectedReason vmcondition.Reason, expectedExistence bool) {
func(vm *virtv2.VirtualMachine, kvvmi *virtv1.VirtualMachineInstance, expectedStatus metav1.ConditionStatus, expectedReason vmcondition.Reason) {
fakeClient, resource, vmState = setupEnvironment(vm, kvvmi)
reconcile()

Expand All @@ -82,51 +82,16 @@ var _ = Describe("TestFirmwareHandler", func() {
Expect(err).NotTo(HaveOccurred())

upToDate, exists := conditions.GetCondition(vmcondition.TypeFirmwareUpToDate, newVM.Status.Conditions)
Expect(exists).To(Equal(expectedExistence))
if exists {
Expect(upToDate.Status).To(Equal(expectedStatus))
Expect(upToDate.Reason).To(Equal(expectedReason.String()))
}
Expect(exists).To(BeTrue())
Expect(upToDate.Status).To(Equal(expectedStatus))
Expect(upToDate.Reason).To(Equal(expectedReason.String()))
},
Entry("Should be up to date", newVM(), newKVVMI(expectedImage), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false),
Entry("Should be up to date because kvvmi is not exists", newVM(), nil, metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false),
Entry("Should be out of date 1", newVM(), newKVVMI("other-image-1"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
Entry("Should be out of date 2", newVM(), newKVVMI("other-image-2"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
Entry("Should be out of date 3", newVM(), newKVVMI("other-image-3"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
Entry("Should be out of date 4", newVM(), newKVVMI("other-image-4"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
Entry("Should be out of date 5", newVM(), newKVVMI("other-image-5"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
)

DescribeTable("Condition TypeFirmwareUpToDate should be in the expected state considering the VM phase",
func(vm *virtv2.VirtualMachine, phase virtv2.MachinePhase, kvvmi *virtv1.VirtualMachineInstance, expectedStatus metav1.ConditionStatus, expectedExistence bool) {
vm.Status.Phase = phase
fakeClient, resource, vmState = setupEnvironment(vm, kvvmi)
reconcile()
newVM := &virtv2.VirtualMachine{}
err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM)
Expect(err).NotTo(HaveOccurred())
upToDate, exists := conditions.GetCondition(vmcondition.TypeFirmwareUpToDate, newVM.Status.Conditions)
Expect(exists).To(Equal(expectedExistence))
if exists {
Expect(upToDate.Status).To(Equal(expectedStatus))
}
},
Entry("Running phase, condition should not be set", newVM(), virtv2.MachineRunning, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
Entry("Running phase, condition should be set", newVM(), virtv2.MachineRunning, newKVVMI("other-image-1"), metav1.ConditionFalse, true),

Entry("Migrating phase, condition should not be set", newVM(), virtv2.MachineMigrating, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
Entry("Migrating phase, condition should be set", newVM(), virtv2.MachineMigrating, newKVVMI("other-image-1"), metav1.ConditionFalse, true),

Entry("Stopping phase, condition should not be set", newVM(), virtv2.MachineStopping, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
Entry("Stopping phase, condition should be set", newVM(), virtv2.MachineStopping, newKVVMI("other-image-1"), metav1.ConditionFalse, true),

Entry("Pending phase, condition should not be set", newVM(), virtv2.MachinePending, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
Entry("Pending phase, condition should not be set", newVM(), virtv2.MachinePending, newKVVMI("other-image-1"), metav1.ConditionUnknown, false),

Entry("Starting phase, condition should not be set", newVM(), virtv2.MachineStarting, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
Entry("Starting phase, condition should not be set", newVM(), virtv2.MachineStarting, newKVVMI("other-image-1"), metav1.ConditionUnknown, false),

Entry("Stopped phase, condition should not be set", newVM(), virtv2.MachineStopped, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
Entry("Stopped phase, condition should not be set", newVM(), virtv2.MachineStopped, newKVVMI("other-image-1"), metav1.ConditionUnknown, false),
Entry("Should be up to date", newVM(), newKVVMI(expectedImage), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate),
Entry("Should be up to date because kvvmi is not exists", newVM(), nil, metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate),
Entry("Should be out of date 1", newVM(), newKVVMI("other-image-1"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
Entry("Should be out of date 2", newVM(), newKVVMI("other-image-2"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
Entry("Should be out of date 3", newVM(), newKVVMI("other-image-3"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
Entry("Should be out of date 4", newVM(), newKVVMI("other-image-4"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
Entry("Should be out of date 5", newVM(), newKVVMI("other-image-5"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
)
})
Loading
Loading