Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The label value of the dependent resoucebinding over 63 characters #1741

Closed
mathlsj opened this issue May 8, 2022 · 18 comments · Fixed by #4989 · May be fixed by #1963
Closed

The label value of the dependent resoucebinding over 63 characters #1741

mathlsj opened this issue May 8, 2022 · 18 comments · Fixed by #4989 · May be fixed by #1963
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@mathlsj
Copy link
Contributor

mathlsj commented May 8, 2022

The dependent resourcebinding label value consists of three components namspace+"_"+name+"-"+kind. it is easy to over 63 characters. we need to split it into three labels:
resourcebinding.karmada.io/depended-by-hash.namespace: namespace
resourcebinding.karmada.io/depended-by-hash.name: name
resourcebinding.karmada.io/depended-by-hash.kind: kind

@RainbowMango
Copy link
Member

Can you help point out which label we are talking about here?

@mathlsj
Copy link
Contributor Author

mathlsj commented May 9, 2022

generate resourcebinding depended label in dependencies_distributor (https://github.com/karmada-io/karmada/blob/master/pkg/dependenciesdistributor/dependencies_distributor.go)

func generateBindingDependedByLabel(bindingNamespace, bindingName string) map[string]string {
	labelKey := generateBindingDependedByLabelKey(bindingNamespace, bindingName)
        # labelValue = ns +"_" + bindingName (name + "-" + kind)
	labelValue := fmt.Sprintf(bindingNamespace + "_" + bindingName)
	return map[string]string{labelKey: labelValue}
}

@RainbowMango
Copy link
Member

Right, here probably exceed 64 characters. May I know what the name and namespace look like in your system?

@RainbowMango
Copy link
Member

Hi @mathlsj I see you send a PR for this.
I'd like to invite the author to talk about the solution. @mrlihanbo

@mathlsj
Copy link
Contributor Author

mathlsj commented Jun 6, 2022

OK。

@mathlsj
Copy link
Contributor Author

mathlsj commented Feb 17, 2023

@RainbowMango What is the progress of this?

@RainbowMango
Copy link
Member

I'm sorry I missed this from my radar. I'll be back on this next week.

@RainbowMango RainbowMango added this to the v1.6 milestone Feb 17, 2023
@RainbowMango
Copy link
Member

Right, here probably exceed 64 characters. May I know what the name and namespace look like in your system?

@mathlsj Can you give an example of your name and namespace?

@RainbowMango
Copy link
Member

we need to split it into three labels:
resourcebinding.karmada.io/depended-by-hash.namespace: namespace
resourcebinding.karmada.io/depended-by-hash.name: name
resourcebinding.karmada.io/depended-by-hash.kind: kind

Split the label could relieve the issue, but I'm afraid it can't solve the problem fundamentally, because the name itself may exceed 63 characters.

Investigation still on-going.

@RainbowMango
Copy link
Member

I've got two solutions here:

  1. Instead of putting the key resourcebinding.karmada.io/depended-by-xxx to a label, we can put it in the annotations.

For doing this, we can't use the label selector to list all the dependencies at listAttachedBindings, maybe we can use MatchingFieldsSelector.

  1. Deprecate the key by extending the ResourceBinding API to build the connection between main RB and attached RBs.

I didn't verify any of the two solutions yet, just an idea.

@mathlsj
Copy link
Contributor Author

mathlsj commented Feb 21, 2023

I tend to use the first one.

I was thinking if there was any other solution to it?

@RainbowMango
Copy link
Member

I was thinking if there was any other solution to it?

Looking forward to hear more solutions.
My concern about the two solutions is the performance, you know there could be a huge mount of RBs in the system.

@mathlsj
Copy link
Contributor Author

mathlsj commented May 15, 2023

I am sorry. i was so busy before. i will deal it continue.

@RainbowMango
Copy link
Member

I'm trying to figure out which PR/Issue should be included in the coming v1.7 release which is planned at the end of this month.
I guess we don't have enough time for this feature, so I'm moving this to v1.8.

Note that @jwcesign is leading the effort for this, see #4000 for the progress.

@RainbowMango RainbowMango modified the milestones: v1.7, v1.8 Aug 25, 2023
@RainbowMango RainbowMango modified the milestones: v1.8, v1.9 Nov 30, 2023
@RainbowMango RainbowMango modified the milestones: v1.9, v1.10 Feb 29, 2024
@RainbowMango
Copy link
Member

@XiShanYongYe-Chang Can you confirm if this issue is fixed?

@XiShanYongYe-Chang
Copy link
Member

The current issue has not been resolved. This is a separate label. This label is not analyzed in #4000. I will use this idea and the resolution experience in #4000 to continue to promote this issue.

/assign

@XiShanYongYe-Chang
Copy link
Member

/kind bug

@karmada-bot karmada-bot added the kind/bug Categorizes issue or PR as related to a bug. label May 27, 2024
@XiShanYongYe-Chang
Copy link
Member

Hi @mathlsj, this issue has been solved, would you like to have a try?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment