Skip to content
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.

Sign up for GitHub

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

Run device manager outside of the error group #2158

Merged
merged 4 commits into from
Jan 8, 2025
Merged

Run device manager outside of the error group #2158

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60a2a57
Run device manager outside of the error group
RonFed Jan 8, 2025
a7a68b7
fix logging
RonFed Jan 8, 2025
577c1e7
Merge branch 'main' into device_manager_goroutine
RonFed Jan 8, 2025
0751f27
Merge branch 'main' into device_manager_goroutine
RonFed Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions odiglet/odiglet.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func New(deviceInjectionCallbacks instrumentation.OtelSdksLsf, factories map[com

// Run starts the Odiglet components and blocks until the context is cancelled, or a critical error occurs.
func (o *Odiglet) Run(ctx context.Context) {
ctx, cancel := context.WithCancel(ctx)
g, groupCtx := errgroup.WithContext(ctx)

if err := o.criClient.Connect(ctx); err != nil {
Expand All @@ -108,11 +109,15 @@ func (o *Odiglet) Run(ctx context.Context) {
// Start device manager
// the device manager library doesn't support passing a context,
// however, internally it uses a context to cancel the device manager once SIGTERM or SIGINT is received.
g.Go(func() error {
// We run it outside of the error group to avoid blocking on Wait() in case of a fatal error.
go func() {
err := runDeviceManager(o.clientset, o.deviceInjectionCallbacks)
log.Logger.V(0).Info("Device manager exited")
return err
})
if err != nil {
log.Logger.Error(err, "Device manager exited with error")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: when existed with error, we will have 2 log lines for this:

Device manager exited
Device manager exited with error

cancel()
}
} ()

g.Go(func() error {
err := o.ebpfManager.Run(groupCtx)
Expand Down
Loading