Move binding's namespace/name from work's label to work's annotation

[XiShanYongYe-Chang]

Signed-off-by: changzhen [email protected]

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

The key of a label must be no more than 63 characters. When a binding object is generated, the name format is as follows: ${name}-${kind}, this may cause the length to exceed the limit.

The key of a annotation has no limits, so i move binding's namespace/name from work's label to work's annotation.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Following labels which previously added on the Work object now have been moved to annotations:
* `resourcebinding.karmada.io/namespace`
* `resourcebinding.karmada.io/name`
* `clusterresourcebinding.karmada.io/name`

[RainbowMango]

Propoase a util:

// HasAnnotation tells if the 'annotation' contains the expected key-value pair.
func HasAnnotation(annotation map[string]string, expectKey string, expectValue string) bool {
	if annotation == nil {
		return false
	}

	value, exist := annotation[expectKey]
	if !exist {
		return false
	}

	return value == expectValue
}

[XiShanYongYe-Chang]

Propoase a util:

// HasAnnotation tells if the 'annotation' contains the expected key-value pair.
func HasAnnotation(annotation map[string]string, expectKey string, expectValue string) bool {
	if annotation == nil {
		return false
	}

	value, exist := annotation[expectKey]
	if !exist {
		return false
	}

	return value == expectValue
}

Got it.

[RainbowMango]

/assign @Garrybest @mrlihanbo @GitHubxsy
Please take a look. This is kind of an API change.

[Garrybest]

I'm afraid that it is potentially expensive that we list all RBs every time and range them to filter the matched ones when we have a lot of RBs. What about only moving names to annotations and leave namespace in labels? So we could list work by namespace label first. That will narrow work list.

[XiShanYongYe-Chang]

Sounds good.

[XiShanYongYe-Chang]

But separating namespaces and namespaces can be confusing. How do you think @RainbowMango

[RainbowMango]

Can we have both the label and annotation? E.g.

diff --git a/pkg/apis/work/v1alpha1/well_known_labels.go b/pkg/apis/work/v1alpha1/well_known_labels.go
index 9cc3d813..657ad8cc 100644
--- a/pkg/apis/work/v1alpha1/well_known_labels.go
+++ b/pkg/apis/work/v1alpha1/well_known_labels.go
@@ -1,6 +1,15 @@
 package v1alpha1

 const (
+       // ResourceBindingReferenceKey is the key of ResourceBinding object.
+       // It is usually a unique hash value of ResourceBinding object's name and 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 name and 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"

The label is used to retrieve Work by RB/CRB and the annotation is used to retrieve RB/CRB by Work.

[XiShanYongYe-Chang]

That's greate. How do you think @Garrybest ?

If that's ok, i am going to modify with this command.

[Garrybest]

So we have both annotations and labels of resourcebinding in work? And the value is a unique hash value? I don't understand why annotations still need to be kept if we keep the labels. Could you give a example?

[RainbowMango]

Good question.

karmada/pkg/controllers/binding/binding_controller.go

Lines 120 to 121 in 1947cf2

	resourcebindingNamespace, namespaceExist := labels[workv1alpha1.ResourceBindingNamespaceLabel]
	resourcebindingName, nameExist := labels[workv1alpha1.ResourceBindingNameLabel]

The binding controller wants to enqueue Binding once the relevant Work is changed, in order to such as, grab status.

[Garrybest]

OK, let's do it.

[Garrybest]

Oh I see. The annotation is used to record the namespace and name for finding the relevant RB of a work. And the hash label is used to select all relevant works of a RB quickly.

[Garrybest]

/lgtm

[XiShanYongYe-Chang]

Oh I see. The annotation is used to record the namespace and name for finding the relevant RB of a work. And the hash label is used to select all relevant works of a RB quickly.

That's true. :)

[RainbowMango]

Generally LGTM.
But since we missed the PR from v0.9.0, so the comments should be updated.

Delete this logic in the next release to prevent incompatibility when upgrading the current release (`v0.9.0`)

[RainbowMango]

/lgtm
/approve

[karmada-bot]

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment