chia7712 commented on code in PR #844:
URL: https://github.com/apache/yunikorn-k8shim/pull/844#discussion_r1609564623
##########
pkg/client/clients.go:
##########
@@ -27,13 +27,19 @@ import (
coreInformerV1 "k8s.io/client-go/informers/core/v1"
schedulingInformerV1 "k8s.io/client-go/informers/scheduling/v1"
storageInformerV1 "k8s.io/client-go/informers/storage/v1"
+ "k8s.io/client-go/tools/cache"
+ "k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/scheduler/framework/plugins/volumebinding"
"github.com/apache/yunikorn-k8shim/pkg/conf"
"github.com/apache/yunikorn-k8shim/pkg/log"
"github.com/apache/yunikorn-scheduler-interface/lib/go/api"
)
+type informer interface {
Review Comment:
> Any informer defined should be started and stopped. To not regress we
could have a simple unit test that checks that all informers defined in the
type are also returned in the getInformers() call. If someone adds a new
informer we can then ensure that they are updated in both places
agree to that "started" and "stopped" are the most important here. It means
`Clients` must have all "created" informers. Maybe we can add a small helper to
collect all created informers into a slice, and then we pass the slice to
`Clients`. For example:
```go
type hasInformer interface {
Informer() cache.SharedIndexInformer
}
func save[T hasInformer](n T, collector *[]cache.SharedIndexInformer) T {
*collector = append(*collector, n.Informer())
return n
}
```
We use a little generic here to avoid casting, and all created informers
should be wrapped by `save`. For example:
```go
var allInformers []cache.SharedIndexInformer
nodeInformer := save(informerFactory.Core().V1().Nodes(), &allInformers)
podInformer := save(informerFactory.Core().V1().Pods(), &allInformers)
```
Finally, `allInformers` have all created informers, and `Clients` will use
it to start/stop all informers.
@wilfred-s WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
