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.

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

feat: add command args for plugin #2992

Merged
merged 3 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions docs/plugins.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,20 @@ data:
metricProviderPlugins: |-
- name: "argoproj-labs/metrics"
location: "file:///tmp/argo-rollouts/metric-plugin"
args:
- "--log-level"
- "debug"
trafficRouterPlugins: |-
- name: "argoproj-labs/nginx"
location: "file:///tmp/argo-rollouts/traffic-plugin"
args:
- "--log-level"
- "debug"
```

As you can see there is a field called `name:` under both `metrics` or `trafficrouters` this is the first place where your
end users will need to configure the name of the plugin. The second location is either in the rollout object or the analysis
template which you can see the examples below.
end users will need to configure the name of the plugin. The second `location` is either in the rollout object or the analysis
template which you can see the examples below. The third `args` holds the command line arguments of the plugin.

#### AnalysisTemplate Example
```yaml
Expand Down
4 changes: 2 additions & 2 deletions metricproviders/plugin/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (m *metricPlugin) startPluginSystem(metric v1alpha1.Metric) (rpc.MetricProv
// There should only ever be one plugin defined in metric.Provider.Plugin per analysis template this gets checked
// during validation
for pluginName := range metric.Provider.Plugin {
pluginPath, err := plugin.GetPluginLocation(pluginName)
pluginPath, args, err := plugin.GetPluginInfo(pluginName)
if err != nil {
return nil, fmt.Errorf("unable to find plugin (%s): %w", pluginName, err)
}
Expand All @@ -64,7 +64,7 @@ func (m *metricPlugin) startPluginSystem(metric v1alpha1.Metric) (rpc.MetricProv
m.pluginClient[pluginName] = goPlugin.NewClient(&goPlugin.ClientConfig{
HandshakeConfig: handshakeConfig,
Plugins: pluginMap,
Cmd: exec.Command(pluginPath),
Cmd: exec.Command(pluginPath, args...),
Managed: true,
})

Expand Down
4 changes: 2 additions & 2 deletions rollout/trafficrouting/plugin/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ func (t *trafficPlugin) startPlugin(pluginName string) (rpc.TrafficRouterPlugin,

if t.pluginClient[pluginName] == nil || t.pluginClient[pluginName].Exited() {

pluginPath, err := plugin.GetPluginLocation(pluginName)
pluginPath, args, err := plugin.GetPluginInfo(pluginName)
if err != nil {
return nil, fmt.Errorf("unable to find plugin (%s): %w", pluginName, err)
}

t.pluginClient[pluginName] = goPlugin.NewClient(&goPlugin.ClientConfig{
HandshakeConfig: handshakeConfig,
Plugins: pluginMap,
Cmd: exec.Command(pluginPath),
Cmd: exec.Command(pluginPath, args...),
Managed: true,
})

Expand Down
1 change: 1 addition & 0 deletions test/e2e/appmesh_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build e2e
// +build e2e

package e2e
Expand Down
14 changes: 7 additions & 7 deletions utils/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,26 @@
"github.com/argoproj/argo-rollouts/utils/config"
)

// GetPluginLocation returns the location of the plugin on the filesystem via plugin name. If the plugin is not
// GetPluginInfo returns the location & command arguments of the plugin on the filesystem via plugin name. If the plugin is not
// configured in the configmap, an error is returned.
func GetPluginLocation(pluginName string) (string, error) {
func GetPluginInfo(pluginName string) (string, []string, error) {
configMap, err := config.GetConfig()
if err != nil {
return "", fmt.Errorf("failed to get config: %w", err)
return "", nil, fmt.Errorf("failed to get config: %w", err)

Check warning on line 17 in utils/plugin/plugin.go

View check run for this annotation

Codecov / codecov/patch

utils/plugin/plugin.go#L17

Added line #L17 was not covered by tests
}

for _, item := range configMap.GetAllPlugins() {
if pluginName == item.Name {
dir, filename, err := config.GetPluginDirectoryAndFilename(item.Name)
if err != nil {
return "", err
return "", nil, err

Check warning on line 24 in utils/plugin/plugin.go

View check run for this annotation

Codecov / codecov/patch

utils/plugin/plugin.go#L24

Added line #L24 was not covered by tests
}
absFilePath, err := filepath.Abs(filepath.Join(defaults.DefaultRolloutPluginFolder, dir, filename))
if err != nil {
return "", fmt.Errorf("failed to get absolute path of plugin folder: %w", err)
return "", nil, fmt.Errorf("failed to get absolute path of plugin folder: %w", err)

Check warning on line 28 in utils/plugin/plugin.go

View check run for this annotation

Codecov / codecov/patch

utils/plugin/plugin.go#L28

Added line #L28 was not covered by tests
}
return absFilePath, nil
return absFilePath, item.Args, nil
}
}
return "", fmt.Errorf("plugin %s not configured in configmap", pluginName)
return "", nil, fmt.Errorf("plugin %s not configured in configmap", pluginName)
}
22 changes: 16 additions & 6 deletions utils/plugin/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,31 @@ import (
"k8s.io/client-go/kubernetes/fake"
)

func TestGetPluginLocation(t *testing.T) {
func TestGetPluginInfo(t *testing.T) {
cmdArgs := []string{"-l 2"}
t.Run("tests getting plugin location of metric provider plugins", func(t *testing.T) {

cm := &v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "argo-rollouts-config",
Namespace: "argo-rollouts",
},
Data: map[string]string{"metricProviderPlugins": "\n - name: argoproj-labs/http\n location: https://test/plugin\n - name: argoproj-labs/http-sha\n location: https://test/plugin\n sha256: 74657374e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
Data: map[string]string{"metricProviderPlugins": "\n - name: argoproj-labs/http\n location: https://test/plugin\n args: [\"-l 2\"]\n - name: argoproj-labs/http-sha\n location: https://test/plugin\n sha256: 74657374e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
}
client := fake.NewSimpleClientset(cm)

_, err := config.InitializeConfig(client, "argo-rollouts-config")
assert.NoError(t, err)

location, err := GetPluginLocation("argoproj-labs/http")
location, args, err := GetPluginInfo("argoproj-labs/http")
assert.NoError(t, err)
fp, err := filepath.Abs(filepath.Join(defaults.DefaultRolloutPluginFolder, "argoproj-labs/http"))
assert.NoError(t, err)
assert.Equal(t, fp, location)
assert.Equal(t, args, cmdArgs)

_, args, _ = GetPluginInfo("argoproj-labs/http-sha")
assert.Equal(t, len(args), 0)
})

t.Run("tests getting plugin location of traffic router plugins", func(t *testing.T) {
Expand All @@ -42,18 +47,22 @@ func TestGetPluginLocation(t *testing.T) {
Name: "argo-rollouts-config",
Namespace: "argo-rollouts",
},
Data: map[string]string{"trafficRouterPlugins": "\n - name: argoproj-labs/router\n location: https://test/plugin\n - name: argoproj-labs/router-sha\n location: https://test/plugin\n sha256: 74657374e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
Data: map[string]string{"trafficRouterPlugins": "\n - name: argoproj-labs/router\n location: https://test/plugin\n args: [\"-l 2\"]\n - name: argoproj-labs/router-sha\n location: https://test/plugin\n sha256: 74657374e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
}
client := fake.NewSimpleClientset(cm)

_, err := config.InitializeConfig(client, "argo-rollouts-config")
assert.NoError(t, err)

location, err := GetPluginLocation("argoproj-labs/router")
location, args, err := GetPluginInfo("argoproj-labs/router")
assert.NoError(t, err)
fp, err := filepath.Abs(filepath.Join(defaults.DefaultRolloutPluginFolder, "argoproj-labs/router"))
assert.NoError(t, err)
assert.Equal(t, fp, location)
assert.Equal(t, args, cmdArgs)

_, args, _ = GetPluginInfo("argoproj-labs/router-sha")
assert.Equal(t, len(args), 0)
})

t.Run("test getting plugin location of a plugin that does not exists", func(t *testing.T) {
Expand All @@ -69,9 +78,10 @@ func TestGetPluginLocation(t *testing.T) {
_, err := config.InitializeConfig(client, "argo-rollouts-config")
assert.NoError(t, err)

location, err := GetPluginLocation("does-not-exist")
location, args, err := GetPluginInfo("does-not-exist")
assert.Error(t, err)
assert.Equal(t, "plugin does-not-exist not configured in configmap", err.Error())
assert.Equal(t, "", location)
assert.Equal(t, len(args), 0)
})
}
3 changes: 3 additions & 0 deletions utils/plugin/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,7 @@ type PluginItem struct {
Name string `json:"name" yaml:"name"`
Location string `json:"location" yaml:"location"`
Sha256 string `json:"sha256" yaml:"sha256"`

// Args holds command line arguments
Args []string `json:"args" yaml:"args"`
}
Loading