Skip to content

PMM-11826 Refactor vmagent configuration handling. #4049

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: v3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions agent/agents/supervisor/supervisor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,74 @@ plugin "raw_exec" {
assert.Equal(t, expectedConfig, string(b))
})

t.Run("VMAgent", func(t *testing.T) {
t.Parallel()
s, teardown := setup(t)
defer teardown()

// Update the config to include VMAgent path
cfg := s.cfg.Get()
cfg.Paths.VMAgent = "/path/to/vmagent"

p := &agentv1.SetStateRequest_AgentProcess{
Type: inventoryv1.AgentType_AGENT_TYPE_VM_AGENT,
TemplateLeftDelim: "{{",
TemplateRightDelim: "}}",
Args: []string{
"-envflag.enable=true",
"-envflag.prefix=VMAGENT_",
"-remoteWrite.tmpDataPath={{.tmp_dir}}/vmagent-temp-dir",
"-promscrape.config={{.TextFiles.vmagentscrapecfg}}",
"-httpListenAddr=127.0.0.1:{{.listen_port}}",
},
Env: []string{
"VMAGENT_remoteWrite_url={{.server_url}}/victoriametrics/api/v1/write",
"VMAGENT_remoteWrite_tlsInsecureSkipVerify={{.server_insecure}}",
"VMAGENT_promscrape_maxScrapeSize=64MiB",
"VMAGENT_remoteWrite_maxDiskUsagePerURL=1073741824",
"VMAGENT_loggerLevel=INFO",
"VMAGENT_remoteWrite_basicAuth_username={{.server_username}}",
"VMAGENT_remoteWrite_basicAuth_password={{.server_password}}",
},
TextFiles: map[string]string{
"vmagentscrapecfg": "global:\n scrape_interval: 15s\n",
},
}
actual, err := s.processParams("vmagent-id", p, 12345)
require.NoError(t, err)

configFilePath := filepath.Join(s.cfg.Get().Paths.TempDir, "agent_type_vm_agent", "vmagent-id", "vmagentscrapecfg")
tempDir := s.cfg.Get().Paths.TempDir
expected := process.Params{
Path: "/path/to/vmagent",
Args: []string{
"-envflag.enable=true",
"-envflag.prefix=VMAGENT_",
"-remoteWrite.tmpDataPath=" + tempDir + "/vmagent-temp-dir",
"-promscrape.config=" + configFilePath,
"-httpListenAddr=127.0.0.1:12345",
},
Env: []string{
"VMAGENT_remoteWrite_url=https://server:443/victoriametrics/api/v1/write",
"VMAGENT_remoteWrite_tlsInsecureSkipVerify=false",
"VMAGENT_promscrape_maxScrapeSize=64MiB",
"VMAGENT_remoteWrite_maxDiskUsagePerURL=1073741824",
"VMAGENT_loggerLevel=INFO",
"VMAGENT_remoteWrite_basicAuth_username=admin",
"VMAGENT_remoteWrite_basicAuth_password=admin",
},
}
assert.Equal(t, expected.Path, actual.Path)
assert.ElementsMatch(t, expected.Args, actual.Args)
assert.ElementsMatch(t, expected.Env, actual.Env)
assert.NotEmpty(t, actual.TemplateParams)
assert.NotEmpty(t, actual.TemplateRenderer)
require.FileExists(t, configFilePath)
b, err := os.ReadFile(configFilePath) //nolint:gosec
require.NoError(t, err)
assert.Equal(t, "global:\n scrape_interval: 15s\n", string(b))
})

