Adjust support for OTel and non-OTel

With OTel, we've introduced some changes, most notably
to the location of the Postgres logs; but also with the
introduction of a `receiver` dir in several locations
which only the collector container has access to.

This PR addresses these issues by using the cluster
to determine the location of the logs, and by excluding
checking the `receiver` dir in several execs through the use
of a '*.*' wildcard.

Issues: [PGO-2358]

Author: benjaminjb

internal/cmd/exec.go

@@ -48,10 +48,22 @@ func (exec Executor) pgBackRestCheck() (string, string, error) {
 }
 
 // postgresqlListLogFiles returns the full path of numLogs log files.
-func (exec Executor) listPGLogFiles(numLogs int) (string, string, error) {
+func (exec Executor) listPGLogFiles(numLogs int, hasInstrumentation bool) (string, string, error) {
 	var stdout, stderr bytes.Buffer
 
-	command := fmt.Sprintf("ls -1dt pgdata/pg[0-9][0-9]/log/* | head -%d", numLogs)
+	location := "pgdata/pg[0-9][0-9]/log/*"
+	if hasInstrumentation {
+		location = "pgdata/logs/postgres/*.*"
+	}
+	// Check both the older and the newer log locations.
+	// If a cluster has used both locations, this will return logs from both.
+	// If a cluster does not have one location or the other, continue without error.
+	// This ensures we get as many logs as possible for whatever combination of
+	// log files exists on this cluster.
+	// Note the "*.*" pattern to exclude the `receiver` directory,
+	// which only the collector container has permission to access.
+	command := fmt.Sprintf("ls -1dt %s | head -%d",
+		location, numLogs)
 	err := exec(nil, &stdout, &stderr, "bash", "-ceu", "--", command)
 
 	return stdout.String(), stderr.String(), err
@@ -73,7 +85,9 @@ func (exec Executor) listPGConfFiles() (string, string, error) {
 func (exec Executor) listBackrestLogFiles() (string, string, error) {
 	var stdout, stderr bytes.Buffer
 
-	command := "ls -1dt pgdata/pgbackrest/log/*"
+	// Note the "*.*" pattern to exclude the `receiver` directory,
+	// which only the collector container has permission to access.
+	command := "ls -1dt pgdata/pgbackrest/log/*.*"
 	err := exec(nil, &stdout, &stderr, "bash", "-ceu", "--", command)
 
 	return stdout.String(), stderr.String(), err
@@ -84,7 +98,9 @@ func (exec Executor) listBackrestLogFiles() (string, string, error) {
 func (exec Executor) listPatroniLogFiles() (string, string, error) {
 	var stdout, stderr bytes.Buffer
 
-	command := "ls -1dt pgdata/patroni/log/*"
+	// Note the "*.*" pattern to exclude the `receiver` directory,
+	// which only the collector container has permission to access.
+	command := "ls -1dt pgdata/patroni/log/*.*"
 	err := exec(nil, &stdout, &stderr, "bash", "-ceu", "--", command)
 
 	return stdout.String(), stderr.String(), err
@@ -95,7 +111,9 @@ func (exec Executor) listPatroniLogFiles() (string, string, error) {
 func (exec Executor) listBackrestRepoHostLogFiles() (string, string, error) {
 	var stdout, stderr bytes.Buffer
 
-	command := "ls -1dt pgbackrest/*/log/*"
+	// Note the "*.*" pattern to exclude the `receiver` directory,
+	// which only the collector container has permission to access.
+	command := "ls -1dt pgbackrest/*/log/*.*"
 	err := exec(nil, &stdout, &stderr, "bash", "-ceu", "--", command)
 
 	return stdout.String(), stderr.String(), err

internal/cmd/exec_test.go

@@ -72,7 +72,22 @@ func TestListPGLogFiles(t *testing.T) {
 			assert.Assert(t, stderr != nil, "should capture stderr")
 			return expected
 		}
-		_, _, err := Executor(exec).listPGLogFiles(1)
+		_, _, err := Executor(exec).listPGLogFiles(1, false)
+		assert.ErrorContains(t, err, "pass-through")
+
+	})
+
+	t.Run("has instrumentation", func(t *testing.T) {
+		expected := errors.New("pass-through")
+		exec := func(
+			stdin io.Reader, stdout, stderr io.Writer, command ...string,
+		) error {
+			assert.DeepEqual(t, command, []string{"bash", "-ceu", "--", "ls -1dt pgdata/logs/postgres/*.* | head -1"})
+			assert.Assert(t, stdout != nil, "should capture stdout")
+			assert.Assert(t, stderr != nil, "should capture stderr")
+			return expected
+		}
+		_, _, err := Executor(exec).listPGLogFiles(1, true)
 		assert.ErrorContains(t, err, "pass-through")
 
 	})
@@ -86,7 +101,7 @@ func TestListPatroniLogFiles(t *testing.T) {
 		exec := func(
 			stdin io.Reader, stdout, stderr io.Writer, command ...string,
 		) error {
-			assert.DeepEqual(t, command, []string{"bash", "-ceu", "--", "ls -1dt pgdata/patroni/log/*"})
+			assert.DeepEqual(t, command, []string{"bash", "-ceu", "--", "ls -1dt pgdata/patroni/log/*.*"})
 			assert.Assert(t, stdout != nil, "should capture stdout")
 			assert.Assert(t, stderr != nil, "should capture stderr")
 			return expected

internal/cmd/export.go

@@ -462,7 +462,7 @@ Collecting PGO CLI logs...
 		// All Postgres Logs on the Postgres Instances (primary and replicas)
 		if numLogs > 0 {
 			err = gatherPostgresLogsAndConfigs(ctx, clientset, restConfig,
-				namespace, clusterName, outputDir, outputFile, numLogs, tw, cmd)
+				namespace, clusterName, outputDir, outputFile, numLogs, tw, cmd, get)
 			if err != nil {
 				writeInfo(cmd, fmt.Sprintf("Error gathering Postgres Logs and Config: %s", err))
 			}
@@ -992,6 +992,7 @@ func gatherPostgresLogsAndConfigs(ctx context.Context,
 	numLogs int,
 	tw *tar.Writer,
 	cmd *cobra.Command,
+	cluster *unstructured.Unstructured,
 ) error {
 	writeInfo(cmd, "Collecting Postgres logs...")
 
@@ -1029,7 +1030,11 @@ func gatherPostgresLogsAndConfigs(ctx context.Context,
 		}
 
 		// Get Postgres Log Files
-		stdout, stderr, err := Executor(exec).listPGLogFiles(numLogs)
+		// Pass a boolean based on whether the cluster has instrumentation
+		// since that will determine where the log files are stored
+		_, hasInstrumentation, _ := unstructured.NestedMap(cluster.Object,
+			"spec", "instrumentation")
+		stdout, stderr, err := Executor(exec).listPGLogFiles(numLogs, hasInstrumentation)
 
 		// Depending upon the list* function above:
 		// An error may happen when err is non-nil or stderr is non-empty.