Skip to content

Commit

Permalink
refactor: use patcher for persistent volumes & volumesnapshot contents
Browse files Browse the repository at this point in the history
  • Loading branch information
FabianKramm committed Jul 19, 2024
1 parent 8d52244 commit 2650c10
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 177 deletions.
80 changes: 39 additions & 41 deletions pkg/controllers/resources/persistentvolumes/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package persistentvolumes

import (
"context"
"fmt"
"time"

"github.com/loft-sh/vcluster/pkg/constants"
Expand All @@ -10,12 +11,13 @@ import (
"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"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/handler"

"github.com/loft-sh/vcluster/pkg/util/translate"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -103,7 +105,7 @@ func (s *persistentVolumeSyncer) SyncToHost(ctx *synccontext.SyncContext, vObj c
return ctrl.Result{}, nil
}

func (s *persistentVolumeSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (ctrl.Result, error) {
func (s *persistentVolumeSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (_ ctrl.Result, retErr error) {
pPersistentVolume := pObj.(*corev1.PersistentVolume)
vPersistentVolume := vObj.(*corev1.PersistentVolume)

Expand Down Expand Up @@ -144,37 +146,6 @@ func (s *persistentVolumeSyncer) Sync(ctx *synccontext.SyncContext, pObj client.
return ctrl.Result{}, ctx.VirtualClient.Delete(ctx, vObj)
}

// check if there is a corresponding virtual pvc
updatedObj, err := s.translateUpdateBackwards(ctx, vPersistentVolume, pPersistentVolume, vPvc)
if err != nil {
return ctrl.Result{}, err
} else if updatedObj != nil {
ctx.Log.Infof("update virtual persistent volume %s, because spec has changed", vPersistentVolume.Name)
translator.PrintChanges(vPersistentVolume, updatedObj, ctx.Log)
err = ctx.VirtualClient.Update(ctx, updatedObj)
if err != nil {
return ctrl.Result{}, err
}

// we will reconcile anyways
return ctrl.Result{}, nil
}

// check status
if !equality.Semantic.DeepEqual(vPersistentVolume.Status, pPersistentVolume.Status) {
updatedObj := vPersistentVolume.DeepCopy()
updatedObj.Status = *pPersistentVolume.Status.DeepCopy()
ctx.Log.Infof("update virtual persistent volume %s, because status has changed", vPersistentVolume.Name)
translator.PrintChanges(vPersistentVolume, updatedObj, ctx.Log)
err = ctx.VirtualClient.Status().Update(ctx, updatedObj)
if err != nil {
return ctrl.Result{}, err
}

// we will reconcile anyways
return ctrl.Result{}, nil
}

// update the physical persistent volume if the virtual has changed
if vPersistentVolume.Annotations == nil || vPersistentVolume.Annotations[constants.HostClusterPersistentVolumeAnnotation] == "" {
if vPersistentVolume.DeletionTimestamp != nil {
Expand All @@ -192,16 +163,43 @@ func (s *persistentVolumeSyncer) Sync(ctx *synccontext.SyncContext, pObj client.
}
return ctrl.Result{}, err
}
}

updatedPv := s.translateUpdate(ctx, vPersistentVolume, pPersistentVolume)
if updatedPv != nil {
ctx.Log.Infof("update physical persistent volume %s, because spec or annotations have changed", updatedPv.Name)
translator.PrintChanges(pPersistentVolume, updatedPv, ctx.Log)
err := ctx.PhysicalClient.Update(ctx, updatedPv)
if err != nil {
return ctrl.Result{}, err
}
// patch objects
patch, err := patcher.NewSyncerPatcher(ctx, pPersistentVolume, vPersistentVolume)
if err != nil {
return ctrl.Result{}, fmt.Errorf("new syncer patcher: %w", err)
}
defer func() {
if err := patch.Patch(ctx, pPersistentVolume, vPersistentVolume); err != nil {
retErr = utilerrors.NewAggregate([]error{retErr, err})
}
}()

// bidirectional update
sourceObject, targetObject := synccontext.SyncSourceTarget(ctx, pPersistentVolume, vPersistentVolume)
targetObject.Spec.PersistentVolumeSource = sourceObject.Spec.PersistentVolumeSource
targetObject.Spec.Capacity = sourceObject.Spec.Capacity
targetObject.Spec.AccessModes = sourceObject.Spec.AccessModes
targetObject.Spec.PersistentVolumeReclaimPolicy = sourceObject.Spec.PersistentVolumeReclaimPolicy
targetObject.Spec.NodeAffinity = sourceObject.Spec.NodeAffinity
targetObject.Spec.VolumeMode = sourceObject.Spec.VolumeMode
targetObject.Spec.MountOptions = sourceObject.Spec.MountOptions

// update virtual object
err = s.translateUpdateBackwards(ctx, vPersistentVolume, pPersistentVolume, vPvc)
if err != nil {
return ctrl.Result{}, err
}

// update virtual status
vPersistentVolume.Status = *pPersistentVolume.Status.DeepCopy()

// update host object
if vPersistentVolume.Annotations == nil || vPersistentVolume.Annotations[constants.HostClusterPersistentVolumeAnnotation] == "" {
// TODO: translate the storage secrets
pPersistentVolume.Spec.StorageClassName = mappings.VirtualToHostName(ctx, vPersistentVolume.Spec.StorageClassName, "", mappings.StorageClasses())
_, pPersistentVolume.Annotations, pPersistentVolume.Labels = s.TranslateMetadataUpdate(ctx, vPersistentVolume, pPersistentVolume)
}

return ctrl.Result{}, nil
Expand Down
1 change: 1 addition & 0 deletions pkg/controllers/resources/persistentvolumes/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ func TestSync(t *testing.T) {
err = syncContext.PhysicalClient.Get(ctx, types.NamespacedName{Name: basePPv.Name}, pPv)
assert.NilError(t, err)

syncContext.EventSource = synccontext.EventSourceHost
_, err = syncer.Sync(syncContext, pPv, vPv)
assert.NilError(t, err)
},
Expand Down
75 changes: 4 additions & 71 deletions pkg/controllers/resources/persistentvolumes/translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"

"github.com/loft-sh/vcluster/pkg/constants"
"github.com/loft-sh/vcluster/pkg/controllers/syncer/translator"
"github.com/loft-sh/vcluster/pkg/mappings"
"github.com/loft-sh/vcluster/pkg/util/translate"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -48,9 +47,7 @@ func (s *persistentVolumeSyncer) translateBackwards(pPv *corev1.PersistentVolume
return vObj
}

func (s *persistentVolumeSyncer) translateUpdateBackwards(ctx context.Context, vPv *corev1.PersistentVolume, pPv *corev1.PersistentVolume, vPvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolume, error) {
var updated *corev1.PersistentVolume

func (s *persistentVolumeSyncer) translateUpdateBackwards(ctx context.Context, vPv *corev1.PersistentVolume, pPv *corev1.PersistentVolume, vPvc *corev1.PersistentVolumeClaim) error {
// build virtual persistent volume
translatedSpec := *pPv.Spec.DeepCopy()
isStorageClassCreatedOnVirtual, isClaimRefCreatedOnVirtual := false, false
Expand Down Expand Up @@ -87,78 +84,14 @@ func (s *persistentVolumeSyncer) translateUpdateBackwards(ctx context.Context, v
// check storage class. Do not copy the name, if it was created on virtual.
if !translate.Default.IsManaged(ctx, pPv) {
if !equality.Semantic.DeepEqual(vPv.Spec.StorageClassName, translatedSpec.StorageClassName) && !isStorageClassCreatedOnVirtual {
updated = translator.NewIfNil(updated, vPv)
updated.Spec.StorageClassName = translatedSpec.StorageClassName
vPv.Spec.StorageClassName = translatedSpec.StorageClassName
}
}

// check claim ref. Do not copy, if it was created on virtual.
if !equality.Semantic.DeepEqual(vPv.Spec.ClaimRef, translatedSpec.ClaimRef) && !isClaimRefCreatedOnVirtual {
updated = translator.NewIfNil(updated, vPv)
updated.Spec.ClaimRef = translatedSpec.ClaimRef
}

// check pv size
if vPv.Annotations != nil && vPv.Annotations[constants.HostClusterPersistentVolumeAnnotation] != "" && !equality.Semantic.DeepEqual(pPv.Spec.Capacity, vPv.Spec.Capacity) {
updated = translator.NewIfNil(updated, vPv)
updated.Spec.Capacity = translatedSpec.Capacity
}

return updated, nil
}

func (s *persistentVolumeSyncer) translateUpdate(ctx context.Context, vPv *corev1.PersistentVolume, pPv *corev1.PersistentVolume) *corev1.PersistentVolume {
var updated *corev1.PersistentVolume

// TODO: translate the storage secrets
if !equality.Semantic.DeepEqual(pPv.Spec.PersistentVolumeSource, vPv.Spec.PersistentVolumeSource) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.PersistentVolumeSource = vPv.Spec.PersistentVolumeSource
}

if !equality.Semantic.DeepEqual(pPv.Spec.Capacity, vPv.Spec.Capacity) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.Capacity = vPv.Spec.Capacity
}

if !equality.Semantic.DeepEqual(pPv.Spec.AccessModes, vPv.Spec.AccessModes) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.AccessModes = vPv.Spec.AccessModes
}

if !equality.Semantic.DeepEqual(pPv.Spec.PersistentVolumeReclaimPolicy, vPv.Spec.PersistentVolumeReclaimPolicy) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.PersistentVolumeReclaimPolicy = vPv.Spec.PersistentVolumeReclaimPolicy
}

translatedStorageClassName := mappings.VirtualToHostName(ctx, vPv.Spec.StorageClassName, "", mappings.StorageClasses())
if !equality.Semantic.DeepEqual(pPv.Spec.StorageClassName, translatedStorageClassName) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.StorageClassName = translatedStorageClassName
}

if !equality.Semantic.DeepEqual(pPv.Spec.NodeAffinity, vPv.Spec.NodeAffinity) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.NodeAffinity = vPv.Spec.NodeAffinity
}

if !equality.Semantic.DeepEqual(pPv.Spec.VolumeMode, vPv.Spec.VolumeMode) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.VolumeMode = vPv.Spec.VolumeMode
}

if !equality.Semantic.DeepEqual(pPv.Spec.MountOptions, vPv.Spec.MountOptions) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.MountOptions = vPv.Spec.MountOptions
}

// check labels & annotations
changed, updatedAnnotations, updatedLabels := s.TranslateMetadataUpdate(ctx, vPv, pPv)
if changed {
updated = translator.NewIfNil(updated, pPv)
updated.Annotations = updatedAnnotations
updated.Labels = updatedLabels
vPv.Spec.ClaimRef = translatedSpec.ClaimRef
}

return updated
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@ package volumesnapshotcontents

import (
"context"
"fmt"
"time"

"github.com/loft-sh/vcluster/pkg/constants"
"github.com/loft-sh/vcluster/pkg/controllers/syncer/translator"
syncer "github.com/loft-sh/vcluster/pkg/controllers/syncer/types"
"github.com/loft-sh/vcluster/pkg/mappings"
"github.com/loft-sh/vcluster/pkg/patcher"
"github.com/loft-sh/vcluster/pkg/util"
"k8s.io/apimachinery/pkg/types"
utilerrors "k8s.io/apimachinery/pkg/util/errors"

volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
synccontext "github.com/loft-sh/vcluster/pkg/controllers/syncer/context"
Expand Down Expand Up @@ -89,7 +92,7 @@ func (s *volumeSnapshotContentSyncer) SyncToHost(ctx *synccontext.SyncContext, v
return ctrl.Result{}, nil
}

func (s *volumeSnapshotContentSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (ctrl.Result, error) {
func (s *volumeSnapshotContentSyncer) Sync(ctx *synccontext.SyncContext, pObj client.Object, vObj client.Object) (_ ctrl.Result, retErr error) {
pVSC := pObj.(*volumesnapshotv1.VolumeSnapshotContent)
vVSC := vObj.(*volumesnapshotv1.VolumeSnapshotContent)

Expand Down Expand Up @@ -135,7 +138,7 @@ func (s *volumeSnapshotContentSyncer) Sync(ctx *synccontext.SyncContext, pObj cl
}

// check if the VolumeSnapshotContent should get synced
sync, vVS, err := s.shouldSync(ctx, pVSC)
sync, _, err := s.shouldSync(ctx, pVSC)
if err != nil {
return ctrl.Result{}, err
} else if !sync {
Expand All @@ -144,22 +147,6 @@ func (s *volumeSnapshotContentSyncer) Sync(ctx *synccontext.SyncContext, pObj cl
return ctrl.Result{}, nil
}

updatedObj := s.translateUpdateBackwards(pVSC, vVSC, vVS)
if updatedObj != nil {
ctx.Log.Infof("update virtual VolumeSnapshotContent %s, because spec or metadata(annotations or labels) have changed", vVSC.Name)
translator.PrintChanges(vObj, updatedObj, ctx.Log)
return ctrl.Result{}, s.virtualClient.Update(ctx, updatedObj)
}

// update virtual status if it differs
if !equality.Semantic.DeepEqual(vVSC.Status, pVSC.Status) {
newVSC := vVSC.DeepCopy()
newVSC.Status = pVSC.Status.DeepCopy()
translator.PrintChanges(vObj, newVSC, ctx.Log)
ctx.Log.Infof("update virtual VolumeSnapshotContent %s, because status has changed", vVSC.Name)
return ctrl.Result{}, s.virtualClient.Status().Update(ctx, newVSC)
}

// update the physical VolumeSnapshotContent if the virtual has changed
if vVSC.Annotations == nil || vVSC.Annotations[constants.HostClusterVSCAnnotation] == "" {
if vVSC.DeletionTimestamp != nil {
Expand All @@ -177,16 +164,30 @@ func (s *volumeSnapshotContentSyncer) Sync(ctx *synccontext.SyncContext, pObj cl
}
return ctrl.Result{}, err
}
}

updatedPv := s.translateUpdate(ctx, vVSC, pVSC)
if updatedPv != nil {
ctx.Log.Infof("update physical VolumeSnapshotContent %s, because spec or annotations have changed", updatedPv.Name)
translator.PrintChanges(pVSC, updatedPv, ctx.Log)
err := ctx.PhysicalClient.Update(ctx, updatedPv)
if err != nil {
return ctrl.Result{}, err
}
// patch objects
patch, err := patcher.NewSyncerPatcher(ctx, pVSC, vVSC)
if err != nil {
return ctrl.Result{}, fmt.Errorf("new syncer patcher: %w", err)
}
defer func() {
if err := patch.Patch(ctx, pVSC, vVSC); err != nil {
retErr = utilerrors.NewAggregate([]error{retErr, err})
}
}()

// update virtual object
s.translateUpdateBackwards(pVSC, vVSC)

// update virtual status
vVSC.Status = pVSC.Status.DeepCopy()

// update host object
if vVSC.Annotations == nil || vVSC.Annotations[constants.HostClusterVSCAnnotation] == "" {
pVSC.Spec.DeletionPolicy = vVSC.Spec.DeletionPolicy
pVSC.Spec.VolumeSnapshotClassName = vVSC.Spec.VolumeSnapshotClassName
_, pVSC.Annotations, pVSC.Labels = s.TranslateMetadataUpdate(ctx, vVSC, pVSC)
}

return ctrl.Result{}, nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@ import (

volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
"github.com/loft-sh/vcluster/pkg/constants"
"github.com/loft-sh/vcluster/pkg/controllers/syncer/translator"
"github.com/loft-sh/vcluster/pkg/mappings"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

Expand Down Expand Up @@ -41,49 +39,14 @@ func (s *volumeSnapshotContentSyncer) translateBackwards(pVSC *volumesnapshotv1.
return vObj
}

func (s *volumeSnapshotContentSyncer) translateUpdateBackwards(pVSC, vVSC *volumesnapshotv1.VolumeSnapshotContent, _ *volumesnapshotv1.VolumeSnapshot) *volumesnapshotv1.VolumeSnapshotContent {
var updated *volumesnapshotv1.VolumeSnapshotContent

func (s *volumeSnapshotContentSyncer) translateUpdateBackwards(pVSC, vVSC *volumesnapshotv1.VolumeSnapshotContent) {
// add a finalizer to ensure that we delete the physical VolumeSnapshotContent object when virtual is being deleted
pCopy := pVSC.DeepCopy()
if pCopy.Finalizers == nil {
pCopy.Finalizers = []string{}
}
controllerutil.AddFinalizer(pCopy, PhysicalVSCGarbageCollectionFinalizer)

if !equality.Semantic.DeepEqual(vVSC.Finalizers, pCopy.Finalizers) {
updated = translator.NewIfNil(updated, vVSC)
updated.Finalizers = pCopy.Finalizers
}

// TODO: consider syncing certain annotations, e.g.:
// "snapshot.storage.kubernetes.io/volumesnapshot-being-deleted" or
// "snapshot.storage.kubernetes.io/volumesnapshot-being-created"

return updated
}

func (s *volumeSnapshotContentSyncer) translateUpdate(ctx context.Context, vVSC *volumesnapshotv1.VolumeSnapshotContent, pVSC *volumesnapshotv1.VolumeSnapshotContent) *volumesnapshotv1.VolumeSnapshotContent {
var updated *volumesnapshotv1.VolumeSnapshotContent

if !equality.Semantic.DeepEqual(pVSC.Spec.DeletionPolicy, vVSC.Spec.DeletionPolicy) {
updated = translator.NewIfNil(updated, pVSC)
updated.Spec.DeletionPolicy = vVSC.Spec.DeletionPolicy
}

if !equality.Semantic.DeepEqual(pVSC.Spec.VolumeSnapshotClassName, vVSC.Spec.VolumeSnapshotClassName) {
updated = translator.NewIfNil(updated, pVSC)
updated.Spec.VolumeSnapshotClassName = vVSC.Spec.VolumeSnapshotClassName
}

changed, updatedAnnotations, updatedLabels := s.TranslateMetadataUpdate(ctx, vVSC, pVSC)
if changed {
updated = translator.NewIfNil(updated, pVSC)
updated.Annotations = updatedAnnotations
updated.Labels = updatedLabels
}

return updated
vVSC.Finalizers = pCopy.Finalizers
}

func translateVolumeSnapshotRefBackwards(ref *corev1.ObjectReference, vVS *volumesnapshotv1.VolumeSnapshot) corev1.ObjectReference {
Expand Down

0 comments on commit 2650c10

Please sign in to comment.