t.Run("BadTemplate", func(t *testing.T) {
t.Parallel()
s, teardown := setup(t)
Expand Down
88 changes: 72 additions & 16 deletions managed/services/agents/vmagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package agents

import (
"fmt"
"net/url"
"os"
"sort"
"strings"
Expand All @@ -31,48 +32,103 @@ var (
maxScrapeSizeDefault = "64MiB"
)

// extractCredentialsFromURL extracts username and password from a URL string.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are comments for not exposed func needed? I mena do we want to keep them? Since func naming is good and clear. Same applies for rest.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it depends, but in this case AI just followed our codebase where we write comments for private functions. It's not necessary, hovewer doesn't harm.

// Returns empty strings if no credentials are found or if there's an error parsing the URL.
func extractCredentialsFromURL(urlStr string) (string, string) {
if urlStr == "" {
return "", ""
}

parsedURL, err := url.Parse(urlStr)
if err != nil || parsedURL.User == nil {
return "", ""
}

username := parsedURL.User.Username()
password := ""
if pwd, ok := parsedURL.User.Password(); ok {
password = pwd
}

return username, password
}

// vmAgentConfig returns desired configuration of vmagent process.
func vmAgentConfig(scrapeCfg string, params victoriaMetricsParams) *agentv1.SetStateRequest_AgentProcess {
serverURL := "{{.server_url}}/victoriametrics/"
var vmUsername, vmPassword string

if params.ExternalVM() {
serverURL = params.URL()

// Extract username and password from external VM URL if present
vmUsername, vmPassword = extractCredentialsFromURL(serverURL)
}

maxScrapeSize := maxScrapeSizeDefault
if space := os.Getenv(maxScrapeSizeEnv); space != "" {
maxScrapeSize = space
}

interfaceToBind := envvars.GetInterfaceToBind()

// Only keep the specified exceptions as command line arguments
args := []string{
fmt.Sprintf("-remoteWrite.url=%sapi/v1/write", serverURL),
"-remoteWrite.tlsInsecureSkipVerify={{.server_insecure}}",
"-envflag.enable=true",
"-envflag.prefix=VMAGENT_",
"-remoteWrite.tmpDataPath={{.tmp_dir}}/vmagent-temp-dir",
"-promscrape.config={{.TextFiles.vmagentscrapecfg}}",
"-promscrape.maxScrapeSize=" + maxScrapeSize,
// 1GB disk queue size.
"-remoteWrite.maxDiskUsagePerURL=1073741824",
"-loggerLevel=INFO",
"-httpListenAddr=" + interfaceToBind + ":{{.listen_port}}",
// needed for login/password at client side.
"-envflag.enable=true",
"-envflag.prefix=VMAGENT_",
}

sort.Strings(args)

// Move all other parameters to environment variables
var envs []string
if !params.ExternalVM() {
envs = []string{
"VMAGENT_remoteWrite_basicAuth_username={{.server_username}}",
"VMAGENT_remoteWrite_basicAuth_password={{.server_password}}",
}
}

// First, collect all VMAGENT_ environment variables from the system
systemEnvs := make(map[string]string)
for _, env := range os.Environ() {
if strings.HasPrefix(env, envvars.ENVvmAgentPrefix) {
envs = append(envs, env)
parts := strings.SplitN(env, "=", 2)
if len(parts) == 2 {
systemEnvs[parts[0]] = parts[1]
}
}
}

// Helper function to add env var only if not already set by system
addEnvIfNotSet := func(key, value string) {
if _, exists := systemEnvs[key]; !exists {
envs = append(envs, key+"="+value)
}
}

// Add the parameters that were previously command line arguments (only if not overridden)
addEnvIfNotSet("VMAGENT_remoteWrite_url", fmt.Sprintf("%sapi/v1/write", serverURL))
addEnvIfNotSet("VMAGENT_remoteWrite_tlsInsecureSkipVerify", "{{.server_insecure}}")
addEnvIfNotSet("VMAGENT_promscrape_maxScrapeSize", maxScrapeSize)
addEnvIfNotSet("VMAGENT_remoteWrite_maxDiskUsagePerURL", "1073741824") // 1GB disk queue size
addEnvIfNotSet("VMAGENT_loggerLevel", "INFO")

// Set authentication based on VM type
if params.ExternalVM() && vmUsername != "" {
// Use credentials from external VM URL
addEnvIfNotSet("VMAGENT_remoteWrite_basicAuth_username", vmUsername)
if vmPassword != "" {
addEnvIfNotSet("VMAGENT_remoteWrite_basicAuth_password", vmPassword)
}
} else if !params.ExternalVM() {
// Use PMM server credentials for internal VM
addEnvIfNotSet("VMAGENT_remoteWrite_basicAuth_username", "{{.server_username}}")
addEnvIfNotSet("VMAGENT_remoteWrite_basicAuth_password", "{{.server_password}}")
}

// Add all system VMAGENT_ environment variables
for key, value := range systemEnvs {
envs = append(envs, key+"="+value)
}

sort.Strings(envs)

res := &agentv1.SetStateRequest_AgentProcess{
Expand Down
Loading
Loading