From 70ce517d225716f21a12dd4935f2ff9507841de1 Mon Sep 17 00:00:00 2001 From: "dmitry.lopatin" Date: Wed, 2 Oct 2024 23:09:34 +0300 Subject: [PATCH 1/5] fix(cvi,vi): unlock pending image from VD ref Signed-off-by: dmitry.lopatin --- api/core/v1alpha2/cvicondition/condition.go | 2 ++ api/core/v1alpha2/vdcondition/condition.go | 9 +++++ api/core/v1alpha2/vicondition/condition.go | 2 ++ .../pkg/controller/cvi/cvi_reconciler.go | 10 +++++- .../cvi/internal/datasource_ready.go | 4 +-- .../controller/cvi/internal/source/errors.go | 16 ++++----- .../cvi/internal/source/object_ref_vd.go | 16 ++++----- .../pkg/controller/service/errors.go | 33 ++++++++++++++++++- .../iso_source_validator.go | 2 +- .../pvc_size_validator.go | 2 +- .../spec_changes_validator.go | 2 +- .../pkg/controller/vd/vd_reconciler.go | 4 +-- .../pkg/controller/vd/vd_webhook.go | 8 ++--- .../vi/internal/datasource_ready.go | 4 +-- .../controller/vi/internal/source/errors.go | 16 ++++----- .../vi/internal/source/object_ref.go | 8 ++--- .../vi/internal/source/object_ref_vd.go | 17 ++++------ .../pkg/controller/vi/vi_reconciler.go | 10 +++++- .../controller/vm/internal/block_device.go | 5 +-- .../vm/internal/block_devices_test.go | 22 +++++++++++++ .../pkg/controller/vm/vm_reconciler.go | 11 ++++++- .../controller/watchers/object_ref_watcher.go | 4 +-- .../pkg/controller/watchers/vd_enqueuer.go | 33 +++++++++++++++++++ 23 files changed, 176 insertions(+), 64 deletions(-) rename images/virtualization-artifact/pkg/controller/vd/internal/{validators => validator}/iso_source_validator.go (99%) rename images/virtualization-artifact/pkg/controller/vd/internal/{validators => validator}/pvc_size_validator.go (99%) rename images/virtualization-artifact/pkg/controller/vd/internal/{validators => validator}/spec_changes_validator.go (99%) diff --git a/api/core/v1alpha2/cvicondition/condition.go b/api/core/v1alpha2/cvicondition/condition.go index 98c3794a86..904efb1878 100644 --- a/api/core/v1alpha2/cvicondition/condition.go +++ b/api/core/v1alpha2/cvicondition/condition.go @@ -56,6 +56,8 @@ const ( ClusterImageNotReady DatasourceReadyReason = "ClusterImageNotReady" // VirtualDiskNotReady indicates that the `VirtualDisk` datasource is not ready, which prevents the import process from starting. VirtualDiskNotReady DatasourceReadyReason = "VirtualDiskNotReady" + // VirtualDiskInUseInRunningVirtualMachine indicates that the `VirtualDisk` attached to running `VirtualMachine` + VirtualDiskInUseInRunningVirtualMachine DatasourceReadyReason = "VirtualDiskInUseInRunningVirtualMachine" // WaitForUserUpload indicates that the `ClusterVirtualImage` is waiting for the user to upload a datasource for the import process to continue. WaitForUserUpload ReadyReason = "WaitForUserUpload" diff --git a/api/core/v1alpha2/vdcondition/condition.go b/api/core/v1alpha2/vdcondition/condition.go index b2ff9acdd9..7c45c47e87 100644 --- a/api/core/v1alpha2/vdcondition/condition.go +++ b/api/core/v1alpha2/vdcondition/condition.go @@ -34,6 +34,8 @@ const ( SnapshottingType Type = "Snapshotting" // StorageClassReadyType indicates whether the storage class is ready. StorageClassReadyType Type = "StorageClassReady" + // InUseType indicates whether the VirtualDisk is attached to a running VirtualMachine or is being used in a process of a VirtualImage creation. + InUseType Type = "InUse" ) type ( @@ -47,6 +49,8 @@ type ( SnapshottingReason string // StorageClassReadyReason represents the various reasons for the Storageclass ready condition type. StorageClassReadyReason string + // InUseReason represents the various reasons for the InUse condition type. + InUseReason = string ) func (s DatasourceReadyReason) String() string { @@ -118,4 +122,9 @@ const ( StorageClassReady StorageClassReadyReason = "StorageClassReady" // StorageClassNotFound indicates that the storage class is not ready StorageClassNotFound StorageClassReadyReason = "StorageClassNotFound" + + // InUseByVirtualImage indicates that the VirtualDisk is being used in a process of a VirtualImage creation. + InUseByVirtualImage InUseReason = "InUseByVirtualImage" + // InUseByVirtualMachine indicates that the VirtualDisk is attached to a running VirtualMachine. + InUseByVirtualMachine InUseReason = "InUseByVirtualMachine" ) diff --git a/api/core/v1alpha2/vicondition/condition.go b/api/core/v1alpha2/vicondition/condition.go index de6fc03cce..f450d1e701 100644 --- a/api/core/v1alpha2/vicondition/condition.go +++ b/api/core/v1alpha2/vicondition/condition.go @@ -64,6 +64,8 @@ const ( ClusterImageNotReady DatasourceReadyReason = "ClusterImageNotReady" // VirtualDiskNotReady indicates that the `VirtualDisk` datasource is not ready, which prevents the import process from starting. VirtualDiskNotReady DatasourceReadyReason = "VirtualDiskNotReady" + // VirtualDiskInUseInRunningVirtualMachine indicates that the `VirtualDisk` attached to running `VirtualMachine` + VirtualDiskInUseInRunningVirtualMachine DatasourceReadyReason = "VirtualDiskInUseInRunningVirtualMachine" // WaitForUserUpload indicates that the `VirtualImage` is waiting for the user to upload a datasource for the import process to continue. WaitForUserUpload ReadyReason = "WaitForUserUpload" diff --git a/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go b/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go index 3e3f4ffae8..22af020741 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go @@ -38,6 +38,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/watchers" "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" ) type Handler interface { @@ -165,7 +166,14 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr return false } - return oldVD.Status.Phase != newVD.Status.Phase + oldInUseCondition, _ := service.GetCondition(vdcondition.InUseType, oldVD.Status.Conditions) + newInUseCondition, _ := service.GetCondition(vdcondition.InUseType, newVD.Status.Conditions) + + if oldVD.Status.Phase != newVD.Status.Phase || len(oldVD.Status.AttachedToVirtualMachines) != len(newVD.Status.AttachedToVirtualMachines) || oldInUseCondition.Status != newInUseCondition.Status { + return true + } + + return false }, }, ); err != nil { diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go b/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go index d6be857216..52b0c26d2b 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go @@ -89,10 +89,10 @@ func (h DatasourceReadyHandler) Handle(ctx context.Context, cvi *virtv2.ClusterV Reason(cvicondition.VirtualDiskNotReady). Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, nil - case errors.As(err, &source.VirtualDiskAttachedToRunningVMError{}): + case errors.As(err, &source.VirtualDiskInUseError{}): cb. Status(metav1.ConditionFalse). - Reason(cvicondition.VirtualDiskNotReady). + Reason(cvicondition.VirtualDiskInUseInRunningVirtualMachine). Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, nil default: diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go index f8daee7182..55de6884e5 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go @@ -65,18 +65,16 @@ func NewVirtualDiskNotReadyError(name string) error { } } -type VirtualDiskAttachedToRunningVMError struct { - name string - vmName string +type VirtualDiskInUseError struct { + name string } -func (e VirtualDiskAttachedToRunningVMError) Error() string { - return fmt.Sprintf("VirtualDisk %q attached to running VirtualMachine %q", e.name, e.vmName) +func (e VirtualDiskInUseError) Error() string { + return fmt.Sprintf("reading from the VirtualDisk is not possible while it is in use by the running VirtualMachine/%s", e.name) } -func NewVirtualDiskAttachedToRunningVMError(name, vmName string) error { - return VirtualDiskAttachedToRunningVMError{ - name: name, - vmName: vmName, +func NewVirtualDiskInUseError(name string) error { + return VirtualDiskInUseError{ + name: name, } } diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go index 2c28b48f68..b7017d9fc1 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go @@ -39,6 +39,8 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/cvicondition" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition" ) type ObjectRefVirtualDisk struct { @@ -226,16 +228,10 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, cvi *virtv2.Cluster return NewVirtualDiskNotReadyError(cvi.Spec.DataSource.ObjectRef.Name) } - if len(vd.Status.AttachedToVirtualMachines) != 0 { - vmName := vd.Status.AttachedToVirtualMachines[0] - - vm, err := object.FetchObject(ctx, types.NamespacedName{Name: vmName.Name, Namespace: vd.Namespace}, ds.client, &virtv2.VirtualMachine{}) - if err != nil { - return err - } - - if vm.Status.Phase != virtv2.MachineStopped { - return NewVirtualDiskAttachedToRunningVMError(vd.Name, vmName.Name) + inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + if inUseCondition.Status == metav1.ConditionTrue { + if inUseCondition.Reason == vdcondition.InUseByVirtualMachine { + return NewVirtualDiskInUseError(vd.Name) } } diff --git a/images/virtualization-artifact/pkg/controller/service/errors.go b/images/virtualization-artifact/pkg/controller/service/errors.go index 341c712a22..424e153b0c 100644 --- a/images/virtualization-artifact/pkg/controller/service/errors.go +++ b/images/virtualization-artifact/pkg/controller/service/errors.go @@ -16,7 +16,10 @@ limitations under the License. package service -import "errors" +import ( + "errors" + "fmt" +) var ( ErrStorageClassNotFound = errors.New("storage class not found") @@ -30,3 +33,31 @@ var ( ErrIPAddressAlreadyExist = errors.New("the IP address is already allocated") ErrIPAddressOutOfRange = errors.New("the IP address is out of range") ) + +type VirtualDiskUsedByImageError struct { + vdName string +} + +func (e VirtualDiskUsedByImageError) Error() string { + return fmt.Sprintf("the virtual disk %q already used by creating image", e.vdName) +} + +func NewVirtualDiskUsedByImageError(name string) error { + return VirtualDiskUsedByImageError{ + vdName: name, + } +} + +type VirtualDiskUsedByVirtualMachineError struct { + vdName string +} + +func (e VirtualDiskUsedByVirtualMachineError) Error() string { + return fmt.Sprintf("the virtual disk %q already used by running virtual machine", e.vdName) +} + +func NewVirtualDiskUsedByVirtualMachineError(name string) error { + return VirtualDiskUsedByVirtualMachineError{ + vdName: name, + } +} diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/validators/iso_source_validator.go b/images/virtualization-artifact/pkg/controller/vd/internal/validator/iso_source_validator.go similarity index 99% rename from images/virtualization-artifact/pkg/controller/vd/internal/validators/iso_source_validator.go rename to images/virtualization-artifact/pkg/controller/vd/internal/validator/iso_source_validator.go index fff4366136..692a1f7d8c 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/validators/iso_source_validator.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/validator/iso_source_validator.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package validators +package validator import ( "context" diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/validators/pvc_size_validator.go b/images/virtualization-artifact/pkg/controller/vd/internal/validator/pvc_size_validator.go similarity index 99% rename from images/virtualization-artifact/pkg/controller/vd/internal/validators/pvc_size_validator.go rename to images/virtualization-artifact/pkg/controller/vd/internal/validator/pvc_size_validator.go index a7c92d88e7..3a69436467 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/validators/pvc_size_validator.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/validator/pvc_size_validator.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package validators +package validator import ( "context" diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/validators/spec_changes_validator.go b/images/virtualization-artifact/pkg/controller/vd/internal/validator/spec_changes_validator.go similarity index 99% rename from images/virtualization-artifact/pkg/controller/vd/internal/validators/spec_changes_validator.go rename to images/virtualization-artifact/pkg/controller/vd/internal/validator/spec_changes_validator.go index 8b7d6acb3b..95e669ef1d 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/validators/spec_changes_validator.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/validator/spec_changes_validator.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package validators +package validator import ( "context" diff --git a/images/virtualization-artifact/pkg/controller/vd/vd_reconciler.go b/images/virtualization-artifact/pkg/controller/vd/vd_reconciler.go index b5b181bf48..c837a0b2c1 100644 --- a/images/virtualization-artifact/pkg/controller/vd/vd_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vd/vd_reconciler.go @@ -232,8 +232,8 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr return fmt.Errorf("error setting watch on CVIs: %w", err) } - w := watcher.NewVirtualDiskSnapshotWatcher(mgr.GetClient()) - if err := w.Watch(mgr, ctr); err != nil { + snapshotWatcher := watcher.NewVirtualDiskSnapshotWatcher(mgr.GetClient()) + if err := snapshotWatcher.Watch(mgr, ctr); err != nil { return fmt.Errorf("error setting watch on VDSnapshots: %w", err) } diff --git a/images/virtualization-artifact/pkg/controller/vd/vd_webhook.go b/images/virtualization-artifact/pkg/controller/vd/vd_webhook.go index ee52484260..94cacb0391 100644 --- a/images/virtualization-artifact/pkg/controller/vd/vd_webhook.go +++ b/images/virtualization-artifact/pkg/controller/vd/vd_webhook.go @@ -24,7 +24,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "github.com/deckhouse/virtualization-controller/pkg/controller/vd/internal/validators" + "github.com/deckhouse/virtualization-controller/pkg/controller/vd/internal/validator" "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -41,9 +41,9 @@ type Validator struct { func NewValidator(client client.Client) *Validator { return &Validator{ validators: []VirtualDiskValidator{ - validators.NewPVCSizeValidator(client), - validators.NewSpecChangesValidator(), - validators.NewISOSourceValidator(client), + validator.NewPVCSizeValidator(client), + validator.NewSpecChangesValidator(), + validator.NewISOSourceValidator(client), }, } } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go b/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go index 3a6689f71f..647e8a6251 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go @@ -89,10 +89,10 @@ func (h DatasourceReadyHandler) Handle(ctx context.Context, vi *virtv2.VirtualIm Reason(vicondition.VirtualDiskNotReady). Message(service.CapitalizeFirstLetter(err.Error() + ".")) return reconcile.Result{}, nil - case errors.As(err, &source.VirtualDiskAttachedToRunningVMError{}): + case errors.As(err, &source.VirtualDiskInUseError{}): cb. Status(metav1.ConditionFalse). - Reason(vicondition.VirtualDiskNotReady). + Reason(vicondition.VirtualDiskInUseInRunningVirtualMachine). Message(service.CapitalizeFirstLetter(err.Error() + ".")) return reconcile.Result{}, nil default: diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go index f8daee7182..55de6884e5 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go @@ -65,18 +65,16 @@ func NewVirtualDiskNotReadyError(name string) error { } } -type VirtualDiskAttachedToRunningVMError struct { - name string - vmName string +type VirtualDiskInUseError struct { + name string } -func (e VirtualDiskAttachedToRunningVMError) Error() string { - return fmt.Sprintf("VirtualDisk %q attached to running VirtualMachine %q", e.name, e.vmName) +func (e VirtualDiskInUseError) Error() string { + return fmt.Sprintf("reading from the VirtualDisk is not possible while it is in use by the running VirtualMachine/%s", e.name) } -func NewVirtualDiskAttachedToRunningVMError(name, vmName string) error { - return VirtualDiskAttachedToRunningVMError{ - name: name, - vmName: vmName, +func NewVirtualDiskInUseError(name string) error { + return VirtualDiskInUseError{ + name: name, } } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref.go index bcbad6fcd6..b312d7ea44 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref.go @@ -108,14 +108,14 @@ func (ds ObjectRefDataSource) StoreToPVC(ctx context.Context, vi *virtv2.Virtual return ds.viObjectRefOnPvc.StoreToPVC(ctx, vi, viRef, cb) } case virtv2.VirtualDiskKind: - viKey := types.NamespacedName{Name: vi.Spec.DataSource.ObjectRef.Name, Namespace: vi.Namespace} - vd, err := object.FetchObject(ctx, viKey, ds.client, &virtv2.VirtualDisk{}) + vdKey := types.NamespacedName{Name: vi.Spec.DataSource.ObjectRef.Name, Namespace: vi.Namespace} + vd, err := object.FetchObject(ctx, vdKey, ds.client, &virtv2.VirtualDisk{}) if err != nil { - return reconcile.Result{}, fmt.Errorf("unable to get VI %s: %w", viKey, err) + return reconcile.Result{}, fmt.Errorf("unable to get VD %s: %w", vdKey, err) } if vd == nil { - return reconcile.Result{}, fmt.Errorf("VD object ref %s is nil", viKey) + return reconcile.Result{}, fmt.Errorf("VD object ref %s is nil", vdKey) } return ds.vdSyncer.StoreToPVC(ctx, vi, vd, cb) diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go index 49a36563c0..cebe6eb742 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go @@ -42,6 +42,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/dvcr" "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition" ) @@ -366,7 +367,7 @@ func (ds ObjectRefVirtualDisk) getEnvSettings(vi *virtv2.VirtualImage, sup *supp func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, vi *virtv2.VirtualImage) error { if vi.Spec.DataSource.ObjectRef == nil || vi.Spec.DataSource.ObjectRef.Kind != virtv2.VirtualImageObjectRefKindVirtualDisk { - return fmt.Errorf("not a %s data source", virtv2.ClusterVirtualImageObjectRefKindVirtualDisk) + return fmt.Errorf("not a %s data source", virtv2.VirtualImageObjectRefKindVirtualDisk) } vd, err := object.FetchObject(ctx, types.NamespacedName{Name: vi.Spec.DataSource.ObjectRef.Name, Namespace: vi.Namespace}, ds.client, &virtv2.VirtualDisk{}) @@ -378,17 +379,11 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, vi *virtv2.VirtualI return NewVirtualDiskNotReadyError(vi.Spec.DataSource.ObjectRef.Name) } - if len(vd.Status.AttachedToVirtualMachines) > 0 { - vmName := vd.Status.AttachedToVirtualMachines[0] - vm, err := object.FetchObject(ctx, types.NamespacedName{Name: vmName.Name, Namespace: vd.Namespace}, ds.client, &virtv2.VirtualMachine{}) - if err != nil { - return err - } - - if vm.Status.Phase != virtv2.MachineStopped { - return NewVirtualDiskAttachedToRunningVMError(vd.Name, vmName.Name) + inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + if inUseCondition.Status == metav1.ConditionTrue { + if inUseCondition.Reason == vdcondition.InUseByVirtualMachine { + return NewVirtualDiskInUseError(vd.Name) } } - return nil } diff --git a/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go b/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go index c30f72a394..6c524f9f95 100644 --- a/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go @@ -40,6 +40,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/watchers" "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" ) type Handler interface { @@ -220,7 +221,14 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr return false } - return oldVD.Status.Phase != newVD.Status.Phase + oldInUseCondition, _ := service.GetCondition(vdcondition.InUseType, oldVD.Status.Conditions) + newInUseCondition, _ := service.GetCondition(vdcondition.InUseType, newVD.Status.Conditions) + + if oldVD.Status.Phase != newVD.Status.Phase || len(oldVD.Status.AttachedToVirtualMachines) != len(newVD.Status.AttachedToVirtualMachines) || oldInUseCondition.Status != newInUseCondition.Status { + return true + } + + return false }, }, ); err != nil { diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go index 29742501d6..a9028d0632 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go @@ -37,6 +37,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" ) @@ -317,8 +318,8 @@ func (h *BlockDeviceHandler) countReadyBlockDevices(vm *virtv2.VirtualMachine, s canStartKVVM = false continue } - - if vd.Status.Phase == virtv2.DiskReady { + readyCondition, _ := service.GetCondition(vdcondition.ReadyType, vd.Status.Conditions) + if readyCondition.Status == metav1.ConditionTrue { ready++ } else { msg := fmt.Sprintf("virtual disk %s is waiting for the it's pvc to be bound", vd.Name) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go index 78d9d1aa75..8855d0a36f 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" ) func TestBlockDeviceHandler(t *testing.T) { @@ -68,6 +69,13 @@ var _ = Describe("BlockDeviceHandler", func() { Status: virtv2.VirtualDiskStatus{ Phase: virtv2.DiskReady, Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-foo"}, + Conditions: []metav1.Condition{ + { + Type: vdcondition.ReadyType, + Reason: vdcondition.Ready, + Status: metav1.ConditionTrue, + }, + }, }, } vdBar = &virtv2.VirtualDisk{ @@ -75,6 +83,13 @@ var _ = Describe("BlockDeviceHandler", func() { Status: virtv2.VirtualDiskStatus{ Phase: virtv2.DiskReady, Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-bar"}, + Conditions: []metav1.Condition{ + { + Type: vdcondition.ReadyType, + Reason: vdcondition.Ready, + Status: metav1.ConditionTrue, + }, + }, }, } vm = &virtv2.VirtualMachine{ @@ -166,6 +181,13 @@ var _ = Describe("BlockDeviceHandler", func() { It("VirtualDisk's target pvc is created", func() { vdFoo.Status.Phase = virtv2.DiskProvisioning + vdFoo.Status.Conditions = []metav1.Condition{ + { + Type: vdcondition.ReadyType, + Reason: vdcondition.Provisioning, + Status: metav1.ConditionFalse, + }, + } state := getBlockDevicesState(vi, cvi, vdFoo, vdBar) ready, canStart, warnings := h.countReadyBlockDevices(vm, state, logger) Expect(ready).To(Equal(3)) diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go b/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go index 5654dce2ff..14c8ea8ac0 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go @@ -40,6 +40,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/watcher" "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" ) type Handler interface { @@ -216,7 +217,15 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr if !oldOk || !newOk { return false } - return oldVd.Status.Phase != newVd.Status.Phase + + oldInUseCondition, _ := service.GetCondition(vdcondition.InUseType, oldVd.Status.Conditions) + newInUseCondition, _ := service.GetCondition(vdcondition.InUseType, newVd.Status.Conditions) + + if oldVd.Status.Phase != newVd.Status.Phase || oldInUseCondition.Status != newInUseCondition.Status { + return true + } + + return false }, }, ); err != nil { diff --git a/images/virtualization-artifact/pkg/controller/watchers/object_ref_watcher.go b/images/virtualization-artifact/pkg/controller/watchers/object_ref_watcher.go index 350ac88649..8fbe9a169a 100644 --- a/images/virtualization-artifact/pkg/controller/watchers/object_ref_watcher.go +++ b/images/virtualization-artifact/pkg/controller/watchers/object_ref_watcher.go @@ -62,8 +62,8 @@ func (w ObjectRefWatcher) Run(mgr manager.Manager, ctr controller.Controller) er source.Kind(mgr.GetCache(), w.enqueuer.GetEnqueueFrom()), handler.EnqueueRequestsFromMapFunc(w.enqueuer.EnqueueRequests), predicate.Funcs{ - CreateFunc: func(e event.CreateEvent) bool { return false }, - DeleteFunc: func(e event.DeleteEvent) bool { return false }, + CreateFunc: func(e event.CreateEvent) bool { return true }, + DeleteFunc: func(e event.DeleteEvent) bool { return true }, UpdateFunc: w.filter.FilterUpdateEvents, }, ) diff --git a/images/virtualization-artifact/pkg/controller/watchers/vd_enqueuer.go b/images/virtualization-artifact/pkg/controller/watchers/vd_enqueuer.go index e82b51b58a..bda8a2859e 100644 --- a/images/virtualization-artifact/pkg/controller/watchers/vd_enqueuer.go +++ b/images/virtualization-artifact/pkg/controller/watchers/vd_enqueuer.go @@ -52,6 +52,39 @@ func (w VirtualDiskRequestEnqueuer) GetEnqueueFrom() client.Object { } func (w VirtualDiskRequestEnqueuer) EnqueueRequests(ctx context.Context, obj client.Object) (requests []reconcile.Request) { + // Enqueue CVI or VI specified by the object ref. + if w.enqueueFromKind == virtv2.VirtualDiskObjectRefKindVirtualImage { + vi, ok := obj.(*virtv2.VirtualImage) + if !ok { + w.logger.Error(fmt.Sprintf("expected a VirtualImage but got a %T", obj)) + return + } + + if vi.Spec.DataSource.Type == virtv2.DataSourceTypeObjectRef && vi.Spec.DataSource.ObjectRef != nil && vi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: vi.Spec.DataSource.ObjectRef.Name, + Namespace: vi.Namespace, + }, + }) + } + } else if w.enqueueFromKind == virtv2.VirtualDiskObjectRefKindClusterVirtualImage { + cvi, ok := obj.(*virtv2.ClusterVirtualImage) + if !ok { + w.logger.Error(fmt.Sprintf("expected a ClusterVirtualImage but got a %T", obj)) + return + } + + if cvi.Spec.DataSource.Type == virtv2.DataSourceTypeObjectRef && cvi.Spec.DataSource.ObjectRef != nil && cvi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: cvi.Spec.DataSource.ObjectRef.Name, + Namespace: cvi.Spec.DataSource.ObjectRef.Namespace, + }, + }) + } + } + var vds virtv2.VirtualDiskList err := w.client.List(ctx, &vds) if err != nil { From 2ccd21d05715bd2bfe22f45d7675a85653da547a Mon Sep 17 00:00:00 2001 From: "dmitry.lopatin" Date: Fri, 15 Nov 2024 16:08:31 +0300 Subject: [PATCH 2/5] fix(api): add new logic allowed use for images and vms Signed-off-by: dmitry.lopatin --- api/core/v1alpha2/vdcondition/condition.go | 14 ++++++---- .../pkg/controller/cvi/cvi_reconciler.go | 5 ++-- .../cvi/internal/datasource_ready.go | 2 +- .../controller/cvi/internal/source/errors.go | 10 +++---- .../cvi/internal/source/object_ref_vd.go | 7 +++-- .../vi/internal/datasource_ready.go | 2 +- .../controller/vi/internal/source/errors.go | 10 +++---- .../vi/internal/source/object_ref_vd.go | 7 ++--- .../pkg/controller/vi/vi_reconciler.go | 5 ++-- .../controller/vm/internal/block_device.go | 10 +++++-- .../vm/internal/block_devices_test.go | 27 ++++++++++++++----- .../pkg/controller/vm/vm_reconciler.go | 5 ++-- 12 files changed, 66 insertions(+), 38 deletions(-) diff --git a/api/core/v1alpha2/vdcondition/condition.go b/api/core/v1alpha2/vdcondition/condition.go index 7c45c47e87..d17e6ab0da 100644 --- a/api/core/v1alpha2/vdcondition/condition.go +++ b/api/core/v1alpha2/vdcondition/condition.go @@ -50,7 +50,7 @@ type ( // StorageClassReadyReason represents the various reasons for the Storageclass ready condition type. StorageClassReadyReason string // InUseReason represents the various reasons for the InUse condition type. - InUseReason = string + InUseReason string ) func (s DatasourceReadyReason) String() string { @@ -73,6 +73,10 @@ func (s StorageClassReadyReason) String() string { return string(s) } +func (s InUseReason) String() string { + return string(s) +} + const ( // DatasourceReady indicates that the datasource is ready for use, allowing the import process to start. DatasourceReady DatasourceReadyReason = "DatasourceReady" @@ -123,8 +127,8 @@ const ( // StorageClassNotFound indicates that the storage class is not ready StorageClassNotFound StorageClassReadyReason = "StorageClassNotFound" - // InUseByVirtualImage indicates that the VirtualDisk is being used in a process of a VirtualImage creation. - InUseByVirtualImage InUseReason = "InUseByVirtualImage" - // InUseByVirtualMachine indicates that the VirtualDisk is attached to a running VirtualMachine. - InUseByVirtualMachine InUseReason = "InUseByVirtualMachine" + // AllowedForImageUsage indicates that the VirtualDisk is permitted for use in a process of an image creation. + AllowedForImageUsage InUseReason = "AllowedForImageUsage" + // AllowedForVirtualMachineUsage indicates that the VirtualDisk is permitted for use by a running VirtualMachine. + AllowedForVirtualMachineUsage InUseReason = "AllowedForVirtualMachineUsage" ) diff --git a/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go b/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go index 22af020741..95de7743da 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/watchers" "github.com/deckhouse/virtualization-controller/pkg/logger" @@ -166,8 +167,8 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr return false } - oldInUseCondition, _ := service.GetCondition(vdcondition.InUseType, oldVD.Status.Conditions) - newInUseCondition, _ := service.GetCondition(vdcondition.InUseType, newVD.Status.Conditions) + oldInUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, oldVD.Status.Conditions) + newInUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, newVD.Status.Conditions) if oldVD.Status.Phase != newVD.Status.Phase || len(oldVD.Status.AttachedToVirtualMachines) != len(newVD.Status.AttachedToVirtualMachines) || oldInUseCondition.Status != newInUseCondition.Status { return true diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go b/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go index 52b0c26d2b..9eb52cdfd8 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go @@ -89,7 +89,7 @@ func (h DatasourceReadyHandler) Handle(ctx context.Context, cvi *virtv2.ClusterV Reason(cvicondition.VirtualDiskNotReady). Message(service.CapitalizeFirstLetter(err.Error())) return reconcile.Result{}, nil - case errors.As(err, &source.VirtualDiskInUseError{}): + case errors.As(err, &source.VirtualDiskNotAllowedForUseError{}): cb. Status(metav1.ConditionFalse). Reason(cvicondition.VirtualDiskInUseInRunningVirtualMachine). diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go index 55de6884e5..3de699998a 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/errors.go @@ -65,16 +65,16 @@ func NewVirtualDiskNotReadyError(name string) error { } } -type VirtualDiskInUseError struct { +type VirtualDiskNotAllowedForUseError struct { name string } -func (e VirtualDiskInUseError) Error() string { - return fmt.Sprintf("reading from the VirtualDisk is not possible while it is in use by the running VirtualMachine/%s", e.name) +func (e VirtualDiskNotAllowedForUseError) Error() string { + return fmt.Sprintf("use of VirtualDisk %s is not allowed, it may be connected to a running VirtualMachine", e.name) } -func NewVirtualDiskInUseError(name string) error { - return VirtualDiskInUseError{ +func NewVirtualDiskNotAllowedForUseError(name string) error { + return VirtualDiskNotAllowedForUseError{ name: name, } } diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go index b7017d9fc1..a199ec291e 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go @@ -40,7 +40,6 @@ import ( virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/cvicondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" - "github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition" ) type ObjectRefVirtualDisk struct { @@ -230,10 +229,10 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, cvi *virtv2.Cluster inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) if inUseCondition.Status == metav1.ConditionTrue { - if inUseCondition.Reason == vdcondition.InUseByVirtualMachine { - return NewVirtualDiskInUseError(vd.Name) + if inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() { + return nil } } - return nil + return NewVirtualDiskNotAllowedForUseError(vd.Name) } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go b/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go index 647e8a6251..cc33352756 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go @@ -89,7 +89,7 @@ func (h DatasourceReadyHandler) Handle(ctx context.Context, vi *virtv2.VirtualIm Reason(vicondition.VirtualDiskNotReady). Message(service.CapitalizeFirstLetter(err.Error() + ".")) return reconcile.Result{}, nil - case errors.As(err, &source.VirtualDiskInUseError{}): + case errors.As(err, &source.VirtualDiskNotAllowedForUseError{}): cb. Status(metav1.ConditionFalse). Reason(vicondition.VirtualDiskInUseInRunningVirtualMachine). diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go index 55de6884e5..3de699998a 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/errors.go @@ -65,16 +65,16 @@ func NewVirtualDiskNotReadyError(name string) error { } } -type VirtualDiskInUseError struct { +type VirtualDiskNotAllowedForUseError struct { name string } -func (e VirtualDiskInUseError) Error() string { - return fmt.Sprintf("reading from the VirtualDisk is not possible while it is in use by the running VirtualMachine/%s", e.name) +func (e VirtualDiskNotAllowedForUseError) Error() string { + return fmt.Sprintf("use of VirtualDisk %s is not allowed, it may be connected to a running VirtualMachine", e.name) } -func NewVirtualDiskInUseError(name string) error { - return VirtualDiskInUseError{ +func NewVirtualDiskNotAllowedForUseError(name string) error { + return VirtualDiskNotAllowedForUseError{ name: name, } } diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go index cebe6eb742..25dac49c5d 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go @@ -381,9 +381,10 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, vi *virtv2.VirtualI inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) if inUseCondition.Status == metav1.ConditionTrue { - if inUseCondition.Reason == vdcondition.InUseByVirtualMachine { - return NewVirtualDiskInUseError(vd.Name) + if inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() { + return nil } } - return nil + + return NewVirtualDiskNotAllowedForUseError(vd.Name) } diff --git a/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go b/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go index 6c524f9f95..c2a3ddb70a 100644 --- a/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go @@ -35,6 +35,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vi/internal/watcher" "github.com/deckhouse/virtualization-controller/pkg/controller/watchers" @@ -221,8 +222,8 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr return false } - oldInUseCondition, _ := service.GetCondition(vdcondition.InUseType, oldVD.Status.Conditions) - newInUseCondition, _ := service.GetCondition(vdcondition.InUseType, newVD.Status.Conditions) + oldInUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, oldVD.Status.Conditions) + newInUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, newVD.Status.Conditions) if oldVD.Status.Phase != newVD.Status.Phase || len(oldVD.Status.AttachedToVirtualMachines) != len(newVD.Status.AttachedToVirtualMachines) || oldInUseCondition.Status != newInUseCondition.Status { return true diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go index a9028d0632..631e327043 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go @@ -318,9 +318,15 @@ func (h *BlockDeviceHandler) countReadyBlockDevices(vm *virtv2.VirtualMachine, s canStartKVVM = false continue } - readyCondition, _ := service.GetCondition(vdcondition.ReadyType, vd.Status.Conditions) + readyCondition, _ := conditions.GetCondition(vdcondition.ReadyType, vd.Status.Conditions) if readyCondition.Status == metav1.ConditionTrue { - ready++ + inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + if inUseCondition.Status == metav1.ConditionTrue && inUseCondition.Reason == vdcondition.AllowedForVirtualMachineUsage.String() { + ready++ + } else { + msg := fmt.Sprintf("virtual disk %s is not allowed for use in the virtual machine, it may be used to create an image", vd.Name) + warnings = append(warnings, msg) + } } else { msg := fmt.Sprintf("virtual disk %s is waiting for the it's pvc to be bound", vd.Name) warnings = append(warnings, msg) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go index 8855d0a36f..ec2e3480b8 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go @@ -71,8 +71,13 @@ var _ = Describe("BlockDeviceHandler", func() { Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-foo"}, Conditions: []metav1.Condition{ { - Type: vdcondition.ReadyType, - Reason: vdcondition.Ready, + Type: vdcondition.ReadyType.String(), + Reason: vdcondition.Ready.String(), + Status: metav1.ConditionTrue, + }, + { + Type: vdcondition.InUseType.String(), + Reason: vdcondition.AllowedForVirtualMachineUsage.String(), Status: metav1.ConditionTrue, }, }, @@ -85,8 +90,13 @@ var _ = Describe("BlockDeviceHandler", func() { Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-bar"}, Conditions: []metav1.Condition{ { - Type: vdcondition.ReadyType, - Reason: vdcondition.Ready, + Type: vdcondition.ReadyType.String(), + Reason: vdcondition.Ready.String(), + Status: metav1.ConditionTrue, + }, + { + Type: vdcondition.InUseType.String(), + Reason: vdcondition.AllowedForVirtualMachineUsage.String(), Status: metav1.ConditionTrue, }, }, @@ -183,10 +193,15 @@ var _ = Describe("BlockDeviceHandler", func() { vdFoo.Status.Phase = virtv2.DiskProvisioning vdFoo.Status.Conditions = []metav1.Condition{ { - Type: vdcondition.ReadyType, - Reason: vdcondition.Provisioning, + Type: vdcondition.ReadyType.String(), + Reason: vdcondition.Provisioning.String(), Status: metav1.ConditionFalse, }, + { + Type: vdcondition.InUseType.String(), + Reason: vdcondition.AllowedForVirtualMachineUsage.String(), + Status: metav1.ConditionTrue, + }, } state := getBlockDevicesState(vi, cvi, vdFoo, vdBar) ready, canStart, warnings := h.countReadyBlockDevices(vm, state, logger) diff --git a/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go b/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go index 14c8ea8ac0..b6e55b9cd8 100644 --- a/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vm/vm_reconciler.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal/state" @@ -218,8 +219,8 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr return false } - oldInUseCondition, _ := service.GetCondition(vdcondition.InUseType, oldVd.Status.Conditions) - newInUseCondition, _ := service.GetCondition(vdcondition.InUseType, newVd.Status.Conditions) + oldInUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, oldVd.Status.Conditions) + newInUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, newVd.Status.Conditions) if oldVd.Status.Phase != newVd.Status.Phase || oldInUseCondition.Status != newInUseCondition.Status { return true From aa7a52b41baa530878f32ad7ad26d05fc63b63e1 Mon Sep 17 00:00:00 2001 From: "dmitry.lopatin" Date: Fri, 15 Nov 2024 23:36:16 +0300 Subject: [PATCH 3/5] fix(vd): add new handler Signed-off-by: dmitry.lopatin --- .../cvi/internal/source/object_ref_vd.go | 6 +- .../pkg/controller/vd/internal/inuse.go | 180 +++++++ .../pkg/controller/vd/internal/inuse_test.go | 492 ++++++++++++++++++ .../pkg/controller/vd/vd_controller.go | 1 + .../vi/internal/source/object_ref_vd.go | 6 +- .../controller/vm/internal/block_device.go | 32 +- .../vm/internal/block_devices_test.go | 79 +++ .../pkg/controller/watchers/cvi_filter.go | 3 +- .../pkg/controller/watchers/vi_filter.go | 3 +- 9 files changed, 783 insertions(+), 19 deletions(-) create mode 100644 images/virtualization-artifact/pkg/controller/vd/internal/inuse.go create mode 100644 images/virtualization-artifact/pkg/controller/vd/internal/inuse_test.go diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go index a199ec291e..384c0372ec 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go @@ -228,10 +228,8 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, cvi *virtv2.Cluster } inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - if inUseCondition.Status == metav1.ConditionTrue { - if inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() { - return nil - } + if inUseCondition.Status == metav1.ConditionTrue && inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() { + return nil } return NewVirtualDiskNotAllowedForUseError(vd.Name) diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go b/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go new file mode 100644 index 0000000000..03f49e0d32 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go @@ -0,0 +1,180 @@ +/* +Copyright 2024 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package internal + +import ( + "context" + "fmt" + "slices" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + virtv1 "kubevirt.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + "github.com/deckhouse/virtualization-controller/pkg/sdk/framework/helper" + virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" +) + +type InUseHandler struct { + client client.Client +} + +func NewInUseHandler(client client.Client) *InUseHandler { + return &InUseHandler{ + client: client, + } +} + +func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (reconcile.Result, error) { + inUseCondition, ok := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + if !ok { + cb := conditions.NewConditionBuilder(vdcondition.InUseType). + Status(metav1.ConditionUnknown). + Reason(conditions.ReasonUnknown). + Generation(vd.Generation) + conditions.SetCondition(cb, &vd.Status.Conditions) + inUseCondition = cb.Condition() + } + + allowUseForVM, allowUseForImage := false, false + + var vms virtv2.VirtualMachineList + err := h.client.List(ctx, &vms, &client.ListOptions{ + Namespace: vd.GetNamespace(), + }) + if err != nil { + return reconcile.Result{}, fmt.Errorf("error getting virtual machines: %w", err) + } + + for _, vm := range vms.Items { + if h.isVDAttachedToVM(vd.GetName(), vm) { + if vm.Status.Phase != virtv2.MachineStopped { + allowUseForVM = isVMReady(vm.Status.Conditions) + + if allowUseForVM { + break + } + } else { + kvvm, err := helper.FetchObject(ctx, types.NamespacedName{Name: vm.Name, Namespace: vm.Namespace}, h.client, &virtv1.VirtualMachine{}) + if err != nil { + return reconcile.Result{}, fmt.Errorf("error getting kvvms: %w", err) + } + + if kvvm != nil && kvvm.Status.StateChangeRequests != nil { + allowUseForVM = true + break + } + } + } + } + + var vis virtv2.VirtualImageList + err = h.client.List(ctx, &vis, &client.ListOptions{ + Namespace: vd.GetNamespace(), + }) + if err != nil { + return reconcile.Result{}, fmt.Errorf("error getting virtual images: %w", err) + } + + allowedPhases := []virtv2.ImagePhase{virtv2.ImageProvisioning, virtv2.ImagePending} + + for _, vi := range vis.Items { + if slices.Contains(allowedPhases, vi.Status.Phase) && vi.Spec.DataSource.Type == virtv2.DataSourceTypeObjectRef && vi.Spec.DataSource.ObjectRef != nil && vi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind { + allowUseForImage = true + break + } + } + + var cvis virtv2.ClusterVirtualImageList + err = h.client.List(ctx, &cvis, &client.ListOptions{}) + if err != nil { + return reconcile.Result{}, fmt.Errorf("error getting cluster virtual images: %w", err) + } + for _, cvi := range cvis.Items { + if slices.Contains(allowedPhases, cvi.Status.Phase) && cvi.Spec.DataSource.Type == virtv2.DataSourceTypeObjectRef && cvi.Spec.DataSource.ObjectRef != nil && cvi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind { + allowUseForImage = true + } + } + + cb := conditions.NewConditionBuilder(vdcondition.InUseType) + + switch { + case allowUseForVM && inUseCondition.Status == metav1.ConditionUnknown: + if inUseCondition.Reason != vdcondition.AllowedForVirtualMachineUsage.String() { + cb. + Generation(vd.Generation). + Status(metav1.ConditionTrue). + Reason(vdcondition.AllowedForVirtualMachineUsage). + Message("") + conditions.SetCondition(cb, &vd.Status.Conditions) + } + case allowUseForImage && inUseCondition.Status == metav1.ConditionUnknown: + if inUseCondition.Reason != vdcondition.AllowedForImageUsage.String() { + cb. + Generation(vd.Generation). + Status(metav1.ConditionTrue). + Reason(vdcondition.AllowedForImageUsage). + Message("") + conditions.SetCondition(cb, &vd.Status.Conditions) + } + default: + if inUseCondition.Reason == vdcondition.AllowedForVirtualMachineUsage.String() && !allowUseForVM || inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() && !allowUseForImage { + cb. + Generation(vd.Generation). + Status(metav1.ConditionUnknown). + Reason(conditions.ReasonUnknown). + Message("") + conditions.SetCondition(cb, &vd.Status.Conditions) + } + + if allowUseForImage || allowUseForVM { + return reconcile.Result{Requeue: true}, nil + } + } + + return reconcile.Result{}, nil +} + +func (h InUseHandler) isVDAttachedToVM(vdName string, vm virtv2.VirtualMachine) bool { + for _, bda := range vm.Status.BlockDeviceRefs { + if bda.Kind == virtv2.DiskDevice && bda.Name == vdName { + return true + } + } + + return false +} + +func isVMReady(conditions []metav1.Condition) bool { + critConditions := []string{ + vmcondition.TypeIPAddressReady.String(), + vmcondition.TypeClassReady.String(), + } + + for _, c := range conditions { + if slices.Contains(critConditions, c.Type) && c.Status == metav1.ConditionFalse { + return false + } + } + + return true +} diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/inuse_test.go b/images/virtualization-artifact/pkg/controller/vd/internal/inuse_test.go new file mode 100644 index 0000000000..98d1025321 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/vd/internal/inuse_test.go @@ -0,0 +1,492 @@ +/* +Copyright 2024 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package internal + +import ( + "context" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + virtv1 "kubevirt.io/api/core/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" + virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" +) + +var _ = ginkgo.Describe("InUseHandler", func() { + var ( + scheme *runtime.Scheme + ctx context.Context + handler *InUseHandler + ) + + ginkgo.BeforeEach(func() { + scheme = runtime.NewScheme() + gomega.Expect(clientgoscheme.AddToScheme(scheme)).To(gomega.Succeed()) + gomega.Expect(virtv2.AddToScheme(scheme)).To(gomega.Succeed()) + gomega.Expect(virtv1.AddToScheme(scheme)).To(gomega.Succeed()) + + ctx = context.TODO() + }) + + ginkgo.Context("when VirtualDisk is not in use", func() { + ginkgo.It("must set status Unknown and reason Unknown", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionUnknown)) + }) + }) + + ginkgo.Context("when VirtualDisk is used by running VirtualMachine", func() { + ginkgo.It("must set status True and reason AllowedForVirtualMachineUsage", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{}, + }, + } + + vm := &virtv2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vm", + Namespace: "default", + }, + Spec: virtv2.VirtualMachineSpec{ + BlockDeviceRefs: []virtv2.BlockDeviceSpecRef{ + { + Kind: virtv2.DiskDevice, + Name: vd.Name, + }, + }, + }, + Status: virtv2.VirtualMachineStatus{ + Phase: virtv2.MachineRunning, + BlockDeviceRefs: []virtv2.BlockDeviceStatusRef{ + { + Kind: virtv2.DiskDevice, + Name: vd.Name, + }, + }, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd, vm).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) + gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForVirtualMachineUsage.String())) + }) + }) + + ginkgo.Context("when VirtualDisk is used by not ready VirtualMachine", func() { + ginkgo.It("must set status True and reason AllowedForVirtualMachineUsage", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{}, + }, + } + + vm := &virtv2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vm", + Namespace: "default", + }, + Status: virtv2.VirtualMachineStatus{ + Conditions: []metav1.Condition{ + { + Type: vmcondition.TypeMigrating.String(), + Status: metav1.ConditionFalse, + }, + { + Type: vmcondition.TypeIPAddressReady.String(), + Status: metav1.ConditionFalse, + }, + }, + BlockDeviceRefs: []virtv2.BlockDeviceStatusRef{ + { + Kind: virtv2.DiskDevice, + Name: vd.Name, + }, + }, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd, vm).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionUnknown)) + }) + }) + + ginkgo.Context("when VirtualDisk is used by VirtualImage", func() { + ginkgo.It("must set status True and reason AllowedForImageUsage", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{}, + }, + } + + vi := &virtv2.VirtualImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vi", + Namespace: "default", + }, + Spec: virtv2.VirtualImageSpec{ + DataSource: virtv2.VirtualImageDataSource{ + Type: virtv2.DataSourceTypeObjectRef, + ObjectRef: &virtv2.VirtualImageObjectRef{ + Kind: virtv2.VirtualDiskKind, + Name: "test-vd", + }, + }, + }, + Status: virtv2.VirtualImageStatus{ + Phase: virtv2.ImageProvisioning, + Conditions: []metav1.Condition{}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd, vi).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) + gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForImageUsage.String())) + }) + }) + + ginkgo.Context("when VirtualDisk is used by ClusterVirtualImage", func() { + ginkgo.It("must set status True and reason AllowedForImageUsage", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{}, + }, + } + + cvi := &virtv2.ClusterVirtualImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vi", + Namespace: "default", + }, + Spec: virtv2.ClusterVirtualImageSpec{ + DataSource: virtv2.ClusterVirtualImageDataSource{ + Type: virtv2.DataSourceTypeObjectRef, + ObjectRef: &virtv2.ClusterVirtualImageObjectRef{ + Kind: virtv2.VirtualDiskKind, + Name: "test-vd", + Namespace: "default", + }, + }, + }, + Status: virtv2.ClusterVirtualImageStatus{ + Phase: virtv2.ImageProvisioning, + Conditions: []metav1.Condition{}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd, cvi).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) + gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForImageUsage.String())) + }) + }) + + ginkgo.Context("when VirtualDisk is used by VirtualImage and VirtualMachine", func() { + ginkgo.It("must set status True and reason AllowedForVirtualMachineUsage", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{}, + }, + } + + vi := &virtv2.VirtualImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vi", + Namespace: "default", + }, + Spec: virtv2.VirtualImageSpec{ + DataSource: virtv2.VirtualImageDataSource{ + Type: virtv2.DataSourceTypeObjectRef, + ObjectRef: &virtv2.VirtualImageObjectRef{ + Kind: virtv2.VirtualDiskKind, + Name: "test-vd", + }, + }, + }, + Status: virtv2.VirtualImageStatus{ + Phase: virtv2.ImageProvisioning, + Conditions: []metav1.Condition{}, + }, + } + + vm := &virtv2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vm", + Namespace: "default", + }, + Status: virtv2.VirtualMachineStatus{ + BlockDeviceRefs: []virtv2.BlockDeviceStatusRef{ + { + Kind: virtv2.DiskDevice, + Name: vd.Name, + }, + }, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd, vi, vm).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) + gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForVirtualMachineUsage.String())) + }) + }) + + ginkgo.Context("when VirtualDisk is used by VirtualMachine after create image", func() { + ginkgo.It("must set status True and reason AllowedForVirtualMachineUsage", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{ + { + Type: vdcondition.InUseType.String(), + Reason: conditions.ReasonUnknown.String(), + Status: metav1.ConditionUnknown, + }, + }, + }, + } + + vm := &virtv2.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vm", + Namespace: "default", + }, + Status: virtv2.VirtualMachineStatus{ + BlockDeviceRefs: []virtv2.BlockDeviceStatusRef{ + { + Kind: virtv2.DiskDevice, + Name: vd.Name, + }, + }, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd, vm).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) + gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForVirtualMachineUsage.String())) + }) + }) + + ginkgo.Context("when VirtualDisk is used by VirtualImage after running VM", func() { + ginkgo.It("must set status True and reason AllowedForImageUsage", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{ + { + Type: vdcondition.InUseType.String(), + Reason: conditions.ReasonUnknown.String(), + Status: metav1.ConditionUnknown, + }, + }, + }, + } + + vi := &virtv2.VirtualImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vi", + Namespace: "default", + }, + Spec: virtv2.VirtualImageSpec{ + DataSource: virtv2.VirtualImageDataSource{ + Type: virtv2.DataSourceTypeObjectRef, + ObjectRef: &virtv2.VirtualImageObjectRef{ + Kind: virtv2.VirtualDiskKind, + Name: "test-vd", + }, + }, + }, + Status: virtv2.VirtualImageStatus{ + Phase: virtv2.ImageProvisioning, + Conditions: []metav1.Condition{}, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd, vi).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) + gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForImageUsage.String())) + }) + }) + + ginkgo.Context("when VirtualDisk is not in use after create image", func() { + ginkgo.It("must set status Unknown and reason Unknown", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{ + { + Type: vdcondition.InUseType.String(), + Reason: vdcondition.AllowedForImageUsage.String(), + Status: metav1.ConditionTrue, + }, + }, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionUnknown)) + }) + }) + + ginkgo.Context("when VirtualDisk is not in use after running VM", func() { + ginkgo.It("must set status Unknown and reason Unknown", func() { + vd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vd", + Namespace: "default", + }, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{ + { + Type: vdcondition.InUseType.String(), + Reason: vdcondition.AllowedForVirtualMachineUsage.String(), + Status: metav1.ConditionTrue, + }, + }, + }, + } + + k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(vd).Build() + handler = NewInUseHandler(k8sClient) + + result, err := handler.Handle(ctx, vd) + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + + cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + gomega.Expect(cond).ToNot(gomega.BeNil()) + gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionUnknown)) + }) + }) +}) diff --git a/images/virtualization-artifact/pkg/controller/vd/vd_controller.go b/images/virtualization-artifact/pkg/controller/vd/vd_controller.go index ab1a093a47..b9cc60d909 100644 --- a/images/virtualization-artifact/pkg/controller/vd/vd_controller.go +++ b/images/virtualization-artifact/pkg/controller/vd/vd_controller.go @@ -84,6 +84,7 @@ func NewController( internal.NewDeletionHandler(sources), internal.NewAttacheeHandler(mgr.GetClient()), internal.NewStatsHandler(stat, importer, uploader), + internal.NewInUseHandler(mgr.GetClient()), ) vdController, err := controller.New(ControllerName, mgr, controller.Options{ diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go index 25dac49c5d..55ed3aee0e 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go @@ -380,10 +380,8 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, vi *virtv2.VirtualI } inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - if inUseCondition.Status == metav1.ConditionTrue { - if inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() { - return nil - } + if inUseCondition.Status == metav1.ConditionTrue && inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() { + return nil } return NewVirtualDiskNotAllowedForUseError(vd.Name) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go index 631e327043..700c618fab 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go @@ -164,6 +164,19 @@ func (h *BlockDeviceHandler) Handle(ctx context.Context, s state.VirtualMachineS return reconcile.Result{RequeueAfter: 60 * time.Second}, nil } + vds, err := s.VirtualDisksByName(ctx) + if err != nil { + return reconcile.Result{}, err + } + + if !h.checkAllowVirtualDisks(vds) { + mgr.Update(cb.Status(metav1.ConditionFalse). + Reason(vmcondition.ReasonBlockDevicesNotReady). + Message("virtual disks are not allowed for use in the virtual machine, it may be used to create an images").Condition()) + changed.Status.Conditions = mgr.Generate() + return reconcile.Result{}, nil + } + mgr.Update(cb.Status(metav1.ConditionTrue). Reason(vmcondition.ReasonBlockDevicesReady). Condition()) @@ -171,6 +184,17 @@ func (h *BlockDeviceHandler) Handle(ctx context.Context, s state.VirtualMachineS return reconcile.Result{}, nil } +func (h *BlockDeviceHandler) checkAllowVirtualDisks(vds map[string]*virtv2.VirtualDisk) bool { + for _, vd := range vds { + inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + if inUseCondition.Status != metav1.ConditionTrue || inUseCondition.Reason != vdcondition.AllowedForVirtualMachineUsage.String() { + return false + } + } + + return true +} + func (h *BlockDeviceHandler) Name() string { return nameBlockDeviceHandler } @@ -320,13 +344,7 @@ func (h *BlockDeviceHandler) countReadyBlockDevices(vm *virtv2.VirtualMachine, s } readyCondition, _ := conditions.GetCondition(vdcondition.ReadyType, vd.Status.Conditions) if readyCondition.Status == metav1.ConditionTrue { - inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - if inUseCondition.Status == metav1.ConditionTrue && inUseCondition.Reason == vdcondition.AllowedForVirtualMachineUsage.String() { - ready++ - } else { - msg := fmt.Sprintf("virtual disk %s is not allowed for use in the virtual machine, it may be used to create an image", vd.Name) - warnings = append(warnings, msg) - } + ready++ } else { msg := fmt.Sprintf("virtual disk %s is waiting for the it's pvc to be bound", vd.Name) warnings = append(warnings, msg) diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go index ec2e3480b8..f8144d245a 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go @@ -25,6 +25,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" ) @@ -133,6 +134,84 @@ var _ = Describe("BlockDeviceHandler", func() { }) }) + Context("BlockDevices are not ready, because VirtualDisk is not allowed", func() { + It("return false", func() { + anyVd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{Name: "anyVd"}, + Status: virtv2.VirtualDiskStatus{ + Phase: virtv2.DiskReady, + Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-foo"}, + Conditions: []metav1.Condition{ + { + Type: vdcondition.ReadyType.String(), + Reason: vdcondition.Ready.String(), + Status: metav1.ConditionTrue, + }, + { + Type: vdcondition.InUseType.String(), + Reason: conditions.ReasonUnknown.String(), + Status: metav1.ConditionUnknown, + }, + }, + }, + } + + vds := map[string]*virtv2.VirtualDisk{ + vdFoo.Name: vdFoo, + vdBar.Name: vdBar, + anyVd.Name: anyVd, + } + + allowed := h.checkAllowVirtualDisks(vds) + Expect(allowed).To(BeFalse()) + }) + }) + + Context("BlockDevices are not ready, because VirtualDisk using for create image", func() { + It("return false", func() { + anyVd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{Name: "anyVd"}, + Status: virtv2.VirtualDiskStatus{ + Phase: virtv2.DiskReady, + Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-foo"}, + Conditions: []metav1.Condition{ + { + Type: vdcondition.ReadyType.String(), + Reason: vdcondition.Ready.String(), + Status: metav1.ConditionTrue, + }, + { + Type: vdcondition.InUseType.String(), + Reason: vdcondition.AllowedForImageUsage.String(), + Status: metav1.ConditionTrue, + }, + }, + }, + } + + vds := map[string]*virtv2.VirtualDisk{ + vdFoo.Name: vdFoo, + vdBar.Name: vdBar, + anyVd.Name: anyVd, + } + + allowed := h.checkAllowVirtualDisks(vds) + Expect(allowed).To(BeFalse()) + }) + }) + + Context("BlockDevices are ready, because VirtualDisk is allowed", func() { + It("return false", func() { + vds := map[string]*virtv2.VirtualDisk{ + vdFoo.Name: vdFoo, + vdBar.Name: vdBar, + } + + allowed := h.checkAllowVirtualDisks(vds) + Expect(allowed).To(BeTrue()) + }) + }) + Context("Image is not ready", func() { It("VirtualImage not ready: cannot start, no warnings", func() { vi.Status.Phase = virtv2.ImagePending diff --git a/images/virtualization-artifact/pkg/controller/watchers/cvi_filter.go b/images/virtualization-artifact/pkg/controller/watchers/cvi_filter.go index 4b3f461252..5cc4d2ef39 100644 --- a/images/virtualization-artifact/pkg/controller/watchers/cvi_filter.go +++ b/images/virtualization-artifact/pkg/controller/watchers/cvi_filter.go @@ -52,6 +52,5 @@ func (f ClusterVirtualImageFilter) FilterUpdateEvents(e event.UpdateEvent) bool return false } - // Triggered only if the resource phase changed to Ready. - return oldCVI.Status.Phase != newCVI.Status.Phase && newCVI.Status.Phase == virtv2.ImageReady + return oldCVI.Status.Phase != newCVI.Status.Phase } diff --git a/images/virtualization-artifact/pkg/controller/watchers/vi_filter.go b/images/virtualization-artifact/pkg/controller/watchers/vi_filter.go index 04895fcd8b..cccde70bac 100644 --- a/images/virtualization-artifact/pkg/controller/watchers/vi_filter.go +++ b/images/virtualization-artifact/pkg/controller/watchers/vi_filter.go @@ -52,6 +52,5 @@ func (f VirtualImageFilter) FilterUpdateEvents(e event.UpdateEvent) bool { return false } - // Triggered only if the resource phase changed to Ready. - return oldVI.Status.Phase != newVI.Status.Phase && newVI.Status.Phase == virtv2.ImageReady + return oldVI.Status.Phase != newVI.Status.Phase } From c7c091c7313365f76237beeeb26c2a721d7289cd Mon Sep 17 00:00:00 2001 From: "dmitry.lopatin" Date: Tue, 3 Dec 2024 22:04:15 +0300 Subject: [PATCH 4/5] fix Signed-off-by: dmitry.lopatin --- .../pkg/controller/vd/internal/inuse.go | 43 ++++++++++++++----- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go b/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go index 03f49e0d32..050895c3c7 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go @@ -21,14 +21,17 @@ import ( "fmt" "slices" + 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/labels" "k8s.io/apimachinery/pkg/types" virtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/sdk/framework/helper" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" @@ -74,7 +77,7 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon break } } else { - kvvm, err := helper.FetchObject(ctx, types.NamespacedName{Name: vm.Name, Namespace: vm.Namespace}, h.client, &virtv1.VirtualMachine{}) + kvvm, err := object.FetchObject(ctx, types.NamespacedName{Name: vm.Name, Namespace: vm.Namespace}, h.client, &virtv1.VirtualMachine{}) if err != nil { return reconcile.Result{}, fmt.Errorf("error getting kvvms: %w", err) } @@ -83,6 +86,22 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon allowUseForVM = true break } + + podList := corev1.PodList{} + err = h.client.List(ctx, &podList, &client.ListOptions{ + Namespace: vm.GetNamespace(), + LabelSelector: labels.SelectorFromSet(map[string]string{virtv1.VirtualMachineNameLabel: vm.GetName()}), + }) + if err != nil && !k8serrors.IsNotFound(err) { + return reconcile.Result{}, fmt.Errorf("unable to list virt-launcher Pod for VM %q: %w", vm.GetName(), err) + } + + for _, pod := range podList.Items { + if pod.Status.Phase == corev1.PodRunning { + allowUseForVM = true + break + } + } } } } @@ -137,17 +156,19 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon conditions.SetCondition(cb, &vd.Status.Conditions) } default: - if inUseCondition.Reason == vdcondition.AllowedForVirtualMachineUsage.String() && !allowUseForVM || inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() && !allowUseForImage { - cb. - Generation(vd.Generation). - Status(metav1.ConditionUnknown). - Reason(conditions.ReasonUnknown). - Message("") - conditions.SetCondition(cb, &vd.Status.Conditions) + needChange := false + + if inUseCondition.Reason == vdcondition.AllowedForVirtualMachineUsage.String() && !allowUseForVM { + needChange = true } - if allowUseForImage || allowUseForVM { - return reconcile.Result{Requeue: true}, nil + if inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() && !allowUseForImage { + needChange = true + } + + if needChange { + cb.Generation(vd.Generation).Status(metav1.ConditionUnknown).Reason(conditions.ReasonUnknown).Message("") + conditions.SetCondition(cb, &vd.Status.Conditions) } } From 053ed6e96838206bbcde9959da54a68ec7c4c905 Mon Sep 17 00:00:00 2001 From: "dmitry.lopatin" Date: Wed, 11 Dec 2024 23:41:09 +0300 Subject: [PATCH 5/5] ++ Signed-off-by: dmitry.lopatin --- api/core/v1alpha2/vdcondition/condition.go | 2 +- .../cvi/internal/source/object_ref_vd.go | 4 +- .../pkg/controller/vd/internal/inuse.go | 29 +-- .../pkg/controller/vd/internal/inuse_test.go | 146 +++++++------- .../vi/internal/source/object_ref_vd.go | 4 +- .../controller/vm/internal/block_device.go | 10 +- .../vm/internal/block_devices_test.go | 185 ++++++++++-------- .../pkg/controller/watchers/vd_enqueuer.go | 96 ++++++--- 8 files changed, 275 insertions(+), 201 deletions(-) diff --git a/api/core/v1alpha2/vdcondition/condition.go b/api/core/v1alpha2/vdcondition/condition.go index d17e6ab0da..d51db69788 100644 --- a/api/core/v1alpha2/vdcondition/condition.go +++ b/api/core/v1alpha2/vdcondition/condition.go @@ -34,7 +34,7 @@ const ( SnapshottingType Type = "Snapshotting" // StorageClassReadyType indicates whether the storage class is ready. StorageClassReadyType Type = "StorageClassReady" - // InUseType indicates whether the VirtualDisk is attached to a running VirtualMachine or is being used in a process of a VirtualImage creation. + // InUseType indicates whether the VirtualDisk is attached to a running VirtualMachine or is being used in a process of an image creation. InUseType Type = "InUse" ) diff --git a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go index 384c0372ec..3a5ba70829 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref_vd.go @@ -228,7 +228,9 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, cvi *virtv2.Cluster } inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - if inUseCondition.Status == metav1.ConditionTrue && inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() { + if inUseCondition.Status == metav1.ConditionTrue && + inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() && + inUseCondition.ObservedGeneration == vd.Status.ObservedGeneration { return nil } diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go b/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go index 050895c3c7..c9265ee2e7 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/inuse.go @@ -22,7 +22,6 @@ import ( "slices" 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/labels" "k8s.io/apimachinery/pkg/types" @@ -71,7 +70,7 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon for _, vm := range vms.Items { if h.isVDAttachedToVM(vd.GetName(), vm) { if vm.Status.Phase != virtv2.MachineStopped { - allowUseForVM = isVMReady(vm.Status.Conditions) + allowUseForVM = isVMCanStart(vm.Status.Conditions) if allowUseForVM { break @@ -92,7 +91,7 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon Namespace: vm.GetNamespace(), LabelSelector: labels.SelectorFromSet(map[string]string{virtv1.VirtualMachineNameLabel: vm.GetName()}), }) - if err != nil && !k8serrors.IsNotFound(err) { + if err != nil { return reconcile.Result{}, fmt.Errorf("unable to list virt-launcher Pod for VM %q: %w", vm.GetName(), err) } @@ -117,7 +116,11 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon allowedPhases := []virtv2.ImagePhase{virtv2.ImageProvisioning, virtv2.ImagePending} for _, vi := range vis.Items { - if slices.Contains(allowedPhases, vi.Status.Phase) && vi.Spec.DataSource.Type == virtv2.DataSourceTypeObjectRef && vi.Spec.DataSource.ObjectRef != nil && vi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind { + if slices.Contains(allowedPhases, vi.Status.Phase) && + vi.Spec.DataSource.Type == virtv2.DataSourceTypeObjectRef && + vi.Spec.DataSource.ObjectRef != nil && + vi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind && + vi.Spec.DataSource.ObjectRef.Name == vd.Name { allowUseForImage = true break } @@ -129,13 +132,16 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon return reconcile.Result{}, fmt.Errorf("error getting cluster virtual images: %w", err) } for _, cvi := range cvis.Items { - if slices.Contains(allowedPhases, cvi.Status.Phase) && cvi.Spec.DataSource.Type == virtv2.DataSourceTypeObjectRef && cvi.Spec.DataSource.ObjectRef != nil && cvi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind { + if slices.Contains(allowedPhases, cvi.Status.Phase) && + cvi.Spec.DataSource.Type == virtv2.DataSourceTypeObjectRef && + cvi.Spec.DataSource.ObjectRef != nil && + cvi.Spec.DataSource.ObjectRef.Kind == virtv2.VirtualDiskKind && + cvi.Spec.DataSource.ObjectRef.Name == vd.Name { allowUseForImage = true } } cb := conditions.NewConditionBuilder(vdcondition.InUseType) - switch { case allowUseForVM && inUseCondition.Status == metav1.ConditionUnknown: if inUseCondition.Reason != vdcondition.AllowedForVirtualMachineUsage.String() { @@ -156,19 +162,20 @@ func (h InUseHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (recon conditions.SetCondition(cb, &vd.Status.Conditions) } default: - needChange := false + setUnknown := false if inUseCondition.Reason == vdcondition.AllowedForVirtualMachineUsage.String() && !allowUseForVM { - needChange = true + setUnknown = true } if inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() && !allowUseForImage { - needChange = true + setUnknown = true } - if needChange { + if setUnknown { cb.Generation(vd.Generation).Status(metav1.ConditionUnknown).Reason(conditions.ReasonUnknown).Message("") conditions.SetCondition(cb, &vd.Status.Conditions) + return reconcile.Result{Requeue: true}, nil } } @@ -185,7 +192,7 @@ func (h InUseHandler) isVDAttachedToVM(vdName string, vm virtv2.VirtualMachine) return false } -func isVMReady(conditions []metav1.Condition) bool { +func isVMCanStart(conditions []metav1.Condition) bool { critConditions := []string{ vmcondition.TypeIPAddressReady.String(), vmcondition.TypeClassReady.String(), diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/inuse_test.go b/images/virtualization-artifact/pkg/controller/vd/internal/inuse_test.go index 98d1025321..f62fed0021 100644 --- a/images/virtualization-artifact/pkg/controller/vd/internal/inuse_test.go +++ b/images/virtualization-artifact/pkg/controller/vd/internal/inuse_test.go @@ -19,8 +19,8 @@ package internal import ( "context" - "github.com/onsi/ginkgo/v2" - "github.com/onsi/gomega" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -34,24 +34,24 @@ import ( "github.com/deckhouse/virtualization/api/core/v1alpha2/vmcondition" ) -var _ = ginkgo.Describe("InUseHandler", func() { +var _ = Describe("InUseHandler", func() { var ( scheme *runtime.Scheme ctx context.Context handler *InUseHandler ) - ginkgo.BeforeEach(func() { + BeforeEach(func() { scheme = runtime.NewScheme() - gomega.Expect(clientgoscheme.AddToScheme(scheme)).To(gomega.Succeed()) - gomega.Expect(virtv2.AddToScheme(scheme)).To(gomega.Succeed()) - gomega.Expect(virtv1.AddToScheme(scheme)).To(gomega.Succeed()) + Expect(clientgoscheme.AddToScheme(scheme)).To(Succeed()) + Expect(virtv2.AddToScheme(scheme)).To(Succeed()) + Expect(virtv1.AddToScheme(scheme)).To(Succeed()) ctx = context.TODO() }) - ginkgo.Context("when VirtualDisk is not in use", func() { - ginkgo.It("must set status Unknown and reason Unknown", func() { + Context("when VirtualDisk is not in use", func() { + It("must set status Unknown and reason Unknown", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -66,17 +66,17 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionUnknown)) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionUnknown)) }) }) - ginkgo.Context("when VirtualDisk is used by running VirtualMachine", func() { - ginkgo.It("must set status True and reason AllowedForVirtualMachineUsage", func() { + Context("when VirtualDisk is used by running VirtualMachine", func() { + It("must set status True and reason AllowedForVirtualMachineUsage", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -115,18 +115,18 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) - gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForVirtualMachineUsage.String())) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(vdcondition.AllowedForVirtualMachineUsage.String())) }) }) - ginkgo.Context("when VirtualDisk is used by not ready VirtualMachine", func() { - ginkgo.It("must set status True and reason AllowedForVirtualMachineUsage", func() { + Context("when VirtualDisk is used by not ready VirtualMachine", func() { + It("it sets Unknown", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -166,17 +166,17 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionUnknown)) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionUnknown)) }) }) - ginkgo.Context("when VirtualDisk is used by VirtualImage", func() { - ginkgo.It("must set status True and reason AllowedForImageUsage", func() { + Context("when VirtualDisk is used by VirtualImage", func() { + It("must set status True and reason AllowedForImageUsage", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -211,18 +211,18 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) - gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForImageUsage.String())) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(vdcondition.AllowedForImageUsage.String())) }) }) - ginkgo.Context("when VirtualDisk is used by ClusterVirtualImage", func() { - ginkgo.It("must set status True and reason AllowedForImageUsage", func() { + Context("when VirtualDisk is used by ClusterVirtualImage", func() { + It("must set status True and reason AllowedForImageUsage", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -258,18 +258,18 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) - gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForImageUsage.String())) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(vdcondition.AllowedForImageUsage.String())) }) }) - ginkgo.Context("when VirtualDisk is used by VirtualImage and VirtualMachine", func() { - ginkgo.It("must set status True and reason AllowedForVirtualMachineUsage", func() { + Context("when VirtualDisk is used by VirtualImage and VirtualMachine", func() { + It("must set status True and reason AllowedForVirtualMachineUsage", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -319,18 +319,18 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) - gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForVirtualMachineUsage.String())) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(vdcondition.AllowedForVirtualMachineUsage.String())) }) }) - ginkgo.Context("when VirtualDisk is used by VirtualMachine after create image", func() { - ginkgo.It("must set status True and reason AllowedForVirtualMachineUsage", func() { + Context("when VirtualDisk is used by VirtualMachine after create image", func() { + It("must set status True and reason AllowedForVirtualMachineUsage", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -366,18 +366,18 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) - gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForVirtualMachineUsage.String())) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(vdcondition.AllowedForVirtualMachineUsage.String())) }) }) - ginkgo.Context("when VirtualDisk is used by VirtualImage after running VM", func() { - ginkgo.It("must set status True and reason AllowedForImageUsage", func() { + Context("when VirtualDisk is used by VirtualImage after running VM", func() { + It("must set status True and reason AllowedForImageUsage", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -418,18 +418,18 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionTrue)) - gomega.Expect(cond.Reason).To(gomega.Equal(vdcondition.AllowedForImageUsage.String())) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(vdcondition.AllowedForImageUsage.String())) }) }) - ginkgo.Context("when VirtualDisk is not in use after create image", func() { - ginkgo.It("must set status Unknown and reason Unknown", func() { + Context("when VirtualDisk is not in use after image creation", func() { + It("must set status Unknown and reason Unknown", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -450,17 +450,17 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{Requeue: true})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionUnknown)) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionUnknown)) }) }) - ginkgo.Context("when VirtualDisk is not in use after running VM", func() { - ginkgo.It("must set status Unknown and reason Unknown", func() { + Context("when VirtualDisk is not in use after VM deletion", func() { + It("must set status Unknown and reason Unknown", func() { vd := &virtv2.VirtualDisk{ ObjectMeta: metav1.ObjectMeta{ Name: "test-vd", @@ -481,12 +481,12 @@ var _ = ginkgo.Describe("InUseHandler", func() { handler = NewInUseHandler(k8sClient) result, err := handler.Handle(ctx, vd) - gomega.Expect(err).ToNot(gomega.HaveOccurred()) - gomega.Expect(result).To(gomega.Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(Equal(ctrl.Result{Requeue: true})) cond, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - gomega.Expect(cond).ToNot(gomega.BeNil()) - gomega.Expect(cond.Status).To(gomega.Equal(metav1.ConditionUnknown)) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionUnknown)) }) }) }) diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go index 55ed3aee0e..9e857fa4ed 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/source/object_ref_vd.go @@ -380,7 +380,9 @@ func (ds ObjectRefVirtualDisk) Validate(ctx context.Context, vi *virtv2.VirtualI } inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - if inUseCondition.Status == metav1.ConditionTrue && inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() { + if inUseCondition.Status == metav1.ConditionTrue && + inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() && + inUseCondition.ObservedGeneration == vd.Status.ObservedGeneration { return nil } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go index 700c618fab..13dc5fc6ce 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_device.go @@ -169,10 +169,10 @@ func (h *BlockDeviceHandler) Handle(ctx context.Context, s state.VirtualMachineS return reconcile.Result{}, err } - if !h.checkAllowVirtualDisks(vds) { + if !h.areVirtualDisksAllowedToUse(vds) { mgr.Update(cb.Status(metav1.ConditionFalse). Reason(vmcondition.ReasonBlockDevicesNotReady). - Message("virtual disks are not allowed for use in the virtual machine, it may be used to create an images").Condition()) + Message("Virtual disks are not allowed to be used in the virtual machine, check where else they are used.").Condition()) changed.Status.Conditions = mgr.Generate() return reconcile.Result{}, nil } @@ -184,10 +184,12 @@ func (h *BlockDeviceHandler) Handle(ctx context.Context, s state.VirtualMachineS return reconcile.Result{}, nil } -func (h *BlockDeviceHandler) checkAllowVirtualDisks(vds map[string]*virtv2.VirtualDisk) bool { +func (h *BlockDeviceHandler) areVirtualDisksAllowedToUse(vds map[string]*virtv2.VirtualDisk) bool { for _, vd := range vds { inUseCondition, _ := conditions.GetCondition(vdcondition.InUseType, vd.Status.Conditions) - if inUseCondition.Status != metav1.ConditionTrue || inUseCondition.Reason != vdcondition.AllowedForVirtualMachineUsage.String() { + if inUseCondition.Status != metav1.ConditionTrue || + inUseCondition.Reason != vdcondition.AllowedForVirtualMachineUsage.String() || + inUseCondition.ObservedGeneration != vd.Status.ObservedGeneration { return false } } diff --git a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go index f8144d245a..30aa9e40bf 100644 --- a/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go +++ b/images/virtualization-artifact/pkg/controller/vm/internal/block_devices_test.go @@ -35,6 +35,113 @@ func TestBlockDeviceHandler(t *testing.T) { RunSpecs(t, "BlockDeviceHandler Suite") } +var _ = Describe("func areVirtualDisksAllowedToUse", func() { + var h *BlockDeviceHandler + var vdFoo *virtv2.VirtualDisk + var vdBar *virtv2.VirtualDisk + + BeforeEach(func() { + h = NewBlockDeviceHandler(nil, &EventRecorderMock{ + EventFunc: func(_ runtime.Object, _, _, _ string) {}, + }) + vdFoo = &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{Name: "vd-foo"}, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{ + { + Type: vdcondition.InUseType.String(), + Reason: vdcondition.AllowedForVirtualMachineUsage.String(), + Status: metav1.ConditionTrue, + }, + }, + }, + } + vdBar = &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{Name: "vd-bar"}, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{ + { + Type: vdcondition.InUseType.String(), + Reason: vdcondition.AllowedForVirtualMachineUsage.String(), + Status: metav1.ConditionTrue, + }, + }, + }, + } + }) + + Context("VirtualDisk is not allowed", func() { + It("returns false", func() { + anyVd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{Name: "anyVd"}, + Status: virtv2.VirtualDiskStatus{ + Conditions: []metav1.Condition{ + { + Type: vdcondition.InUseType.String(), + Reason: conditions.ReasonUnknown.String(), + Status: metav1.ConditionUnknown, + }, + }, + }, + } + + vds := map[string]*virtv2.VirtualDisk{ + vdFoo.Name: vdFoo, + vdBar.Name: vdBar, + anyVd.Name: anyVd, + } + + allowed := h.areVirtualDisksAllowedToUse(vds) + Expect(allowed).To(BeFalse()) + }) + }) + + Context("VirtualDisk is used to create image", func() { + It("returns false", func() { + anyVd := &virtv2.VirtualDisk{ + ObjectMeta: metav1.ObjectMeta{Name: "anyVd"}, + Status: virtv2.VirtualDiskStatus{ + Phase: virtv2.DiskReady, + Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-foo"}, + Conditions: []metav1.Condition{ + { + Type: vdcondition.ReadyType.String(), + Reason: vdcondition.Ready.String(), + Status: metav1.ConditionTrue, + }, + { + Type: vdcondition.InUseType.String(), + Reason: vdcondition.AllowedForImageUsage.String(), + Status: metav1.ConditionTrue, + }, + }, + }, + } + + vds := map[string]*virtv2.VirtualDisk{ + vdFoo.Name: vdFoo, + vdBar.Name: vdBar, + anyVd.Name: anyVd, + } + + allowed := h.areVirtualDisksAllowedToUse(vds) + Expect(allowed).To(BeFalse()) + }) + }) + + Context("VirtualDisk is allowed", func() { + It("returns true", func() { + vds := map[string]*virtv2.VirtualDisk{ + vdFoo.Name: vdFoo, + vdBar.Name: vdBar, + } + + allowed := h.areVirtualDisksAllowedToUse(vds) + Expect(allowed).To(BeTrue()) + }) + }) +}) + var _ = Describe("BlockDeviceHandler", func() { var h *BlockDeviceHandler var logger *slog.Logger @@ -134,84 +241,6 @@ var _ = Describe("BlockDeviceHandler", func() { }) }) - Context("BlockDevices are not ready, because VirtualDisk is not allowed", func() { - It("return false", func() { - anyVd := &virtv2.VirtualDisk{ - ObjectMeta: metav1.ObjectMeta{Name: "anyVd"}, - Status: virtv2.VirtualDiskStatus{ - Phase: virtv2.DiskReady, - Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-foo"}, - Conditions: []metav1.Condition{ - { - Type: vdcondition.ReadyType.String(), - Reason: vdcondition.Ready.String(), - Status: metav1.ConditionTrue, - }, - { - Type: vdcondition.InUseType.String(), - Reason: conditions.ReasonUnknown.String(), - Status: metav1.ConditionUnknown, - }, - }, - }, - } - - vds := map[string]*virtv2.VirtualDisk{ - vdFoo.Name: vdFoo, - vdBar.Name: vdBar, - anyVd.Name: anyVd, - } - - allowed := h.checkAllowVirtualDisks(vds) - Expect(allowed).To(BeFalse()) - }) - }) - - Context("BlockDevices are not ready, because VirtualDisk using for create image", func() { - It("return false", func() { - anyVd := &virtv2.VirtualDisk{ - ObjectMeta: metav1.ObjectMeta{Name: "anyVd"}, - Status: virtv2.VirtualDiskStatus{ - Phase: virtv2.DiskReady, - Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-foo"}, - Conditions: []metav1.Condition{ - { - Type: vdcondition.ReadyType.String(), - Reason: vdcondition.Ready.String(), - Status: metav1.ConditionTrue, - }, - { - Type: vdcondition.InUseType.String(), - Reason: vdcondition.AllowedForImageUsage.String(), - Status: metav1.ConditionTrue, - }, - }, - }, - } - - vds := map[string]*virtv2.VirtualDisk{ - vdFoo.Name: vdFoo, - vdBar.Name: vdBar, - anyVd.Name: anyVd, - } - - allowed := h.checkAllowVirtualDisks(vds) - Expect(allowed).To(BeFalse()) - }) - }) - - Context("BlockDevices are ready, because VirtualDisk is allowed", func() { - It("return false", func() { - vds := map[string]*virtv2.VirtualDisk{ - vdFoo.Name: vdFoo, - vdBar.Name: vdBar, - } - - allowed := h.checkAllowVirtualDisks(vds) - Expect(allowed).To(BeTrue()) - }) - }) - Context("Image is not ready", func() { It("VirtualImage not ready: cannot start, no warnings", func() { vi.Status.Phase = virtv2.ImagePending diff --git a/images/virtualization-artifact/pkg/controller/watchers/vd_enqueuer.go b/images/virtualization-artifact/pkg/controller/watchers/vd_enqueuer.go index bda8a2859e..753a288d03 100644 --- a/images/virtualization-artifact/pkg/controller/watchers/vd_enqueuer.go +++ b/images/virtualization-artifact/pkg/controller/watchers/vd_enqueuer.go @@ -51,8 +51,44 @@ func (w VirtualDiskRequestEnqueuer) GetEnqueueFrom() client.Object { return w.enqueueFromObj } -func (w VirtualDiskRequestEnqueuer) EnqueueRequests(ctx context.Context, obj client.Object) (requests []reconcile.Request) { - // Enqueue CVI or VI specified by the object ref. +func (w VirtualDiskRequestEnqueuer) EnqueueRequestsFromVDs(ctx context.Context, obj client.Object) (requests []reconcile.Request) { + var vds virtv2.VirtualDiskList + err := w.client.List(ctx, &vds) + if err != nil { + w.logger.Error(fmt.Sprintf("failed to list vd: %s", err)) + return + } + + for _, vd := range vds.Items { + dsReady, _ := conditions.GetCondition(vdcondition.DatasourceReadyType, vd.Status.Conditions) + if dsReady.Status == metav1.ConditionTrue { + continue + } + + if vd.Spec.DataSource == nil || vd.Spec.DataSource.Type != virtv2.DataSourceTypeObjectRef { + continue + } + + ref := vd.Spec.DataSource.ObjectRef + + if ref == nil || ref.Kind != w.enqueueFromKind { + continue + } + + if ref.Name == obj.GetName() { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: vd.Namespace, + Name: vd.Name, + }, + }) + } + } + + return +} + +func (w VirtualDiskRequestEnqueuer) EnqueueRequestsFromVIs(obj client.Object) (requests []reconcile.Request) { if w.enqueueFromKind == virtv2.VirtualDiskObjectRefKindVirtualImage { vi, ok := obj.(*virtv2.VirtualImage) if !ok { @@ -68,7 +104,12 @@ func (w VirtualDiskRequestEnqueuer) EnqueueRequests(ctx context.Context, obj cli }, }) } - } else if w.enqueueFromKind == virtv2.VirtualDiskObjectRefKindClusterVirtualImage { + } + return +} + +func (w VirtualDiskRequestEnqueuer) EnqueueRequestsFromCVIs(obj client.Object) (requests []reconcile.Request) { + if w.enqueueFromKind == virtv2.VirtualDiskObjectRefKindClusterVirtualImage { cvi, ok := obj.(*virtv2.ClusterVirtualImage) if !ok { w.logger.Error(fmt.Sprintf("expected a ClusterVirtualImage but got a %T", obj)) @@ -84,39 +125,30 @@ func (w VirtualDiskRequestEnqueuer) EnqueueRequests(ctx context.Context, obj cli }) } } + return +} - var vds virtv2.VirtualDiskList - err := w.client.List(ctx, &vds) - if err != nil { - w.logger.Error(fmt.Sprintf("failed to list vd: %s", err)) - return - } - - for _, vd := range vds.Items { - dsReady, _ := conditions.GetCondition(vdcondition.DatasourceReadyType, vd.Status.Conditions) - if dsReady.Status == metav1.ConditionTrue { - continue - } - - if vd.Spec.DataSource == nil || vd.Spec.DataSource.Type != virtv2.DataSourceTypeObjectRef { - continue - } +func (w VirtualDiskRequestEnqueuer) EnqueueRequests(ctx context.Context, obj client.Object) (requests []reconcile.Request) { + vds := w.EnqueueRequestsFromVDs(ctx, obj) + vdsFromVIs := w.EnqueueRequestsFromVIs(obj) + vdsFromCVIs := w.EnqueueRequestsFromCVIs(obj) - ref := vd.Spec.DataSource.ObjectRef + uniqueRequests := make(map[reconcile.Request]struct{}) - if ref == nil || ref.Kind != w.enqueueFromKind { - continue - } + for _, req := range vds { + uniqueRequests[req] = struct{}{} + } + for _, req := range vdsFromVIs { + uniqueRequests[req] = struct{}{} + } + for _, req := range vdsFromCVIs { + uniqueRequests[req] = struct{}{} + } - if ref.Name == obj.GetName() { - requests = append(requests, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Namespace: vd.Namespace, - Name: vd.Name, - }, - }) - } + var aggregatedResults []reconcile.Request + for req := range uniqueRequests { + aggregatedResults = append(aggregatedResults, req) } - return + return aggregatedResults }