From b15089aa694c97473d0edaba5c386f5a4f10ee14 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Fri, 21 Nov 2025 12:17:23 +0300 Subject: [PATCH 01/11] wip Signed-off-by: Daniil Loktev --- .../pkg/common/snapshot/naming.go | 58 ++++ .../pkg/common/snapshot/naming_test.go | 252 ++++++++++++++++++ .../service/restorer/secret_restorer.go | 34 ++- .../vdsnapshot/internal/life_cycle.go | 27 +- .../vmrestore/internal/interfaces.go | 1 + .../vmrestore/internal/life_cycle.go | 3 +- .../pkg/controller/vmrestore/internal/mock.go | 50 ++++ .../vmsnapshot/internal/interfaces.go | 1 + .../vmsnapshot/internal/life_cycle.go | 10 +- .../vmsnapshot/internal/life_cycle_test.go | 9 + .../controller/vmsnapshot/internal/mock.go | 50 ++++ 11 files changed, 482 insertions(+), 13 deletions(-) create mode 100644 images/virtualization-artifact/pkg/common/snapshot/naming.go create mode 100644 images/virtualization-artifact/pkg/common/snapshot/naming_test.go diff --git a/images/virtualization-artifact/pkg/common/snapshot/naming.go b/images/virtualization-artifact/pkg/common/snapshot/naming.go new file mode 100644 index 0000000000..164d6042b8 --- /dev/null +++ b/images/virtualization-artifact/pkg/common/snapshot/naming.go @@ -0,0 +1,58 @@ +/* +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 snapshot + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/types" +) + +const ( + vmSnapshotSecretPrefix = "d8v-vms-" + vdSnapshotVolumeSnapshotPrefix = "d8v-vds-" + + maxResourceNameLength = 253 + uuidLength = 36 +) + +func GetVMSnapshotSecretName(name string, uid types.UID) string { + maxNameLength := maxResourceNameLength - len(vmSnapshotSecretPrefix) - 1 - uuidLength + truncatedName := truncateName(name, maxNameLength) + return fmt.Sprintf("%s%s-%s", vmSnapshotSecretPrefix, truncatedName, uid) +} + +func GetLegacyVMSnapshotSecretName(name string) string { + return name +} + +func GetVDSnapshotVolumeSnapshotName(name string, uid types.UID) string { + maxNameLength := maxResourceNameLength - len(vdSnapshotVolumeSnapshotPrefix) - 1 - uuidLength + truncatedName := truncateName(name, maxNameLength) + return fmt.Sprintf("%s%s-%s", vdSnapshotVolumeSnapshotPrefix, truncatedName, uid) +} + +func GetLegacyVDSnapshotVolumeSnapshotName(name string) string { + return name +} + +func truncateName(name string, maxLength int) string { + if len(name) <= maxLength { + return name + } + return name[:maxLength] +} diff --git a/images/virtualization-artifact/pkg/common/snapshot/naming_test.go b/images/virtualization-artifact/pkg/common/snapshot/naming_test.go new file mode 100644 index 0000000000..bb56f4fbb1 --- /dev/null +++ b/images/virtualization-artifact/pkg/common/snapshot/naming_test.go @@ -0,0 +1,252 @@ +/* +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 snapshot + +import ( + "strings" + "testing" + + "k8s.io/apimachinery/pkg/types" +) + +func TestGetVMSnapshotSecretName(t *testing.T) { + testUID := types.UID("12345678-1234-1234-1234-123456789012") + + tests := []struct { + name string + inputName string + expectedLen int + hasPrefix bool + hasSuffix bool + }{ + { + name: "short name", + inputName: "my-snapshot", + expectedLen: len("d8v-vms-my-snapshot-") + 36, + hasPrefix: true, + hasSuffix: true, + }, + { + name: "very long name", + inputName: strings.Repeat("a", 300), + expectedLen: 253, + hasPrefix: true, + hasSuffix: true, + }, + { + name: "empty name", + inputName: "", + expectedLen: len("d8v-vms--") + 36, + hasPrefix: true, + hasSuffix: true, + }, + { + name: "name at limit", + inputName: strings.Repeat("b", 208), + expectedLen: 253, + hasPrefix: true, + hasSuffix: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GetVMSnapshotSecretName(tt.inputName, testUID) + + if len(result) != tt.expectedLen { + t.Errorf("expected length %d, got %d (result: %s)", tt.expectedLen, len(result), result) + } + + if tt.hasPrefix && !strings.HasPrefix(result, "d8v-vms-") { + t.Errorf("expected prefix 'd8v-vms-', got %s", result) + } + + if tt.hasSuffix && !strings.HasSuffix(result, string(testUID)) { + t.Errorf("expected suffix '%s', got %s", testUID, result) + } + + if len(result) > maxResourceNameLength { + t.Errorf("result exceeds max length: %d > %d", len(result), maxResourceNameLength) + } + }) + } +} + +func TestGetVDSnapshotVolumeSnapshotName(t *testing.T) { + testUID := types.UID("87654321-4321-4321-4321-210987654321") + + tests := []struct { + name string + inputName string + expectedLen int + hasPrefix bool + hasSuffix bool + }{ + { + name: "short name", + inputName: "my-disk-snapshot", + expectedLen: len("d8v-vds-my-disk-snapshot-") + 36, + hasPrefix: true, + hasSuffix: true, + }, + { + name: "very long name", + inputName: strings.Repeat("c", 300), + expectedLen: 253, + hasPrefix: true, + hasSuffix: true, + }, + { + name: "empty name", + inputName: "", + expectedLen: len("d8v-vds--") + 36, + hasPrefix: true, + hasSuffix: true, + }, + { + name: "name at limit", + inputName: strings.Repeat("d", 208), + expectedLen: 253, + hasPrefix: true, + hasSuffix: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GetVDSnapshotVolumeSnapshotName(tt.inputName, testUID) + + if len(result) != tt.expectedLen { + t.Errorf("expected length %d, got %d (result: %s)", tt.expectedLen, len(result), result) + } + + if tt.hasPrefix && !strings.HasPrefix(result, "d8v-vds-") { + t.Errorf("expected prefix 'd8v-vds-', got %s", result) + } + + if tt.hasSuffix && !strings.HasSuffix(result, string(testUID)) { + t.Errorf("expected suffix '%s', got %s", testUID, result) + } + + if len(result) > maxResourceNameLength { + t.Errorf("result exceeds max length: %d > %d", len(result), maxResourceNameLength) + } + }) + } +} + +func TestGetLegacyVMSnapshotSecretName(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "simple name", + input: "my-snapshot", + expected: "my-snapshot", + }, + { + name: "long name", + input: strings.Repeat("a", 300), + expected: strings.Repeat("a", 300), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GetLegacyVMSnapshotSecretName(tt.input) + if result != tt.expected { + t.Errorf("expected %s, got %s", tt.expected, result) + } + }) + } +} + +func TestGetLegacyVDSnapshotVolumeSnapshotName(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "simple name", + input: "my-disk-snapshot", + expected: "my-disk-snapshot", + }, + { + name: "long name", + input: strings.Repeat("b", 300), + expected: strings.Repeat("b", 300), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := GetLegacyVDSnapshotVolumeSnapshotName(tt.input) + if result != tt.expected { + t.Errorf("expected %s, got %s", tt.expected, result) + } + }) + } +} + +func TestTruncateName(t *testing.T) { + tests := []struct { + name string + input string + maxLength int + expected string + }{ + { + name: "no truncation needed", + input: "short", + maxLength: 10, + expected: "short", + }, + { + name: "truncation needed", + input: "very-long-name", + maxLength: 5, + expected: "very-", + }, + { + name: "exact length", + input: "exact", + maxLength: 5, + expected: "exact", + }, + { + name: "empty string", + input: "", + maxLength: 10, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := truncateName(tt.input, tt.maxLength) + if result != tt.expected { + t.Errorf("expected %s, got %s", tt.expected, result) + } + if len(result) > tt.maxLength { + t.Errorf("result exceeds max length: %d > %d", len(result), tt.maxLength) + } + }) + } +} diff --git a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go index 38d41ae158..0638d15c7d 100644 --- a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go +++ b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go @@ -24,11 +24,13 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/deckhouse/virtualization-controller/pkg/common/object" + "github.com/deckhouse/virtualization-controller/pkg/common/snapshot" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -43,6 +45,36 @@ func NewSecretRestorer(client client.Client) *SecretRestorer { } } +func (r SecretRestorer) Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { + newSecretName := snapshot.GetVMSnapshotSecretName(vmSnapshot.Name, vmSnapshot.UID) + secret := &corev1.Secret{} + err := r.client.Get(ctx, types.NamespacedName{ + Name: newSecretName, + Namespace: vmSnapshot.Namespace, + }, secret) + if err == nil { + return secret, nil + } + + if !k8serrors.IsNotFound(err) { + return nil, fmt.Errorf("failed to get secret with new name %s: %w", newSecretName, err) + } + + legacySecretName := snapshot.GetLegacyVMSnapshotSecretName(vmSnapshot.Name) + err = r.client.Get(ctx, types.NamespacedName{ + Name: legacySecretName, + Namespace: vmSnapshot.Namespace, + }, secret) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil, fmt.Errorf("snapshot secret not found (tried %s and %s)", newSecretName, legacySecretName) + } + return nil, fmt.Errorf("failed to get secret with legacy name %s: %w", legacySecretName, err) + } + + return secret, nil +} + func (r SecretRestorer) Store(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { secret := corev1.Secret{ TypeMeta: metav1.TypeMeta{ @@ -50,7 +82,7 @@ func (r SecretRestorer) Store(ctx context.Context, vm *v1alpha2.VirtualMachine, APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Name: vmSnapshot.Name, + Name: snapshot.GetVMSnapshotSecretName(vmSnapshot.Name, vmSnapshot.UID), Namespace: vmSnapshot.Namespace, OwnerReferences: []metav1.OwnerReference{ service.MakeOwnerReference(vmSnapshot), diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go index 1d6f6ab8fc..fbfd20e106 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go @@ -24,11 +24,13 @@ import ( vsv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" + "github.com/deckhouse/virtualization-controller/pkg/common/snapshot" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/logger" @@ -65,7 +67,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu conditions.SetCondition(cb, &vdSnapshot.Status.Conditions) }() - vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot.Name, vdSnapshot.Namespace) + vs, err := h.getVolumeSnapshotWithFallback(ctx, vdSnapshot) if err != nil { setPhaseConditionToFailed(cb, &vdSnapshot.Status.Phase, err) return reconcile.Result{}, err @@ -260,7 +262,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu vs = &vsv1.VolumeSnapshot{ ObjectMeta: metav1.ObjectMeta{ Annotations: anno, - Name: vdSnapshot.Name, + Name: snapshot.GetVDSnapshotVolumeSnapshotName(vdSnapshot.Name, vdSnapshot.UID), Namespace: vdSnapshot.Namespace, OwnerReferences: []metav1.OwnerReference{ service.MakeOwnerReference(vdSnapshot), @@ -411,3 +413,24 @@ func (h LifeCycleHandler) unfreezeFilesystemIfFailed(ctx context.Context, vdSnap return nil } + +func (h LifeCycleHandler) getVolumeSnapshotWithFallback(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { + newVSName := snapshot.GetVDSnapshotVolumeSnapshotName(vdSnapshot.Name, vdSnapshot.UID) + vs, err := h.snapshotter.GetVolumeSnapshot(ctx, newVSName, vdSnapshot.Namespace) + if err == nil { + return vs, nil + } + + if !k8serrors.IsNotFound(err) { + return nil, fmt.Errorf("failed to get volume snapshot with new name %s: %w", newVSName, err) + } + + legacyVSName := snapshot.GetLegacyVDSnapshotVolumeSnapshotName(vdSnapshot.Name) + vs, err = h.snapshotter.GetVolumeSnapshot(ctx, legacyVSName, vdSnapshot.Namespace) + if err != nil { + return nil, err + } + + return vs, nil +} + diff --git a/images/virtualization-artifact/pkg/controller/vmrestore/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vmrestore/internal/interfaces.go index 3e72cf4467..7af372753c 100644 --- a/images/virtualization-artifact/pkg/controller/vmrestore/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vmrestore/internal/interfaces.go @@ -27,6 +27,7 @@ import ( //go:generate go tool moq -rm -out mock.go . Restorer type Restorer interface { + Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) RestoreVirtualMachine(ctx context.Context, secret *corev1.Secret) (*v1alpha2.VirtualMachine, error) RestoreProvisioner(ctx context.Context, secret *corev1.Secret) (*corev1.Secret, error) RestoreVirtualMachineIPAddress(ctx context.Context, secret *corev1.Secret) (*v1alpha2.VirtualMachineIPAddress, error) diff --git a/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go index e8751ca91f..1bb8dc7f87 100644 --- a/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go @@ -125,8 +125,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmRestore *v1alpha2.Virtua return reconcile.Result{}, err } - restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} - restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, h.client, &corev1.Secret{}) + restorerSecret, err := h.restorer.Get(ctx, vmSnapshot) if err != nil { setPhaseConditionToFailed(cb, &vmRestore.Status.Phase, err) return reconcile.Result{}, err diff --git a/images/virtualization-artifact/pkg/controller/vmrestore/internal/mock.go b/images/virtualization-artifact/pkg/controller/vmrestore/internal/mock.go index 4165a3dd57..f5724e703c 100644 --- a/images/virtualization-artifact/pkg/controller/vmrestore/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vmrestore/internal/mock.go @@ -20,6 +20,9 @@ var _ Restorer = &RestorerMock{} // // // make and configure a mocked Restorer // mockedRestorer := &RestorerMock{ +// GetFunc: func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { +// panic("mock out the Get method") +// }, // RestoreMACAddressOrderFunc: func(ctx context.Context, secret *corev1.Secret) ([]string, error) { // panic("mock out the RestoreMACAddressOrder method") // }, @@ -45,6 +48,9 @@ var _ Restorer = &RestorerMock{} // // } type RestorerMock struct { + // GetFunc mocks the Get method. + GetFunc func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) + // RestoreMACAddressOrderFunc mocks the RestoreMACAddressOrder method. RestoreMACAddressOrderFunc func(ctx context.Context, secret *corev1.Secret) ([]string, error) @@ -65,6 +71,13 @@ type RestorerMock struct { // calls tracks calls to the methods. calls struct { + // Get holds details about calls to the Get method. + Get []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // VmSnapshot is the vmSnapshot argument value. + VmSnapshot *v1alpha2.VirtualMachineSnapshot + } // RestoreMACAddressOrder holds details about calls to the RestoreMACAddressOrder method. RestoreMACAddressOrder []struct { // Ctx is the ctx argument value. @@ -108,6 +121,7 @@ type RestorerMock struct { Secret *corev1.Secret } } + lockGet sync.RWMutex lockRestoreMACAddressOrder sync.RWMutex lockRestoreProvisioner sync.RWMutex lockRestoreVirtualMachine sync.RWMutex @@ -116,6 +130,42 @@ type RestorerMock struct { lockRestoreVirtualMachineMACAddresses sync.RWMutex } +// Get calls GetFunc. +func (mock *RestorerMock) Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { + if mock.GetFunc == nil { + panic("RestorerMock.GetFunc: method is nil but Restorer.Get was just called") + } + callInfo := struct { + Ctx context.Context + VmSnapshot *v1alpha2.VirtualMachineSnapshot + }{ + Ctx: ctx, + VmSnapshot: vmSnapshot, + } + mock.lockGet.Lock() + mock.calls.Get = append(mock.calls.Get, callInfo) + mock.lockGet.Unlock() + return mock.GetFunc(ctx, vmSnapshot) +} + +// GetCalls gets all the calls that were made to Get. +// Check the length with: +// +// len(mockedRestorer.GetCalls()) +func (mock *RestorerMock) GetCalls() []struct { + Ctx context.Context + VmSnapshot *v1alpha2.VirtualMachineSnapshot +} { + var calls []struct { + Ctx context.Context + VmSnapshot *v1alpha2.VirtualMachineSnapshot + } + mock.lockGet.RLock() + calls = mock.calls.Get + mock.lockGet.RUnlock() + return calls +} + // RestoreMACAddressOrder calls RestoreMACAddressOrderFunc. func (mock *RestorerMock) RestoreMACAddressOrder(ctx context.Context, secret *corev1.Secret) ([]string, error) { if mock.RestoreMACAddressOrderFunc == nil { diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go index 6690332a57..16b19a4d30 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go @@ -27,6 +27,7 @@ import ( //go:generate go tool moq -rm -out mock.go . Storer Snapshotter type Storer interface { + Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) Store(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) } diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go index 42219dfdbd..acb2d46802 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go @@ -599,14 +599,8 @@ func (h LifeCycleHandler) ensureSecret(ctx context.Context, vm *v1alpha2.Virtual var secret *corev1.Secret var err error - if vmSnapshot.Status.VirtualMachineSnapshotSecretName != "" { - secret, err = h.snapshotter.GetSecret(ctx, vmSnapshot.Status.VirtualMachineSnapshotSecretName, vmSnapshot.Namespace) - if err != nil { - return err - } - } - - if secret == nil { + secret, err = h.storer.Get(ctx, vmSnapshot) + if err != nil { secret, err = h.storer.Store(ctx, vm, vmSnapshot) if err != nil { return err diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go index 66561c384a..42f4d4fba1 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go @@ -144,6 +144,15 @@ var _ = Describe("LifeCycle handler", func() { }, } + storer = &StorerMock{ + GetFunc: func(_ context.Context, _ *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { + return secret, nil + }, + StoreFunc: func(_ context.Context, _ *v1alpha2.VirtualMachine, _ *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { + return secret, nil + }, + } + var err error fakeClient, err = testutil.NewFakeClientWithObjects(vd, vm, secret, vmSnapshot, vdSnapshot) Expect(err).NotTo(HaveOccurred()) diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go index 134a491adb..6c380752c6 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go @@ -20,6 +20,9 @@ var _ Storer = &StorerMock{} // // // make and configure a mocked Storer // mockedStorer := &StorerMock{ +// GetFunc: func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { +// panic("mock out the Get method") +// }, // StoreFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { // panic("mock out the Store method") // }, @@ -30,11 +33,21 @@ var _ Storer = &StorerMock{} // // } type StorerMock struct { + // GetFunc mocks the Get method. + GetFunc func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) + // StoreFunc mocks the Store method. StoreFunc func(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) // calls tracks calls to the methods. calls struct { + // Get holds details about calls to the Get method. + Get []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // VmSnapshot is the vmSnapshot argument value. + VmSnapshot *v1alpha2.VirtualMachineSnapshot + } // Store holds details about calls to the Store method. Store []struct { // Ctx is the ctx argument value. @@ -45,9 +58,46 @@ type StorerMock struct { VmSnapshot *v1alpha2.VirtualMachineSnapshot } } + lockGet sync.RWMutex lockStore sync.RWMutex } +// Get calls GetFunc. +func (mock *StorerMock) Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { + if mock.GetFunc == nil { + panic("StorerMock.GetFunc: method is nil but Storer.Get was just called") + } + callInfo := struct { + Ctx context.Context + VmSnapshot *v1alpha2.VirtualMachineSnapshot + }{ + Ctx: ctx, + VmSnapshot: vmSnapshot, + } + mock.lockGet.Lock() + mock.calls.Get = append(mock.calls.Get, callInfo) + mock.lockGet.Unlock() + return mock.GetFunc(ctx, vmSnapshot) +} + +// GetCalls gets all the calls that were made to Get. +// Check the length with: +// +// len(mockedStorer.GetCalls()) +func (mock *StorerMock) GetCalls() []struct { + Ctx context.Context + VmSnapshot *v1alpha2.VirtualMachineSnapshot +} { + var calls []struct { + Ctx context.Context + VmSnapshot *v1alpha2.VirtualMachineSnapshot + } + mock.lockGet.RLock() + calls = mock.calls.Get + mock.lockGet.RUnlock() + return calls +} + // Store calls StoreFunc. func (mock *StorerMock) Store(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { if mock.StoreFunc == nil { From af7c6530c235e82bf5435ca4708d17b80f802f9e Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 24 Nov 2025 11:42:33 +0300 Subject: [PATCH 02/11] wip Signed-off-by: Daniil Loktev --- .../pkg/common/snapshot/naming.go | 58 ---- .../pkg/common/snapshot/naming_test.go | 252 ------------------ .../service/restorer/secret_restorer.go | 39 +-- .../pkg/controller/supplements/fetch.go | 11 + .../pkg/controller/supplements/generator.go | 17 ++ .../controller/supplements/generator_test.go | 11 + .../vdsnapshot/internal/life_cycle.go | 34 ++- 7 files changed, 72 insertions(+), 350 deletions(-) delete mode 100644 images/virtualization-artifact/pkg/common/snapshot/naming.go delete mode 100644 images/virtualization-artifact/pkg/common/snapshot/naming_test.go diff --git a/images/virtualization-artifact/pkg/common/snapshot/naming.go b/images/virtualization-artifact/pkg/common/snapshot/naming.go deleted file mode 100644 index 164d6042b8..0000000000 --- a/images/virtualization-artifact/pkg/common/snapshot/naming.go +++ /dev/null @@ -1,58 +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 snapshot - -import ( - "fmt" - - "k8s.io/apimachinery/pkg/types" -) - -const ( - vmSnapshotSecretPrefix = "d8v-vms-" - vdSnapshotVolumeSnapshotPrefix = "d8v-vds-" - - maxResourceNameLength = 253 - uuidLength = 36 -) - -func GetVMSnapshotSecretName(name string, uid types.UID) string { - maxNameLength := maxResourceNameLength - len(vmSnapshotSecretPrefix) - 1 - uuidLength - truncatedName := truncateName(name, maxNameLength) - return fmt.Sprintf("%s%s-%s", vmSnapshotSecretPrefix, truncatedName, uid) -} - -func GetLegacyVMSnapshotSecretName(name string) string { - return name -} - -func GetVDSnapshotVolumeSnapshotName(name string, uid types.UID) string { - maxNameLength := maxResourceNameLength - len(vdSnapshotVolumeSnapshotPrefix) - 1 - uuidLength - truncatedName := truncateName(name, maxNameLength) - return fmt.Sprintf("%s%s-%s", vdSnapshotVolumeSnapshotPrefix, truncatedName, uid) -} - -func GetLegacyVDSnapshotVolumeSnapshotName(name string) string { - return name -} - -func truncateName(name string, maxLength int) string { - if len(name) <= maxLength { - return name - } - return name[:maxLength] -} diff --git a/images/virtualization-artifact/pkg/common/snapshot/naming_test.go b/images/virtualization-artifact/pkg/common/snapshot/naming_test.go deleted file mode 100644 index bb56f4fbb1..0000000000 --- a/images/virtualization-artifact/pkg/common/snapshot/naming_test.go +++ /dev/null @@ -1,252 +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 snapshot - -import ( - "strings" - "testing" - - "k8s.io/apimachinery/pkg/types" -) - -func TestGetVMSnapshotSecretName(t *testing.T) { - testUID := types.UID("12345678-1234-1234-1234-123456789012") - - tests := []struct { - name string - inputName string - expectedLen int - hasPrefix bool - hasSuffix bool - }{ - { - name: "short name", - inputName: "my-snapshot", - expectedLen: len("d8v-vms-my-snapshot-") + 36, - hasPrefix: true, - hasSuffix: true, - }, - { - name: "very long name", - inputName: strings.Repeat("a", 300), - expectedLen: 253, - hasPrefix: true, - hasSuffix: true, - }, - { - name: "empty name", - inputName: "", - expectedLen: len("d8v-vms--") + 36, - hasPrefix: true, - hasSuffix: true, - }, - { - name: "name at limit", - inputName: strings.Repeat("b", 208), - expectedLen: 253, - hasPrefix: true, - hasSuffix: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := GetVMSnapshotSecretName(tt.inputName, testUID) - - if len(result) != tt.expectedLen { - t.Errorf("expected length %d, got %d (result: %s)", tt.expectedLen, len(result), result) - } - - if tt.hasPrefix && !strings.HasPrefix(result, "d8v-vms-") { - t.Errorf("expected prefix 'd8v-vms-', got %s", result) - } - - if tt.hasSuffix && !strings.HasSuffix(result, string(testUID)) { - t.Errorf("expected suffix '%s', got %s", testUID, result) - } - - if len(result) > maxResourceNameLength { - t.Errorf("result exceeds max length: %d > %d", len(result), maxResourceNameLength) - } - }) - } -} - -func TestGetVDSnapshotVolumeSnapshotName(t *testing.T) { - testUID := types.UID("87654321-4321-4321-4321-210987654321") - - tests := []struct { - name string - inputName string - expectedLen int - hasPrefix bool - hasSuffix bool - }{ - { - name: "short name", - inputName: "my-disk-snapshot", - expectedLen: len("d8v-vds-my-disk-snapshot-") + 36, - hasPrefix: true, - hasSuffix: true, - }, - { - name: "very long name", - inputName: strings.Repeat("c", 300), - expectedLen: 253, - hasPrefix: true, - hasSuffix: true, - }, - { - name: "empty name", - inputName: "", - expectedLen: len("d8v-vds--") + 36, - hasPrefix: true, - hasSuffix: true, - }, - { - name: "name at limit", - inputName: strings.Repeat("d", 208), - expectedLen: 253, - hasPrefix: true, - hasSuffix: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := GetVDSnapshotVolumeSnapshotName(tt.inputName, testUID) - - if len(result) != tt.expectedLen { - t.Errorf("expected length %d, got %d (result: %s)", tt.expectedLen, len(result), result) - } - - if tt.hasPrefix && !strings.HasPrefix(result, "d8v-vds-") { - t.Errorf("expected prefix 'd8v-vds-', got %s", result) - } - - if tt.hasSuffix && !strings.HasSuffix(result, string(testUID)) { - t.Errorf("expected suffix '%s', got %s", testUID, result) - } - - if len(result) > maxResourceNameLength { - t.Errorf("result exceeds max length: %d > %d", len(result), maxResourceNameLength) - } - }) - } -} - -func TestGetLegacyVMSnapshotSecretName(t *testing.T) { - tests := []struct { - name string - input string - expected string - }{ - { - name: "simple name", - input: "my-snapshot", - expected: "my-snapshot", - }, - { - name: "long name", - input: strings.Repeat("a", 300), - expected: strings.Repeat("a", 300), - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := GetLegacyVMSnapshotSecretName(tt.input) - if result != tt.expected { - t.Errorf("expected %s, got %s", tt.expected, result) - } - }) - } -} - -func TestGetLegacyVDSnapshotVolumeSnapshotName(t *testing.T) { - tests := []struct { - name string - input string - expected string - }{ - { - name: "simple name", - input: "my-disk-snapshot", - expected: "my-disk-snapshot", - }, - { - name: "long name", - input: strings.Repeat("b", 300), - expected: strings.Repeat("b", 300), - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := GetLegacyVDSnapshotVolumeSnapshotName(tt.input) - if result != tt.expected { - t.Errorf("expected %s, got %s", tt.expected, result) - } - }) - } -} - -func TestTruncateName(t *testing.T) { - tests := []struct { - name string - input string - maxLength int - expected string - }{ - { - name: "no truncation needed", - input: "short", - maxLength: 10, - expected: "short", - }, - { - name: "truncation needed", - input: "very-long-name", - maxLength: 5, - expected: "very-", - }, - { - name: "exact length", - input: "exact", - maxLength: 5, - expected: "exact", - }, - { - name: "empty string", - input: "", - maxLength: 10, - expected: "", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := truncateName(tt.input, tt.maxLength) - if result != tt.expected { - t.Errorf("expected %s, got %s", tt.expected, result) - } - if len(result) > tt.maxLength { - t.Errorf("result exceeds max length: %d > %d", len(result), tt.maxLength) - } - }) - } -} diff --git a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go index 0638d15c7d..4b07880f1c 100644 --- a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go +++ b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go @@ -24,14 +24,13 @@ import ( "fmt" corev1 "k8s.io/api/core/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/deckhouse/virtualization-controller/pkg/common/object" - "github.com/deckhouse/virtualization-controller/pkg/common/snapshot" "github.com/deckhouse/virtualization-controller/pkg/controller/service" + "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -46,44 +45,22 @@ func NewSecretRestorer(client client.Client) *SecretRestorer { } func (r SecretRestorer) Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { - newSecretName := snapshot.GetVMSnapshotSecretName(vmSnapshot.Name, vmSnapshot.UID) - secret := &corev1.Secret{} - err := r.client.Get(ctx, types.NamespacedName{ - Name: newSecretName, - Namespace: vmSnapshot.Namespace, - }, secret) - if err == nil { - return secret, nil - } - - if !k8serrors.IsNotFound(err) { - return nil, fmt.Errorf("failed to get secret with new name %s: %w", newSecretName, err) - } - - legacySecretName := snapshot.GetLegacyVMSnapshotSecretName(vmSnapshot.Name) - err = r.client.Get(ctx, types.NamespacedName{ - Name: legacySecretName, - Namespace: vmSnapshot.Namespace, - }, secret) - if err != nil { - if k8serrors.IsNotFound(err) { - return nil, fmt.Errorf("snapshot secret not found (tried %s and %s)", newSecretName, legacySecretName) - } - return nil, fmt.Errorf("failed to get secret with legacy name %s: %w", legacySecretName, err) - } - - return secret, nil + supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) + return supplements.FetchSupplement(ctx, r.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) } func (r SecretRestorer) Store(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { + supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) + secretName := supGen.CommonSupplement() + secret := corev1.Secret{ TypeMeta: metav1.TypeMeta{ Kind: "Secret", APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Name: snapshot.GetVMSnapshotSecretName(vmSnapshot.Name, vmSnapshot.UID), - Namespace: vmSnapshot.Namespace, + Name: secretName.Name, + Namespace: secretName.Namespace, OwnerReferences: []metav1.OwnerReference{ service.MakeOwnerReference(vmSnapshot), }, diff --git a/images/virtualization-artifact/pkg/controller/supplements/fetch.go b/images/virtualization-artifact/pkg/controller/supplements/fetch.go index 1b150f9e93..7bdbee5cf4 100644 --- a/images/virtualization-artifact/pkg/controller/supplements/fetch.go +++ b/images/virtualization-artifact/pkg/controller/supplements/fetch.go @@ -49,6 +49,9 @@ const ( SupplementCABundleConfigMap SupplementType = "CABundleConfigMap" SupplementImagePullSecret SupplementType = "ImagePullSecret" SupplementUploaderTLSSecret SupplementType = "UploaderTLSSecret" + + // Snapshots + SupplementSnapshot SupplementType = "Snapshot" ) // GetSupplementName returns the name for the requested supplement type @@ -88,6 +91,10 @@ func GetSupplementName(gen Generator, supplementType SupplementType) (types.Name case SupplementUploaderTLSSecret: return gen.UploaderTLSSecretForIngress(), nil + // Snapshots + case SupplementSnapshot: + return gen.CommonSupplement(), nil + default: return types.NamespacedName{}, fmt.Errorf("unknown supplement type: %s", supplementType) } @@ -130,6 +137,10 @@ func GetLegacySupplementName(gen Generator, supplementType SupplementType) (type case SupplementUploaderTLSSecret: return gen.LegacyUploaderTLSSecretForIngress(), nil + // Snapshots + case SupplementSnapshot: + return gen.LegacySnapshotSupplement(), nil + default: return types.NamespacedName{}, fmt.Errorf("unknown supplement type: %s", supplementType) } diff --git a/images/virtualization-artifact/pkg/controller/supplements/generator.go b/images/virtualization-artifact/pkg/controller/supplements/generator.go index 0238009b95..1fa103aa37 100644 --- a/images/virtualization-artifact/pkg/controller/supplements/generator.go +++ b/images/virtualization-artifact/pkg/controller/supplements/generator.go @@ -56,6 +56,7 @@ type Generator interface { UploaderTLSSecretForIngress() types.NamespacedName ImagePullSecret() types.NamespacedName NetworkPolicy() types.NamespacedName + CommonSupplement() types.NamespacedName LegacyBounderPod() types.NamespacedName LegacyImporterPod() types.NamespacedName @@ -70,6 +71,7 @@ type Generator interface { LegacyDVCRAuthSecretForDV() types.NamespacedName LegacyUploaderTLSSecretForIngress() types.NamespacedName LegacyImagePullSecret() types.NamespacedName + LegacySnapshotSupplement() types.NamespacedName } // Generator calculates names for supplemental resources, e.g. ImporterPod, AuthSecret or CABundleConfigMap. @@ -177,6 +179,12 @@ func (g *generator) NetworkPolicy() types.NamespacedName { return g.generateName(tplCommon, kvalidation.DNS1123SubdomainMaxLength) } +// CommonSupplement generates name for common supplemental resources with d8v--- format. +// Used for snapshot-related resources (VMS Secret, VDS VolumeSnapshot). +func (g *generator) CommonSupplement() types.NamespacedName { + return g.generateName(tplCommon, kvalidation.DNS1123SubdomainMaxLength) +} + // PersistentVolumeClaim generates name for underlying PersistentVolumeClaim. // PVC is always one for vmd/vmi, so prefix is used. func (g *generator) PersistentVolumeClaim() types.NamespacedName { @@ -270,3 +278,12 @@ func (g *generator) LegacyDataVolume() types.NamespacedName { func (g *generator) LegacyPersistentVolumeClaim() types.NamespacedName { return g.LegacyDataVolume() } + +// LegacySnapshotSupplement generates old format name for snapshot-related resources. +// Returns just the name without any prefix or UID (legacy naming). +func (g *generator) LegacySnapshotSupplement() types.NamespacedName { + return types.NamespacedName{ + Name: g.name, + Namespace: g.namespace, + } +} diff --git a/images/virtualization-artifact/pkg/controller/supplements/generator_test.go b/images/virtualization-artifact/pkg/controller/supplements/generator_test.go index 56c140346a..8a055a471c 100644 --- a/images/virtualization-artifact/pkg/controller/supplements/generator_test.go +++ b/images/virtualization-artifact/pkg/controller/supplements/generator_test.go @@ -65,8 +65,18 @@ var _ = Describe("Generator", func() { Entry("DataVolume", func(g Generator) types.NamespacedName { return g.DataVolume() }, "vi"), Entry("PersistentVolumeClaim", func(g Generator) types.NamespacedName { return g.PersistentVolumeClaim() }, "vi"), Entry("NetworkPolicy", func(g Generator) types.NamespacedName { return g.NetworkPolicy() }, "vi"), + Entry("CommonSupplement", func(g Generator) types.NamespacedName { return g.CommonSupplement() }, "vi"), ) + It("should generate legacy snapshot supplement name without prefix or UID", func() { + name := "test-snapshot" + gen = NewGenerator("vms", name, namespace, uid) + result := gen.LegacySnapshotSupplement() + + Expect(result.Name).To(Equal(name)) + Expect(result.Namespace).To(Equal(namespace)) + }) + DescribeTable("should truncate long names to respect limits", func(method func(Generator) types.NamespacedName, maxLength int) { name := strings.Repeat("very-long-resource-name-", 30) @@ -91,6 +101,7 @@ var _ = Describe("Generator", func() { Entry("DataVolume - 253 limit", func(g Generator) types.NamespacedName { return g.DataVolume() }, kvalidation.DNS1123SubdomainMaxLength), Entry("PersistentVolumeClaim - 253 limit", func(g Generator) types.NamespacedName { return g.PersistentVolumeClaim() }, kvalidation.DNS1123SubdomainMaxLength), Entry("NetworkPolicy - 253 limit", func(g Generator) types.NamespacedName { return g.NetworkPolicy() }, kvalidation.DNS1123SubdomainMaxLength), + Entry("CommonSupplement - 253 limit", func(g Generator) types.NamespacedName { return g.CommonSupplement() }, kvalidation.DNS1123SubdomainMaxLength), ) }) }) diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go index fbfd20e106..bf99c4db80 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go @@ -30,9 +30,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/deckhouse/virtualization-controller/pkg/common/annotations" - "github.com/deckhouse/virtualization-controller/pkg/common/snapshot" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" + "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization-controller/pkg/logger" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdscondition" @@ -259,11 +259,18 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu } } + supGen := supplements.NewGenerator("vds", vdSnapshot.Name, vdSnapshot.Namespace, vdSnapshot.UID) + vsName, err := supplements.GetSupplementName(supGen, supplements.SupplementSnapshotCommon) + if err != nil { + setPhaseConditionToFailed(cb, &vdSnapshot.Status.Phase, err) + return reconcile.Result{}, err + } + vs = &vsv1.VolumeSnapshot{ ObjectMeta: metav1.ObjectMeta{ Annotations: anno, - Name: snapshot.GetVDSnapshotVolumeSnapshotName(vdSnapshot.Name, vdSnapshot.UID), - Namespace: vdSnapshot.Namespace, + Name: vsName.Name, + Namespace: vsName.Namespace, OwnerReferences: []metav1.OwnerReference{ service.MakeOwnerReference(vdSnapshot), }, @@ -415,22 +422,31 @@ func (h LifeCycleHandler) unfreezeFilesystemIfFailed(ctx context.Context, vdSnap } func (h LifeCycleHandler) getVolumeSnapshotWithFallback(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { - newVSName := snapshot.GetVDSnapshotVolumeSnapshotName(vdSnapshot.Name, vdSnapshot.UID) - vs, err := h.snapshotter.GetVolumeSnapshot(ctx, newVSName, vdSnapshot.Namespace) + supGen := supplements.NewGenerator("vds", vdSnapshot.Name, vdSnapshot.Namespace, vdSnapshot.UID) + + newVSName, err := supplements.GetSupplementName(supGen, supplements.SupplementSnapshotCommon) + if err != nil { + return nil, err + } + + vs, err := h.snapshotter.GetVolumeSnapshot(ctx, newVSName.Name, newVSName.Namespace) if err == nil { return vs, nil } if !k8serrors.IsNotFound(err) { - return nil, fmt.Errorf("failed to get volume snapshot with new name %s: %w", newVSName, err) + return nil, fmt.Errorf("failed to get volume snapshot with new name %s: %w", newVSName.Name, err) } - legacyVSName := snapshot.GetLegacyVDSnapshotVolumeSnapshotName(vdSnapshot.Name) - vs, err = h.snapshotter.GetVolumeSnapshot(ctx, legacyVSName, vdSnapshot.Namespace) + legacyVSName, err := supplements.GetLegacySupplementName(supGen, supplements.SupplementSnapshotCommon) + if err != nil { + return nil, err + } + + vs, err = h.snapshotter.GetVolumeSnapshot(ctx, legacyVSName.Name, legacyVSName.Namespace) if err != nil { return nil, err } return vs, nil } - From 10127841d4b752edde89b92ec5e47f23c1273bdd Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 24 Nov 2025 12:40:26 +0300 Subject: [PATCH 03/11] wip Signed-off-by: Daniil Loktev --- .../controller/service/snapshot_service.go | 11 ++++-- .../pkg/controller/supplements/generator.go | 2 - .../vdsnapshot/internal/deletion.go | 2 +- .../vdsnapshot/internal/interfaces.go | 2 +- .../vdsnapshot/internal/life_cycle.go | 39 +------------------ .../vmrestore/internal/life_cycle.go | 4 +- .../vmsnapshot/internal/interfaces.go | 3 +- .../vmsnapshot/internal/life_cycle.go | 10 ++++- 8 files changed, 23 insertions(+), 50 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/service/snapshot_service.go b/images/virtualization-artifact/pkg/controller/service/snapshot_service.go index bcc45dce48..b5420eb64c 100644 --- a/images/virtualization-artifact/pkg/controller/service/snapshot_service.go +++ b/images/virtualization-artifact/pkg/controller/service/snapshot_service.go @@ -29,6 +29,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization/api/client/kubeclient" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" @@ -227,12 +228,14 @@ func (s *SnapshotService) GetVirtualMachine(ctx context.Context, name, namespace return object.FetchObject(ctx, types.NamespacedName{Namespace: namespace, Name: name}, s.client, &v1alpha2.VirtualMachine{}) } -func (s *SnapshotService) GetVolumeSnapshot(ctx context.Context, name, namespace string) (*vsv1.VolumeSnapshot, error) { - return object.FetchObject(ctx, types.NamespacedName{Namespace: namespace, Name: name}, s.client, &vsv1.VolumeSnapshot{}) +func (s *SnapshotService) GetVolumeSnapshot(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { + supGen := supplements.NewGenerator("vds", vdSnapshot.Name, vdSnapshot.Namespace, vdSnapshot.UID) + return supplements.FetchSupplement(ctx, s.client, supGen, supplements.SupplementSnapshot, &vsv1.VolumeSnapshot{}) } -func (s *SnapshotService) GetSecret(ctx context.Context, name, namespace string) (*corev1.Secret, error) { - return object.FetchObject(ctx, types.NamespacedName{Namespace: namespace, Name: name}, s.client, &corev1.Secret{}) +func (s *SnapshotService) GetSecret(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { + supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) + return supplements.FetchSupplement(ctx, s.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) } func (s *SnapshotService) CreateVirtualDiskSnapshot(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*v1alpha2.VirtualDiskSnapshot, error) { diff --git a/images/virtualization-artifact/pkg/controller/supplements/generator.go b/images/virtualization-artifact/pkg/controller/supplements/generator.go index 1fa103aa37..e7c925dc5c 100644 --- a/images/virtualization-artifact/pkg/controller/supplements/generator.go +++ b/images/virtualization-artifact/pkg/controller/supplements/generator.go @@ -180,7 +180,6 @@ func (g *generator) NetworkPolicy() types.NamespacedName { } // CommonSupplement generates name for common supplemental resources with d8v--- format. -// Used for snapshot-related resources (VMS Secret, VDS VolumeSnapshot). func (g *generator) CommonSupplement() types.NamespacedName { return g.generateName(tplCommon, kvalidation.DNS1123SubdomainMaxLength) } @@ -280,7 +279,6 @@ func (g *generator) LegacyPersistentVolumeClaim() types.NamespacedName { } // LegacySnapshotSupplement generates old format name for snapshot-related resources. -// Returns just the name without any prefix or UID (legacy naming). func (g *generator) LegacySnapshotSupplement() types.NamespacedName { return types.NamespacedName{ Name: g.name, diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go index 9731011350..bde5049bb5 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go @@ -43,7 +43,7 @@ func (h DeletionHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtua log := logger.FromContext(ctx).With(logger.SlogHandler(deletionHandlerName)) if vdSnapshot.DeletionTimestamp != nil { - vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot.Name, vdSnapshot.Namespace) + vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot) if err != nil { return reconcile.Result{}, err } diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go index 845deafc10..cfea6318a3 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go @@ -41,5 +41,5 @@ type LifeCycleSnapshotter interface { GetPersistentVolumeClaim(ctx context.Context, name, namespace string) (*corev1.PersistentVolumeClaim, error) GetVirtualDisk(ctx context.Context, name, namespace string) (*v1alpha2.VirtualDisk, error) GetVirtualMachine(ctx context.Context, name, namespace string) (*v1alpha2.VirtualMachine, error) - GetVolumeSnapshot(ctx context.Context, name, namespace string) (*vsv1.VolumeSnapshot, error) + GetVolumeSnapshot(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) } diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go index bf99c4db80..66d3118379 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go @@ -67,7 +67,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu conditions.SetCondition(cb, &vdSnapshot.Status.Conditions) }() - vs, err := h.getVolumeSnapshotWithFallback(ctx, vdSnapshot) + vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot) if err != nil { setPhaseConditionToFailed(cb, &vdSnapshot.Status.Phase, err) return reconcile.Result{}, err @@ -259,12 +259,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu } } - supGen := supplements.NewGenerator("vds", vdSnapshot.Name, vdSnapshot.Namespace, vdSnapshot.UID) - vsName, err := supplements.GetSupplementName(supGen, supplements.SupplementSnapshotCommon) - if err != nil { - setPhaseConditionToFailed(cb, &vdSnapshot.Status.Phase, err) - return reconcile.Result{}, err - } + vsName := supplements.NewGenerator("vds", vdSnapshot.Name, vdSnapshot.Namespace, vdSnapshot.UID).CommonSupplement() vs = &vsv1.VolumeSnapshot{ ObjectMeta: metav1.ObjectMeta{ @@ -420,33 +415,3 @@ func (h LifeCycleHandler) unfreezeFilesystemIfFailed(ctx context.Context, vdSnap return nil } - -func (h LifeCycleHandler) getVolumeSnapshotWithFallback(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { - supGen := supplements.NewGenerator("vds", vdSnapshot.Name, vdSnapshot.Namespace, vdSnapshot.UID) - - newVSName, err := supplements.GetSupplementName(supGen, supplements.SupplementSnapshotCommon) - if err != nil { - return nil, err - } - - vs, err := h.snapshotter.GetVolumeSnapshot(ctx, newVSName.Name, newVSName.Namespace) - if err == nil { - return vs, nil - } - - if !k8serrors.IsNotFound(err) { - return nil, fmt.Errorf("failed to get volume snapshot with new name %s: %w", newVSName.Name, err) - } - - legacyVSName, err := supplements.GetLegacySupplementName(supGen, supplements.SupplementSnapshotCommon) - if err != nil { - return nil, err - } - - vs, err = h.snapshotter.GetVolumeSnapshot(ctx, legacyVSName.Name, legacyVSName.Namespace) - if err != nil { - return nil, err - } - - return vs, nil -} diff --git a/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go index 1bb8dc7f87..e15ffb65bb 100644 --- a/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go @@ -35,6 +35,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" + "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization-controller/pkg/controller/vmrestore/internal/restorer" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -125,7 +126,8 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmRestore *v1alpha2.Virtua return reconcile.Result{}, err } - restorerSecret, err := h.restorer.Get(ctx, vmSnapshot) + supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) + restorerSecret, err := supplements.FetchSupplement(ctx, h.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) if err != nil { setPhaseConditionToFailed(cb, &vmRestore.Status.Phase, err) return reconcile.Result{}, err diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go index 16b19a4d30..7181d2d96c 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go @@ -27,12 +27,11 @@ import ( //go:generate go tool moq -rm -out mock.go . Storer Snapshotter type Storer interface { - Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) Store(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) } type Snapshotter interface { - GetSecret(ctx context.Context, name, namespace string) (*corev1.Secret, error) + getSecret(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) GetVirtualMachine(ctx context.Context, name, namespace string) (*v1alpha2.VirtualMachine, error) GetVirtualDisk(ctx context.Context, name, namespace string) (*v1alpha2.VirtualDisk, error) GetPersistentVolumeClaim(ctx context.Context, name, namespace string) (*corev1.PersistentVolumeClaim, error) diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go index acb2d46802..054c89c5ab 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go @@ -599,8 +599,14 @@ func (h LifeCycleHandler) ensureSecret(ctx context.Context, vm *v1alpha2.Virtual var secret *corev1.Secret var err error - secret, err = h.storer.Get(ctx, vmSnapshot) - if err != nil { + if vmSnapshot.Status.VirtualMachineSnapshotSecretName != "" { + secret, err = h.snapshotter.GetSecret(ctx, vmSnapshot) + if err != nil { + return err + } + } + + if secret == nil { secret, err = h.storer.Store(ctx, vm, vmSnapshot) if err != nil { return err From 12a3bbdbd23868b5bdeeb57e9399fb23865ae8f1 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 24 Nov 2025 13:33:41 +0300 Subject: [PATCH 04/11] wip Signed-off-by: Daniil Loktev --- .../service/restorer/secret_restorer.go | 5 -- .../vdsnapshot/internal/life_cycle.go | 1 - .../vdsnapshot/internal/life_cycle_test.go | 12 +-- .../controller/vdsnapshot/internal/mock.go | 34 ++++---- .../vmrestore/internal/interfaces.go | 1 - .../pkg/controller/vmrestore/internal/mock.go | 50 ----------- .../vmsnapshot/internal/interfaces.go | 2 +- .../vmsnapshot/internal/life_cycle_test.go | 5 +- .../controller/vmsnapshot/internal/mock.go | 84 ++++--------------- 9 files changed, 36 insertions(+), 158 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go index 4b07880f1c..22742f0791 100644 --- a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go +++ b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go @@ -44,11 +44,6 @@ func NewSecretRestorer(client client.Client) *SecretRestorer { } } -func (r SecretRestorer) Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { - supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) - return supplements.FetchSupplement(ctx, r.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) -} - func (r SecretRestorer) Store(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) secretName := supGen.CommonSupplement() diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go index 66d3118379..ea34b21642 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go @@ -24,7 +24,6 @@ import ( vsv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" corev1 "k8s.io/api/core/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/reconcile" diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go index 56200e6019..207eb2e801 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go @@ -86,7 +86,7 @@ var _ = Describe("LifeCycle handler", func() { GetVirtualMachineFunc: func(_ context.Context, _, _ string) (*v1alpha2.VirtualMachine, error) { return nil, nil }, - GetVolumeSnapshotFunc: func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { + GetVolumeSnapshotFunc: func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { return nil, nil }, } @@ -106,7 +106,7 @@ var _ = Describe("LifeCycle handler", func() { }) It("The volume snapshot has failed", func() { - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { vs.Status = &vsv1.VolumeSnapshotStatus{ Error: &vsv1.VolumeSnapshotError{ Message: ptr.To("error"), @@ -127,7 +127,7 @@ var _ = Describe("LifeCycle handler", func() { }) It("The volume snapshot is not ready yet", func() { - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { return vs, nil } @@ -143,7 +143,7 @@ var _ = Describe("LifeCycle handler", func() { }) It("The volume snapshot is ready", func() { - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { vs.Status = &vsv1.VolumeSnapshotStatus{ ReadyToUse: ptr.To(true), } @@ -260,7 +260,7 @@ var _ = Describe("LifeCycle handler", func() { snapshotter.IsFrozenFunc = func(_ *v1alpha2.VirtualMachine) bool { return true } - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { vs.Status = &vsv1.VolumeSnapshotStatus{ ReadyToUse: ptr.To(true), } @@ -283,7 +283,7 @@ var _ = Describe("LifeCycle handler", func() { snapshotter.IsFrozenFunc = func(_ *v1alpha2.VirtualMachine) bool { return true } - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { vs.Status = &vsv1.VolumeSnapshotStatus{ ReadyToUse: ptr.To(true), } diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go index a063e0df10..a4e5804f55 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go @@ -120,7 +120,7 @@ var _ LifeCycleSnapshotter = &LifeCycleSnapshotterMock{} // GetVirtualMachineFunc: func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualMachine, error) { // panic("mock out the GetVirtualMachine method") // }, -// GetVolumeSnapshotFunc: func(ctx context.Context, name string, namespace string) (*vsv1.VolumeSnapshot, error) { +// GetVolumeSnapshotFunc: func(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { // panic("mock out the GetVolumeSnapshot method") // }, // IsFrozenFunc: func(vm *v1alpha2.VirtualMachine) bool { @@ -158,7 +158,7 @@ type LifeCycleSnapshotterMock struct { GetVirtualMachineFunc func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualMachine, error) // GetVolumeSnapshotFunc mocks the GetVolumeSnapshot method. - GetVolumeSnapshotFunc func(ctx context.Context, name string, namespace string) (*vsv1.VolumeSnapshot, error) + GetVolumeSnapshotFunc func(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) // IsFrozenFunc mocks the IsFrozen method. IsFrozenFunc func(vm *v1alpha2.VirtualMachine) bool @@ -229,10 +229,8 @@ type LifeCycleSnapshotterMock struct { GetVolumeSnapshot []struct { // Ctx is the ctx argument value. Ctx context.Context - // Name is the name argument value. - Name string - // Namespace is the namespace argument value. - Namespace string + // VdSnapshot is the vdSnapshot argument value. + VdSnapshot *v1alpha2.VirtualDiskSnapshot } // IsFrozen holds details about calls to the IsFrozen method. IsFrozen []struct { @@ -530,23 +528,21 @@ func (mock *LifeCycleSnapshotterMock) GetVirtualMachineCalls() []struct { } // GetVolumeSnapshot calls GetVolumeSnapshotFunc. -func (mock *LifeCycleSnapshotterMock) GetVolumeSnapshot(ctx context.Context, name string, namespace string) (*vsv1.VolumeSnapshot, error) { +func (mock *LifeCycleSnapshotterMock) GetVolumeSnapshot(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { if mock.GetVolumeSnapshotFunc == nil { panic("LifeCycleSnapshotterMock.GetVolumeSnapshotFunc: method is nil but LifeCycleSnapshotter.GetVolumeSnapshot was just called") } callInfo := struct { - Ctx context.Context - Name string - Namespace string + Ctx context.Context + VdSnapshot *v1alpha2.VirtualDiskSnapshot }{ - Ctx: ctx, - Name: name, - Namespace: namespace, + Ctx: ctx, + VdSnapshot: vdSnapshot, } mock.lockGetVolumeSnapshot.Lock() mock.calls.GetVolumeSnapshot = append(mock.calls.GetVolumeSnapshot, callInfo) mock.lockGetVolumeSnapshot.Unlock() - return mock.GetVolumeSnapshotFunc(ctx, name, namespace) + return mock.GetVolumeSnapshotFunc(ctx, vdSnapshot) } // GetVolumeSnapshotCalls gets all the calls that were made to GetVolumeSnapshot. @@ -554,14 +550,12 @@ func (mock *LifeCycleSnapshotterMock) GetVolumeSnapshot(ctx context.Context, nam // // len(mockedLifeCycleSnapshotter.GetVolumeSnapshotCalls()) func (mock *LifeCycleSnapshotterMock) GetVolumeSnapshotCalls() []struct { - Ctx context.Context - Name string - Namespace string + Ctx context.Context + VdSnapshot *v1alpha2.VirtualDiskSnapshot } { var calls []struct { - Ctx context.Context - Name string - Namespace string + Ctx context.Context + VdSnapshot *v1alpha2.VirtualDiskSnapshot } mock.lockGetVolumeSnapshot.RLock() calls = mock.calls.GetVolumeSnapshot diff --git a/images/virtualization-artifact/pkg/controller/vmrestore/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vmrestore/internal/interfaces.go index 7af372753c..3e72cf4467 100644 --- a/images/virtualization-artifact/pkg/controller/vmrestore/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vmrestore/internal/interfaces.go @@ -27,7 +27,6 @@ import ( //go:generate go tool moq -rm -out mock.go . Restorer type Restorer interface { - Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) RestoreVirtualMachine(ctx context.Context, secret *corev1.Secret) (*v1alpha2.VirtualMachine, error) RestoreProvisioner(ctx context.Context, secret *corev1.Secret) (*corev1.Secret, error) RestoreVirtualMachineIPAddress(ctx context.Context, secret *corev1.Secret) (*v1alpha2.VirtualMachineIPAddress, error) diff --git a/images/virtualization-artifact/pkg/controller/vmrestore/internal/mock.go b/images/virtualization-artifact/pkg/controller/vmrestore/internal/mock.go index f5724e703c..4165a3dd57 100644 --- a/images/virtualization-artifact/pkg/controller/vmrestore/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vmrestore/internal/mock.go @@ -20,9 +20,6 @@ var _ Restorer = &RestorerMock{} // // // make and configure a mocked Restorer // mockedRestorer := &RestorerMock{ -// GetFunc: func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { -// panic("mock out the Get method") -// }, // RestoreMACAddressOrderFunc: func(ctx context.Context, secret *corev1.Secret) ([]string, error) { // panic("mock out the RestoreMACAddressOrder method") // }, @@ -48,9 +45,6 @@ var _ Restorer = &RestorerMock{} // // } type RestorerMock struct { - // GetFunc mocks the Get method. - GetFunc func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) - // RestoreMACAddressOrderFunc mocks the RestoreMACAddressOrder method. RestoreMACAddressOrderFunc func(ctx context.Context, secret *corev1.Secret) ([]string, error) @@ -71,13 +65,6 @@ type RestorerMock struct { // calls tracks calls to the methods. calls struct { - // Get holds details about calls to the Get method. - Get []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // VmSnapshot is the vmSnapshot argument value. - VmSnapshot *v1alpha2.VirtualMachineSnapshot - } // RestoreMACAddressOrder holds details about calls to the RestoreMACAddressOrder method. RestoreMACAddressOrder []struct { // Ctx is the ctx argument value. @@ -121,7 +108,6 @@ type RestorerMock struct { Secret *corev1.Secret } } - lockGet sync.RWMutex lockRestoreMACAddressOrder sync.RWMutex lockRestoreProvisioner sync.RWMutex lockRestoreVirtualMachine sync.RWMutex @@ -130,42 +116,6 @@ type RestorerMock struct { lockRestoreVirtualMachineMACAddresses sync.RWMutex } -// Get calls GetFunc. -func (mock *RestorerMock) Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { - if mock.GetFunc == nil { - panic("RestorerMock.GetFunc: method is nil but Restorer.Get was just called") - } - callInfo := struct { - Ctx context.Context - VmSnapshot *v1alpha2.VirtualMachineSnapshot - }{ - Ctx: ctx, - VmSnapshot: vmSnapshot, - } - mock.lockGet.Lock() - mock.calls.Get = append(mock.calls.Get, callInfo) - mock.lockGet.Unlock() - return mock.GetFunc(ctx, vmSnapshot) -} - -// GetCalls gets all the calls that were made to Get. -// Check the length with: -// -// len(mockedRestorer.GetCalls()) -func (mock *RestorerMock) GetCalls() []struct { - Ctx context.Context - VmSnapshot *v1alpha2.VirtualMachineSnapshot -} { - var calls []struct { - Ctx context.Context - VmSnapshot *v1alpha2.VirtualMachineSnapshot - } - mock.lockGet.RLock() - calls = mock.calls.Get - mock.lockGet.RUnlock() - return calls -} - // RestoreMACAddressOrder calls RestoreMACAddressOrderFunc. func (mock *RestorerMock) RestoreMACAddressOrder(ctx context.Context, secret *corev1.Secret) ([]string, error) { if mock.RestoreMACAddressOrderFunc == nil { diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go index 7181d2d96c..8b1401a705 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go @@ -31,7 +31,7 @@ type Storer interface { } type Snapshotter interface { - getSecret(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) + GetSecret(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) GetVirtualMachine(ctx context.Context, name, namespace string) (*v1alpha2.VirtualMachine, error) GetVirtualDisk(ctx context.Context, name, namespace string) (*v1alpha2.VirtualDisk, error) GetPersistentVolumeClaim(ctx context.Context, name, namespace string) (*corev1.PersistentVolumeClaim, error) diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go index 42f4d4fba1..46bdddd5d3 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go @@ -136,7 +136,7 @@ var _ = Describe("LifeCycle handler", func() { UnfreezeFunc: func(ctx context.Context, _, _ string) error { return nil }, - GetSecretFunc: func(_ context.Context, _, _ string) (*corev1.Secret, error) { + GetSecretFunc: func(_ context.Context, _ *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { return secret, nil }, GetVirtualDiskSnapshotFunc: func(_ context.Context, _, _ string) (*v1alpha2.VirtualDiskSnapshot, error) { @@ -145,9 +145,6 @@ var _ = Describe("LifeCycle handler", func() { } storer = &StorerMock{ - GetFunc: func(_ context.Context, _ *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { - return secret, nil - }, StoreFunc: func(_ context.Context, _ *v1alpha2.VirtualMachine, _ *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { return secret, nil }, diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go index 6c380752c6..245d71c4fa 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go @@ -20,9 +20,6 @@ var _ Storer = &StorerMock{} // // // make and configure a mocked Storer // mockedStorer := &StorerMock{ -// GetFunc: func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { -// panic("mock out the Get method") -// }, // StoreFunc: func(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { // panic("mock out the Store method") // }, @@ -33,21 +30,11 @@ var _ Storer = &StorerMock{} // // } type StorerMock struct { - // GetFunc mocks the Get method. - GetFunc func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) - // StoreFunc mocks the Store method. StoreFunc func(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) // calls tracks calls to the methods. calls struct { - // Get holds details about calls to the Get method. - Get []struct { - // Ctx is the ctx argument value. - Ctx context.Context - // VmSnapshot is the vmSnapshot argument value. - VmSnapshot *v1alpha2.VirtualMachineSnapshot - } // Store holds details about calls to the Store method. Store []struct { // Ctx is the ctx argument value. @@ -58,46 +45,9 @@ type StorerMock struct { VmSnapshot *v1alpha2.VirtualMachineSnapshot } } - lockGet sync.RWMutex lockStore sync.RWMutex } -// Get calls GetFunc. -func (mock *StorerMock) Get(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { - if mock.GetFunc == nil { - panic("StorerMock.GetFunc: method is nil but Storer.Get was just called") - } - callInfo := struct { - Ctx context.Context - VmSnapshot *v1alpha2.VirtualMachineSnapshot - }{ - Ctx: ctx, - VmSnapshot: vmSnapshot, - } - mock.lockGet.Lock() - mock.calls.Get = append(mock.calls.Get, callInfo) - mock.lockGet.Unlock() - return mock.GetFunc(ctx, vmSnapshot) -} - -// GetCalls gets all the calls that were made to Get. -// Check the length with: -// -// len(mockedStorer.GetCalls()) -func (mock *StorerMock) GetCalls() []struct { - Ctx context.Context - VmSnapshot *v1alpha2.VirtualMachineSnapshot -} { - var calls []struct { - Ctx context.Context - VmSnapshot *v1alpha2.VirtualMachineSnapshot - } - mock.lockGet.RLock() - calls = mock.calls.Get - mock.lockGet.RUnlock() - return calls -} - // Store calls StoreFunc. func (mock *StorerMock) Store(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { if mock.StoreFunc == nil { @@ -163,7 +113,7 @@ var _ Snapshotter = &SnapshotterMock{} // GetPersistentVolumeClaimFunc: func(ctx context.Context, name string, namespace string) (*corev1.PersistentVolumeClaim, error) { // panic("mock out the GetPersistentVolumeClaim method") // }, -// GetSecretFunc: func(ctx context.Context, name string, namespace string) (*corev1.Secret, error) { +// GetSecretFunc: func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { // panic("mock out the GetSecret method") // }, // GetVirtualDiskFunc: func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualDisk, error) { @@ -204,7 +154,7 @@ type SnapshotterMock struct { GetPersistentVolumeClaimFunc func(ctx context.Context, name string, namespace string) (*corev1.PersistentVolumeClaim, error) // GetSecretFunc mocks the GetSecret method. - GetSecretFunc func(ctx context.Context, name string, namespace string) (*corev1.Secret, error) + GetSecretFunc func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) // GetVirtualDiskFunc mocks the GetVirtualDisk method. GetVirtualDiskFunc func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualDisk, error) @@ -266,10 +216,8 @@ type SnapshotterMock struct { GetSecret []struct { // Ctx is the ctx argument value. Ctx context.Context - // Name is the name argument value. - Name string - // Namespace is the namespace argument value. - Namespace string + // VmSnapshot is the vmSnapshot argument value. + VmSnapshot *v1alpha2.VirtualMachineSnapshot } // GetVirtualDisk holds details about calls to the GetVirtualDisk method. GetVirtualDisk []struct { @@ -515,23 +463,21 @@ func (mock *SnapshotterMock) GetPersistentVolumeClaimCalls() []struct { } // GetSecret calls GetSecretFunc. -func (mock *SnapshotterMock) GetSecret(ctx context.Context, name string, namespace string) (*corev1.Secret, error) { +func (mock *SnapshotterMock) GetSecret(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { if mock.GetSecretFunc == nil { panic("SnapshotterMock.GetSecretFunc: method is nil but Snapshotter.GetSecret was just called") } callInfo := struct { - Ctx context.Context - Name string - Namespace string + Ctx context.Context + VmSnapshot *v1alpha2.VirtualMachineSnapshot }{ - Ctx: ctx, - Name: name, - Namespace: namespace, + Ctx: ctx, + VmSnapshot: vmSnapshot, } mock.lockGetSecret.Lock() mock.calls.GetSecret = append(mock.calls.GetSecret, callInfo) mock.lockGetSecret.Unlock() - return mock.GetSecretFunc(ctx, name, namespace) + return mock.GetSecretFunc(ctx, vmSnapshot) } // GetSecretCalls gets all the calls that were made to GetSecret. @@ -539,14 +485,12 @@ func (mock *SnapshotterMock) GetSecret(ctx context.Context, name string, namespa // // len(mockedSnapshotter.GetSecretCalls()) func (mock *SnapshotterMock) GetSecretCalls() []struct { - Ctx context.Context - Name string - Namespace string + Ctx context.Context + VmSnapshot *v1alpha2.VirtualMachineSnapshot } { var calls []struct { - Ctx context.Context - Name string - Namespace string + Ctx context.Context + VmSnapshot *v1alpha2.VirtualMachineSnapshot } mock.lockGetSecret.RLock() calls = mock.calls.GetSecret From bd493b899278d44753a641c73aa1116326d6d851 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 24 Nov 2025 14:18:39 +0300 Subject: [PATCH 05/11] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/service/restorer/secret_restorer.go | 3 +-- .../vmop/snapshot/internal/step/process_clone_step.go | 5 +++-- .../controller/vmop/snapshot/internal/step/process_step.go | 5 +++-- .../controller/vmrestore/internal/watcher/vmbda_watcher.go | 6 +++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go index 22742f0791..553673da1e 100644 --- a/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go +++ b/images/virtualization-artifact/pkg/controller/service/restorer/secret_restorer.go @@ -45,8 +45,7 @@ func NewSecretRestorer(client client.Client) *SecretRestorer { } func (r SecretRestorer) Store(ctx context.Context, vm *v1alpha2.VirtualMachine, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { - supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) - secretName := supGen.CommonSupplement() + secretName := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID).CommonSupplement() secret := corev1.Secret{ TypeMeta: metav1.TypeMeta{ diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go index 08dcb57d9c..a6a72ca10b 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go @@ -31,6 +31,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/service/restorer" + "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -82,8 +83,8 @@ func (s ProcessCloneStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachin return &reconcile.Result{}, err } - restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} - restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, s.client, &corev1.Secret{}) + supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) + restorerSecret, err := supplements.FetchSupplement(ctx, s.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) if err != nil { common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go index d86237cd10..e24853fd63 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go @@ -28,6 +28,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service/restorer" + "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -72,8 +73,8 @@ func (s ProcessRestoreStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach return &reconcile.Result{}, err } - restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} - restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, s.client, &corev1.Secret{}) + supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) + restorerSecret, err := supplements.FetchSupplement(ctx, s.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) if err != nil { common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err diff --git a/images/virtualization-artifact/pkg/controller/vmrestore/internal/watcher/vmbda_watcher.go b/images/virtualization-artifact/pkg/controller/vmrestore/internal/watcher/vmbda_watcher.go index f57362d437..d4d8d802e0 100644 --- a/images/virtualization-artifact/pkg/controller/vmrestore/internal/watcher/vmbda_watcher.go +++ b/images/virtualization-artifact/pkg/controller/vmrestore/internal/watcher/vmbda_watcher.go @@ -30,7 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" "github.com/deckhouse/deckhouse/pkg/log" - "github.com/deckhouse/virtualization-controller/pkg/common/object" + "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" vmrestore "github.com/deckhouse/virtualization-controller/pkg/controller/vmrestore/internal" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -81,8 +81,8 @@ func (w VirtualMachineBlockDeviceAttachmentWatcher) enqueueRequests(ctx context. continue } - restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} - restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, w.client, &corev1.Secret{}) + supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) + restorerSecret, err := supplements.FetchSupplement(ctx, w.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) if err != nil { log.Error(fmt.Sprintf("failed to get virtualMachineSnapshotSecret: %s", err)) return From d51f94de558824b367bed3d25f08b9d5df5115e6 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 24 Nov 2025 14:36:39 +0300 Subject: [PATCH 06/11] wip Signed-off-by: Daniil Loktev --- .../controller/service/snapshot_service.go | 11 +++--- .../pkg/controller/supplements/fetch.go | 11 ------ .../vdsnapshot/internal/deletion.go | 2 +- .../vdsnapshot/internal/interfaces.go | 2 +- .../vdsnapshot/internal/life_cycle.go | 2 +- .../controller/vdsnapshot/internal/mock.go | 34 +++++++++++-------- .../internal/step/process_clone_step.go | 5 ++- .../snapshot/internal/step/process_step.go | 5 ++- .../vmrestore/internal/life_cycle.go | 5 ++- .../internal/watcher/vmbda_watcher.go | 6 ++-- .../vmsnapshot/internal/interfaces.go | 2 +- .../vmsnapshot/internal/life_cycle.go | 2 +- .../controller/vmsnapshot/internal/mock.go | 34 +++++++++++-------- 13 files changed, 58 insertions(+), 63 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/service/snapshot_service.go b/images/virtualization-artifact/pkg/controller/service/snapshot_service.go index b5420eb64c..bcc45dce48 100644 --- a/images/virtualization-artifact/pkg/controller/service/snapshot_service.go +++ b/images/virtualization-artifact/pkg/controller/service/snapshot_service.go @@ -29,7 +29,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization/api/client/kubeclient" "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" @@ -228,14 +227,12 @@ func (s *SnapshotService) GetVirtualMachine(ctx context.Context, name, namespace return object.FetchObject(ctx, types.NamespacedName{Namespace: namespace, Name: name}, s.client, &v1alpha2.VirtualMachine{}) } -func (s *SnapshotService) GetVolumeSnapshot(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { - supGen := supplements.NewGenerator("vds", vdSnapshot.Name, vdSnapshot.Namespace, vdSnapshot.UID) - return supplements.FetchSupplement(ctx, s.client, supGen, supplements.SupplementSnapshot, &vsv1.VolumeSnapshot{}) +func (s *SnapshotService) GetVolumeSnapshot(ctx context.Context, name, namespace string) (*vsv1.VolumeSnapshot, error) { + return object.FetchObject(ctx, types.NamespacedName{Namespace: namespace, Name: name}, s.client, &vsv1.VolumeSnapshot{}) } -func (s *SnapshotService) GetSecret(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { - supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) - return supplements.FetchSupplement(ctx, s.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) +func (s *SnapshotService) GetSecret(ctx context.Context, name, namespace string) (*corev1.Secret, error) { + return object.FetchObject(ctx, types.NamespacedName{Namespace: namespace, Name: name}, s.client, &corev1.Secret{}) } func (s *SnapshotService) CreateVirtualDiskSnapshot(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*v1alpha2.VirtualDiskSnapshot, error) { diff --git a/images/virtualization-artifact/pkg/controller/supplements/fetch.go b/images/virtualization-artifact/pkg/controller/supplements/fetch.go index 7bdbee5cf4..1b150f9e93 100644 --- a/images/virtualization-artifact/pkg/controller/supplements/fetch.go +++ b/images/virtualization-artifact/pkg/controller/supplements/fetch.go @@ -49,9 +49,6 @@ const ( SupplementCABundleConfigMap SupplementType = "CABundleConfigMap" SupplementImagePullSecret SupplementType = "ImagePullSecret" SupplementUploaderTLSSecret SupplementType = "UploaderTLSSecret" - - // Snapshots - SupplementSnapshot SupplementType = "Snapshot" ) // GetSupplementName returns the name for the requested supplement type @@ -91,10 +88,6 @@ func GetSupplementName(gen Generator, supplementType SupplementType) (types.Name case SupplementUploaderTLSSecret: return gen.UploaderTLSSecretForIngress(), nil - // Snapshots - case SupplementSnapshot: - return gen.CommonSupplement(), nil - default: return types.NamespacedName{}, fmt.Errorf("unknown supplement type: %s", supplementType) } @@ -137,10 +130,6 @@ func GetLegacySupplementName(gen Generator, supplementType SupplementType) (type case SupplementUploaderTLSSecret: return gen.LegacyUploaderTLSSecretForIngress(), nil - // Snapshots - case SupplementSnapshot: - return gen.LegacySnapshotSupplement(), nil - default: return types.NamespacedName{}, fmt.Errorf("unknown supplement type: %s", supplementType) } diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go index bde5049bb5..76e6baf2b1 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/deletion.go @@ -43,7 +43,7 @@ func (h DeletionHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtua log := logger.FromContext(ctx).With(logger.SlogHandler(deletionHandlerName)) if vdSnapshot.DeletionTimestamp != nil { - vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot) + vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot.Status.VolumeSnapshotName, vdSnapshot.Namespace) if err != nil { return reconcile.Result{}, err } diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go index cfea6318a3..845deafc10 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/interfaces.go @@ -41,5 +41,5 @@ type LifeCycleSnapshotter interface { GetPersistentVolumeClaim(ctx context.Context, name, namespace string) (*corev1.PersistentVolumeClaim, error) GetVirtualDisk(ctx context.Context, name, namespace string) (*v1alpha2.VirtualDisk, error) GetVirtualMachine(ctx context.Context, name, namespace string) (*v1alpha2.VirtualMachine, error) - GetVolumeSnapshot(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) + GetVolumeSnapshot(ctx context.Context, name, namespace string) (*vsv1.VolumeSnapshot, error) } diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go index ea34b21642..882af58917 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go @@ -66,7 +66,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu conditions.SetCondition(cb, &vdSnapshot.Status.Conditions) }() - vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot) + vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot.Status.VolumeSnapshotName, vdSnapshot.Namespace) if err != nil { setPhaseConditionToFailed(cb, &vdSnapshot.Status.Phase, err) return reconcile.Result{}, err diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go index a4e5804f55..a063e0df10 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/mock.go @@ -120,7 +120,7 @@ var _ LifeCycleSnapshotter = &LifeCycleSnapshotterMock{} // GetVirtualMachineFunc: func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualMachine, error) { // panic("mock out the GetVirtualMachine method") // }, -// GetVolumeSnapshotFunc: func(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { +// GetVolumeSnapshotFunc: func(ctx context.Context, name string, namespace string) (*vsv1.VolumeSnapshot, error) { // panic("mock out the GetVolumeSnapshot method") // }, // IsFrozenFunc: func(vm *v1alpha2.VirtualMachine) bool { @@ -158,7 +158,7 @@ type LifeCycleSnapshotterMock struct { GetVirtualMachineFunc func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualMachine, error) // GetVolumeSnapshotFunc mocks the GetVolumeSnapshot method. - GetVolumeSnapshotFunc func(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) + GetVolumeSnapshotFunc func(ctx context.Context, name string, namespace string) (*vsv1.VolumeSnapshot, error) // IsFrozenFunc mocks the IsFrozen method. IsFrozenFunc func(vm *v1alpha2.VirtualMachine) bool @@ -229,8 +229,10 @@ type LifeCycleSnapshotterMock struct { GetVolumeSnapshot []struct { // Ctx is the ctx argument value. Ctx context.Context - // VdSnapshot is the vdSnapshot argument value. - VdSnapshot *v1alpha2.VirtualDiskSnapshot + // Name is the name argument value. + Name string + // Namespace is the namespace argument value. + Namespace string } // IsFrozen holds details about calls to the IsFrozen method. IsFrozen []struct { @@ -528,21 +530,23 @@ func (mock *LifeCycleSnapshotterMock) GetVirtualMachineCalls() []struct { } // GetVolumeSnapshot calls GetVolumeSnapshotFunc. -func (mock *LifeCycleSnapshotterMock) GetVolumeSnapshot(ctx context.Context, vdSnapshot *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { +func (mock *LifeCycleSnapshotterMock) GetVolumeSnapshot(ctx context.Context, name string, namespace string) (*vsv1.VolumeSnapshot, error) { if mock.GetVolumeSnapshotFunc == nil { panic("LifeCycleSnapshotterMock.GetVolumeSnapshotFunc: method is nil but LifeCycleSnapshotter.GetVolumeSnapshot was just called") } callInfo := struct { - Ctx context.Context - VdSnapshot *v1alpha2.VirtualDiskSnapshot + Ctx context.Context + Name string + Namespace string }{ - Ctx: ctx, - VdSnapshot: vdSnapshot, + Ctx: ctx, + Name: name, + Namespace: namespace, } mock.lockGetVolumeSnapshot.Lock() mock.calls.GetVolumeSnapshot = append(mock.calls.GetVolumeSnapshot, callInfo) mock.lockGetVolumeSnapshot.Unlock() - return mock.GetVolumeSnapshotFunc(ctx, vdSnapshot) + return mock.GetVolumeSnapshotFunc(ctx, name, namespace) } // GetVolumeSnapshotCalls gets all the calls that were made to GetVolumeSnapshot. @@ -550,12 +554,14 @@ func (mock *LifeCycleSnapshotterMock) GetVolumeSnapshot(ctx context.Context, vdS // // len(mockedLifeCycleSnapshotter.GetVolumeSnapshotCalls()) func (mock *LifeCycleSnapshotterMock) GetVolumeSnapshotCalls() []struct { - Ctx context.Context - VdSnapshot *v1alpha2.VirtualDiskSnapshot + Ctx context.Context + Name string + Namespace string } { var calls []struct { - Ctx context.Context - VdSnapshot *v1alpha2.VirtualDiskSnapshot + Ctx context.Context + Name string + Namespace string } mock.lockGetVolumeSnapshot.RLock() calls = mock.calls.GetVolumeSnapshot diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go index a6a72ca10b..08dcb57d9c 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_clone_step.go @@ -31,7 +31,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/service/restorer" - "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -83,8 +82,8 @@ func (s ProcessCloneStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMachin return &reconcile.Result{}, err } - supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) - restorerSecret, err := supplements.FetchSupplement(ctx, s.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) + restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} + restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, s.client, &corev1.Secret{}) if err != nil { common.SetPhaseCloneConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err diff --git a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go index e24853fd63..d86237cd10 100644 --- a/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go +++ b/images/virtualization-artifact/pkg/controller/vmop/snapshot/internal/step/process_step.go @@ -28,7 +28,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service/restorer" - "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization-controller/pkg/controller/vmop/snapshot/internal/common" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -73,8 +72,8 @@ func (s ProcessRestoreStep) Take(ctx context.Context, vmop *v1alpha2.VirtualMach return &reconcile.Result{}, err } - supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) - restorerSecret, err := supplements.FetchSupplement(ctx, s.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) + restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} + restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, s.client, &corev1.Secret{}) if err != nil { common.SetPhaseConditionToFailed(s.cb, &vmop.Status.Phase, err) return &reconcile.Result{}, err diff --git a/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go index e15ffb65bb..e8751ca91f 100644 --- a/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vmrestore/internal/life_cycle.go @@ -35,7 +35,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" - "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" "github.com/deckhouse/virtualization-controller/pkg/controller/vmrestore/internal/restorer" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization/api/core/v1alpha2" @@ -126,8 +125,8 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vmRestore *v1alpha2.Virtua return reconcile.Result{}, err } - supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) - restorerSecret, err := supplements.FetchSupplement(ctx, h.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) + restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} + restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, h.client, &corev1.Secret{}) if err != nil { setPhaseConditionToFailed(cb, &vmRestore.Status.Phase, err) return reconcile.Result{}, err diff --git a/images/virtualization-artifact/pkg/controller/vmrestore/internal/watcher/vmbda_watcher.go b/images/virtualization-artifact/pkg/controller/vmrestore/internal/watcher/vmbda_watcher.go index d4d8d802e0..f57362d437 100644 --- a/images/virtualization-artifact/pkg/controller/vmrestore/internal/watcher/vmbda_watcher.go +++ b/images/virtualization-artifact/pkg/controller/vmrestore/internal/watcher/vmbda_watcher.go @@ -30,7 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" "github.com/deckhouse/deckhouse/pkg/log" - "github.com/deckhouse/virtualization-controller/pkg/controller/supplements" + "github.com/deckhouse/virtualization-controller/pkg/common/object" vmrestore "github.com/deckhouse/virtualization-controller/pkg/controller/vmrestore/internal" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -81,8 +81,8 @@ func (w VirtualMachineBlockDeviceAttachmentWatcher) enqueueRequests(ctx context. continue } - supGen := supplements.NewGenerator("vms", vmSnapshot.Name, vmSnapshot.Namespace, vmSnapshot.UID) - restorerSecret, err := supplements.FetchSupplement(ctx, w.client, supGen, supplements.SupplementSnapshot, &corev1.Secret{}) + restorerSecretKey := types.NamespacedName{Namespace: vmSnapshot.Namespace, Name: vmSnapshot.Status.VirtualMachineSnapshotSecretName} + restorerSecret, err := object.FetchObject(ctx, restorerSecretKey, w.client, &corev1.Secret{}) if err != nil { log.Error(fmt.Sprintf("failed to get virtualMachineSnapshotSecret: %s", err)) return diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go index 8b1401a705..6690332a57 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/interfaces.go @@ -31,7 +31,7 @@ type Storer interface { } type Snapshotter interface { - GetSecret(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) + GetSecret(ctx context.Context, name, namespace string) (*corev1.Secret, error) GetVirtualMachine(ctx context.Context, name, namespace string) (*v1alpha2.VirtualMachine, error) GetVirtualDisk(ctx context.Context, name, namespace string) (*v1alpha2.VirtualDisk, error) GetPersistentVolumeClaim(ctx context.Context, name, namespace string) (*corev1.PersistentVolumeClaim, error) diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go index 054c89c5ab..42219dfdbd 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle.go @@ -600,7 +600,7 @@ func (h LifeCycleHandler) ensureSecret(ctx context.Context, vm *v1alpha2.Virtual var err error if vmSnapshot.Status.VirtualMachineSnapshotSecretName != "" { - secret, err = h.snapshotter.GetSecret(ctx, vmSnapshot) + secret, err = h.snapshotter.GetSecret(ctx, vmSnapshot.Status.VirtualMachineSnapshotSecretName, vmSnapshot.Namespace) if err != nil { return err } diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go index 245d71c4fa..134a491adb 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/mock.go @@ -113,7 +113,7 @@ var _ Snapshotter = &SnapshotterMock{} // GetPersistentVolumeClaimFunc: func(ctx context.Context, name string, namespace string) (*corev1.PersistentVolumeClaim, error) { // panic("mock out the GetPersistentVolumeClaim method") // }, -// GetSecretFunc: func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { +// GetSecretFunc: func(ctx context.Context, name string, namespace string) (*corev1.Secret, error) { // panic("mock out the GetSecret method") // }, // GetVirtualDiskFunc: func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualDisk, error) { @@ -154,7 +154,7 @@ type SnapshotterMock struct { GetPersistentVolumeClaimFunc func(ctx context.Context, name string, namespace string) (*corev1.PersistentVolumeClaim, error) // GetSecretFunc mocks the GetSecret method. - GetSecretFunc func(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) + GetSecretFunc func(ctx context.Context, name string, namespace string) (*corev1.Secret, error) // GetVirtualDiskFunc mocks the GetVirtualDisk method. GetVirtualDiskFunc func(ctx context.Context, name string, namespace string) (*v1alpha2.VirtualDisk, error) @@ -216,8 +216,10 @@ type SnapshotterMock struct { GetSecret []struct { // Ctx is the ctx argument value. Ctx context.Context - // VmSnapshot is the vmSnapshot argument value. - VmSnapshot *v1alpha2.VirtualMachineSnapshot + // Name is the name argument value. + Name string + // Namespace is the namespace argument value. + Namespace string } // GetVirtualDisk holds details about calls to the GetVirtualDisk method. GetVirtualDisk []struct { @@ -463,21 +465,23 @@ func (mock *SnapshotterMock) GetPersistentVolumeClaimCalls() []struct { } // GetSecret calls GetSecretFunc. -func (mock *SnapshotterMock) GetSecret(ctx context.Context, vmSnapshot *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { +func (mock *SnapshotterMock) GetSecret(ctx context.Context, name string, namespace string) (*corev1.Secret, error) { if mock.GetSecretFunc == nil { panic("SnapshotterMock.GetSecretFunc: method is nil but Snapshotter.GetSecret was just called") } callInfo := struct { - Ctx context.Context - VmSnapshot *v1alpha2.VirtualMachineSnapshot + Ctx context.Context + Name string + Namespace string }{ - Ctx: ctx, - VmSnapshot: vmSnapshot, + Ctx: ctx, + Name: name, + Namespace: namespace, } mock.lockGetSecret.Lock() mock.calls.GetSecret = append(mock.calls.GetSecret, callInfo) mock.lockGetSecret.Unlock() - return mock.GetSecretFunc(ctx, vmSnapshot) + return mock.GetSecretFunc(ctx, name, namespace) } // GetSecretCalls gets all the calls that were made to GetSecret. @@ -485,12 +489,14 @@ func (mock *SnapshotterMock) GetSecret(ctx context.Context, vmSnapshot *v1alpha2 // // len(mockedSnapshotter.GetSecretCalls()) func (mock *SnapshotterMock) GetSecretCalls() []struct { - Ctx context.Context - VmSnapshot *v1alpha2.VirtualMachineSnapshot + Ctx context.Context + Name string + Namespace string } { var calls []struct { - Ctx context.Context - VmSnapshot *v1alpha2.VirtualMachineSnapshot + Ctx context.Context + Name string + Namespace string } mock.lockGetSecret.RLock() calls = mock.calls.GetSecret From d1bf6ec20fc2e23e63667c15c040092fdfb55907 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 24 Nov 2025 14:44:18 +0300 Subject: [PATCH 07/11] wip Signed-off-by: Daniil Loktev --- .../vdsnapshot/internal/life_cycle_test.go | 12 ++++++------ .../vmsnapshot/internal/life_cycle_test.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go index 207eb2e801..56200e6019 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle_test.go @@ -86,7 +86,7 @@ var _ = Describe("LifeCycle handler", func() { GetVirtualMachineFunc: func(_ context.Context, _, _ string) (*v1alpha2.VirtualMachine, error) { return nil, nil }, - GetVolumeSnapshotFunc: func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { + GetVolumeSnapshotFunc: func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { return nil, nil }, } @@ -106,7 +106,7 @@ var _ = Describe("LifeCycle handler", func() { }) It("The volume snapshot has failed", func() { - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { vs.Status = &vsv1.VolumeSnapshotStatus{ Error: &vsv1.VolumeSnapshotError{ Message: ptr.To("error"), @@ -127,7 +127,7 @@ var _ = Describe("LifeCycle handler", func() { }) It("The volume snapshot is not ready yet", func() { - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { return vs, nil } @@ -143,7 +143,7 @@ var _ = Describe("LifeCycle handler", func() { }) It("The volume snapshot is ready", func() { - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { vs.Status = &vsv1.VolumeSnapshotStatus{ ReadyToUse: ptr.To(true), } @@ -260,7 +260,7 @@ var _ = Describe("LifeCycle handler", func() { snapshotter.IsFrozenFunc = func(_ *v1alpha2.VirtualMachine) bool { return true } - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { vs.Status = &vsv1.VolumeSnapshotStatus{ ReadyToUse: ptr.To(true), } @@ -283,7 +283,7 @@ var _ = Describe("LifeCycle handler", func() { snapshotter.IsFrozenFunc = func(_ *v1alpha2.VirtualMachine) bool { return true } - snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _ *v1alpha2.VirtualDiskSnapshot) (*vsv1.VolumeSnapshot, error) { + snapshotter.GetVolumeSnapshotFunc = func(_ context.Context, _, _ string) (*vsv1.VolumeSnapshot, error) { vs.Status = &vsv1.VolumeSnapshotStatus{ ReadyToUse: ptr.To(true), } diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go index 46bdddd5d3..0ba5d6726e 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go @@ -136,7 +136,7 @@ var _ = Describe("LifeCycle handler", func() { UnfreezeFunc: func(ctx context.Context, _, _ string) error { return nil }, - GetSecretFunc: func(_ context.Context, _ *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { + GetSecretFunc: func(_ context.Context, _, _ string) (*corev1.Secret, error) { return secret, nil }, GetVirtualDiskSnapshotFunc: func(_ context.Context, _, _ string) (*v1alpha2.VirtualDiskSnapshot, error) { From 7864e3dd4bf0e973949b3efdf728bc24b3128617 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 24 Nov 2025 14:48:22 +0300 Subject: [PATCH 08/11] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/vmsnapshot/internal/life_cycle_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go index 0ba5d6726e..66561c384a 100644 --- a/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go +++ b/images/virtualization-artifact/pkg/controller/vmsnapshot/internal/life_cycle_test.go @@ -144,12 +144,6 @@ var _ = Describe("LifeCycle handler", func() { }, } - storer = &StorerMock{ - StoreFunc: func(_ context.Context, _ *v1alpha2.VirtualMachine, _ *v1alpha2.VirtualMachineSnapshot) (*corev1.Secret, error) { - return secret, nil - }, - } - var err error fakeClient, err = testutil.NewFakeClientWithObjects(vd, vm, secret, vmSnapshot, vdSnapshot) Expect(err).NotTo(HaveOccurred()) From 7d7fe1029e3587ddebbc56805b9fc7e85e5f86cc Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Mon, 24 Nov 2025 17:06:38 +0300 Subject: [PATCH 09/11] wip Signed-off-by: Daniil Loktev --- .../pkg/controller/supplements/generator.go | 9 --------- .../pkg/controller/supplements/generator_test.go | 9 --------- 2 files changed, 18 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/supplements/generator.go b/images/virtualization-artifact/pkg/controller/supplements/generator.go index e7c925dc5c..4ebd0e917b 100644 --- a/images/virtualization-artifact/pkg/controller/supplements/generator.go +++ b/images/virtualization-artifact/pkg/controller/supplements/generator.go @@ -71,7 +71,6 @@ type Generator interface { LegacyDVCRAuthSecretForDV() types.NamespacedName LegacyUploaderTLSSecretForIngress() types.NamespacedName LegacyImagePullSecret() types.NamespacedName - LegacySnapshotSupplement() types.NamespacedName } // Generator calculates names for supplemental resources, e.g. ImporterPod, AuthSecret or CABundleConfigMap. @@ -277,11 +276,3 @@ func (g *generator) LegacyDataVolume() types.NamespacedName { func (g *generator) LegacyPersistentVolumeClaim() types.NamespacedName { return g.LegacyDataVolume() } - -// LegacySnapshotSupplement generates old format name for snapshot-related resources. -func (g *generator) LegacySnapshotSupplement() types.NamespacedName { - return types.NamespacedName{ - Name: g.name, - Namespace: g.namespace, - } -} diff --git a/images/virtualization-artifact/pkg/controller/supplements/generator_test.go b/images/virtualization-artifact/pkg/controller/supplements/generator_test.go index 8a055a471c..6df9f79a5c 100644 --- a/images/virtualization-artifact/pkg/controller/supplements/generator_test.go +++ b/images/virtualization-artifact/pkg/controller/supplements/generator_test.go @@ -68,15 +68,6 @@ var _ = Describe("Generator", func() { Entry("CommonSupplement", func(g Generator) types.NamespacedName { return g.CommonSupplement() }, "vi"), ) - It("should generate legacy snapshot supplement name without prefix or UID", func() { - name := "test-snapshot" - gen = NewGenerator("vms", name, namespace, uid) - result := gen.LegacySnapshotSupplement() - - Expect(result.Name).To(Equal(name)) - Expect(result.Namespace).To(Equal(namespace)) - }) - DescribeTable("should truncate long names to respect limits", func(method func(Generator) types.NamespacedName, maxLength int) { name := strings.Repeat("very-long-resource-name-", 30) From 53653add43970651989078d5bf99647b0eaf7b3b Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Tue, 25 Nov 2025 11:13:08 +0300 Subject: [PATCH 10/11] fix vs name Signed-off-by: Daniil Loktev --- .../pkg/controller/vi/internal/source/step/create_pvc_step.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/create_pvc_step.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/create_pvc_step.go index 02d4008806..3d1336e2b5 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/step/create_pvc_step.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/step/create_pvc_step.go @@ -169,7 +169,7 @@ func (s CreatePersistentVolumeClaimStep) buildPVC(vi *v1alpha2.VirtualImage, vs DataSource: &corev1.TypedLocalObjectReference{ APIGroup: ptr.To(vs.GroupVersionKind().Group), Kind: vs.Kind, - Name: vi.Spec.DataSource.ObjectRef.Name, + Name: vs.Name, }, } From 40b07221993dfc1fe4262f30aa7c9f8153189fc4 Mon Sep 17 00:00:00 2001 From: Daniil Loktev Date: Thu, 4 Dec 2025 15:57:13 +0300 Subject: [PATCH 11/11] fix comments Signed-off-by: Daniil Loktev --- .../pkg/controller/vdsnapshot/internal/life_cycle.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go index e05eb144a2..e3a4aeca02 100644 --- a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go +++ b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go @@ -59,6 +59,10 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu defer func() { conditions.SetCondition(cb, &vdSnapshot.Status.Conditions) }() + if vdSnapshot.Status.VolumeSnapshotName == "" { + vdSnapshot.Status.VolumeSnapshotName = supplements.NewGenerator("vds", vdSnapshot.Name, vdSnapshot.Namespace, vdSnapshot.UID).CommonSupplement().Name + } + vs, err := h.snapshotter.GetVolumeSnapshot(ctx, vdSnapshot.Status.VolumeSnapshotName, vdSnapshot.Namespace) if err != nil { setPhaseConditionToFailed(cb, &vdSnapshot.Status.Phase, err) @@ -331,13 +335,11 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *v1alpha2.Virtu } } - vsName := supplements.NewGenerator("vds", vdSnapshot.Name, vdSnapshot.Namespace, vdSnapshot.UID).CommonSupplement() - vs = &vsv1.VolumeSnapshot{ ObjectMeta: metav1.ObjectMeta{ Annotations: anno, - Name: vsName.Name, - Namespace: vsName.Namespace, + Name: vdSnapshot.Status.VolumeSnapshotName, + Namespace: vdSnapshot.Namespace, OwnerReferences: []metav1.OwnerReference{ service.MakeOwnerReference(vdSnapshot), },