Merge pull request #1947 from facchettos/poddisruption

added patcher logic for pdb
loft-sh · Jul 18, 2024 · 47ac763 · 47ac763
2 parents 372597d + 9faf34c
commit 47ac763
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 33 deletions.
diff --git a/pkg/controllers/resources/poddisruptionbudgets/syncer.go b/pkg/controllers/resources/poddisruptionbudgets/syncer.go
@@ -1,12 +1,16 @@
 package poddisruptionbudgets
 
 import (
+	"fmt"
+
 	"github.com/loft-sh/vcluster/pkg/controllers/syncer"
 	synccontext "github.com/loft-sh/vcluster/pkg/controllers/syncer/context"
 	"github.com/loft-sh/vcluster/pkg/controllers/syncer/translator"
 	syncertypes "github.com/loft-sh/vcluster/pkg/controllers/syncer/types"
 	"github.com/loft-sh/vcluster/pkg/mappings"
+	"github.com/loft-sh/vcluster/pkg/patcher"
 	policyv1 "k8s.io/api/policy/v1"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -29,13 +33,25 @@ func (pdb *pdbSyncer) SyncToHost(ctx *synccontext.SyncContext, vObj client.Objec
 	return pdb.SyncToHostCreate(ctx, vObj, pdb.translate(ctx, vObj.(*policyv1.PodDisruptionBudget)))
 }
 
-func (pdb *pdbSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (ctrl.Result, error) {
+func (pdb *pdbSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (_ ctrl.Result, retErr error) {
 	vPDB := vObj.(*policyv1.PodDisruptionBudget)
 	pPDB := pObj.(*policyv1.PodDisruptionBudget)
-	newPDB := pdb.translateUpdate(ctx, pPDB, vPDB)
-	if newPDB != nil {
-		translator.PrintChanges(pObj, newPDB, ctx.Log)
+
+	patch, err := patcher.NewSyncerPatcher(ctx, pPDB, vPDB)
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("new syncer patcher: %w", err)
 	}
 
-	return pdb.SyncToHostUpdate(ctx, vObj, newPDB)
+	defer func() {
+		if err := patch.Patch(ctx, pPDB, vPDB); err != nil {
+			retErr = utilerrors.NewAggregate([]error{retErr, err})
+		}
+		if retErr != nil {
+			pdb.EventRecorder().Eventf(vObj, "Warning", "SyncError", "Error syncing: %v", retErr)
+		}
+	}()
+
+	pdb.translateUpdate(ctx.Context, pPDB, vPDB)
+
+	return ctrl.Result{}, nil
 }
diff --git a/pkg/controllers/resources/poddisruptionbudgets/syncer_test.go b/pkg/controllers/resources/poddisruptionbudgets/syncer_test.go
@@ -93,7 +93,7 @@ func TestSync(t *testing.T) {
 			},
 			Sync: func(ctx *synccontext.RegisterContext) {
 				syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
-				_, err := syncer.(*pdbSyncer).SyncToHost(syncCtx, vclusterPDB)
+				_, err := syncer.(*pdbSyncer).SyncToHost(syncCtx, vclusterPDB.DeepCopy())
 				assert.NilError(t, err)
 			},
 		},
@@ -113,7 +113,7 @@ func TestSync(t *testing.T) {
 			},
 			Sync: func(ctx *synccontext.RegisterContext) {
 				syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
-				_, err := syncer.(*pdbSyncer).Sync(syncCtx, hostClusterSyncedPDB, vclusterUpdatedPDB)
+				_, err := syncer.(*pdbSyncer).Sync(syncCtx, hostClusterSyncedPDB.DeepCopy(), vclusterUpdatedPDB.DeepCopy())
 				assert.NilError(t, err)
 			},
 		},
@@ -133,7 +133,7 @@ func TestSync(t *testing.T) {
 			},
 			Sync: func(ctx *synccontext.RegisterContext) {
 				syncCtx, syncer := generictesting.FakeStartSyncer(t, ctx, New)
-				_, err := syncer.(*pdbSyncer).Sync(syncCtx, hostClusterSyncedPDB, vclusterUpdatedSelectorPDB)
+				_, err := syncer.(*pdbSyncer).Sync(syncCtx, hostClusterSyncedPDB.DeepCopy(), vclusterUpdatedSelectorPDB.DeepCopy())
 				assert.NilError(t, err)
 			},
 		},

diff --git a/pkg/controllers/resources/poddisruptionbudgets/translate.go b/pkg/controllers/resources/poddisruptionbudgets/translate.go
@@ -3,10 +3,8 @@ package poddisruptionbudgets
 import (
 	"context"
 
-	"github.com/loft-sh/vcluster/pkg/controllers/syncer/translator"
 	"github.com/loft-sh/vcluster/pkg/util/translate"
 	policyv1 "k8s.io/api/policy/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
 )
 
 func (pdb *pdbSyncer) translate(ctx context.Context, vObj *policyv1.PodDisruptionBudget) *policyv1.PodDisruptionBudget {
@@ -17,31 +15,15 @@ func (pdb *pdbSyncer) translate(ctx context.Context, vObj *policyv1.PodDisruptio
 	return newPDB
 }
 
-func (pdb *pdbSyncer) translateUpdate(ctx context.Context, pObj, vObj *policyv1.PodDisruptionBudget) *policyv1.PodDisruptionBudget {
-	var updated *policyv1.PodDisruptionBudget
-
-	// check max available and min available in spec
-	if !equality.Semantic.DeepEqual(vObj.Spec.MaxUnavailable, pObj.Spec.MaxUnavailable) ||
-		!equality.Semantic.DeepEqual(vObj.Spec.MinAvailable, pObj.Spec.MinAvailable) {
-		updated = translator.NewIfNil(updated, pObj)
-		updated.Spec.MaxUnavailable = vObj.Spec.MaxUnavailable
-		updated.Spec.MinAvailable = vObj.Spec.MinAvailable
-	}
+func (pdb *pdbSyncer) translateUpdate(ctx context.Context, pObj, vObj *policyv1.PodDisruptionBudget) {
+	pObj.Spec.MaxUnavailable = vObj.Spec.MaxUnavailable
+	pObj.Spec.MinAvailable = vObj.Spec.MinAvailable
 
 	// check annotations
-	changed, updatedAnnotations, updatedLabels := pdb.TranslateMetadataUpdate(ctx, vObj, pObj)
-	if changed {
-		updated = translator.NewIfNil(updated, pObj)
-		updated.Annotations = updatedAnnotations
-		updated.Labels = updatedLabels
-	}
+	_, updatedAnnotations, updatedLabels := pdb.TranslateMetadataUpdate(ctx, vObj, pObj)
+	pObj.Annotations = updatedAnnotations
+	pObj.Labels = updatedLabels
 
 	// check LabelSelector
-	vObjLabelSelector := translate.Default.TranslateLabelSelector(vObj.Spec.Selector)
-	if !equality.Semantic.DeepEqual(vObjLabelSelector, pObj.Spec.Selector) {
-		updated = translator.NewIfNil(updated, pObj)
-		updated.Spec.Selector = vObjLabelSelector
-	}
-
-	return updated
+	pObj.Spec.Selector = translate.Default.TranslateLabelSelector(vObj.Spec.Selector)
 }