Skip to content

Commit 38abc17

Browse files
author
Ryan Zhang
committed
test
1 parent f8e71c8 commit 38abc17

File tree

11 files changed

+87
-66
lines changed

11 files changed

+87
-66
lines changed

‎apis/placement/v1beta1/clusterresourceplacement_types.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ import (
2727
)
2828

2929
const (
30-
// ClusterResourcePlacementCleanupFinalizer is a finalizer added by the CRP controller to all CRPs, to make sure
31-
// that the CRP controller can react to CRP deletions if necessary.
32-
ClusterResourcePlacementCleanupFinalizer = fleetPrefix + "crp-cleanup"
30+
// PlacementCleanupFinalizer is a finalizer added by the placement controller to all placement objects, to make sure
31+
// that the placement controller can react to placement object deletions if necessary.
32+
PlacementCleanupFinalizer = fleetPrefix + "crp-cleanup"
3333

34-
// SchedulerCleanupFinalizer is a finalizer added by the scheduler to CRPs, to make sure
35-
// that all bindings derived from a CRP can be cleaned up after the CRP is deleted.
34+
// SchedulerCleanupFinalizer is a finalizer added by the scheduler to placement objects, to make sure
35+
// that all bindings derived from a placement object can be cleaned up after the placement object is deleted.
3636
SchedulerCleanupFinalizer = fleetPrefix + "scheduler-cleanup"
3737
)
3838

‎pkg/controllers/clusterresourceplacement/controller.go‎

Lines changed: 54 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,13 @@ var resourceSnapshotResourceSizeLimit = 800 * (1 << 10) // 800KB
5858
const controllerResyncPeriod = 15 * time.Minute
5959

