Use recommended context.Context passing #112
Conversation
Documentation in context.Context states never to embed a context.Context in a struct. It should always be passed to functions and methods, ideally as the first parameter. Kubernetes' Go API follows this syntax, and since it is much easier to fix the issue in kubectl-rook-ceph compared to in rook/rook, we should do so. The old k8sutil.Context has been renamed to k8sutil.Clientsets to reflect its new purpose. Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>
| context.RookClientset, err = rookclient.NewForConfig(context.KubeConfig) | ||
| clientsets.Rook, err = rookclient.NewForConfig(clientsets.KubeConfig) | ||
| if err != nil { | ||
| logging.Fatal(err) | ||
| } | ||
|
|
||
| context.RookClientset, err = rookclient.NewForConfig(context.KubeConfig) | ||
| clientsets.Kube, err = k8s.NewForConfig(clientsets.KubeConfig) | ||
| if err != nil { | ||
| logging.Fatal(err) | ||
| } | ||
|
|
||
| context.Clientset, err = k8s.NewForConfig(context.KubeConfig) | ||
| if err != nil { | ||
| logging.Fatal(err) | ||
| } |
There was a problem hiding this comment.
This also fixes a duplicate context.RookClientset, err = ...
| context := GetContext() | ||
| fmt.Println(exec.RunCommandInOperatorPod(context, cmd.Use, args, OperatorNamespace, CephClusterNamespace, true)) | ||
| clientsets := GetClientsets() | ||
| fmt.Println(exec.RunCommandInOperatorPod(cmd.Context(), clientsets, cmd.Use, args, OperatorNamespace, CephClusterNamespace, true)) |
There was a problem hiding this comment.
One of the core reasons to make this change, versus letting it stand, is to make sure all commands explicitly use cmd.Context() for their context.
There was a problem hiding this comment.
Documentation in context.Context states never to embed a context.Context in a struct. It should always be passed to functions and methods, ideally as the first parameter.
We don't need to pass the context as a pointer? Seems like it would be similar behavior to pass it by value as to pass it around with a struct, but I didn't go dig into the whole issue.
There was a problem hiding this comment.
context.Context is an interface, and I think it fails to compile if a * is in front of the param.
Documentation in context.Context states never to embed a context.Context in a struct. It should always be passed to functions and methods, ideally as the first parameter. Kubernetes' Go API follows this syntax, and since it is much easier to fix the issue in kubectl-rook-ceph compared to in rook/rook, we should do so.
The old k8sutil.Context has been renamed to k8sutil.Clientsets to reflect its new purpose.