diff --git a/pkg/controllers/gracefuleviction/crb_graceful_eviction_controller.go b/pkg/controllers/gracefuleviction/crb_graceful_eviction_controller.go index 0b6f856956cf..5b4a13d1d259 100644 --- a/pkg/controllers/gracefuleviction/crb_graceful_eviction_controller.go +++ b/pkg/controllers/gracefuleviction/crb_graceful_eviction_controller.go @@ -83,17 +83,12 @@ func (c *CRBGracefulEvictionController) SetupWithManager(mgr controllerruntime.M CreateFunc: func(createEvent event.CreateEvent) bool { return false }, UpdateFunc: func(updateEvent event.UpdateEvent) bool { newObj := updateEvent.ObjectNew.(*workv1alpha2.ClusterResourceBinding) - oldObj := updateEvent.ObjectOld.(*workv1alpha2.ClusterResourceBinding) if len(newObj.Spec.GracefulEvictionTasks) == 0 { return false } - if newObj.Status.SchedulerObservedGeneration != newObj.Generation { - return false - } - - return !reflect.DeepEqual(newObj.Spec.GracefulEvictionTasks, oldObj.Spec.GracefulEvictionTasks) + return newObj.Status.SchedulerObservedGeneration == newObj.Generation }, DeleteFunc: func(deleteEvent event.DeleteEvent) bool { return false }, GenericFunc: func(genericEvent event.GenericEvent) bool { return false }, diff --git a/pkg/controllers/gracefuleviction/evictiontask.go b/pkg/controllers/gracefuleviction/evictiontask.go index 5d1fa0cbccee..9fdadcd4db6d 100644 --- a/pkg/controllers/gracefuleviction/evictiontask.go +++ b/pkg/controllers/gracefuleviction/evictiontask.go @@ -45,16 +45,44 @@ func assessEvictionTasks(bindingSpec workv1alpha2.ResourceBindingSpec, } func assessSingleTask(task workv1alpha2.GracefulEvictionTask, opt assessmentOption) *workv1alpha2.GracefulEvictionTask { - // TODO(): gradually evict replica as per observed status. - // task exceeds timeout if metav1.Now().After(task.CreationTimestamp.Add(opt.timeout)) { return nil } + if allScheduledResourceInHealthyState(opt) { + return nil + } + return &task } +func allScheduledResourceInHealthyState(opt assessmentOption) bool { + for _, targetCluster := range opt.scheduleResult { + var statusItem *workv1alpha2.AggregatedStatusItem + + // find the observed status of targetCluster + for index, aggregatedStatus := range opt.observedStatus { + if aggregatedStatus.ClusterName == targetCluster.Name { + statusItem = &opt.observedStatus[index] + break + } + } + + // no observed status found, maybe the resource hasn't been applied + if statusItem == nil { + return false + } + + // resource not in healthy state + if statusItem.Health != workv1alpha2.ResourceHealthy { + return false + } + } + + return true +} + func nextRetry(tasks []workv1alpha2.GracefulEvictionTask, timeout time.Duration, timeNow time.Time) time.Duration { if len(tasks) == 0 { return 0 diff --git a/pkg/controllers/gracefuleviction/evictiontask_test.go b/pkg/controllers/gracefuleviction/evictiontask_test.go index 8637b5eb7a54..df2dc1d8b757 100644 --- a/pkg/controllers/gracefuleviction/evictiontask_test.go +++ b/pkg/controllers/gracefuleviction/evictiontask_test.go @@ -32,6 +32,9 @@ func Test_assessSingleTask(t *testing.T) { }, opt: assessmentOption{ timeout: timeout, + scheduleResult: []workv1alpha2.TargetCluster{ + {Name: "memberA"}, + }, }, }, want: &workv1alpha2.GracefulEvictionTask{ @@ -48,10 +51,76 @@ 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", + 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"}, + }, + observedStatus: []workv1alpha2.AggregatedStatusItem{ + {ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy}, + }, + }, + }, + want: nil, + }, + { + name: "binding scheduled result is unhealthy, task has no effect", + 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"}, + }, + observedStatus: []workv1alpha2.AggregatedStatusItem{ + {ClusterName: "memberA", Health: workv1alpha2.ResourceUnhealthy}, + }, + }, + }, + want: &workv1alpha2.GracefulEvictionTask{ + FromCluster: "member1", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -1)}, + }, + }, + { + name: "binding scheduled result is unknown, task has no effect", + 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"}, + }, + observedStatus: []workv1alpha2.AggregatedStatusItem{ + {ClusterName: "memberA", Health: workv1alpha2.ResourceUnknown}, + }, + }, + }, + want: &workv1alpha2.GracefulEvictionTask{ + FromCluster: "member1", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -1)}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -81,6 +150,9 @@ 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"}, @@ -105,6 +177,9 @@ func Test_assessEvictionTasks(t *testing.T) { name: "tasks that do not exceed the timeout should do nothing", args: args{ bindingSpec: workv1alpha2.ResourceBindingSpec{ + Clusters: []workv1alpha2.TargetCluster{ + {Name: "memberA"}, + }, GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{ { FromCluster: "member1", @@ -135,6 +210,9 @@ func Test_assessEvictionTasks(t *testing.T) { name: "tasks that exceed the timeout should be removed", args: args{ bindingSpec: workv1alpha2.ResourceBindingSpec{ + Clusters: []workv1alpha2.TargetCluster{ + {Name: "memberA"}, + }, GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{ { FromCluster: "member1", @@ -156,6 +234,10 @@ func Test_assessEvictionTasks(t *testing.T) { name: "mixed tasks", args: args{ bindingSpec: workv1alpha2.ResourceBindingSpec{ + Clusters: []workv1alpha2.TargetCluster{ + {Name: "memberA"}, + {Name: "memberB"}, + }, GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{ { FromCluster: "member1", @@ -185,6 +267,144 @@ func Test_assessEvictionTasks(t *testing.T) { }, }, }, + { + name: "tasks that do not exceed the timeout and someone binding scheduled result is missing, should do nothing", + args: args{ + bindingSpec: workv1alpha2.ResourceBindingSpec{ + Clusters: []workv1alpha2.TargetCluster{ + {Name: "memberA"}, + {Name: "memberB"}, + }, + GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{ + { + FromCluster: "member1", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -1)}, + }, + { + FromCluster: "member2", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -2)}, + }, + }, + }, + observedStatus: []workv1alpha2.AggregatedStatusItem{ + {ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy}, + }, + timeout: timeout, + now: timeNow, + }, + want: []workv1alpha2.GracefulEvictionTask{ + { + FromCluster: "member1", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -1)}, + }, + { + FromCluster: "member2", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -2)}, + }, + }, + }, + { + name: "tasks that do not exceed the timeout and binding scheduled result is healthy, tasks need to be removed", + args: args{ + bindingSpec: workv1alpha2.ResourceBindingSpec{ + Clusters: []workv1alpha2.TargetCluster{ + {Name: "memberA"}, + {Name: "memberB"}, + }, + GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{ + { + FromCluster: "member1", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -1)}, + }, + { + FromCluster: "member2", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -2)}, + }, + }, + }, + observedStatus: []workv1alpha2.AggregatedStatusItem{ + {ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy}, + {ClusterName: "memberB", Health: workv1alpha2.ResourceHealthy}, + }, + timeout: timeout, + now: timeNow, + }, + want: nil, + }, + { + name: "tasks that do not exceed the timeout and someone binding scheduled result is unhealthy, should do nothing", + args: args{ + bindingSpec: workv1alpha2.ResourceBindingSpec{ + Clusters: []workv1alpha2.TargetCluster{ + {Name: "memberA"}, + {Name: "memberB"}, + }, + GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{ + { + FromCluster: "member1", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -1)}, + }, + { + FromCluster: "member2", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -2)}, + }, + }, + }, + observedStatus: []workv1alpha2.AggregatedStatusItem{ + {ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy}, + {ClusterName: "memberB", Health: workv1alpha2.ResourceUnhealthy}, + }, + timeout: timeout, + now: timeNow, + }, + want: []workv1alpha2.GracefulEvictionTask{ + { + FromCluster: "member1", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -1)}, + }, + { + FromCluster: "member2", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -2)}, + }, + }, + }, + { + name: "tasks that do not exceed the timeout and someone binding scheduled result is unknown, should do nothing", + args: args{ + bindingSpec: workv1alpha2.ResourceBindingSpec{ + Clusters: []workv1alpha2.TargetCluster{ + {Name: "memberA"}, + {Name: "memberB"}, + }, + GracefulEvictionTasks: []workv1alpha2.GracefulEvictionTask{ + { + FromCluster: "member1", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -1)}, + }, + { + FromCluster: "member2", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -2)}, + }, + }, + }, + observedStatus: []workv1alpha2.AggregatedStatusItem{ + {ClusterName: "memberA", Health: workv1alpha2.ResourceHealthy}, + {ClusterName: "memberB", Health: workv1alpha2.ResourceUnknown}, + }, + timeout: timeout, + now: timeNow, + }, + want: []workv1alpha2.GracefulEvictionTask{ + { + FromCluster: "member1", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -1)}, + }, + { + FromCluster: "member2", + CreationTimestamp: metav1.Time{Time: timeNow.Add(time.Minute * -2)}, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/controllers/gracefuleviction/rb_graceful_eviction_controller.go b/pkg/controllers/gracefuleviction/rb_graceful_eviction_controller.go index ff683bec67c7..f48f727d99a3 100644 --- a/pkg/controllers/gracefuleviction/rb_graceful_eviction_controller.go +++ b/pkg/controllers/gracefuleviction/rb_graceful_eviction_controller.go @@ -83,17 +83,12 @@ func (c *RBGracefulEvictionController) SetupWithManager(mgr controllerruntime.Ma CreateFunc: func(createEvent event.CreateEvent) bool { return false }, UpdateFunc: func(updateEvent event.UpdateEvent) bool { newObj := updateEvent.ObjectNew.(*workv1alpha2.ResourceBinding) - oldObj := updateEvent.ObjectOld.(*workv1alpha2.ResourceBinding) if len(newObj.Spec.GracefulEvictionTasks) == 0 { return false } - if newObj.Status.SchedulerObservedGeneration != newObj.Generation { - return false - } - - return !reflect.DeepEqual(newObj.Spec.GracefulEvictionTasks, oldObj.Spec.GracefulEvictionTasks) + return newObj.Status.SchedulerObservedGeneration == newObj.Generation }, DeleteFunc: func(deleteEvent event.DeleteEvent) bool { return false }, GenericFunc: func(genericEvent event.GenericEvent) bool { return false },