Skip to content

Commit

Permalink
fix for synced pvs and retain SC
Browse files Browse the repository at this point in the history
  • Loading branch information
@facchettos
facchettos committed Apr 29, 2024
1 parent 8c9d335 commit 90d3139
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 33 deletions.
9 changes: 6 additions & 3 deletions pkg/controllers/resources/persistentvolumeclaims/syncer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@ const (
)

func New(ctx *synccontext.RegisterContext) (syncer.Object, error) {
storageClassesEnabled := ctx.Config.Sync.ToHost.StorageClasses.Enabled
syncSCToHost := ctx.Config.Sync.ToHost.StorageClasses.Enabled
syncSCFromHost := ctx.Config.Sync.FromHost.StorageClasses.Enabled
excludedAnnotations := []string{bindCompletedAnnotation, boundByControllerAnnotation, storageProvisionerAnnotation}
return &persistentVolumeClaimSyncer{
NamespacedTranslator: translator.NewNamespacedTranslator(ctx, "persistent-volume-claim", &corev1.PersistentVolumeClaim{}, excludedAnnotations...),

storageClassesEnabled: storageClassesEnabled,
syncSCToHost: syncSCToHost,
syncSCFromHost: syncSCFromHost,

Check failure on line 45 in pkg/controllers/resources/persistentvolumeclaims/syncer.go

View workflow job for this annotation

GitHub Actions / Execute all go tests 

cannot use syncSCFromHost (variable of type "github.com/loft-sh/vcluster/config".StrBool) as bool value in struct literal

Check failure on line 45 in pkg/controllers/resources/persistentvolumeclaims/syncer.go

View workflow job for this annotation

GitHub Actions / lint 

cannot use syncSCFromHost (variable of type "github.com/loft-sh/vcluster/config".StrBool) as bool value in struct literal) (typecheck)

Check failure on line 45 in pkg/controllers/resources/persistentvolumeclaims/syncer.go

View workflow job for this annotation

GitHub Actions / lint 

cannot use syncSCFromHost (variable of type "github.com/loft-sh/vcluster/config".StrBool) as bool value in struct literal (typecheck)

Check failure on line 45 in pkg/controllers/resources/persistentvolumeclaims/syncer.go

View workflow job for this annotation

GitHub Actions / build-and-push-syncer-image 

cannot use syncSCFromHost (variable of type "github.com/loft-sh/vcluster/config".StrBool) as bool value in struct literal
schedulerEnabled: ctx.Config.ControlPlane.Advanced.VirtualScheduler.Enabled,
useFakePersistentVolumes: !ctx.Config.Sync.ToHost.PersistentVolumes.Enabled,
}, nil
Expand All @@ -49,7 +51,8 @@ func New(ctx *synccontext.RegisterContext) (syncer.Object, error) {
type persistentVolumeClaimSyncer struct {
translator.NamespacedTranslator

storageClassesEnabled bool
syncSCToHost bool
syncSCFromHost bool
schedulerEnabled bool
useFakePersistentVolumes bool
}
Expand Down
7 changes: 2 additions & 5 deletions pkg/controllers/resources/persistentvolumeclaims/translate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (s *persistentVolumeClaimSyncer) translateSelector(ctx *synccontext.SyncCon
}

// translate storage class if we manage those in vcluster
if s.storageClassesEnabled && storageClassName != "" {
if s.syncSCToHost && storageClassName != "" {
translated := translate.Default.PhysicalNameClusterScoped(storageClassName)
delete(vPvc.Annotations, deprecatedStorageClassAnnotation)
vPvc.Spec.StorageClassName = &translated
Expand All @@ -61,11 +61,8 @@ func (s *persistentVolumeClaimSyncer) translateSelector(ctx *synccontext.SyncCon
if vPvc.Spec.Selector != nil {
vPvc.Spec.Selector = translate.Default.TranslateLabelSelectorCluster(vPvc.Spec.Selector)
}
if vPvc.Spec.VolumeName != "" {
vPvc.Spec.VolumeName = translate.Default.PhysicalNameClusterScoped(vPvc.Spec.VolumeName)
}
// check if the storage class exists in the physical cluster
if !s.storageClassesEnabled && storageClassName != "" {
if !(s.syncSCToHost || s.syncSCFromHost) && storageClassName != "" {
// Should the PVC be dynamically provisioned or not?
if vPvc.Spec.Selector == nil && vPvc.Spec.VolumeName == "" {
err := ctx.PhysicalClient.Get(ctx.Context, types.NamespacedName{Name: storageClassName}, &storagev1.StorageClass{})
Expand Down
84 changes: 59 additions & 25 deletions pkg/controllers/resources/persistentvolumes/syncer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ func NewSyncer(ctx *synccontext.RegisterContext) (syncertypes.Object, error) {
return &persistentVolumeSyncer{
Translator: translator.NewClusterTranslator(ctx, "persistentvolume", &corev1.PersistentVolume{}, NewPersistentVolumeTranslator(), HostClusterPersistentVolumeAnnotation),

virtualClient: ctx.VirtualManager.GetClient(),
virtualClient: ctx.VirtualManager.GetClient(),
physicalClient: ctx.PhysicalManager.GetClient(),
useFakePersistentVolumes: !ctx.Config.Sync.ToHost.PersistentVolumes.Enabled,
}, nil
}

Expand Down Expand Up @@ -66,7 +68,9 @@ func NewPersistentVolumeTranslator() translate.PhysicalNameTranslator {
type persistentVolumeSyncer struct {
translator.Translator

virtualClient client.Client
virtualClient client.Client
physicalClient client.Client
useFakePersistentVolumes bool
}

var _ syncertypes.IndicesRegisterer = &persistentVolumeSyncer{}
Expand Down Expand Up @@ -142,26 +146,32 @@ func (s *persistentVolumeSyncer) Sync(ctx *synccontext.SyncContext, pObj client.
}

// check if the persistent volume should get synced
sync, vPvc, err := s.shouldSync(ctx.Context, pPersistentVolume)
sync, vPvc, pPvc, err := s.shouldSync(ctx.Context, pPersistentVolume)
if err != nil {
return ctrl.Result{}, err
} else if !sync {
ctx.Log.Infof("delete virtual persistent volume %s, because there is no virtual persistent volume claim with that volume", vPersistentVolume.Name)
return ctrl.Result{}, ctx.VirtualClient.Delete(ctx.Context, vObj)
}

syncBack := true
if !s.useFakePersistentVolumes && vPersistentVolume.Spec.ClaimRef == nil && pPersistentVolume.Spec.ClaimRef != nil && pPvc == nil {
syncBack = false
}
// check if there is a corresponding virtual pvc
updatedObj := s.translateUpdateBackwards(vPersistentVolume, pPersistentVolume, vPvc)
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.Context, updatedObj)
if err != nil {
return ctrl.Result{}, err
}
if syncBack {
updatedObj := s.translateUpdateBackwards(vPersistentVolume, pPersistentVolume, vPvc)
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.Context, updatedObj)
if err != nil {
return ctrl.Result{}, err
}

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

// check status
Expand All @@ -180,7 +190,7 @@ func (s *persistentVolumeSyncer) Sync(ctx *synccontext.SyncContext, pObj client.
}

// update the physical persistent volume if the virtual has changed
if vPersistentVolume.Annotations == nil || vPersistentVolume.Annotations[HostClusterPersistentVolumeAnnotation] == "" {
if vPersistentVolume.Annotations == nil || vPersistentVolume.Annotations[HostClusterPersistentVolumeAnnotation] == "" || vPersistentVolume.Spec.ClaimRef == nil {
if vPersistentVolume.DeletionTimestamp != nil {
if pPersistentVolume.DeletionTimestamp != nil {
return ctrl.Result{}, nil
Expand Down Expand Up @@ -221,7 +231,7 @@ var _ syncertypes.ToVirtualSyncer = &persistentVolumeSyncer{}

func (s *persistentVolumeSyncer) SyncToVirtual(ctx *synccontext.SyncContext, pObj client.Object) (ctrl.Result, error) {
pPersistentVolume := pObj.(*corev1.PersistentVolume)
sync, vPvc, err := s.shouldSync(ctx.Context, pPersistentVolume)
sync, vPvc, _, err := s.shouldSync(ctx.Context, pPersistentVolume)
if err != nil {
return ctrl.Result{}, err
} else if translate.Default.IsManagedCluster(pObj) {
Expand All @@ -240,33 +250,57 @@ func (s *persistentVolumeSyncer) SyncToVirtual(ctx *synccontext.SyncContext, pOb
return ctrl.Result{}, nil
}

func (s *persistentVolumeSyncer) shouldSync(ctx context.Context, pObj *corev1.PersistentVolume) (bool, *corev1.PersistentVolumeClaim, error) {
func (s *persistentVolumeSyncer) shouldSync(ctx context.Context, pObj *corev1.PersistentVolume) (shouldSync bool, vpvc *corev1.PersistentVolumeClaim, ppvc *corev1.PersistentVolumeClaim, err error) {
// is there an assigned PVC?
if pObj.Spec.ClaimRef == nil {
if translate.Default.IsManagedCluster(pObj) {
return true, nil, nil
return true, nil, nil, nil
}

return false, nil, nil
return false, nil, nil, nil
}

vPvc := &corev1.PersistentVolumeClaim{}
err := clienthelper.GetByIndex(ctx, s.virtualClient, vPvc, constants.IndexByPhysicalName, pObj.Spec.ClaimRef.Namespace+"/"+pObj.Spec.ClaimRef.Name)
err = clienthelper.GetByIndex(ctx, s.virtualClient, vPvc, constants.IndexByPhysicalName, pObj.Spec.ClaimRef.Namespace+"/"+pObj.Spec.ClaimRef.Name)
if err != nil {
if !kerrors.IsNotFound(err) {
return false, nil, err
return false, nil, nil, err
} else if translate.Default.IsManagedCluster(pObj) {
return true, nil, nil
return true, nil, nil, nil
}

namespace, err := translate.Default.LegacyGetTargetNamespace()
if err != nil {
return false, nil, nil
return false, nil, nil, nil
}
return pObj.Spec.ClaimRef.Namespace == namespace && pObj.Spec.PersistentVolumeReclaimPolicy == corev1.PersistentVolumeReclaimRetain, nil, nil
return pObj.Spec.ClaimRef.Namespace == namespace && pObj.Spec.PersistentVolumeReclaimPolicy == corev1.PersistentVolumeReclaimRetain, nil, nil, nil
}

err = clienthelper.GetByIndex(ctx, s.virtualClient, vPvc, constants.IndexByPhysicalName, pObj.Spec.ClaimRef.Namespace+"/"+pObj.Spec.ClaimRef.Name)
if err != nil {
if !kerrors.IsNotFound(err) {
return false, nil, nil, err
} else if translate.Default.IsManagedCluster(pObj) {
return true, nil, nil, nil
}

namespace, err := translate.Default.LegacyGetTargetNamespace()
if err != nil {
return false, nil, nil, nil
}
return pObj.Spec.ClaimRef.Namespace == namespace && pObj.Spec.PersistentVolumeReclaimPolicy == corev1.PersistentVolumeReclaimRetain, nil, nil, nil
}

pPvc := &corev1.PersistentVolumeClaim{}
err = clienthelper.GetByIndex(ctx, s.virtualClient, pPvc, constants.IndexByPhysicalName, pObj.Spec.ClaimRef.Namespace+"/"+pObj.Spec.ClaimRef.Name)
s.physicalClient.Get(ctx, types.NamespacedName{Namespace: pObj.Spec.ClaimRef.Namespace, Name: pObj.Spec.ClaimRef.Name}, pPvc)

Check failure on line 296 in pkg/controllers/resources/persistentvolumes/syncer.go

View workflow job for this annotation

GitHub Actions / lint 

Error return value of `s.physicalClient.Get` is not checked (errcheck)
if kerrors.IsNotFound(err) {
return true, vPvc, nil, nil
} else if err != nil {
return true, vPvc, nil, err
}

return true, vPvc, nil
return true, vPvc, pPvc, nil
}

func (s *persistentVolumeSyncer) IsManaged(ctx context.Context, pObj client.Object) (bool, error) {
Expand All @@ -275,7 +309,7 @@ func (s *persistentVolumeSyncer) IsManaged(ctx context.Context, pObj client.Obje
return false, nil
}

sync, _, err := s.shouldSync(ctx, pPv)
sync, _, _, err := s.shouldSync(ctx, pPv)
if err != nil {
return false, nil
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/controllers/resources/persistentvolumes/translate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ func (s *persistentVolumeSyncer) translateUpdate(ctx context.Context, vPv *corev
updated.Spec.Capacity = vPv.Spec.Capacity
}

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

if !equality.Semantic.DeepEqual(pPv.Spec.AccessModes, vPv.Spec.AccessModes) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.AccessModes = vPv.Spec.AccessModes
Expand All @@ -124,6 +129,10 @@ func (s *persistentVolumeSyncer) translateUpdate(ctx context.Context, vPv *corev
}

translatedStorageClassName := translateStorageClass(vPv.Spec.StorageClassName)
if !s.useFakePersistentVolumes {
translatedStorageClassName = vPv.Spec.StorageClassName
}

if !equality.Semantic.DeepEqual(pPv.Spec.StorageClassName, translatedStorageClassName) {
updated = translator.NewIfNil(updated, pPv)
updated.Spec.StorageClassName = translatedStorageClassName
Expand Down

0 comments on commit 90d3139

Please sign in to comment.