From 501e4cd5689436b70833ca1558f0e786da56eef6 Mon Sep 17 00:00:00 2001 From: Sveta Smirnova Date: Thu, 20 Apr 2023 03:12:52 +0300 Subject: [PATCH] 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 | 3 +- .../pt-k8s-debug-collector/dumper/dumper.go | 8 ++- src/go/pt-k8s-debug-collector/main_test.go | 52 ++++++++++++++++++- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/src/go/Makefile b/src/go/Makefile index 2b35ee85..7de80acc 100644 --- a/src/go/Makefile +++ b/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 diff --git a/src/go/pt-k8s-debug-collector/dumper/dumper.go b/src/go/pt-k8s-debug-collector/dumper/dumper.go index 3a469f7c..8c748209 100644 --- a/src/go/pt-k8s-debug-collector/dumper/dumper.go +++ b/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 []byte(fmt.Sprintf("stderr: %s, stdout: %s", errb.String(), outb.String())), nil + return outb.Bytes(), nil } func (d *Dumper) getCR(crName string, namespace string) (crSecrets, error) { diff --git a/src/go/pt-k8s-debug-collector/main_test.go b/src/go/pt-k8s-debug-collector/main_test.go index 6475583e..fb547447 100644 --- a/src/go/pt-k8s-debug-collector/main_test.go +++ b/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) + } + } +}