From 6e509d4496ae245ba8e382e823e4e15f726917b1 Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Tue, 25 Nov 2025 19:49:23 +0000 Subject: [PATCH 1/8] make statefulset work in hub Signed-off-by: Wei Weng --- .../placement/resource_selector.go | 14 +++++ .../placement/resource_selector_test.go | 58 +++++++++++++++++++ test/e2e/enveloped_object_placement_test.go | 2 +- .../resource_placement_hub_workload_test.go | 17 +++++- test/e2e/resource_placement_rollout_test.go | 2 +- ...tatefulset.yaml => statefulset-basic.yaml} | 0 ....yaml => statefulset-invalid-storage.yaml} | 0 .../resources/statefulset-with-storage.yaml | 28 +++++++++ test/e2e/rollout_test.go | 2 +- test/e2e/utils_test.go | 29 ++++++++-- 10 files changed, 142 insertions(+), 10 deletions(-) rename test/e2e/resources/{test-statefulset.yaml => statefulset-basic.yaml} (100%) rename test/e2e/resources/{statefulset-with-volume.yaml => statefulset-invalid-storage.yaml} (100%) create mode 100644 test/e2e/resources/statefulset-with-storage.yaml diff --git a/pkg/controllers/placement/resource_selector.go b/pkg/controllers/placement/resource_selector.go index 51a4d995a..3de97efa2 100644 --- a/pkg/controllers/placement/resource_selector.go +++ b/pkg/controllers/placement/resource_selector.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + pvutil "k8s.io/component-helpers/storage/volume" "k8s.io/klog/v2" "k8s.io/kubectl/pkg/util/deployment" "sigs.k8s.io/controller-runtime/pkg/client" @@ -444,6 +445,15 @@ func generateRawContent(object *unstructured.Unstructured) ([]byte, error) { delete(annots, corev1.LastAppliedConfigAnnotation) // Remove the revision annotation set by deployment controller. delete(annots, deployment.RevisionAnnotation) + // Remove node-specific annotations from PVCs that would break when propagated to member clusters + // These annotations reference specific nodes from the hub cluster which don't exist on member clusters + // The member cluster's storage provisioner will set appropriate values for its own nodes + // All annotations below are listed in well-known labels, annotations and taints document: + // https://kubernetes.io/docs/reference/labels-annotations-taints/ + delete(annots, pvutil.AnnSelectedNode) // Node selected for volume binding + delete(annots, pvutil.AnnBindCompleted) // Binding completion status + delete(annots, pvutil.AnnBoundByController) // Controller binding status + delete(annots, pvutil.AnnBetaStorageProvisioner) // Beta storage provisioner annotation if len(annots) == 0 { object.SetAnnotations(nil) } else { @@ -491,6 +501,10 @@ func generateRawContent(object *unstructured.Unstructured) ([]byte, error) { unstructured.RemoveNestedField(object.Object, "spec", "template", "metadata", "labels", "controller-uid") unstructured.RemoveNestedField(object.Object, "spec", "template", "metadata", "labels", "batch.kubernetes.io/controller-uid") } + } else if object.GetKind() == "PersistentVolumeClaim" && object.GetAPIVersion() == "v1" { + // Remove volumeName which references a specific PV from the hub cluster that won't exist on member clusters. + // The member cluster's storage provisioner will create and bind a new PV. + unstructured.RemoveNestedField(object.Object, "spec", "volumeName") } rawContent, err := object.MarshalJSON() diff --git a/pkg/controllers/placement/resource_selector_test.go b/pkg/controllers/placement/resource_selector_test.go index 29d62f426..5149de64e 100644 --- a/pkg/controllers/placement/resource_selector_test.go +++ b/pkg/controllers/placement/resource_selector_test.go @@ -27,12 +27,14 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" utilrand "k8s.io/apimachinery/pkg/util/rand" + pvutil "k8s.io/component-helpers/storage/volume" "k8s.io/kubectl/pkg/util/deployment" "k8s.io/utils/ptr" @@ -243,6 +245,62 @@ func TestGenerateResourceContent(t *testing.T) { }, }, }, + "PersistentVolumeClaim with node-specific annotations": { + resource: corev1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "PersistentVolumeClaim", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-namespace", + Annotations: map[string]string{ + pvutil.AnnSelectedNode: "hub-control-plane", + pvutil.AnnBindCompleted: "yes", + pvutil.AnnBoundByController: "yes", + pvutil.AnnBetaStorageProvisioner: "kubernetes.io/no-provisioner", + "custom-annotation": "should-remain", + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }, + }, + StorageClassName: ptr.To("standard"), + VolumeName: "pvc-12345-from-hub-cluster", + }, + }, + wantResource: corev1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "PersistentVolumeClaim", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-namespace", + Annotations: map[string]string{ + "custom-annotation": "should-remain", + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }, + }, + StorageClassName: ptr.To("standard"), + // VolumeName should be removed + }, + }, + }, } for testName, tt := range tests { diff --git a/test/e2e/enveloped_object_placement_test.go b/test/e2e/enveloped_object_placement_test.go index b1923b281..3ea46b66b 100644 --- a/test/e2e/enveloped_object_placement_test.go +++ b/test/e2e/enveloped_object_placement_test.go @@ -226,7 +226,7 @@ var _ = Describe("placing wrapped resources using a CRP", func() { // read the test resources. readDeploymentTestManifest(&testDeployment) readDaemonSetTestManifest(&testDaemonSet) - readStatefulSetTestManifest(&testStatefulSet, true) + readStatefulSetTestManifest(&testStatefulSet, StatefulSetWithStorage) readEnvelopeResourceTestManifest(&testResourceEnvelope) }) diff --git a/test/e2e/resource_placement_hub_workload_test.go b/test/e2e/resource_placement_hub_workload_test.go index c248319d0..be3016a5a 100644 --- a/test/e2e/resource_placement_hub_workload_test.go +++ b/test/e2e/resource_placement_hub_workload_test.go @@ -43,6 +43,7 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res readDeploymentTestManifest(&testDeployment) readDaemonSetTestManifest(&testDaemonSet) readJobTestManifest(&testJob) + readStatefulSetTestManifest(&testStatefulSet, StatefulSetWithStorage) workNamespace := appNamespace() // Create namespace and workloads @@ -105,9 +106,23 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res Name: testJob.Name, Namespace: workNamespace.Name, }, + // PVCs created by StatefulSet controller from volumeClaimTemplates + // Kubernetes StatefulSet controller uses naming convention: -- + { + Version: "v1", + Kind: "PersistentVolumeClaim", + Name: fmt.Sprintf("%s-%s-%d", testStatefulSet.Spec.VolumeClaimTemplates[0].Name, testStatefulSet.Name, 0), + Namespace: workNamespace.Name, + }, + { + Version: "v1", + Kind: "PersistentVolumeClaim", + Name: fmt.Sprintf("%s-%s-%d", testStatefulSet.Spec.VolumeClaimTemplates[0].Name, testStatefulSet.Name, 1), + Namespace: workNamespace.Name, + }, } // Use customizedPlacementStatusUpdatedActual with resourceIsTrackable=false - // because Jobs don't have availability tracking like Deployments/DaemonSets do + // because Jobs and PVCs don't have availability tracking like Deployments/DaemonSets do crpKey := types.NamespacedName{Name: crpName} crpStatusUpdatedActual := customizedPlacementStatusUpdatedActual(crpKey, wantSelectedResources, allMemberClusterNames, nil, "0", false) Eventually(crpStatusUpdatedActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected") diff --git a/test/e2e/resource_placement_rollout_test.go b/test/e2e/resource_placement_rollout_test.go index 6548a2a15..d780e2d12 100644 --- a/test/e2e/resource_placement_rollout_test.go +++ b/test/e2e/resource_placement_rollout_test.go @@ -69,7 +69,7 @@ var _ = Describe("placing namespaced scoped resources using a RP with rollout", testDaemonSet = appv1.DaemonSet{} readDaemonSetTestManifest(&testDaemonSet) testStatefulSet = appv1.StatefulSet{} - readStatefulSetTestManifest(&testStatefulSet, false) + readStatefulSetTestManifest(&testStatefulSet, StatefulSetBasic) testService = corev1.Service{} readServiceTestManifest(&testService) testJob = batchv1.Job{} diff --git a/test/e2e/resources/test-statefulset.yaml b/test/e2e/resources/statefulset-basic.yaml similarity index 100% rename from test/e2e/resources/test-statefulset.yaml rename to test/e2e/resources/statefulset-basic.yaml diff --git a/test/e2e/resources/statefulset-with-volume.yaml b/test/e2e/resources/statefulset-invalid-storage.yaml similarity index 100% rename from test/e2e/resources/statefulset-with-volume.yaml rename to test/e2e/resources/statefulset-invalid-storage.yaml diff --git a/test/e2e/resources/statefulset-with-storage.yaml b/test/e2e/resources/statefulset-with-storage.yaml new file mode 100644 index 000000000..a9df178e5 --- /dev/null +++ b/test/e2e/resources/statefulset-with-storage.yaml @@ -0,0 +1,28 @@ +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: test-ss +spec: + selector: + matchLabels: + app: test-ss + serviceName: "test-ss-svc" + replicas: 2 + template: + metadata: + labels: + app: test-ss + spec: + terminationGracePeriodSeconds: 10 + containers: + - name: pause + image: k8s.gcr.io/pause:3.8 + volumeClaimTemplates: + - metadata: + name: test-ss-pvc + spec: + accessModes: [ "ReadWriteOnce" ] + storageClassName: "standard" + resources: + requests: + storage: 100Mi diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go index 1ff978366..5abfb684f 100644 --- a/test/e2e/rollout_test.go +++ b/test/e2e/rollout_test.go @@ -342,7 +342,7 @@ var _ = Describe("placing wrapped resources using a CRP", Ordered, func() { BeforeAll(func() { // Create the test resources. - readStatefulSetTestManifest(&testStatefulSet, false) + readStatefulSetTestManifest(&testStatefulSet, StatefulSetBasic) readEnvelopeResourceTestManifest(&testStatefulSetEnvelope) wantSelectedResources = []placementv1beta1.ResourceIdentifier{ { diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 36df25cf1..b847ceb91 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -57,6 +57,18 @@ import ( "github.com/kubefleet-dev/kubefleet/test/e2e/framework" ) +// StatefulSetVariant represents different StatefulSet configurations for testing +type StatefulSetVariant int + +const ( + // StatefulSetBasic is a StatefulSet without any persistent volume claims + StatefulSetBasic StatefulSetVariant = iota + // StatefulSetInvalidStorage is a StatefulSet with a non-existent storage class + StatefulSetInvalidStorage + // StatefulSetWithStorage is a StatefulSet with a valid standard storage class + StatefulSetWithStorage +) + var ( croTestAnnotationKey = "cro-test-annotation" croTestAnnotationValue = "cro-test-annotation-val" @@ -1537,13 +1549,18 @@ func readDaemonSetTestManifest(testDaemonSet *appsv1.DaemonSet) { Expect(err).Should(Succeed()) } -func readStatefulSetTestManifest(testStatefulSet *appsv1.StatefulSet, withVolume bool) { +func readStatefulSetTestManifest(testStatefulSet *appsv1.StatefulSet, variant StatefulSetVariant) { By("Read the statefulSet resource") - if withVolume { - Expect(utils.GetObjectFromManifest("resources/statefulset-with-volume.yaml", testStatefulSet)).Should(Succeed()) - } else { - Expect(utils.GetObjectFromManifest("resources/test-statefulset.yaml", testStatefulSet)).Should(Succeed()) - } + var manifestPath string + switch variant { + case StatefulSetBasic: + manifestPath = "resources/statefulset-basic.yaml" + case StatefulSetInvalidStorage: + manifestPath = "resources/statefulset-invalid-storage.yaml" + case StatefulSetWithStorage: + manifestPath = "resources/statefulset-with-storage.yaml" + } + Expect(utils.GetObjectFromManifest(manifestPath, testStatefulSet)).Should(Succeed()) } func readServiceTestManifest(testService *corev1.Service) { From 676df2ac78755c6cc16f314fc9bb07fd73229cf1 Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Tue, 25 Nov 2025 19:59:31 +0000 Subject: [PATCH 2/8] fix test Signed-off-by: Wei Weng --- .../resource_placement_hub_workload_test.go | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/test/e2e/resource_placement_hub_workload_test.go b/test/e2e/resource_placement_hub_workload_test.go index be3016a5a..a1c58a155 100644 --- a/test/e2e/resource_placement_hub_workload_test.go +++ b/test/e2e/resource_placement_hub_workload_test.go @@ -37,6 +37,7 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res var testDeployment appsv1.Deployment var testDaemonSet appsv1.DaemonSet var testJob batchv1.Job + var testStatefulSet appsv1.StatefulSet BeforeAll(func() { // Read the test manifests @@ -52,9 +53,11 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res testDeployment.Namespace = workNamespace.Name testDaemonSet.Namespace = workNamespace.Name testJob.Namespace = workNamespace.Name + testStatefulSet.Namespace = workNamespace.Name Expect(hubClient.Create(ctx, &testDeployment)).To(Succeed(), "Failed to create test deployment %s", testDeployment.Name) Expect(hubClient.Create(ctx, &testDaemonSet)).To(Succeed(), "Failed to create test daemonset %s", testDaemonSet.Name) Expect(hubClient.Create(ctx, &testJob)).To(Succeed(), "Failed to create test job %s", testJob.Name) + Expect(hubClient.Create(ctx, &testStatefulSet)).To(Succeed(), "Failed to create test statefulset %s", testStatefulSet.Name) // Create the CRP that selects the namespace By("creating CRP that selects the namespace") @@ -106,6 +109,13 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res Name: testJob.Name, Namespace: workNamespace.Name, }, + { + Group: "apps", + Version: "v1", + Kind: "StatefulSet", + Name: testStatefulSet.Name, + Namespace: workNamespace.Name, + }, // PVCs created by StatefulSet controller from volumeClaimTemplates // Kubernetes StatefulSet controller uses naming convention: -- { @@ -185,6 +195,13 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res "Hub job should complete successfully") }) + It("should verify hub statefulset is ready", func() { + By("checking hub statefulset status") + statefulSetReadyActual := waitForStatefulSetToBeReady(hubClient, &testStatefulSet) + Eventually(statefulSetReadyActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), + "Hub statefulset should be ready before placement") + }) + It("should place the deployment on all member clusters", func() { By("verifying deployment is placed and ready on all member clusters") for idx := range allMemberClusters { @@ -221,6 +238,24 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res } }) + It("should place the statefulset on all member clusters", func() { + By("verifying statefulset is placed and ready on all member clusters") + for idx := range allMemberClusters { + memberCluster := allMemberClusters[idx] + statefulsetPlacedActual := waitForStatefulSetPlacementToReady(memberCluster, &testStatefulSet) + Eventually(statefulsetPlacedActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place statefulset on member cluster %s", memberCluster.ClusterName) + } + }) + + It("should verify statefulset replicas are ready on all clusters", func() { + By("checking statefulset status on each cluster") + for _, cluster := range allMemberClusters { + statefulSetReadyActual := waitForStatefulSetToBeReady(cluster.KubeClient, &testStatefulSet) + Eventually(statefulSetReadyActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), + "StatefulSet should be ready on cluster %s", cluster.ClusterName) + } + }) + It("should verify deployment replicas are ready on all clusters", func() { By("checking deployment status on each cluster") for _, cluster := range allMemberClusters { @@ -247,6 +282,46 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res }) }) +func waitForStatefulSetToBeReady(kubeClient client.Client, testStatefulSet *appsv1.StatefulSet) func() error { + return func() error { + var statefulSet appsv1.StatefulSet + if err := kubeClient.Get(ctx, types.NamespacedName{ + Name: testStatefulSet.Name, + Namespace: testStatefulSet.Namespace, + }, &statefulSet); err != nil { + return err + } + + // Verify statefulset is ready + if statefulSet.Status.ObservedGeneration != statefulSet.Generation { + return fmt.Errorf("statefulset has stale status: observed generation %d != generation %d", + statefulSet.Status.ObservedGeneration, statefulSet.Generation) + } + + requiredReplicas := int32(1) + if statefulSet.Spec.Replicas != nil { + requiredReplicas = *statefulSet.Spec.Replicas + } + + if statefulSet.Status.CurrentReplicas != requiredReplicas { + return fmt.Errorf("statefulset not ready: %d/%d current replicas", + statefulSet.Status.CurrentReplicas, requiredReplicas) + } + + if statefulSet.Status.UpdatedReplicas != requiredReplicas { + return fmt.Errorf("statefulset not updated: %d/%d updated replicas", + statefulSet.Status.UpdatedReplicas, requiredReplicas) + } + + if statefulSet.Status.CurrentReplicas != statefulSet.Status.UpdatedReplicas { + return fmt.Errorf("statefulset replicas not synchronized: %d current != %d updated", + statefulSet.Status.CurrentReplicas, statefulSet.Status.UpdatedReplicas) + } + + return nil + } +} + func waitForJobToComplete(kubeClient client.Client, testJob *batchv1.Job) func() error { return func() error { var job batchv1.Job From 3e06c8d76a8643addbdb4bfd9bcb9180a2aba813 Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Tue, 25 Nov 2025 20:22:37 +0000 Subject: [PATCH 3/8] remove unintended change Signed-off-by: Wei Weng --- test/e2e/enveloped_object_placement_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/enveloped_object_placement_test.go b/test/e2e/enveloped_object_placement_test.go index 3ea46b66b..b52451958 100644 --- a/test/e2e/enveloped_object_placement_test.go +++ b/test/e2e/enveloped_object_placement_test.go @@ -226,7 +226,7 @@ var _ = Describe("placing wrapped resources using a CRP", func() { // read the test resources. readDeploymentTestManifest(&testDeployment) readDaemonSetTestManifest(&testDaemonSet) - readStatefulSetTestManifest(&testStatefulSet, StatefulSetWithStorage) + readStatefulSetTestManifest(&testStatefulSet, StatefulSetInvalidStorage) readEnvelopeResourceTestManifest(&testResourceEnvelope) }) From 1befc44ae988b983d312c67aa46abe037a34651b Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Wed, 26 Nov 2025 02:14:13 +0000 Subject: [PATCH 4/8] remove more annotations Signed-off-by: Wei Weng --- pkg/controllers/placement/resource_selector.go | 9 ++++++--- pkg/controllers/placement/resource_selector_test.go | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/controllers/placement/resource_selector.go b/pkg/controllers/placement/resource_selector.go index 3de97efa2..100ff5961 100644 --- a/pkg/controllers/placement/resource_selector.go +++ b/pkg/controllers/placement/resource_selector.go @@ -445,15 +445,18 @@ func generateRawContent(object *unstructured.Unstructured) ([]byte, error) { delete(annots, corev1.LastAppliedConfigAnnotation) // Remove the revision annotation set by deployment controller. delete(annots, deployment.RevisionAnnotation) - // Remove node-specific annotations from PVCs that would break when propagated to member clusters - // These annotations reference specific nodes from the hub cluster which don't exist on member clusters - // The member cluster's storage provisioner will set appropriate values for its own nodes + // Remove node-specific and provisioning-related annotations from PVCs that would break when propagated to member clusters + // These annotations reference specific nodes/provisioners from the hub cluster which don't exist on member clusters + // The member cluster's storage provisioner will set appropriate values for its own environment // All annotations below are listed in well-known labels, annotations and taints document: // https://kubernetes.io/docs/reference/labels-annotations-taints/ delete(annots, pvutil.AnnSelectedNode) // Node selected for volume binding delete(annots, pvutil.AnnBindCompleted) // Binding completion status delete(annots, pvutil.AnnBoundByController) // Controller binding status + delete(annots, pvutil.AnnStorageProvisioner) // Storage provisioner annotation delete(annots, pvutil.AnnBetaStorageProvisioner) // Beta storage provisioner annotation + delete(annots, pvutil.AnnDynamicallyProvisioned) // Dynamically provisioned by annotation + delete(annots, pvutil.AnnMigratedTo) // CSI migration annotation if len(annots) == 0 { object.SetAnnotations(nil) } else { diff --git a/pkg/controllers/placement/resource_selector_test.go b/pkg/controllers/placement/resource_selector_test.go index 5149de64e..5bd354e7e 100644 --- a/pkg/controllers/placement/resource_selector_test.go +++ b/pkg/controllers/placement/resource_selector_test.go @@ -258,7 +258,10 @@ func TestGenerateResourceContent(t *testing.T) { pvutil.AnnSelectedNode: "hub-control-plane", pvutil.AnnBindCompleted: "yes", pvutil.AnnBoundByController: "yes", + pvutil.AnnStorageProvisioner: "kubernetes.io/aws-ebs", pvutil.AnnBetaStorageProvisioner: "kubernetes.io/no-provisioner", + pvutil.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", + pvutil.AnnMigratedTo: "ebs.csi.aws.com", "custom-annotation": "should-remain", }, }, From 59143129b419fa247ee91c1e828b86de116c695a Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Wed, 26 Nov 2025 02:31:18 +0000 Subject: [PATCH 5/8] track pvc availability Signed-off-by: Wei Weng --- .../workapplier/availability_tracker.go | 24 ++++ .../workapplier/availability_tracker_test.go | 134 ++++++++++++++++++ pkg/utils/common.go | 6 + 3 files changed, 164 insertions(+) diff --git a/pkg/controllers/workapplier/availability_tracker.go b/pkg/controllers/workapplier/availability_tracker.go index 021c7babd..80f161e4d 100644 --- a/pkg/controllers/workapplier/availability_tracker.go +++ b/pkg/controllers/workapplier/availability_tracker.go @@ -104,6 +104,8 @@ func trackInMemberClusterObjAvailabilityByGVR( return trackCRDAvailability(inMemberClusterObj) case utils.PodDisruptionBudgetGVR: return trackPDBAvailability(inMemberClusterObj) + case utils.PersistentVolumeClaimGVR: + return trackPVCAvailability(inMemberClusterObj) default: if isDataResource(*gvr) { klog.V(2).InfoS("The object from the member cluster is a data object, consider it to be immediately available", @@ -269,6 +271,28 @@ func trackPDBAvailability(curObj *unstructured.Unstructured) (ManifestProcessing return AvailabilityResultTypeNotYetAvailable, nil } +// trackPVCAvailability tracks the availability of a persistent volume claim in the member cluster. +func trackPVCAvailability(inMemberClusterObj *unstructured.Unstructured) (ManifestProcessingAvailabilityResultType, error) { + var pvc corev1.PersistentVolumeClaim + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(inMemberClusterObj.Object, &pvc); err != nil { + wrappedErr := fmt.Errorf("failed to convert the unstructured object to a persistent volume claim: %w", err) + _ = controller.NewUnexpectedBehaviorError(wrappedErr) + return AvailabilityResultTypeFailed, wrappedErr + } + + // Check if the PVC is bound. + // A PVC is considered available when it's in the Bound phase, meaning it has been + // successfully bound to a PersistentVolume and is ready to be used by pods. + if pvc.Status.Phase == corev1.ClaimBound { + klog.V(2).InfoS("PersistentVolumeClaim is available", "pvc", klog.KObj(inMemberClusterObj)) + return AvailabilityResultTypeAvailable, nil + } + + klog.V(2).InfoS("PersistentVolumeClaim is not ready yet, will check later to see if it becomes available", + "pvc", klog.KObj(inMemberClusterObj), "phase", pvc.Status.Phase) + return AvailabilityResultTypeNotYetAvailable, nil +} + // isDataResource checks if the resource is a data resource; such resources are // available immediately after creation. func isDataResource(gvr schema.GroupVersionResource) bool { diff --git a/pkg/controllers/workapplier/availability_tracker_test.go b/pkg/controllers/workapplier/availability_tracker_test.go index 03a212f83..ac4b9c849 100644 --- a/pkg/controllers/workapplier/availability_tracker_test.go +++ b/pkg/controllers/workapplier/availability_tracker_test.go @@ -802,6 +802,100 @@ func TestTrackPDBAvailability(t *testing.T) { } } +// TestTrackPVCAvailability tests the trackPVCAvailability function. +func TestTrackPVCAvailability(t *testing.T) { + boundPVC := &corev1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "PersistentVolumeClaim", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc-bound", + Namespace: nsName, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + + pendingPVC := &corev1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "PersistentVolumeClaim", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc-pending", + Namespace: nsName, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimPending, + }, + } + + lostPVC := &corev1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "PersistentVolumeClaim", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc-lost", + Namespace: nsName, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimLost, + }, + } + + testCases := []struct { + name string + pvc *corev1.PersistentVolumeClaim + wantAvailabilityResultType ManifestProcessingAvailabilityResultType + }{ + { + name: "available PVC (bound)", + pvc: boundPVC, + wantAvailabilityResultType: AvailabilityResultTypeAvailable, + }, + { + name: "unavailable PVC (pending)", + pvc: pendingPVC, + wantAvailabilityResultType: AvailabilityResultTypeNotYetAvailable, + }, + { + name: "unavailable PVC (lost)", + pvc: lostPVC, + wantAvailabilityResultType: AvailabilityResultTypeNotYetAvailable, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gotResTyp, err := trackPVCAvailability(toUnstructured(t, tc.pvc)) + if err != nil { + t.Fatalf("trackPVCAvailability() = %v, want no error", err) + } + if gotResTyp != tc.wantAvailabilityResultType { + t.Errorf("manifestProcessingAvailabilityResultType = %v, want %v", gotResTyp, tc.wantAvailabilityResultType) + } + }) + } +} + // TestTrackInMemberClusterObjAvailabilityByGVR tests the trackInMemberClusterObjAvailabilityByGVR function. func TestTrackInMemberClusterObjAvailabilityByGVR(t *testing.T) { availableDeploy := deploy.DeepCopy() @@ -875,6 +969,34 @@ func TestTrackInMemberClusterObjAvailabilityByGVR(t *testing.T) { }, } + availablePVC := &corev1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "PersistentVolumeClaim", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: nsName, + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + } + + unavailablePVC := &corev1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "PersistentVolumeClaim", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc-pending", + Namespace: nsName, + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimPending, + }, + } + testCases := []struct { name string gvr schema.GroupVersionResource @@ -995,6 +1117,18 @@ func TestTrackInMemberClusterObjAvailabilityByGVR(t *testing.T) { inMemberClusterObj: toUnstructured(t, &schedulingv1.PriorityClass{}), wantAvailabilityResultType: AvailabilityResultTypeAvailable, }, + { + name: "available persistent volume claim (bound)", + gvr: utils.PersistentVolumeClaimGVR, + inMemberClusterObj: toUnstructured(t, availablePVC), + wantAvailabilityResultType: AvailabilityResultTypeAvailable, + }, + { + name: "unavailable persistent volume claim (pending)", + gvr: utils.PersistentVolumeClaimGVR, + inMemberClusterObj: toUnstructured(t, unavailablePVC), + wantAvailabilityResultType: AvailabilityResultTypeNotYetAvailable, + }, } for _, tc := range testCases { diff --git a/pkg/utils/common.go b/pkg/utils/common.go index cf1899182..6c6faf8d1 100644 --- a/pkg/utils/common.go +++ b/pkg/utils/common.go @@ -438,6 +438,12 @@ var ( Resource: "clusterrolebindings", } + PersistentVolumeClaimGVR = schema.GroupVersionResource{ + Group: corev1.GroupName, + Version: corev1.SchemeGroupVersion.Version, + Resource: "persistentvolumeclaims", + } + PersistentVolumeClaimGVK = schema.GroupVersionKind{ Group: corev1.GroupName, Version: corev1.SchemeGroupVersion.Version, From b3edb75bf019f670088a6d4f163c4a715d4bb441 Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Wed, 26 Nov 2025 02:31:56 +0000 Subject: [PATCH 6/8] use cmp.diff to compare status Signed-off-by: Wei Weng --- .../resource_placement_hub_workload_test.go | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/test/e2e/resource_placement_hub_workload_test.go b/test/e2e/resource_placement_hub_workload_test.go index a1c58a155..cd30812cf 100644 --- a/test/e2e/resource_placement_hub_workload_test.go +++ b/test/e2e/resource_placement_hub_workload_test.go @@ -19,6 +19,7 @@ package e2e import ( "fmt" + "github.com/google/go-cmp/cmp" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" @@ -132,7 +133,7 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res }, } // Use customizedPlacementStatusUpdatedActual with resourceIsTrackable=false - // because Jobs and PVCs don't have availability tracking like Deployments/DaemonSets do + // because Jobs don't have availability tracking like Deployments/DaemonSets/StatefulSets/PVCs do crpKey := types.NamespacedName{Name: crpName} crpStatusUpdatedActual := customizedPlacementStatusUpdatedActual(crpKey, wantSelectedResources, allMemberClusterNames, nil, "0", false) Eventually(crpStatusUpdatedActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected") @@ -293,29 +294,25 @@ func waitForStatefulSetToBeReady(kubeClient client.Client, testStatefulSet *apps } // Verify statefulset is ready - if statefulSet.Status.ObservedGeneration != statefulSet.Generation { - return fmt.Errorf("statefulset has stale status: observed generation %d != generation %d", - statefulSet.Status.ObservedGeneration, statefulSet.Generation) - } - requiredReplicas := int32(1) if statefulSet.Spec.Replicas != nil { requiredReplicas = *statefulSet.Spec.Replicas } - if statefulSet.Status.CurrentReplicas != requiredReplicas { - return fmt.Errorf("statefulset not ready: %d/%d current replicas", - statefulSet.Status.CurrentReplicas, requiredReplicas) + wantStatus := appsv1.StatefulSetStatus{ + ObservedGeneration: statefulSet.Generation, + CurrentReplicas: requiredReplicas, + UpdatedReplicas: requiredReplicas, } - if statefulSet.Status.UpdatedReplicas != requiredReplicas { - return fmt.Errorf("statefulset not updated: %d/%d updated replicas", - statefulSet.Status.UpdatedReplicas, requiredReplicas) + gotStatus := appsv1.StatefulSetStatus{ + ObservedGeneration: statefulSet.Status.ObservedGeneration, + CurrentReplicas: statefulSet.Status.CurrentReplicas, + UpdatedReplicas: statefulSet.Status.UpdatedReplicas, } - if statefulSet.Status.CurrentReplicas != statefulSet.Status.UpdatedReplicas { - return fmt.Errorf("statefulset replicas not synchronized: %d current != %d updated", - statefulSet.Status.CurrentReplicas, statefulSet.Status.UpdatedReplicas) + if diff := cmp.Diff(wantStatus, gotStatus); diff != "" { + return fmt.Errorf("statefulset not ready (-want +got):\n%s", diff) } return nil From 540fe6f0a29c0852802c2455364931d6051ad94e Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Mon, 1 Dec 2025 15:56:37 +0000 Subject: [PATCH 7/8] do not propagate PVCs Signed-off-by: Wei Weng --- .../workapplier/availability_tracker.go | 24 ---- .../workapplier/availability_tracker_test.go | 134 ------------------ pkg/utils/common.go | 9 +- pkg/utils/common_test.go | 13 ++ .../e2e/placement_selecting_resources_test.go | 2 +- .../resource_placement_hub_workload_test.go | 16 +-- ...urce_placement_selecting_resources_test.go | 2 +- 7 files changed, 19 insertions(+), 181 deletions(-) diff --git a/pkg/controllers/workapplier/availability_tracker.go b/pkg/controllers/workapplier/availability_tracker.go index 1d8c92f17..725ca3a9e 100644 --- a/pkg/controllers/workapplier/availability_tracker.go +++ b/pkg/controllers/workapplier/availability_tracker.go @@ -115,8 +115,6 @@ func trackInMemberClusterObjAvailabilityByGVR( return trackCRDAvailability(inMemberClusterObj) case utils.PodDisruptionBudgetGVR: return trackPDBAvailability(inMemberClusterObj) - case utils.PersistentVolumeClaimGVR: - return trackPVCAvailability(inMemberClusterObj) default: if isDataResource(*gvr) { klog.V(2).InfoS("The object from the member cluster is a data object, consider it to be immediately available", @@ -282,28 +280,6 @@ func trackPDBAvailability(curObj *unstructured.Unstructured) (ManifestProcessing return AvailabilityResultTypeNotYetAvailable, nil } -// trackPVCAvailability tracks the availability of a persistent volume claim in the member cluster. -func trackPVCAvailability(inMemberClusterObj *unstructured.Unstructured) (ManifestProcessingAvailabilityResultType, error) { - var pvc corev1.PersistentVolumeClaim - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(inMemberClusterObj.Object, &pvc); err != nil { - wrappedErr := fmt.Errorf("failed to convert the unstructured object to a persistent volume claim: %w", err) - _ = controller.NewUnexpectedBehaviorError(wrappedErr) - return AvailabilityResultTypeFailed, wrappedErr - } - - // Check if the PVC is bound. - // A PVC is considered available when it's in the Bound phase, meaning it has been - // successfully bound to a PersistentVolume and is ready to be used by pods. - if pvc.Status.Phase == corev1.ClaimBound { - klog.V(2).InfoS("PersistentVolumeClaim is available", "pvc", klog.KObj(inMemberClusterObj)) - return AvailabilityResultTypeAvailable, nil - } - - klog.V(2).InfoS("PersistentVolumeClaim is not ready yet, will check later to see if it becomes available", - "pvc", klog.KObj(inMemberClusterObj), "phase", pvc.Status.Phase) - return AvailabilityResultTypeNotYetAvailable, nil -} - // isDataResource checks if the resource is a data resource; such resources are // available immediately after creation. func isDataResource(gvr schema.GroupVersionResource) bool { diff --git a/pkg/controllers/workapplier/availability_tracker_test.go b/pkg/controllers/workapplier/availability_tracker_test.go index 6977b4cc7..1bc43c328 100644 --- a/pkg/controllers/workapplier/availability_tracker_test.go +++ b/pkg/controllers/workapplier/availability_tracker_test.go @@ -802,100 +802,6 @@ func TestTrackPDBAvailability(t *testing.T) { } } -// TestTrackPVCAvailability tests the trackPVCAvailability function. -func TestTrackPVCAvailability(t *testing.T) { - boundPVC := &corev1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "PersistentVolumeClaim", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc-bound", - Namespace: nsName, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{ - corev1.ReadWriteOnce, - }, - }, - Status: corev1.PersistentVolumeClaimStatus{ - Phase: corev1.ClaimBound, - }, - } - - pendingPVC := &corev1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "PersistentVolumeClaim", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc-pending", - Namespace: nsName, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{ - corev1.ReadWriteOnce, - }, - }, - Status: corev1.PersistentVolumeClaimStatus{ - Phase: corev1.ClaimPending, - }, - } - - lostPVC := &corev1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "PersistentVolumeClaim", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc-lost", - Namespace: nsName, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{ - corev1.ReadWriteOnce, - }, - }, - Status: corev1.PersistentVolumeClaimStatus{ - Phase: corev1.ClaimLost, - }, - } - - testCases := []struct { - name string - pvc *corev1.PersistentVolumeClaim - wantAvailabilityResultType ManifestProcessingAvailabilityResultType - }{ - { - name: "available PVC (bound)", - pvc: boundPVC, - wantAvailabilityResultType: AvailabilityResultTypeAvailable, - }, - { - name: "unavailable PVC (pending)", - pvc: pendingPVC, - wantAvailabilityResultType: AvailabilityResultTypeNotYetAvailable, - }, - { - name: "unavailable PVC (lost)", - pvc: lostPVC, - wantAvailabilityResultType: AvailabilityResultTypeNotYetAvailable, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - gotResTyp, err := trackPVCAvailability(toUnstructured(t, tc.pvc)) - if err != nil { - t.Fatalf("trackPVCAvailability() = %v, want no error", err) - } - if gotResTyp != tc.wantAvailabilityResultType { - t.Errorf("manifestProcessingAvailabilityResultType = %v, want %v", gotResTyp, tc.wantAvailabilityResultType) - } - }) - } -} - // TestTrackInMemberClusterObjAvailabilityByGVR tests the trackInMemberClusterObjAvailabilityByGVR function. func TestTrackInMemberClusterObjAvailabilityByGVR(t *testing.T) { availableDeploy := deploy.DeepCopy() @@ -969,34 +875,6 @@ func TestTrackInMemberClusterObjAvailabilityByGVR(t *testing.T) { }, } - availablePVC := &corev1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "PersistentVolumeClaim", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc", - Namespace: nsName, - }, - Status: corev1.PersistentVolumeClaimStatus{ - Phase: corev1.ClaimBound, - }, - } - - unavailablePVC := &corev1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "PersistentVolumeClaim", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc-pending", - Namespace: nsName, - }, - Status: corev1.PersistentVolumeClaimStatus{ - Phase: corev1.ClaimPending, - }, - } - testCases := []struct { name string gvr schema.GroupVersionResource @@ -1117,18 +995,6 @@ func TestTrackInMemberClusterObjAvailabilityByGVR(t *testing.T) { inMemberClusterObj: toUnstructured(t, &schedulingv1.PriorityClass{}), wantAvailabilityResultType: AvailabilityResultTypeAvailable, }, - { - name: "available persistent volume claim (bound)", - gvr: utils.PersistentVolumeClaimGVR, - inMemberClusterObj: toUnstructured(t, availablePVC), - wantAvailabilityResultType: AvailabilityResultTypeAvailable, - }, - { - name: "unavailable persistent volume claim (pending)", - gvr: utils.PersistentVolumeClaimGVR, - inMemberClusterObj: toUnstructured(t, unavailablePVC), - wantAvailabilityResultType: AvailabilityResultTypeNotYetAvailable, - }, } for _, tc := range testCases { diff --git a/pkg/utils/common.go b/pkg/utils/common.go index 6c6faf8d1..63225001b 100644 --- a/pkg/utils/common.go +++ b/pkg/utils/common.go @@ -438,12 +438,6 @@ var ( Resource: "clusterrolebindings", } - PersistentVolumeClaimGVR = schema.GroupVersionResource{ - Group: corev1.GroupName, - Version: corev1.SchemeGroupVersion.Version, - Resource: "persistentvolumeclaims", - } - PersistentVolumeClaimGVK = schema.GroupVersionKind{ Group: corev1.GroupName, Version: corev1.SchemeGroupVersion.Version, @@ -547,6 +541,9 @@ func ShouldPropagateObj(informerManager informer.Manager, uObj *unstructured.Uns if secret.Type == corev1.SecretTypeServiceAccountToken { return false, nil } + case corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"): + // Skip PersistentVolumeClaims to avoid conflicts with the PVCs created by statefulset controller + return false, nil case corev1.SchemeGroupVersion.WithKind("Endpoints"): // we assume that all endpoints with the same name of a service is created by the service controller if _, err := informerManager.Lister(ServiceGVR).ByNamespace(uObj.GetNamespace()).Get(uObj.GetName()); err != nil { diff --git a/pkg/utils/common_test.go b/pkg/utils/common_test.go index 35716885f..e6f9433d3 100644 --- a/pkg/utils/common_test.go +++ b/pkg/utils/common_test.go @@ -1317,6 +1317,19 @@ func TestShouldPropagateObj_PodAndReplicaSet(t *testing.T) { ownerReferences: nil, want: true, }, + { + name: "PersistentVolumeClaim should NOT propagate", + obj: map[string]interface{}{ + "apiVersion": "v1", + "kind": "PersistentVolumeClaim", + "metadata": map[string]interface{}{ + "name": "test-pvc", + "namespace": "default", + }, + }, + ownerReferences: nil, + want: false, + }, } for _, tt := range tests { diff --git a/test/e2e/placement_selecting_resources_test.go b/test/e2e/placement_selecting_resources_test.go index 601173394..e060e6920 100644 --- a/test/e2e/placement_selecting_resources_test.go +++ b/test/e2e/placement_selecting_resources_test.go @@ -1392,11 +1392,11 @@ var _ = Describe("creating CRP and checking selected resources order", Ordered, It("should update CRP status with the correct order of the selected resources", func() { // Define the expected resources in order + // Note: PVCs are not propagated, so they should not appear in selected resources expectedResources := []placementv1beta1.ResourceIdentifier{ {Kind: "Namespace", Name: nsName, Version: "v1"}, {Kind: "Secret", Name: secret.Name, Namespace: nsName, Version: "v1"}, {Kind: "ConfigMap", Name: configMap.Name, Namespace: nsName, Version: "v1"}, - {Kind: "PersistentVolumeClaim", Name: pvc.Name, Namespace: nsName, Version: "v1"}, {Group: "rbac.authorization.k8s.io", Kind: "Role", Name: role.Name, Namespace: nsName, Version: "v1"}, } diff --git a/test/e2e/resource_placement_hub_workload_test.go b/test/e2e/resource_placement_hub_workload_test.go index cd30812cf..97634f9a8 100644 --- a/test/e2e/resource_placement_hub_workload_test.go +++ b/test/e2e/resource_placement_hub_workload_test.go @@ -117,23 +117,9 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res Name: testStatefulSet.Name, Namespace: workNamespace.Name, }, - // PVCs created by StatefulSet controller from volumeClaimTemplates - // Kubernetes StatefulSet controller uses naming convention: -- - { - Version: "v1", - Kind: "PersistentVolumeClaim", - Name: fmt.Sprintf("%s-%s-%d", testStatefulSet.Spec.VolumeClaimTemplates[0].Name, testStatefulSet.Name, 0), - Namespace: workNamespace.Name, - }, - { - Version: "v1", - Kind: "PersistentVolumeClaim", - Name: fmt.Sprintf("%s-%s-%d", testStatefulSet.Spec.VolumeClaimTemplates[0].Name, testStatefulSet.Name, 1), - Namespace: workNamespace.Name, - }, } // Use customizedPlacementStatusUpdatedActual with resourceIsTrackable=false - // because Jobs don't have availability tracking like Deployments/DaemonSets/StatefulSets/PVCs do + // because Jobs don't have availability tracking like Deployments/DaemonSets/StatefulSets do crpKey := types.NamespacedName{Name: crpName} crpStatusUpdatedActual := customizedPlacementStatusUpdatedActual(crpKey, wantSelectedResources, allMemberClusterNames, nil, "0", false) Eventually(crpStatusUpdatedActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected") diff --git a/test/e2e/resource_placement_selecting_resources_test.go b/test/e2e/resource_placement_selecting_resources_test.go index 069dd3269..8083da339 100644 --- a/test/e2e/resource_placement_selecting_resources_test.go +++ b/test/e2e/resource_placement_selecting_resources_test.go @@ -1013,10 +1013,10 @@ var _ = Describe("testing RP selecting resources", Label("resourceplacement"), f It("should update RP status with the correct order of the selected resources", func() { // Define the expected resources in order. + // Note: PVCs are not propagated, so they should not appear in selected resources expectedResources := []placementv1beta1.ResourceIdentifier{ {Kind: "Secret", Name: secret.Name, Namespace: nsName, Version: "v1"}, {Kind: "ConfigMap", Name: configMap.Name, Namespace: nsName, Version: "v1"}, - {Kind: "PersistentVolumeClaim", Name: pvc.Name, Namespace: nsName, Version: "v1"}, {Group: "rbac.authorization.k8s.io", Kind: "Role", Name: role.Name, Namespace: nsName, Version: "v1"}, } From d37ed21bf002253209e83a15b5ce09dafdde4003 Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Mon, 1 Dec 2025 16:00:25 +0000 Subject: [PATCH 8/8] remove PVC annotation logic Signed-off-by: Wei Weng --- .../placement/resource_selector.go | 17 ------ .../placement/resource_selector_test.go | 61 ------------------- 2 files changed, 78 deletions(-) diff --git a/pkg/controllers/placement/resource_selector.go b/pkg/controllers/placement/resource_selector.go index 100ff5961..51a4d995a 100644 --- a/pkg/controllers/placement/resource_selector.go +++ b/pkg/controllers/placement/resource_selector.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" - pvutil "k8s.io/component-helpers/storage/volume" "k8s.io/klog/v2" "k8s.io/kubectl/pkg/util/deployment" "sigs.k8s.io/controller-runtime/pkg/client" @@ -445,18 +444,6 @@ func generateRawContent(object *unstructured.Unstructured) ([]byte, error) { delete(annots, corev1.LastAppliedConfigAnnotation) // Remove the revision annotation set by deployment controller. delete(annots, deployment.RevisionAnnotation) - // Remove node-specific and provisioning-related annotations from PVCs that would break when propagated to member clusters - // These annotations reference specific nodes/provisioners from the hub cluster which don't exist on member clusters - // The member cluster's storage provisioner will set appropriate values for its own environment - // All annotations below are listed in well-known labels, annotations and taints document: - // https://kubernetes.io/docs/reference/labels-annotations-taints/ - delete(annots, pvutil.AnnSelectedNode) // Node selected for volume binding - delete(annots, pvutil.AnnBindCompleted) // Binding completion status - delete(annots, pvutil.AnnBoundByController) // Controller binding status - delete(annots, pvutil.AnnStorageProvisioner) // Storage provisioner annotation - delete(annots, pvutil.AnnBetaStorageProvisioner) // Beta storage provisioner annotation - delete(annots, pvutil.AnnDynamicallyProvisioned) // Dynamically provisioned by annotation - delete(annots, pvutil.AnnMigratedTo) // CSI migration annotation if len(annots) == 0 { object.SetAnnotations(nil) } else { @@ -504,10 +491,6 @@ func generateRawContent(object *unstructured.Unstructured) ([]byte, error) { unstructured.RemoveNestedField(object.Object, "spec", "template", "metadata", "labels", "controller-uid") unstructured.RemoveNestedField(object.Object, "spec", "template", "metadata", "labels", "batch.kubernetes.io/controller-uid") } - } else if object.GetKind() == "PersistentVolumeClaim" && object.GetAPIVersion() == "v1" { - // Remove volumeName which references a specific PV from the hub cluster that won't exist on member clusters. - // The member cluster's storage provisioner will create and bind a new PV. - unstructured.RemoveNestedField(object.Object, "spec", "volumeName") } rawContent, err := object.MarshalJSON() diff --git a/pkg/controllers/placement/resource_selector_test.go b/pkg/controllers/placement/resource_selector_test.go index 5bd354e7e..29d62f426 100644 --- a/pkg/controllers/placement/resource_selector_test.go +++ b/pkg/controllers/placement/resource_selector_test.go @@ -27,14 +27,12 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" utilrand "k8s.io/apimachinery/pkg/util/rand" - pvutil "k8s.io/component-helpers/storage/volume" "k8s.io/kubectl/pkg/util/deployment" "k8s.io/utils/ptr" @@ -245,65 +243,6 @@ func TestGenerateResourceContent(t *testing.T) { }, }, }, - "PersistentVolumeClaim with node-specific annotations": { - resource: corev1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "PersistentVolumeClaim", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc", - Namespace: "test-namespace", - Annotations: map[string]string{ - pvutil.AnnSelectedNode: "hub-control-plane", - pvutil.AnnBindCompleted: "yes", - pvutil.AnnBoundByController: "yes", - pvutil.AnnStorageProvisioner: "kubernetes.io/aws-ebs", - pvutil.AnnBetaStorageProvisioner: "kubernetes.io/no-provisioner", - pvutil.AnnDynamicallyProvisioned: "kubernetes.io/aws-ebs", - pvutil.AnnMigratedTo: "ebs.csi.aws.com", - "custom-annotation": "should-remain", - }, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{ - corev1.ReadWriteOnce, - }, - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("1Gi"), - }, - }, - StorageClassName: ptr.To("standard"), - VolumeName: "pvc-12345-from-hub-cluster", - }, - }, - wantResource: corev1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "PersistentVolumeClaim", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc", - Namespace: "test-namespace", - Annotations: map[string]string{ - "custom-annotation": "should-remain", - }, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{ - corev1.ReadWriteOnce, - }, - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("1Gi"), - }, - }, - StorageClassName: ptr.To("standard"), - // VolumeName should be removed - }, - }, - }, } for testName, tt := range tests {