From cc9a6da92cef7f1d7afd8ee1cfad56a5229ab9ca Mon Sep 17 00:00:00 2001 From: changzhen Date: Wed, 22 Sep 2021 17:03:20 +0800 Subject: [PATCH] move binding's namespace/name from work's label to annotation Signed-off-by: changzhen --- pkg/apis/work/v1alpha2/well_known_labels.go | 10 +++ pkg/controllers/binding/binding_controller.go | 36 +++++---- .../cluster_resource_binding_controller.go | 29 +++++--- pkg/controllers/binding/common.go | 35 ++++++--- pkg/util/helper/binding.go | 73 +++++++++++++++++-- pkg/util/helper/work.go | 12 +++ pkg/util/helper/workstatus.go | 69 ++++++++++++++---- pkg/util/names/names.go | 13 ++++ 8 files changed, 222 insertions(+), 55 deletions(-) diff --git a/pkg/apis/work/v1alpha2/well_known_labels.go b/pkg/apis/work/v1alpha2/well_known_labels.go index 40c242f335d7..a96a4d71c250 100644 --- a/pkg/apis/work/v1alpha2/well_known_labels.go +++ b/pkg/apis/work/v1alpha2/well_known_labels.go @@ -1,6 +1,16 @@ package v1alpha2 const ( + // ResourceBindingReferenceKey is the key of ResourceBinding object. + // It is usually a unique hash value of ResourceBinding object's namespace and name, intended to be added to the Work object. + // It will be used to retrieve all Works objects that derived from a specific ResourceBinding object. + ResourceBindingReferenceKey = "resourcebinding.karmada.io/key" + + // ClusterResourceBindingReferenceKey is the key of ClusterResourceBinding object. + // It is usually a unique hash value of ClusterResourceBinding object's namespace and name, intended to be added to the Work object. + // It will be used to retrieve all Works objects that derived by a specific ClusterResourceBinding object. + ClusterResourceBindingReferenceKey = "clusterresourcebinding.karmada.io/key" + // ResourceBindingNamespaceLabel is added to objects to specify associated ResourceBinding's namespace. ResourceBindingNamespaceLabel = "resourcebinding.karmada.io/namespace" diff --git a/pkg/controllers/binding/binding_controller.go b/pkg/controllers/binding/binding_controller.go index d20f44cd89da..0dfd69689c86 100644 --- a/pkg/controllers/binding/binding_controller.go +++ b/pkg/controllers/binding/binding_controller.go @@ -6,7 +6,6 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" "k8s.io/client-go/tools/record" @@ -55,10 +54,8 @@ func (c *ResourceBindingController) Reconcile(ctx context.Context, req controlle } if !binding.DeletionTimestamp.IsZero() { - if err := helper.DeleteWorks(c.Client, labels.Set{ - workv1alpha2.ResourceBindingNamespaceLabel: req.Namespace, - workv1alpha2.ResourceBindingNameLabel: req.Name, - }); err != nil { + klog.V(4).Infof("Begin to delete works owned by binding(%s).", req.NamespacedName.String()) + if err := helper.DeleteWorkByRBNamespaceAndName(c.Client, req.Namespace, req.Name); err != nil { klog.Errorf("Failed to delete works related to %s/%s: %v", binding.GetNamespace(), binding.GetName(), err) return controllerruntime.Result{Requeue: true}, err } @@ -136,18 +133,31 @@ func (c *ResourceBindingController) SetupWithManager(mgr controllerruntime.Manag func(a client.Object) []reconcile.Request { var requests []reconcile.Request + // TODO: Delete this logic in the next release to prevent incompatibility when upgrading the current release (v0.9.0). labels := a.GetLabels() - resourcebindingNamespace, namespaceExist := labels[workv1alpha2.ResourceBindingNamespaceLabel] - resourcebindingName, nameExist := labels[workv1alpha2.ResourceBindingNameLabel] - if !namespaceExist || !nameExist { - return nil + crNamespace, namespaceExist := labels[workv1alpha2.ResourceBindingNamespaceLabel] + crName, nameExist := labels[workv1alpha2.ResourceBindingNameLabel] + if namespaceExist && nameExist { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: crNamespace, + Name: crName, + }, + }) } - namespacesName := types.NamespacedName{ - Namespace: resourcebindingNamespace, - Name: resourcebindingName, + + annotations := a.GetAnnotations() + crNamespace, namespaceExist = annotations[workv1alpha2.ResourceBindingNamespaceLabel] + crName, nameExist = annotations[workv1alpha2.ResourceBindingNameLabel] + if namespaceExist && nameExist { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: crNamespace, + Name: crName, + }, + }) } - requests = append(requests, reconcile.Request{NamespacedName: namespacesName}) return requests }) diff --git a/pkg/controllers/binding/cluster_resource_binding_controller.go b/pkg/controllers/binding/cluster_resource_binding_controller.go index 1d975887942d..5e2915dddc04 100644 --- a/pkg/controllers/binding/cluster_resource_binding_controller.go +++ b/pkg/controllers/binding/cluster_resource_binding_controller.go @@ -6,7 +6,6 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" "k8s.io/client-go/tools/record" @@ -55,9 +54,8 @@ func (c *ClusterResourceBindingController) Reconcile(ctx context.Context, req co } if !clusterResourceBinding.DeletionTimestamp.IsZero() { - if err := helper.DeleteWorks(c.Client, labels.Set{ - workv1alpha2.ClusterResourceBindingLabel: req.Name, - }); err != nil { + klog.V(4).Infof("Begin to delete works owned by binding(%s).", req.NamespacedName.String()) + if err := helper.DeleteWorkByCRBName(c.Client, req.Name); err != nil { klog.Errorf("Failed to delete works related to %s: %v", clusterResourceBinding.GetName(), err) return controllerruntime.Result{Requeue: true}, err } @@ -130,16 +128,27 @@ func (c *ClusterResourceBindingController) SetupWithManager(mgr controllerruntim func(a client.Object) []reconcile.Request { var requests []reconcile.Request + // TODO: Delete this logic in the next release to prevent incompatibility when upgrading the current release (v0.9.0). labels := a.GetLabels() - clusterResourcebindingName, nameExist := labels[workv1alpha2.ClusterResourceBindingLabel] - if !nameExist { - return nil + crbName, nameExist := labels[workv1alpha2.ClusterResourceBindingLabel] + if nameExist { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: crbName, + }, + }) } - namespacesName := types.NamespacedName{ - Name: clusterResourcebindingName, + + annotations := a.GetAnnotations() + crbName, nameExist = annotations[workv1alpha2.ClusterResourceBindingLabel] + if nameExist { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: crbName, + }, + }) } - requests = append(requests, reconcile.Request{NamespacedName: namespacesName}) return requests }) diff --git a/pkg/controllers/binding/common.go b/pkg/controllers/binding/common.go index be8dabb4d2b0..6af7cae14940 100644 --- a/pkg/controllers/binding/common.go +++ b/pkg/controllers/binding/common.go @@ -97,7 +97,8 @@ func ensureWork(c client.Client, workload *unstructured.Unstructured, overrideMa } } - annotations, err := recordAppliedOverrides(cops, ops) + annotations := mergeAnnotations(clonedWorkload, binding, scope) + annotations, err = recordAppliedOverrides(cops, ops, annotations) if err != nil { klog.Errorf("failed to record appliedOverrides, Error: %v", err) return err @@ -150,22 +151,36 @@ func mergeLabel(workload *unstructured.Unstructured, workNamespace string, bindi var workLabel = make(map[string]string) util.MergeLabel(workload, workv1alpha2.WorkNamespaceLabel, workNamespace) util.MergeLabel(workload, workv1alpha2.WorkNameLabel, names.GenerateWorkName(workload.GetKind(), workload.GetName(), workload.GetNamespace())) - if scope == apiextensionsv1.NamespaceScoped { - util.MergeLabel(workload, workv1alpha2.ResourceBindingNamespaceLabel, binding.GetNamespace()) - util.MergeLabel(workload, workv1alpha2.ResourceBindingNameLabel, binding.GetName()) - workLabel[workv1alpha2.ResourceBindingNamespaceLabel] = binding.GetNamespace() - workLabel[workv1alpha2.ResourceBindingNameLabel] = binding.GetName() + util.MergeLabel(workload, workv1alpha2.ResourceBindingReferenceKey, names.GenerateBindingReferenceKey(binding.GetNamespace(), binding.GetName())) + workLabel[workv1alpha2.ResourceBindingReferenceKey] = names.GenerateBindingReferenceKey(binding.GetNamespace(), binding.GetName()) } else { - util.MergeLabel(workload, workv1alpha2.ClusterResourceBindingLabel, binding.GetName()) - workLabel[workv1alpha2.ClusterResourceBindingLabel] = binding.GetName() + util.MergeLabel(workload, workv1alpha2.ClusterResourceBindingReferenceKey, names.GenerateBindingReferenceKey("", binding.GetName())) + workLabel[workv1alpha2.ClusterResourceBindingReferenceKey] = names.GenerateBindingReferenceKey("", binding.GetName()) } - return workLabel } -func recordAppliedOverrides(cops *overridemanager.AppliedOverrides, ops *overridemanager.AppliedOverrides) (map[string]string, error) { +func mergeAnnotations(workload *unstructured.Unstructured, binding metav1.Object, scope apiextensionsv1.ResourceScope) map[string]string { annotations := make(map[string]string) + if scope == apiextensionsv1.NamespaceScoped { + util.MergeAnnotation(workload, workv1alpha2.ResourceBindingNamespaceLabel, binding.GetNamespace()) + util.MergeAnnotation(workload, workv1alpha2.ResourceBindingNameLabel, binding.GetName()) + annotations[workv1alpha2.ResourceBindingNamespaceLabel] = binding.GetNamespace() + annotations[workv1alpha2.ResourceBindingNameLabel] = binding.GetName() + } else { + util.MergeAnnotation(workload, workv1alpha2.ClusterResourceBindingLabel, binding.GetName()) + annotations[workv1alpha2.ClusterResourceBindingLabel] = binding.GetName() + } + + return annotations +} + +func recordAppliedOverrides(cops *overridemanager.AppliedOverrides, ops *overridemanager.AppliedOverrides, + annotations map[string]string) (map[string]string, error) { + if annotations == nil { + annotations = make(map[string]string) + } if cops != nil { appliedBytes, err := cops.MarshalJSON() diff --git a/pkg/util/helper/binding.go b/pkg/util/helper/binding.go index 1e6df632fd98..bc93a7913e07 100644 --- a/pkg/util/helper/binding.go +++ b/pkg/util/helper/binding.go @@ -6,6 +6,7 @@ import ( 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" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -80,29 +81,51 @@ func GetBindingClusterNames(targetClusters []workv1alpha2.TargetCluster) []strin // 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) { - workList := &workv1alpha1.WorkList{} + var needJudgeWorks []workv1alpha1.Work if scope == apiextensionsv1.NamespaceScoped { - selector := labels.SelectorFromSet(labels.Set{ + // TODO: Delete this logic in the next release to prevent incompatibility when upgrading the current release (v0.9.0). + workList, err := GetWorksByLabelSelector(c, labels.SelectorFromSet(labels.Set{ workv1alpha2.ResourceBindingNamespaceLabel: bindingNamespace, workv1alpha2.ResourceBindingNameLabel: 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...) - if err := c.List(context.TODO(), workList, &client.ListOptions{LabelSelector: selector}); err != nil { + workList, err = GetWorksByLabelSelector(c, labels.SelectorFromSet(labels.Set{ + workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey(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 { - selector := labels.SelectorFromSet(labels.Set{ + // TODO: Delete this logic in the next release to prevent incompatibility when upgrading the current release (v0.9.0). + workList, err := GetWorksByLabelSelector(c, labels.SelectorFromSet(labels.Set{ workv1alpha2.ClusterResourceBindingLabel: bindingName, - }) + })) + if err != nil { + klog.Errorf("Failed to get works by ClusterResourceBinding(%s): %v", bindingName, err) + return nil, err + } + needJudgeWorks = append(needJudgeWorks, workList.Items...) - if err := c.List(context.TODO(), workList, &client.ListOptions{LabelSelector: selector}); err != nil { + workList, err = GetWorksByLabelSelector(c, labels.SelectorFromSet(labels.Set{ + workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", bindingName), + })) + if err != nil { + klog.Errorf("Failed to get works by ClusterResourceBinding(%s): %v", bindingName, err) return nil, err } + needJudgeWorks = append(needJudgeWorks, workList.Items...) } var orphanWorks []workv1alpha1.Work expectClusters := sets.NewString(clusterNames...) - for _, work := range workList.Items { + for _, work := range needJudgeWorks { workTargetCluster, err := names.GetClusterName(work.GetNamespace()) if err != nil { klog.Errorf("Failed to get cluster name which Work %s/%s belongs to. Error: %v.", @@ -173,6 +196,37 @@ func GetWorks(c client.Client, ls labels.Set) (*workv1alpha1.WorkList, error) { return works, c.List(context.TODO(), works, listOpt) } +// DeleteWorkByRBNamespaceAndName will delete all Work objects by ResourceBinding namespace and name. +func DeleteWorkByRBNamespaceAndName(c client.Client, namespace, name string) error { + err := DeleteWorks(c, labels.Set{ + workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey(namespace, name), + }) + if err != nil { + return err + } + + // TODO: Delete this logic in the next release to prevent incompatibility when upgrading the current release (v0.9.0). + return DeleteWorks(c, labels.Set{ + workv1alpha2.ResourceBindingNamespaceLabel: namespace, + workv1alpha2.ResourceBindingNameLabel: name, + }) +} + +// DeleteWorkByCRBName will delete all Work objects by ClusterResourceBinding name. +func DeleteWorkByCRBName(c client.Client, name string) error { + err := DeleteWorks(c, labels.Set{ + workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", name), + }) + if err != nil { + return err + } + + // TODO: Delete this logic in the next release to prevent incompatibility when upgrading the current release (v0.9.0). + return DeleteWorks(c, labels.Set{ + workv1alpha2.ClusterResourceBindingLabel: name, + }) +} + // DeleteWorks will delete all Work objects by labels. func DeleteWorks(c client.Client, selector labels.Set) error { workList, err := GetWorks(c, selector) @@ -185,6 +239,9 @@ func DeleteWorks(c client.Client, selector labels.Set) error { for index, work := range workList.Items { if err := c.Delete(context.TODO(), &workList.Items[index]); err != nil { klog.Errorf("Failed to delete work(%s/%s): %v", work.Namespace, work.Name, err) + if apierrors.IsNotFound(err) { + continue + } errs = append(errs, err) } } diff --git a/pkg/util/helper/work.go b/pkg/util/helper/work.go index 29a99f07e601..92592a9db71a 100644 --- a/pkg/util/helper/work.go +++ b/pkg/util/helper/work.go @@ -5,6 +5,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -59,3 +60,14 @@ func CreateOrUpdateWork(client client.Client, workMeta metav1.ObjectMeta, resour return nil } + +// GetWorksByLabelSelector get WorkList by matching label selector. +func GetWorksByLabelSelector(c client.Client, selector labels.Selector) (*workv1alpha1.WorkList, error) { + workList := &workv1alpha1.WorkList{} + err := c.List(context.TODO(), workList, &client.ListOptions{LabelSelector: selector}) + if err != nil { + return nil, err + } + + return workList, nil +} diff --git a/pkg/util/helper/workstatus.go b/pkg/util/helper/workstatus.go index 967ca983d5b2..64dd62481787 100644 --- a/pkg/util/helper/workstatus.go +++ b/pkg/util/helper/workstatus.go @@ -22,10 +22,33 @@ import ( // AggregateResourceBindingWorkStatus will collect all work statuses with current ResourceBinding objects, // then aggregate status info to current ResourceBinding status. func AggregateResourceBindingWorkStatus(c client.Client, binding *workv1alpha2.ResourceBinding, workload *unstructured.Unstructured) error { - aggregatedStatuses, err := assembleWorkStatus(c, labels.SelectorFromSet(labels.Set{ - workv1alpha2.ResourceBindingNamespaceLabel: binding.Namespace, - workv1alpha2.ResourceBindingNameLabel: binding.Name, - }), workload) + var targetWorks []workv1alpha1.Work + + // TODO: Delete this logic in the next release to prevent incompatibility when upgrading the current release (v0.9.0). + workList, err := GetWorksByLabelSelector(c, labels.SelectorFromSet( + labels.Set{ + workv1alpha2.ResourceBindingNamespaceLabel: binding.Namespace, + workv1alpha2.ResourceBindingNameLabel: binding.Name, + }, + )) + if err != nil { + klog.Errorf("Failed to get works by ResourceBinding(%s/%s): %v", binding.Namespace, binding.Name, err) + return err + } + targetWorks = append(targetWorks, workList.Items...) + + workList, err = GetWorksByLabelSelector(c, labels.SelectorFromSet( + labels.Set{ + workv1alpha2.ResourceBindingReferenceKey: names.GenerateBindingReferenceKey(binding.Namespace, binding.Name), + }, + )) + if err != nil { + klog.Errorf("Failed to get works by ResourceBinding(%s/%s): %v", binding.Namespace, binding.Name, err) + return err + } + targetWorks = append(targetWorks, workList.Items...) + + aggregatedStatuses, err := assembleWorkStatus(targetWorks, workload) if err != nil { return err } @@ -47,9 +70,32 @@ func AggregateResourceBindingWorkStatus(c client.Client, binding *workv1alpha2.R // AggregateClusterResourceBindingWorkStatus will collect all work statuses with current ClusterResourceBinding objects, // then aggregate status info to current ClusterResourceBinding status. func AggregateClusterResourceBindingWorkStatus(c client.Client, binding *workv1alpha2.ClusterResourceBinding, workload *unstructured.Unstructured) error { - aggregatedStatuses, err := assembleWorkStatus(c, labels.SelectorFromSet(labels.Set{ - workv1alpha2.ClusterResourceBindingLabel: binding.Name, - }), workload) + var targetWorks []workv1alpha1.Work + + // TODO: Delete this logic in the next release to prevent incompatibility when upgrading the current release (v0.9.0). + workList, err := GetWorksByLabelSelector(c, labels.SelectorFromSet( + labels.Set{ + workv1alpha2.ClusterResourceBindingLabel: binding.Name, + }, + )) + if err != nil { + klog.Errorf("Failed to get works by ClusterResourceBinding(%s): %v", binding.Name, err) + return err + } + targetWorks = append(targetWorks, workList.Items...) + + workList, err = GetWorksByLabelSelector(c, labels.SelectorFromSet( + labels.Set{ + workv1alpha2.ClusterResourceBindingReferenceKey: names.GenerateBindingReferenceKey("", binding.Name), + }, + )) + if err != nil { + klog.Errorf("Failed to get works by ClusterResourceBinding(%s): %v", binding.Name, err) + return err + } + targetWorks = append(targetWorks, workList.Items...) + + aggregatedStatuses, err := assembleWorkStatus(targetWorks, workload) if err != nil { return err } @@ -69,14 +115,9 @@ func AggregateClusterResourceBindingWorkStatus(c client.Client, binding *workv1a } // assemble workStatuses from workList which list by selector and match with workload. -func assembleWorkStatus(c client.Client, selector labels.Selector, workload *unstructured.Unstructured) ([]workv1alpha2.AggregatedStatusItem, error) { - workList := &workv1alpha1.WorkList{} - if err := c.List(context.TODO(), workList, &client.ListOptions{LabelSelector: selector}); err != nil { - return nil, err - } - +func assembleWorkStatus(works []workv1alpha1.Work, workload *unstructured.Unstructured) ([]workv1alpha2.AggregatedStatusItem, error) { statuses := make([]workv1alpha2.AggregatedStatusItem, 0) - for _, work := range workList.Items { + for _, work := range works { identifierIndex, err := GetManifestIndex(work.Spec.Workload.Manifests, workload) if err != nil { klog.Errorf("Failed to get manifestIndex of workload in work.Spec.Workload.Manifests. Error: %v.", err) diff --git a/pkg/util/names/names.go b/pkg/util/names/names.go index a90cadb7412c..01d5fe134f0e 100644 --- a/pkg/util/names/names.go +++ b/pkg/util/names/names.go @@ -58,6 +58,19 @@ func GenerateBindingName(kind, name string) string { return strings.ToLower(name + "-" + kind) } +// GenerateBindingReferenceKey will generate the key of binding object with the hash of its namespace and name. +func GenerateBindingReferenceKey(namespace, name string) string { + var bindingName string + if len(namespace) == 0 { + bindingName = namespace + "-" + name + } else { + bindingName = name + } + hash := fnv.New32a() + hashutil.DeepHashObject(hash, bindingName) + return rand.SafeEncodeString(fmt.Sprint(hash.Sum32())) +} + // GenerateWorkName will generate work name by its name and the hash of its namespace, kind and name. func GenerateWorkName(kind, name, namespace string) string { var workName string