aviold event duplicates #45
Conversation
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
testing/it_sidecar/it_sidecar.go
Outdated
| func listenForEvents(ctx context.Context, clientset *kubernetes.Clientset, onFailure func(*v1.Event)) { | ||
|
|
||
| kubeInformerFactory := informers.NewFilteredSharedInformerFactory(clientset, time.Second*30, *namespace, nil) | ||
| kubeInformerFactory := informers.NewSharedInformerFactoryWithOptions(clientset, 0, informers.WithNamespace(*namespace)) |
There was a problem hiding this comment.
The listenForEvents function does not handle the case where the namespace variable might be nil or an empty string. This could lead to unexpected behavior or runtime errors.
Recommended Solution:
Add a check to ensure that the namespace variable is not nil or empty before proceeding with the creation of the SharedInformerFactory. If it is, handle the error appropriately, possibly by logging an error message and returning early from the function.
| var wg sync.WaitGroup | ||
| wg.Add(len(ports)) | ||
| for _, port := range ports { | ||
| ep, err := clientset.CoreV1().Endpoints(*namespace).Get(ctx, serviceName, meta_v1.GetOptions{}) | ||
| ep, err := clientset.CoreV1().Endpoints(namespace).Get(ctx, serviceName, meta_v1.GetOptions{}) | ||
| if err != nil { | ||
| return fmt.Errorf("error listing endpoints for service %s: %v", serviceName, err) | ||
| } |
There was a problem hiding this comment.
The Get call to retrieve endpoints for a service is made inside a loop without any rate limiting or retries. This can lead to throttling by the Kubernetes API server if there are many ports, causing potential failures.
Recommended Solution:
Implement rate limiting and retries for the Get call to ensure that the function can handle cases where the API server throttles requests. Use a backoff strategy to retry the request in case of transient errors.
| clientset = kubernetes.NewForConfigOrDie(config) | ||
|
|
||
| go func() { | ||
| err := stern.Run(ctx, *namespace, clientset, allowErrors, disablePodLogs) | ||
| err := stern.Run(ctx, namespace, clientset, allowErrors, disablePodLogs) | ||
| if err != nil { | ||
| log.Print(err) | ||
| } |
There was a problem hiding this comment.
The error handling in the goroutine that runs stern.Run is insufficient. If stern.Run returns an error, it is only logged, and no further action is taken. This can lead to silent failures where the main program continues running without the necessary sidecar functionality.
Recommended Solution:
Enhance the error handling by implementing a mechanism to signal the main program to terminate or take corrective action if stern.Run fails. This could involve canceling the context or sending a signal to a monitoring channel.
All events are displayed in a test log every 30 seconds, which is misleading. We would like to see only new events.