Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cvi,vi): unlock pending vi/cvi from vd ref #416

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/core/v1alpha2/cvicondition/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 13 additions & 0 deletions api/core/v1alpha2/vdcondition/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
LopatinDmitr marked this conversation as resolved.
Show resolved Hide resolved
InUseType Type = "InUse"
)

type (
Expand All @@ -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 {
Expand All @@ -69,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"
Expand Down Expand Up @@ -118,4 +126,9 @@ const (
StorageClassReady StorageClassReadyReason = "StorageClassReady"
// StorageClassNotFound indicates that the storage class is not ready
StorageClassNotFound StorageClassReadyReason = "StorageClassNotFound"

// 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"
)
2 changes: 2 additions & 0 deletions api/core/v1alpha2/vicondition/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ 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"
virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
"github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition"
)

type Handler interface {
Expand Down Expand Up @@ -165,7 +167,14 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr
return false
}

return oldVD.Status.Phase != newVD.Status.Phase
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
}

return false
},
},
); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.VirtualDiskNotAllowedForUseError{}):
cb.
Status(metav1.ConditionFalse).
Reason(cvicondition.VirtualDiskNotReady).
Reason(cvicondition.VirtualDiskInUseInRunningVirtualMachine).
Message(service.CapitalizeFirstLetter(err.Error()))
return reconcile.Result{}, nil
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,16 @@ func NewVirtualDiskNotReadyError(name string) error {
}
}

type VirtualDiskAttachedToRunningVMError struct {
name string
vmName string
type VirtualDiskNotAllowedForUseError struct {
name string
}

func (e VirtualDiskAttachedToRunningVMError) Error() string {
return fmt.Sprintf("VirtualDisk %q attached to running VirtualMachine %q", e.name, e.vmName)
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 NewVirtualDiskAttachedToRunningVMError(name, vmName string) error {
return VirtualDiskAttachedToRunningVMError{
name: name,
vmName: vmName,
func NewVirtualDiskNotAllowedForUseError(name string) error {
return VirtualDiskNotAllowedForUseError{
name: name,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ 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"
)

type ObjectRefVirtualDisk struct {
Expand Down Expand Up @@ -226,18 +227,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 && inUseCondition.Reason == vdcondition.AllowedForImageUsage.String() {
return nil
}

return nil
return NewVirtualDiskNotAllowedForUseError(vd.Name)
}
33 changes: 32 additions & 1 deletion images/virtualization-artifact/pkg/controller/service/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ limitations under the License.

package service

import "errors"
import (
"errors"
"fmt"
)

var (
ErrStorageClassNotFound = errors.New("storage class not found")
Expand All @@ -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,
}
}
Loading
Loading