-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: rate limit node collector restarts on config change #1357
Conversation
… seconds and after 20 seconds.
@@ -57,18 +66,17 @@ func syncDataCollection(instApps *odigosv1.InstrumentedApplicationList, dests *o | |||
logger := log.FromContext(ctx) | |||
logger.V(0).Info("Syncing data collection") | |||
|
|||
configData, err := syncConfigMap(instApps, dests, processors, dataCollection, ctx, c, scheme) | |||
_, err := syncConfigMap(instApps, dests, processors, dataCollection, ctx, c, scheme) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I calculate this later -> when that patch triggers
} | ||
|
||
// runFunctionWithDelayAndSkipNewCalls runs the function with the specified delay and skips new calls until the function execution is finished | ||
func (dm *DelayManager) runFunctionWithDelayAndSkipNewCalls(delay time.Duration, fn func(args ...interface{}) (*appsv1.DaemonSet, error), fnArgs ...interface{}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to use a library function for this if possible.
Looks like https://pkg.go.dev/k8s.io/apimachinery@v0.30.2/pkg/util/wait#PollUntilContextTimeout might be relevant or other functions from that lib
|
||
func getConfigMap(ctx context.Context, c client.Client, collectorGroup *odigosv1.CollectorsGroup) (*v1.ConfigMap, error) { | ||
configMap := &v1.ConfigMap{} | ||
if err := c.Get(ctx, client.ObjectKey{Namespace: collectorGroup.Namespace, Name: collectorGroup.Name}, configMap); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use the config map name directly as in https://github.com/odigos-io/odigos/blob/main/k8sutils/pkg/consts/consts.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! added 2 nit comments
if err == nil { | ||
return | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we log something if we run out of retries and give up?
otherwise, the error is silently dropped, right?
and should we also log something on success to aid debugging in case there are issues?
No description provided.