Skip to content

Commit a983a33

Browse files
committed
++ suggestions from review
Signed-off-by: Ivan Mikheykin <ivan.mikheykin@flant.com>
1 parent af8fbd7 commit a983a33

File tree

6 files changed

+18
-29
lines changed

6 files changed

+18
-29
lines changed

images/virtualization-artifact/cmd/virtualization-controller/main.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ func main() {
352352
}
353353

354354
liveMigrationLogger := logger.NewControllerLogger(livemigration.ControllerName, logLevel, logOutput, logDebugVerbosity, logDebugControllerList)
355-
if err = livemigration.NewController(ctx, mgr, liveMigrationLogger, liveMigrationSettings); err != nil {
355+
if err = livemigration.SetupController(ctx, mgr, liveMigrationLogger, liveMigrationSettings); err != nil {
356356
log.Error(err.Error())
357357
os.Exit(1)
358358
}

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

+2-10
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ limitations under the License.
1717
package vmop
1818

1919
import (
20-
"k8s.io/utils/ptr"
21-
2220
"github.com/deckhouse/virtualization-controller/pkg/builder/meta"
2321
"github.com/deckhouse/virtualization/api/core/v1alpha2"
2422
)
@@ -48,14 +46,8 @@ func WithVirtualMachine(vm string) Option {
4846
}
4947
}
5048