6060
func (r *Reconciler) Reconcile(ctx context.Context, key controller.QueueKey) (ctrl.Result, error) {
61-
keyStr, ok := key.(string)
61+
placementKey, ok := key.(queue.PlacementKey)
6262
if !ok {
6363
err := fmt.Errorf("get place key %+v not of type string", key)
6464
klog.ErrorS(controller.NewUnexpectedBehaviorError(err), "We have encountered a fatal error that can't be retried, requeue after a day")
6565
return ctrl.Result{}, nil // ignore this unexpected error
6666
}
6767

68-
placementKey := queue.PlacementKey(keyStr)
6968
startTime := time.Now()
7069
klog.V(2).InfoS("Placement reconciliation starts", "placementKey", placementKey)
7170
defer func() {
@@ -88,8 +87,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, key controller.QueueKey) (ct
8887
}
8988

9089
// register finalizer
91-
if !controllerutil.ContainsFinalizer(placementObj, fleetv1beta1.ClusterResourcePlacementCleanupFinalizer) {
92-
controllerutil.AddFinalizer(placementObj, fleetv1beta1.ClusterResourcePlacementCleanupFinalizer)
90+
if !controllerutil.ContainsFinalizer(placementObj, fleetv1beta1.PlacementCleanupFinalizer) {
91+
controllerutil.AddFinalizer(placementObj, fleetv1beta1.PlacementCleanupFinalizer)
9392
if err := r.Client.Update(ctx, placementObj); err != nil {
9493
klog.ErrorS(err, "Failed to add placement finalizer", "placement", klog.KObj(placementObj))
9594
return ctrl.Result{}, controller.NewUpdateIgnoreConflictError(err)
@@ -101,20 +100,20 @@ func (r *Reconciler) Reconcile(ctx context.Context, key controller.QueueKey) (ct
101100

102101
func (r *Reconciler) handleDelete(ctx context.Context, placementObj fleetv1beta1.PlacementObj) (ctrl.Result, error) {
103102
placementKObj := klog.KObj(placementObj)
104-
if !controllerutil.ContainsFinalizer(placementObj, fleetv1beta1.ClusterResourcePlacementCleanupFinalizer) {
103+
if !controllerutil.ContainsFinalizer(placementObj, fleetv1beta1.PlacementCleanupFinalizer) {
105104
klog.V(4).InfoS("placement is being deleted and no cleanup work needs to be done by the CRP controller, waiting for the scheduler to cleanup the bindings", "placement", placementKObj)
106105
return ctrl.Result{}, nil
107106
}
108107
klog.V(2).InfoS("Removing snapshots created by placement", "placement", placementKObj)
109108
if err := r.deleteClusterSchedulingPolicySnapshots(ctx, placementObj); err != nil {
110109
return ctrl.Result{}, err
111110
}
112-
if err := r.deleteClusterResourceSnapshots(ctx, placementObj); err != nil {
111+
if err := r.deleteResourceSnapshots(ctx, placementObj); err != nil {
113112
return ctrl.Result{}, err
114113
}
115-
114+
// change the metrics to add nameplace of namespace
116115
metrics.FleetPlacementStatusLastTimeStampSeconds.DeletePartialMatch(prometheus.Labels{"name": placementObj.GetName()})
117-
controllerutil.RemoveFinalizer(placementObj, fleetv1beta1.ClusterResourcePlacementCleanupFinalizer)
116+
controllerutil.RemoveFinalizer(placementObj, fleetv1beta1.PlacementCleanupFinalizer)
118117
if err := r.Client.Update(ctx, placementObj); err != nil {
119118
klog.ErrorS(err, "Failed to remove placement finalizer", "placement", placementKObj)
120119
return ctrl.Result{}, err
@@ -124,37 +123,59 @@ func (r *Reconciler) handleDelete(ctx context.Context, placementObj fleetv1beta1
124123
return ctrl.Result{}, nil
125124
}
126125

126+
// deleteClusterSchedulingPolicySnapshots deletes all the policy snapshots owned by the placement.
127+
// For cluster-scoped placements (ClusterResourcePlacement), it deletes ClusterSchedulingPolicySnapshots.
128+
// For namespaced placements (ResourcePlacement), it deletes SchedulingPolicySnapshots.
127129
func (r *Reconciler) deleteClusterSchedulingPolicySnapshots(ctx context.Context, placementObj fleetv1beta1.PlacementObj) error {
128-
snapshotList := &fleetv1beta1.ClusterSchedulingPolicySnapshotList{}
129130
placementKObj := klog.KObj(placementObj)
130-
if err := r.UncachedReader.List(ctx, snapshotList, client.MatchingLabels{fleetv1beta1.PlacementTrackingLabel: placementObj.GetName()}); err != nil {
131-
klog.ErrorS(err, "Failed to list all clusterSchedulingPolicySnapshots", "placement", placementKObj)
132-
return controller.NewAPIServerError(false, err)
133-
}
134-
for i := range snapshotList.Items {
135-
if err := r.Client.Delete(ctx, &snapshotList.Items[i]); err != nil && !apierrors.IsNotFound(err) {
136-
klog.ErrorS(err, "Failed to delete clusterSchedulingPolicySnapshot", "placement", placementKObj, "clusterSchedulingPolicySnapshot", klog.KObj(&snapshotList.Items[i]))
131+
labelSelector := client.MatchingLabels{fleetv1beta1.PlacementTrackingLabel: placementObj.GetName()}
132+
133+
// Check if the placement is namespaced or cluster-scoped
134+
if placementObj.GetNamespace() != "" {
135+
// This is a namespaced ResourcePlacement - delete SchedulingPolicySnapshots
136+
if err := r.Client.DeleteAllOf(ctx, &fleetv1beta1.SchedulingPolicySnapshot{},
137+
labelSelector, client.InNamespace(placementObj.GetNamespace())); err != nil {
138+
klog.ErrorS(err, "Failed to delete schedulingPolicySnapshots", "placement", placementKObj)
137139
return controller.NewAPIServerError(false, err)
138140
}
141+
klog.V(2).InfoS("Deleted schedulingPolicySnapshots", "placement", placementKObj)
142+
} else {
143+
// This is a cluster-scoped ClusterResourcePlacement - delete ClusterSchedulingPolicySnapshots
144+
if err := r.Client.DeleteAllOf(ctx, &fleetv1beta1.ClusterSchedulingPolicySnapshot{},
145+
labelSelector); err != nil {
146+
klog.ErrorS(err, "Failed to delete clusterSchedulingPolicySnapshots", "placement", placementKObj)
147+
return controller.NewAPIServerError(false, err)
148+
}
149+
klog.V(2).InfoS("Deleted clusterSchedulingPolicySnapshots", "placement", placementKObj)
139150
}
140-
klog.V(2).InfoS("Deleted clusterSchedulingPolicySnapshots", "placement", placementKObj, "numberOfSnapshots", len(snapshotList.Items))
151+
141152
return nil
142153
}
143154

144-
func (r *Reconciler) deleteClusterResourceSnapshots(ctx context.Context, placementObj fleetv1beta1.PlacementObj) error {
145-
snapshotList := &fleetv1beta1.ClusterResourceSnapshotList{}
155+
func (r *Reconciler) deleteResourceSnapshots(ctx context.Context, placementObj fleetv1beta1.PlacementObj) error {
146156
placementKObj := klog.KObj(placementObj)
147-
if err := r.UncachedReader.List(ctx, snapshotList, client.MatchingLabels{fleetv1beta1.PlacementTrackingLabel: placementObj.GetName()}); err != nil {
148-
klog.ErrorS(err, "Failed to list all clusterResourceSnapshots", "placement", placementKObj)
149-
return controller.NewAPIServerError(false, err)
150-
}
151-
for i := range snapshotList.Items {
152-
if err := r.Client.Delete(ctx, &snapshotList.Items[i]); err != nil && !apierrors.IsNotFound(err) {
153-
klog.ErrorS(err, "Failed to delete clusterResourceSnapshots", "placement", placementKObj, "clusterResourceSnapshot", klog.KObj(&snapshotList.Items[i]))
157+
labelSelector := client.MatchingLabels{
158+
fleetv1beta1.PlacementTrackingLabel: placementObj.GetName(),
159+
}
160+
// Check if the placement is namespaced or cluster-scoped
161+
if placementObj.GetNamespace() != "" {
162+
// This is a namespaced ResourcePlacement - delete ResourceSnapshots
163+
if err := r.Client.DeleteAllOf(ctx, &fleetv1beta1.ResourceSnapshot{},
164+
labelSelector, client.InNamespace(placementObj.GetNamespace())); err != nil {
165+
klog.ErrorS(err, "Failed to delete ResourceSnapshots", "placement", placementKObj)
154166
return controller.NewAPIServerError(false, err)
155167
}
168+
klog.V(2).InfoS("Deleted ResourceSnapshots", "placement", placementKObj)
169+
} else {
170+
// This is a cluster-scoped ClusterResourcePlacement - delete ClusterResourceSnapshots
171+
if err := r.Client.DeleteAllOf(ctx, &fleetv1beta1.ClusterResourceSnapshot{},
172+
labelSelector); err != nil {
173+
klog.ErrorS(err, "Failed to delete ClusterResourceSnapshots", "placement", placementKObj)
174+
return controller.NewAPIServerError(false, err)
175+
}
176+
klog.V(2).InfoS("Deleted ClusterResourceSnapshots", "placement", placementKObj)
156177
}
157-
klog.V(2).InfoS("Deleted clusterResourceSnapshots", "placement", placementKObj, "numberOfSnapshots", len(snapshotList.Items))
178+
158179
return nil
159180
}
160181

@@ -226,7 +247,7 @@ func (r *Reconciler) handleUpdate(ctx context.Context, placementObj fleetv1beta1
226247
latestResourceSnapshotKObj := klog.KObj(latestResourceSnapshot)
227248
// We cannot create the resource snapshot immediately because of the resource snapshot creation interval.
228249
// Rebuild the seletedResourceIDs using the latestResourceSnapshot.
229-
latestResourceSnapshotIndex, err := labels.ExtractResourceIndexFromClusterResourceSnapshot(latestResourceSnapshot)
250+
latestResourceSnapshotIndex, err := labels.ExtractResourceIndexFromResourceSnapshot(latestResourceSnapshot)
230251
if err != nil {
231252
klog.ErrorS(err, "Failed to extract the resource index from the clusterResourceSnapshot", "placement", placementKObj, "clusterResourceSnapshot", latestResourceSnapshotKObj)
232253
return ctrl.Result{}, controller.NewUnexpectedBehaviorError(err)
@@ -453,7 +474,7 @@ func (r *Reconciler) deleteRedundantResourceSnapshots(ctx context.Context, crp *
453474
// snapshots from the end.
454475
for i := len(sortedList.Items) - 1; i >= 0; i-- {
455476
snapshotKObj := klog.KObj(&sortedList.Items[i])
456-
ii, err := labels.ExtractResourceIndexFromClusterResourceSnapshot(&sortedList.Items[i])
477+
ii, err := labels.ExtractResourceIndexFromResourceSnapshot(&sortedList.Items[i])
457478
if err != nil {
458479
klog.ErrorS(err, "Failed to parse the resource index label", "clusterResourcePlacement", crpKObj, "clusterResourceSnapshot", snapshotKObj)
459480
return controller.NewUnexpectedBehaviorError(err)
@@ -900,7 +921,7 @@ func (r *Reconciler) lookupLatestResourceSnapshot(ctx context.Context, crp *flee
900921
return nil, -1, controller.NewAPIServerError(true, err)
901922
}
902923
if len(snapshotList.Items) == 1 {
903-
resourceIndex, err := labels.ExtractResourceIndexFromClusterResourceSnapshot(&snapshotList.Items[0])
924+
resourceIndex, err := labels.ExtractResourceIndexFromResourceSnapshot(&snapshotList.Items[0])
904925
if err != nil {
905926
klog.ErrorS(err, "Failed to parse the resource index label", "clusterResourceSnapshot", klog.KObj(&snapshotList.Items[0]))
906927
return nil, -1, controller.NewUnexpectedBehaviorError(err)
@@ -923,7 +944,7 @@ func (r *Reconciler) lookupLatestResourceSnapshot(ctx context.Context, crp *flee
923944
return nil, -1, nil
924945
}
925946
latestSnapshot := &sortedList.Items[len(sortedList.Items)-1]
926-
resourceIndex, err := labels.ExtractResourceIndexFromClusterResourceSnapshot(latestSnapshot)
947+
resourceIndex, err := labels.ExtractResourceIndexFromResourceSnapshot(latestSnapshot)
927948
if err != nil {
928949
klog.ErrorS(err, "Failed to parse the resource index label", "clusterResourcePlacement", crpKObj, "clusterResourceSnapshot", klog.KObj(latestSnapshot))
929950
return nil, -1, controller.NewUnexpectedBehaviorError(err)
@@ -946,12 +967,12 @@ func (r *Reconciler) listSortedResourceSnapshots(ctx context.Context, crp *fleet
946967
sort.Slice(snapshotList.Items, func(i, j int) bool {
947968
iKObj := klog.KObj(&snapshotList.Items[i])
948969
jKObj := klog.KObj(&snapshotList.Items[j])
949-
ii, err := labels.ExtractResourceIndexFromClusterResourceSnapshot(&snapshotList.Items[i])
970+
ii, err := labels.ExtractResourceIndexFromResourceSnapshot(&snapshotList.Items[i])
950971
if err != nil {
951972
klog.ErrorS(err, "Failed to parse the resource index label", "clusterResourcePlacement", crpKObj, "clusterResourceSnapshot", iKObj)
952973
errs = append(errs, err)
953974
}
954-
ji, err := labels.ExtractResourceIndexFromClusterResourceSnapshot(&snapshotList.Items[j])
975+
ji, err := labels.ExtractResourceIndexFromResourceSnapshot(&snapshotList.Items[j])
955976
if err != nil {
956977
klog.ErrorS(err, "Failed to parse the resource index label", "clusterResourcePlacement", crpKObj, "clusterResourceSnapshot", jKObj)
957978
errs = append(errs, err)

‎pkg/controllers/clusterresourceplacement/controller_integration_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
413413
wantCRP := &placementv1beta1.ClusterResourcePlacement{
414414
ObjectMeta: metav1.ObjectMeta{
415415
Name: testCRPName,
416-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
416+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
417417
},
418418
Spec: crp.Spec,
419419
Status: placementv1beta1.PlacementStatus{
@@ -452,7 +452,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
452452
wantCRP := &placementv1beta1.ClusterResourcePlacement{
453453
ObjectMeta: metav1.ObjectMeta{
454454
Name: testCRPName,
455-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
455+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
456456
},
457457
Spec: crp.Spec,
458458
Status: placementv1beta1.PlacementStatus{
@@ -509,7 +509,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
509509
wantCRP := &placementv1beta1.ClusterResourcePlacement{
510510
ObjectMeta: metav1.ObjectMeta{
511511
Name: testCRPName,
512-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
512+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
513513
},
514514
Spec: crp.Spec,
515515
Status: placementv1beta1.PlacementStatus{
@@ -568,7 +568,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
568568
wantCRP := &placementv1beta1.ClusterResourcePlacement{
569569
ObjectMeta: metav1.ObjectMeta{
570570
Name: testCRPName,
571-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
571+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
572572
},
573573
Spec: crp.Spec,
574574
Status: placementv1beta1.PlacementStatus{
@@ -778,7 +778,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
778778
wantCRP := &placementv1beta1.ClusterResourcePlacement{
779779
ObjectMeta: metav1.ObjectMeta{
780780
Name: testCRPName,
781-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
781+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
782782
},
783783
Spec: crp.Spec,
784784
Status: placementv1beta1.PlacementStatus{
@@ -1172,7 +1172,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
11721172
wantCRP := &placementv1beta1.ClusterResourcePlacement{
11731173
ObjectMeta: metav1.ObjectMeta{
11741174
Name: testCRPName,
1175-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
1175+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
11761176
},
11771177
Spec: crp.Spec,
11781178
Status: placementv1beta1.PlacementStatus{
@@ -1430,7 +1430,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
14301430
wantCRP := &placementv1beta1.ClusterResourcePlacement{
14311431
ObjectMeta: metav1.ObjectMeta{
14321432
Name: testCRPName,
1433-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
1433+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
14341434
},
14351435
Spec: crp.Spec,
14361436
Status: placementv1beta1.PlacementStatus{
@@ -1471,7 +1471,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
14711471
wantCRP := &placementv1beta1.ClusterResourcePlacement{
14721472
ObjectMeta: metav1.ObjectMeta{
14731473
Name: testCRPName,
1474-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
1474+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
14751475
},
14761476
Spec: crp.Spec,
14771477
Status: placementv1beta1.PlacementStatus{
@@ -1654,7 +1654,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
16541654
wantCRP := &placementv1beta1.ClusterResourcePlacement{
16551655
ObjectMeta: metav1.ObjectMeta{
16561656
Name: testCRPName,
1657-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
1657+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
16581658
},
16591659
Spec: crp.Spec,
16601660
Status: placementv1beta1.PlacementStatus{
@@ -1935,7 +1935,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
19351935
wantCRP := &placementv1beta1.ClusterResourcePlacement{
19361936
ObjectMeta: metav1.ObjectMeta{
19371937
Name: testCRPName,
1938-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
1938+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
19391939
},
19401940
Spec: crp.Spec,
19411941
Status: placementv1beta1.PlacementStatus{
@@ -2008,7 +2008,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
20082008
wantCRP := &placementv1beta1.ClusterResourcePlacement{
20092009
ObjectMeta: metav1.ObjectMeta{
20102010
Name: testCRPName,
2011-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
2011+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
20122012
},
20132013
Spec: crp.Spec,
20142014
Status: placementv1beta1.PlacementStatus{
@@ -2056,7 +2056,7 @@ var _ = Describe("Test ClusterResourcePlacement Controller", func() {
20562056
wantCRP := &placementv1beta1.ClusterResourcePlacement{
20572057
ObjectMeta: metav1.ObjectMeta{
20582058
Name: testCRPName,
2059-
Finalizers: []string{placementv1beta1.ClusterResourcePlacementCleanupFinalizer},
2059+
Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer},
20602060
},
20612061
Spec: crp.Spec,
20622062
Status: placementv1beta1.PlacementStatus{

‎pkg/controllers/clusterresourceplacement/controller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3383,7 +3383,7 @@ func TestHandleDelete(t *testing.T) {
33833383
t.Run(tc.name, func(t *testing.T) {
33843384
ctx := context.Background()
33853385
crp := clusterResourcePlacementForTest()
3386-
crp.Finalizers = []string{fleetv1beta1.ClusterResourcePlacementCleanupFinalizer}
3386+
crp.Finalizers = []string{fleetv1beta1.PlacementCleanupFinalizer}
33873387
now := metav1.Now()
33883388
crp.DeletionTimestamp = &now
33893389
objects := []client.Object{crp}

‎pkg/controllers/workgenerator/controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,7 @@ func (r *Reconciler) upsertWork(ctx context.Context, newWork, existingWork *flee
786786
klog.ErrorS(controller.NewUnexpectedBehaviorError(err), "work has invalid parent resource index", "work", workObj)
787787
} else {
788788
// we already checked the label in fetchAllResourceSnapShots function so no need to check again
789-
resourceIndex, _ := labels.ExtractResourceIndexFromClusterResourceSnapshot(resourceSnapshot)
789+
resourceIndex, _ := labels.ExtractResourceIndexFromResourceSnapshot(resourceSnapshot)
790790
if workResourceIndex == resourceIndex {
791791
// no need to do anything if the work is generated from the same resource/override snapshots.
792792
// Note that apply strategy is updated separately beforehand.

0 commit comments

Comments
 (0)