From dedcac89b6414c0023fa2a464c6c05284a8358d3 Mon Sep 17 00:00:00 2001 From: facchettos Date: Fri, 12 Jul 2024 14:39:58 +0200 Subject: [PATCH 1/6] migrated to new patchers and syncers --- .../resources/namespaces/syncer.go | 28 ++++++++++++------- .../resources/namespaces/translate.go | 16 ++++------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/pkg/controllers/resources/namespaces/syncer.go b/pkg/controllers/resources/namespaces/syncer.go index f4a0d81df..d01cbefb7 100644 --- a/pkg/controllers/resources/namespaces/syncer.go +++ b/pkg/controllers/resources/namespaces/syncer.go @@ -1,13 +1,17 @@ package namespaces import ( + "fmt" + "github.com/loft-sh/vcluster/pkg/constants" synccontext "github.com/loft-sh/vcluster/pkg/controllers/syncer/context" "github.com/loft-sh/vcluster/pkg/controllers/syncer/translator" + "github.com/loft-sh/vcluster/pkg/patcher" syncertypes "github.com/loft-sh/vcluster/pkg/types" "github.com/loft-sh/vcluster/pkg/util/translate" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilerrors "k8s.io/apimachinery/pkg/util/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -68,18 +72,22 @@ func (s *namespaceSyncer) SyncToHost(ctx *synccontext.SyncContext, vObj client.O return ctrl.Result{}, s.EnsureWorkloadServiceAccount(ctx, newNamespace.Name) } -func (s *namespaceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (ctrl.Result, error) { - updated := s.translateUpdate(ctx.Context, pObj.(*corev1.Namespace), vObj.(*corev1.Namespace)) - if updated != nil { - ctx.Log.Infof("updating physical namespace %s, because virtual namespace has changed", updated.Name) - translator.PrintChanges(pObj, updated, ctx.Log) - err := ctx.PhysicalClient.Update(ctx.Context, updated) - if err != nil { - return ctrl.Result{}, err - } +func (s *namespaceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (_ ctrl.Result, retErr error) { + patch, err := patcher.NewSyncerPatcher(ctx, pObj, vObj) + if err != nil { + return ctrl.Result{}, fmt.Errorf("new syncer patcher: %w", err) } + defer func() { + if err := patch.Patch(ctx, pObj, vObj); err != nil { + retErr = utilerrors.NewAggregate([]error{retErr, err}) + } + }() + // cast objects + pNamespace, vNamespace, sourceObject, targetObject := synccontext.Cast[*corev1.Namespace](ctx, pObj, vObj) + + s.translateUpdate(ctx.Context, pNamespace, vNamespace, sourceObject, targetObject) - return ctrl.Result{}, s.EnsureWorkloadServiceAccount(ctx, pObj.GetName()) + return ctrl.Result{}, nil } func (s *namespaceSyncer) EnsureWorkloadServiceAccount(ctx *synccontext.SyncContext, pNamespace string) error { diff --git a/pkg/controllers/resources/namespaces/translate.go b/pkg/controllers/resources/namespaces/translate.go index 060a87632..675c3c317 100644 --- a/pkg/controllers/resources/namespaces/translate.go +++ b/pkg/controllers/resources/namespaces/translate.go @@ -2,10 +2,9 @@ package namespaces import ( "context" + "maps" - "github.com/loft-sh/vcluster/pkg/controllers/syncer/translator" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -24,8 +23,8 @@ func (s *namespaceSyncer) translate(ctx context.Context, vObj client.Object) *co return newNamespace } -func (s *namespaceSyncer) translateUpdate(ctx context.Context, pObj, vObj *corev1.Namespace) *corev1.Namespace { - var updated *corev1.Namespace +func (s *namespaceSyncer) translateUpdate(ctx context.Context, pObj, vObj, sourceObject, targetObject *corev1.Namespace) { + targetObject.Spec = sourceObject.Spec _, updatedAnnotations, updatedLabels := s.TranslateMetadataUpdate(ctx, vObj, pObj) if updatedLabels == nil { @@ -38,11 +37,8 @@ func (s *namespaceSyncer) translateUpdate(ctx context.Context, pObj, vObj *corev // set the kubernetes.io/metadata.name label updatedLabels[corev1.LabelMetadataName] = pObj.Name // check if any labels or annotations changed - if !equality.Semantic.DeepEqual(updatedAnnotations, pObj.GetAnnotations()) || !equality.Semantic.DeepEqual(updatedLabels, pObj.GetLabels()) { - updated = translator.NewIfNil(updated, pObj) - updated.Annotations = updatedAnnotations - updated.Labels = updatedLabels + if !maps.Equal(updatedAnnotations, pObj.GetAnnotations()) || !maps.Equal(updatedLabels, pObj.GetLabels()) { + pObj.Annotations = updatedAnnotations + pObj.Labels = updatedLabels } - - return updated } From f637df8560d6d2b85a51a0ca215d1758ea1a5641 Mon Sep 17 00:00:00 2001 From: facchettos Date: Fri, 12 Jul 2024 15:08:05 +0200 Subject: [PATCH 2/6] removed spec syncing --- pkg/controllers/resources/namespaces/translate.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/controllers/resources/namespaces/translate.go b/pkg/controllers/resources/namespaces/translate.go index 675c3c317..e0fdafe19 100644 --- a/pkg/controllers/resources/namespaces/translate.go +++ b/pkg/controllers/resources/namespaces/translate.go @@ -24,8 +24,6 @@ func (s *namespaceSyncer) translate(ctx context.Context, vObj client.Object) *co } func (s *namespaceSyncer) translateUpdate(ctx context.Context, pObj, vObj, sourceObject, targetObject *corev1.Namespace) { - targetObject.Spec = sourceObject.Spec - _, updatedAnnotations, updatedLabels := s.TranslateMetadataUpdate(ctx, vObj, pObj) if updatedLabels == nil { updatedLabels = map[string]string{} From 5f5268115178034a88e596bd5e1de48ff97b0d4a Mon Sep 17 00:00:00 2001 From: facchettos Date: Fri, 12 Jul 2024 15:39:13 +0200 Subject: [PATCH 3/6] fixed return value --- pkg/controllers/resources/namespaces/syncer.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/controllers/resources/namespaces/syncer.go b/pkg/controllers/resources/namespaces/syncer.go index d01cbefb7..fd5b09543 100644 --- a/pkg/controllers/resources/namespaces/syncer.go +++ b/pkg/controllers/resources/namespaces/syncer.go @@ -11,7 +11,6 @@ import ( "github.com/loft-sh/vcluster/pkg/util/translate" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilerrors "k8s.io/apimachinery/pkg/util/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -72,21 +71,24 @@ func (s *namespaceSyncer) SyncToHost(ctx *synccontext.SyncContext, vObj client.O return ctrl.Result{}, s.EnsureWorkloadServiceAccount(ctx, newNamespace.Name) } -func (s *namespaceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (_ ctrl.Result, retErr error) { +func (s *namespaceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (ctrl.Result, error) { patch, err := patcher.NewSyncerPatcher(ctx, pObj, vObj) if err != nil { return ctrl.Result{}, fmt.Errorf("new syncer patcher: %w", err) } - defer func() { - if err := patch.Patch(ctx, pObj, vObj); err != nil { - retErr = utilerrors.NewAggregate([]error{retErr, err}) - } - }() // cast objects pNamespace, vNamespace, sourceObject, targetObject := synccontext.Cast[*corev1.Namespace](ctx, pObj, vObj) s.translateUpdate(ctx.Context, pNamespace, vNamespace, sourceObject, targetObject) + err = s.EnsureWorkloadServiceAccount(ctx, pNamespace.Name) + if err != nil { + return ctrl.Result{}, err + } + + if err := patch.Patch(ctx, pObj, vObj); err != nil { + return ctrl.Result{}, err + } return ctrl.Result{}, nil } From c9c946209f2106e371d747275d55337be68adeb8 Mon Sep 17 00:00:00 2001 From: facchettos Date: Fri, 12 Jul 2024 15:45:16 +0200 Subject: [PATCH 4/6] simplified --- pkg/controllers/resources/namespaces/syncer.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/controllers/resources/namespaces/syncer.go b/pkg/controllers/resources/namespaces/syncer.go index fd5b09543..b7ba06753 100644 --- a/pkg/controllers/resources/namespaces/syncer.go +++ b/pkg/controllers/resources/namespaces/syncer.go @@ -86,10 +86,7 @@ func (s *namespaceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, return ctrl.Result{}, err } - if err := patch.Patch(ctx, pObj, vObj); err != nil { - return ctrl.Result{}, err - } - return ctrl.Result{}, nil + return ctrl.Result{}, patch.Patch(ctx, pObj, vObj) } func (s *namespaceSyncer) EnsureWorkloadServiceAccount(ctx *synccontext.SyncContext, pNamespace string) error { From ceb566b1770a1d15507b0b13cbe3a997a9d06d84 Mon Sep 17 00:00:00 2001 From: facchettos Date: Fri, 12 Jul 2024 16:02:12 +0200 Subject: [PATCH 5/6] unused vars --- pkg/controllers/resources/namespaces/syncer.go | 4 ++-- pkg/controllers/resources/namespaces/translate.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/controllers/resources/namespaces/syncer.go b/pkg/controllers/resources/namespaces/syncer.go index b7ba06753..a292c0e61 100644 --- a/pkg/controllers/resources/namespaces/syncer.go +++ b/pkg/controllers/resources/namespaces/syncer.go @@ -77,9 +77,9 @@ func (s *namespaceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, return ctrl.Result{}, fmt.Errorf("new syncer patcher: %w", err) } // cast objects - pNamespace, vNamespace, sourceObject, targetObject := synccontext.Cast[*corev1.Namespace](ctx, pObj, vObj) + pNamespace, vNamespace, _, _ := synccontext.Cast[*corev1.Namespace](ctx, pObj, vObj) - s.translateUpdate(ctx.Context, pNamespace, vNamespace, sourceObject, targetObject) + s.translateUpdate(ctx.Context, pNamespace, vNamespace) err = s.EnsureWorkloadServiceAccount(ctx, pNamespace.Name) if err != nil { diff --git a/pkg/controllers/resources/namespaces/translate.go b/pkg/controllers/resources/namespaces/translate.go index e0fdafe19..28b89c50e 100644 --- a/pkg/controllers/resources/namespaces/translate.go +++ b/pkg/controllers/resources/namespaces/translate.go @@ -23,7 +23,7 @@ func (s *namespaceSyncer) translate(ctx context.Context, vObj client.Object) *co return newNamespace } -func (s *namespaceSyncer) translateUpdate(ctx context.Context, pObj, vObj, sourceObject, targetObject *corev1.Namespace) { +func (s *namespaceSyncer) translateUpdate(ctx context.Context, pObj, vObj *corev1.Namespace) { _, updatedAnnotations, updatedLabels := s.TranslateMetadataUpdate(ctx, vObj, pObj) if updatedLabels == nil { updatedLabels = map[string]string{} From c08e0e36bf58a7df6e52063cd0e9270032ecf6f2 Mon Sep 17 00:00:00 2001 From: facchettos Date: Fri, 12 Jul 2024 16:32:30 +0200 Subject: [PATCH 6/6] now uses defer and updates objs even when workload fails --- pkg/controllers/resources/namespaces/syncer.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/controllers/resources/namespaces/syncer.go b/pkg/controllers/resources/namespaces/syncer.go index a292c0e61..9ae64d165 100644 --- a/pkg/controllers/resources/namespaces/syncer.go +++ b/pkg/controllers/resources/namespaces/syncer.go @@ -11,6 +11,7 @@ import ( "github.com/loft-sh/vcluster/pkg/util/translate" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilerrors "k8s.io/apimachinery/pkg/util/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -71,22 +72,24 @@ func (s *namespaceSyncer) SyncToHost(ctx *synccontext.SyncContext, vObj client.O return ctrl.Result{}, s.EnsureWorkloadServiceAccount(ctx, newNamespace.Name) } -func (s *namespaceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (ctrl.Result, error) { +func (s *namespaceSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (_ ctrl.Result, retErr error) { patch, err := patcher.NewSyncerPatcher(ctx, pObj, vObj) if err != nil { return ctrl.Result{}, fmt.Errorf("new syncer patcher: %w", err) } + + defer func() { + if err := patch.Patch(ctx, pObj, vObj); err != nil { + retErr = utilerrors.NewAggregate([]error{retErr, err}) + } + }() + // cast objects pNamespace, vNamespace, _, _ := synccontext.Cast[*corev1.Namespace](ctx, pObj, vObj) s.translateUpdate(ctx.Context, pNamespace, vNamespace) - err = s.EnsureWorkloadServiceAccount(ctx, pNamespace.Name) - if err != nil { - return ctrl.Result{}, err - } - - return ctrl.Result{}, patch.Patch(ctx, pObj, vObj) + return ctrl.Result{}, s.EnsureWorkloadServiceAccount(ctx, pNamespace.Name) } func (s *namespaceSyncer) EnsureWorkloadServiceAccount(ctx *synccontext.SyncContext, pNamespace string) error {