Show logs for a specific plugin (#868)

Add a new flag to the `logs` command to show the logs only for a
specific plugin. If not provided, or no pods are found for the plugin,
default to showing all logs.

We didn't have a way to uniquely identify pods for a plugin, so this
change adds a new label when creating Pods and DaemonSets which can
be used to filter the pods. This means that the flag will not work
for older versions of Sonobuoy, but it will still default to showing
all logs which was the original behaviour.

Signed-off-by: Bridget McErlean <[email protected]>

cmd/sonobuoy/app/logs.go

@@ -35,6 +35,7 @@ const (
 var logFlags struct {
 	namespace  string
 	follow     bool
+	plugin     string
 	kubeconfig Kubeconfig
 }
 
@@ -52,6 +53,7 @@ func NewCmdLogs() *cobra.Command {
 	)
 	AddKubeconfigFlag(&logFlags.kubeconfig, cmd.Flags())
 	AddNamespaceFlag(&logFlags.namespace, cmd.Flags())
+	cmd.Flags().StringVarP(&logFlags.plugin, pluginFlag, "p", "", "Show logs for a specific plugin")
 	return cmd
 }
 
@@ -65,6 +67,7 @@ func getLogs(cmd *cobra.Command, args []string) {
 	logConfig := client.NewLogConfig()
 	logConfig.Namespace = logFlags.namespace
 	logConfig.Follow = logFlags.follow
+	logConfig.Plugin = logFlags.plugin
 
 	logreader, err := sbc.LogReader(logConfig)
 	if err != nil {

pkg/client/interfaces.go

@@ -40,6 +40,8 @@ type LogConfig struct {
 	Follow bool
 	// Namespace is the namespace the sonobuoy aggregator is running in.
 	Namespace string
+	// Plugin is the name of the plugin to show the logs of.
+	Plugin string
 	// Out is the writer to write to.
 	Out io.Writer
 }

pkg/client/logs.go

@@ -22,6 +22,7 @@ import (
 	"sync"
 
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -109,6 +110,32 @@ func (r *Reader) Read(p []byte) (int, error) {
 	return len(data), nil
 }
 
+// getPodsForLogs retrieves the pods to stream logs from. If a plugin name has been provided, retrieve the pods with
+// only the plugin label matching that plugin name. If no pods are found, or no plugin has been specified, retrieve
+// all pods within the namespace.
+func getPodsForLogs(client kubernetes.Interface, cfg *LogConfig) (*v1.PodList, error) {
+	if cfg.Plugin != "" {
+		selector := metav1.AddLabelToSelector(&metav1.LabelSelector{}, "sonobuoy-plugin", cfg.Plugin)
+		options := metav1.ListOptions{LabelSelector: metav1.FormatLabelSelector(selector)}
+		pods, err := client.CoreV1().Pods(cfg.Namespace).List(options)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to list pods")
+		}
+
+		if len(pods.Items) != 0 {
+			return pods, nil
+		}
+
+		logrus.Warningf("failed to find pods for plugin %q, defaulting to all pods", cfg.Plugin)
+	}
+
+	pods, err := client.CoreV1().Pods(cfg.Namespace).List(metav1.ListOptions{})
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to list pods")
+	}
+	return pods, nil
+}
+
 // LogReader configures a Reader that provides an io.Reader interface to a merged stream of logs from various containers.
 func (s *SonobuoyClient) LogReader(cfg *LogConfig) (*Reader, error) {
 	if cfg == nil {
@@ -124,9 +151,9 @@ func (s *SonobuoyClient) LogReader(cfg *LogConfig) (*Reader, error) {
 		return nil, err
 	}
 
-	pods, err := client.CoreV1().Pods(cfg.Namespace).List(metav1.ListOptions{})
+	pods, err := getPodsForLogs(client, cfg)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to list pods")
+		return nil, err
 	}
 
 	// We must make sure the error channel has capacity enough so that it never blocks

pkg/client/logs_test.go

@@ -21,6 +21,11 @@ import (
 	"testing"
 
 	"github.com/pkg/errors"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes/fake"
+	k8stesting "k8s.io/client-go/testing"
 )
 
 func TestLateErrors(t *testing.T) {
@@ -244,3 +249,106 @@ func TestLogReaderInvalidConfig(t *testing.T) {
 		}
 	}
 }
+
+func TestPodsForLogs(t *testing.T) {
+	pluginName := "my-plugin"
+	pluginPod := corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Labels: map[string]string{
+				"sonobuoy-plugin": pluginName,
+			},
+		},
+	}
+	allPods := &corev1.PodList{Items: []corev1.Pod{pluginPod, corev1.Pod{}}}
+	testCases := []struct {
+		desc                   string
+		pluginName             string
+		expectedCallCount      int
+		expectedLabelSelectors []string
+		podListErrors          []error
+		expectedPodCount       int
+		expectedError          error
+	}{
+		{
+			desc:                   "No plugin specified results in all pods being fetched once",
+			pluginName:             "",
+			expectedCallCount:      1,
+			expectedLabelSelectors: []string{""},
+			podListErrors:          []error{nil},
+			expectedPodCount:       2,
+			expectedError:          nil,
+		},
+		{
+			desc:                   "Plugin specified results in plugin pods being fetched once",
+			pluginName:             pluginName,
+			expectedCallCount:      1,
+			expectedLabelSelectors: []string{"sonobuoy-plugin=my-plugin"},
+			podListErrors:          []error{nil},
+			expectedPodCount:       1,
+			expectedError:          nil,
+		},
+		{
+			desc:                   "Error when fetching plugin pods results in error being returned",
+			pluginName:             pluginName,
+			expectedCallCount:      1,
+			expectedLabelSelectors: []string{"sonobuoy-plugin=my-plugin"},
+			podListErrors:          []error{errors.New("error")},
+			expectedPodCount:       1,
+			expectedError:          errors.New("failed to list pods: error"),
+		},
+		{
+			desc:                   "Unknown plugin specified results in plugin pods being fetched, then retry with all pods fetched",
+			pluginName:             "unknown-plugin",
+			expectedCallCount:      2,
+			expectedLabelSelectors: []string{"sonobuoy-plugin=unknown-plugin", ""},
+			podListErrors:          []error{nil, nil},
+			expectedPodCount:       2,
+			expectedError:          nil,
+		},
+		{
+			desc:                   "Error when fetching plugin pods on retry results in error being returned",
+			pluginName:             "unknown-plugin",
+			expectedCallCount:      2,
+			expectedLabelSelectors: []string{"sonobuoy-plugin=unknown-plugin", ""},
+			podListErrors:          []error{nil, errors.New("error")},
+			expectedPodCount:       2,
+			expectedError:          errors.New("failed to list pods: error"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			cfg := &LogConfig{Plugin: tc.pluginName}
+
+			callCount := 0
+			fclient := fake.NewSimpleClientset()
+			fclient.PrependReactor("list", "pods", func(action k8stesting.Action) (bool, runtime.Object, error) {
+				if callCount >= tc.expectedCallCount {
+					t.Fatal("Unexpected call to list pods")
+				}
+
+				listAction := action.(k8stesting.ListAction)
+				labelSelector := listAction.GetListRestrictions().Labels.String()
+				if labelSelector != tc.expectedLabelSelectors[callCount] {
+					t.Errorf("expected label selector to be %q, got %q", tc.expectedLabelSelectors[callCount], labelSelector)
+				}
+				err := tc.podListErrors[callCount]
+				callCount++
+				return true, allPods, err
+			})
+
+			pods, err := getPodsForLogs(fclient, cfg)
+			if tc.expectedError != nil {
+				if err.Error() != tc.expectedError.Error() {
+					t.Errorf("Unexpected error result, expected %q, got %q", tc.expectedError, err)
+				}
+
+			} else {
+				if len(pods.Items) != tc.expectedPodCount {
+					t.Errorf("Unexpected number of pods returned, expected %d, got %d", tc.expectedPodCount, len(pods.Items))
+				}
+			}
+
+		})
+	}
+}

