ClusterWideKeyFunc don't get runtime.Object GVK

[xigang]

What happened:
call ClusterWideKeyFunc() method don't get runtime.Object GVK information.

code: https://github.com/karmada-io/karmada/blob/master/pkg/util/fedinformer/keys/keys.go#L63

// ClusterWideKeyFunc generates a ClusterWideKey for object.
func ClusterWideKeyFunc(obj interface{}) (ClusterWideKey, error) {
	key := ClusterWideKey{}

	runtimeObject, ok := obj.(runtime.Object)
	if !ok {
		klog.Errorf("Invalid object")
		return key, fmt.Errorf("not runtime object")
	}

	metaInfo, err := meta.Accessor(obj)
	if err != nil { // should not happen
		return key, fmt.Errorf("object has no meta: %v", err)
	}

	gvk := runtimeObject.GetObjectKind().GroupVersionKind() // this gvk might be empty.
	key.Group = gvk.Group
	key.Version = gvk.Version
	key.Kind = gvk.Kind
	key.Namespace = metaInfo.GetNamespace()
	key.Name = metaInfo.GetName()

	return key, nil
}

Test code output：

What you expected to happen:
Expect to get GVK information from runtime.Object

How to reproduce it (as minimally and precisely as possible):

Test code:

package main

import (
	"context"
	"encoding/json"
	"flag"
	"fmt"
	"path/filepath"

	apiv1 "k8s.io/api/core/v1"
	"k8s.io/apimachinery/pkg/api/meta"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/apimachinery/pkg/runtime"
	"k8s.io/client-go/kubernetes"
	"k8s.io/client-go/tools/clientcmd"
	"k8s.io/client-go/util/homedir"
	"k8s.io/klog/v2"
)

// ClusterWideKey is the object key which is a unique identifier under a cluster, across all resources.
type ClusterWideKey struct {
	// Group is the API Group of resource being referenced.
	Group string

	// Version is the API Version of the resource being referenced.
	Version string

	// Kind is the type of resource being referenced.
	Kind string

	// Namespace is the name of a namespace.
	Namespace string

	// Name is the name of resource being referenced.
	Name string
}

// ClusterWideKeyFunc generates a ClusterWideKey for object.
func ClusterWideKeyFunc(obj interface{}) (ClusterWideKey, error) {
	key := ClusterWideKey{}

	runtimeObject, ok := obj.(runtime.Object)
	if !ok {
		klog.Errorf("Invalid object")
		return key, fmt.Errorf("not runtime object")
	}

	metaInfo, err := meta.Accessor(obj)
	if err != nil { // should not happen
		return key, fmt.Errorf("object has no meta: %v", err)
	}

	gvk := runtimeObject.GetObjectKind().GroupVersionKind()
	key.Group = gvk.Group
	key.Version = gvk.Version
	key.Kind = gvk.Kind
	key.Namespace = metaInfo.GetNamespace()
	key.Name = metaInfo.GetName()

	return key, nil
}

func main() {
	var kubeconfig *string
	if home := homedir.HomeDir(); home != "" {
		kubeconfig = flag.String("kubeconfig", filepath.Join(home, ".kube", "config"), "(optional) absolute path to the kubeconfig file")
	} else {
		kubeconfig = flag.String("kubeconfig", "", "absolute path to the kubeconfig file")
	}
	flag.Parse()

	config, err := clientcmd.BuildConfigFromFlags("", *kubeconfig)
	if err != nil {
		panic(err)
	}
	clientset, err := kubernetes.NewForConfig(config)
	if err != nil {
		panic(err)
	}

	deploymentsClient := clientset.AppsV1().Deployments(apiv1.NamespaceDefault)

	// List Deployments
	fmt.Printf("Listing deployments in namespace %q:\n", apiv1.NamespaceDefault)
	list, err := deploymentsClient.List(context.TODO(), metav1.ListOptions{})
	if err != nil {
		panic(err)
	}
	for _, d := range list.Items {
		objectKey, err := ClusterWideKeyFunc(&d)
		if err != nil {
			fmt.Println(err)
			return
		}

		data, _ := json.MarshalIndent(&objectKey, " ", "   ")
		fmt.Printf("object name: %v key: %v\n", d.Name, string(data))
	}

}

[xigang]

/cc @RainbowMango @XiShanYongYe-Chang

[RainbowMango]

@xigang Could you tell me what are you doing with the GetClusterWideKey?

[RainbowMango]

call ClusterWideKeyFunc() method don't get runtime.Object GVK information.

I don't know why you run this demo.

data, _ := json.MarshalIndent(&objectKey, " ", " ")

I guess you can print the objectKey before marshall it.

[xigang]

