Skip to content

Commit af8fbd7

Browse files
committed
++ fix force flag calculations, add tests for VMOP
Signed-off-by: Ivan Mikheykin <ivan.mikheykin@flant.com>
1 parent 0c0fc98 commit af8fbd7

9 files changed

Lines changed: 199 additions & 32 deletions

File tree

images/virtualization-artifact/pkg/builder/vmop/option.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,9 @@ func WithForce() Option {
5353
vmop.Spec.Force = ptr.To(true)
5454
}
5555
}
56+
57+
func WithForceFalse() Option {
58+
return func(vmop *v1alpha2.VirtualMachineOperation) {
59+
vmop.Spec.Force = ptr.To(false)
60+
}
61+
}

images/virtualization-artifact/pkg/builder/vmop/vmop.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ import (
2424

2525
func New(options ...Option) *v1alpha2.VirtualMachineOperation {
2626
vmop := NewEmpty("", "")
27-
ApplyOptions(vmop, options)
27+
ApplyOptions(vmop, options...)
2828
return vmop
2929
}
3030

31-
func ApplyOptions(vmop *v1alpha2.VirtualMachineOperation, opts []Option) {
31+
func ApplyOptions(vmop *v1alpha2.VirtualMachineOperation, opts ...Option) {
3232
if vmop == nil {
3333
return
3434
}

images/virtualization-artifact/pkg/controller/livemigration/internal/dynamic_settings_handler.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2626

2727
"github.com/deckhouse/virtualization-controller/pkg/config"
28-
internalservice "github.com/deckhouse/virtualization-controller/pkg/controller/livemigration/internal/service"
28+
"github.com/deckhouse/virtualization-controller/pkg/livemigration"
2929
"github.com/deckhouse/virtualization-controller/pkg/logger"
3030
"github.com/deckhouse/virtualization/api/core/v1alpha2"
3131
)
@@ -67,14 +67,17 @@ func (h *DynamicSettingsHandler) Handle(ctx context.Context, kvvmi *virtv1.Virtu
6767
return reconcile.Result{}, err
6868
}
6969

70-
effectivePolicy, autoConverge := internalservice.CalculateEffectivePolicy(vm, vmopInProgress)
70+
effectivePolicy, autoConverge, err := livemigration.CalculateEffectivePolicy(vm, vmopInProgress)
71+
if err != nil {
72+
return reconcile.Result{}, err
73+
}
7174

72-
conf := internalservice.NewMigrationConfiguration(h.moduleSettings, autoConverge)
75+
conf := livemigration.NewMigrationConfiguration(h.moduleSettings, autoConverge)
7376

7477
kvvmi.Status.MigrationState.MigrationConfiguration = conf
7578

7679
log.Debug("Set migrationConfiguration on KVVMI",
77-
"migrationConfiguration", internalservice.DumpKVVMIMigrationConfiguration(kvvmi),
80+
"migrationConfiguration", livemigration.DumpKVVMIMigrationConfiguration(kvvmi),
7881
"policy", effectivePolicy,
7982
"autoConverge", autoConverge,
8083
)

images/virtualization-artifact/pkg/controller/livemigration/live_migration_reconciler.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ import (
2626
"sigs.k8s.io/controller-runtime/pkg/manager"
2727
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2828

29-
internalservice "github.com/deckhouse/virtualization-controller/pkg/controller/livemigration/internal/service"
3029
"github.com/deckhouse/virtualization-controller/pkg/controller/livemigration/internal/watcher"
3130
"github.com/deckhouse/virtualization-controller/pkg/controller/reconciler"
31+
"github.com/deckhouse/virtualization-controller/pkg/livemigration"
3232
"github.com/deckhouse/virtualization-controller/pkg/logger"
3333
)
3434

@@ -88,11 +88,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
8888
return h.Handle(ctx, kvvmi.Changed())
8989
})
9090
rec.SetResourceUpdater(func(ctx context.Context) error {
91-
if internalservice.IsMigrationConfigurationChanged(kvvmi.Current(), kvvmi.Changed()) {
91+
if livemigration.IsMigrationConfigurationChanged(kvvmi.Current(), kvvmi.Changed()) {
9292
// Directly update kvvmi and not use kvvmi.Update as kvvmi status is a regular field, not a subresource.
9393
log.Debug("About to update changed kvvmi",
94-
"changed.migration.configuration", internalservice.DumpKVVMIMigrationConfiguration(kvvmi.Changed()),
95-
"current.migration.configuration", internalservice.DumpKVVMIMigrationConfiguration(kvvmi.Current()),
94+
"changed.migration.configuration", livemigration.DumpKVVMIMigrationConfiguration(kvvmi.Changed()),
95+
"current.migration.configuration", livemigration.DumpKVVMIMigrationConfiguration(kvvmi.Current()),
9696
)
9797
if err := r.client.Update(ctx, kvvmi.Changed()); err != nil {
9898
return fmt.Errorf("error updating status subresource: %w", err)
@@ -101,7 +101,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
101101
}
102102

103103
log.Debug("Reconcile kvvmi without updating",
104-
"current.migration.configuration", internalservice.DumpKVVMIMigrationConfiguration(kvvmi.Current()),
104+
"current.migration.configuration", livemigration.DumpKVVMIMigrationConfiguration(kvvmi.Current()),
105105
)
106106
return nil
107107
})

images/virtualization-artifact/pkg/controller/vmop/internal/deletion_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ var _ = Describe("DeletionHandler", func() {
5959
newVmop := func(phase virtv2.VMOPPhase, opts ...vmopbuilder.Option) *virtv2.VirtualMachineOperation {
6060
vmop := vmopbuilder.NewEmpty(name, namespace)
6161
vmop.Status.Phase = phase
62-
vmopbuilder.ApplyOptions(vmop, opts)
62+
vmopbuilder.ApplyOptions(vmop, opts...)
6363
return vmop
6464
}
6565

images/virtualization-artifact/pkg/controller/vmop/internal/lifecycle.go

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
3434
"github.com/deckhouse/virtualization-controller/pkg/controller/vmop/internal/service"
3535
"github.com/deckhouse/virtualization-controller/pkg/eventrecord"
36+
"github.com/deckhouse/virtualization-controller/pkg/livemigration"
3637
"github.com/deckhouse/virtualization-controller/pkg/logger"
3738
virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
3839
"github.com/deckhouse/virtualization/api/core/v1alpha2/vmopcondition"
@@ -193,7 +194,7 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *virtv2.VirtualMachin
193194
return reconcile.Result{}, nil
194195
}
195196

196-
// Check if force flag is applicable for liveMigrationPolicy.
197+
// Check if force flag is applicable for effective liveMigrationPolicy.
197198
msg, isApplicable, err := h.isApplicableForLiveMigrationPolicy(ctx, vmop, vm)
198199
if err != nil {
199200
return reconcile.Result{}, err
@@ -208,6 +209,8 @@ func (h LifecycleHandler) Handle(ctx context.Context, vmop *virtv2.VirtualMachin
208209
Message(msg),
209210
&vmop.Status.Conditions)
210211
return reconcile.Result{}, nil
212+
} else if msg != "" {
213+
h.recorder.Event(vmop, corev1.EventTypeNormal, virtv2.ReasonVMOPStarted, msg)
211214
}
212215

213216
return reconcile.Result{}, nil
@@ -324,7 +327,7 @@ func (h LifecycleHandler) otherMigrationsAreInProgress(ctx context.Context, vmop
324327
}
325328

326329
func (h LifecycleHandler) isApplicableForLiveMigrationPolicy(ctx context.Context, vmop *virtv2.VirtualMachineOperation, vm *virtv2.VirtualMachine) (string, bool, error) {
327-
// No need to check if operation is not related to migrations.
330+
// No need to check live migration policy if operation is not related to migrations.
328331
if !commonvmop.IsMigration(vmop) {
329332
return "", true, nil
330333
}
@@ -334,21 +337,12 @@ func (h LifecycleHandler) isApplicableForLiveMigrationPolicy(ctx context.Context
334337
return "", true, nil
335338
}
336339

337-
// Get effective migrationPolicy from vm/vmclass and global default.
338-
effectiveLiveMigrationPolicy := virtv2.PreferSafeMigrationPolicy
339-
if vm.Spec.LiveMigrationPolicy != "" {
340-
effectiveLiveMigrationPolicy = vm.Spec.LiveMigrationPolicy
341-
}
342-
343-
isApplicable := true
344-
msg := ""
345-
346-
if effectiveLiveMigrationPolicy == virtv2.AlwaysSafeMigrationPolicy {
347-
isApplicable = false
348-
if *vmop.Spec.Force {
349-
msg = fmt.Sprintf("Operation type %s is not applicable for liveMigrationPolicy %s with the force flag on", vmop.Spec.Type, effectiveLiveMigrationPolicy)
350-
}
340+
effectivePolicy, autoConverge, err := livemigration.CalculateEffectivePolicy(*vm, vmop)
341+
if err != nil {
342+
msg := fmt.Sprintf("Operation is invalid: %v", err)
343+
return msg, false, nil
351344
}
352345

353-
return msg, isApplicable, nil
346+
msg := fmt.Sprintf("Migration settings for operation type %s: liveMigrationPolicy %s, autoConverge %v", vmop.Spec.Type, effectivePolicy, autoConverge)
347+
return msg, true, nil
354348
}
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
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+
"sigs.k8s.io/controller-runtime/pkg/client"
25+
26+
vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm"
27+
vmopbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vmop"
28+
"github.com/deckhouse/virtualization-controller/pkg/common/testutil"
29+
"github.com/deckhouse/virtualization-controller/pkg/controller/reconciler"
30+
"github.com/deckhouse/virtualization-controller/pkg/eventrecord"
31+
virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
32+
)
33+
34+
var _ = Describe("LifecycleHandler", func() {
35+
const (
36+
name = "test"
37+
namespace = "default"
38+
)
39+
40+
var (
41+
ctx context.Context
42+
fakeClient client.WithWatch
43+
srv *reconciler.Resource[*virtv2.VirtualMachineOperation, virtv2.VirtualMachineOperationStatus]
44+
recorderMock *eventrecord.EventRecorderLoggerMock
45+
)
46+
47+
BeforeEach(func() {
48+
ctx = testutil.ContextBackgroundWithNoOpLogger()
49+
recorderMock = &eventrecord.EventRecorderLoggerMock{
50+
EventFunc: func(_ client.Object, _, _, _ string) {},
51+
EventfFunc: func(_ client.Object, _, _, _ string, _ ...interface{}) {},
52+
}
53+
})
54+
55+
newVMOPEvictPending := func(opts ...vmopbuilder.Option) *virtv2.VirtualMachineOperation {
56+
vmop := vmopbuilder.NewEmpty(name, namespace)
57+
vmop.Status.Phase = virtv2.VMOPPhasePending
58+
vmopbuilder.ApplyOptions(vmop,
59+
vmopbuilder.WithType(virtv2.VMOPTypeEvict),
60+
vmopbuilder.WithVirtualMachine(name),
61+
)
62+
vmopbuilder.ApplyOptions(vmop, opts...)
63+
return vmop
64+
}
65+
66+
newVM := func(vmPolicy virtv2.LiveMigrationPolicy) *virtv2.VirtualMachine {
67+
vm := vmbuilder.NewEmpty(name, namespace)
68+
vm.Spec.LiveMigrationPolicy = vmPolicy
69+
vm.Spec.RunPolicy = virtv2.AlwaysOnPolicy
70+
vm.Status.Phase = virtv2.MachineRunning
71+
72+
return vm
73+
}
74+
75+
DescribeTable("Evict operation for migration policy", func(vmop *virtv2.VirtualMachineOperation, vmPolicy virtv2.LiveMigrationPolicy, expectedPhase virtv2.VMOPPhase) {
76+
vm := newVM(vmPolicy)
77+
78+
fakeClient, srv = setupEnvironment(vmop, vm)
79+
80+
h := NewLifecycleHandler(fakeClient, NewSvcOpCreator(fakeClient), recorderMock)
81+
_, err := h.Handle(ctx, srv.Changed())
82+
Expect(err).NotTo(HaveOccurred())
83+
84+
Expect(srv.Changed().Status.Phase).To(Equal(expectedPhase), "should updated status phase, conditions: %+v", srv.Changed().Status.Conditions)
85+
},
86+
// AlwaysSafe cases.
87+
Entry("is ok for AlwaysSafe and force=nil",
88+
newVMOPEvictPending(),
89+
virtv2.AlwaysSafeMigrationPolicy,
90+
virtv2.VMOPPhasePending,
91+
),
92+
Entry("is ok for AlwaysSafe and force=false",
93+
newVMOPEvictPending(vmopbuilder.WithForceFalse()),
94+
virtv2.AlwaysSafeMigrationPolicy,
95+
virtv2.VMOPPhasePending,
96+
),
97+
Entry("should become Failed for AlwaysSafe and force=true",
98+
newVMOPEvictPending(vmopbuilder.WithForce()),
99+
virtv2.AlwaysSafeMigrationPolicy,
100+
virtv2.VMOPPhaseFailed,
101+
),
102+
103+
// PreferSafe cases.
104+
Entry("is ok for PreferSafe and force=nil",
105+
newVMOPEvictPending(),
106+
virtv2.PreferSafeMigrationPolicy,
107+
virtv2.VMOPPhasePending,
108+
),
109+
Entry("is ok for PreferSafe and force=false",
110+
newVMOPEvictPending(vmopbuilder.WithForceFalse()),
111+
virtv2.PreferSafeMigrationPolicy,
112+
virtv2.VMOPPhasePending,
113+
),
114+
Entry("is ok for PreferSafe and force=true",
115+
newVMOPEvictPending(vmopbuilder.WithForce()),
116+
virtv2.PreferSafeMigrationPolicy,
117+
virtv2.VMOPPhasePending,
118+
),
119+
120+
// AlwaysForced cases.
121+
Entry("is ok for AlwaysForced and force=nil",
122+
newVMOPEvictPending(),
123+
virtv2.AlwaysForcedMigrationPolicy,
124+
virtv2.VMOPPhasePending,
125+
),
126+
Entry("should become Failed for AlwaysForced and force=false",
127+
newVMOPEvictPending(vmopbuilder.WithForceFalse()),
128+
virtv2.AlwaysForcedMigrationPolicy,
129+
virtv2.VMOPPhaseFailed,
130+
),
131+
Entry("is ok for AlwaysForced and force=true",
132+
newVMOPEvictPending(vmopbuilder.WithForce()),
133+
virtv2.AlwaysForcedMigrationPolicy,
134+
virtv2.VMOPPhasePending,
135+
),
136+
137+
// PreferForced cases.
138+
Entry("is ok for PreferForced and force=nil",
139+
newVMOPEvictPending(),
140+
virtv2.PreferForcedMigrationPolicy,
141+
virtv2.VMOPPhasePending,
142+
),
143+
Entry("is ok for PreferForced and force=false",
144+
newVMOPEvictPending(vmopbuilder.WithForceFalse()),
145+
virtv2.PreferForcedMigrationPolicy,
146+
virtv2.VMOPPhasePending,
147+
),
148+
Entry("is ok for PreferForced and force=true",
149+
newVMOPEvictPending(vmopbuilder.WithForce()),
150+
virtv2.PreferForcedMigrationPolicy,
151+
virtv2.VMOPPhasePending,
152+
),
153+
)
154+
})

images/virtualization-artifact/pkg/controller/livemigration/internal/service/migration_configuration.go renamed to images/virtualization-artifact/pkg/livemigration/migration_configuration.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package service
17+
package livemigration
1818

1919
import (
2020
"encoding/json"

images/virtualization-artifact/pkg/controller/livemigration/internal/service/policy.go renamed to images/virtualization-artifact/pkg/livemigration/policy.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package service
17+
package livemigration
1818

1919
import (
20+
"fmt"
21+
2022
"k8s.io/utils/ptr"
2123

2224
"github.com/deckhouse/virtualization-controller/pkg/config"
@@ -35,7 +37,7 @@ func AutoConvergeForPolicy(policy v1alpha2.LiveMigrationPolicy) (autoConverge *b
3537

3638
// CalculateEffectivePolicy merges live migration policy from default value and from VM.
3739
// Also, autoConverge value may be overridden from VMOP.
38-
func CalculateEffectivePolicy(vm v1alpha2.VirtualMachine, vmop *v1alpha2.VirtualMachineOperation) (effectivePolicy v1alpha2.LiveMigrationPolicy, autoConverge bool) {
40+
func CalculateEffectivePolicy(vm v1alpha2.VirtualMachine, vmop *v1alpha2.VirtualMachineOperation) (effectivePolicy v1alpha2.LiveMigrationPolicy, autoConverge bool, err error) {
3941
effectivePolicy = config.DefaultLiveMigrationPolicy
4042

4143
if vm.Spec.LiveMigrationPolicy != "" {
@@ -52,6 +54,14 @@ func CalculateEffectivePolicy(vm v1alpha2.VirtualMachine, vmop *v1alpha2.Virtual
5254
if vmop.Spec.Force != nil {
5355
autoConvergePtr = vmop.Spec.Force
5456
}
57+
case v1alpha2.AlwaysSafeMigrationPolicy:
58+
if vmop.Spec.Force != nil && *vmop.Spec.Force {
59+
return effectivePolicy, *autoConvergePtr, fmt.Errorf("force=true is not applicable for VM liveMigrationPolicy %s", effectivePolicy)
60+
}
61+
case v1alpha2.AlwaysForcedMigrationPolicy:
62+
if vmop.Spec.Force != nil && !*vmop.Spec.Force {
63+
return effectivePolicy, *autoConvergePtr, fmt.Errorf("force=false is not applicable for VM liveMigrationPolicy %s", effectivePolicy)
64+
}
5565
}
5666
}
5767

0 commit comments

Comments
 (0)