refactor(vi): use condition builder as condition api in VirtualImage …

…controller (#513) Use condition builder as condition api in VirtualImage controller. --------- Signed-off-by: Valeriy Khorunzhin <[email protected]>
deckhouse · Nov 15, 2024 · 4db8365 · 4db8365
1 parent ac7c8e2
commit 4db8365
Show file tree

Hide file tree

Showing 15 changed files with 517 additions and 409 deletions.
diff --git a/api/core/v1alpha2/vicondition/condition.go b/api/core/v1alpha2/vicondition/condition.go
@@ -17,7 +17,11 @@ limitations under the License.
 package vicondition
 
 // Type represents the various condition types for the `VirtualImage`.
-type Type = string
+type Type string
+
+func (s Type) String() string {
+	return string(s)
+}
 
 const (
 	// DatasourceReadyType indicates whether the datasource (for example, a `VirtualImage`) is ready, allowing the import process for the `VirtualImage` to start.
@@ -30,13 +34,25 @@ const (
 
 type (
 	// DatasourceReadyReason represents the various reasons for the DatasourceReady condition type.
-	DatasourceReadyReason = string
+	DatasourceReadyReason string
 	// ReadyReason represents the various reasons for the Ready condition type.
-	ReadyReason = string
+	ReadyReason string
 	// StorageClassReadyReason represents the various reasons for the StorageClassReady condition type.
-	StorageClassReadyReason = string
+	StorageClassReadyReason string
 )
 
+func (s DatasourceReadyReason) String() string {
+	return string(s)
+}
+
+func (s ReadyReason) String() string {
+	return string(s)
+}
+
+func (s StorageClassReadyReason) String() string {
+	return string(s)
+}
+
 const (
 	// DatasourceReady indicates that the datasource is ready for use, allowing the import process to start.
 	DatasourceReady DatasourceReadyReason = "DatasourceReady"

diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go b/images/virtualization-artifact/pkg/controller/vi/internal/datasource_ready.go
@@ -42,16 +42,9 @@ func NewDatasourceReadyHandler(sources *source.Sources) *DatasourceReadyHandler
 }
 
 func (h DatasourceReadyHandler) Handle(ctx context.Context, vi *virtv2.VirtualImage) (reconcile.Result, error) {
-	condition, ok := service.GetCondition(vicondition.DatasourceReadyType, vi.Status.Conditions)
-	if !ok {
-		condition = metav1.Condition{
-			Type:   vicondition.DatasourceReadyType,
-			Status: metav1.ConditionUnknown,
-			Reason: conditions.ReasonUnknown.String(),
-		}
-	}
+	cb := conditions.NewConditionBuilder(vicondition.DatasourceReadyType).Generation(vi.Generation)
 
-	defer func() { service.SetCondition(condition, &vi.Status.Conditions) }()
+	defer func() { conditions.SetCondition(cb, &vi.Status.Conditions) }()
 
 	if vi.DeletionTimestamp != nil {
 		return reconcile.Result{}, nil
@@ -60,41 +53,47 @@ func (h DatasourceReadyHandler) Handle(ctx context.Context, vi *virtv2.VirtualIm
 	s, ok := h.sources.For(vi.Spec.DataSource.Type)
 	if !ok {
 		err := fmt.Errorf("data source validator not found for type: %s", vi.Spec.DataSource.Type)
-		condition.Message = err.Error()
+		cb.Message(err.Error())
 		return reconcile.Result{}, err
 	}
 
 	err := s.Validate(ctx, vi)
 	switch {
 	case err == nil:
-		condition.Status = metav1.ConditionTrue
-		condition.Reason = vicondition.DatasourceReady
-		condition.Message = ""
+		cb.
+			Status(metav1.ConditionTrue).
+			Reason(vicondition.DatasourceReady).
+			Message("")
 		return reconcile.Result{}, nil
 	case errors.Is(err, source.ErrSecretNotFound):
-		condition.Status = metav1.ConditionFalse
-		condition.Reason = vicondition.ContainerRegistrySecretNotFound
-		condition.Message = service.CapitalizeFirstLetter(err.Error() + ".")
+		cb.
+			Status(metav1.ConditionFalse).
+			Reason(vicondition.ContainerRegistrySecretNotFound).
+			Message(service.CapitalizeFirstLetter(err.Error() + "."))
 		return reconcile.Result{}, nil
 	case errors.As(err, &source.ImageNotReadyError{}):
-		condition.Status = metav1.ConditionFalse
-		condition.Reason = vicondition.ImageNotReady
-		condition.Message = service.CapitalizeFirstLetter(err.Error() + ".")
+		cb.
+			Status(metav1.ConditionFalse).
+			Reason(vicondition.ImageNotReady).
+			Message(service.CapitalizeFirstLetter(err.Error() + "."))
 		return reconcile.Result{}, nil
 	case errors.As(err, &source.ClusterImageNotReadyError{}):
-		condition.Status = metav1.ConditionFalse
-		condition.Reason = vicondition.ClusterImageNotReady
-		condition.Message = service.CapitalizeFirstLetter(err.Error() + ".")
+		cb.
+			Status(metav1.ConditionFalse).
+			Reason(vicondition.ClusterImageNotReady).
+			Message(service.CapitalizeFirstLetter(err.Error() + "."))
 		return reconcile.Result{}, nil
 	case errors.As(err, &source.VirtualDiskNotReadyError{}):
-		condition.Status = metav1.ConditionFalse
-		condition.Reason = vicondition.VirtualDiskNotReady
-		condition.Message = service.CapitalizeFirstLetter(err.Error() + ".")
+		cb.
+			Status(metav1.ConditionFalse).
+			Reason(vicondition.VirtualDiskNotReady).
+			Message(service.CapitalizeFirstLetter(err.Error() + "."))
 		return reconcile.Result{}, nil
 	case errors.As(err, &source.VirtualDiskAttachedToRunningVMError{}):
-		condition.Status = metav1.ConditionFalse
-		condition.Reason = vicondition.VirtualDiskNotReady
-		condition.Message = service.CapitalizeFirstLetter(err.Error() + ".")
+		cb.
+			Status(metav1.ConditionFalse).
+			Reason(vicondition.VirtualDiskNotReady).
+			Message(service.CapitalizeFirstLetter(err.Error() + "."))
 		return reconcile.Result{}, nil
 	default:
 		return reconcile.Result{}, err

diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle.go b/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle.go
@@ -25,7 +25,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
-	"github.com/deckhouse/virtualization-controller/pkg/controller/service"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2/vicondition"
 )
@@ -43,15 +42,15 @@ func NewLifeCycleHandler(sources Sources, client client.Client) *LifeCycleHandle
 }
 
 func (h LifeCycleHandler) Handle(ctx context.Context, vi *virtv2.VirtualImage) (reconcile.Result, error) {
-	readyCondition, ok := service.GetCondition(vicondition.ReadyType, vi.Status.Conditions)
+	readyCondition, ok := conditions.GetCondition(vicondition.ReadyType, vi.Status.Conditions)
+
 	if !ok {
-		readyCondition = metav1.Condition{
-			Type:   vicondition.ReadyType,
-			Status: metav1.ConditionUnknown,
-			Reason: conditions.ReasonUnknown.String(),
-		}
+		cb := conditions.NewConditionBuilder(vicondition.ReadyType).
+			Generation(vi.Generation).
+			Status(metav1.ConditionUnknown).
+			Reason(conditions.ReasonUnknown)
 
-		service.SetCondition(readyCondition, &vi.Status.Conditions)
+		conditions.SetCondition(cb, &vi.Status.Conditions)
 	}
 
 	if vi.DeletionTimestamp != nil {
@@ -63,7 +62,7 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vi *virtv2.VirtualImage) (
 		vi.Status.Phase = virtv2.ImagePending
 	}
 
-	dataSourceReadyCondition, exists := service.GetCondition(vicondition.DatasourceReadyType, vi.Status.Conditions)
+	dataSourceReadyCondition, exists := conditions.GetCondition(vicondition.DatasourceReadyType, vi.Status.Conditions)
 	if !exists {
 		return reconcile.Result{}, fmt.Errorf("condition %s not found, but required", vicondition.DatasourceReadyType)
 	}
@@ -91,16 +90,18 @@ func (h LifeCycleHandler) Handle(ctx context.Context, vi *virtv2.VirtualImage) (
 		return reconcile.Result{Requeue: true}, nil
 	}
 
-	storageClassReadyCondition, ok := service.GetCondition(vicondition.StorageClassReadyType, vi.Status.Conditions)
+	storageClassReadyCondition, ok := conditions.GetCondition(vicondition.StorageClassReadyType, vi.Status.Conditions)
 	if !ok {
 		return reconcile.Result{Requeue: true}, fmt.Errorf("condition %s not found", vicondition.StorageClassReadyType)
 	}
 
 	if readyCondition.Status != metav1.ConditionTrue && vi.Spec.Storage == virtv2.StorageKubernetes && storageClassReadyCondition.Status != metav1.ConditionTrue {
-		readyCondition.Status = metav1.ConditionFalse
-		readyCondition.Reason = vicondition.StorageClassNotReady
-		readyCondition.Message = "Storage class is not ready, please read the StorageClassReady condition state."
-		service.SetCondition(readyCondition, &vi.Status.Conditions)
+		readyCB := conditions.NewConditionBuilder(vicondition.ReadyType).
+			Generation(vi.Generation).
+			Status(metav1.ConditionFalse).
+			Reason(vicondition.StorageClassNotReady).
+			Message("Storage class is not ready, please read the StorageClassReady condition state.")
+		conditions.SetCondition(readyCB, &vi.Status.Conditions)
 	}
 
 	if vi.Spec.Storage == virtv2.StorageKubernetes &&

diff --git a/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle_test.go b/images/virtualization-artifact/pkg/controller/vi/internal/life_cycle_test.go
@@ -33,19 +33,19 @@ var _ = Describe("LifeCycleHandler Run", func() {
 	DescribeTable(
 		"Check on LifeCycle.Cleanup after spec changes",
 		func(args cleanupAfterSpecChangeTestArgs) {
-			args.ReadyCondition.Type = vicondition.ReadyType
+			args.ReadyCondition.Type = vicondition.ReadyType.String()
 			var sourcesMock SourcesMock
 			cleanUpCalled := false
 			vi := virtv2.VirtualImage{
 				Status: virtv2.VirtualImageStatus{
 					Conditions: []metav1.Condition{
 						args.ReadyCondition,
 						{
-							Type:   vicondition.StorageClassReadyType,
+							Type:   vicondition.StorageClassReadyType.String(),
 							Status: metav1.ConditionTrue,
 						},
 						{
-							Type:   vicondition.DatasourceReadyType,
+							Type:   vicondition.DatasourceReadyType.String(),
 							Status: metav1.ConditionTrue,
 						},
 					},
@@ -112,8 +112,8 @@ var _ = Describe("LifeCycleHandler Run", func() {
 	DescribeTable(
 		"Verification that LifeCycle.CleanUp is called after the StorageClassReady status becomes false",
 		func(args cleanupAfterScNotReadyTestArgs) {
-			args.ReadyCondition.Type = vicondition.ReadyType
-			args.StorageClassReadyCondition.Type = vicondition.StorageClassReadyType
+			args.ReadyCondition.Type = vicondition.ReadyType.String()
+			args.StorageClassReadyCondition.Type = vicondition.StorageClassReadyType.String()
 			var sourcesMock SourcesMock
 			cleanUpCalled := false
 			vi := virtv2.VirtualImage{
@@ -125,7 +125,7 @@ var _ = Describe("LifeCycleHandler Run", func() {
 						args.ReadyCondition,
 						args.StorageClassReadyCondition,
 						{
-							Type:   vicondition.DatasourceReadyType,
+							Type:   vicondition.DatasourceReadyType.String(),
 							Status: metav1.ConditionTrue,
 						},
 					},