From b8629452453a4ba60d7214998c147df2651275da Mon Sep 17 00:00:00 2001 From: Sveta Smirnova Date: Tue, 14 Nov 2023 16:55:56 +0300 Subject: [PATCH 1/2] PT-2281 - provide container name for copying files in the dump - Changed getIndividualFiles function and Dumper data structure, so we can specify container name for PXC and other operators which store logs in the separate container. --- .../pt-k8s-debug-collector/dumper/dumper.go | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/go/pt-k8s-debug-collector/dumper/dumper.go b/src/go/pt-k8s-debug-collector/dumper/dumper.go index b44555a1..bac07fa9 100644 --- a/src/go/pt-k8s-debug-collector/dumper/dumper.go +++ b/src/go/pt-k8s-debug-collector/dumper/dumper.go @@ -20,16 +20,17 @@ import ( // Dumper struct is for dumping cluster type Dumper struct { - cmd string - kubeconfig string - resources []string - filePaths []string - namespace string - location string - errors string - mode int64 - crType string - forwardport string + cmd string + kubeconfig string + resources []string + filePaths []string + fileContainer string + namespace string + location string + errors string + mode int64 + crType string + forwardport string } var resourcesRe = regexp.MustCompile(`(\w+)\.(\w+).percona\.com`) @@ -124,6 +125,7 @@ func New(location, namespace, resource string, kubeconfig string, forwardport st "var/lib/mysql/mysqld.post.processing.log", "var/lib/mysql/auto.cnf", ) + d.fileContainer = "logs" } d.resources = resources d.crType = resource @@ -372,7 +374,7 @@ type crSecrets struct { // TODO: check if resource parameter is really needed func (d *Dumper) getIndividualFiles(resource, namespace string, podName, path, location string, tw *tar.Writer) error { - args := []string{"-n", namespace, "cp", podName + ":" + path, "/dev/stdout"} + args := []string{"-n", namespace, "-c", d.fileContainer, "cp", podName + ":" + path, "/dev/stdout"} output, err := d.runCmd(args...) if err != nil { d.logError(err.Error(), args...) From 43ae487f40d5a4ae21ee490448aabb998e34de81 Mon Sep 17 00:00:00 2001 From: Sveta Smirnova Date: Wed, 29 Nov 2023 05:06:27 +0300 Subject: [PATCH 2/2] PT-2281 - provide container name for copying files in the dump - Implemented error handling when container name is not specified - Added test case for error handling --- .../pt-k8s-debug-collector/dumper/dumper.go | 10 ++++++---- .../dumper/dumper_test.go | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 src/go/pt-k8s-debug-collector/dumper/dumper_test.go diff --git a/src/go/pt-k8s-debug-collector/dumper/dumper.go b/src/go/pt-k8s-debug-collector/dumper/dumper.go index bac07fa9..ba1120e7 100644 --- a/src/go/pt-k8s-debug-collector/dumper/dumper.go +++ b/src/go/pt-k8s-debug-collector/dumper/dumper.go @@ -274,7 +274,7 @@ func (d *Dumper) DumpCluster() error { // get individual Logs location = filepath.Join(d.location, ns.Name, pod.Name) for _, path := range d.filePaths { - err = d.getIndividualFiles(resourceType(d.crType), ns.Name, pod.Name, path, location, tw) + err = d.getIndividualFiles(ns.Name, pod.Name, path, location, tw) if err != nil { d.logError(err.Error(), "get file "+path+" for pod "+pod.Name) log.Printf("Error: get %s file: %v", path, err) @@ -372,13 +372,15 @@ type crSecrets struct { } `json:"spec"` } -// TODO: check if resource parameter is really needed -func (d *Dumper) getIndividualFiles(resource, namespace string, podName, path, location string, tw *tar.Writer) error { +func (d *Dumper) getIndividualFiles(namespace string, podName, path, location string, tw *tar.Writer) error { + if len(d.fileContainer) == 0 { + return errors.Errorf("Logs container name is not specified for resource %s in namespace %s", resourceType(d.crType), d.namespace) + } args := []string{"-n", namespace, "-c", d.fileContainer, "cp", podName + ":" + path, "/dev/stdout"} output, err := d.runCmd(args...) if err != nil { d.logError(err.Error(), args...) - log.Printf("Error: get path %s for resource %s in namespace %s: %v", path, resource, d.namespace, err) + log.Printf("Error: get path %s for resource %s in namespace %s: %v", path, resourceType(d.crType), d.namespace, err) return addToArchive(location, d.mode, []byte(err.Error()), tw) } diff --git a/src/go/pt-k8s-debug-collector/dumper/dumper_test.go b/src/go/pt-k8s-debug-collector/dumper/dumper_test.go new file mode 100644 index 00000000..d540e1e7 --- /dev/null +++ b/src/go/pt-k8s-debug-collector/dumper/dumper_test.go @@ -0,0 +1,20 @@ +package dumper + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +/* +Unit test for non-existing logs container name error handling +*/ + +func TestGetIndividualFilesError(t *testing.T) { + d := New("", "", "psmdb", "", "") + + err := d.getIndividualFiles("", "", "", "", nil) + + assert.Error(t, err) + assert.ErrorContains(t, err, "Logs container name is not specified") +}