Reconcile results directory and mount on server

pkg/client/gen.go

@@ -29,6 +29,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/vmware-tanzu/sonobuoy/pkg/config"
+	"github.com/vmware-tanzu/sonobuoy/pkg/discovery"
 	"github.com/vmware-tanzu/sonobuoy/pkg/errlog"
 	kutil "github.com/vmware-tanzu/sonobuoy/pkg/k8s"
 	"github.com/vmware-tanzu/sonobuoy/pkg/plugin/driver"
@@ -195,7 +196,7 @@ func (*SonobuoyClient) GenerateManifestAndPlugins(cfg *GenConfig) ([]byte, []*ma
 	}
 
 	cfg.PluginEnvOverrides, plugins = applyAutoEnvVars(cfg.KubeVersion, cfg.Config.ResultsDir, cfg.Config.ProgressUpdatesPort, cfg.PluginEnvOverrides, plugins)
-	autoAttachResultsDir(plugins, cfg.Config.ResultsDir)
+	discovery.AutoAttachResultsDir(plugins, cfg.Config.ResultsDir)
 	if err := applyEnvOverrides(cfg.PluginEnvOverrides, plugins); err != nil {
 		return nil, nil, err
 	}
@@ -667,39 +668,6 @@ func processEnvVals(envVars map[string]string) ([]corev1.EnvVar, map[string]stru
 	return newEnv, removeVals
 }
 
