Skip to content

Commit 7d012b5

Browse files
authored
fix(vm): prevent a false "Restart Required" condition when restarting the VM (#1910)
Signed-off-by: Dmitry Lopatin <dmitry.lopatin@flant.com>
1 parent 1357d5f commit 7d012b5

File tree

3 files changed

+7
-18
lines changed

3 files changed

+7
-18
lines changed

images/virtualization-artifact/pkg/common/vm/vm.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,13 @@ import (
2222
"strings"
2323

2424
corev1 "k8s.io/api/core/v1"
25-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2625
"k8s.io/apimachinery/pkg/labels"
2726
"k8s.io/apimachinery/pkg/types"
2827
virtv1 "kubevirt.io/api/core/v1"
2928
"sigs.k8s.io/controller-runtime/pkg/client"
3029

3130
"github.com/deckhouse/virtualization-controller/pkg/common/object"
32-
"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
3331
"github.com/deckhouse/virtualization/api/core/v1alpha2"
34-
"github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition"
3532
)
3633

3734
// VMContainerNameSuffix - a name suffix for container with virt-launcher, libvirt and qemu processes.
@@ -78,15 +75,6 @@ func ApprovalMode(vm *v1alpha2.VirtualMachine) v1alpha2.RestartApprovalMode {
7875
return vm.Spec.Disruptions.RestartApprovalMode
7976
}
8077

81-
func RestartRequired(vm *v1alpha2.VirtualMachine) bool {
82-
if vm == nil {
83-
return false
84-
}
85-
86-
cond, _ := conditions.GetCondition(vmcondition.TypeAwaitingRestartToApplyConfiguration, vm.Status.Conditions)
87-
return cond.Status == metav1.ConditionTrue
88-
}
89-
9078
func IsComputeContainer(name string) bool {
9179
return strings.HasSuffix(name, VMContainerNameSuffix)
9280
}

images/virtualization-artifact/pkg/controller/vm/internal/service/migration_volumes.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import (
3636

3737
"github.com/deckhouse/virtualization-controller/pkg/common/patch"
3838
commonvd "github.com/deckhouse/virtualization-controller/pkg/common/vd"
39-
commonvm "github.com/deckhouse/virtualization-controller/pkg/common/vm"
4039
commonvmop "github.com/deckhouse/virtualization-controller/pkg/common/vmop"
4140
"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
4241
"github.com/deckhouse/virtualization-controller/pkg/controller/kvbuilder"
@@ -63,15 +62,15 @@ func NewMigrationVolumesService(client client.Client, makeKVVMFromSpec func(ctx
6362
}
6463
}
6564

66-
func (s MigrationVolumesService) SyncVolumes(ctx context.Context, vmState state.VirtualMachineState) (reconcile.Result, error) {
65+
func (s MigrationVolumesService) SyncVolumes(ctx context.Context, vmState state.VirtualMachineState, restartRequired bool) (reconcile.Result, error) {
6766
log := logger.FromContext(ctx).With("func", "SyncVolumes")
6867
log.Debug("Start")
6968
defer log.Debug("End")
7069

7170
vm := vmState.VirtualMachine().Changed()
7271

7372
// TODO: refactor syncKVVM and allow migration
74-
if commonvm.RestartRequired(vm) {
73+
if restartRequired {
7574
log.Info("Virtualmachine is restart required, skip volume migration.")
7675
return reconcile.Result{}, nil
7776
}

images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ import (
4949
const nameSyncKvvmHandler = "SyncKvvmHandler"
5050

5151
type syncVolumesService interface {
52-
SyncVolumes(ctx context.Context, s state.VirtualMachineState) (reconcile.Result, error)
52+
SyncVolumes(ctx context.Context, s state.VirtualMachineState, restartRequired bool) (reconcile.Result, error)
5353
}
5454

5555
func NewSyncKvvmHandler(dvcrSettings *dvcr.Settings, client client.Client, recorder eventrecord.EventRecorderLogger, syncVolumesService syncVolumesService) *SyncKvvmHandler {
@@ -226,8 +226,10 @@ func (h *SyncKvvmHandler) Handle(ctx context.Context, s state.VirtualMachineStat
226226

227227
// 5. Set RestartRequired from KVVM condition.
228228
if cbAwaitingRestart.Condition().Status == metav1.ConditionFalse && kvvm != nil {
229+
// The check for StateChangeRequests is added to ignore the RestartRequired condition when it is set while
230+
// the virtual machine is in the process of rebooting.
229231
cond, _ := conditions.GetKVVMCondition(virtv1.VirtualMachineRestartRequired, kvvm.Status.Conditions)
230-
if cond.Status == corev1.ConditionTrue {
232+
if cond.Status == corev1.ConditionTrue && len(kvvm.Status.StateChangeRequests) == 0 {
231233
msg := "Please restart the virtual machine to synchronize its configuration."
232234
log.Error(msg)
233235
cbAwaitingRestart.
@@ -238,7 +240,7 @@ func (h *SyncKvvmHandler) Handle(ctx context.Context, s state.VirtualMachineStat
238240
}
239241

240242
// 6. Sync migrating volumes if needed.
241-
result, migrateVolumesErr := h.syncVolumesService.SyncVolumes(ctx, s)
243+
result, migrateVolumesErr := h.syncVolumesService.SyncVolumes(ctx, s, cbAwaitingRestart.Condition().Status == metav1.ConditionTrue)
242244
if migrateVolumesErr != nil {
243245
errs = errors.Join(errs, fmt.Errorf("failed to sync migrating volumes: %w", migrateVolumesErr))
244246
}

0 commit comments

Comments
 (0)