PT-2248 - pt-k8s-debug-collector does not run pg_gather with K8SPG 2 (#654)

* PT-2248 - pt-k8s-debug-collector does not run pg_gather with K8SPG 2

- Added check for K8SPG 2, so can run pg_gather for it
- Added new allowed value for option --resource:
-- pgv2 for K8SPG 2
-- auto to auto-detect custom resource
- Option --resource has now default value "auto"
- Updated documentation
- Added test cases for new options

* PT-2248 - pt-k8s-debug-collector does not run pg_gather with K8SPG 2

- Implemented custom user and secrets handling (in case when no default
  user exists).

docs/pt-k8s-debug-collector.rst

@@ -133,13 +133,17 @@ Targeted custom resource name. Supported values:
 
 * ``psmdb`` - MongoDB
 
-* ``pg`` - PostgreSQL
+* ``pg`` - PostgreSQL Operator v1 (deprecated)
+
+* ``pgv2`` - PostgreSQL Operator v2
 
 * ``ps`` - MySQL
 
 * ``none`` - Collect only general Kubernetes data, do not collect anything specific to the particular operator).
 
-Default: ``none``
+* ``auto`` - Auto-detect custom resource
+
+Default: ``auto``
 
 ``--namespace``
 

src/go/pt-k8s-debug-collector/README.rst

@@ -133,13 +133,17 @@ Targeted custom resource name. Supported values:
 
 * ``psmdb`` - MongoDB
 
-* ``pg`` - PostgreSQL
+* ``pg`` - PostgreSQL Operator v1 (deprecated)
+
+* ``pgv2`` - PostgreSQL Operator v2
 
 * ``ps`` - MySQL
 
 * ``none`` - Collect only general Kubernetes data, do not collect anything specific to the particular operator).
 
-Default: ``none``
+* ``auto`` - Auto-detect custom resource
+
+Default: ``auto``
 
 ``--namespace``
 

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

@@ -10,6 +10,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 
@@ -31,8 +32,18 @@ type Dumper struct {
 	forwardport string
 }
 
