From aca01127f8af7b5970ab6e9ded4e0800c4875f48 Mon Sep 17 00:00:00 2001 From: Hussein Galal Date: Tue, 16 Sep 2025 13:01:12 +0300 Subject: [PATCH] Fix PVC sync and sync defaults (#458) * Fix PVC sync and sync defaults Signed-off-by: galal-hussein * Fix PVC sync and sync defaults Signed-off-by: galal-hussein * fixes to pvc sync Signed-off-by: galal-hussein * increase the timeout on the e2e test Signed-off-by: galal-hussein * configure the syncConfig correctly in vcp Signed-off-by: galal-hussein * update docs Signed-off-by: galal-hussein * fix policy unit test Signed-off-by: galal-hussein * revert timeout of the test to 20 second Signed-off-by: galal-hussein --------- Signed-off-by: galal-hussein --- charts/k3k/crds/k3k.io_clusters.yaml | 2 +- charts/k3k/crds/k3k.io_virtualclusterpolicies.yaml | 7 ++++--- cli/cmds/cluster_create.go | 11 ----------- docs/crds/crd-docs.md | 4 ++-- k3k-kubelet/controller/syncer/configmap_test.go | 2 +- k3k-kubelet/controller/syncer/ingress_test.go | 2 +- .../controller/syncer/persistentvolumeclaims.go | 2 +- .../controller/syncer/persistentvolumeclaims_test.go | 2 +- k3k-kubelet/controller/syncer/pod.go | 4 ++-- k3k-kubelet/controller/syncer/priority_class_test.go | 2 +- k3k-kubelet/controller/syncer/secret_test.go | 2 +- k3k-kubelet/controller/syncer/service_test.go | 2 +- pkg/apis/k3k.io/v1alpha1/types.go | 9 +++++---- pkg/apis/k3k.io/v1alpha1/zz_generated.deepcopy.go | 12 ++++++++++-- pkg/controller/policy/policy_test.go | 12 ++++++++++-- 15 files changed, 41 insertions(+), 34 deletions(-) diff --git a/charts/k3k/crds/k3k.io_clusters.yaml b/charts/k3k/crds/k3k.io_clusters.yaml index 0f5ee8ab..10b6c270 100644 --- a/charts/k3k/crds/k3k.io_clusters.yaml +++ b/charts/k3k/crds/k3k.io_clusters.yaml @@ -613,7 +613,7 @@ spec: type: object persistentVolumeClaims: default: - enabled: false + enabled: true description: PersistentVolumeClaims resources sync configuration. properties: enabled: diff --git a/charts/k3k/crds/k3k.io_virtualclusterpolicies.yaml b/charts/k3k/crds/k3k.io_virtualclusterpolicies.yaml index 7b1af813..66195a41 100644 --- a/charts/k3k/crds/k3k.io_virtualclusterpolicies.yaml +++ b/charts/k3k/crds/k3k.io_virtualclusterpolicies.yaml @@ -226,8 +226,9 @@ spec: x-kubernetes-list-type: atomic type: object sync: - description: Sync specifies the synchronization configuration for - resources that can be applied to clusters + default: {} + description: Sync specifies the resources types that will be synced + from virtual cluster to host cluster. properties: configmaps: default: @@ -263,7 +264,7 @@ spec: type: object persistentVolumeClaims: default: - enabled: false + enabled: true description: PersistentVolumeClaims resources sync configuration. properties: enabled: diff --git a/cli/cmds/cluster_create.go b/cli/cmds/cluster_create.go index 6f2b0abc..f90a1464 100644 --- a/cli/cmds/cluster_create.go +++ b/cli/cmds/cluster_create.go @@ -195,17 +195,6 @@ func newCluster(name, namespace string, config *CreateConfig) *v1alpha1.Cluster StorageClassName: ptr.To(config.storageClassName), StorageRequestSize: config.storageRequestSize, }, - Sync: v1alpha1.SyncConfig{ - ConfigMaps: v1alpha1.ConfigMapSyncConfig{ - Enabled: true, - }, - Secrets: v1alpha1.SecretSyncConfig{ - Enabled: true, - }, - Services: v1alpha1.ServiceSyncConfig{ - Enabled: true, - }, - }, MirrorHostNodes: config.mirrorHostNodes, }, } diff --git a/docs/crds/crd-docs.md b/docs/crds/crd-docs.md index 5651cb79..56a929b6 100644 --- a/docs/crds/crd-docs.md +++ b/docs/crds/crd-docs.md @@ -429,7 +429,7 @@ _Appears in:_ | `configmaps` _[ConfigMapSyncConfig](#configmapsyncconfig)_ | ConfigMaps resources sync configuration. | \{ enabled:true \} | | | `secrets` _[SecretSyncConfig](#secretsyncconfig)_ | Secrets resources sync configuration. | \{ enabled:true \} | | | `ingresses` _[IngressSyncConfig](#ingresssyncconfig)_ | Ingresses resources sync configuration. | \{ enabled:false \} | | -| `persistentVolumeClaims` _[PersistentVolumeClaimSyncConfig](#persistentvolumeclaimsyncconfig)_ | PersistentVolumeClaims resources sync configuration. | \{ enabled:false \} | | +| `persistentVolumeClaims` _[PersistentVolumeClaimSyncConfig](#persistentvolumeclaimsyncconfig)_ | PersistentVolumeClaims resources sync configuration. | \{ enabled:true \} | | | `priorityClasses` _[PriorityClassSyncConfig](#priorityclasssyncconfig)_ | PriorityClasses resources sync configuration. | \{ enabled:false \} | | @@ -491,7 +491,7 @@ _Appears in:_ | `allowedMode` _[ClusterMode](#clustermode)_ | AllowedMode specifies the allowed cluster provisioning mode. Defaults to "shared". | shared | Enum: [shared virtual]
| | `disableNetworkPolicy` _boolean_ | DisableNetworkPolicy indicates whether to disable the creation of a default network policy for cluster isolation. | | | | `podSecurityAdmissionLevel` _[PodSecurityAdmissionLevel](#podsecurityadmissionlevel)_ | PodSecurityAdmissionLevel specifies the pod security admission level applied to the pods in the namespace. | | Enum: [privileged baseline restricted]
| -| `sync` _[SyncConfig](#syncconfig)_ | Sync specifies the synchronization configuration for resources that can be applied to clusters | | | +| `sync` _[SyncConfig](#syncconfig)_ | Sync specifies the resources types that will be synced from virtual cluster to host cluster. | \{ \} | | diff --git a/k3k-kubelet/controller/syncer/configmap_test.go b/k3k-kubelet/controller/syncer/configmap_test.go index 873dad3b..4b8621ac 100644 --- a/k3k-kubelet/controller/syncer/configmap_test.go +++ b/k3k-kubelet/controller/syncer/configmap_test.go @@ -41,7 +41,7 @@ var ConfigMapTests = func() { Namespace: namespace, }, Spec: v1alpha1.ClusterSpec{ - Sync: v1alpha1.SyncConfig{ + Sync: &v1alpha1.SyncConfig{ ConfigMaps: v1alpha1.ConfigMapSyncConfig{ Enabled: true, }, diff --git a/k3k-kubelet/controller/syncer/ingress_test.go b/k3k-kubelet/controller/syncer/ingress_test.go index 80c7874e..578f1dd0 100644 --- a/k3k-kubelet/controller/syncer/ingress_test.go +++ b/k3k-kubelet/controller/syncer/ingress_test.go @@ -43,7 +43,7 @@ var IngressTests = func() { Namespace: namespace, }, Spec: v1alpha1.ClusterSpec{ - Sync: v1alpha1.SyncConfig{ + Sync: &v1alpha1.SyncConfig{ Ingresses: v1alpha1.IngressSyncConfig{ Enabled: true, }, diff --git a/k3k-kubelet/controller/syncer/persistentvolumeclaims.go b/k3k-kubelet/controller/syncer/persistentvolumeclaims.go index 1884a1dd..2aa3ecdc 100644 --- a/k3k-kubelet/controller/syncer/persistentvolumeclaims.go +++ b/k3k-kubelet/controller/syncer/persistentvolumeclaims.go @@ -102,7 +102,7 @@ func (r *PVCReconciler) Reconcile(ctx context.Context, req reconcile.Request) (r // handle deletion if !virtPVC.DeletionTimestamp.IsZero() { // deleting the synced pvc if exists - if err := r.HostClient.Delete(ctx, syncedPVC); !apierrors.IsNotFound(err) { + if err := r.HostClient.Delete(ctx, syncedPVC); err != nil && !apierrors.IsNotFound(err) { return reconcile.Result{}, err } // remove the finalizer after cleaning up the synced pvc diff --git a/k3k-kubelet/controller/syncer/persistentvolumeclaims_test.go b/k3k-kubelet/controller/syncer/persistentvolumeclaims_test.go index a43b4c8d..d602c170 100644 --- a/k3k-kubelet/controller/syncer/persistentvolumeclaims_test.go +++ b/k3k-kubelet/controller/syncer/persistentvolumeclaims_test.go @@ -42,7 +42,7 @@ var PVCTests = func() { Namespace: namespace, }, Spec: v1alpha1.ClusterSpec{ - Sync: v1alpha1.SyncConfig{ + Sync: &v1alpha1.SyncConfig{ PersistentVolumeClaims: v1alpha1.PersistentVolumeClaimSyncConfig{ Enabled: true, }, diff --git a/k3k-kubelet/controller/syncer/pod.go b/k3k-kubelet/controller/syncer/pod.go index e9994d6b..ee110b12 100644 --- a/k3k-kubelet/controller/syncer/pod.go +++ b/k3k-kubelet/controller/syncer/pod.go @@ -66,7 +66,7 @@ func (r *PodReconciler) filterResources(object ctrlruntimeclient.Object) bool { } func (r *PodReconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { - log := ctrl.LoggerFrom(ctx).WithValues("cluster", r.ClusterName, "clusterNamespace", r.ClusterName) + log := ctrl.LoggerFrom(ctx).WithValues("cluster", r.ClusterName, "clusterNamespace", r.ClusterNamespace) ctx = ctrl.LoggerInto(ctx, log) var ( @@ -74,7 +74,7 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req reconcile.Request) (r cluster v1alpha1.Cluster ) - if err := r.HostClient.Get(ctx, types.NamespacedName{Name: r.ClusterName, Namespace: r.ClusterName}, &cluster); err != nil { + if err := r.HostClient.Get(ctx, types.NamespacedName{Name: r.ClusterName, Namespace: r.ClusterNamespace}, &cluster); err != nil { return reconcile.Result{}, err } diff --git a/k3k-kubelet/controller/syncer/priority_class_test.go b/k3k-kubelet/controller/syncer/priority_class_test.go index b0eea775..b2bdefa6 100644 --- a/k3k-kubelet/controller/syncer/priority_class_test.go +++ b/k3k-kubelet/controller/syncer/priority_class_test.go @@ -42,7 +42,7 @@ var PriorityClassTests = func() { Namespace: namespace, }, Spec: v1alpha1.ClusterSpec{ - Sync: v1alpha1.SyncConfig{ + Sync: &v1alpha1.SyncConfig{ PriorityClasses: v1alpha1.PriorityClassSyncConfig{ Enabled: true, }, diff --git a/k3k-kubelet/controller/syncer/secret_test.go b/k3k-kubelet/controller/syncer/secret_test.go index 829b8b45..712e9f76 100644 --- a/k3k-kubelet/controller/syncer/secret_test.go +++ b/k3k-kubelet/controller/syncer/secret_test.go @@ -41,7 +41,7 @@ var SecretTests = func() { Namespace: namespace, }, Spec: v1alpha1.ClusterSpec{ - Sync: v1alpha1.SyncConfig{ + Sync: &v1alpha1.SyncConfig{ Secrets: v1alpha1.SecretSyncConfig{ Enabled: true, }, diff --git a/k3k-kubelet/controller/syncer/service_test.go b/k3k-kubelet/controller/syncer/service_test.go index 436fdb0a..e63071d3 100644 --- a/k3k-kubelet/controller/syncer/service_test.go +++ b/k3k-kubelet/controller/syncer/service_test.go @@ -42,7 +42,7 @@ var ServiceTests = func() { Namespace: namespace, }, Spec: v1alpha1.ClusterSpec{ - Sync: v1alpha1.SyncConfig{ + Sync: &v1alpha1.SyncConfig{ Services: v1alpha1.ServiceSyncConfig{ Enabled: true, }, diff --git a/pkg/apis/k3k.io/v1alpha1/types.go b/pkg/apis/k3k.io/v1alpha1/types.go index 51d9ed87..559f4b83 100644 --- a/pkg/apis/k3k.io/v1alpha1/types.go +++ b/pkg/apis/k3k.io/v1alpha1/types.go @@ -181,7 +181,7 @@ type ClusterSpec struct { // // +kubebuilder:default={} // +optional - Sync SyncConfig `json:"sync,omitempty"` + Sync *SyncConfig `json:"sync,omitempty"` } // SyncConfig will contain the resources that should be synced from virtual cluster to host cluster. @@ -204,7 +204,7 @@ type SyncConfig struct { Ingresses IngressSyncConfig `json:"ingresses,omitempty"` // PersistentVolumeClaims resources sync configuration. // - // +kubebuilder:default={"enabled": false} + // +kubebuilder:default={"enabled": true} PersistentVolumeClaims PersistentVolumeClaimSyncConfig `json:"persistentVolumeClaims,omitempty"` // PriorityClasses resources sync configuration. // @@ -597,10 +597,11 @@ type VirtualClusterPolicySpec struct { // +optional PodSecurityAdmissionLevel *PodSecurityAdmissionLevel `json:"podSecurityAdmissionLevel,omitempty"` - // Sync specifies the synchronization configuration for resources that can be applied to clusters + // Sync specifies the resources types that will be synced from virtual cluster to host cluster. // + // +kubebuilder:default={} // +optional - Sync SyncConfig `json:"sync,omitempty"` + Sync *SyncConfig `json:"sync,omitempty"` } // PodSecurityAdmissionLevel is the policy level applied to the pods in the namespace. diff --git a/pkg/apis/k3k.io/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/k3k.io/v1alpha1/zz_generated.deepcopy.go index c0c715a2..cafde3fd 100644 --- a/pkg/apis/k3k.io/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/k3k.io/v1alpha1/zz_generated.deepcopy.go @@ -164,7 +164,11 @@ func (in *ClusterSpec) DeepCopyInto(out *ClusterSpec) { } } out.CustomCAs = in.CustomCAs - in.Sync.DeepCopyInto(&out.Sync) + if in.Sync != nil { + in, out := &in.Sync, &out.Sync + *out = new(SyncConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterSpec. @@ -615,7 +619,11 @@ func (in *VirtualClusterPolicySpec) DeepCopyInto(out *VirtualClusterPolicySpec) *out = new(PodSecurityAdmissionLevel) **out = **in } - in.Sync.DeepCopyInto(&out.Sync) + if in.Sync != nil { + in, out := &in.Sync, &out.Sync + *out = new(SyncConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VirtualClusterPolicySpec. diff --git a/pkg/controller/policy/policy_test.go b/pkg/controller/policy/policy_test.go index 46a1bc95..fe2469bd 100644 --- a/pkg/controller/policy/policy_test.go +++ b/pkg/controller/policy/policy_test.go @@ -8,6 +8,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" v1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -195,8 +196,12 @@ var _ = Describe("VirtualClusterPolicy Controller", Label("controller"), Label(" // Check baseline + // get policy again + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(policy), policy) + Expect(err).To(Not(HaveOccurred())) + policy.Spec.PodSecurityAdmissionLevel = &baseline - err := k8sClient.Update(ctx, policy) + err = k8sClient.Update(ctx, policy) Expect(err).To(Not(HaveOccurred())) // wait a bit for the namespace to be updated @@ -486,8 +491,11 @@ var _ = Describe("VirtualClusterPolicy Controller", Label("controller"), Label(" WithPolling(time.Second). Should(BeNil()) + // get policy again + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(policy), policy) + Expect(err).To(Not(HaveOccurred())) policy.Spec.Quota = nil - err := k8sClient.Update(ctx, policy) + err = k8sClient.Update(ctx, policy) Expect(err).To(Not(HaveOccurred())) // wait for a bit for the resourceQuota to be deleted