51-
func WithForce() Option {
52-
return func(vmop *v1alpha2.VirtualMachineOperation) {
53-
vmop.Spec.Force = ptr.To(true)
54-
}
55-
}
56-
57-
func WithForceFalse() Option {
49+
func WithForce(force *bool) Option {
5850
return func(vmop *v1alpha2.VirtualMachineOperation) {
59-
vmop.Spec.Force = ptr.To(false)
51+
vmop.Spec.Force = force
6052
}
6153
}

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

+4-8
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package internal
1919
import (
2020
"context"
2121

22-
"k8s.io/apimachinery/pkg/types"
2322
virtv1 "kubevirt.io/api/core/v1"
2423
"sigs.k8s.io/controller-runtime/pkg/client"
2524
"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -52,17 +51,14 @@ func (h *DynamicSettingsHandler) Handle(ctx context.Context, kvvmi *virtv1.Virtu
5251
}
5352

5453
var vm v1alpha2.VirtualMachine
55-
vmKey := types.NamespacedName{
56-
Namespace: kvvmi.Namespace,
57-
Name: kvvmi.Name,
58-
}
54+
vmKey := client.ObjectKeyFromObject(kvvmi)
5955
err := h.client.Get(ctx, vmKey, &vm)
6056
if err != nil {
6157
return reconcile.Result{}, err
6258
}
6359

6460
// Fetch InProgress vmop
65-
vmopInProgress, err := h.GetVMOPInProgressForVM(ctx, vmKey)
61+
vmopInProgress, err := h.getVMOPInProgressForVM(ctx, vmKey)
6662
if err != nil {
6763
return reconcile.Result{}, err
6864
}
@@ -98,8 +94,8 @@ func (h *DynamicSettingsHandler) shouldUpdateMigrationConfiguration(kvvmi *virtv
9894
!kvvmi.Status.MigrationState.Completed
9995
}
10096

101-
// GetVMOPInProgressForVM check if there is at least one VMOP for the same VM in progress phase.
102-
func (h *DynamicSettingsHandler) GetVMOPInProgressForVM(ctx context.Context, vmKey client.ObjectKey) (*v1alpha2.VirtualMachineOperation, error) {
97+
// getVMOPInProgressForVM check if there is at least one VMOP for the same VM in progress phase.
98+
func (h *DynamicSettingsHandler) getVMOPInProgressForVM(ctx context.Context, vmKey client.ObjectKey) (*v1alpha2.VirtualMachineOperation, error) {
10399
var vmopList v1alpha2.VirtualMachineOperationList
104100
err := h.client.List(ctx, &vmopList, client.InNamespace(vmKey.Namespace))
105101
if err != nil {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import (
2929
"github.com/deckhouse/virtualization/api/core/v1alpha2"
3030
)
3131

32-
var _ = Describe("TestEvacuationHandler", func() {
32+
var _ = Describe("TestDynamicSettingsHandler", func() {
3333
const (
3434
vmName = "vm-migratable"
3535
vmNamespace = "default"

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const (
3434
ControllerName = "live-migration-controller"
3535
)
3636

37-
func NewController(
37+
func SetupController(
3838
ctx context.Context,
3939
mgr manager.Manager,
4040
log *log.Logger,

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

+9-8
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
. "github.com/onsi/ginkgo/v2"
2323
. "github.com/onsi/gomega"
24+
"k8s.io/utils/ptr"
2425
"sigs.k8s.io/controller-runtime/pkg/client"
2526

2627
vmbuilder "github.com/deckhouse/virtualization-controller/pkg/builder/vm"
@@ -90,12 +91,12 @@ var _ = Describe("LifecycleHandler", func() {
9091
virtv2.VMOPPhasePending,
9192
),
9293
Entry("is ok for AlwaysSafe and force=false",
93-
newVMOPEvictPending(vmopbuilder.WithForceFalse()),
94+
newVMOPEvictPending(vmopbuilder.WithForce(ptr.To(false))),
9495
virtv2.AlwaysSafeMigrationPolicy,
9596
virtv2.VMOPPhasePending,
9697
),
9798
Entry("should become Failed for AlwaysSafe and force=true",
98-
newVMOPEvictPending(vmopbuilder.WithForce()),
99+
newVMOPEvictPending(vmopbuilder.WithForce(ptr.To(true))),
99100
virtv2.AlwaysSafeMigrationPolicy,
100101
virtv2.VMOPPhaseFailed,
101102
),
@@ -107,12 +108,12 @@ var _ = Describe("LifecycleHandler", func() {
107108
virtv2.VMOPPhasePending,
108109
),
109110
Entry("is ok for PreferSafe and force=false",
110-
newVMOPEvictPending(vmopbuilder.WithForceFalse()),
111+
newVMOPEvictPending(vmopbuilder.WithForce(ptr.To(false))),
111112
virtv2.PreferSafeMigrationPolicy,
112113
virtv2.VMOPPhasePending,
113114
),
114115
Entry("is ok for PreferSafe and force=true",
115-
newVMOPEvictPending(vmopbuilder.WithForce()),
116+
newVMOPEvictPending(vmopbuilder.WithForce(ptr.To(true))),
116117
virtv2.PreferSafeMigrationPolicy,
117118
virtv2.VMOPPhasePending,
118119
),
@@ -124,12 +125,12 @@ var _ = Describe("LifecycleHandler", func() {
124125
virtv2.VMOPPhasePending,
125126
),
126127
Entry("should become Failed for AlwaysForced and force=false",
127-
newVMOPEvictPending(vmopbuilder.WithForceFalse()),
128+
newVMOPEvictPending(vmopbuilder.WithForce(ptr.To(false))),
128129
virtv2.AlwaysForcedMigrationPolicy,
129130
virtv2.VMOPPhaseFailed,
130131
),
131132
Entry("is ok for AlwaysForced and force=true",
132-
newVMOPEvictPending(vmopbuilder.WithForce()),
133+
newVMOPEvictPending(vmopbuilder.WithForce(ptr.To(true))),
133134
virtv2.AlwaysForcedMigrationPolicy,
134135
virtv2.VMOPPhasePending,
135136
),
@@ -141,12 +142,12 @@ var _ = Describe("LifecycleHandler", func() {
141142
virtv2.VMOPPhasePending,
142143
),
143144
Entry("is ok for PreferForced and force=false",
144-
newVMOPEvictPending(vmopbuilder.WithForceFalse()),
145+
newVMOPEvictPending(vmopbuilder.WithForce(ptr.To(false))),
145146
virtv2.PreferForcedMigrationPolicy,
146147
virtv2.VMOPPhasePending,
147148
),
148149
Entry("is ok for PreferForced and force=true",
149-
newVMOPEvictPending(vmopbuilder.WithForce()),
150+
newVMOPEvictPending(vmopbuilder.WithForce(ptr.To(true))),
150151
virtv2.PreferForcedMigrationPolicy,
151152
virtv2.VMOPPhasePending,
152153
),

0 commit comments

Comments
 (0)