pkg/plugin/driver/daemonset/daemonset.go

@@ -91,9 +91,10 @@ func (p *Plugin) createDaemonSetDefinition(hostname string, cert *tls.Certificat
 		annotations[k] = v
 	}
 	labels := map[string]string{
-		"component":    "sonobuoy",
-		"tier":         "analysis",
-		"sonobuoy-run": p.SessionID,
+		"component":       "sonobuoy",
+		"tier":            "analysis",
+		"sonobuoy-run":    p.SessionID,
+		"sonobuoy-plugin": p.GetName(),
 	}
 
 	ds.ObjectMeta = metav1.ObjectMeta{

pkg/plugin/driver/daemonset/daemonset_test.go

@@ -57,11 +57,12 @@ func createClientCertificate(name string) (*tls.Certificate, error) {
 }
 
 func TestCreateDaemonSetDefintion(t *testing.T) {
+	pluginName := "test-plugin"
 	testDaemonSet := NewPlugin(
 		manifest.Manifest{
 			SonobuoyConfig: manifest.SonobuoyConfig{
 				Driver:     "DaemonSet",
-				PluginName: "test-plugin",
+				PluginName: pluginName,
 			},
 			Spec: manifest.Container{
 				Container: corev1.Container{
@@ -96,14 +97,14 @@ func TestCreateDaemonSetDefintion(t *testing.T) {
 	if err != nil {
 		t.Fatalf("couldn't make CA Authority %v", err)
 	}
-	clientCert, err := auth.ClientKeyPair("test-job")
+	clientCert, err := auth.ClientKeyPair(pluginName)
 	if err != nil {
 		t.Fatalf("couldn't make client certificate %v", err)
 	}
 
 	daemonSet := testDaemonSet.createDaemonSetDefinition("", clientCert, &corev1.Pod{})
 
-	expectedName := fmt.Sprintf("sonobuoy-test-plugin-daemon-set-%v", testDaemonSet.SessionID)
+	expectedName := fmt.Sprintf("sonobuoy-%v-daemon-set-%v", pluginName, testDaemonSet.SessionID)
 	if daemonSet.Name != expectedName {
 		t.Errorf("Expected daemonSet name %v, got %v", expectedName, daemonSet.Name)
 	}
@@ -112,6 +113,10 @@ func TestCreateDaemonSetDefintion(t *testing.T) {
 		t.Errorf("Expected daemonSet namespace %v, got %v", expectedNamespace, daemonSet.Namespace)
 	}
 
+	pluginLabel := "sonobuoy-plugin"
+	if daemonSet.Labels[pluginLabel] != pluginName {
+		t.Errorf("Expected daemonSet to have label %q with value %q, but had value %q", pluginLabel, pluginName, daemonSet.Labels[pluginLabel])
+	}
 	containers := daemonSet.Spec.Template.Spec.Containers
 
 	expectedContainers := 2

pkg/plugin/driver/job/job.go

@@ -84,9 +84,10 @@ func (p *Plugin) createPodDefinition(hostname string, cert *tls.Certificate, own
 		annotations[k] = v
 	}
 	labels := map[string]string{
-		"component":    "sonobuoy",
-		"tier":         "analysis",
-		"sonobuoy-run": p.SessionID,
+		"component":       "sonobuoy",
+		"tier":            "analysis",
+		"sonobuoy-run":    p.SessionID,
+		"sonobuoy-plugin": p.GetName(),
 	}
 
 	pod.ObjectMeta = metav1.ObjectMeta{

pkg/plugin/driver/job/job_test.go

@@ -58,10 +58,11 @@ func createClientCertificate(name string) (*tls.Certificate, error) {
 }
 
 func TestCreatePodDefinition(t *testing.T) {
+	pluginName := "test-job"
 	testPlugin := NewPlugin(
 		manifest.Manifest{
 			SonobuoyConfig: manifest.SonobuoyConfig{
-				PluginName: "test-job",
+				PluginName: pluginName,
 			},
 			Spec: manifest.Container{
 				Container: corev1.Container{
@@ -97,14 +98,14 @@ func TestCreatePodDefinition(t *testing.T) {
 		t.Fatalf("couldn't make CA Authority %v", err)
 	}
 
-	clientCert, err := auth.ClientKeyPair("test-job")
+	clientCert, err := auth.ClientKeyPair(pluginName)
 	if err != nil {
 		t.Fatalf("couldn't make client certificate %v", err)
 	}
 
 	pod := testPlugin.createPodDefinition("", clientCert, &corev1.Pod{})
 
-	expectedName := fmt.Sprintf("sonobuoy-test-job-job-%v", testPlugin.SessionID)
+	expectedName := fmt.Sprintf("sonobuoy-%v-job-%v", pluginName, testPlugin.SessionID)
 	if pod.Name != expectedName {
 		t.Errorf("Expected pod name %v, got %v", expectedName, pod.Name)
 	}
@@ -113,6 +114,11 @@ func TestCreatePodDefinition(t *testing.T) {
 		t.Errorf("Expected pod namespace %v, got %v", expectedNamespace, pod.Namespace)
 	}
 
+	pluginLabel := "sonobuoy-plugin"
+	if pod.Labels[pluginLabel] != pluginName {
+		t.Errorf("Expected pod to have label %q with value %q, but had value %q", pluginLabel, pluginName, pod.Labels[pluginLabel])
+	}
+
 	expectedContainers := 2
 	if len(pod.Spec.Containers) != expectedContainers {
 		t.Errorf("Expected to have %v containers, got %v", expectedContainers, len(pod.Spec.Containers))