Skip to content

Commit a9f428f

Browse files
LopatinDmitrLopatinDmitr
authored
fix(vm): add removing VM's conditions by design (#1023)
Signed-off-by: Dmitry Lopatin <dmitry.lopatin@flant.com>
1 parent cb45c22 commit a9f428f

17 files changed

Lines changed: 1297 additions & 264 deletions

File tree

‎api/core/v1alpha2/vmcondition/condition.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ const (
2929
TypeRunning Type = "Running"
3030
TypeMigrating Type = "Migrating"
3131
TypeMigratable Type = "Migratable"
32-
TypePodStarted Type = "PodStarted"
3332
TypeProvisioningReady Type = "ProvisioningReady"
3433
TypeAgentReady Type = "AgentReady"
3534
TypeAgentVersionNotSupported Type = "AgentVersionNotSupported"
@@ -81,10 +80,6 @@ const (
8180
ReasonRestartAwaitingVMClassChangesExist Reason = "RestartAwaitingVMClassChangesExist"
8281
ReasonRestartNoNeed Reason = "NoNeedRestart"
8382

84-
ReasonPodStarted Reason = "PodStarted"
85-
ReasonPodNotFound Reason = "PodNotFound"
86-
ReasonPodNotStarted Reason = "PodNotStarted"
87-
8883
ReasonMigratable Reason = "VirtualMachineMigratable"
8984
ReasonNotMigratable Reason = "VirtualMachineNotMigratable"
9085

@@ -94,14 +89,14 @@ const (
9489
ReasonVmIsNotRunning Reason = "VirtualMachineNotRunning"
9590
ReasonVmIsRunning Reason = "VirtualMachineRunning"
9691
ReasonInternalVirtualMachineError Reason = "InternalVirtualMachineError"
92+
ReasonPodNotStarted Reason = "PodNotStarted"
9793

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

10197
WaitingForTheSnapshotToStart Reason = "WaitingForTheSnapshotToStart"
10298
ReasonSnapshottingInProgress Reason = "SnapshottingInProgress"
10399

104-
ReasonSizingPolicyMatched Reason = "SizingPolicyMatched"
105100
ReasonSizingPolicyNotMatched Reason = "SizingPolicyNotMatched"
106101
ReasonVirtualMachineClassTerminating Reason = "VirtualMachineClassTerminating"
107102
ReasonVirtualMachineClassNotExists Reason = "VirtalMachineClassNotExists"

‎images/virtualization-artifact/pkg/controller/vm/internal/agent.go‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,6 @@ import (
3131

3232
const nameAgentHandler = "AgentHandler"
3333

34-
var agentConditions = []vmcondition.Type{
35-
vmcondition.TypeAgentReady,
36-
vmcondition.TypeAgentVersionNotSupported,
37-
}
38-
3934
func NewAgentHandler() *AgentHandler {
4035
return &AgentHandler{}
4136
}
@@ -49,10 +44,6 @@ func (h *AgentHandler) Handle(ctx context.Context, s state.VirtualMachineState)
4944
current := s.VirtualMachine().Current()
5045
changed := s.VirtualMachine().Changed()
5146

52-
if update := addAllUnknown(changed, agentConditions...); update {
53-
return reconcile.Result{Requeue: true}, nil
54-
}
55-
5647
if isDeletion(current) {
5748
return reconcile.Result{}, nil
5849
}
@@ -80,7 +71,14 @@ func (h *AgentHandler) syncAgentReady(vm *virtv2.VirtualMachine, kvvmi *virtv1.V
8071

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

83-
defer func() { conditions.SetCondition(cb, &vm.Status.Conditions) }()
74+
defer func() {
75+
phase := vm.Status.Phase
76+
if phase == virtv2.MachinePending || phase == virtv2.MachineStarting || phase == virtv2.MachineStopped {
77+
conditions.RemoveCondition(vmcondition.TypeAgentReady, &vm.Status.Conditions)
78+
} else {
79+
conditions.SetCondition(cb, &vm.Status.Conditions)
80+
}
81+
}()
8482

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

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

119-
defer func() { conditions.SetCondition(cb, &vm.Status.Conditions) }()
117+
defer func() {
118+
switch vm.Status.Phase {
119+
case virtv2.MachinePending, virtv2.MachineStarting, virtv2.MachineStopped:
120+
conditions.RemoveCondition(vmcondition.TypeAgentVersionNotSupported, &vm.Status.Conditions)
121+
122+
default:
123+
if cb.Condition().Status == metav1.ConditionTrue {
124+
conditions.SetCondition(cb, &vm.Status.Conditions)
125+
} else {
126+
conditions.RemoveCondition(vmcondition.TypeAgentVersionNotSupported, &vm.Status.Conditions)
127+
}
128+
}
129+
}()
120130

121131
if kvvmi == nil {
122132
cb.Status(metav1.ConditionFalse).

‎images/virtualization-artifact/pkg/controller/vm/internal/agent_test.go‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
/*
2+
Copyright 2025 Flant JSC
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package internal
18+
19+
import (
20+
"context"
21+
22+
. "github.com/onsi/ginkgo/v2"
23+
. "github.com/onsi/gomega"
24+
corev1 "k8s.io/api/core/v1"
25+
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
virtv1 "kubevirt.io/api/core/v1"
28+
"sigs.k8s.io/controller-runtime/pkg/client"
29+
30+
vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm"
31+
"github.com/deckhouse/virtualization-controller/pkg/common/testutil"
32+
"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
33+
"github.com/deckhouse/virtualization-controller/pkg/controller/reconciler"
34+
"github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state"
35+
virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
36+
"github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition"
37+
)
38+
39+
var _ = Describe("AgentHandler Tests", func() {
40+
const (
41+
name = "vm-agent"
42+
namespace = "default"
43+
)
44+
45+
var (
46+
ctx = testutil.ContextBackgroundWithNoOpLogger()
47+
fakeClient client.WithWatch
48+
resource *reconciler.Resource[*virtv2.VirtualMachine, virtv2.VirtualMachineStatus]
49+
vmState state.VirtualMachineState
50+
)
51+
52+
AfterEach(func() {
53+
fakeClient = nil
54+
resource = nil
55+
vmState = nil
56+
})
57+
58+
newVM := func(phase virtv2.MachinePhase) *virtv2.VirtualMachine {
59+
vm := vmbuilder.NewEmpty(name, namespace)
60+
vm.Status.Phase = phase
61+
return vm
62+
}
63+
64+
newKVVMI := func(agentConnected bool, agentUnsupported bool) *virtv1.VirtualMachineInstance {
65+
kvvmi := newEmptyKVVMI(name, namespace)
66+
conditions := make([]virtv1.VirtualMachineInstanceCondition, 0)
67+
if agentConnected {
68+
conditions = append(conditions, virtv1.VirtualMachineInstanceCondition{
69+
Type: virtv1.VirtualMachineInstanceAgentConnected,
70+
Status: corev1.ConditionTrue,
71+
})
72+
}
73+
if agentUnsupported {
74+
conditions = append(conditions, virtv1.VirtualMachineInstanceCondition{
75+
Type: virtv1.VirtualMachineInstanceUnsupportedAgent,
76+
Status: corev1.ConditionTrue,
77+
})
78+
} else {
79+
conditions = append(conditions, virtv1.VirtualMachineInstanceCondition{
80+
Type: virtv1.VirtualMachineInstanceUnsupportedAgent,
81+
Status: corev1.ConditionFalse,
82+
})
83+
}
84+
85+
kvvmi.Status.Conditions = conditions
86+
return kvvmi
87+
}
88+
89+
reconcile := func() {
90+
h := NewAgentHandler()
91+
_, err := h.Handle(ctx, vmState)
92+
Expect(err).NotTo(HaveOccurred())
93+
err = resource.Update(context.Background())
94+
Expect(err).NotTo(HaveOccurred())
95+
}
96+
97+
DescribeTable("AgentReady Condition Tests",
98+
func(phase virtv2.MachinePhase, agentConnected bool, expectedStatus metav1.ConditionStatus, expectedExistence bool) {
99+
vm := newVM(phase)
100+
kvvmi := newKVVMI(agentConnected, false)
101+
fakeClient, resource, vmState = setupEnvironment(vm, kvvmi)
102+
103+
reconcile()
104+
105+
newVM := &virtv2.VirtualMachine{}
106+
err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM)
107+
Expect(err).NotTo(HaveOccurred())
108+
109+
cond, exists := conditions.GetCondition(vmcondition.TypeAgentReady, newVM.Status.Conditions)
110+
Expect(exists).To(Equal(expectedExistence))
111+
if exists {
112+
Expect(cond.Status).To(Equal(expectedStatus))
113+
}
114+
},
115+
Entry("Should add AgentReady as True if agent is connected", virtv2.MachineRunning, true, metav1.ConditionTrue, true),
116+
Entry("Should add AgentReady as False if agent is not connected", virtv2.MachineRunning, false, metav1.ConditionFalse, true),
117+
118+
Entry("Should add AgentReady as True if agent is connected", virtv2.MachineStopping, true, metav1.ConditionTrue, true),
119+
Entry("Should add AgentReady as False if agent is not connected", virtv2.MachineStopping, false, metav1.ConditionFalse, true),
120+
121+
Entry("Should add AgentReady as True if agent is connected", virtv2.MachineMigrating, true, metav1.ConditionTrue, true),
122+
Entry("Should add AgentReady as False if agent is not connected", virtv2.MachineMigrating, false, metav1.ConditionFalse, true),
123+
124+
Entry("Should not add AgentReady if VM is in Pending phase and the agent is connected", virtv2.MachinePending, true, metav1.ConditionUnknown, false),
125+
Entry("Should not add AgentReady if VM is in Pending phase and the agent is not connected", virtv2.MachinePending, false, metav1.ConditionUnknown, false),
126+
127+
Entry("Should not add AgentReady if VM is in Starting phase and the agent is connected", virtv2.MachineStarting, true, metav1.ConditionUnknown, false),
128+
Entry("Should not add AgentReady if VM is in Starting phase and the agent is not connected", virtv2.MachineStarting, false, metav1.ConditionUnknown, false),
129+
130+
Entry("Should not add AgentReady if VM is in Stopped phase and the agent is connected", virtv2.MachineStopped, true, metav1.ConditionUnknown, false),
131+
Entry("Should not add AgentReady if VM is in Stopped phase and the agent is not connected", virtv2.MachineStopped, false, metav1.ConditionUnknown, false),
132+
)
133+
134+
DescribeTable("AgentVersionNotSupported Condition Tests",
135+
func(phase virtv2.MachinePhase, agentUnsupported bool, expectedStatus metav1.ConditionStatus, expectedExistence bool) {
136+
vm := newVM(phase)
137+
vmi := newKVVMI(true, agentUnsupported)
138+
fakeClient, resource, vmState = setupEnvironment(vm, vmi)
139+
140+
reconcile()
141+
142+
newVM := &virtv2.VirtualMachine{}
143+
err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM)
144+
Expect(err).NotTo(HaveOccurred())
145+
146+
cond, exists := conditions.GetCondition(vmcondition.TypeAgentVersionNotSupported, newVM.Status.Conditions)
147+
Expect(exists).To(Equal(expectedExistence))
148+
if exists {
149+
Expect(cond.Status).To(Equal(expectedStatus))
150+
}
151+
},
152+
Entry("Should set unsupported version condition as True in Running phase", virtv2.MachineRunning, true, metav1.ConditionTrue, true),
153+
Entry("Should not set unsupported version condition as False in Running phase", virtv2.MachineRunning, false, metav1.ConditionUnknown, false),
154+
155+
Entry("Should set unsupported version condition as True in Stopping phase", virtv2.MachineStopping, true, metav1.ConditionTrue, true),
156+
Entry("Should set unsupported version condition as False in Stopping phase", virtv2.MachineStopping, false, metav1.ConditionUnknown, false),
157+
158+
Entry("Should set unsupported version condition as True in Migrating phase", virtv2.MachineMigrating, true, metav1.ConditionTrue, true),
159+
Entry("Should set unsupported version condition as False in Migrating phase", virtv2.MachineMigrating, false, metav1.ConditionUnknown, false),
160+
161+
Entry("Should not set unsupported version condition as True in Pending phase", virtv2.MachinePending, true, metav1.ConditionUnknown, false),
162+
Entry("Should not set unsupported version condition as False in Pending phase", virtv2.MachinePending, false, metav1.ConditionUnknown, false),
163+
164+
Entry("Should not set unsupported version condition as True in Starting phase", virtv2.MachineStarting, true, metav1.ConditionUnknown, false),
165+
Entry("Should not set unsupported version condition as False in Starting phase", virtv2.MachineStarting, false, metav1.ConditionUnknown, false),
166+
167+
Entry("Should not set unsupported version condition as True in Stopped phase", virtv2.MachineStopped, true, metav1.ConditionUnknown, false),
168+
Entry("Should not set unsupported version condition as False in Stopped phase", virtv2.MachineStopped, false, metav1.ConditionUnknown, false),
169+
)
170+
})

‎images/virtualization-artifact/pkg/controller/vm/internal/firmware.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,24 @@ func (h *FirmwareHandler) syncFirmwareUpToDate(vm *virtv2.VirtualMachine, kvvmi
6868
upToDate := kvvmi == nil || kvvmi.Status.LauncherContainerImageVersion == "" || kvvmi.Status.LauncherContainerImageVersion == h.image
6969

7070
cb := conditions.NewConditionBuilder(vmcondition.TypeFirmwareUpToDate).Generation(vm.GetGeneration())
71-
if upToDate {
72-
cb.Status(metav1.ConditionTrue).
73-
Reason(vmcondition.ReasonFirmwareUpToDate).
74-
Message("")
71+
defer func() {
72+
switch vm.Status.Phase {
73+
case virtv2.MachinePending, virtv2.MachineStarting, virtv2.MachineStopped:
74+
conditions.RemoveCondition(vmcondition.TypeFirmwareUpToDate, &vm.Status.Conditions)
75+
76+
default:
77+
if cb.Condition().Status == metav1.ConditionFalse {
78+
conditions.SetCondition(cb, &vm.Status.Conditions)
79+
} else {
80+
conditions.RemoveCondition(vmcondition.TypeFirmwareUpToDate, &vm.Status.Conditions)
81+
}
82+
}
83+
}()
84+
85+
if !upToDate {
86+
cb.Status(metav1.ConditionFalse).
87+
Reason(vmcondition.ReasonFirmwareOutOfDate).
88+
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.")
7589
conditions.SetCondition(cb, &vm.Status.Conditions)
76-
return
7790
}
78-
79-
cb.Status(metav1.ConditionFalse).
80-
Reason(vmcondition.ReasonFirmwareOutOfDate).
81-
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.")
82-
conditions.SetCondition(cb, &vm.Status.Conditions)
8391
}

‎images/virtualization-artifact/pkg/controller/vm/internal/firmware_test.go‎

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ var _ = Describe("TestFirmwareHandler", func() {
7373
}
7474

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

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

8484
upToDate, exists := conditions.GetCondition(vmcondition.TypeFirmwareUpToDate, newVM.Status.Conditions)
85-
Expect(exists).To(BeTrue())
86-
Expect(upToDate.Status).To(Equal(expectedStatus))
87-
Expect(upToDate.Reason).To(Equal(expectedReason.String()))
85+
Expect(exists).To(Equal(expectedExistence))
86+
if exists {
87+
Expect(upToDate.Status).To(Equal(expectedStatus))
88+
Expect(upToDate.Reason).To(Equal(expectedReason.String()))
89+
}
8890
},
89-
Entry("Should be up to date", newVM(), newKVVMI(expectedImage), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate),
90-
Entry("Should be up to date because kvvmi is not exists", newVM(), nil, metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate),
91-
Entry("Should be out of date 1", newVM(), newKVVMI("other-image-1"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
92-
Entry("Should be out of date 2", newVM(), newKVVMI("other-image-2"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
93-
Entry("Should be out of date 3", newVM(), newKVVMI("other-image-3"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
94-
Entry("Should be out of date 4", newVM(), newKVVMI("other-image-4"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
95-
Entry("Should be out of date 5", newVM(), newKVVMI("other-image-5"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate),
91+
Entry("Should be up to date", newVM(), newKVVMI(expectedImage), metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false),
92+
Entry("Should be up to date because kvvmi is not exists", newVM(), nil, metav1.ConditionTrue, vmcondition.ReasonFirmwareUpToDate, false),
93+
Entry("Should be out of date 1", newVM(), newKVVMI("other-image-1"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
94+
Entry("Should be out of date 2", newVM(), newKVVMI("other-image-2"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
95+
Entry("Should be out of date 3", newVM(), newKVVMI("other-image-3"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
96+
Entry("Should be out of date 4", newVM(), newKVVMI("other-image-4"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
97+
Entry("Should be out of date 5", newVM(), newKVVMI("other-image-5"), metav1.ConditionFalse, vmcondition.ReasonFirmwareOutOfDate, true),
98+
)
99+
100+
DescribeTable("Condition TypeFirmwareUpToDate should be in the expected state considering the VM phase",
101+
func(vm *virtv2.VirtualMachine, phase virtv2.MachinePhase, kvvmi *virtv1.VirtualMachineInstance, expectedStatus metav1.ConditionStatus, expectedExistence bool) {
102+
vm.Status.Phase = phase
103+
fakeClient, resource, vmState = setupEnvironment(vm, kvvmi)
104+
reconcile()
105+
newVM := &virtv2.VirtualMachine{}
106+
err := fakeClient.Get(ctx, client.ObjectKeyFromObject(vm), newVM)
107+
Expect(err).NotTo(HaveOccurred())
108+
upToDate, exists := conditions.GetCondition(vmcondition.TypeFirmwareUpToDate, newVM.Status.Conditions)
109+
Expect(exists).To(Equal(expectedExistence))
110+
if exists {
111+
Expect(upToDate.Status).To(Equal(expectedStatus))
112+
}
113+
},
114+
Entry("Running phase, condition should not be set", newVM(), virtv2.MachineRunning, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
115+
Entry("Running phase, condition should be set", newVM(), virtv2.MachineRunning, newKVVMI("other-image-1"), metav1.ConditionFalse, true),
116+
117+
Entry("Migrating phase, condition should not be set", newVM(), virtv2.MachineMigrating, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
118+
Entry("Migrating phase, condition should be set", newVM(), virtv2.MachineMigrating, newKVVMI("other-image-1"), metav1.ConditionFalse, true),
119+
120+
Entry("Stopping phase, condition should not be set", newVM(), virtv2.MachineStopping, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
121+
Entry("Stopping phase, condition should be set", newVM(), virtv2.MachineStopping, newKVVMI("other-image-1"), metav1.ConditionFalse, true),
122+
123+
Entry("Pending phase, condition should not be set", newVM(), virtv2.MachinePending, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
124+
Entry("Pending phase, condition should not be set", newVM(), virtv2.MachinePending, newKVVMI("other-image-1"), metav1.ConditionUnknown, false),
125+
126+
Entry("Starting phase, condition should not be set", newVM(), virtv2.MachineStarting, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
127+
Entry("Starting phase, condition should not be set", newVM(), virtv2.MachineStarting, newKVVMI("other-image-1"), metav1.ConditionUnknown, false),
128+
129+
Entry("Stopped phase, condition should not be set", newVM(), virtv2.MachineStopped, newKVVMI(expectedImage), metav1.ConditionUnknown, false),
130+
Entry("Stopped phase, condition should not be set", newVM(), virtv2.MachineStopped, newKVVMI("other-image-1"), metav1.ConditionUnknown, false),
96131
)
97132
})

0 commit comments

Comments
 (0)