From 37b7d4edcb6765d3f2b22df0e86f6ca5fc8156ec Mon Sep 17 00:00:00 2001 From: Valeriy Khorunzhin Date: Wed, 15 Jan 2025 11:50:33 +0300 Subject: [PATCH] delete condition add reason Signed-off-by: Valeriy Khorunzhin --- api/core/v1alpha2/vmcondition/condition.go | 4 +- .../controller/vm/internal/block_device.go | 18 ++++- .../vm/internal/block_device_limiter.go | 80 ------------------- .../vm/internal/block_devices_test.go | 19 ++++- .../pkg/controller/vm/internal/interfaces.go | 14 +++- .../pkg/controller/vm/internal/mock.go | 74 +++++++++++++++++ .../pkg/controller/vm/internal/sync_kvvm.go | 5 -- .../pkg/controller/vm/vm_controller.go | 3 +- 8 files changed, 122 insertions(+), 95 deletions(-) delete mode 100644 images/virtualization-artifact/pkg/controller/vm/internal/block_device_limiter.go diff --git a/api/core/v1alpha2/vmcondition/condition.go b/api/core/v1alpha2/vmcondition/condition.go index 7ff2fb56ab..9cd2e33262 100644 --- a/api/core/v1alpha2/vmcondition/condition.go +++ b/api/core/v1alpha2/vmcondition/condition.go @@ -38,7 +38,6 @@ const ( TypeFilesystemReady Type = "FilesystemReady" TypeSizingPolicyMatched Type = "SizingPolicyMatched" TypeSnapshotting Type = "Snapshotting" - TypeDiskAttachmentCapacityAvailable Type = "DiskAttachmentCapacityAvailable" ) type Reason string @@ -102,8 +101,7 @@ const ( ReasonVirtualMachineClassTerminating Reason = "VirtualMachineClassTerminating" ReasonVirtualMachineClassNotExists Reason = "VirtalMachineClassNotExists" - ReasonBlockDeviceCapacityAvailable Reason = "BlockDeviceCapacityAvailable" - ReasonBlockDeviceCapacityReached Reason = "BlockDeviceCapacityReached" + ReasonBlockDeviceCapacityReached Reason = "BlockDeviceCapacityReached" ReasonPodTerminatingReason Reason = "PodTerminating" ReasonPodNotExistsReason Reason = "PodNotExists" diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go index 7b034c7abd..9c5defe4b9 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go @@ -43,10 +43,11 @@ import ( const nameBlockDeviceHandler = "BlockDeviceHandler" -func NewBlockDeviceHandler(cl client.Client, recorder eventrecord.EventRecorderLogger) *BlockDeviceHandler { +func NewBlockDeviceHandler(cl client.Client, recorder eventrecord.EventRecorderLogger, blockDeviceService IBlockDeviceService) *BlockDeviceHandler { return &BlockDeviceHandler{ client: cl, recorder: recorder, + service: blockDeviceService, viProtection: service.NewProtectionService(cl, virtv2.FinalizerVIProtection), cviProtection: service.NewProtectionService(cl, virtv2.FinalizerCVIProtection), @@ -57,6 +58,7 @@ func NewBlockDeviceHandler(cl client.Client, recorder eventrecord.EventRecorderL type BlockDeviceHandler struct { client client.Client recorder eventrecord.EventRecorderLogger + service IBlockDeviceService viProtection *service.ProtectionService cviProtection *service.ProtectionService @@ -95,6 +97,20 @@ func (h *BlockDeviceHandler) Handle(ctx context.Context, s state.VirtualMachineS return reconcile.Result{}, fmt.Errorf("unable to add block devices finalizers: %w", err) } + // Get number of connected block devices + // If it greater limit then set condition to false + blockDeviceAttachedCount, err := h.service.CountBlockDevicesAttachedToVm(ctx, changed) + if err != nil { + return reconcile.Result{}, err + } + + if blockDeviceAttachedCount > common.VmBlockDeviceAttachedLimit { + cb. + Status(metav1.ConditionFalse). + Reason(vmcondition.ReasonBlockDeviceCapacityReached). + Message(fmt.Sprintf("Can not attach %d block devices (%d is maximum) to `VirtualMachine` %q", blockDeviceAttachedCount, common.VmBlockDeviceAttachedLimit, changed.Name)) + } + // Get hot plugged BlockDeviceRefs from vmbdas. vmbdaStatusRefs, err := h.getBlockDeviceStatusRefsFromVMBDA(ctx, s) if err != nil { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_limiter.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device_limiter.go deleted file mode 100644 index cbc46d2e60..0000000000 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device_limiter.go +++ /dev/null @@ -1,80 +0,0 @@ -/* -Copyright 2024 Flant JSC - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package internal - -import ( - "context" - "fmt" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - "github.com/deckhouse/virtualization-controller/pkg/common" - "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/service" - "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" -) - -const nameBlockDeviceLimiterHandler = "BlockDeviceLimiterHandler" - -type BlockDeviceLimiterHandler struct { - service *service.BlockDeviceService -} - -func NewBlockDeviceLimiterHandler(service *service.BlockDeviceService) *BlockDeviceLimiterHandler { - return &BlockDeviceLimiterHandler{service: service} -} - -func (h *BlockDeviceLimiterHandler) Handle(ctx context.Context, s state.VirtualMachineState) (reconcile.Result, error) { - current := s.VirtualMachine().Current() - changed := s.VirtualMachine().Changed() - - if isDeletion(current) { - return reconcile.Result{}, nil - } - - if updated := addAllUnknown(changed, vmcondition.TypeDiskAttachmentCapacityAvailable); updated { - return reconcile.Result{Requeue: true}, nil - } - - blockDeviceAttachedCount, err := h.service.CountBlockDevicesAttachedToVm(ctx, changed) - if err != nil { - return reconcile.Result{}, err - } - - cb := conditions.NewConditionBuilder(vmcondition.TypeDiskAttachmentCapacityAvailable).Generation(changed.Generation) - defer func() { conditions.SetCondition(cb, &changed.Status.Conditions) }() - - if blockDeviceAttachedCount <= common.VmBlockDeviceAttachedLimit { - cb. - Status(metav1.ConditionTrue). - Reason(vmcondition.ReasonBlockDeviceCapacityAvailable). - Message("") - } else { - cb. - Status(metav1.ConditionFalse). - Reason(vmcondition.ReasonBlockDeviceCapacityReached). - Message(fmt.Sprintf("Can not attach %d block devices (%d is maximum) to `VirtualMachine` %q", blockDeviceAttachedCount, common.VmBlockDeviceAttachedLimit, changed.Name)) - } - - return reconcile.Result{}, nil -} - -func (h *BlockDeviceLimiterHandler) Name() string { - return nameBlockDeviceLimiterHandler -} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go index ebff776ee5..4cc9a461a4 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go @@ -17,6 +17,7 @@ limitations under the License. package internal import ( + "context" "log/slog" "testing" @@ -41,10 +42,17 @@ var _ = Describe("func areVirtualDisksAllowedToUse", func() { var vdFoo *virtv2.VirtualDisk var vdBar *virtv2.VirtualDisk + blockDevicesConnectedCount := 3 + + blockDeviceHandlerMock := &IBlockDeviceServiceMock{} + blockDeviceHandlerMock.CountBlockDevicesAttachedToVmFunc = func(_ context.Context, vm *virtv2.VirtualMachine) (int, error) { + return blockDevicesConnectedCount, nil + } + BeforeEach(func() { h = NewBlockDeviceHandler(nil, &eventrecord.EventRecorderLoggerMock{ EventFunc: func(_ client.Object, _, _, _ string) {}, - }) + }, blockDeviceHandlerMock) vdFoo = &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{Name: "vd-foo"}, Status: virtv2.VirtualDiskStatus{ @@ -152,6 +160,13 @@ var _ = Describe("BlockDeviceHandler", func() { var vdFoo *virtv2.VirtualDisk var vdBar *virtv2.VirtualDisk + blockDevicesConnectedCount := 3 + + blockDeviceHandlerMock := &IBlockDeviceServiceMock{} + blockDeviceHandlerMock.CountBlockDevicesAttachedToVmFunc = func(_ context.Context, vm *virtv2.VirtualMachine) (int, error) { + return blockDevicesConnectedCount, nil + } + getBlockDevicesState := func(vi *virtv2.VirtualImage, cvi *virtv2.ClusterVirtualImage, vdFoo, vdBar *virtv2.VirtualDisk) BlockDevicesState { return BlockDevicesState{ VIByName: map[string]*virtv2.VirtualImage{vi.Name: vi}, @@ -164,7 +179,7 @@ var _ = Describe("BlockDeviceHandler", func() { logger = slog.Default() h = NewBlockDeviceHandler(nil, &eventrecord.EventRecorderLoggerMock{ EventFunc: func(_ client.Object, _, _, _ string) {}, - }) + }, blockDeviceHandlerMock) vi = &virtv2.VirtualImage{ ObjectMeta: metav1.ObjectMeta{Name: "vi-01"}, Status: virtv2.VirtualImageStatus{Phase: virtv2.ImageReady}, diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go index 789616ce71..b378b6fe4c 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/interfaces.go @@ -16,8 +16,18 @@ limitations under the License. package internal -import "k8s.io/client-go/tools/record" +import ( + "context" -//go:generate moq -rm -out mock.go . EventRecorder + "k8s.io/client-go/tools/record" + + virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" +) + +//go:generate moq -rm -out mock.go . EventRecorder IBlockDeviceService type EventRecorder = record.EventRecorder + +type IBlockDeviceService interface { + CountBlockDevicesAttachedToVm(ctx context.Context, vm *virtv2.VirtualMachine) (int, error) +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/mock.go b/images/virtualization-artifact/pkg/controller/vm/internal/mock.go index fc1683fba7..bc1ae524a3 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/mock.go @@ -4,6 +4,8 @@ package internal import ( + "context" + virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" "k8s.io/apimachinery/pkg/runtime" "sync" ) @@ -233,3 +235,75 @@ func (mock *EventRecorderMock) EventfCalls() []struct { mock.lockEventf.RUnlock() return calls } + +// Ensure, that IBlockDeviceServiceMock does implement IBlockDeviceService. +// If this is not the case, regenerate this file with moq. +var _ IBlockDeviceService = &IBlockDeviceServiceMock{} + +// IBlockDeviceServiceMock is a mock implementation of IBlockDeviceService. +// +// func TestSomethingThatUsesIBlockDeviceService(t *testing.T) { +// +// // make and configure a mocked IBlockDeviceService +// mockedIBlockDeviceService := &IBlockDeviceServiceMock{ +// CountBlockDevicesAttachedToVmFunc: func(ctx context.Context, vm *virtv2.VirtualMachine) (int, error) { +// panic("mock out the CountBlockDevicesAttachedToVm method") +// }, +// } +// +// // use mockedIBlockDeviceService in code that requires IBlockDeviceService +// // and then make assertions. +// +// } +type IBlockDeviceServiceMock struct { + // CountBlockDevicesAttachedToVmFunc mocks the CountBlockDevicesAttachedToVm method. + CountBlockDevicesAttachedToVmFunc func(ctx context.Context, vm *virtv2.VirtualMachine) (int, error) + + // calls tracks calls to the methods. + calls struct { + // CountBlockDevicesAttachedToVm holds details about calls to the CountBlockDevicesAttachedToVm method. + CountBlockDevicesAttachedToVm []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // VM is the vm argument value. + VM *virtv2.VirtualMachine + } + } + lockCountBlockDevicesAttachedToVm sync.RWMutex +} + +// CountBlockDevicesAttachedToVm calls CountBlockDevicesAttachedToVmFunc. +func (mock *IBlockDeviceServiceMock) CountBlockDevicesAttachedToVm(ctx context.Context, vm *virtv2.VirtualMachine) (int, error) { + if mock.CountBlockDevicesAttachedToVmFunc == nil { + panic("IBlockDeviceServiceMock.CountBlockDevicesAttachedToVmFunc: method is nil but IBlockDeviceService.CountBlockDevicesAttachedToVm was just called") + } + callInfo := struct { + Ctx context.Context + VM *virtv2.VirtualMachine + }{ + Ctx: ctx, + VM: vm, + } + mock.lockCountBlockDevicesAttachedToVm.Lock() + mock.calls.CountBlockDevicesAttachedToVm = append(mock.calls.CountBlockDevicesAttachedToVm, callInfo) + mock.lockCountBlockDevicesAttachedToVm.Unlock() + return mock.CountBlockDevicesAttachedToVmFunc(ctx, vm) +} + +// CountBlockDevicesAttachedToVmCalls gets all the calls that were made to CountBlockDevicesAttachedToVm. +// Check the length with: +// +// len(mockedIBlockDeviceService.CountBlockDevicesAttachedToVmCalls()) +func (mock *IBlockDeviceServiceMock) CountBlockDevicesAttachedToVmCalls() []struct { + Ctx context.Context + VM *virtv2.VirtualMachine +} { + var calls []struct { + Ctx context.Context + VM *virtv2.VirtualMachine + } + mock.lockCountBlockDevicesAttachedToVm.RLock() + calls = mock.calls.CountBlockDevicesAttachedToVm + mock.lockCountBlockDevicesAttachedToVm.RUnlock() + return calls +} diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go index 8df15dc1c1..27778c5018 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go @@ -229,11 +229,6 @@ func (h *SyncKvvmHandler) isWaiting(vm *virtv2.VirtualMachine) bool { return true } - case vmcondition.TypeDiskAttachmentCapacityAvailable: - if c.Status != metav1.ConditionTrue { - return true - } - case vmcondition.TypeSizingPolicyMatched: if c.Status != metav1.ConditionTrue { return true diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go index 88b7b813a5..5efc25f46e 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_controller.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_controller.go @@ -55,8 +55,7 @@ func SetupController( internal.NewDeletionHandler(client), internal.NewClassHandler(client, recorder), internal.NewIPAMHandler(ipam.New(), client, recorder), - internal.NewBlockDeviceHandler(client, recorder), - internal.NewBlockDeviceLimiterHandler(blockDeviceService), + internal.NewBlockDeviceHandler(client, recorder, blockDeviceService), internal.NewProvisioningHandler(client), internal.NewAgentHandler(), internal.NewFilesystemHandler(),