karmada icon indicating copy to clipboard operation
karmada copied to clipboard

Make auto-generated policy become more readable

Open carlory opened this issue 2 years ago • 1 comments

What would you like to be added:

refactor the names. GeneratePolicyName function.

Why is this needed:

when we execute the karmadactl apply or karmadactl promote command, pp or cpp will be generated and applied to karmada-apiserver.

but the name of auto-generated policy is not easy readable unless the users explicitly set the resource name like web-deployment, web-service. So should we auto-generate policy name format as <name>-<kind>-<hash-key> or is there a better way?

[root@master67 mci]# karmadactl apply -f all.yaml --all-clusters
deployment.apps/web created
propagationpolicy.policy.karmada.io/web-6d7f8d5f5b created
service/web created
propagationpolicy.policy.karmada.io/web-76579ccd86 created
serviceexport.multicluster.x-k8s.io/web created
propagationpolicy.policy.karmada.io/web-7d98b55cd7 created
serviceimport.multicluster.x-k8s.io/web created
propagationpolicy.policy.karmada.io/web-75fdb9ccbd created

see #2000 for details.

/cc @lonelyCZ @RainbowMango

/assign

carlory avatar Jul 11 '22 05:07 carlory

When there are different versions of a resource in a file, if the policy name is produced according to the rules of <name>-<kind>-<hash-key>, the same resource will correspond to multiple policies. hash-key is generated via gvk.

Maybe we should modify the implementation of the generator function, here is an example:

// GeneratePolicyName generates the propagationPolicy name
func GeneratePolicyName(name, kind, group string) string {
  hash := fnv.New32a()
  hashutil.DeepHashObject(hash, group)
  return strings.ToLower(fmt.Sprintf("%s-%s-%s", name, kind, rand.SafeEncodeString(fmt.Sprint(hash.Sum32())))
}

however, when there are resources of the same name and type in a file, but they do not belong to the same API group, -- is not easy readable.

// GeneratePolicyName generates the propagationPolicy name
func GeneratePolicyName(name, kind, group string) string {
  if group == "" {
    return strings.ToLower(fmt.Sprintf("%s-%s", name, kind))
  }
  group = strings.ReplaceAll(group, ".", "-")
  return strings.ToLower(fmt.Sprintf("%s-%s-%s", name, kind, group))
}

@lonelyCZ @RainbowMango what do you think ?

carlory avatar Jul 11 '22 05:07 carlory

The policy naming rule has been modified. /close

XiShanYongYe-Chang avatar Mar 01 '24 09:03 XiShanYongYe-Chang

@XiShanYongYe-Chang: Closing this issue.

In response to this:

The policy naming rule has been modified. /close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

karmada-bot avatar Mar 01 '24 09:03 karmada-bot