Simplify the way mode and plugin selection is processed

- Mode will no longer remove systemd-logs when in quick mode. This requires extra configuration code and provides little to no benefit since the logs gathering plugin is so fast. Meanwhile, by removing it we simplify the code in a way that makes upcoming changes regarding e2e flags more simple. - PluginSelection (the field in the config file) will continue to be used for filtering which plugins are run on the server, but will no longer be the appropriate place to list plugins in order to run them. Modern sonobuoy code had simply been supporting this legacy logic from when sonobuoy usually only ran our default plugins and never had other fields to track them. Signed-off-by: John Schnake <[email protected]>
vmware-tanzu · Jul 1, 2021 · 564f622 · 564f622
1 parent 7af6504
commit 564f622
Show file tree

Hide file tree

Showing 8 changed files with 165 additions and 34 deletions.
diff --git a/cmd/sonobuoy/app/gen.go b/cmd/sonobuoy/app/gen.go
@@ -23,7 +23,6 @@ import (
 	"github.com/vmware-tanzu/sonobuoy/pkg/client"
 	"github.com/vmware-tanzu/sonobuoy/pkg/config"
 	"github.com/vmware-tanzu/sonobuoy/pkg/errlog"
-	"github.com/vmware-tanzu/sonobuoy/pkg/image"
 	imagepkg "github.com/vmware-tanzu/sonobuoy/pkg/image"
 
 	"github.com/pkg/errors"
@@ -122,7 +121,7 @@ func (g *genFlags) Config() (*client.GenConfig, error) {
 	var e2eImage, e2eRegistry, imageVersion string
 
 	switch g.k8sVersion {
-	case "", image.ConformanceImageVersionAuto, image.ConformanceImageVersionLatest, image.ConformanceImageVersionIgnore:
+	case "", imagepkg.ConformanceImageVersionAuto, imagepkg.ConformanceImageVersionLatest, imagepkg.ConformanceImageVersionIgnore:
 		var discoveryClient discovery.ServerVersionInterface
 		if kubeclient != nil {
 			discoveryClient = kubeclient.DiscoveryClient
@@ -134,7 +133,7 @@ func (g *genFlags) Config() (*client.GenConfig, error) {
 		e2eRegistry, imageVersion, err = g.k8sVersion.Get(discoveryClient, imagepkg.DevVersionURL)
 		if err != nil {
 			if errors.Cause(err) == imagepkg.ErrImageVersionNoClient &&
-				g.k8sVersion != image.ConformanceImageVersionIgnore {
+				g.k8sVersion != imagepkg.ConformanceImageVersionIgnore {
 				return nil, errors.Wrap(err, kubeError.Error())
 			}
 			return nil, err

diff --git a/pkg/client/gen.go b/pkg/client/gen.go
@@ -102,20 +102,13 @@ func (*SonobuoyClient) GenerateManifest(cfg *GenConfig) ([]byte, error) {
 		}
 	}
 
-	// Support legacy logic for the time being.
+	// If the user didnt provide plugins at all fallback to our original
+	// defaults. Legacy logic was that the user could specify plugins via the
+	// config.PluginSelection field but since the CLI handles custom plugins
+	// we moved to the list of actual plugin data. The PluginSelection is only
+	// a server-side capability to run a subset of available plugins.
 	if len(cfg.DynamicPlugins) == 0 && len(cfg.StaticPlugins) == 0 {
-		if conf.PluginSelections != nil {
-			// Empty (but non-nil) means run nothing. Setting any value means run
-			// those explicitly.
-			for _, v := range conf.PluginSelections {
-				cfg.DynamicPlugins = append(cfg.DynamicPlugins, v.Name)
-			}
-		} else {
-			// Nil plugin selection now means to run all plugins that are loaded.
-			// If the user didnt provide plugins at all fallback to our original
-			// defaults.
-			cfg.DynamicPlugins = []string{e2ePluginName, systemdLogsName}
-		}
+		cfg.DynamicPlugins = []string{e2ePluginName, systemdLogsName}
 	}
 
 	plugins := []*manifest.Manifest{}

diff --git a/pkg/client/gen_test.go b/pkg/client/gen_test.go
@@ -192,7 +192,7 @@ func TestGenerateManifestGolden(t *testing.T) {
 			},
 			goldenFile: filepath.Join("testdata", "systemd-logs-default.golden"),
 		}, {
-			name: "Enabling SSH (legacy plugin choice)",
+			name: "Enabling SSH (via legacy plugin choice)",
 			inputcm: &client.GenConfig{
 				E2EConfig: &client.E2EConfig{},
 				Config: &config.Config{
@@ -203,15 +203,15 @@ func TestGenerateManifestGolden(t *testing.T) {
 			},
 			goldenFile: filepath.Join("testdata", "ssh.golden"),
 		}, {
-			name: "Empty array leads to 0 plugins (legacy plugin choice)",
+			name: "Empty array leads to default plugins, not 0",
 			inputcm: &client.GenConfig{
 				E2EConfig: &client.E2EConfig{},
 				Config: fromConfig(func(c *config.Config) *config.Config {
 					c.PluginSelections = []plugin.Selection{}
 					return c
 				}),
 			},
-			goldenFile: filepath.Join("testdata", "no-plugins-via-selection.golden"),
+			goldenFile: filepath.Join("testdata", "default-plugins-via-selection.golden"),
 		}, {
 			// For backwards compatibility.
 			name: "Nil plugin selection and no manual choice leads to e2e/systemd",

diff --git a/pkg/client/mode.go b/pkg/client/mode.go
@@ -19,8 +19,6 @@ package client
 import (
 	"fmt"
 	"strings"
-
-	"github.com/vmware-tanzu/sonobuoy/pkg/plugin"
 )
 
 // Mode identifies a specific mode of running Sonobuoy.
@@ -57,8 +55,6 @@ var modeMap = map[string]Mode{
 type ModeConfig struct {
 	// E2EConfig is the focus and skip vars for the conformance tests.
 	E2EConfig E2EConfig
-	// Selectors are the plugins selected by this mode.
-	Selectors []plugin.Selection
 }
 
 // String needed for pflag.Value
@@ -89,10 +85,6 @@ func (m *Mode) Get() *ModeConfig {
 				Focus:    `\[Conformance\]`,
 				Parallel: "false",
 			},
-			Selectors: []plugin.Selection{
-				{Name: "e2e"},
-				{Name: "systemd-logs"},
-			},
 		}
 	case NonDisruptiveConformance:
 		return &ModeConfig{
@@ -101,20 +93,13 @@ func (m *Mode) Get() *ModeConfig {
 				Skip:     nonDisruptiveSkipList,
 				Parallel: "false",
 			},
-			Selectors: []plugin.Selection{
-				{Name: "e2e"},
-				{Name: "systemd-logs"},
-			},
 		}
 	case Quick:
 		return &ModeConfig{
 			E2EConfig: E2EConfig{
 				Focus:    "Pods should be submitted and removed",
 				Parallel: "false",
 			},
-			Selectors: []plugin.Selection{
-				{Name: "e2e"},
-			},
 		}
 	default:
 		return nil

diff --git a/.../testdata/no-plugins-via-selection.golden → ...data/default-plugins-via-selection.golden b/.../testdata/no-plugins-via-selection.golden → ...data/default-plugins-via-selection.golden
@@ -24,6 +24,69 @@ metadata:
   namespace: sonobuoy
 ---
 apiVersion: v1
+data:
+  plugin-0.yaml: |
+    podSpec:
+      containers: []
+      nodeSelector:
+        kubernetes.io/os: linux
+      restartPolicy: Never
+      serviceAccountName: sonobuoy-serviceaccount
+      tolerations:
+      - effect: NoSchedule
+        key: node-role.kubernetes.io/master
+        operator: Exists
+      - key: CriticalAddonsOnly
+        operator: Exists
+      - key: kubernetes.io/e2e-evict-taint-key
+        operator: Exists
+    sonobuoy-config:
+      driver: Job
+      plugin-name: e2e
+      result-format: junit
+    spec:
+      command:
+      - /run_e2e.sh
+      env:
+      - name: E2E_FOCUS
+      - name: E2E_PARALLEL
+      - name: E2E_SKIP
+      - name: E2E_USE_GO_RUNNER
+        value: "true"
+      name: e2e
+      resources: {}
+      volumeMounts:
+      - mountPath: /tmp/results
+        name: results
+  plugin-1.yaml: |
+    sonobuoy-config:
+      driver: DaemonSet
+      plugin-name: systemd-logs
+      result-format: raw
+    spec:
+      command:
+      - /bin/sh
+      - -c
+      - /get_systemd_logs.sh; while true; do echo "Plugin is complete. Sleeping indefinitely
+        to avoid container exit and automatic restarts from Kubernetes"; sleep 3600; done
+      env:
+      - name: CHROOT_DIR
+        value: /node
+      - name: NODE_NAME
+        valueFrom:
+          fieldRef:
+            fieldPath: spec.nodeName
+      - name: RESULTS_DIR
+        value: /tmp/results
+      name: systemd-logs
+      resources: {}
+      securityContext:
+        privileged: true
+      volumeMounts:
+      - mountPath: /tmp/results
+        name: results
+      - mountPath: /node
+        name: root
 kind: ConfigMap
 metadata:
   labels:

diff --git a/pkg/client/testdata/e2e-default.golden b/pkg/client/testdata/e2e-default.golden
@@ -58,6 +58,35 @@ data:
       volumeMounts:
       - mountPath: /tmp/results
         name: results
+  plugin-1.yaml: |
+    sonobuoy-config:
+      driver: DaemonSet
+      plugin-name: systemd-logs
+      result-format: raw
+    spec:
+      command:
+      - /bin/sh
+      - -c
+      - /get_systemd_logs.sh; while true; do echo "Plugin is complete. Sleeping indefinitely
+        to avoid container exit and automatic restarts from Kubernetes"; sleep 3600; done
+      env:
+      - name: CHROOT_DIR
+        value: /node
+      - name: NODE_NAME
+        valueFrom:
+          fieldRef:
+            fieldPath: spec.nodeName
+      - name: RESULTS_DIR
+        value: /tmp/results
+      name: systemd-logs
+      resources: {}
+      securityContext:
+        privileged: true
+      volumeMounts:
+      - mountPath: /tmp/results
+        name: results
+      - mountPath: /node
+        name: root
 kind: ConfigMap
 metadata:
   labels:

diff --git a/pkg/client/testdata/ssh.golden b/pkg/client/testdata/ssh.golden
@@ -82,6 +82,35 @@ data:
         name: results
       - mountPath: /root/.ssh
         name: sshkey-vol
+  plugin-1.yaml: |
+    sonobuoy-config:
+      driver: DaemonSet
+      plugin-name: systemd-logs
+      result-format: raw
+    spec:
+      command:
+      - /bin/sh
+      - -c
+      - /get_systemd_logs.sh; while true; do echo "Plugin is complete. Sleeping indefinitely
+        to avoid container exit and automatic restarts from Kubernetes"; sleep 3600; done
+      env:
+      - name: CHROOT_DIR
+        value: /node
+      - name: NODE_NAME
+        valueFrom:
+          fieldRef:
+            fieldPath: spec.nodeName
+      - name: RESULTS_DIR
+        value: /tmp/results
+      name: systemd-logs
+      resources: {}
+      securityContext:
+        privileged: true
+      volumeMounts:
+      - mountPath: /tmp/results
+        name: results
+      - mountPath: /node
+        name: root
 kind: ConfigMap
 metadata:
   labels:

diff --git a/pkg/client/testdata/systemd-logs-default.golden b/pkg/client/testdata/systemd-logs-default.golden
@@ -26,6 +26,39 @@ metadata:
 apiVersion: v1
 data:
   plugin-0.yaml: |
+    podSpec:
+      containers: []
+      nodeSelector:
+        kubernetes.io/os: linux
+      restartPolicy: Never
+      serviceAccountName: sonobuoy-serviceaccount
+      tolerations:
+      - effect: NoSchedule
+        key: node-role.kubernetes.io/master
+        operator: Exists
+      - key: CriticalAddonsOnly
+        operator: Exists
+      - key: kubernetes.io/e2e-evict-taint-key
+        operator: Exists
+    sonobuoy-config:
+      driver: Job
+      plugin-name: e2e
+      result-format: junit
+    spec:
+      command:
+      - /run_e2e.sh
+      env:
+      - name: E2E_FOCUS
+      - name: E2E_PARALLEL
+      - name: E2E_SKIP
+      - name: E2E_USE_GO_RUNNER
+        value: "true"
+      name: e2e
+      resources: {}
+      volumeMounts:
+      - mountPath: /tmp/results
+        name: results
+  plugin-1.yaml: |
     sonobuoy-config:
       driver: DaemonSet
       plugin-name: systemd-logs