-// autoAttachResultsDir will either add the volumemount for the results dir or modify the existing
-// one to have the right path set.
-func autoAttachResultsDir(plugins []*manifest.Manifest, resultsDir string) {
-	for pluginIndex := range plugins {
-		containers := []*corev1.Container{&plugins[pluginIndex].Spec.Container}
-		if plugins[pluginIndex].PodSpec != nil {
-			for containerIndex := range plugins[pluginIndex].PodSpec.Containers {
-				containers = append(containers, &plugins[pluginIndex].PodSpec.Containers[containerIndex])
-			}
-		}
-		addOrUpdateResultsMount(resultsDir, containers...)
-	}
-}
-
-func addOrUpdateResultsMount(resultsDir string, containers ...*corev1.Container) {
-	for ci := range containers {
-		foundOnPlugin := false
-		for vmi, vm := range containers[ci].VolumeMounts {
-			if vm.Name == "results" {
-				containers[ci].VolumeMounts[vmi].MountPath = resultsDir
-				foundOnPlugin = true
-				break
-			}
-		}
-		if !foundOnPlugin {
-			containers[ci].VolumeMounts = append(containers[ci].VolumeMounts, corev1.VolumeMount{
-				Name:      "results",
-				MountPath: resultsDir,
-			})
-		}
-	}
-}
-
 func applyAutoEnvVars(imageVersion, resultsDir, progressPort string, env map[string]map[string]string, plugins []*manifest.Manifest) (map[string]map[string]string, []*manifest.Manifest) {
 	// Set env on all plugins and swap out dynamic images.
 	if env == nil {

pkg/discovery/discovery.go

@@ -30,9 +30,14 @@ import (
 	"github.com/vmware-tanzu/sonobuoy/pkg/client/results"
 	"github.com/vmware-tanzu/sonobuoy/pkg/config"
 	"github.com/vmware-tanzu/sonobuoy/pkg/errlog"
+	kutil "github.com/vmware-tanzu/sonobuoy/pkg/k8s"
 	"github.com/vmware-tanzu/sonobuoy/pkg/plugin"
 	pluginaggregation "github.com/vmware-tanzu/sonobuoy/pkg/plugin/aggregation"
+	"github.com/vmware-tanzu/sonobuoy/pkg/plugin/driver/daemonset"
+	"github.com/vmware-tanzu/sonobuoy/pkg/plugin/driver/job"
+	"github.com/vmware-tanzu/sonobuoy/pkg/plugin/manifest"
 	"github.com/vmware-tanzu/sonobuoy/pkg/tarball"
+	corev1 "k8s.io/api/core/v1"
 
 	"github.com/pkg/errors"
 	"github.com/rifflock/lfshook"
@@ -49,6 +54,9 @@ const (
 	// MetaLocation is the place under which snapshot metadata (query times, config) is stored
 	// Also stored in query.go
 	MetaLocation = "meta"
+
+	sonobuoyResultsDirKey       = "SONOBUOY_RESULTS_DIR"
+	legacySonobuoyResultsDirKey = "RESULTS_DIR"
 )
 
 type RunInfo struct {
@@ -134,6 +142,7 @@ func Run(restConf *rest.Config, cfg *config.Config) (errCount int) {
 	}
 
 	// 4. Run the plugin aggregator. Save this error for clear logging later.
+	avoidResultsDirIssue(cfg.LoadedPlugins, cfg.ResultsDir)
 	runErr := pluginaggregation.Run(kubeClient, cfg.LoadedPlugins, cfg.Aggregation, cfg.ProgressUpdatesPort, cfg.ResultsDir, cfg.Namespace, outpath)
 	trackErrorsFor("running plugins")(runErr)
 
@@ -224,6 +233,34 @@ func Run(restConf *rest.Config, cfg *config.Config) (errCount int) {
 	return errCount
 }
 
+// avoidResultsDirIssue prevents issue #1688; a mismatch between the
+// resultsDir specified in the config and what is on the plugins. If that happens
+// the worker and plugin fail to communicate and the plugin hangs.
+func avoidResultsDirIssue(plugins []plugin.Interface, dir string) {
+	if os.Getenv("SONOBUOY_DISABLE_RECONCILIATION") == "true" {
+		logrus.Debugln("Skipping transforms to avoid issues with legacy RESULTS_DIR since SONOBUOY_DISABLE_RECONCILIATION=true was set on the aggregator pod.")
+		return
+	}
+
+	logrus.Debugln("Applying transforms to avoid issues with legacy RESULTS_DIR. To skip this transformation, set SONOBUOY_DISABLE_RECONCILIATION=true on the aggregator pod.")
+	dirEnvVars := []corev1.EnvVar{
+		{Name: sonobuoyResultsDirKey, Value: dir},
+		{Name: legacySonobuoyResultsDirKey, Value: dir},
+	}
+	for _, p := range plugins {
+		if j, ok := p.(*job.Plugin); ok {
+			AutoAttachResultsDir([]*manifest.Manifest{&j.Definition}, dir)
+			j.Definition.Spec.Env = kutil.MergeEnv(dirEnvVars, j.Definition.Spec.Env, nil)
+			continue
+		}
+		if ds, ok := p.(*daemonset.Plugin); ok {
+			AutoAttachResultsDir([]*manifest.Manifest{&ds.Definition}, dir)
+			ds.Definition.Spec.Env = kutil.MergeEnv(dirEnvVars, ds.Definition.Spec.Env, nil)
+			continue
+		}
+	}
+}
+
 func statusCounts(item *results.Item, startingCounts map[string]int) {
 	if item == nil {
 		return
@@ -361,3 +398,36 @@ func setPodStatusAnnotation(client kubernetes.Interface, namespace string, statu
 	_, err = client.CoreV1().Pods(namespace).Patch(context.TODO(), podName, types.MergePatchType, patchBytes, metav1.PatchOptions{})
 	return err
 }
+
+// AutoAttachResultsDir will either add the volumemount for the results dir or modify the existing
+// one to have the right path set.
+func AutoAttachResultsDir(plugins []*manifest.Manifest, resultsDir string) {
+	for pluginIndex := range plugins {
+		containers := []*corev1.Container{&plugins[pluginIndex].Spec.Container}
+		if plugins[pluginIndex].PodSpec != nil {
+			for containerIndex := range plugins[pluginIndex].PodSpec.Containers {
+				containers = append(containers, &plugins[pluginIndex].PodSpec.Containers[containerIndex])
+			}
+		}
+		addOrUpdateResultsMount(resultsDir, containers...)
+	}
+}
+
+func addOrUpdateResultsMount(resultsDir string, containers ...*corev1.Container) {
+	for ci := range containers {
+		foundOnPlugin := false
+		for vmi, vm := range containers[ci].VolumeMounts {
+			if vm.Name == "results" {
+				containers[ci].VolumeMounts[vmi].MountPath = resultsDir
+				foundOnPlugin = true
+				break
+			}
+		}
+		if !foundOnPlugin {
+			containers[ci].VolumeMounts = append(containers[ci].VolumeMounts, corev1.VolumeMount{
+				Name:      "results",
+				MountPath: resultsDir,
+			})
+		}
+	}
+}

test/integration/sonobuoy_integration_test.go

@@ -347,6 +347,60 @@ func TestQuick(t *testing.T) {
 	})
 }
 
+// TestQuickLegacyFix runs a real "--mode quick" check against the cluster with the yaml from v0.54.0
+// which suffered from issues with agreement regarding ResultsDir.
+func TestQuickLegacyFix(t *testing.T) {
+	t.Parallel()
+	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+
+	// Hardcoded namespace due to nature of test being from a file.
+	ns, cleanup := getNamespace(t)
+	// Doing a deletion check here rather than as a separate test so-as not to waste the extra compute time.
+	defer func(t *testing.T, ns string) {
+		if err := deleteComplete(t, ns); err != nil {
+			t.Fatalf("Failed to completely delete resources: %v", err)
+		}
+	}(t, ns)
+	defer cleanup(true)
+
+	// Get and modify data so it targets the right sonobuoy image and namespace.
+	runData, err := ioutil.ReadFile("./testdata/issue1688.yaml")
+	if err != nil {
+		t.Fatalf("Failed to read run data file: %v", err)
+	}
+
+	tmpfile, err := ioutil.TempFile("", "")
+	if err != nil {
+		t.Fatalf("Failed to create necessary tmpfile: %v", err)
+	}
+
+	curVersion := mustRunSonobuoyCommandWithContext(context.Background(), t, ns, "version --short")
+	imgName := strings.TrimSpace(fmt.Sprintf("sonobuoy/sonobuoy:%v", curVersion.String()))
+	runData = bytes.ReplaceAll(runData, []byte("REPLACE_NS"), []byte(ns))
+	runData = bytes.ReplaceAll(runData, []byte("REPLACE_IMAGE"), []byte(imgName))
+
+	if _, err := tmpfile.Write(runData); err != nil {
+		t.Fatalf("Failed to rewrite test data as needed for 1688 test: %v", err)
+	}
+	if err := tmpfile.Close(); err != nil {
+		t.Fatalf("Failed to close tmpfile: %v", err)
+	}
+
+	// Use kubectl to apply the run so that we don't have the CLI modifying the data.
+	args := fmt.Sprintf("apply -f %v", tmpfile.Name())
+	if out, err := runCommandWithContext(context.TODO(), t, kubectl, args); err != nil {
+		t.Fatalf("Failed to launch run for 1688: %v %v", err, out.String())
+	}
+
+	// Now we can use sonobuoy to wait for results.
+	mustRunSonobuoyCommandWithContext(ctx, t, ns, fmt.Sprintf("wait -n %v", ns))
+
+	checkStatusForPluginErrors(ctx, t, ns, "e2ecustom", 0)
+	tb := mustDownloadTarball(ctx, t, ns)
+	tb = saveToArtifacts(t, tb)
+}
+
 // deleteComplete is the logic that checks that we deleted the namespace and our clusterRole[Bindings]
 func deleteComplete(t *testing.T, ns string) error {
 	out, err := runCommandWithContext(context.TODO(), t, kubectl, fmt.Sprintf("get clusterroles sonobuoy-serviceaccount-%v -o yaml", ns))