PT-2169 - pt-k8s-debug-collector integration of pg_gather requires croping first line of the output file (#615)

* PT-2169 - pt-k8s-debug-collector integration of pg_gather requires croping first line of the output file

Modified pt-k8s-debug-collector so it redirects only STDOUT to summary.txt
STDERR is stored in the logs and recorded in summary.txt only if summary fails with the error
Modified Makefile, so it does not include closing bracket into the version string
Created test case for the fix

* PT-2169 - pt-k8s-debug-collector integration of pg_gather requires croping first line of the output

Removed else as requested by Ege

src/go/Makefile

@@ -17,9 +17,10 @@ else
 endif
 
 GO := go
+CP := )
 pkgs = $(shell find . -type d -name "pt-*" -exec basename {} \;)
 # VERSION ?=$(shell git describe --abbrev=0) doesn't always work here, need to use git log
-VERSION ?=$(shell git log --no-walk --tags --pretty="%H %d" --decorate=short | head -n1 | awk  -F'[, ]' '{ print $$4; }')
+VERSION ?=$(shell git log --no-walk --tags --pretty="%H %d" --decorate=short | head -n1 | awk  -F'[, $(CP)]' '{ print $$4; }')
 BUILD=$(BUILD_DATE)
 GOVERSION=$(shell go version | cut --delimiter=" " -f3)
 GOUTILSDIR ?= $(GOPATH)/bin

src/go/pt-k8s-debug-collector/dumper/dumper.go

@@ -6,7 +6,6 @@ import (
 	"compress/gzip"
 	"encoding/base64"
 	"encoding/json"
-	"fmt"
 	"log"
 	"os"
 	"os/exec"
@@ -309,7 +308,7 @@ func (d *Dumper) getResource(name, namespace string, ignoreNotFound bool, tw *ta
 }
 
 func (d *Dumper) logError(err string, args ...string) {
-	d.errors += d.cmd + " " + strings.Join(args, " ") + ": " + err + "\n"
+	d.errors += d.cmd + " " + strings.Join(args, " ") + "\n" + err + "\n\n"
 }
 
 func addToArchive(location string, mode int64, content []byte, tw *tar.Writer) error {
@@ -463,10 +462,9 @@ func (d *Dumper) getPodSummary(resource, podName, crName string, namespace strin
 	cmd.Stderr = &errb
 	err := cmd.Run()
 	if err != nil {
-		return nil, errors.Errorf("error: %v, stderr: %s, stdout: %s", err, errb.String(), outb.String())
+		return nil, errors.Errorf("error: %v\nstderr: %sstdout: %s", err, errb.String(), outb.String())
 	}
-
+	return outb.Bytes(), nil
-	return []byte(fmt.Sprintf("stderr: %s, stdout: %s", errb.String(), outb.String())), nil
 }
 
 func (d *Dumper) getCR(crName string, namespace string) (crSecrets, error) {

src/go/pt-k8s-debug-collector/main_test.go

@@ -5,9 +5,9 @@ import (
 	"os"
 	"os/exec"
 	"path"
+	"regexp"
 	"strings"
 	"testing"
-	"regexp"
 
 	"golang.org/x/exp/slices"
 )
@@ -169,8 +169,56 @@ func TestVersionOption(t *testing.T) {
 		t.Errorf("error executing pt-k8s-debug-collector --version: %s", err.Error())
 	}
 	// We are using MustCompile here, because hard-coded RE should not fail
-	re := regexp.MustCompile(TOOLNAME + `\n.*Version \d+\.\d+\.\d+\n`)
+	re := regexp.MustCompile(TOOLNAME + `\n.*Version v?\d+\.\d+\.\d+\n`)
 	if !re.Match(out) {
 		t.Errorf("pt-k8s-debug-collector --version returns wrong result:\n%s", out)
 	}
 }
+
+/*
+If we handle error properly
+*/
+func TestPT_2169(t *testing.T) {
+	busyport, _ := os.Getwd() // we are using wrong socket for ssh tunnel here to ensure we get error
+
+	testcmd := []string{"sh", "-c", "tar -xf cluster-dump.tar.gz --wildcards '*/summary.txt' --to-command 'grep stderr:' 2>/dev/null | wc -l"}
+	tests := []struct {
+		name       string
+		want       string
+		port       string
+		kubeconfig string
+	}{
+		{
+			name:       "pg",
+			want:       "3",
+			port:       busyport,
+			kubeconfig: os.Getenv("KUBECONFIG_PG"),
+		},
+		{
+			name:       "pg",
+			want:       "0",
+			port:       os.Getenv("FORWARDPORT"),
+			kubeconfig: os.Getenv("KUBECONFIG_PG"),
+		},
+	}
+
+	for _, test := range tests {
+		cmd := exec.Command("../../../bin/pt-k8s-debug-collector", "--kubeconfig", test.kubeconfig, "--forwardport", test.port, "--resource", test.name)
+		if err := cmd.Run(); err != nil {
+			t.Errorf("error executing pt-k8s-debug-collector: %s", err.Error())
+		}
+		defer func() {
+			cmd = exec.Command("rm", "-f", "cluster-dump.tar.gz")
+			if err := cmd.Run(); err != nil {
+				t.Errorf("error cleaning up test data: %s", err.Error())
+			}
+		}()
+		out, err := exec.Command(testcmd[0], testcmd[1:]...).Output()
+		if err != nil {
+			t.Errorf("test %s, error running command %s:\n%s\n\nCommand output:\n%s", test.name, testcmd, err.Error(), out)
+		}
+		if strings.TrimRight(bytes.NewBuffer(out).String(), "\n") != test.want {
+			t.Errorf("test %s, output is not as expected\nOutput: %s\nWanted: %s", test.name, out, test.want)
+		}
+	}
+}