From b6c7320f70778ceaa2fa545700b7c62a94fc7f4c Mon Sep 17 00:00:00 2001 From: changzhen Date: Fri, 5 Aug 2022 16:24:22 +0800 Subject: [PATCH] make changes to binding-controller to adopt graceful eviction Signed-off-by: changzhen --- pkg/controllers/binding/binding_controller.go | 35 +++++++++++------- .../cluster_resource_binding_controller.go | 31 ++++++++++------ pkg/util/helper/binding.go | 37 ++++++++----------- pkg/util/helper/workstatus.go | 15 +++----- pkg/util/helper/workstatus_test.go | 14 ++++--- 5 files changed, 68 insertions(+), 64 deletions(-) diff --git a/pkg/controllers/binding/binding_controller.go b/pkg/controllers/binding/binding_controller.go index e3c904674b53..a7b085069e4f 100644 --- a/pkg/controllers/binding/binding_controller.go +++ b/pkg/controllers/binding/binding_controller.go @@ -96,20 +96,7 @@ func (c *ResourceBindingController) removeFinalizer(rb *workv1alpha2.ResourceBin // syncBinding will sync resourceBinding to Works. func (c *ResourceBindingController) syncBinding(binding *workv1alpha2.ResourceBinding) (controllerruntime.Result, error) { - clusterNames := helper.GetBindingClusterNames(binding.Spec.Clusters, binding.Spec.RequiredBy) - works, err := helper.FindOrphanWorks(c.Client, binding.Namespace, binding.Name, clusterNames, apiextensionsv1.NamespaceScoped) - if err != nil { - klog.Errorf("Failed to find orphan works by resourceBinding(%s/%s). Error: %v.", - binding.GetNamespace(), binding.GetName(), err) - c.EventRecorder.Event(binding, corev1.EventTypeWarning, workv1alpha2.EventReasonCleanupWorkFailed, err.Error()) - return controllerruntime.Result{Requeue: true}, err - } - - err = helper.RemoveOrphanWorks(c.Client, works) - if err != nil { - klog.Errorf("Failed to remove orphan works by resourceBinding(%s/%s). Error: %v.", - binding.GetNamespace(), binding.GetName(), err) - c.EventRecorder.Event(binding, corev1.EventTypeWarning, workv1alpha2.EventReasonCleanupWorkFailed, err.Error()) + if err := c.removeOrphanWorks(binding); err != nil { return controllerruntime.Result{Requeue: true}, err } @@ -164,6 +151,26 @@ func (c *ResourceBindingController) syncBinding(binding *workv1alpha2.ResourceBi return controllerruntime.Result{}, nil } +func (c *ResourceBindingController) removeOrphanWorks(binding *workv1alpha2.ResourceBinding) error { + works, err := helper.FindOrphanWorks(c.Client, binding.Namespace, binding.Name, helper.ObtainBindingSpecExistingClusters(binding.Spec)) + if err != nil { + klog.Errorf("Failed to find orphan works by resourceBinding(%s/%s). Error: %v.", + binding.GetNamespace(), binding.GetName(), err) + c.EventRecorder.Event(binding, corev1.EventTypeWarning, workv1alpha2.EventReasonCleanupWorkFailed, err.Error()) + return err + } + + err = helper.RemoveOrphanWorks(c.Client, works) + if err != nil { + klog.Errorf("Failed to remove orphan works by resourceBinding(%s/%s). Error: %v.", + binding.GetNamespace(), binding.GetName(), err) + c.EventRecorder.Event(binding, corev1.EventTypeWarning, workv1alpha2.EventReasonCleanupWorkFailed, err.Error()) + return err + } + + return nil +} + // updateResourceStatus will try to calculate the summary status and update to original object // that the ResourceBinding refer to. func (c *ResourceBindingController) updateResourceStatus(binding *workv1alpha2.ResourceBinding) error { diff --git a/pkg/controllers/binding/cluster_resource_binding_controller.go b/pkg/controllers/binding/cluster_resource_binding_controller.go index 93716502d4f7..75f9f6740cd4 100644 --- a/pkg/controllers/binding/cluster_resource_binding_controller.go +++ b/pkg/controllers/binding/cluster_resource_binding_controller.go @@ -91,18 +91,7 @@ func (c *ClusterResourceBindingController) removeFinalizer(crb *workv1alpha2.Clu // syncBinding will sync clusterResourceBinding to Works. func (c *ClusterResourceBindingController) syncBinding(binding *workv1alpha2.ClusterResourceBinding) (controllerruntime.Result, error) { - clusterNames := helper.GetBindingClusterNames(binding.Spec.Clusters, binding.Spec.RequiredBy) - works, err := helper.FindOrphanWorks(c.Client, "", binding.Name, clusterNames, apiextensionsv1.ClusterScoped) - if err != nil { - klog.Errorf("Failed to find orphan works by ClusterResourceBinding(%s). Error: %v.", binding.GetName(), err) - c.EventRecorder.Event(binding, corev1.EventTypeWarning, workv1alpha2.EventReasonCleanupWorkFailed, err.Error()) - return controllerruntime.Result{Requeue: true}, err - } - - err = helper.RemoveOrphanWorks(c.Client, works) - if err != nil { - klog.Errorf("Failed to remove orphan works by clusterResourceBinding(%s). Error: %v.", binding.GetName(), err) - c.EventRecorder.Event(binding, corev1.EventTypeWarning, workv1alpha2.EventReasonCleanupWorkFailed, err.Error()) + if err := c.removeOrphanWorks(binding); err != nil { return controllerruntime.Result{Requeue: true}, err } @@ -145,6 +134,24 @@ func (c *ClusterResourceBindingController) syncBinding(binding *workv1alpha2.Clu return controllerruntime.Result{}, nil } +func (c *ClusterResourceBindingController) removeOrphanWorks(binding *workv1alpha2.ClusterResourceBinding) error { + works, err := helper.FindOrphanWorks(c.Client, "", binding.Name, helper.ObtainBindingSpecExistingClusters(binding.Spec)) + if err != nil { + klog.Errorf("Failed to find orphan works by ClusterResourceBinding(%s). Error: %v.", binding.GetName(), err) + c.EventRecorder.Event(binding, corev1.EventTypeWarning, workv1alpha2.EventReasonCleanupWorkFailed, err.Error()) + return err + } + + err = helper.RemoveOrphanWorks(c.Client, works) + if err != nil { + klog.Errorf("Failed to remove orphan works by clusterResourceBinding(%s). Error: %v.", binding.GetName(), err) + c.EventRecorder.Event(binding, corev1.EventTypeWarning, workv1alpha2.EventReasonCleanupWorkFailed, err.Error()) + return err + } + + return nil +} + // SetupWithManager creates a controller and register to controller manager. func (c *ClusterResourceBindingController) SetupWithManager(mgr controllerruntime.Manager) error { workFn := handler.MapFunc( diff --git a/pkg/util/helper/binding.go b/pkg/util/helper/binding.go index 702b175ba4dc..81d29102063a 100644 --- a/pkg/util/helper/binding.go +++ b/pkg/util/helper/binding.go @@ -5,7 +5,6 @@ import ( "sort" corev1 "k8s.io/api/core/v1" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -72,40 +71,34 @@ func HasScheduledReplica(scheduleResult []workv1alpha2.TargetCluster) bool { return false } -// GetBindingClusterNames will get clusterName list from bind clusters field and requiredBy field. -func GetBindingClusterNames(targetClusters []workv1alpha2.TargetCluster, bindingSnapshot []workv1alpha2.BindingSnapshot) []string { - clusterNames := util.ConvertToClusterNames(targetClusters) - for _, binding := range bindingSnapshot { +// ObtainBindingSpecExistingClusters will obtain the cluster slice existing in the binding's spec field. +func ObtainBindingSpecExistingClusters(bindingSpec workv1alpha2.ResourceBindingSpec) sets.String { + clusterNames := util.ConvertToClusterNames(bindingSpec.Clusters) + for _, binding := range bindingSpec.RequiredBy { for _, targetCluster := range binding.Clusters { clusterNames.Insert(targetCluster.Name) } } - return clusterNames.List() + for _, task := range bindingSpec.GracefulEvictionTasks { + clusterNames.Insert(task.FromCluster) + } + + return clusterNames } // FindOrphanWorks retrieves all works that labeled with current binding(ResourceBinding or ClusterResourceBinding) objects, // then pick the works that not meet current binding declaration. -func FindOrphanWorks(c client.Client, bindingNamespace, bindingName string, clusterNames []string, scope apiextensionsv1.ResourceScope) ([]workv1alpha1.Work, error) { +func FindOrphanWorks(c client.Client, bindingNamespace, bindingName string, expectClusters sets.String) ([]workv1alpha1.Work, error) { var needJudgeWorks []workv1alpha1.Work - if scope == apiextensionsv1.NamespaceScoped { - workList, err := GetWorksByBindingNamespaceName(c, bindingNamespace, bindingName) - if err != nil { - klog.Errorf("Failed to get works by ResourceBinding(%s/%s): %v", bindingNamespace, bindingName, err) - return nil, err - } - needJudgeWorks = append(needJudgeWorks, workList.Items...) - } else { - workList, err := GetWorksByBindingNamespaceName(c, "", bindingName) - if err != nil { - klog.Errorf("Failed to get works by ClusterResourceBinding(%s): %v", bindingName, err) - return nil, err - } - needJudgeWorks = append(needJudgeWorks, workList.Items...) + workList, err := GetWorksByBindingNamespaceName(c, bindingNamespace, bindingName) + if err != nil { + klog.Errorf("Failed to get works by binding object (%s/%s): %v", bindingNamespace, bindingName, err) + return nil, err } + needJudgeWorks = append(needJudgeWorks, workList.Items...) var orphanWorks []workv1alpha1.Work - expectClusters := sets.NewString(clusterNames...) for _, work := range needJudgeWorks { workTargetCluster, err := names.GetClusterName(work.GetNamespace()) if err != nil { diff --git a/pkg/util/helper/workstatus.go b/pkg/util/helper/workstatus.go index 6a387e9f99a6..c55a5f605a4d 100644 --- a/pkg/util/helper/workstatus.go +++ b/pkg/util/helper/workstatus.go @@ -120,7 +120,7 @@ func AggregateClusterResourceBindingWorkStatus(c client.Client, binding *workv1a } func generateFullyAppliedCondition(spec workv1alpha2.ResourceBindingSpec, aggregatedStatuses []workv1alpha2.AggregatedStatusItem) metav1.Condition { - clusterNames := GetBindingClusterNames(spec.Clusters, spec.RequiredBy) + clusterNames := ObtainBindingSpecExistingClusters(spec) if worksFullyApplied(aggregatedStatuses, clusterNames) { return util.NewCondition(workv1alpha2.FullyApplied, FullyAppliedSuccessReason, FullyAppliedSuccessMessage, metav1.ConditionTrue) } @@ -235,7 +235,7 @@ func equalIdentifier(targetIdentifier *workv1alpha1.ResourceIdentifier, ordinal } // worksFullyApplied checks if all works are applied according the scheduled result and collected status. -func worksFullyApplied(aggregatedStatuses []workv1alpha2.AggregatedStatusItem, targetClusters []string) bool { +func worksFullyApplied(aggregatedStatuses []workv1alpha2.AggregatedStatusItem, targetClusters sets.String) bool { // short path: not scheduled if len(targetClusters) == 0 { return false @@ -246,17 +246,12 @@ func worksFullyApplied(aggregatedStatuses []workv1alpha2.AggregatedStatusItem, t return false } - targetClusterSet := sets.String{} - for i := range targetClusters { - targetClusterSet.Insert(targetClusters[i]) - } - - for _, aggregatedSatusItem := range aggregatedStatuses { - if !aggregatedSatusItem.Applied { + for _, aggregatedStatusItem := range aggregatedStatuses { + if !aggregatedStatusItem.Applied { return false } - if !targetClusterSet.Has(aggregatedSatusItem.ClusterName) { + if !targetClusters.Has(aggregatedStatusItem.ClusterName) { return false } } diff --git a/pkg/util/helper/workstatus_test.go b/pkg/util/helper/workstatus_test.go index f991e3e4ea4f..33e8857a0846 100644 --- a/pkg/util/helper/workstatus_test.go +++ b/pkg/util/helper/workstatus_test.go @@ -3,13 +3,15 @@ package helper import ( "testing" + "k8s.io/apimachinery/pkg/util/sets" + workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2" ) func TestWorksFullyApplied(t *testing.T) { type args struct { aggregatedStatuses []workv1alpha2.AggregatedStatusItem - targetClusters []string + targetClusters sets.String } tests := []struct { name string @@ -33,7 +35,7 @@ func TestWorksFullyApplied(t *testing.T) { name: "no aggregatedStatuses", args: args{ aggregatedStatuses: nil, - targetClusters: []string{"member1"}, + targetClusters: sets.NewString("member1"), }, want: false, }, @@ -46,7 +48,7 @@ func TestWorksFullyApplied(t *testing.T) { Applied: true, }, }, - targetClusters: []string{"member1", "member2"}, + targetClusters: sets.NewString("member1", "member2"), }, want: false, }, @@ -63,7 +65,7 @@ func TestWorksFullyApplied(t *testing.T) { Applied: true, }, }, - targetClusters: []string{"member1", "member2"}, + targetClusters: sets.NewString("member1", "member2"), }, want: true, }, @@ -80,7 +82,7 @@ func TestWorksFullyApplied(t *testing.T) { Applied: false, }, }, - targetClusters: []string{"member1", "member2"}, + targetClusters: sets.NewString("member1", "member2"), }, want: false, }, @@ -93,7 +95,7 @@ func TestWorksFullyApplied(t *testing.T) { Applied: true, }, }, - targetClusters: []string{"member2"}, + targetClusters: sets.NewString("member2"), }, want: false, },