PMM-14858 Skip built-in ClickHouse scrape for external instances

managed/cmd/pmm-managed/main.go

@@ -729,6 +729,7 @@ func main() { //nolint:gocognit,maintidx,cyclop
 	clickhouseAddrF := kingpin.Flag("clickhouse-addr", "Clickhouse database address").Default("127.0.0.1:9000").Envar("PMM_CLICKHOUSE_ADDR").String()
 	clickhouseUsernameF := kingpin.Flag("clickhouse-username", "Clickhouse database user").Default("default").Envar("PMM_CLICKHOUSE_USER").String()
 	clickhousePasswordF := kingpin.Flag("clickhouse-password", "Clickhouse database user password").Default("clickhouse").Envar("PMM_CLICKHOUSE_PASSWORD").String()
+	clickhouseBuiltinDisabledF := kingpin.Flag("clickhouse-disable-builtin", "Disable the built-in ClickHouse").Envar("PMM_DISABLE_BUILTIN_CLICKHOUSE").Bool()
 
 	watchtowerHostF := kingpin.Flag("watchtower-host", "Watchtower host").Default("http://watchtower:8080").Envar("PMM_WATCHTOWER_HOST").URL()
 
@@ -824,15 +825,19 @@ func main() { //nolint:gocognit,maintidx,cyclop
 	grafanadb.DSN.DB = "grafana"
 	grafanadb.DSN.Params = q.Encode()
 
-	chURI := url.URL{
-		Scheme: "tcp",
-		User:   url.UserPassword(*clickhouseUsernameF, *clickhousePasswordF),
-		Host:   *clickhouseAddrF,
-		Path:   *clickHouseDatabaseF,
+	chParams, err := models.NewClickHouseParams(
+		*clickhouseAddrF,
+		*clickHouseDatabaseF,
+		*clickhouseUsernameF,
+		*clickhousePasswordF,
+		*clickhouseBuiltinDisabledF,
+	)
+	if err != nil {
+		l.Panicf("cannot load clickhouse params: %+v", err)
 	}
 
 	qanDB := ds.QanDBSelect
-	qanDB.DSN = chURI.String()
+	qanDB.DSN = chParams.URL().String()
 
 	ds.VM.Address = *victoriaMetricsURLF
 
@@ -882,7 +887,7 @@ func main() { //nolint:gocognit,maintidx,cyclop
 
 	cleaner := clean.New(db)
 	externalRules := vmalert.NewExternalRules()
-	vmdb, err := victoriametrics.NewVictoriaMetrics(*victoriaMetricsConfigF, db, vmParams, haService)
+	vmdb, err := victoriametrics.NewVictoriaMetrics(*victoriaMetricsConfigF, db, vmParams, chParams, haService)
 	if err != nil {
 		l.Panicf("VictoriaMetrics service problem: %+v", err)
 	}
@@ -1022,7 +1027,7 @@ func main() { //nolint:gocognit,maintidx,cyclop
 	versionCache := versioncache.New(db, versioner)
 
 	dumpService := dump.New(db, &dump.URLs{
-		ClickhouseURL: chURI.String(),
+		ClickhouseURL: chParams.URL().String(),
 		VMURL:         *victoriaMetricsURLF,
 	})
 

managed/models/clickhouse_params.go

@@ -0,0 +1,74 @@
+// Copyright (C) 2023 Percona LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+package models
+
+import (
+	"fmt"
+	"net"
+	"net/url"
+	"strconv"
+)
+
+// ClickHouseParams represents ClickHouse server params.
+type ClickHouseParams struct {
+	BuiltinDisabled bool
+	url             *url.URL
+}
+
+// ExternalClickHouse returns true if ClickHouse is configured externally.
+func (p *ClickHouseParams) ExternalClickHouse() bool {
+	return p.url.Hostname() != "127.0.0.1"
+}
+
+// URL returns the ClickHouse URL.
+func (p *ClickHouseParams) URL() *url.URL {
+	u := *p.url
+	return &u
+}
+
+// NewClickHouseParams returns validated ClickHouse configuration params,
+// or an error if any required field is missing or malformed.
+func NewClickHouseParams(addr, dbName, dbUsername, dbPassword string, builtinDisabled bool) (*ClickHouseParams, error) {
+	if addr == "" {
+		return nil, fmt.Errorf("addr is required")
+	}
+	host, port, err := net.SplitHostPort(addr)
+	if err != nil {
+		return nil, fmt.Errorf("invalid addr %q: %w", addr, err)
+	}
+	if host == "" {
+		return nil, fmt.Errorf("invalid addr %q: empty host", addr)
+	}
+	if _, err := strconv.ParseUint(port, 10, 16); err != nil {
+		return nil, fmt.Errorf("invalid port in addr %q: %w", addr, err)
+	}
+	if dbName == "" {
+		return nil, fmt.Errorf("database name is required")
+	}
+	if dbUsername == "" {
+		return nil, fmt.Errorf("username is required")
+	}
+
+	return &ClickHouseParams{
+		BuiltinDisabled: builtinDisabled,
+		url: &url.URL{
+			Scheme: "tcp",
+			User:   url.UserPassword(dbUsername, dbPassword),
+			Host:   addr,
+			Path:   dbName,
+		},
+	}, nil
+}

managed/models/clickhouse_params_test.go

@@ -0,0 +1,101 @@
+// Copyright (C) 2023 Percona LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+package models
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewClickHouseParams(t *testing.T) {
+	t.Run("valid", func(t *testing.T) {
+		p, err := NewClickHouseParams("127.0.0.1:9000", "pmm", "default", "clickhouse", false)
+		require.NoError(t, err)
+		assert.Equal(t, "tcp://default:clickhouse@127.0.0.1:9000/pmm", p.URL().String())
+		assert.False(t, p.BuiltinDisabled)
+	})
+
+	t.Run("valid builtin disabled", func(t *testing.T) {
+		p, err := NewClickHouseParams("127.0.0.1:9000", "pmm", "default", "clickhouse", true)
+		require.NoError(t, err)
+		assert.True(t, p.BuiltinDisabled)
+	})
+
+	t.Run("valid empty password", func(t *testing.T) {
+		_, err := NewClickHouseParams("127.0.0.1:9000", "pmm", "default", "", false)
+		require.NoError(t, err)
+	})
+
+	errCases := []struct {
+		name       string
+		addr       string
+		dbName     string
+		dbUsername string
+		dbPassword string
+		wantErrSub string
+	}{
+		{"empty addr", "", "pmm", "default", "clickhouse", "addr is required"},
+		{"missing port", "127.0.0.1", "pmm", "default", "clickhouse", "invalid addr"},
+		{"empty host", ":9000", "pmm", "default", "clickhouse", "empty host"},
+		{"non numeric port", "localhost:abc", "pmm", "default", "clickhouse", "invalid port"},
+		{"port out of range", "localhost:99999", "pmm", "default", "clickhouse", "invalid port"},
+		{"empty db name", "127.0.0.1:9000", "", "default", "clickhouse", "database name is required"},
+		{"empty username", "127.0.0.1:9000", "pmm", "", "clickhouse", "username is required"},
+	}
+	for _, tc := range errCases {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := NewClickHouseParams(tc.addr, tc.dbName, tc.dbUsername, tc.dbPassword, false)
+			require.Error(t, err)
+			assert.ErrorContains(t, err, tc.wantErrSub)
+		})
+	}
+}
+
+func TestCHParamsExternalClickHouse(t *testing.T) {
+	cases := []struct {
+		name string
+		addr string
+		want bool
+	}{
+		{"loopback", "127.0.0.1:9000", false},
+		{"localhost", "localhost:9000", true},
+		{"external host", "ch-01.test.net:9000", true},
+		{"wildcard", "0.0.0.0:9000", true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			p, err := NewClickHouseParams(tc.addr, "pmm", "default", "clickhouse", false)
+			require.NoError(t, err)
+			assert.Equal(t, tc.want, p.ExternalClickHouse())
+		})
+	}
+}
+
+func TestCHParamsURL(t *testing.T) {
+	t.Run("simple", func(t *testing.T) {
+		p, err := NewClickHouseParams("127.0.0.1:9000", "pmm", "default", "clickhouse", false)
+		require.NoError(t, err)
+		assert.Equal(t, "tcp://default:clickhouse@127.0.0.1:9000/pmm", p.URL().String())
+	})
+
+	t.Run("password with special chars", func(t *testing.T) {
+		p, err := NewClickHouseParams("127.0.0.1:9000", "pmm", "default", "p@ss/word", false)
+		require.NoError(t, err)
+		assert.Equal(t, "tcp://default:p%40ss%2Fword@127.0.0.1:9000/pmm", p.URL().String())
+	})
+}

managed/services/victoriametrics/prometheus.go

@@ -87,7 +87,7 @@ func AddScrapeConfigs(l *logrus.Entry, cfg *config.Config, q *reform.Querier, //
 		}
 		switch {
 		case pushMetrics:
-			paramsHost = "127.0.0.1"
+			paramsHost = localhost
 		case agent.PMMAgentID != nil:
 			pmmAgentNode = &models.Node{NodeID: pointer.GetString(pmmAgent.RunsOnNodeID)}
 			if err = q.Reload(pmmAgentNode); err != nil {
@@ -273,6 +273,20 @@ func addInternalServicesToScrape(s models.MetricsResolutions, svc *Service, pmmS
 	}
 
 	if svc.params.ExternalVM() {
+		svc.l.Infof("Skip scrape config for ClickHouse, VictoriaMetrics is configured to run externally.")
+		return cfg
+	}
+
+	if svc.chParams.BuiltinDisabled {
+		svc.l.Infof("Skip scrape config for ClickHouse, scraping has been disabled for builtin clickhouse.")
+		return cfg
+	}
+
+	if svc.chParams.ExternalClickHouse() {
+		svc.l.Warnf(
+			"External ClickHouse detected (%s); skipping built-in ClickHouse scrape (127.0.0.1:9363). Set PMM_DISABLE_BUILTIN_CLICKHOUSE=true to make this explicit.",
+			svc.chParams.URL().Redacted(),
+		)
 		return cfg
 	}
 

managed/services/victoriametrics/scrape_configs.go

@@ -55,7 +55,7 @@ func scrapeConfigForClickhouse(mr time.Duration, pmmServerNodeName string) *conf
 		MetricsPath:    "/metrics",
 		ServiceDiscoveryConfig: config.ServiceDiscoveryConfig{
 			StaticConfigs: []*config.Group{{
-				Targets: []string{"127.0.0.1:9363"},
+				Targets: []string{localhost + ":9363"},
 				Labels:  map[string]string{"instance": pmmServerNodeName},
 			}},
 		},
@@ -70,7 +70,7 @@ func scrapeConfigForGrafana(interval time.Duration, pmmServerNodeName string) *c
 		MetricsPath:    "/graph/metrics",
 		ServiceDiscoveryConfig: config.ServiceDiscoveryConfig{
 			StaticConfigs: []*config.Group{{
-				Targets: []string{"127.0.0.1:3000"},
+				Targets: []string{localhost + ":3000"},
 				Labels:  map[string]string{"instance": pmmServerNodeName},
 			}},
 		},
@@ -85,7 +85,7 @@ func scrapeConfigForPMMManaged(interval time.Duration, pmmServerNodeName string)
 		MetricsPath:    "/debug/metrics",
 		ServiceDiscoveryConfig: config.ServiceDiscoveryConfig{
 			StaticConfigs: []*config.Group{{
-				Targets: []string{"127.0.0.1:7773"},
+				Targets: []string{localhost + ":7773"},
 				Labels:  map[string]string{"instance": pmmServerNodeName},
 			}},
 		},
@@ -100,7 +100,7 @@ func scrapeConfigForQANAPI2(interval time.Duration, pmmServerNodeName string) *c
 		MetricsPath:    "/debug/metrics",
 		ServiceDiscoveryConfig: config.ServiceDiscoveryConfig{
 			StaticConfigs: []*config.Group{{
-				Targets: []string{"127.0.0.1:9933"},
+				Targets: []string{localhost + ":9933"},
 				Labels:  map[string]string{"instance": pmmServerNodeName},
 			}},
 		},
@@ -116,7 +116,7 @@ func scrapeConfigForNomadServer(resolution time.Duration, pmmServerNodeName stri
 		Scheme:         "https",
 		ServiceDiscoveryConfig: config.ServiceDiscoveryConfig{
 			StaticConfigs: []*config.Group{{
-				Targets: []string{"127.0.0.1:4646"},
+				Targets: []string{localhost + ":4646"},
 				Labels:  map[string]string{"instance": pmmServerNodeName},
 			}},
 		},

managed/services/victoriametrics/victoriametrics.go

@@ -49,6 +49,8 @@ const (
 	victoriametricsDir     = "/srv/victoriametrics"
 	victoriametricsDataDir = "/srv/victoriametrics/data"
 	dirPerm                = os.FileMode(0o775)
+
+	localhost = "127.0.0.1"
 )
 
 var checkFailedRE = regexp.MustCompile(`(?s)cannot unmarshal data: (.+)`)
@@ -60,26 +62,37 @@ type Service struct {
 	baseURL          *url.URL
 	client           *http.Client
 
-	params *models.VictoriaMetricsParams
+	params   *models.VictoriaMetricsParams
+	chParams *models.ClickHouseParams
 
 	l         *logrus.Entry
 	reloadCh  chan struct{}
 	haService haService
 }
 
 // NewVictoriaMetrics creates new VictoriaMetrics service.
-func NewVictoriaMetrics(scrapeConfigPath string, db *reform.DB, params *models.VictoriaMetricsParams, haService haService) (*Service, error) {
+func NewVictoriaMetrics(
+	scrapeConfigPath string,
+	db *reform.DB,
+	params *models.VictoriaMetricsParams,
+	chParams *models.ClickHouseParams,
+	haService haService,
+) (*Service, error) {
 	u, err := url.Parse(params.URL())
 	if err != nil {
 		return nil, err
 	}
+	if chParams == nil {
+		return nil, fmt.Errorf("ClickHouseParams is required")
+	}
 
 	return &Service{
 		scrapeConfigPath: scrapeConfigPath,
 		db:               db,
 		baseURL:          u,
 		client:           &http.Client{}, // TODO instrument with utils/irt; see vmalert package https://jira.percona.com/browse/PMM-7229
 		params:           params,
+		chParams:         chParams,
 		l:                logrus.WithField("component", "victoriametrics"),
 		reloadCh:         make(chan struct{}, 1),
 		haService:        haService,
@@ -435,7 +448,7 @@ func scrapeConfigForVMAlert(interval time.Duration, pmmServerNodeName string) *c
 		ServiceDiscoveryConfig: config.ServiceDiscoveryConfig{
 			StaticConfigs: []*config.Group{
 				{
-					Targets: []string{"127.0.0.1:8880"},
+					Targets: []string{localhost + ":8880"},
 					Labels:  map[string]string{"instance": pmmServerNodeName},
 				},
 			},