Another small refactor for sending pgbackrest logs in Repo Host to additional volume.

Author: dsessler7

internal/pgbackrest/config.go

@@ -134,21 +134,7 @@ func CreatePGBackRestConfigMapIntent(ctx context.Context, postgresCluster *v1bet
 
 		if collector.OpenTelemetryLogsOrMetricsEnabled(ctx, postgresCluster) {
 			// Get pgbackrest log path for repo host pod
-			var pgBackRestLogPath string
-			for _, repo := range postgresCluster.Spec.Backups.PGBackRest.Repos {
-				if repo.Volume != nil {
-					// If the user has set a log path in the spec, use it.
-					// Otherwise, default to /pgbackrest/repo#/log
-					if postgresCluster.Spec.Backups.PGBackRest.RepoHost != nil &&
-						postgresCluster.Spec.Backups.PGBackRest.RepoHost.Log != nil &&
-						postgresCluster.Spec.Backups.PGBackRest.RepoHost.Log.Path != "" {
-						pgBackRestLogPath = postgresCluster.Spec.Backups.PGBackRest.RepoHost.Log.Path
-					} else {
-						pgBackRestLogPath = fmt.Sprintf(naming.PGBackRestRepoLogPath, repo.Name)
-					}
-					break
-				}
-			}
+			pgBackRestLogPath := generateRepoHostLogPath(postgresCluster)
 
 			err = collector.AddToConfigMap(ctx, collector.NewConfigForPgBackrestRepoHostPod(
 				ctx,
@@ -190,21 +176,7 @@ func CreatePGBackRestConfigMapIntent(ctx context.Context, postgresCluster *v1bet
 func MakePGBackrestLogDir(template *corev1.PodTemplateSpec,
 	cluster *v1beta1.PostgresCluster) string {
 
-	var pgBackRestLogPath string
-	for _, repo := range cluster.Spec.Backups.PGBackRest.Repos {
-		if repo.Volume != nil {
-			// If the user has set a log path in the spec, use it.
-			// Otherwise, default to /pgbackrest/repo#/log
-			if cluster.Spec.Backups.PGBackRest.RepoHost != nil &&
-				cluster.Spec.Backups.PGBackRest.RepoHost.Log != nil &&
-				cluster.Spec.Backups.PGBackRest.RepoHost.Log.Path != "" {
-				pgBackRestLogPath = cluster.Spec.Backups.PGBackRest.RepoHost.Log.Path
-			} else {
-				pgBackRestLogPath = fmt.Sprintf(naming.PGBackRestRepoLogPath, repo.Name)
-			}
-			break
-		}
-	}
+	pgBackRestLogPath := generateRepoHostLogPath(cluster)
 
 	container := corev1.Container{
 		// TODO(log-rotation): The second argument here should be the path
@@ -795,3 +767,27 @@ func serverConfig(cluster *v1beta1.PostgresCluster) iniSectionSet {
 		"global:server": server,
 	}
 }
+
+// generateRepoHostLogPath takes a postgrescluster and returns the log path that
+// should be used by pgbackrest in the Repo Host Pod based on the repos specified
+// and whether the user has specified a log path.
+//
+// This function assumes that the backups/pgbackrest spec is present in cluster.
+func generateRepoHostLogPath(cluster *v1beta1.PostgresCluster) string {
+	var pgBackRestLogPath string
+	for _, repo := range cluster.Spec.Backups.PGBackRest.Repos {
+		if repo.Volume != nil {
+			// If the user has set a log path in the spec, use it.
+			// Otherwise, default to /pgbackrest/repo#/log
+			if cluster.Spec.Backups.PGBackRest.RepoHost != nil &&
+				cluster.Spec.Backups.PGBackRest.RepoHost.Log != nil &&
+				cluster.Spec.Backups.PGBackRest.RepoHost.Log.Path != "" {
+				pgBackRestLogPath = cluster.Spec.Backups.PGBackRest.RepoHost.Log.Path
+			} else {
+				pgBackRestLogPath = fmt.Sprintf(naming.PGBackRestRepoLogPath, repo.Name)
+			}
+			break
+		}
+	}
+	return pgBackRestLogPath
+}

internal/pgbackrest/config_test.go

@@ -840,3 +840,89 @@ log-level-stderr = error
 log-timestamp = n
 `)
 }
+
+func TestGenerateRepoHostLogPath(t *testing.T) {
+	cluster := v1beta1.PostgresCluster{}
+	cluster.Namespace = "ns1"
+	cluster.Name = "hippo-dance"
+
+	cluster.Spec.Port = initialize.Int32(2345)
+	cluster.Spec.PostgresVersion = 12
+
+	cluster.Spec.Backups = v1beta1.Backups{
+		PGBackRest: v1beta1.PGBackRestArchive{},
+	}
+
+	t.Run("NoReposNoRepoHost", func(t *testing.T) {
+		cluster := cluster.DeepCopy()
+		assert.Equal(t, generateRepoHostLogPath(cluster), "")
+	})
+
+	t.Run("NoVolumeRepo", func(t *testing.T) {
+		cluster := cluster.DeepCopy()
+		cluster.Spec.Backups.PGBackRest.Repos = []v1beta1.PGBackRestRepo{
+			{
+				Name: "repo1",
+				GCS:  &v1beta1.RepoGCS{},
+			},
+		}
+		assert.Equal(t, generateRepoHostLogPath(cluster), "")
+	})
+
+	t.Run("OneVolumeRepo", func(t *testing.T) {
+		cluster := cluster.DeepCopy()
+		cluster.Spec.Backups.PGBackRest.Repos = []v1beta1.PGBackRestRepo{
+			{
+				Name:   "repo1",
+				Volume: &v1beta1.RepoPVC{},
+			},
+		}
+		assert.Equal(t, generateRepoHostLogPath(cluster), "/pgbackrest/repo1/log")
+	})
+
+	t.Run("TwoVolumeRepos", func(t *testing.T) {
+		cluster := cluster.DeepCopy()
+		cluster.Spec.Backups.PGBackRest.Repos = []v1beta1.PGBackRestRepo{
+			{
+				Name:   "repo1",
+				Volume: &v1beta1.RepoPVC{},
+			},
+			{
+				Name:   "repo2",
+				Volume: &v1beta1.RepoPVC{},
+			},
+		}
+		assert.Equal(t, generateRepoHostLogPath(cluster), "/pgbackrest/repo1/log")
+	})
+
+	t.Run("VolumeRepoNotFirst", func(t *testing.T) {
+		cluster := cluster.DeepCopy()
+		cluster.Spec.Backups.PGBackRest.Repos = []v1beta1.PGBackRestRepo{
+			{
+				Name: "repo1",
+				GCS:  &v1beta1.RepoGCS{},
+			},
+			{
+				Name:   "repo2",
+				Volume: &v1beta1.RepoPVC{},
+			},
+		}
+		assert.Equal(t, generateRepoHostLogPath(cluster), "/pgbackrest/repo2/log")
+	})
+
+	t.Run("LogPathSpecified", func(t *testing.T) {
+		cluster := cluster.DeepCopy()
+		cluster.Spec.Backups.PGBackRest.Repos = []v1beta1.PGBackRestRepo{
+			{
+				Name:   "repo1",
+				Volume: &v1beta1.RepoPVC{},
+			},
+		}
+		cluster.Spec.Backups.PGBackRest.RepoHost = &v1beta1.PGBackRestRepoHost{
+			Log: &v1beta1.LoggingConfiguration{
+				Path: "/some/directory",
+			},
+		}
+		assert.Equal(t, generateRepoHostLogPath(cluster), "/some/directory")
+	})
+}