karmada-io
/
karmada
mirror of https://github.com/karmada-io/karmada.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

to prevent the generation judgment from always being false and block gracefulEvictionTask clearance, move the generation judgment logic to the reconcile 

Signed-off-by: changzhen <changzhen5@huawei.com>
This commit is contained in:
changzhen 2025-03-24 15:30:59 +08:00
parent 0235dc67c9
commit cbfc085283
6 changed files with 81 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
pkg/controllers/gracefuleviction/crb_graceful_eviction_controller.go
View File

@ -78,7 +78,12 @@ func (c *CRBGracefulEvictionController) Reconcile(ctx context.Context, req contr
}
func (c *CRBGracefulEvictionController) syncBinding(ctx context.Context, binding *workv1alpha2.ClusterResourceBinding) (time.Duration, error) {
keptTask, evictedClusters := assessEvictionTasks(binding.Spec, binding.Status.AggregatedStatus, c.GracefulEvictionTimeout, metav1.Now())
keptTask, evictedClusters := assessEvictionTasks(binding.Spec.GracefulEvictionTasks, metav1.Now(), assessmentOption{
timeout: c.GracefulEvictionTimeout,
scheduleResult: binding.Spec.Clusters,
observedStatus: binding.Status.AggregatedStatus,
hasScheduled: binding.Status.SchedulerObservedGeneration == binding.Generation,
})
if reflect.DeepEqual(binding.Spec.GracefulEvictionTasks, keptTask) {
return nextRetry(keptTask, c.GracefulEvictionTimeout, metav1.Now().Time), nil
}
@ -104,21 +109,13 @@ func (c *CRBGracefulEvictionController) SetupWithManager(mgr controllerruntime.M
clusterResourceBindingPredicateFn := predicate.Funcs{
CreateFunc: func(createEvent event.CreateEvent) bool {
newObj := createEvent.Object.(*workv1alpha2.ClusterResourceBinding)
if len(newObj.Spec.GracefulEvictionTasks) == 0 {
return false
}
// When the current component is restarted and there are still tasks in the
// GracefulEvictionTasks queue, we need to continue the procession.
return newObj.Status.SchedulerObservedGeneration == newObj.Generation
return len(newObj.Spec.GracefulEvictionTasks) != 0
},
UpdateFunc: func(updateEvent event.UpdateEvent) bool {
newObj := updateEvent.ObjectNew.(*workv1alpha2.ClusterResourceBinding)
if len(newObj.Spec.GracefulEvictionTasks) == 0 {
return false
}
return newObj.Status.SchedulerObservedGeneration == newObj.Generation
return len(newObj.Spec.GracefulEvictionTasks) != 0
},
DeleteFunc: func(event.DeleteEvent) bool { return false },
GenericFunc: func(event.GenericEvent) bool { return false },

3
pkg/controllers/gracefuleviction/crb_graceful_eviction_controller_test.go
View File

@ -67,7 +67,8 @@ func TestCRBGracefulEvictionController_Reconcile(t *testing.T) {
name: "binding with active graceful eviction tasks",
binding: &workv1alpha2.ClusterResourceBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "test-binding",
Name: "test-binding",
Generation: 1,
},
Spec: workv1alpha2.ResourceBindingSpec{
GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{

33
pkg/controllers/gracefuleviction/evictiontask.go
View File

@ -29,19 +29,17 @@ type assessmentOption struct {
timeout time.Duration
scheduleResult []workv1alpha2.TargetCluster
observedStatus []workv1alpha2.AggregatedStatusItem
hasScheduled bool
}
// assessEvictionTasks assesses each task according to graceful eviction rules and
// returns the tasks that should be kept.
func assessEvictionTasks(bindingSpec workv1alpha2.ResourceBindingSpec,
observedStatus []workv1alpha2.AggregatedStatusItem,
timeout time.Duration,
now metav1.Time,
) ([]workv1alpha2.GracefulEvictionTask, []string) {
// The now time is used as the input parameter to facilitate the unit test.
func assessEvictionTasks(tasks []workv1alpha2.GracefulEvictionTask, now metav1.Time, opt assessmentOption) ([]workv1alpha2.GracefulEvictionTask, []string) {
var keptTasks []workv1alpha2.GracefulEvictionTask
var evictedClusters []string
for _, task := range bindingSpec.GracefulEvictionTasks {
for _, task := range tasks {
// set creation timestamp for new task
if task.CreationTimestamp.IsZero() {
task.CreationTimestamp = &now
@ -49,12 +47,12 @@ func assessEvictionTasks(bindingSpec workv1alpha2.ResourceBindingSpec,
continue
}
if task.GracePeriodSeconds != nil {
opt.timeout = time.Duration(*task.GracePeriodSeconds) * time.Second
}
// assess task according to observed status
kt := assessSingleTask(task, assessmentOption{
scheduleResult: bindingSpec.Clusters,
timeout: timeout,
observedStatus: observedStatus,
})
kt := assessSingleTask(task, opt)
if kt != nil {
keptTasks = append(keptTasks, *kt)
} else {
@ -75,16 +73,14 @@ func assessSingleTask(task workv1alpha2.GracefulEvictionTask, opt assessmentOpti
return nil
}
timeout := opt.timeout
if task.GracePeriodSeconds != nil {
timeout = time.Duration(*task.GracePeriodSeconds) * time.Second
}
// task exceeds timeout
if metav1.Now().After(task.CreationTimestamp.Add(timeout)) {
if metav1.Now().After(task.CreationTimestamp.Add(opt.timeout)) {
return nil
}
if allScheduledResourceInHealthyState(opt) {
// Only when the binding object has been scheduled can further judgment be made.
// Otherwise, the binding status may be the old, which will affect the correctness of the judgment.
if opt.hasScheduled && allScheduledResourceInHealthyState(opt) {
return nil
}
@ -117,7 +113,7 @@ func allScheduledResourceInHealthyState(opt assessmentOption) bool {
return true
}
func nextRetry(tasks []workv1alpha2.GracefulEvictionTask, timeout time.Duration, timeNow time.Time) time.Duration {
func nextRetry(tasks []workv1alpha2.GracefulEvictionTask, gracefulTimeout time.Duration, timeNow time.Time) time.Duration {
if len(tasks) == 0 {
return 0
}
@ -132,6 +128,7 @@ func nextRetry(tasks []workv1alpha2.GracefulEvictionTask, timeout time.Duration,
if tasks[i].SuppressDeletion != nil {
continue
}
timeout := gracefulTimeout
if tasks[i].GracePeriodSeconds != nil {
timeout = time.Duration(*tasks[i].GracePeriodSeconds) * time.Second
}

141
pkg/controllers/gracefuleviction/evictiontask_test.go
View File

@ -41,17 +41,15 @@ func Test_assessSingleTask(t *testing.T) {
want *workv1alpha2.GracefulEvictionTask
}{
{
name: "task that doesn't exceed the timeout",
name: "task doesn't exceed the timeout, hasScheduled is false, task has no change",
args: args{
task: workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
CreationTimestamp: &metav1.Time{Time: timeNow.Add(time.Minute * -1)},
},
opt: assessmentOption{
timeout: timeout,
scheduleResult: []workv1alpha2.TargetCluster{
{Name: "memberA"},
},
timeout: timeout,
hasScheduled: false,
},
},
want: &workv1alpha2.GracefulEvictionTask{
@ -60,7 +58,7 @@ func Test_assessSingleTask(t *testing.T) {
},
},
{
name: "task that exceeds the timeout",
name: "task exceeds the timeout, task will be nil",
args: args{
task: workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
@ -68,15 +66,12 @@ func Test_assessSingleTask(t *testing.T) {
},
opt: assessmentOption{
timeout: timeout,
scheduleResult: []workv1alpha2.TargetCluster{
{Name: "memberA"},
},
},
},
want: nil,
},
{
name: "binding scheduled result is healthy, task should be nil",
name: "task doesn't exceed the timeout, hasScheduled is true, scheduled result is healthy, task will be nil",
args: args{
task: workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
@ -90,12 +85,13 @@ func Test_assessSingleTask(t *testing.T) {
observedStatus: []workv1alpha2.AggregatedStatusItem{
{ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy},
},
hasScheduled: true,
},
},
want: nil,
},
{
name: "binding scheduled result is unhealthy, task has no effect",
name: "task doesn't exceed the timeout, hasScheduled is true, scheduled result is unhealthy, task has no change",
args: args{
task: workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
@ -109,6 +105,7 @@ func Test_assessSingleTask(t *testing.T) {
observedStatus: []workv1alpha2.AggregatedStatusItem{
{ClusterName: "memberA", Health: workv1alpha2.ResourceUnhealthy},
},
hasScheduled: true,
},
},
want: &workv1alpha2.GracefulEvictionTask{
@ -117,7 +114,7 @@ func Test_assessSingleTask(t *testing.T) {
},
},
{
name: "binding scheduled result is unknown, task has no effect",
name: "task doesn't exceed the timeout, hasScheduled is true, scheduled result is unknown, task has no change",
args: args{
task: workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
@ -131,6 +128,7 @@ func Test_assessSingleTask(t *testing.T) {
observedStatus: []workv1alpha2.AggregatedStatusItem{
{ClusterName: "memberA", Health: workv1alpha2.ResourceUnknown},
},
hasScheduled: true,
},
},
want: &workv1alpha2.GracefulEvictionTask{
@ -139,66 +137,13 @@ func Test_assessSingleTask(t *testing.T) {
},
},
{
name: "gracePeriodSeconds is declared in gracefulEvictionTask and timeout is not reached",
args: args{
task: workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
GracePeriodSeconds: ptr.To[int32](30),
CreationTimestamp: &metav1.Time{Time: timeNow.Add(time.Minute * -1)},
},
opt: assessmentOption{
timeout: timeout,
scheduleResult: []workv1alpha2.TargetCluster{
{Name: "memberA"},
},
observedStatus: []workv1alpha2.AggregatedStatusItem{
{ClusterName: "memberA", Health: workv1alpha2.ResourceUnknown},
},
},
},
want: nil,
},
{
name: "gracePeriodSeconds is declared in gracefulEvictionTask and timeout is reached",
args: args{
task: workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
GracePeriodSeconds: ptr.To[int32](120),
CreationTimestamp: &metav1.Time{Time: timeNow.Add(time.Minute * -1)},
},
opt: assessmentOption{
timeout: timeout,
scheduleResult: []workv1alpha2.TargetCluster{
{Name: "memberA"},
},
observedStatus: []workv1alpha2.AggregatedStatusItem{
{ClusterName: "memberA", Health: workv1alpha2.ResourceUnknown},
},
},
},
want: &workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
GracePeriodSeconds: ptr.To[int32](120),
CreationTimestamp: &metav1.Time{Time: timeNow.Add(time.Minute * -1)},
},
},
{
name: "suppressDeletion is declared in gracefulEvictionTask and is true",
name: "suppressDeletion is declared, value is true",
args: args{
task: workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
SuppressDeletion: ptr.To[bool](true),
CreationTimestamp: &metav1.Time{Time: timeNow.Add(time.Minute * -1)},
},
opt: assessmentOption{
timeout: timeout,
scheduleResult: []workv1alpha2.TargetCluster{
{Name: "memberA"},
},
observedStatus: []workv1alpha2.AggregatedStatusItem{
{ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy},
},
},
},
want: &workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
@ -207,22 +152,13 @@ func Test_assessSingleTask(t *testing.T) {
},
},
{
name: "suppressDeletion is declared in gracefulEvictionTask and is false",
name: "suppressDeletion is declared, value is false",
args: args{
task: workv1alpha2.GracefulEvictionTask{
FromCluster: "member1",
SuppressDeletion: ptr.To[bool](false),
CreationTimestamp: &metav1.Time{Time: timeNow.Add(time.Minute * -1)},
},
opt: assessmentOption{
timeout: timeout,
scheduleResult: []workv1alpha2.TargetCluster{
{Name: "memberA"},
},
observedStatus: []workv1alpha2.AggregatedStatusItem{
{ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy},
},
},
},
want: nil,
},
@ -245,6 +181,7 @@ func Test_assessEvictionTasks(t *testing.T) {
observedStatus []workv1alpha2.AggregatedStatusItem
timeout time.Duration
now metav1.Time
hasScheduled bool
}
tests := []struct {
name string
@ -256,17 +193,13 @@ func Test_assessEvictionTasks(t *testing.T) {
name: "tasks without creation timestamp",
args: args{
bindingSpec: workv1alpha2.ResourceBindingSpec{
Clusters: []workv1alpha2.TargetCluster{
{Name: "memberA"},
},
GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{
{FromCluster: "member1"},
{FromCluster: "member2"},
},
},
observedStatus: []workv1alpha2.AggregatedStatusItem{},
timeout: timeout,
now: timeNow,
timeout: timeout,
now: timeNow,
},
wantTask: []workv1alpha2.GracefulEvictionTask{
{
@ -281,7 +214,7 @@ func Test_assessEvictionTasks(t *testing.T) {
wantCluster: nil,
},
{
name: "tasks that do not exceed the timeout should do nothing",
name: "all tasks do not exceed the timeout, but hasScheduled is false, all tasks should do nothing",
args: args{
bindingSpec: workv1alpha2.ResourceBindingSpec{
Clusters: []workv1alpha2.TargetCluster{
@ -301,6 +234,7 @@ func Test_assessEvictionTasks(t *testing.T) {
observedStatus: []workv1alpha2.AggregatedStatusItem{},
timeout: timeout,
now: timeNow,
hasScheduled: false,
},
wantTask: []workv1alpha2.GracefulEvictionTask{
{
@ -315,7 +249,7 @@ func Test_assessEvictionTasks(t *testing.T) {
wantCluster: nil,
},
{
name: "tasks that exceed the timeout should be removed",
name: "task that exceed the timeout should be removed, task do not exceed the timeout should do nothing",
args: args{
bindingSpec: workv1alpha2.ResourceBindingSpec{
Clusters: []workv1alpha2.TargetCluster{
@ -328,16 +262,22 @@ func Test_assessEvictionTasks(t *testing.T) {
},
{
FromCluster: "member2",
CreationTimestamp: &metav1.Time{Time: timeNow.Add(time.Minute * -5)},
CreationTimestamp: &metav1.Time{Time: timeNow.Add(time.Minute * -1)},
},
},
},
observedStatus: []workv1alpha2.AggregatedStatusItem{},
timeout: timeout,
now: timeNow,
hasScheduled: true,
},
wantTask: nil,
wantCluster: []string{"member1", "member2"},
wantTask: []workv1alpha2.GracefulEvictionTask{
{
FromCluster: "member2",
CreationTimestamp: &metav1.Time{Time: timeNow.Add(time.Minute * -1)},
},
},
wantCluster: []string{"member1"},
},
{
name: "mixed tasks",
@ -399,8 +339,9 @@ func Test_assessEvictionTasks(t *testing.T) {
observedStatus: []workv1alpha2.AggregatedStatusItem{
{ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy},
},
timeout: timeout,
now: timeNow,
timeout: timeout,
now: timeNow,
hasScheduled: true,
},
wantTask: []workv1alpha2.GracefulEvictionTask{
{
@ -437,8 +378,9 @@ func Test_assessEvictionTasks(t *testing.T) {
{ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy},
{ClusterName: "memberB", Health: workv1alpha2.ResourceHealthy},
},
timeout: timeout,
now: timeNow,
timeout: timeout,
now: timeNow,
hasScheduled: true,
},
wantTask: nil,
wantCluster: []string{"member1", "member2"},
@ -466,8 +408,9 @@ func Test_assessEvictionTasks(t *testing.T) {
{ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy},
{ClusterName: "memberB", Health: workv1alpha2.ResourceUnhealthy},
},
timeout: timeout,
now: timeNow,
timeout: timeout,
now: timeNow,
hasScheduled: true,
},
wantTask: []workv1alpha2.GracefulEvictionTask{
{
@ -504,8 +447,9 @@ func Test_assessEvictionTasks(t *testing.T) {
{ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy},
{ClusterName: "memberB", Health: workv1alpha2.ResourceUnknown},
},
timeout: timeout,
now: timeNow,
timeout: timeout,
now: timeNow,
hasScheduled: true,
},
wantTask: []workv1alpha2.GracefulEvictionTask{
{
@ -522,7 +466,12 @@ func Test_assessEvictionTasks(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if gotTask, gotCluster := assessEvictionTasks(tt.args.bindingSpec, tt.args.observedStatus, tt.args.timeout, tt.args.now); !reflect.DeepEqual(gotTask, tt.wantTask) || !reflect.DeepEqual(gotCluster, tt.wantCluster) {
if gotTask, gotCluster := assessEvictionTasks(tt.args.bindingSpec.GracefulEvictionTasks, tt.args.now, assessmentOption{
timeout: tt.args.timeout,
scheduleResult: tt.args.bindingSpec.Clusters,
observedStatus: tt.args.observedStatus,
hasScheduled: true,
}); !reflect.DeepEqual(gotTask, tt.wantTask) || !reflect.DeepEqual(gotCluster, tt.wantCluster) {
t.Errorf("assessEvictionTasks() = (%v, %v), want (%v, %v)", gotTask, gotCluster, tt.wantTask, tt.wantCluster)
}
})

19
pkg/controllers/gracefuleviction/rb_graceful_eviction_controller.go
View File

@ -78,7 +78,12 @@ func (c *RBGracefulEvictionController) Reconcile(ctx context.Context, req contro
}
func (c *RBGracefulEvictionController) syncBinding(ctx context.Context, binding *workv1alpha2.ResourceBinding) (time.Duration, error) {
keptTask, evictedCluster := assessEvictionTasks(binding.Spec, binding.Status.AggregatedStatus, c.GracefulEvictionTimeout, metav1.Now())
keptTask, evictedCluster := assessEvictionTasks(binding.Spec.GracefulEvictionTasks, metav1.Now(), assessmentOption{
timeout: c.GracefulEvictionTimeout,
scheduleResult: binding.Spec.Clusters,
observedStatus: binding.Status.AggregatedStatus,
hasScheduled: binding.Status.SchedulerObservedGeneration == binding.Generation,
})
if reflect.DeepEqual(binding.Spec.GracefulEvictionTasks, keptTask) {
return nextRetry(keptTask, c.GracefulEvictionTimeout, metav1.Now().Time), nil
}
@ -104,21 +109,13 @@ func (c *RBGracefulEvictionController) SetupWithManager(mgr controllerruntime.Ma
resourceBindingPredicateFn := predicate.Funcs{
CreateFunc: func(createEvent event.CreateEvent) bool {
newObj := createEvent.Object.(*workv1alpha2.ResourceBinding)
if len(newObj.Spec.GracefulEvictionTasks) == 0 {
return false
}
// When the current component is restarted and there are still tasks in the
// GracefulEvictionTasks queue, we need to continue the procession.
return newObj.Status.SchedulerObservedGeneration == newObj.Generation
return len(newObj.Spec.GracefulEvictionTasks) != 0
},
UpdateFunc: func(updateEvent event.UpdateEvent) bool {
newObj := updateEvent.ObjectNew.(*workv1alpha2.ResourceBinding)
if len(newObj.Spec.GracefulEvictionTasks) == 0 {
return false
}
return newObj.Status.SchedulerObservedGeneration == newObj.Generation
return len(newObj.Spec.GracefulEvictionTasks) != 0
},
DeleteFunc: func(event.DeleteEvent) bool { return false },
GenericFunc: func(event.GenericEvent) bool { return false },

5
pkg/controllers/gracefuleviction/rb_graceful_eviction_controller_test.go
View File

@ -69,8 +69,9 @@ func TestRBGracefulEvictionController_Reconcile(t *testing.T) {
name: "binding with active graceful eviction tasks",
binding: &workv1alpha2.ResourceBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "test-binding",
Namespace: "default",
Name: "test-binding",
Namespace: "default",
Generation: 1,
},
Spec: workv1alpha2.ResourceBindingSpec{
GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{