fix(api): rewrite conditions with empty reasons (#498)

yaroslavborbat · web-flow · commit 3f6c2275ed71 · 2024-11-05T17:13:05.000+03:00
rewrite conditions with empty reasons
---------

Signed-off-by: yaroslavborbat &lt;yaroslav.752@gmail.com&gt;
diff --git a/api/core/v1alpha2/virtual_machine_ip_address_lease.go b/api/core/v1alpha2/virtual_machine_ip_address_lease.go
@@ -18,6 +18,11 @@ package v1alpha2
 
 import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+const (
+	VirtualMachineIPAddressLeaseKind     = "VirtualMachineIPAddressLease"
+	VirtualMachineIPAddressLeaseResource = "virtualmachineipaddresleases"
+)
+
 // VirtualMachineIPAddressLease defines fact of issued lease for `VirtualMachineIPAddress`.
 // +genclient
 // +genclient:nonNamespaced
diff --git a/api/core/v1alpha2/virtual_machine_operation.go b/api/core/v1alpha2/virtual_machine_operation.go
@@ -19,8 +19,8 @@ package v1alpha2
 import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 const (
-	VMOPKind     = "VirtualMachineOperation"
-	VMOPResource = "virtualmachineoperations"
+	VirtualMachineOperationKind     = "VirtualMachineOperation"
+	VirtualmachineOperationResource = "virtualmachineoperations"
 )
 
 // VirtualMachineOperation resource provides the ability to declaratively manage state changes of virtual machines.
diff --git a/images/virtualization-artifact/pkg/controller/service/resource.go b/images/virtualization-artifact/pkg/controller/service/resource.go
@@ -30,7 +30,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/deckhouse/virtualization-controller/pkg/common/patch"
+	"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"
 )
 
 type ResourceObject[T, ST any] interface {
@@ -103,11 +105,68 @@ func (r *Resource[T, ST]) Changed() T {
 	return r.changedObj
 }
 
+// rewriteObject is part of the transition from version 1.14, where you can specify empty reasons. After version 1.15, this feature is not needed.
+// TODO: Delete me after release v1.15
+func rewriteObject(obj client.Object) {
+	var conds []metav1.Condition
+
+	switch obj.GetObjectKind().GroupVersionKind().Kind {
+	case virtv2.VirtualMachineKind:
+		vm := obj.(*virtv2.VirtualMachine)
+		conds = vm.Status.Conditions
+	case virtv2.VirtualDiskKind:
+		vd := obj.(*virtv2.VirtualDisk)
+		conds = vd.Status.Conditions
+	case virtv2.VirtualImageKind:
+		vi := obj.(*virtv2.VirtualImage)
+		conds = vi.Status.Conditions
+	case virtv2.ClusterVirtualImageKind:
+		cvi := obj.(*virtv2.ClusterVirtualImage)
+		conds = cvi.Status.Conditions
+	case virtv2.VirtualMachineBlockDeviceAttachmentKind:
+		vmbda := obj.(*virtv2.VirtualMachineBlockDeviceAttachment)
+		conds = vmbda.Status.Conditions
+	case virtv2.VirtualMachineIPAddressKind:
+		ip := obj.(*virtv2.VirtualMachineIPAddress)
+		conds = ip.Status.Conditions
+	case virtv2.VirtualMachineIPAddressLeaseKind:
+		ipl := obj.(*virtv2.VirtualMachineIPAddressLease)
+		conds = ipl.Status.Conditions
+	case virtv2.VirtualMachineOperationKind:
+		vmop := obj.(*virtv2.VirtualMachineOperation)
+		conds = vmop.Status.Conditions
+	case virtv2.VirtualDiskSnapshotKind:
+		snap := obj.(*virtv2.VirtualDiskSnapshot)
+		conds = snap.Status.Conditions
+	case virtv2.VirtualMachineClassKind:
+		class := obj.(*virtv2.VirtualMachineClass)
+		conds = class.Status.Conditions
+	case virtv2.VirtualMachineRestoreKind:
+		restore := obj.(*virtv2.VirtualMachineRestore)
+		conds = restore.Status.Conditions
+	case virtv2.VirtualMachineSnapshotKind:
+		snap := obj.(*virtv2.VirtualMachineSnapshot)
+		conds = snap.Status.Conditions
+	}
+
+	rewriteEmptyReason(conds)
+}
+
+func rewriteEmptyReason(conds []metav1.Condition) {
+	for i := range conds {
+		if conds[i].Reason == "" {
+			conds[i].Reason = conditions.ReasonUnknown.String()
+		}
+	}
+}
+
 func (r *Resource[T, ST]) Update(ctx context.Context) error {
 	if r.IsEmpty() {
 		return nil
 	}
 
+	rewriteObject(r.changedObj)
+
 	if !reflect.DeepEqual(r.getObjStatus(r.currentObj), r.getObjStatus(r.changedObj)) {
 		// Save some metadata fields.
 		finalizers := r.changedObj.GetFinalizers()
diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/datasource_ready.go b/images/virtualization-artifact/pkg/controller/vd/internal/datasource_ready.go
@@ -57,7 +57,7 @@ func (h DatasourceReadyHandler) Handle(ctx context.Context, vd *virtv2.VirtualDi
 
 	if vd.DeletionTimestamp != nil {
 		condition.Status = metav1.ConditionUnknown
-		condition.Reason = ""
+		condition.Reason = conditions.ReasonUnknown.String()
 		condition.Message = ""
 		return reconcile.Result{}, nil
 	}
diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/datasource_ready_test.go b/images/virtualization-artifact/pkg/controller/vd/internal/datasource_ready_test.go
@@ -23,6 +23,7 @@ import (
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vd/internal/source"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition"
@@ -55,7 +56,7 @@ func TestDatasourceReadyHandler_Handle(t *testing.T) {
 		condition := vd.Status.Conditions[0]
 		require.Equal(t, vdcondition.DatasourceReadyType, condition.Type)
 		require.Equal(t, metav1.ConditionUnknown, condition.Status)
-		require.Equal(t, "", condition.Reason)
+		require.Equal(t, conditions.ReasonUnknown.String(), condition.Reason)
 	})
 
 	t.Run("VirtualDisk with Blank DataSource", func(t *testing.T) {
diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/resizing.go b/images/virtualization-artifact/pkg/controller/vd/internal/resizing.go
@@ -60,15 +60,15 @@ func (h ResizingHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (re
 
 	if vd.DeletionTimestamp != nil {
 		condition.Status = metav1.ConditionUnknown
-		condition.Reason = ""
+		condition.Reason = conditions.ReasonUnknown.String()
 		condition.Message = ""
 		return reconcile.Result{}, nil
 	}
 
 	readyCondition, ok := service.GetCondition(vdcondition.ReadyType, vd.Status.Conditions)
 	if !ok || readyCondition.Status != metav1.ConditionTrue {
 		condition.Status = metav1.ConditionUnknown
-		condition.Reason = ""
+		condition.Reason = conditions.ReasonUnknown.String()
 		condition.Message = ""
 		return reconcile.Result{}, nil
 	}
@@ -81,14 +81,14 @@ func (h ResizingHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk) (re
 
 	if pvc == nil {
 		condition.Status = metav1.ConditionUnknown
-		condition.Reason = ""
+		condition.Reason = conditions.ReasonUnknown.String()
 		condition.Message = "Underlying PersistentVolumeClaim not found: resizing is not possible."
 		return reconcile.Result{}, nil
 	}
 
 	if pvc.Status.Phase != corev1.ClaimBound {
 		condition.Status = metav1.ConditionUnknown
-		condition.Reason = ""
+		condition.Reason = conditions.ReasonUnknown.String()
 		condition.Message = "Underlying PersistentVolumeClaim not bound: resizing is not possible."
 		return reconcile.Result{}, nil
 	}
diff --git a/images/virtualization-artifact/pkg/controller/vd/internal/snapshotting.go b/images/virtualization-artifact/pkg/controller/vd/internal/snapshotting.go
@@ -52,15 +52,15 @@ func (h SnapshottingHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk)
 
 	if vd.DeletionTimestamp != nil {
 		condition.Status = metav1.ConditionUnknown
-		condition.Reason = ""
+		condition.Reason = conditions.ReasonUnknown.String()
 		condition.Message = ""
 		return reconcile.Result{}, nil
 	}
 
 	readyCondition, ok := service.GetCondition(vdcondition.ReadyType, vd.Status.Conditions)
 	if !ok || readyCondition.Status != metav1.ConditionTrue {
 		condition.Status = metav1.ConditionUnknown
-		condition.Reason = ""
+		condition.Reason = conditions.ReasonUnknown.String()
 		condition.Message = ""
 		return reconcile.Result{}, nil
 	}
@@ -94,7 +94,7 @@ func (h SnapshottingHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk)
 	}
 
 	condition.Status = metav1.ConditionUnknown
-	condition.Reason = ""
+	condition.Reason = conditions.ReasonUnknown.String()
 	condition.Message = ""
 	return reconcile.Result{}, nil
 }
diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/life_cycle.go
@@ -79,7 +79,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vdSnapshot *virtv2.Virtual
 	if vdSnapshot.DeletionTimestamp != nil {
 		vdSnapshot.Status.Phase = virtv2.VirtualDiskSnapshotPhaseTerminating
 		condition.Status = metav1.ConditionUnknown
-		condition.Reason = ""
+		condition.Reason = conditions.ReasonUnknown.String()
 		condition.Message = ""
 
 		return reconcile.Result{}, nil
diff --git a/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/virtual_disk_ready.go b/images/virtualization-artifact/pkg/controller/vdsnapshot/internal/virtual_disk_ready.go
@@ -54,7 +54,7 @@ func (h VirtualDiskReadyHandler) Handle(ctx context.Context, vdSnapshot *virtv2.
 
 	if vdSnapshot.DeletionTimestamp != nil {
 		condition.Status = metav1.ConditionUnknown
-		condition.Reason = ""
+		condition.Reason = conditions.ReasonUnknown.String()
 		condition.Message = ""
 		return reconcile.Result{}, nil
 	}
diff --git a/tests/performance/shatal/internal/shatal/modifier.go b/tests/performance/shatal/internal/shatal/modifier.go
@@ -76,7 +76,7 @@ func (s *Modifier) Do(ctx context.Context, vm v1alpha2.VirtualMachine) {
 
 	vmop := v1alpha2.VirtualMachineOperation{
 		TypeMeta: metav1.TypeMeta{
-			Kind:       v1alpha2.VMOPKind,
+			Kind:       v1alpha2.VirtualMachineOperationKind,
 			APIVersion: v1alpha2.Version,
 		},
 		ObjectMeta: metav1.ObjectMeta{

Original file line number	Diff line number	Diff line change
`@@ -19,8 +19,8 @@ package v1alpha2`
`19`	`19`	`import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"`
`20`	`20`
`21`	`21`	`const (`
`22`		`- VMOPKind = "VirtualMachineOperation"`
`23`		`- VMOPResource = "virtualmachineoperations"`
	`22`	`+ VirtualMachineOperationKind = "VirtualMachineOperation"`
	`23`	`+ VirtualmachineOperationResource = "virtualmachineoperations"`
`24`	`24`	`)`
`25`	`25`
`26`	`26`	`// VirtualMachineOperation resource provides the ability to declaratively manage state changes of virtual machines.`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ func (h DatasourceReadyHandler) Handle(ctx context.Context, vd *virtv2.VirtualDi`
`57`	`57`
`58`	`58`	`if vd.DeletionTimestamp != nil {`
`59`	`59`	`condition.Status = metav1.ConditionUnknown`
`60`		`- condition.Reason = ""`
	`60`	`+ condition.Reason = conditions.ReasonUnknown.String()`
`61`	`61`	`condition.Message = ""`
`62`	`62`	`return reconcile.Result{}, nil`
`63`	`63`	`}`
Original file line number	Diff line number	Diff line change
`@@ -52,15 +52,15 @@ func (h SnapshottingHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk)`
`52`	`52`
`53`	`53`	`if vd.DeletionTimestamp != nil {`
`54`	`54`	`condition.Status = metav1.ConditionUnknown`
`55`		`- condition.Reason = ""`
	`55`	`+ condition.Reason = conditions.ReasonUnknown.String()`
`56`	`56`	`condition.Message = ""`
`57`	`57`	`return reconcile.Result{}, nil`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`readyCondition, ok := service.GetCondition(vdcondition.ReadyType, vd.Status.Conditions)`
`61`	`61`	`if !ok \|\| readyCondition.Status != metav1.ConditionTrue {`
`62`	`62`	`condition.Status = metav1.ConditionUnknown`
`63`		`- condition.Reason = ""`
	`63`	`+ condition.Reason = conditions.ReasonUnknown.String()`
`64`	`64`	`condition.Message = ""`
`65`	`65`	`return reconcile.Result{}, nil`
`66`	`66`	`}`
`@@ -94,7 +94,7 @@ func (h SnapshottingHandler) Handle(ctx context.Context, vd *virtv2.VirtualDisk)`
`94`	`94`	`}`
`95`	`95`
`96`	`96`	`condition.Status = metav1.ConditionUnknown`
`97`		`- condition.Reason = ""`
	`97`	`+ condition.Reason = conditions.ReasonUnknown.String()`
`98`	`98`	`condition.Message = ""`
`99`	`99`	`return reconcile.Result{}, nil`
`100`	`100`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ func (h VirtualDiskReadyHandler) Handle(ctx context.Context, vdSnapshot *virtv2.`
`54`	`54`
`55`	`55`	`if vdSnapshot.DeletionTimestamp != nil {`
`56`	`56`	`condition.Status = metav1.ConditionUnknown`
`57`		`- condition.Reason = ""`
	`57`	`+ condition.Reason = conditions.ReasonUnknown.String()`
`58`	`58`	`condition.Message = ""`
`59`	`59`	`return reconcile.Result{}, nil`
`60`	`60`	`}`