Move permanent flags into ExporterStartCommand

Issue: PGO-635

Author: cbandy

internal/controller/postgrescluster/pgmonitor.go

@@ -269,9 +269,7 @@ func addPGMonitorExporterToInstancePodSpec(
 Image: config.PGExporterContainerImage(cluster),
 ImagePullPolicy: cluster.Spec.ImagePullPolicy,
 Resources: cluster.Spec.Monitoring.PGMonitor.Exporter.Resources,
-Command: pgmonitor.ExporterStartCommand([]string{
-pgmonitor.ExporterExtendQueryPathFlag, pgmonitor.ExporterWebListenAddressFlag,
-}),
+Command: pgmonitor.ExporterStartCommand(),
 Env: []corev1.EnvVar{
 {Name: "DATA_SOURCE_URI", Value: fmt.Sprintf("%s:%d/%s", pgmonitor.ExporterHost, *cluster.Spec.Port, pgmonitor.ExporterDB)},
 {Name: "DATA_SOURCE_USER", Value: pgmonitor.MonitoringUser},
@@ -400,9 +398,7 @@ func configureExporterTLS(cluster *v1beta1.PostgresCluster, template *corev1.Pod
 }}
 
 exporterContainer.VolumeMounts = append(exporterContainer.VolumeMounts, mounts...)
-exporterContainer.Command = pgmonitor.ExporterStartCommand([]string{
-pgmonitor.ExporterExtendQueryPathFlag, pgmonitor.ExporterWebListenAddressFlag, pgmonitor.ExporterWebConfigFileFlag,
-})
+exporterContainer.Command = pgmonitor.ExporterStartCommand(pgmonitor.ExporterWebConfigFileFlag)
 }
 }
 

internal/pgmonitor/exporter.go

@@ -43,9 +43,7 @@ const (
 
 // postgres_exporter command flags
 var (
-ExporterExtendQueryPathFlag = "--extend.query-path=/tmp/queries.yml"
-ExporterWebListenAddressFlag = fmt.Sprintf("--web.listen-address=:%d", ExporterPort)
-ExporterWebConfigFileFlag = "--web.config.file=/web-config/web-config.yml"
+ExporterWebConfigFileFlag = "--web.config.file=/web-config/web-config.yml"
 )
 
 // Defaults for certain values used in queries.yml
@@ -120,26 +118,31 @@ func GenerateDefaultExporterQueries(ctx context.Context, cluster *v1beta1.Postgr
 // ExporterStartCommand generates an entrypoint that will create a master queries file and
 // start the postgres_exporter. It will repeat those steps if it notices a change in
 // the source queries files.
-func ExporterStartCommand(commandFlags []string) []string {
+func ExporterStartCommand(commandFlags ...string) []string {
 script := strings.Join([]string{
 // Older images do not have the command on the PATH.
 `PATH="$PATH:$(echo /opt/cpm/bin/postgres_exporter-*)"`,
 
 // Set up temporary file to hold postgres_exporter process id
 `POSTGRES_EXPORTER_PIDFILE=/tmp/postgres_exporter.pid`,
 
+`postgres_exporter_flags=(`,
+`'--extend.query-path=/tmp/queries.yml'`,
+fmt.Sprintf(`'--web.listen-address=:%d'`, ExporterPort),
+`"$@")`,
+
 // declare function that will combine custom queries file and default
 // queries and start the postgres_exporter
 `start_postgres_exporter() {`,
-`cat /conf/* > /tmp/queries.yml`,
-`echo "Starting postgres_exporter with the following flags..."`,
-`echo "$@"`,
-`postgres_exporter "$@" &`,
-`echo $! > $POSTGRES_EXPORTER_PIDFILE`,
+` cat /conf/* > /tmp/queries.yml`,
+` echo "Starting postgres_exporter with the following flags..."`,
+` echo "${postgres_exporter_flags[@]}"`,
+` postgres_exporter "${postgres_exporter_flags[@]}" &`,
+` echo $! > $POSTGRES_EXPORTER_PIDFILE`,
 `}`,
 
 // run function to combine queries files and start postgres_exporter
-`start_postgres_exporter "$@"`,
+`start_postgres_exporter`,
 
 // Create a file descriptor with a no-op process that will not get
 // cleaned up
@@ -153,7 +156,7 @@ func ExporterStartCommand(commandFlags []string) []string {
 // something must have changed, so kill the postgres_exporter and rerun
 // the function to combine queries files and start postgres_exporter
 ` if ([ "/conf" -nt "/proc/self/fd/${fd}" ] || [ "/opt/crunchy/password" -nt "/proc/self/fd/${fd}" ]) \`,
-` && kill $(head -1 ${POSTGRES_EXPORTER_PIDFILE?}) && start_postgres_exporter "$@";`,
+` && kill $(head -1 ${POSTGRES_EXPORTER_PIDFILE?}) && start_postgres_exporter;`,
 ` then`,
 
 // When something changes we want to get rid of the old file descriptor, get a fresh one

internal/pgmonitor/exporter_test.go

@@ -24,7 +24,9 @@ import (
 "testing"
 
 "gotest.tools/v3/assert"
+"sigs.k8s.io/yaml"
 
+"github.com/crunchydata/postgres-operator/internal/testing/cmp"
 "github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
 )
 
@@ -48,14 +50,25 @@ func TestGenerateDefaultExporterQueries(t *testing.T) {
 }
 
 func TestExporterStartCommand(t *testing.T) {
-t.Run("OneFlag", func(t *testing.T) {
-commandSlice := ExporterStartCommand([]string{"--testFlag"})
-assert.DeepEqual(t, commandSlice[:3], []string{"bash", "-ceu", "--"})
-assert.DeepEqual(t, commandSlice[4:], []string{"postgres_exporter_watcher", "--testFlag"})
+t.Run("NoInput", func(t *testing.T) {
+command := ExporterStartCommand()
+assert.DeepEqual(t, command[:3], []string{"bash", "-ceu", "--"})
+assert.Assert(t, len(command) > 3)
+script := command[3]
+
+assert.Assert(t, cmp.Contains(script, "'--extend.query-path=/tmp/queries.yml'"))
+assert.Assert(t, cmp.Contains(script, "'--web.listen-address=:9187'"))
+
+t.Run("PrettyYAML", func(t *testing.T) {
+b, err := yaml.Marshal(script)
+assert.NilError(t, err)
+assert.Assert(t, strings.HasPrefix(string(b), `|`),
+"expected literal block scalar, got:\n%s", b)
+})
 })
 
 t.Run("MultipleFlags", func(t *testing.T) {
-commandSlice := ExporterStartCommand([]string{"--firstTestFlag", "--secondTestFlag"})
+commandSlice := ExporterStartCommand("--firstTestFlag", "--secondTestFlag")
 assert.DeepEqual(t, commandSlice[:3], []string{"bash", "-ceu", "--"})
 assert.DeepEqual(t, commandSlice[4:], []string{"postgres_exporter_watcher", "--firstTestFlag", "--secondTestFlag"})
 })