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

managed/cmd/pmm-managed/main.go

@@ -726,6 +726,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()
 
@@ -821,15 +822,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
 
@@ -879,7 +884,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)
 	}
@@ -1019,7 +1024,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 DSN.
+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

@@ -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/victoriametrics.go

@@ -60,26 +60,31 @@ 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,

managed/services/victoriametrics/victoriametrics_test.go

@@ -45,10 +45,13 @@ func setup(t *testing.T) (*reform.DB, *Service, []byte) {
 	vmParams, err := models.NewVictoriaMetricsParams(models.BasePrometheusConfigPath, models.VMBaseURL)
 	check.NoError(err)
 
+	chParams, err := models.NewClickHouseParams("127.0.0.1:9000", "pmm", "default", "clickhouse", false)
+	check.NoError(err)
+
 	mockHaService := newMockHaService(t)
 	mockHaService.On("Params").Return(&models.HAParams{Enabled: false, NodeID: "pmm-ha-service-0"}).Maybe()
 	mockHaService.On("IsLeader").Return(true).Maybe()
-	svc, err := NewVictoriaMetrics(configPath, db, vmParams, mockHaService)
+	svc, err := NewVictoriaMetrics(configPath, db, vmParams, chParams, mockHaService)
 	check.NoError(err)
 
 	original, err := os.ReadFile(configPath)
@@ -991,3 +994,58 @@ scrape_configs:
 	assert.NoError(t, err)
 	assert.Equal(t, expected, string(newcfg), "actual:\n%s", newcfg)
 }
+
+func TestVMConfig_OmitsClickhouseScrape(t *testing.T) {
+	newSvc := func(t *testing.T, chParams *models.ClickHouseParams, vmURL string) (*reform.DB, *Service) {
+		t.Helper()
+		sqlDB := testdb.Open(t, models.SkipFixtures, nil)
+		db := reform.NewDB(sqlDB, postgresql.Dialect, reform.NewPrintfLogger(t.Logf))
+		vmParams, err := models.NewVictoriaMetricsParams(models.BasePrometheusConfigPath, vmURL)
+		require.NoError(t, err)
+
+		mockHaService := newMockHaService(t)
+		mockHaService.On("Params").Return(&models.HAParams{Enabled: false, NodeID: "pmm-ha-service-0"}).Maybe()
+		mockHaService.On("IsLeader").Return(true).Maybe()
+
+		svc, err := NewVictoriaMetrics(configPath, db, vmParams, chParams, mockHaService)
+		require.NoError(t, err)
+		require.NoError(t, svc.IsReady(t.Context()))
+		t.Cleanup(func() { _ = db.DBInterface().(*sql.DB).Close() })
+		return db, svc
+	}
+
+	newCHParams := func(t *testing.T, addr string, builtinDisabled bool) *models.ClickHouseParams {
+		t.Helper()
+		chp, err := models.NewClickHouseParams(addr, "pmm", "default", "clickhouse", builtinDisabled)
+		require.NoError(t, err)
+		return chp
+	}
+
+	cases := []struct {
+		name            string
+		addr            string
+		builtinDisabled bool
+		vmURL           string
+		wantClickhouse  bool
+	}{
+		{"internal enabled positive control", "127.0.0.1:9000", false, models.VMBaseURL, true},
+		{"external addr, builtin not disabled (auto-skip)", "ch.external:9000", false, models.VMBaseURL, false},
+		{"builtin disabled", "127.0.0.1:9000", true, models.VMBaseURL, false},
+		{"external addr, builtin disabled", "ch.external:9000", true, models.VMBaseURL, false},
+		{"external VM short-circuits CH scrape", "127.0.0.1:9000", false, "http://vm.external:9090/prometheus/", false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			_, svc := newSvc(t, newCHParams(t, tc.addr, tc.builtinDisabled), tc.vmURL)
+			cfg, err := svc.marshalConfig(svc.loadBaseConfig())
+			require.NoError(t, err)
+			if tc.wantClickhouse {
+				assert.Contains(t, string(cfg), "127.0.0.1:9363")
+				assert.Contains(t, string(cfg), "job_name: clickhouse")
+			} else {
+				assert.NotContains(t, string(cfg), "127.0.0.1:9363")
+				assert.NotContains(t, string(cfg), "job_name: clickhouse")
+			}
+		})
+	}
+}