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 +}