fix

Signed-off-by: dmitry.lopatin <[email protected]>
deckhouse · Oct 29, 2024 · 7b7bcd5 · 7b7bcd5
1 parent e599b02
commit 7b7bcd5
Show file tree

Hide file tree

Showing 24 changed files with 289 additions and 64 deletions.
diff --git 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
@@ -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
@@ -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_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
@@ -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
@@ -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
@@ -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"
 )
 
@@ -217,16 +218,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
@@ -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/...ternal/validators/iso_source_validator.go → ...nternal/validator/iso_source_validator.go b/...ternal/validators/iso_source_validator.go → ...nternal/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/...internal/validators/pvc_size_validator.go → .../internal/validator/pvc_size_validator.go b/...internal/validators/pvc_size_validator.go → .../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/...rnal/validators/spec_changes_validator.go → ...ernal/validator/spec_changes_validator.go b/...rnal/validators/spec_changes_validator.go → ...ernal/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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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),
 	}
 }