Skip to content

Commit 5832681

Browse files
weng271190436Wei Weng
weng271190436
and
Wei Weng
authored
feat: make hub statefulset work by stripping some properties from generated PVCs (#347)
* make statefulset work in hub Signed-off-by: Wei Weng <[email protected]> * fix test Signed-off-by: Wei Weng <[email protected]> * remove unintended change Signed-off-by: Wei Weng <[email protected]> * remove more annotations Signed-off-by: Wei Weng <[email protected]> * track pvc availability Signed-off-by: Wei Weng <[email protected]> * use cmp.diff to compare status Signed-off-by: Wei Weng <[email protected]> * do not propagate PVCs Signed-off-by: Wei Weng <[email protected]> * remove PVC annotation logic Signed-off-by: Wei Weng <[email protected]> --------- Signed-off-by: Wei Weng <[email protected]> Co-authored-by: Wei Weng <[email protected]>
1 parent d6e3712 commit 5832681

12 files changed

+146
-12
lines changed

‎pkg/utils/common.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,9 @@ func ShouldPropagateObj(informerManager informer.Manager, uObj *unstructured.Uns
541541
if secret.Type == corev1.SecretTypeServiceAccountToken {
542542
return false, nil
543543
}
544+
case corev1.SchemeGroupVersion.WithKind("PersistentVolumeClaim"):
545+
// Skip PersistentVolumeClaims to avoid conflicts with the PVCs created by statefulset controller
546+
return false, nil
544547
case corev1.SchemeGroupVersion.WithKind("Endpoints"):
545548
// we assume that all endpoints with the same name of a service is created by the service controller
546549
if _, err := informerManager.Lister(ServiceGVR).ByNamespace(uObj.GetNamespace()).Get(uObj.GetName()); err != nil {

‎pkg/utils/common_test.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,6 +1317,19 @@ func TestShouldPropagateObj_PodAndReplicaSet(t *testing.T) {
13171317
ownerReferences: nil,
13181318
want: true,
13191319
},
1320+
{
1321+
name: "PersistentVolumeClaim should NOT propagate",
1322+
obj: map[string]interface{}{
1323+
"apiVersion": "v1",
1324+
"kind": "PersistentVolumeClaim",
1325+
"metadata": map[string]interface{}{
1326+
"name": "test-pvc",
1327+
"namespace": "default",
1328+
},
1329+
},
1330+
ownerReferences: nil,
1331+
want: false,
1332+
},
13201333
}
13211334

13221335
for _, tt := range tests {

‎test/e2e/enveloped_object_placement_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ var _ = Describe("placing wrapped resources using a CRP", func() {
226226
// read the test resources.
227227
readDeploymentTestManifest(&testDeployment)
228228
readDaemonSetTestManifest(&testDaemonSet)
229-
readStatefulSetTestManifest(&testStatefulSet, true)
229+
readStatefulSetTestManifest(&testStatefulSet, StatefulSetInvalidStorage)
230230
readEnvelopeResourceTestManifest(&testResourceEnvelope)
231231
})
232232

‎test/e2e/placement_selecting_resources_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1392,11 +1392,11 @@ var _ = Describe("creating CRP and checking selected resources order", Ordered,
13921392

13931393
It("should update CRP status with the correct order of the selected resources", func() {
13941394
// Define the expected resources in order
1395+
// Note: PVCs are not propagated, so they should not appear in selected resources
13951396
expectedResources := []placementv1beta1.ResourceIdentifier{
13961397
{Kind: "Namespace", Name: nsName, Version: "v1"},
13971398
{Kind: "Secret", Name: secret.Name, Namespace: nsName, Version: "v1"},
13981399
{Kind: "ConfigMap", Name: configMap.Name, Namespace: nsName, Version: "v1"},
1399-
{Kind: "PersistentVolumeClaim", Name: pvc.Name, Namespace: nsName, Version: "v1"},
14001400
{Group: "rbac.authorization.k8s.io", Kind: "Role", Name: role.Name, Namespace: nsName, Version: "v1"},
14011401
}
14021402

‎test/e2e/resource_placement_hub_workload_test.go‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package e2e
1919
import (
2020
"fmt"
2121

22+
"github.com/google/go-cmp/cmp"
2223
. "github.com/onsi/ginkgo/v2"
2324
. "github.com/onsi/gomega"
2425
appsv1 "k8s.io/api/apps/v1"
@@ -37,12 +38,14 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res
3738
var testDeployment appsv1.Deployment
3839
var testDaemonSet appsv1.DaemonSet
3940
var testJob batchv1.Job
41+
var testStatefulSet appsv1.StatefulSet
4042

4143
BeforeAll(func() {
4244
// Read the test manifests
4345
readDeploymentTestManifest(&testDeployment)
4446
readDaemonSetTestManifest(&testDaemonSet)
4547
readJobTestManifest(&testJob)
48+
readStatefulSetTestManifest(&testStatefulSet, StatefulSetWithStorage)
4649
workNamespace := appNamespace()
4750

4851
// Create namespace and workloads
@@ -51,9 +54,11 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res
5154
testDeployment.Namespace = workNamespace.Name
5255
testDaemonSet.Namespace = workNamespace.Name
5356
testJob.Namespace = workNamespace.Name
57+
testStatefulSet.Namespace = workNamespace.Name
5458
Expect(hubClient.Create(ctx, &testDeployment)).To(Succeed(), "Failed to create test deployment %s", testDeployment.Name)
5559
Expect(hubClient.Create(ctx, &testDaemonSet)).To(Succeed(), "Failed to create test daemonset %s", testDaemonSet.Name)
5660
Expect(hubClient.Create(ctx, &testJob)).To(Succeed(), "Failed to create test job %s", testJob.Name)
61+
Expect(hubClient.Create(ctx, &testStatefulSet)).To(Succeed(), "Failed to create test statefulset %s", testStatefulSet.Name)
5762

5863
// Create the CRP that selects the namespace
5964
By("creating CRP that selects the namespace")
@@ -105,9 +110,16 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res
105110
Name: testJob.Name,
106111
Namespace: workNamespace.Name,
107112
},
113+
{
114+
Group: "apps",
115+
Version: "v1",
116+
Kind: "StatefulSet",
117+
Name: testStatefulSet.Name,
118+
Namespace: workNamespace.Name,
119+
},
108120
}
109121
// Use customizedPlacementStatusUpdatedActual with resourceIsTrackable=false
110-
// because Jobs don't have availability tracking like Deployments/DaemonSets do
122+
// because Jobs don't have availability tracking like Deployments/DaemonSets/StatefulSets do
111123
crpKey := types.NamespacedName{Name: crpName}
112124
crpStatusUpdatedActual := customizedPlacementStatusUpdatedActual(crpKey, wantSelectedResources, allMemberClusterNames, nil, "0", false)
113125
Eventually(crpStatusUpdatedActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP status as expected")
@@ -170,6 +182,13 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res
170182
"Hub job should complete successfully")
171183
})
172184

185+
It("should verify hub statefulset is ready", func() {
186+
By("checking hub statefulset status")
187+
statefulSetReadyActual := waitForStatefulSetToBeReady(hubClient, &testStatefulSet)
188+
Eventually(statefulSetReadyActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(),
189+
"Hub statefulset should be ready before placement")
190+
})
191+
173192
It("should place the deployment on all member clusters", func() {
174193
By("verifying deployment is placed and ready on all member clusters")
175194
for idx := range allMemberClusters {
@@ -206,6 +225,24 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res
206225
}
207226
})
208227

228+
It("should place the statefulset on all member clusters", func() {
229+
By("verifying statefulset is placed and ready on all member clusters")
230+
for idx := range allMemberClusters {
231+
memberCluster := allMemberClusters[idx]
232+
statefulsetPlacedActual := waitForStatefulSetPlacementToReady(memberCluster, &testStatefulSet)
233+
Eventually(statefulsetPlacedActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place statefulset on member cluster %s", memberCluster.ClusterName)
234+
}
235+
})
236+
237+
It("should verify statefulset replicas are ready on all clusters", func() {
238+
By("checking statefulset status on each cluster")
239+
for _, cluster := range allMemberClusters {
240+
statefulSetReadyActual := waitForStatefulSetToBeReady(cluster.KubeClient, &testStatefulSet)
241+
Eventually(statefulSetReadyActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(),
242+
"StatefulSet should be ready on cluster %s", cluster.ClusterName)
243+
}
244+
})
245+
209246
It("should verify deployment replicas are ready on all clusters", func() {
210247
By("checking deployment status on each cluster")
211248
for _, cluster := range allMemberClusters {
@@ -232,6 +269,42 @@ var _ = Describe("placing workloads using a CRP with PickAll policy", Label("res
232269
})
233270
})
234271

272+
func waitForStatefulSetToBeReady(kubeClient client.Client, testStatefulSet *appsv1.StatefulSet) func() error {
273+
return func() error {
274+
var statefulSet appsv1.StatefulSet
275+
if err := kubeClient.Get(ctx, types.NamespacedName{
276+
Name: testStatefulSet.Name,
277+
Namespace: testStatefulSet.Namespace,
278+
}, &statefulSet); err != nil {
279+
return err
280+
}
281+
282+
// Verify statefulset is ready
283+
requiredReplicas := int32(1)
284+
if statefulSet.Spec.Replicas != nil {
285+
requiredReplicas = *statefulSet.Spec.Replicas
286+
}
287+
288+
wantStatus := appsv1.StatefulSetStatus{
289+
ObservedGeneration: statefulSet.Generation,
290+
CurrentReplicas: requiredReplicas,
291+
UpdatedReplicas: requiredReplicas,
292+
}
293+
294+
gotStatus := appsv1.StatefulSetStatus{
295+
ObservedGeneration: statefulSet.Status.ObservedGeneration,
296+
CurrentReplicas: statefulSet.Status.CurrentReplicas,
297+
UpdatedReplicas: statefulSet.Status.UpdatedReplicas,
298+
}
299+
300+
if diff := cmp.Diff(wantStatus, gotStatus); diff != "" {
301+
return fmt.Errorf("statefulset not ready (-want +got):\n%s", diff)
302+
}
303+
304+
return nil
305+
}
306+
}
307+
235308
func waitForJobToComplete(kubeClient client.Client, testJob *batchv1.Job) func() error {
236309
return func() error {
237310
var job batchv1.Job

‎test/e2e/resource_placement_rollout_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ var _ = Describe("placing namespaced scoped resources using a RP with rollout",
6969
testDaemonSet = appv1.DaemonSet{}
7070
readDaemonSetTestManifest(&testDaemonSet)
7171
testStatefulSet = appv1.StatefulSet{}
72-
readStatefulSetTestManifest(&testStatefulSet, false)
72+
readStatefulSetTestManifest(&testStatefulSet, StatefulSetBasic)
7373
testService = corev1.Service{}
7474
readServiceTestManifest(&testService)
7575
testJob = batchv1.Job{}

‎test/e2e/resource_placement_selecting_resources_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1013,10 +1013,10 @@ var _ = Describe("testing RP selecting resources", Label("resourceplacement"), f
10131013

10141014
It("should update RP status with the correct order of the selected resources", func() {
10151015
// Define the expected resources in order.
1016+
// Note: PVCs are not propagated, so they should not appear in selected resources
10161017
expectedResources := []placementv1beta1.ResourceIdentifier{
10171018
{Kind: "Secret", Name: secret.Name, Namespace: nsName, Version: "v1"},
10181019
{Kind: "ConfigMap", Name: configMap.Name, Namespace: nsName, Version: "v1"},
1019-
{Kind: "PersistentVolumeClaim", Name: pvc.Name, Namespace: nsName, Version: "v1"},
10201020
{Group: "rbac.authorization.k8s.io", Kind: "Role", Name: role.Name, Namespace: nsName, Version: "v1"},
10211021
}
10221022

test/e2e/resources/test-statefulset.yaml renamed to test/e2e/resources/statefulset-basic.yaml

File renamed without changes.

test/e2e/resources/statefulset-with-volume.yaml renamed to test/e2e/resources/statefulset-invalid-storage.yaml

File renamed without changes.

‎test/e2e/resources/statefulset-with-storage.yaml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
apiVersion: apps/v1
2+
kind: StatefulSet
3+
metadata:
4+
name: test-ss
5+
spec:
6+
selector:
7+
matchLabels:
8+
app: test-ss
9+
serviceName: "test-ss-svc"
10+
replicas: 2
11+
template:
12+
metadata:
13+
labels:
14+
app: test-ss
15+
spec:
16+
terminationGracePeriodSeconds: 10
17+
containers:
18+
- name: pause
19+
image: k8s.gcr.io/pause:3.8
20+
volumeClaimTemplates:
21+
- metadata:
22+
name: test-ss-pvc
23+
spec:
24+
accessModes: [ "ReadWriteOnce" ]
25+
storageClassName: "standard"
26+
resources:
27+
requests:
28+
storage: 100Mi

0 commit comments

Comments
 (0)