From 1e14d2310ce229259e1b8eb925f8870e0abf9f85 Mon Sep 17 00:00:00 2001 From: Brian Neville <29639579+brianneville@users.noreply.github.com> Date: Sat, 13 Jun 2026 13:27:35 +0100 Subject: [PATCH] containers/docker: support restarting plugins In the same way that containers that are stopped can be restarted by providing just their instance name, make it so that stopped plugins can be restarted if StartPlugin RPC is provided only the instance name. To disambiguate starting/restarting on the client side, this change also adds a new command for the containerz client to explicitly restart: containerz plugin restart --instance INSTANCE --- client/start_plugin.go | 11 ++ cmd/plugin_start.go | 26 ++++- containers/docker/plugin_list.go | 40 ++++--- containers/docker/plugin_start.go | 99 +++++++++++++++-- containers/docker/plugin_start_test.go | 143 ++++++++++++++++++++++--- 5 files changed, 284 insertions(+), 35 deletions(-) diff --git a/client/start_plugin.go b/client/start_plugin.go index dec1d19..239e2ac 100644 --- a/client/start_plugin.go +++ b/client/start_plugin.go @@ -11,6 +11,17 @@ import ( // StartPlugin starts the requested plugin identified by instance. func (c *Client) StartPlugin(ctx context.Context, name, instance, configFile string) error { + if instance == "" { + return fmt.Errorf("instance name cannot be empty") + } + if configFile == "" && name == "" { + // restart an existing plugin instance. + _, err := c.cli.StartPlugin(ctx, &cpb.StartPluginRequest{ + InstanceName: instance, + }) + return err + } + buf, err := os.ReadFile(configFile) if err != nil { return fmt.Errorf("failed to read config file: %w", err) diff --git a/cmd/plugin_start.go b/cmd/plugin_start.go index ebb44cb..3e85762 100644 --- a/cmd/plugin_start.go +++ b/cmd/plugin_start.go @@ -29,15 +29,15 @@ var pluginStartCmd = &cobra.Command{ Short: "Start a plugin", RunE: func(command *cobra.Command, args []string) error { if instance == "" { - fmt.Println("--instance must be provided") + return fmt.Errorf("--instance must be provided") } if name == "" { - fmt.Println("--name must be provided") + return fmt.Errorf("--name must be provided") } if configFile == "" { - fmt.Println("--config must be provided") + return fmt.Errorf("--config must be provided") } if err := containerzClient.StartPlugin(command.Context(), name, instance, configFile); err != nil { @@ -49,9 +49,29 @@ var pluginStartCmd = &cobra.Command{ }, } +var pluginRestartCmd = &cobra.Command{ + Use: "restart", + Short: "Restart a plugin", + RunE: func(command *cobra.Command, args []string) error { + if instance == "" { + return fmt.Errorf("--instance must be provided") + } + + if err := containerzClient.StartPlugin(command.Context(), name, instance, configFile); err != nil { + return err + } + + fmt.Printf("Successfully restarted %s\n", instance) + return nil + }, +} + func init() { pluginCmd.AddCommand(pluginStartCmd) pluginStartCmd.PersistentFlags().StringVar(&instance, "instance", "", "plugin instance name") pluginStartCmd.PersistentFlags().StringVar(&name, "name", "", "plugin name") pluginStartCmd.PersistentFlags().StringVar(&configFile, "config", "", "plugin config file") + + pluginCmd.AddCommand(pluginRestartCmd) + pluginRestartCmd.PersistentFlags().StringVar(&instance, "instance", "", "plugin instance name") } diff --git a/containers/docker/plugin_list.go b/containers/docker/plugin_list.go index b7f3548..11ba845 100644 --- a/containers/docker/plugin_list.go +++ b/containers/docker/plugin_list.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" cpb "github.com/openconfig/gnoi/containerz" @@ -14,21 +15,12 @@ import ( // PluginList lists all plugins on a target. If instance is not empty, it will return a plugin // named `instance` if it exists. func (m *Manager) PluginList(ctx context.Context, instance string) (*cpb.ListPluginsResponse, error) { - resp, err := m.client.PluginList(ctx, filters.Args{}) + res := &cpb.ListPluginsResponse{} + plugins, err := m.listMatchingPlugins(ctx, instance) if err != nil { - return nil, fmt.Errorf("failed to list plugins: %w", err) + return nil, err } - - res := &cpb.ListPluginsResponse{} - for _, plugin := range resp { - if instance != "" { - // plugin.Name will have format :. - // instance_name (from StartPluginRequest) does not have a tag, - // so cut off only the name here. - if pluginName, _, _ := strings.Cut(plugin.Name, ":"); pluginName != instance { - continue - } - } + for _, plugin := range plugins { conf, err := json.MarshalIndent(plugin.Config, "", " ") if err != nil { return nil, fmt.Errorf("unable to marshal plugin config: %v", err) @@ -43,3 +35,25 @@ func (m *Manager) PluginList(ctx context.Context, instance string) (*cpb.ListPlu return res, nil } + +func (m *Manager) listMatchingPlugins(ctx context.Context, instance string) ([]*types.Plugin, error) { + resp, err := m.client.PluginList(ctx, filters.Args{}) + if err != nil { + return nil, fmt.Errorf("failed to list plugins: %w", err) + } + if instance == "" { + return resp, nil + } + + var plugins []*types.Plugin + for _, plugin := range resp { + // plugin.Name will have format :. + // instance_name (from StartPluginRequest) does not have a tag, + // so cut off only the name here. + if pluginName, _, _ := strings.Cut(plugin.Name, ":"); pluginName != instance { + continue + } + plugins = append(plugins, plugin) + } + return plugins, nil +} diff --git a/containers/docker/plugin_start.go b/containers/docker/plugin_start.go index 7db5bd6..e7de512 100644 --- a/containers/docker/plugin_start.go +++ b/containers/docker/plugin_start.go @@ -6,9 +6,19 @@ import ( "os" "path/filepath" + "github.com/docker/docker/api/types" "github.com/docker/docker/pkg/archive" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) - "github.com/docker/docker/api/types" +type pluginState int + +const ( + _ pluginState = iota + running + stopped + notPresent ) var ( @@ -26,10 +36,16 @@ const ( rootfsDir = "rootfs" ) -// PluginStart loads the deployed plugin tarball (expected to be in /plugins) into the container -// runtime. +// PluginStart does the following: // -// The operations performed here are based on this [documentation](https://docs.docker.com/engine/extend/#developing-a-plugin). +// If the plugin is already present but stopped, and the request +// contains only the instance name of that plugin, then the plugin will be re-started. +// +// If the plugin is not present, then it loads the deployed plugin tarball +// (expected to be in /plugins) into the container runtime. +// +// The operations performed to load and enabled the deployed plugin are based on +// this [documentation](https://docs.docker.com/engine/extend/#developing-a-plugin). // The process is as follows: // 0. The plugin image was uploaded in a previous deploy operation. // 1. Unpack the plugin in a scratch space. The image must be unpacked under a `rootfs` directory. @@ -37,6 +53,39 @@ const ( // 3. Tar up the result // 4. Push the tarball to docker and enable the plugin. func (m *Manager) PluginStart(ctx context.Context, name, instance, config string) error { + + currentState, err := m.getPluginState(ctx, instance) + if err != nil { + return err + } + switch currentState { + case running: + return fmt.Errorf("plugin %q is already started", instance) + case notPresent: + if err := checkStartPluginRequest(name, instance, config); err != nil { + return err + } + if err := m.createPlugin(ctx, name, instance, config); err != nil { + return err + } + return m.enablePlugin(ctx, instance) + case stopped: + if err := checkRestartPluginRequest(name, instance, config); err != nil { + return err + } + return m.enablePlugin(ctx, instance) + } + return nil +} + +func (m *Manager) enablePlugin(ctx context.Context, instance string) error { + if err := m.client.PluginEnable(ctx, instance, types.PluginEnableOptions{}); err != nil { + return fmt.Errorf("failed to enable plugin: %w", err) + } + return nil +} + +func (m *Manager) createPlugin(ctx context.Context, name, instance, config string) error { f, err := os.Open(filepath.Join(pluginLocation, fmt.Sprintf("%s.tar", name))) if err != nil { return fmt.Errorf("failed to open plugin tar: %w", err) @@ -77,10 +126,48 @@ func (m *Manager) PluginStart(ctx context.Context, name, instance, config string }); err != nil { return fmt.Errorf("failed to create plugin: %w", err) } + return nil +} - if err := m.client.PluginEnable(ctx, instance, types.PluginEnableOptions{}); err != nil { - return fmt.Errorf("failed to enable plugin: %w", err) +func (m *Manager) getPluginState(ctx context.Context, instance string) (pluginState, error) { + if instance == "" { + return 0, fmt.Errorf("instance name must be specified") + } + plugins, err := m.listMatchingPlugins(ctx, instance) + if err != nil { + return 0, err } + if len(plugins) == 0 { + // plugin doesnt exist yet, we'll be starting it for the first time + return notPresent, nil + } + if len(plugins) > 1 { + return 0, fmt.Errorf("mutliple plugins found for instance name %q", instance) + } + pluginToRestart := plugins[0] + if pluginToRestart.Enabled { + // handling this is the callers responsibility, we just want to report the state. + return running, nil + } + return stopped, nil +} +func checkStartPluginRequest(name, instance, config string) error { + if instance == "" || name == "" || config == "" { + return status.Errorf(codes.InvalidArgument, + "plugin instance %q is not present."+ + " please provide the instance_name, name, config to start it."+ + " got instance_name=%[1]q, name=%q, config=%q", instance, name, config) + } + return nil +} + +func checkRestartPluginRequest(name, instance, config string) error { + if instance == "" || name != "" || config != "" { + return status.Errorf(codes.InvalidArgument, + "plugin instance %q is not enabled."+ + " please provide only the instance_name to restart it."+ + " got instance_name=%[1]q, name=%q, config=%q", instance, name, config) + } return nil } diff --git a/containers/docker/plugin_start_test.go b/containers/docker/plugin_start_test.go index 7b55ef1..d0a025d 100644 --- a/containers/docker/plugin_start_test.go +++ b/containers/docker/plugin_start_test.go @@ -4,15 +4,23 @@ import ( "context" "fmt" "io" + "os" + "path/filepath" "testing" "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" ) type fakePluginStartingDocker struct { + plugins types.PluginsListResponse fakeDocker } +func (f *fakePluginStartingDocker) PluginList(ctx context.Context, filter filters.Args) (types.PluginsListResponse, error) { + return f.plugins, nil +} + func (f *fakePluginStartingDocker) PluginCreate(ctx context.Context, createCtx io.Reader, options types.PluginCreateOptions) error { return nil } @@ -24,12 +32,13 @@ func (f *fakePluginStartingDocker) PluginEnable(ctx context.Context, name string func TestPluginStart(t *testing.T) { pluginLocation = "testdata/" tests := []struct { - name string - inName string - inInstance string - inConfig string - wantErr bool - withHook startPluginHookFunc + name string + inName string + inInstance string + inConfig string + expectedErr error + withHook startPluginHookFunc + PluginList []*types.Plugin }{ { name: "valid-plugin", @@ -37,12 +46,37 @@ func TestPluginStart(t *testing.T) { inInstance: "test-instance", inConfig: "test-config", }, + { + name: "missing-name", + inInstance: "test-instance", + inConfig: "test-config", + expectedErr: nonNilErr(t, func() error { + return checkStartPluginRequest("", "test-instance", "test-config") + }), + }, + { + name: "missing-config", + inName: "test-name", + inInstance: "test-instance", + expectedErr: nonNilErr(t, func() error { + return checkStartPluginRequest("test-name", "test-instance", "") + }), + }, + { + name: "missing-instance", + inName: "test-name", + inConfig: "test-config", + expectedErr: fmt.Errorf("instance name must be specified"), + }, { name: "invalid-plugin", inName: "no-such-plugin", inInstance: "test-instance", inConfig: "test-config", - wantErr: true, + expectedErr: func() error { + _, err := os.Open(filepath.Join(pluginLocation, "no-such-plugin.tar")) + return fmt.Errorf("failed to open plugin tar: %w", err) + }(), }, { name: "valid-plugin-working-hook", @@ -63,7 +97,75 @@ func TestPluginStart(t *testing.T) { pluginReader io.ReadCloser) (io.ReadCloser, error) { return nil, fmt.Errorf("failed hook") }, - wantErr: true, + expectedErr: fmt.Errorf( + "failed to run startup plugin hook with error failed hook"), + }, + { + name: "existing-plugin-restarted", + inInstance: "test-instance", + PluginList: []*types.Plugin{{ + Name: "test-instance:latest", + }}, + }, + { + name: "existing-plugin-bad-restart-request-config", + inInstance: "test-instance", + inConfig: "config", + PluginList: []*types.Plugin{{ + Name: "test-instance:latest", + }}, + expectedErr: nonNilErr(t, func() error { + return checkRestartPluginRequest("", "test-instance", "config") + }), + }, + { + name: "existing-plugin-bad-restart-request-name", + inInstance: "test-instance", + inName: "name", + PluginList: []*types.Plugin{{ + Name: "test-instance:latest", + }}, + expectedErr: nonNilErr(t, func() error { + return checkRestartPluginRequest("name", "test-instance", "") + }), + }, + { + name: "existing-plugin-bad-restart-request-name+config", + inInstance: "test-instance", + inName: "name", + inConfig: "config", + PluginList: []*types.Plugin{{ + Name: "test-instance:latest", + }}, + expectedErr: nonNilErr(t, func() error { + return checkRestartPluginRequest("name", "test-instance", "config") + }), + }, + { + name: "existing-plugin-running", + inInstance: "test-instance", + PluginList: []*types.Plugin{{ + Enabled: true, + Name: "test-instance:latest", + }}, + expectedErr: fmt.Errorf(`plugin "test-instance" is already started`), + }, + { + name: "no-instance-name", + inName: "data", + inInstance: "", + expectedErr: fmt.Errorf("instance name must be specified"), + }, + { + name: "multiple-matching-instances", + inInstance: "test-instance", + PluginList: []*types.Plugin{{ + Name: "test-instance:v1", + }, { + Name: "test-instance:v0", + }}, + expectedErr: fmt.Errorf( + `mutliple plugins found for instance name "test-instance"`), }, } @@ -80,11 +182,18 @@ func TestPluginStart(t *testing.T) { }) } - mgr := New(&fakePluginStartingDocker{}) - if err := mgr.PluginStart(ctx, tc.inName, tc.inInstance, - tc.inConfig); (err != nil) != tc.wantErr { - t.Errorf("PluginStart(%q, %q, %q) returned error: %v, want error=%t", - tc.inName, tc.inInstance, tc.inConfig, err, tc.wantErr) + mgr := New(&fakePluginStartingDocker{ + plugins: tc.PluginList, + }) + err := mgr.PluginStart(ctx, tc.inName, tc.inInstance, + tc.inConfig) + wantErr := tc.expectedErr != nil + if (err != nil) != wantErr { + t.Errorf("PluginStart(%q, %q, %q) returned error: %v, want error=%s", + tc.inName, tc.inInstance, tc.inConfig, err, tc.expectedErr) + } + if wantErr && err.Error() != tc.expectedErr.Error() { + t.Errorf("expected error %s, got %s", tc.expectedErr, err) } if (tc.withHook != nil) != ranHook { t.Errorf("failed to run start plugin hook") @@ -92,3 +201,11 @@ func TestPluginStart(t *testing.T) { }) } } + +func nonNilErr(t *testing.T, f func() error) error { + if err := f(); err != nil { + return err + } + t.Fatal("want non-nil error") + return nil +}