diff --git a/api/core/v1alpha2/cvicondition/condition.go b/api/core/v1alpha2/cvicondition/condition.go index 529db762d..079a20a11 100644 --- a/api/core/v1alpha2/cvicondition/condition.go +++ b/api/core/v1alpha2/cvicondition/condition.go @@ -44,6 +44,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 622e2e1d7..3ebb29160 100644 --- a/api/core/v1alpha2/vdcondition/condition.go +++ b/api/core/v1alpha2/vdcondition/condition.go @@ -28,6 +28,8 @@ const ( ResizedType Type = "Resized" // SnapshottingType indicates whether the disk snapshotting operation is in progress. SnapshottingType Type = "Snapshotting" + // 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 ( @@ -39,6 +41,8 @@ type ( ResizedReason = string // SnapshottingReason represents the various reasons for the Snapshotting condition type. SnapshottingReason = string + // InUseReason represents the various reasons for the InUse condition type. + InUseReason = string ) const ( @@ -83,4 +87,9 @@ const ( Snapshotting SnapshottingReason = "Snapshotting" // SnapshottingNotAvailable indicates that the snapshotting operation is not available for now. SnapshottingNotAvailable SnapshottingReason = "NotAvailable" + + // 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 0b3031a68..274c8f9e1 100644 --- a/api/core/v1alpha2/vicondition/condition.go +++ b/api/core/v1alpha2/vicondition/condition.go @@ -44,6 +44,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_controller.go b/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go index c28a605ab..336482a7e 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go +++ b/images/virtualization-artifact/pkg/controller/cvi/cvi_controller.go @@ -62,11 +62,12 @@ func NewController( protection := service.NewProtectionService(mgr.GetClient(), virtv2.FinalizerCVIProtection) importer := service.NewImporterService(dvcr, mgr.GetClient(), importerImage, requirements, PodPullPolicy, PodVerbose, ControllerName, protection) uploader := service.NewUploaderService(dvcr, mgr.GetClient(), uploaderImage, requirements, PodPullPolicy, PodVerbose, ControllerName, protection) + lockService := service.NewLockService(mgr.GetClient()) sources := source.NewSources() sources.Set(virtv2.DataSourceTypeHTTP, source.NewHTTPDataSource(stat, importer, dvcr, ns)) sources.Set(virtv2.DataSourceTypeContainerImage, source.NewRegistryDataSource(stat, importer, dvcr, mgr.GetClient(), ns)) - sources.Set(virtv2.DataSourceTypeObjectRef, source.NewObjectRefDataSource(stat, importer, dvcr, mgr.GetClient(), ns)) + sources.Set(virtv2.DataSourceTypeObjectRef, source.NewObjectRefDataSource(stat, importer, dvcr, mgr.GetClient(), ns, lockService)) sources.Set(virtv2.DataSourceTypeUpload, source.NewUploadDataSource(stat, uploader, dvcr, ns)) reconciler := NewReconciler( diff --git a/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go b/images/virtualization-artifact/pkg/controller/cvi/cvi_reconciler.go index 3e3f4ffae..22af02074 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 444758a02..d3fb50b77 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/datasource_ready.go @@ -89,9 +89,9 @@ func (h DatasourceReadyHandler) Handle(ctx context.Context, cvi *virtv2.ClusterV condition.Reason = cvicondition.VirtualDiskNotReady condition.Message = service.CapitalizeFirstLetter(err.Error()) return reconcile.Result{}, nil - case errors.As(err, &source.VirtualDiskAttachedToRunningVMError{}): + case errors.As(err, &source.VirtualDiskInUseError{}): condition.Status = metav1.ConditionFalse - condition.Reason = cvicondition.VirtualDiskNotReady + condition.Reason = cvicondition.VirtualDiskInUseInRunningVirtualMachine condition.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 f8daee718..55de6884e 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.go b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref.go index 36ba7f0ae..dba33a94b 100644 --- a/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref.go +++ b/images/virtualization-artifact/pkg/controller/cvi/internal/source/object_ref.go @@ -56,6 +56,7 @@ func NewObjectRefDataSource( dvcrSettings *dvcr.Settings, client client.Client, controllerNamespace string, + lockService *service.LockService, ) *ObjectRefDataSource { return &ObjectRefDataSource{ statService: statService, @@ -65,7 +66,7 @@ func NewObjectRefDataSource( controllerNamespace: controllerNamespace, viOnPvcSyncer: NewObjectRefVirtualImageOnPvc(importerService, dvcrSettings, statService), - vdSyncer: NewObjectRefVirtualDisk(importerService, client, controllerNamespace, dvcrSettings, statService), + vdSyncer: NewObjectRefVirtualDisk(importerService, client, controllerNamespace, dvcrSettings, statService, lockService), } } 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 606efd0f7..ebd64ba41 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 @@ -36,6 +36,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/sdk/framework/helper" 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" ) @@ -45,15 +46,17 @@ type ObjectRefVirtualDisk struct { statService Stat dvcrSettings *dvcr.Settings controllerNamespace string + lockService *service.LockService } -func NewObjectRefVirtualDisk(importerService Importer, client client.Client, controllerNamespace string, dvcrSettings *dvcr.Settings, statService Stat) *ObjectRefVirtualDisk { +func NewObjectRefVirtualDisk(importerService Importer, client client.Client, controllerNamespace string, dvcrSettings *dvcr.Settings, statService Stat, lockService *service.LockService) *ObjectRefVirtualDisk { return &ObjectRefVirtualDisk{ importerService: importerService, statService: statService, dvcrSettings: dvcrSettings, client: client, controllerNamespace: controllerNamespace, + lockService: lockService, } } @@ -81,12 +84,28 @@ func (ds ObjectRefVirtualDisk) Sync(ctx context.Context, cvi *virtv2.ClusterVirt return reconcile.Result{}, err } + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + return CleanUp(ctx, cvi, ds) case cc.IsTerminating(pod): cvi.Status.Phase = virtv2.ImagePending log.Info("Cleaning up...") + + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + case pod == nil: + err = ds.lockService.LockVirtualDiskByVirtualImage(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + cvi.Status.Progress = ds.statService.GetProgress(cvi.GetUID(), pod, cvi.Status.Progress) cvi.Status.Target.RegistryURL = ds.statService.GetDVCRImageName(pod) @@ -99,8 +118,16 @@ func (ds ObjectRefVirtualDisk) Sync(ctx context.Context, cvi *virtv2.ClusterVirt case err == nil: // OK. case cc.ErrQuotaExceeded(err): + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } return setQuotaExceededPhaseCondition(condition, &cvi.Status.Phase, err, cvi.CreationTimestamp), nil default: + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } setPhaseConditionToFailed(condition, &cvi.Status.Phase, fmt.Errorf("unexpected error: %w", err)) return reconcile.Result{}, err } @@ -114,6 +141,11 @@ func (ds ObjectRefVirtualDisk) Sync(ctx context.Context, cvi *virtv2.ClusterVirt return reconcile.Result{Requeue: true}, nil case cc.IsPodComplete(pod): + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + err = ds.statService.CheckPod(pod) if err != nil { cvi.Status.Phase = virtv2.ImageFailed @@ -144,6 +176,10 @@ func (ds ObjectRefVirtualDisk) Sync(ctx context.Context, cvi *virtv2.ClusterVirt default: err = ds.statService.CheckPod(pod) if err != nil { + unLockErr := ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if unLockErr != nil { + return reconcile.Result{}, unLockErr + } cvi.Status.Phase = virtv2.ImageFailed switch { @@ -217,16 +253,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 := helper.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, _ := service.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/lock_service.go b/images/virtualization-artifact/pkg/controller/service/lock_service.go new file mode 100644 index 000000000..d197f2785 --- /dev/null +++ b/images/virtualization-artifact/pkg/controller/service/lock_service.go @@ -0,0 +1,89 @@ +package service + +import ( + "context" + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" +) + +type LockService struct { + client client.Client +} + +func NewLockService( + client client.Client, +) *LockService { + return &LockService{ + client: client, + } +} + +func (s LockService) LockVirtualDiskByVirtualImage(ctx context.Context, vd *virtv2.VirtualDisk) error { + inUseCondition, ok := GetCondition(vdcondition.InUseType, vd.Status.Conditions) + + if !ok || inUseCondition.Status == metav1.ConditionUnknown { + inUseCondition = metav1.Condition{ + Type: vdcondition.InUseType, + Reason: vdcondition.InUseByVirtualImage, + Status: metav1.ConditionTrue, + } + + SetCondition(inUseCondition, &vd.Status.Conditions) + err := s.client.Status().Update(ctx, vd) + if err != nil { + return err + } + } else { + if inUseCondition.Reason != vdcondition.InUseByVirtualImage { + return fmt.Errorf("virtual disk %q already used by running virtual machine", vd.GetName()) + } + } + + return nil +} + +func (s LockService) UnlockVirtualDisk(ctx context.Context, vd *virtv2.VirtualDisk) error { + inUseCondition, ok := GetCondition(vdcondition.InUseType, vd.Status.Conditions) + if ok { + inUseCondition = metav1.Condition{ + Type: vdcondition.InUseType, + Status: metav1.ConditionUnknown, + } + + SetCondition(inUseCondition, &vd.Status.Conditions) + err := s.client.Status().Update(ctx, vd) + if err != nil { + return err + } + } + + return nil +} + +func (s LockService) LockVirtualDiskByVirtualMachine(ctx context.Context, vd *virtv2.VirtualDisk) error { + inUseCondition, ok := GetCondition(vdcondition.InUseType, vd.Status.Conditions) + if !ok || inUseCondition.Status == metav1.ConditionUnknown { + inUseCondition = metav1.Condition{ + Type: vdcondition.InUseType, + Reason: vdcondition.InUseByVirtualMachine, + Status: metav1.ConditionTrue, + } + + SetCondition(inUseCondition, &vd.Status.Conditions) + err := s.client.Status().Update(ctx, vd) + if err != nil { + return err + } + } else { + if inUseCondition.Reason != vdcondition.InUseByVirtualMachine { + return fmt.Errorf("virtual disk %q already used by creating virtual disk", vd.GetName()) + } + } + + return nil +} 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 1b645248c..993fa98b5 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 d7dab22bb..64d6cd6c5 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 158eafda4..f9dbe1e6e 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 0b0e7cace..7bd4482e7 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 ee5248426..94cacb039 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 9d11ed922..876f40e5e 100644 --- a/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go +++ b/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go @@ -27,7 +27,6 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vi/internal/source" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" - "github.com/deckhouse/virtualization/api/core/v1alpha2/cvicondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition" ) @@ -90,9 +89,9 @@ func (h DatasourceReadyHandler) Handle(ctx context.Context, vi *virtv2.VirtualIm condition.Reason = vicondition.VirtualDiskNotReady condition.Message = service.CapitalizeFirstLetter(err.Error() + ".") return reconcile.Result{}, nil - case errors.As(err, &source.VirtualDiskAttachedToRunningVMError{}): + case errors.As(err, &source.VirtualDiskInUseError{}): condition.Status = metav1.ConditionFalse - condition.Reason = cvicondition.VirtualDiskNotReady + condition.Reason = vicondition.VirtualDiskInUseInRunningVirtualMachine condition.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 f8daee718..55de6884e 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 a840af752..e7b96251c 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 @@ -65,6 +65,7 @@ func NewObjectRefDataSource( client client.Client, diskService *service.DiskService, storageClassForPVC string, + lockService *service.LockService, ) *ObjectRefDataSource { return &ObjectRefDataSource{ statService: statService, @@ -74,7 +75,7 @@ func NewObjectRefDataSource( diskService: diskService, storageClassForPVC: storageClassForPVC, viObjectRefOnPvc: NewObjectRefDataVirtualImageOnPVC(statService, importerService, dvcrSettings, client, diskService, storageClassForPVC), - vdSyncer: NewObjectRefVirtualDisk(importerService, client, diskService, dvcrSettings, statService, storageClassForPVC), + vdSyncer: NewObjectRefVirtualDisk(importerService, client, diskService, dvcrSettings, statService, storageClassForPVC, lockService), } } 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 7dc5ea460..c16bef3fa 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 @@ -40,6 +40,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/sdk/framework/helper" "github.com/deckhouse/virtualization-controller/pkg/util" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" + "github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition" "github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition" ) @@ -50,9 +51,10 @@ type ObjectRefVirtualDisk struct { dvcrSettings *dvcr.Settings client client.Client storageClassForPVC string + lockService *service.LockService } -func NewObjectRefVirtualDisk(importerService Importer, client client.Client, diskService *service.DiskService, dvcrSettings *dvcr.Settings, statService Stat, storageClassForPVC string) *ObjectRefVirtualDisk { +func NewObjectRefVirtualDisk(importerService Importer, client client.Client, diskService *service.DiskService, dvcrSettings *dvcr.Settings, statService Stat, storageClassForPVC string, lockService *service.LockService) *ObjectRefVirtualDisk { return &ObjectRefVirtualDisk{ importerService: importerService, client: client, @@ -60,6 +62,7 @@ func NewObjectRefVirtualDisk(importerService Importer, client client.Client, dis statService: statService, dvcrSettings: dvcrSettings, storageClassForPVC: storageClassForPVC, + lockService: lockService, } } @@ -87,12 +90,28 @@ func (ds ObjectRefVirtualDisk) StoreToDVCR(ctx context.Context, vi *virtv2.Virtu return reconcile.Result{}, err } + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + return CleanUpSupplements(ctx, vi, ds) case common.IsTerminating(pod): vi.Status.Phase = virtv2.ImagePending log.Info("Cleaning up...") + + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + case pod == nil: + err = ds.lockService.LockVirtualDiskByVirtualImage(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + vi.Status.Progress = ds.statService.GetProgress(vi.GetUID(), pod, vi.Status.Progress) vi.Status.Target.RegistryURL = ds.statService.GetDVCRImageName(pod) @@ -105,8 +124,16 @@ func (ds ObjectRefVirtualDisk) StoreToDVCR(ctx context.Context, vi *virtv2.Virtu case err == nil: // OK. case common.ErrQuotaExceeded(err): + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } return setQuotaExceededPhaseCondition(condition, &vi.Status.Phase, err, vi.CreationTimestamp), nil default: + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } setPhaseConditionToFailed(condition, &vi.Status.Phase, fmt.Errorf("unexpected error: %w", err)) return reconcile.Result{}, err } @@ -120,6 +147,11 @@ func (ds ObjectRefVirtualDisk) StoreToDVCR(ctx context.Context, vi *virtv2.Virtu return reconcile.Result{Requeue: true}, nil case common.IsPodComplete(pod): + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + err = ds.statService.CheckPod(pod) if err != nil { vi.Status.Phase = virtv2.ImageFailed @@ -150,6 +182,11 @@ func (ds ObjectRefVirtualDisk) StoreToDVCR(ctx context.Context, vi *virtv2.Virtu default: err = ds.statService.CheckPod(pod) if err != nil { + unLockErr := ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if unLockErr != nil { + return reconcile.Result{}, unLockErr + } + vi.Status.Phase = virtv2.ImageFailed switch { @@ -218,12 +255,26 @@ func (ds ObjectRefVirtualDisk) StoreToPVC(ctx context.Context, vi *virtv2.Virtua return reconcile.Result{}, err } + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + return CleanUpSupplements(ctx, vi, ds) case common.AnyTerminating(dv, pvc): log.Info("Waiting for supplements to be terminated") + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } case dv == nil: log.Info("Start import to PVC") + err = ds.lockService.LockVirtualDiskByVirtualImage(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + vi.Status.Progress = "0%" vi.Status.SourceUID = util.GetPointer(vdRef.GetUID()) @@ -241,6 +292,11 @@ func (ds ObjectRefVirtualDisk) StoreToPVC(ctx context.Context, vi *virtv2.Virtua err = ds.diskService.StartImmediate(ctx, size, ptr.To(ds.storageClassForPVC), source, vi, supgen) if updated, err := setPhaseConditionFromStorageError(err, vi, condition); err != nil || updated { + unLockErr := ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if unLockErr != nil { + return reconcile.Result{}, unLockErr + } + return reconcile.Result{}, err } @@ -259,6 +315,11 @@ func (ds ObjectRefVirtualDisk) StoreToPVC(ctx context.Context, vi *virtv2.Virtua case ds.diskService.IsImportDone(dv, pvc): log.Info("Import has completed", "dvProgress", dv.Status.Progress, "dvPhase", dv.Status.Phase, "pvcPhase", pvc.Status.Phase) + err = ds.lockService.UnlockVirtualDisk(ctx, vdRef) + if err != nil { + return reconcile.Result{}, err + } + vi.Status.Phase = virtv2.ImageReady condition.Status = metav1.ConditionTrue condition.Reason = vicondition.Ready @@ -341,7 +402,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 := helper.FetchObject(ctx, types.NamespacedName{Name: vi.Spec.DataSource.ObjectRef.Name, Namespace: vi.Namespace}, ds.client, &virtv2.VirtualDisk{}) @@ -353,17 +414,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 := helper.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, _ := service.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_controller.go b/images/virtualization-artifact/pkg/controller/vi/vi_controller.go index 7f37697b4..5dbc34667 100644 --- a/images/virtualization-artifact/pkg/controller/vi/vi_controller.go +++ b/images/virtualization-artifact/pkg/controller/vi/vi_controller.go @@ -63,11 +63,12 @@ func NewController( importer := service.NewImporterService(dvcr, mgr.GetClient(), importerImage, requirements, PodPullPolicy, PodVerbose, ControllerName, protection) uploader := service.NewUploaderService(dvcr, mgr.GetClient(), uploaderImage, requirements, PodPullPolicy, PodVerbose, ControllerName, protection) disk := service.NewDiskService(mgr.GetClient(), dvcr, protection) + lockService := service.NewLockService(mgr.GetClient()) sources := source.NewSources() sources.Set(virtv2.DataSourceTypeHTTP, source.NewHTTPDataSource(stat, importer, dvcr, disk, storageClassForVirtualImageOnPVC)) sources.Set(virtv2.DataSourceTypeContainerImage, source.NewRegistryDataSource(stat, importer, dvcr, mgr.GetClient(), disk, storageClassForVirtualImageOnPVC)) - sources.Set(virtv2.DataSourceTypeObjectRef, source.NewObjectRefDataSource(stat, importer, dvcr, mgr.GetClient(), disk, storageClassForVirtualImageOnPVC)) + sources.Set(virtv2.DataSourceTypeObjectRef, source.NewObjectRefDataSource(stat, importer, dvcr, mgr.GetClient(), disk, storageClassForVirtualImageOnPVC, lockService)) sources.Set(virtv2.DataSourceTypeUpload, source.NewUploadDataSource(stat, uploader, dvcr, disk, storageClassForVirtualImageOnPVC)) reconciler := NewReconciler( diff --git a/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go b/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go index b34fa5f88..0a5e2f6ba 100644 --- a/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vi/vi_reconciler.go @@ -39,6 +39,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 { @@ -219,7 +220,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 4b1dab110..966dca605 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" ) @@ -299,9 +300,16 @@ func (h *BlockDeviceHandler) countReadyBlockDevices(vm *virtv2.VirtualMachine, s canStartKVVM = false continue } - - if vd.Status.Phase == virtv2.DiskReady { - ready++ + readyCondition, _ := service.GetCondition(vdcondition.ReadyType, vd.Status.Conditions) + if readyCondition.Status == metav1.ConditionTrue { + inUseByVICondition, _ := service.GetCondition(vdcondition.InUseType, vd.Status.Conditions) + if inUseByVICondition.Status != metav1.ConditionTrue { + ready++ + } else { + msg := fmt.Sprintf("The virtual disk %s is being used to create a virtual image.", vd.Name) + warnings = append(warnings, msg) + canStartKVVM = false + } } 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 6c88718ee..9d451261c 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) { @@ -72,6 +73,17 @@ var _ = Describe("BlockDeviceHandler", func() { Status: virtv2.VirtualDiskStatus{ Phase: virtv2.DiskReady, Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-foo"}, + Conditions: []metav1.Condition{ + { + Type: vdcondition.InUseType, + Status: metav1.ConditionUnknown, + }, + { + Type: vdcondition.ReadyType, + Reason: vdcondition.Ready, + Status: metav1.ConditionTrue, + }, + }, }, } vdBar = &virtv2.VirtualDisk{ @@ -79,6 +91,17 @@ var _ = Describe("BlockDeviceHandler", func() { Status: virtv2.VirtualDiskStatus{ Phase: virtv2.DiskReady, Target: virtv2.DiskTarget{PersistentVolumeClaim: "pvc-bar"}, + Conditions: []metav1.Condition{ + { + Type: vdcondition.InUseType, + Status: metav1.ConditionUnknown, + }, + { + Type: vdcondition.ReadyType, + Reason: vdcondition.Ready, + Status: metav1.ConditionTrue, + }, + }, }, } vm = &virtv2.VirtualMachine{ @@ -170,6 +193,17 @@ var _ = Describe("BlockDeviceHandler", func() { It("VirtualDisk's target pvc is created", func() { vdFoo.Status.Phase = virtv2.DiskProvisioning + vdFoo.Status.Conditions = []metav1.Condition{ + { + Type: vdcondition.InUseType, + Status: metav1.ConditionUnknown, + }, + { + 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 5654dce2f..14c8ea8ac 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 2f6fa9d54..3aece3437 100644 --- a/images/virtualization-artifact/pkg/controller/watchers/object_ref_watcher.go +++ b/images/virtualization-artifact/pkg/controller/watchers/object_ref_watcher.go @@ -61,8 +61,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 95bf14482..5b0457e46 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 {