feat: refactor Karmada resource labels

[jwcesign]

What type of PR is this?
/kind feature

What this PR does / why we need it:
Please referring to #4000

Which issue(s) this PR fixes:
Fixes #4000

Special notes for your reviewer:
In PR:#4007, we add the owner information into the annotation.
This PR just use the annotation and the uid label for the handling logic.

Does this PR introduce a user-facing change?:

Do not include the owner information label in the resource labels. This information is already present in the resource annotations.

[RainbowMango]

Hi @jwcesign Just a reminder, this PR has conflicts, and it'd better to solve the conflicts first.

[wu0407]

has any progress?

[jwcesign]

Hi @wu0407, I will complete it by the end of this week. Once it's done, I'll notify you for your review.

[RainbowMango]

@jwcesign Could you please summarize the whole solution and point out what should be included in this PR(or v1.7)?
It's hard for people who didn't attend the community meeting to understand and review.

[whitewindmills]

could you extend util.MergeAnnotation and util.MergeLabel? they are not efficient, every annotation and label need to get object's and set. they should looks like this:

// MergeAnnotations adds annotations for the given object, keep the value unchanged if key exist.
func MergeAnnotations(obj *unstructured.Unstructured, annos map[string]string) {
	objectAnnotation := obj.GetAnnotations()
	if objectAnnotation == nil {
		objectAnnotation = make(map[string]string, len(annos))
	}

	for annotationKey, annotationValue := range annos {
		if _, exist := objectAnnotation[annotationKey]; !exist {
			objectAnnotation[annotationKey] = annotationValue
		}
	}
	obj.SetAnnotations(objectAnnotation)
}

[jwcesign]

I think we can do it in another PR

[whitewindmills]

thanks for your hard work 😊. this is a huge review effort, maybe we need more reviews.

[XiShanYongYe-Chang]

Still on reviewing.

[XiShanYongYe-Chang]

Feel free to remove this part of the logic in the method, as using uid should ensure global uniqueness now.
Ask @chaunceyjiang to help make sure again.

[jwcesign]

/cc @whitewindmills @XiShanYongYe-Chang @chaunceyjiang

[chaunceyjiang]

Before starting the review, I have a question to ask:
Is the generation of UUID stable? If only using the default algorithm of ETCD, then after backing up Karmada, this UUID cannot be guaranteed to be the same after recovery. (use velero)

If Karmada has been restored and the UUID has changed, can Karmada still work properly?

@jwcesign @RainbowMango @XiShanYongYe-Chang @whitewindmills

[jwcesign]

If Karmada has been restored and the UUID has changed, can Karmada still work properly?

I will check it, in my opinion, I think the uid label should be updated(We can get the information from annotation and update it correctly, once it has changed)

[RainbowMango]

Is the generation of UUID stable? If only using the default algorithm of ETCD, then after backing up Karmada, this UUID cannot be guaranteed to be the same after recovery. (use velero)

Good question!
I think the UUID(.metadata.uid) is stored in ETCD, after restoring, all objects probably use the previous uuid. But if restoring the object by applying again, then the UUID would change to a new one.

[chaunceyjiang]

But if restoring the object by applying again, then the UUID would change to a new one.

Yes, if we delete the work object through uuid, will the work object never be deleted when uuid changes? @jwcesign @RainbowMango

[jwcesign]

Hi, @chaunceyjiang
If using velero, I think users should only backup Workload template(like deployment)/CPP/PP/CR/CRD， so If the detector can see the uid is changed, and update it(easy to implement), then all the resource labels next(rb/crb/work/workload in member clusters) will be correct, because they will be recreated.

So, this means, velero only should back up the top-level resources, not back up the derived resources(like work, rb).

If you back up the derived resources, like work, the old work will never be deleted(The label selector can't find the work now).

What do you think?

[karmada-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from xishanyongye-chang after the PR has been reviewed.

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

Needs approval from an approver in each of these files:

• pkg/controllers/OWNERS
• pkg/util/OWNERS
• test/OWNERS

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

[jwcesign]

/close

Track in the issue