Skip to content

Commit

Permalink
PMM-12422 use public IP for exporters in pull mode (#2642)
Browse files Browse the repository at this point in the history
* use pmm agent for node IP lookup

* re-order switch cases

* make address selection more explicit

* always expose in pull mode

* update tests
  • Loading branch information
idoqo authored Nov 27, 2023
1 parent 66f6ecb commit 01d08b4
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 39 deletions.
6 changes: 3 additions & 3 deletions managed/services/agents/agents.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,11 @@ func ensureAuthParams(exporter *models.Agent, params *agentpb.SetStateRequest_Ag
// getExporterListenAddress returns the appropriate listen address to use for a given exporter.
func getExporterListenAddress(node *models.Node, exporter *models.Agent) string {

Check failure on line 146 in managed/services/agents/agents.go

View workflow job for this annotation

GitHub Actions / Checks

`getExporterListenAddress` - `node` is unused (unparam)
switch {
case !exporter.PushMetrics && node != nil:
return node.Address
case exporter.ExposeExporter:
return "0.0.0.0"
default:
case exporter.PushMetrics:
return "127.0.0.1"
default:
return "0.0.0.0"
}
}
26 changes: 18 additions & 8 deletions managed/services/agents/agents_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,33 +51,43 @@ func TestPathsBaseForDifferentVersions(t *testing.T) {
}

func TestGetExporterListenAddress(t *testing.T) {
t.Run("uses node address in pull mode", func(t *testing.T) {
t.Run("uses 127.0.0.1 in push mode", func(t *testing.T) {
node := &models.Node{
Address: "1.2.3.4",
}
exporter := &models.Agent{}
exporter := &models.Agent{
PushMetrics: true,
}

assert.Equal(t, "1.2.3.4", getExporterListenAddress(node, exporter))
assert.Equal(t, "127.0.0.1", getExporterListenAddress(node, exporter))
})
t.Run("uses 127.0.0.1 in push mode", func(t *testing.T) {
t.Run("exposes exporter address when enabled in push mode", func(t *testing.T) {
node := &models.Node{
Address: "1.2.3.4",
}
exporter := &models.Agent{
PushMetrics: true,
PushMetrics: true,
ExposeExporter: true,
}

assert.Equal(t, "127.0.0.1", getExporterListenAddress(node, exporter))
assert.Equal(t, "0.0.0.0", getExporterListenAddress(node, exporter))
})
t.Run("exposes exporter address when enabled", func(t *testing.T) {
t.Run("exposes exporter address when enabled in pull mode", func(t *testing.T) {
node := &models.Node{
Address: "1.2.3.4",
}
exporter := &models.Agent{
PushMetrics: true,
PushMetrics: false,
ExposeExporter: true,
}

assert.Equal(t, "0.0.0.0", getExporterListenAddress(node, exporter))
})
t.Run("exposes exporter address if node IP is unavailable in pull mode", func(t *testing.T) {
exporter := &models.Agent{
PushMetrics: false,
}

assert.Equal(t, "0.0.0.0", getExporterListenAddress(nil, exporter))
})
}
22 changes: 11 additions & 11 deletions managed/services/agents/mongodb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestMongodbExporterConfig225(t *testing.T) {
"--compatible-mode",
"--discovering-mode",
"--mongodb.global-conn-pool",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"MONGODB_URI=mongodb://username:s3cur3%20p%[email protected]:27017/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000",
Expand All @@ -78,7 +78,7 @@ func TestMongodbExporterConfig225(t *testing.T) {
"--discovering-mode",
"--mongodb.collstats-colls=col1,col2,col3",
"--mongodb.global-conn-pool",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
}
actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion)
require.NoError(t, err)
Expand Down Expand Up @@ -113,7 +113,7 @@ func TestMongodbExporterConfig226(t *testing.T) {
"--compatible-mode",
"--discovering-mode",
"--mongodb.global-conn-pool",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"MONGODB_URI=mongodb://username:s3cur3%20p%[email protected]:27017/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000",
Expand Down Expand Up @@ -141,7 +141,7 @@ func TestMongodbExporterConfig226(t *testing.T) {
"--mongodb.collstats-colls=col1,col2,col3",
"--mongodb.global-conn-pool",
"--mongodb.indexstats-colls=col1,col2,col3",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
}
actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion)
require.NoError(t, err)
Expand All @@ -168,7 +168,7 @@ func TestMongodbExporterConfig226(t *testing.T) {
"--mongodb.collstats-colls=col1,col2,col3",
"--mongodb.global-conn-pool",
"--mongodb.indexstats-colls=col1,col2,col3",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
}
actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion)
require.NoError(t, err)
Expand Down Expand Up @@ -196,7 +196,7 @@ func TestMongodbExporterConfig226(t *testing.T) {
"--mongodb.collstats-colls=db1.col1.one,db2.col2,db3",
"--mongodb.global-conn-pool",
"--mongodb.indexstats-colls=db1.col1.one,db2.col2,db3",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
}
actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion)
require.NoError(t, err)
Expand All @@ -223,7 +223,7 @@ func TestMongodbExporterConfig226(t *testing.T) {
"--mongodb.collstats-colls=db1.col1.one,db2.col2,db3",
"--mongodb.global-conn-pool",
"--mongodb.indexstats-colls=db1.col1.one,db2.col2,db3",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
}
actual, err := mongodbExporterConfig(node, mongodb, exporter, exposeSecrets, pmmAgentVersion)
require.NoError(t, err)
Expand Down Expand Up @@ -258,7 +258,7 @@ func TestMongodbExporterConfig(t *testing.T) {
"--collect.topmetrics",
"--no-collect.connpoolstats",
"--no-collect.indexusage",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"MONGODB_URI=mongodb://username:s3cur3%20p%[email protected]:27017/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000",
Expand Down Expand Up @@ -343,7 +343,7 @@ func TestMongodbExporterConfig(t *testing.T) {
"--collect.database",
"--no-collect.connpoolstats",
"--no-collect.indexusage",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
}
require.NoError(t, err)
Expand Down Expand Up @@ -376,7 +376,7 @@ func TestNewMongodbExporterConfig(t *testing.T) {
Args: []string{
"--compatible-mode",
"--mongodb.global-conn-pool",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"MONGODB_URI=mongodb://username:s3cur3%20p%[email protected]:27017/?connectTimeoutMS=1000&directConnection=true&serverSelectionTimeoutMS=1000",
Expand Down Expand Up @@ -430,7 +430,7 @@ func TestMongodbExporterConfig228_WebConfigAuth(t *testing.T) {
"--compatible-mode",
"--discovering-mode",
"--mongodb.global-conn-pool",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
"--web.config={{ .TextFiles.webConfigPlaceholder }}",
}

Expand Down
6 changes: 3 additions & 3 deletions managed/services/agents/mysql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestMySQLdExporterConfig(t *testing.T) {
"--exporter.global-conn-pool",
"--exporter.max-idle-conns=3",
"--exporter.max-open-conns=3",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s",
Expand Down Expand Up @@ -193,7 +193,7 @@ func TestMySQLdExporterConfigTablestatsGroupDisabled(t *testing.T) {
"--mysql.ssl-ca-file={{ .TextFiles.tlsCa }}",
"--mysql.ssl-cert-file={{ .TextFiles.tlsCert }}",
"--mysql.ssl-key-file={{ .TextFiles.tlsKey }}",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s&tls=custom",
Expand Down Expand Up @@ -292,7 +292,7 @@ func TestMySQLdExporterConfigDisabledCollectors(t *testing.T) {
"--exporter.global-conn-pool",
"--exporter.max-idle-conns=3",
"--exporter.max-open-conns=3",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s",
Expand Down
6 changes: 3 additions & 3 deletions managed/services/agents/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func TestNodeExporterConfig(t *testing.T) {
"--no-collector.xfs",
"--no-collector.zfs",
"--web.disable-exporter-metrics",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"HTTP_AUTH=pmm:agent-id",
Expand Down Expand Up @@ -247,7 +247,7 @@ func TestNodeExporterConfig(t *testing.T) {
"--no-collector.xfs",
"--no-collector.zfs",
"--web.disable-exporter-metrics",
"--web.listen-address=:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"HTTP_AUTH=pmm:agent-id",
Expand Down Expand Up @@ -282,7 +282,7 @@ func TestNodeExporterConfig(t *testing.T) {
"--collector.textfile.directory.lr=" + pathsBase(agentVersion, "{{", "}}") + "/collectors/textfile-collector/low-resolution",
"--collector.textfile.directory.mr=" + pathsBase(agentVersion, "{{", "}}") + "/collectors/textfile-collector/medium-resolution",
"--web.disable-exporter-metrics",
"--web.listen-address=:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"HTTP_AUTH=pmm:agent-id",
Expand Down
12 changes: 6 additions & 6 deletions managed/services/agents/postgresql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (s *PostgresExporterConfigTestSuite) SetupTest() {
"--collect.custom_query.lr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/low-resolution",
"--collect.custom_query.mr",
"--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"DATA_SOURCE_NAME=postgres://username:s3cur3%20p%[email protected]:5432/postgres?connect_timeout=1&sslmode=disable",
Expand Down Expand Up @@ -167,7 +167,7 @@ func (s *PostgresExporterConfigTestSuite) TestDisabledCollectors() {
"--collect.custom_query.lr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/low-resolution",
"--collect.custom_query.mr",
"--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
}
requireNoDuplicateFlags(s.T(), actual.Args)
Expand Down Expand Up @@ -205,7 +205,7 @@ func TestAutoDiscovery(t *testing.T) {
"--collect.custom_query.lr.directory=" + pathsBase(pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/low-resolution",
"--collect.custom_query.mr",
"--collect.custom_query.mr.directory=" + pathsBase(pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"DATA_SOURCE_NAME=postgres://username:s3cur3%20p%[email protected]:5432/postgres?connect_timeout=1&sslmode=disable",
Expand Down Expand Up @@ -324,7 +324,7 @@ func (s *PostgresExporterConfigTestSuite) TestAzureTimeout() {
"--collect.custom_query.mr",
"--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution",
"--exclude-databases=template0,template1,postgres,cloudsqladmin,pmm-managed-dev,azure_maintenance,rdsadmin",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"DATA_SOURCE_NAME=postgres://username:s3cur3%20p%[email protected]:5432/postgres?connect_timeout=5&sslmode=disable",
Expand Down Expand Up @@ -370,7 +370,7 @@ func (s *PostgresExporterConfigTestSuite) TestPrometheusWebConfig() {
"--collect.custom_query.mr",
"--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution",
"--exclude-databases=template0,template1,postgres,cloudsqladmin,pmm-managed-dev,azure_maintenance,rdsadmin",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
"--web.config={{ .TextFiles.webConfigPlaceholder }}",
},
Env: []string{
Expand Down Expand Up @@ -419,7 +419,7 @@ func (s *PostgresExporterConfigTestSuite) TestSSLSni() {
"--collect.custom_query.mr",
"--collect.custom_query.mr.directory=" + pathsBase(s.pmmAgentVersion, "{{", "}}") + "/collectors/custom-queries/postgresql/medium-resolution",
"--exclude-databases=template0,template1,postgres,cloudsqladmin,pmm-managed-dev,azure_maintenance,rdsadmin",
"--web.listen-address=1.2.3.4:{{ .listen_port }}",
"--web.listen-address=0.0.0.0:{{ .listen_port }}",
"--web.config={{ .TextFiles.webConfigPlaceholder }}",
},
Env: []string{
Expand Down
8 changes: 4 additions & 4 deletions managed/services/agents/proxysql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestProxySQLExporterConfig(t *testing.T) {
"-collect.mysql_connection_pool",
"-collect.mysql_status",
"-collect.stats_memory_metrics",
"-web.listen-address=1.2.3.4:{{ .listen_port }}",
"-web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s",
Expand Down Expand Up @@ -89,7 +89,7 @@ func TestProxySQLExporterConfig(t *testing.T) {
Args: []string{
"-collect.mysql_connection_pool",
"-collect.mysql_status",
"-web.listen-address=1.2.3.4:{{ .listen_port }}",
"-web.listen-address=0.0.0.0:{{ .listen_port }}",
},
}
require.Equal(t, expected.Args, actual.Args)
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestProxySQLExporterConfig(t *testing.T) {
"-collect.mysql_status",
"-collect.stats_command_counter",
"-collect.stats_memory_metrics",
"-web.listen-address=1.2.3.4:{{ .listen_port }}",
"-web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s",
Expand Down Expand Up @@ -161,7 +161,7 @@ func TestProxySQLExporterConfig(t *testing.T) {
"-collect.runtime_mysql_servers",
"-collect.stats_command_counter",
"-collect.stats_memory_metrics",
"-web.listen-address=1.2.3.4:{{ .listen_port }}",
"-web.listen-address=0.0.0.0:{{ .listen_port }}",
},
Env: []string{
"DATA_SOURCE_NAME=username:s3cur3 p@$$w0r4.@tcp(1.2.3.4:3306)/?timeout=1s",
Expand Down
2 changes: 1 addition & 1 deletion managed/services/agents/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func (u *StateUpdater) sendSetStateRequest(ctx context.Context, agent *pmmAgentI
if err != nil {
return err
}
node, _ := models.FindNodeByID(u.db.Querier, pointer.GetString(row.NodeID))
node, _ := models.FindNodeByID(u.db.Querier, pointer.GetString(pmmAgent.RunsOnNodeID))
switch row.AgentType { //nolint:exhaustive
case models.MySQLdExporterType:
agentProcesses[row.AgentID] = mysqldExporterConfig(node, service, row, redactMode, pmmAgentVersion)
Expand Down

0 comments on commit 01d08b4

Please sign in to comment.