@xigang Could you tell me what are you doing with the GetClusterWideKey?

Updated the test code to change GetClusterWideKey to ClusterWideKeyFunc https://github.com/karmada-io/karmada/blob/master/pkg/util/fedinformer/keys/keys.go#L63

[xigang]

call ClusterWideKeyFunc() method don't get runtime.Object GVK information.

I don't know why you run this demo.

data, _ := json.MarshalIndent(&objectKey, " ", " ")

I guess you can print the objectKey before marshall it.

@RainbowMango The GVK will returned is null from the ClusterWideKeyFunc L77 line code.

gvk := runtimeObject.GetObjectKind().GroupVersionKind()

ref:
kubernetes/kubernetes#3030
kubernetes/kubernetes#80609

[RainbowMango]

Updated the test code to change GetClusterWideKey to ClusterWideKeyFunc

Where is the GetClusterWideKey ?

[whitewindmills]

It has nothing to do with karmada because List won't return object's gvk using client-go.

[xigang]

It has nothing to do with karmada because List won't return object's gvk using client-go.

@whitewindmills Yes, but this ClusterWideKeyFunc needs to handle scenarios where GVK is empty?

eg:

import (
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
)

// ClusterWideKeyFunc generates a ClusterWideKey for object.
func ClusterWideKeyFunc(obj interface{}, scheme *runtime.Scheme) (ClusterWideKey, error) {
	key := ClusterWideKey{}

	runtimeObject, ok := obj.(runtime.Object)
	if !ok {
		klog.Errorf("Invalid object")
		return key, fmt.Errorf("not runtime object")
	}

	metaInfo, err := meta.Accessor(obj)
	if err != nil { // should not happen
		return key, fmt.Errorf("object has no meta: %v", err)
	}

	gvk, err := apiutil.GVKForObject(runtimeObject, scheme)
	if err != nil {
		return key, fmt.Errorf("missing apiVersion or kind and cannot assigin it: %v", err)
	}
	key.Group = gvk.Group
	key.Version = gvk.Version
	key.Kind = gvk.Kind
	key.Namespace = metaInfo.GetNamespace()
	key.Name = metaInfo.GetName()

	return key, nil
}

[xigang]

Updated the test code to change GetClusterWideKey to ClusterWideKeyFunc

Where is the GetClusterWideKey ?

Don't need to care about GetClusterWideKey, focus ClusterWideKeyFunc function GVK: = runtimeObject. GetObjectKind () GroupVersionKind ()

[RainbowMango]

Yes, but this ClusterWideKeyFunc needs to handle scenarios where GVK is empty?

Oh, I get what you mean now.
Does any place that Karmada uses this function to handle objects list out?

[xigang]

Yes, but this ClusterWideKeyFunc needs to handle scenarios where GVK is empty?

Oh, I get what you mean now. Does any place that Karmada uses this function to handle objects list out?

Yes, the karmada doesn't have list objects, but would it be safer to replace the following code?

#3275 (comment)

[RainbowMango]

Yes, the karmada doesn't have list objects, but would it be safer to replace the following code?

I agree, but I think we can check if the group/version/kind is empty, but shouldn't add the schema parameter.
In addition, we should add a comment to explain why we need it.

/remove-kind bug
/kind cleanup

[whitewindmills]

Hi @RainbowMango
Can this issue be assigned to me?

[RainbowMango]

Thanks @whitewindmills.

Sure thing. We always follow first-come-first-served rule, since this issue hasn't been picked by anybody, that means anybody can assign it to him/her (type /assign in a separate line) if want help.

But, given @xigang spotted this, I'm not sure if @xigang wants to have a try.

@xigang Please let me know if you want help.

@whitewindmills I guess we can wait for @xigang for a while(like, by the end of today), if no response from @xigang, then you can assign it to you, thanks in advance.

By the way, if @xigang gonna take this, you still can help to review, that's also a worthy contribution.

[xigang]

@RainbowMango Thanks, I can fix this issue. I recently submit a pr. ：）

[xigang]

Thanks @whitewindmills.

Sure thing. We always follow first-come-first-served rule, since this issue hasn't been picked by anybody, that means anybody can assign it to him/her (type /assign in a separate line) if want help.

But, given @xigang spotted this, I'm not sure if @xigang wants to have a try.

@xigang Please let me know if you want help.

@whitewindmills I guess we can wait for @xigang for a while(like, by the end of today), if no response from @xigang, then you can assign it to you, thanks in advance.

By the way, if @xigang gonna take this, you still can help to review, that's also a worthy contribution.

@RainbowMango @whitewindmills PR has been submitted. #3288

[RainbowMango]

/assign @xigang
in favor of #3288