+var resourcesRe = regexp.MustCompile(`(\w+)\.(\w+).percona\.com`)
+
 // New return new Dumper object
 func New(location, namespace, resource string, kubeconfig string, forwardport string) Dumper {
+	d := Dumper{
+		cmd:         "kubectl",
+		kubeconfig:  kubeconfig,
+		location:    "cluster-dump",
+		mode:        int64(0o777),
+		namespace:   namespace,
+		forwardport: forwardport,
+	}
 	resources := []string{
 		"pods",
 		"replicasets",
@@ -43,7 +54,6 @@ func New(location, namespace, resource string, kubeconfig string, forwardport st
 		"configmaps",
 		"cronjobs",
 		"jobs",
-		"podsecuritypolicies",
 		"poddisruptionbudgets",
 		"clusterrolebindings",
 		"clusterroles",
@@ -53,56 +63,72 @@ func New(location, namespace, resource string, kubeconfig string, forwardport st
 		"persistentvolumeclaims",
 		"persistentvolumes",
 	}
-	filePaths := make([]string, 0)
+
-	if len(resource) > 0 {
+	switch resource {
-		if resourceType(resource) == "pxc" {
+	case "auto":
-			resources = append(resources,
+		result, err := d.runCmd("api-resources", "-o", "name")
-				"perconaxtradbclusterbackups",
+		if err != nil {
-				"perconaxtradbclusterrestores",
+			log.Panicf("Cannot get API resources and option --resource=auto specified:\n%s", err)
-				"perconaxtradbclusters")
-			filePaths = append(filePaths,
-				"var/lib/mysql/mysqld-error.log",
-				"var/lib/mysql/innobackup.backup.log",
-				"var/lib/mysql/innobackup.move.log",
-				"var/lib/mysql/innobackup.prepare.log",
-				"var/lib/mysql/grastate.dat",
-				"var/lib/mysql/gvwstate.dat",
-				"var/lib/mysql/mysqld.post.processing.log",
-				"var/lib/mysql/auto.cnf",
-			)
-		} else if resourceType(resource) == "psmdb" {
-			resources = append(resources,
-				"perconaservermongodbbackups",
-				"perconaservermongodbrestores",
-				"perconaservermongodbs",
-			)
-		} else if resourceType(resource) == "pg" {
-			resources = append(resources,
-				"perconapgclusters",
-				"pgclusters",
-				"pgpolicies",
-				"pgreplicas",
-				"pgtasks",
-			)
-		} else if resourceType(resource) == "ps" {
-			resources = append(resources,
-				"perconaservermysqlbackups",
-				"perconaservermysqlrestores",
-				"perconaservermysqls",
-			)
 		}
+		matches := resourcesRe.FindAllStringSubmatch(string(result), -1)
+		if len(matches) == 0 {
+			resource = "none"
+			break
+		}
+		for _, match := range matches {
+			resources = append(resources, match[1])
+			resource = match[2]
+		}
+	case "pg":
+		resources = append(resources,
+			"perconapgclusters",
+			"pgclusters",
+			"pgpolicies",
+			"pgreplicas",
+			"pgtasks",
+		)
+	case "pgv2":
+		resources = append(resources,
+			"perconapgbackups",
+			"perconapgclusters",
+			"perconapgrestores",
+		)
+	case "pxc":
+		resources = append(resources,
+			"perconaxtradbclusterbackups",
+			"perconaxtradbclusterrestores",
+			"perconaxtradbclusters",
+		)
+	case "ps":
+		resources = append(resources,
+			"perconaservermysqlbackups",
+			"perconaservermysqlrestores",
+			"perconaservermysqls",
+		)
+	case "psmdb":
+		resources = append(resources,
+			"perconaservermongodbbackups",
+			"perconaservermongodbrestores",
+			"perconaservermongodbs",
+		)
 	}
-	return Dumper{
+	filePaths := make([]string, 0)
-		cmd:         "kubectl",
+	if resourceType(resource) == "pxc" {
-		kubeconfig:  kubeconfig,
+		filePaths = append(filePaths,
-		resources:   resources,
+			"var/lib/mysql/mysqld-error.log",
-		filePaths:   filePaths,
+			"var/lib/mysql/innobackup.backup.log",
-		location:    "cluster-dump",
+			"var/lib/mysql/innobackup.move.log",
-		mode:        int64(0o777),
+			"var/lib/mysql/innobackup.prepare.log",
-		namespace:   namespace,
+			"var/lib/mysql/grastate.dat",
-		crType:      resource,
+			"var/lib/mysql/gvwstate.dat",
-		forwardport: forwardport,
+			"var/lib/mysql/mysqld.post.processing.log",
+			"var/lib/mysql/auto.cnf",
+		)
 	}
+	d.resources = resources
+	d.crType = resource
+	d.filePaths = filePaths
+	return d
 }
 
 type k8sPods struct {
@@ -217,10 +243,13 @@ func (d *Dumper) DumpCluster() error {
 				}
 			}
 			if pod.Labels["app.kubernetes.io/component"] == component ||
-				(component == "pg" && pod.Labels["pgo-pg-database"] == "true") {
+				(component == "pg" && pod.Labels["pgo-pg-database"] == "true") ||
+				(component == "pgv2" && pod.Labels["pgv2.percona.com/version"] != "" && pod.Labels["postgres-operator.crunchydata.com/instance"] != "") {
 				var crName string
 				if component == "pg" {
 					crName = pod.Labels["pg-cluster"]
+				} else if component == "pgv2" {
+					crName = pod.Labels["postgres-operator.crunchydata.com/cluster"]
 				} else {
 					crName = pod.Labels["app.kubernetes.io/instance"]
 				}
@@ -334,6 +363,10 @@ type crSecrets struct {
 		Secrets    struct {
 			Users string `json:"users,omitempty"`
 		} `json:"secrets,omitempty"`
+		Users []struct {
+			Name       string `json:"name,omitempty"`
+			SecretName string `json:"secretName,omitempty"`
+		} `json:"users,omitempty"`
 	} `json:"spec"`
 }
 
@@ -384,7 +417,7 @@ func (d *Dumper) getPodSummary(resource, podName, crName string, namespace strin
 		}
 		ports = port + ":3306"
 		summCmdName = "pt-mysql-summary"
-		summCmdArgs = []string{"--host=127.0.0.1", "--port=" + port, "--user=root", "--password=" + string(pass)}
+		summCmdArgs = []string{"--host=127.0.0.1", "--port=" + port, "--user=root", "--password='" + string(pass) + "'"}
 	case "pg":
 		var user, pass, port string
 		if d.forwardport != "" {
@@ -415,7 +448,46 @@ func (d *Dumper) getPodSummary(resource, podName, crName string, namespace strin
 		ports = port + ":5432"
 		summCmdName = "sh"
 		summCmdArgs = []string{"-c", "curl https://raw.githubusercontent.com/percona/support-snippets/master/postgresql/pg_gather/gather.sql" +
-			" 2>/dev/null | PGPASSWORD=" + string(pass) + " psql -X --host=127.0.0.1 --port=" + port + " --user=" + user}
+			" 2>/dev/null | PGPASSWORD='" + string(pass) + "' psql -X --host=127.0.0.1 --port=" + port + " --user='" + user + "'"}
+	case "pgv2":
+		var user, pass, port string
+		if d.forwardport != "" {
+			port = d.forwardport
+		} else {
+			port = "5432"
+		}
+		cr, err := d.getCR("perconapgclusters/"+crName, namespace)
+		if err != nil {
+			return nil, errors.Wrap(err, "get cr")
+		}
+		if cr.Spec.SecretName != "" {
+			user, err = d.getDataFromSecret(cr.Spec.SecretName, "user", namespace)
+		} else if len(cr.Spec.Users) > 0 && cr.Spec.Users[0].Name != "" {
+			user = cr.Spec.Users[0].Name
+		} else {
+			user, err = d.getDataFromSecret(crName+"-pguser-"+crName, "user", namespace)
+		}
+		if err != nil {
+			return nil, errors.Wrap(err, "get user from PostgreSQL users secret")
+		}
+		if cr.Spec.SecretName != "" {
+			pass, err = d.getDataFromSecret(cr.Spec.SecretName, "password", namespace)
+		} else if len(cr.Spec.Users) > 0 {
+			if cr.Spec.Users[0].SecretName != "" {
+				pass, err = d.getDataFromSecret(cr.Spec.Users[0].SecretName, "password", namespace)
+			} else {
+				pass, err = d.getDataFromSecret(crName+"-pguser-"+user, "password", namespace)
+			}
+		} else {
+			pass, err = d.getDataFromSecret(crName+"-pguser-"+crName, "password", namespace)
+		}
+		if err != nil {
+			return nil, errors.Wrap(err, "get password from PostgreSQL users secret")
+		}
+		ports = port + ":5432"
+		summCmdName = "sh"
+		summCmdArgs = []string{"-c", "curl https://raw.githubusercontent.com/percona/support-snippets/master/postgresql/pg_gather/gather.sql" +
+			" 2>/dev/null | PGPASSWORD='" + string(pass) + "' psql -X --host=127.0.0.1 --port=" + port + " --user='" + user + "'"}
 	case "psmdb":
 		var port string
 		if d.forwardport != "" {
@@ -437,7 +509,7 @@ func (d *Dumper) getPodSummary(resource, podName, crName string, namespace strin
 		}
 		ports = port + ":27017"
 		summCmdName = "pt-mongodb-summary"
-		summCmdArgs = []string{"--username=" + user, "--password=" + pass, "--authenticationDatabase=admin", "127.0.0.1:" + port}
+		summCmdArgs = []string{"--username='" + user + "'", "--password='" + pass + "'", "--authenticationDatabase=admin", "127.0.0.1:" + port}
 	}
 
 	cmdPortFwd := exec.Command(d.cmd, "port-forward", "pod/"+podName, ports, "-n", namespace, "--kubeconfig", d.kubeconfig)
@@ -501,6 +573,8 @@ func resourceType(s string) string {
 		return "psmdb"
 	} else if s == "pg" || strings.HasPrefix(s, "pg/") {
 		return "pg"
+	} else if s == "pgv2" || strings.HasPrefix(s, "pgv2/") {
+		return "pgv2"
 	} else if s == "ps" || strings.HasPrefix(s, "ps/") {
 		return "ps"
 	}

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

@@ -30,7 +30,7 @@ func main() {
 	version := false
 
 	flag.StringVar(&namespace, "namespace", "", "Namespace for collecting data. If empty data will be collected from all namespaces")
-	flag.StringVar(&resource, "resource", "none", "Collect data, specific to the resource. Supported values: pxc, psmdb, pg, ps, none")
+	flag.StringVar(&resource, "resource", "auto", "Collect data, specific to the resource. Supported values: pxc, psmdb, pg, pgv2, ps, none, auto")
 	flag.StringVar(&clusterName, "cluster", "", "Cluster name")
 	flag.StringVar(&kubeconfig, "kubeconfig", "", "Path to kubeconfig")
 	flag.StringVar(&forwardport, "forwardport", "", "Port to use for  port forwarding")

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

@@ -20,6 +20,7 @@ This test requires:
 -- KUBECONFIG_PS for K8SPS
 -- KUBECONFIG_PSMDB for K8SPSMDB
 -- KUBECONFIG_PG for K8SPG
+-- KUBECONFIG_PG2 for K8SPG version 2
 
 You can additionally set option FORWARDPORT if you want to use custom port when testing summaries.
 
@@ -31,6 +32,15 @@ go test -timeout 6000s
 We do not explicitly test --kubeconfig and --forwardport options, because they are used in other tests.
 */
 
+/*
+Tests TODO:
+
+- Test clusters with custom user and secrets. With the way we currently test,
+  we just need to create a cluster with particular options. But it is already
+  time and resource consuming operation. So we need to either test only getCR
+  function or create a mock cluster, or find a better way to deploy test clusters.
+*/
+
 /*
 Tests collection of the individual files by pt-k8s-debug-collector.
 Requires running K8SPXC instance and kubectl, configured to access that instance by default.
@@ -40,9 +50,9 @@ func TestIndividualFiles(t *testing.T) {
 		t.Skip("TestIndividualFiles requires K8SPXC")
 	}
 	tests := []struct {
-		name        string
+		name         string
-		cmd         []string
+		cmd          []string
-		want        []string
+		want         []string
 		preprocessor func(string) string
 	}{
 		{
@@ -80,24 +90,26 @@ func TestIndividualFiles(t *testing.T) {
 		},
 	}
 
-	cmd := exec.Command("../../../bin/pt-k8s-debug-collector", "--kubeconfig", os.Getenv("KUBECONFIG_PXC"), "--forwardport", os.Getenv("FORWARDPORT"), "--resource", "pxc")
+	for _, resource := range []string{"pxc", "auto"} {
-	if err := cmd.Run(); err != nil {
+		cmd := exec.Command("../../../bin/pt-k8s-debug-collector", "--kubeconfig", os.Getenv("KUBECONFIG_PXC"), "--forwardport", os.Getenv("FORWARDPORT"), "--resource", resource)
-		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())
+			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())
+			}
+		}()
 
-	for _, test := range tests {
+		for _, test := range tests {
-		out, err := exec.Command(test.cmd[0], test.cmd[1:]...).CombinedOutput()
+			out, err := exec.Command(test.cmd[0], test.cmd[1:]...).CombinedOutput()
-		if err != nil {
+			if err != nil {
-			t.Errorf("test %s, error running command %s:\n%s\n\nCommand output:\n%s", test.name, test.cmd[0], err.Error(), out)
+				t.Errorf("test %s, error running command %s:\n%s\n\nCommand output:\n%s", test.name, test.cmd[0], err.Error(), out)
-		}
+			}
-		if test.preprocessor(bytes.NewBuffer(out).String()) != strings.Join(test.want, "\n") {
+			if test.preprocesor(bytes.NewBuffer(out).String()) != strings.Join(test.want, "\n") {
-			t.Errorf("test %s, output is not as expected\nOutput: %s\nWanted: %s", test.name, test.preprocessor(bytes.NewBuffer(out).String()), test.want)
+				t.Errorf("test %s, output is not as expected\nOutput: %s\nWanted: %s", test.name, test.preprocesor(bytes.NewBuffer(out).String()), test.want)
+			}
 		}
 	}
 }
@@ -109,38 +121,80 @@ func TestResourceOption(t *testing.T) {
 	testcmd := []string{"sh", "-c", "tar -tf cluster-dump.tar.gz --wildcards '*/summary.txt' 2>/dev/null | wc -l"}
 	tests := []struct {
 		name       string
+		resource   string
 		want       string
 		kubeconfig string
 	}{
 		{
 			name:       "none",
+			resource:   "none",
 			want:       "0",
 			kubeconfig: "",
 		},
 		{
 			name:       "pxc",
+			resource:   "pxc",
 			want:       "3",
 			kubeconfig: os.Getenv("KUBECONFIG_PXC"),
 		},
 		{
 			name:       "ps",
+			resource:   "ps",
 			want:       "3",
 			kubeconfig: os.Getenv("KUBECONFIG_PS"),
 		},
 		{
 			name:       "psmdb",
+			resource:   "psmdb",
 			want:       "3",
 			kubeconfig: os.Getenv("KUBECONFIG_PSMDB"),
 		},
 		{
 			name:       "pg",
+			resource:   "pg",
 			want:       "3",
 			kubeconfig: os.Getenv("KUBECONFIG_PG"),
 		},
+		{
+			name:       "pgv2",
+			resource:   "pgv2",
+			want:       "3",
+			kubeconfig: os.Getenv("KUBECONFIG_PG2"),
+		},
+		{
+			name:       "auto pxc",
+			resource:   "auto",
+			want:       "3",
+			kubeconfig: os.Getenv("KUBECONFIG_PXC"),
+		},
+		{
+			name:       "auto ps",
+			resource:   "auto",
+			want:       "3",
+			kubeconfig: os.Getenv("KUBECONFIG_PS"),
+		},
+		{
+			name:       "auto psmdb",
+			resource:   "auto",
+			want:       "3",
+			kubeconfig: os.Getenv("KUBECONFIG_PSMDB"),
+		},
+		{
+			name:       "auto pg",
+			resource:   "auto",
+			want:       "3",
+			kubeconfig: os.Getenv("KUBECONFIG_PG"),
+		},
+		{
+			name:       "auto pgv2",
+			resource:   "auto",
+			want:       "3",
+			kubeconfig: os.Getenv("KUBECONFIG_PG2"),
+		},
 	}
 
 	for _, test := range tests {
-		cmd := exec.Command("../../../bin/pt-k8s-debug-collector", "--kubeconfig", test.kubeconfig, "--forwardport", os.Getenv("FORWARDPORT"), "--resource", test.name)
+		cmd := exec.Command("../../../bin/pt-k8s-debug-collector", "--kubeconfig", test.kubeconfig, "--forwardport", os.Getenv("FORWARDPORT"), "--resource", test.resource)
 		if err := cmd.Run(); err != nil {
 			t.Errorf("error executing pt-k8s-debug-collector: %s", err.Error())
 		}
@@ -184,18 +238,21 @@ func TestPT_2169(t *testing.T) {
 	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
+		resource   string
 		want       string
 		port       string
 		kubeconfig string
 	}{
 		{
-			name:       "pg",
+			name:       "pg with busy port",
+			resource:   "pg",
 			want:       "3",
 			port:       busyport,
 			kubeconfig: os.Getenv("KUBECONFIG_PG"),
 		},
 		{
-			name:       "pg",
+			name:       "pg no error",
+			resource:   "pg",
 			want:       "0",
 			port:       os.Getenv("FORWARDPORT"),
 			kubeconfig: os.Getenv("KUBECONFIG_PG"),
@@ -203,7 +260,7 @@ func TestPT_2169(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		cmd := exec.Command("../../../bin/pt-k8s-debug-collector", "--kubeconfig", test.kubeconfig, "--forwardport", test.port, "--resource", test.name)
+		cmd := exec.Command("../../../bin/pt-k8s-debug-collector", "--kubeconfig", test.kubeconfig, "--forwardport", test.port, "--resource", test.resource)
 		if err := cmd.Run(); err != nil {
 			t.Errorf("error executing pt-k8s-debug-collector: %s", err.Error())
 		}