Skip to content

Commit 1258855

Browse files
karmada-botkarmada-bot
authored
Merge pull request #6671 from liaolecheng/feature/direct-purge-delete-before-create
Ensure in directly purgeMode: resources removed from old cluster before being created in new cluster
2 parents caa18b1 + 2746a92 commit 1258855

File tree

5 files changed

+322
-0
lines changed

5 files changed

+322
-0
lines changed

‎pkg/controllers/binding/binding_controller.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,17 @@ func (c *ResourceBindingController) syncBinding(ctx context.Context, binding *wo
113113
return controllerruntime.Result{}, err
114114
}
115115

116+
needWaitForCleanup, err := c.checkDirectPurgeOrphanWorks(ctx, binding)
117+
if err != nil {
118+
return controllerruntime.Result{}, err
119+
}
120+
if needWaitForCleanup {
121+
msg := fmt.Sprintf("There are works in clusters with PurgeMode 'Directly' not deleted for ResourceBinding(%s/%s), skip syncing works",
122+
binding.Namespace, binding.Name)
123+
klog.V(4).InfoS(msg, "namespace", binding.GetNamespace(), "binding", binding.GetName())
124+
return controllerruntime.Result{RequeueAfter: requeueIntervalForDirectlyPurge}, nil
125+
}
126+
116127
workload, err := helper.FetchResourceTemplate(ctx, c.DynamicClient, c.InformerManager, c.RESTMapper, binding.Spec.Resource)
117128
if err != nil {
118129
if apierrors.IsNotFound(err) {
@@ -160,6 +171,18 @@ func (c *ResourceBindingController) removeOrphanWorks(ctx context.Context, bindi
160171
return nil
161172
}
162173

174+
// checkDirectPurgeOrphanWorks checks whether there are orphan works in clusters with PurgeMode 'Directly'.
175+
func (c *ResourceBindingController) checkDirectPurgeOrphanWorks(ctx context.Context, binding *workv1alpha2.ResourceBinding) (bool, error) {
176+
works, err := helper.FindWorksInClusters(ctx, c.Client, binding.Namespace, binding.Name,
177+
binding.Labels[workv1alpha2.ResourceBindingPermanentIDLabel], helper.ObtainClustersWithPurgeModeDirectly(binding.Spec))
178+
if err != nil {
179+
klog.ErrorS(err, "Failed to find orphaned works in clusters with PurgeMode 'Directly'", "namespace", binding.GetNamespace(), "binding", binding.GetName())
180+
return false, err
181+
}
182+
183+
return len(works) > 0, nil
184+
}
185+
163186
// SetupWithManager creates a controller and register to controller manager.
164187
func (c *ResourceBindingController) SetupWithManager(mgr controllerruntime.Manager) error {
165188
return controllerruntime.NewControllerManagedBy(mgr).

‎pkg/controllers/binding/cluster_resource_binding_controller.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,15 @@ func (c *ClusterResourceBindingController) syncBinding(ctx context.Context, bind
112112
if err := c.removeOrphanWorks(ctx, binding); err != nil {
113113
return controllerruntime.Result{}, err
114114
}
115+
needWaitForCleanup, err := c.checkDirectPurgeOrphanWorks(ctx, binding)
116+
if err != nil {
117+
return controllerruntime.Result{}, err
118+
}
119+
if needWaitForCleanup {
120+
msg := fmt.Sprintf("There are works in clusters with PurgeMode 'Directly' not deleted yet for ClusterResourceBinding(%s).", binding.Name)
121+
klog.V(4).InfoS(msg, "ClusterResourceBinding", binding.Name)
122+
return controllerruntime.Result{RequeueAfter: requeueIntervalForDirectlyPurge}, nil
123+
}
115124

116125
workload, err := helper.FetchResourceTemplate(ctx, c.DynamicClient, c.InformerManager, c.RESTMapper, binding.Spec.Resource)
117126
if err != nil {
@@ -160,6 +169,18 @@ func (c *ClusterResourceBindingController) removeOrphanWorks(ctx context.Context
160169
return nil
161170
}
162171

172+
// checkDirectPurgeOrphanWorks checks whether there are orphan works in clusters with PurgeMode 'Directly'.
173+
func (c *ClusterResourceBindingController) checkDirectPurgeOrphanWorks(ctx context.Context, binding *workv1alpha2.ClusterResourceBinding) (bool, error) {
174+
works, err := helper.FindWorksInClusters(ctx, c.Client, "", binding.Name, binding.Labels[workv1alpha2.ClusterResourceBindingPermanentIDLabel], helper.ObtainClustersWithPurgeModeDirectly(binding.Spec))
175+
if err != nil {
176+
klog.ErrorS(err, "Failed to find orphaned works in clusters with PurgeMode 'Directly'", "ClusterResourceBinding", binding.Name)
177+
c.EventRecorder.Event(binding, corev1.EventTypeWarning, events.EventReasonCleanupWorkFailed, err.Error())
178+
return false, err
179+
}
180+
181+
return len(works) > 0, nil
182+
}
183+
163184
// SetupWithManager creates a controller and register to controller manager.
164185
func (c *ClusterResourceBindingController) SetupWithManager(mgr controllerruntime.Manager) error {
165186
return controllerruntime.NewControllerManagedBy(mgr).

‎pkg/controllers/binding/common.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package binding
1919
import (
2020
"context"
2121
"strconv"
22+
"time"
2223

2324
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2425
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -41,6 +42,11 @@ import (
4142
"github.com/karmada-io/karmada/pkg/util/overridemanager"
4243
)
4344

45+
const (
46+
// requeueIntervalForDirectlyPurge is the requeue interval for binding when there are works in clusters with PurgeMode 'Directly'.
47+
requeueIntervalForDirectlyPurge = 5 * time.Second
48+
)
49+
4450
// ensureWork ensure Work to be created or updated.
4551
func ensureWork(
4652
ctx context.Context, c client.Client, resourceInterpreter resourceinterpreter.ResourceInterpreter, workload *unstructured.Unstructured,

‎pkg/util/helper/binding.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,20 @@ func ObtainBindingSpecExistingClusters(bindingSpec workv1alpha2.ResourceBindingS
200200
return clusterNames
201201
}
202202

203+
// ObtainClustersWithPurgeModeDirectly will obtain the cluster slice whose eviction tasks are with PurgeModeDirectly.
204+
func ObtainClustersWithPurgeModeDirectly(bindingSpec workv1alpha2.ResourceBindingSpec) sets.Set[string] {
205+
clusterNames := sets.New[string]()
206+
for _, task := range bindingSpec.GracefulEvictionTasks {
207+
//nolint:staticcheck
208+
// disable `deprecation` check for backward compatibility.
209+
if task.PurgeMode == policyv1alpha1.PurgeModeDirectly ||
210+
task.PurgeMode == policyv1alpha1.Immediately {
211+
clusterNames.Insert(task.FromCluster)
212+
}
213+
}
214+
return clusterNames
215+
}
216+
203217
// FindOrphanWorks retrieves all works that labeled with current binding(ResourceBinding or ClusterResourceBinding) objects,
204218
// then pick the works that not meet current binding declaration.
205219
func FindOrphanWorks(ctx context.Context, c client.Client, bindingNamespace, bindingName, bindingID string, expectClusters sets.Set[string]) ([]workv1alpha1.Work, error) {
@@ -241,6 +255,29 @@ func RemoveOrphanWorks(ctx context.Context, c client.Client, works []workv1alpha
241255
return errors.NewAggregate(errs)
242256
}
243257

258+
// FindWorksInClusters retrieves works that belong to the specified clusters.
259+
func FindWorksInClusters(ctx context.Context, c client.Client, bindingNamespace, bindingName, bindingID string, targetClusters sets.Set[string]) ([]workv1alpha1.Work, error) {
260+
workList, err := GetWorksByBindingID(ctx, c, bindingID, bindingNamespace != "")
261+
if err != nil {
262+
klog.Errorf("Failed to get works by binding object (%s/%s): %v", bindingNamespace, bindingName, err)
263+
return nil, err
264+
}
265+
266+
var filteredWorks []workv1alpha1.Work
267+
for _, work := range workList.Items {
268+
workTargetCluster, err := names.GetClusterName(work.GetNamespace())
269+
if err != nil {
270+
klog.Errorf("Failed to get cluster name for Work %s/%s. Error: %v", work.GetNamespace(), work.GetName(), err)
271+
return nil, err
272+
}
273+
if targetClusters.Has(workTargetCluster) {
274+
filteredWorks = append(filteredWorks, work)
275+
}
276+
}
277+
278+
return filteredWorks, nil
279+
}
280+
244281
// FetchResourceTemplate fetches the resource template to be propagated.
245282
func FetchResourceTemplate(
246283
ctx context.Context,

‎pkg/util/helper/binding_test.go‎

Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,6 +611,196 @@ func TestFindOrphanWorks(t *testing.T) {
611611
}
612612
}
613613

614+
func TestFindWorksInClusters(t *testing.T) {
615+
type args struct {
616+
c client.Client
617+
bindingNamespace string
618+
bindingName string
619+
bindingID string
620+
targetClusters sets.Set[string]
621+
}
622+
tests := []struct {
623+
name string
624+
args args
625+
want []workv1alpha1.Work
626+
wantErr bool
627+
}{
628+
{
629+
name: "namespace scope: no works in target clusters",
630+
args: args{
631+
c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(
632+
&workv1alpha1.Work{
633+
ObjectMeta: metav1.ObjectMeta{
634+
Name: "work1",
635+
Namespace: names.ExecutionSpacePrefix + "cluster1",
636+
ResourceVersion: "999",
637+
Labels: map[string]string{
638+
workv1alpha2.ResourceBindingPermanentIDLabel: "binding-id",
639+
},
640+
},
641+
},
642+
&workv1alpha1.Work{
643+
ObjectMeta: metav1.ObjectMeta{
644+
Name: "work2",
645+
Namespace: names.ExecutionSpacePrefix + "cluster2",
646+
ResourceVersion: "999",
647+
Labels: map[string]string{
648+
workv1alpha2.ResourceBindingPermanentIDLabel: "binding-id",
649+
},
650+
},
651+
},
652+
).WithIndex(
653+
&workv1alpha1.Work{},
654+
indexregistry.WorkIndexByLabelResourceBindingID,
655+
indexregistry.GenLabelIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
656+
).Build(),
657+
bindingNamespace: "default",
658+
bindingName: "test-binding",
659+
bindingID: "binding-id",
660+
targetClusters: sets.New("cluster3"),
661+
},
662+
want: nil,
663+
wantErr: false,
664+
},
665+
{
666+
name: "namespace scope: some works in target clusters",
667+
args: args{
668+
c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(
669+
&workv1alpha1.Work{
670+
ObjectMeta: metav1.ObjectMeta{
671+
Name: "work1",
672+
Namespace: names.ExecutionSpacePrefix + "cluster1",
673+
ResourceVersion: "999",
674+
Labels: map[string]string{
675+
workv1alpha2.ResourceBindingPermanentIDLabel: "binding-id",
676+
},
677+
},
678+
},
679+
&workv1alpha1.Work{
680+
ObjectMeta: metav1.ObjectMeta{
681+
Name: "work2",
682+
Namespace: names.ExecutionSpacePrefix + "cluster2",
683+
ResourceVersion: "999",
684+
Labels: map[string]string{
685+
workv1alpha2.ResourceBindingPermanentIDLabel: "binding-id",
686+
},
687+
},
688+
},
689+
).WithIndex(
690+
&workv1alpha1.Work{},
691+
indexregistry.WorkIndexByLabelResourceBindingID,
692+
indexregistry.GenLabelIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
693+
).Build(),
694+
bindingNamespace: "default",
695+
bindingName: "test-binding",
696+
bindingID: "binding-id",
697+
targetClusters: sets.New("cluster1"),
698+
},
699+
want: []workv1alpha1.Work{
700+
{
701+
ObjectMeta: metav1.ObjectMeta{
702+
Name: "work1",
703+
Namespace: names.ExecutionSpacePrefix + "cluster1",
704+
ResourceVersion: "999",
705+
Labels: map[string]string{
706+
workv1alpha2.ResourceBindingPermanentIDLabel: "binding-id",
707+
},
708+
},
709+
},
710+
},
711+
wantErr: false,
712+
},
713+
{
714+
name: "namespace scope: error getting cluster name",
715+
args: args{
716+
c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(
717+
&workv1alpha1.Work{
718+
ObjectMeta: metav1.ObjectMeta{
719+
Name: "work1",
720+
Namespace: "invalid-namespace",
721+
ResourceVersion: "999",
722+
Labels: map[string]string{
723+
workv1alpha2.ResourceBindingPermanentIDLabel: "binding-id",
724+
},
725+
},
726+
},
727+
).WithIndex(
728+
&workv1alpha1.Work{},
729+
indexregistry.WorkIndexByLabelResourceBindingID,
730+
indexregistry.GenLabelIndexerFunc(workv1alpha2.ResourceBindingPermanentIDLabel),
731+
).Build(),
732+
bindingNamespace: "default",
733+
bindingName: "test-binding",
734+
bindingID: "binding-id",
735+
targetClusters: sets.New("cluster1"),
736+
},
737+
want: nil,
738+
wantErr: true,
739+
},
740+
{
741+
name: "cluster scope: some works in target clusters",
742+
args: args{
743+
c: fake.NewClientBuilder().WithScheme(gclient.NewSchema()).WithObjects(
744+
&workv1alpha1.Work{
745+
ObjectMeta: metav1.ObjectMeta{
746+
Name: "work3",
747+
Namespace: names.ExecutionSpacePrefix + "clusterA",
748+
ResourceVersion: "999",
749+
Labels: map[string]string{
750+
workv1alpha2.ClusterResourceBindingPermanentIDLabel: "cluster-binding-id",
751+
},
752+
},
753+
},
754+
&workv1alpha1.Work{
755+
ObjectMeta: metav1.ObjectMeta{
756+
Name: "work4",
757+
Namespace: names.ExecutionSpacePrefix + "clusterB",
758+
ResourceVersion: "999",
759+
Labels: map[string]string{
760+
workv1alpha2.ClusterResourceBindingPermanentIDLabel: "cluster-binding-id",
761+
},
762+
},
763+
},
764+
).WithIndex(
765+
&workv1alpha1.Work{},
766+
indexregistry.WorkIndexByLabelClusterResourceBindingID,
767+
indexregistry.GenLabelIndexerFunc(workv1alpha2.ClusterResourceBindingPermanentIDLabel),
768+
).Build(),
769+
bindingNamespace: "",
770+
bindingName: "cluster-binding",
771+
bindingID: "cluster-binding-id",
772+
targetClusters: sets.New("clusterA"),
773+
},
774+
want: []workv1alpha1.Work{
775+
{
776+
ObjectMeta: metav1.ObjectMeta{
777+
Name: "work3",
778+
Namespace: names.ExecutionSpacePrefix + "clusterA",
779+
ResourceVersion: "999",
780+
Labels: map[string]string{
781+
workv1alpha2.ClusterResourceBindingPermanentIDLabel: "cluster-binding-id",
782+
},
783+
},
784+
},
785+
},
786+
wantErr: false,
787+
},
788+
}
789+
790+
for _, tt := range tests {
791+
t.Run(tt.name, func(t *testing.T) {
792+
got, err := FindWorksInClusters(context.TODO(), tt.args.c, tt.args.bindingNamespace, tt.args.bindingName, tt.args.bindingID, tt.args.targetClusters)
793+
if (err != nil) != tt.wantErr {
794+
t.Errorf("FindWorksInClusters() error = %v, wantErr %v", err, tt.wantErr)
795+
return
796+
}
797+
if !reflect.DeepEqual(got, tt.want) {
798+
t.Errorf("FindWorksInClusters() got = %v, want %v", got, tt.want)
799+
}
800+
})
801+
}
802+
}
803+
614804
func TestRemoveOrphanWorks(t *testing.T) {
615805
makeWork := func(name string) *workv1alpha1.Work {
616806
return &workv1alpha1.Work{
@@ -1468,3 +1658,48 @@ func TestFindTargetStatusItemByCluster(t *testing.T) {
14681658
})
14691659
}
14701660
}
1661+
1662+
func TestObtainClustersWithPurgeModeDirectly(t *testing.T) {
1663+
tests := []struct {
1664+
name string
1665+
bindingSpec workv1alpha2.ResourceBindingSpec
1666+
wantClusters sets.Set[string]
1667+
}{
1668+
{
1669+
name: "Multiple clusters with different PurgeModes",
1670+
bindingSpec: workv1alpha2.ResourceBindingSpec{
1671+
GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{
1672+
{
1673+
FromCluster: "cluster1",
1674+
PurgeMode: policyv1alpha1.PurgeModeDirectly,
1675+
},
1676+
{
1677+
FromCluster: "cluster2",
1678+
PurgeMode: policyv1alpha1.PurgeModeDirectly,
1679+
},
1680+
{
1681+
FromCluster: "cluster3",
1682+
PurgeMode: policyv1alpha1.PurgeModeGracefully,
1683+
},
1684+
},
1685+
},
1686+
wantClusters: sets.New("cluster1", "cluster2"),
1687+
},
1688+
{
1689+
name: "Empty GracefulEvictionTasks",
1690+
bindingSpec: workv1alpha2.ResourceBindingSpec{
1691+
GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{},
1692+
},
1693+
wantClusters: sets.New[string](),
1694+
},
1695+
}
1696+
1697+
for _, tt := range tests {
1698+
t.Run(tt.name, func(t *testing.T) {
1699+
gotClusters := ObtainClustersWithPurgeModeDirectly(tt.bindingSpec)
1700+
if !gotClusters.Equal(tt.wantClusters) {
1701+
t.Errorf("ObtainClustersWithPurgeModeDirectly() = %v, want %v", gotClusters.UnsortedList(), tt.wantClusters.UnsortedList())
1702+
}
1703+
})
1704+
}
1705+
}

0 commit comments

Comments
 (0)