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

Jump to bottom

sync sandboxes and containers after starting the pre-installed plugins #43

Merged
merged 2 commits into from
Sep 24, 2024
Merged

sync sandboxes and containers after starting the pre-installed plugins #43

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b881bc4
sync sandboxes and containers after starting the pre-installed plugins
Iceber May 18, 2023
eada085
ignore pre-installed plugins that did not sync successfully
Iceber Jun 4, 2024
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
33 changes: 28 additions & 5 deletions pkg/adaptation/adaptation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,25 +336,48 @@ func (r *Adaptation) startPlugins() (retErr error) {
}()

for i, name := range names {
log.Infof(noCtx, "starting plugin %q...", name)
log.Infof(noCtx, "starting pre-installed NRI plugin %q...", name)

id := ids[i]

p, err := r.newLaunchedPlugin(r.pluginPath, id, name, configs[i])
if err != nil {
return fmt.Errorf("failed to start NRI plugin %q: %w", name, err)
log.Warnf(noCtx, "failed to initialize pre-installed NRI plugin %q: %v", name, err)
continue
}

if err := p.start(r.name, r.version); err != nil {
return err
log.Warnf(noCtx, "failed to start pre-installed NRI plugin %q: %v", name, err)
continue
}

plugins = append(plugins, p)
}

// Although the error returned by syncPlugins may not be nil, r.syncFn could still ignores this error and returns a nil error.
// We need to make sure that the plugins are successfully synchronized in the `plugins`
syncPlugins := func(ctx context.Context, pods []*PodSandbox, containers []*Container) (updates []*ContainerUpdate, err error) {
startedPlugins := plugins
plugins = make([]*plugin, 0, len(plugins))
for _, plugin := range startedPlugins {
us, err := plugin.synchronize(ctx, pods, containers)
if err != nil {
plugin.stop()
log.Warnf(noCtx, "failed to synchronize pre-installed NRI plugin %q: %v", plugin.name(), err)
continue
}

plugins = append(plugins, plugin)
updates = append(updates, us...)
log.Infof(noCtx, "pre-installed NRI plugin %q synchronization success", plugin.name())
}
return updates, nil
}
if err := r.syncFn(noCtx, syncPlugins); err != nil {
mikebrow marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("failed to synchronize pre-installed NRI Plugins: %w", err)
Copy link
Member

Choose a reason for hiding this comment

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

This is another thing I now start to have 2nd thoughts about. Do we really want to prevent the runtime from starting up, if any pre-installed plugin fails to start or synchronize ? Or would it be better to just log the errors, ignore failed plugins and continue... Any thoughts on that @mikebrow ?

}

r.plugins = plugins
r.sortPlugins()

return nil
}

Expand Down
Loading