PMM-14267 Fix persistence of changed listen ports#5007
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v3 #5007 +/- ##
==========================================
- Coverage 45.90% 45.86% -0.05%
==========================================
Files 366 367 +1
Lines 38438 38552 +114
==========================================
+ Hits 17646 17680 +34
- Misses 19093 19172 +79
- Partials 1699 1700 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
278d929 to
ca76922
Compare
ca76922 to
e66c34f
Compare
| golangci_lint_flags: "-c=.golangci.yml" | ||
| golangci_lint_version: v2.6.2 # Version should match specified in Makefile | ||
|
|
||
| - name: Run go-consistent |
There was a problem hiding this comment.
I'm removing the go-consistent tool for two major reasons:
- part of its functionality is already covered by golangci-lint
- it is inflexible and difficult to configure, for example it can't reliably skip tests, which requires workarounds and is simply annoying
|
|
||
| agent := &models.Agent{AgentID: agentID} | ||
| err := q.Reload(agent) | ||
| agent, err := models.FindAgentByID(q, agentID) |
There was a problem hiding this comment.
Refactored in favor of using the model.
| disable-version-string: True | ||
| with-expecter: False | ||
| resolve-type-alias: False | ||
| issue-845-fix: True |
There was a problem hiding this comment.
Fixed a mockery warning.
| RequestConfigurationUpdate() | ||
| // ForceConfigurationUpdate triggers immediate synchronous configuration update, | ||
| // bypassing the batch delay. Use this for critical updates like port changes. | ||
| ForceConfigurationUpdate(ctx context.Context) error |
There was a problem hiding this comment.
why a separate method is required? Can't port change be handled via RequestConfigurationUpdate?
There was a problem hiding this comment.
This one is synchronous, and RequestConfigurationUpdate is async. We previously used that one, and it worked well, except in cases where the port number is changed. In such case, we need to synchronously update VM's configuration, otherwise there will be bugs like the one in question.
PMM-14267
Link to the Feature Build: SUBMODULES-4217
This pull request introduces a mechanism for immediate VictoriaMetrics configuration updates when an agent's listen port changes, preventing stale port scraping. It adds a new method to the
prometheusServiceinterface, updates the handler logic to detect port changes, and provides comprehensive unit tests for this detection. Additionally, mock implementations and interface definitions across multiple packages are updated to support the new method.Immediate VictoriaMetrics configuration update on port changes:
ForceConfigurationUpdate(ctx context.Context) errorto theprometheusServiceinterface inmanaged/services/agents/deps.go,managed/services/inventory/deps.go,managed/services/management/deps.go, andmanaged/services/server/deps.go, along with documentation explaining its purpose for critical updates like port changes. [1] [2] [3] [4]managed/services/agents/handler.goto detect listen port changes using the newcheckPortChangedfunction, and triggerForceConfigurationUpdatefor VictoriaMetrics when a port change is detected. [1] [2] [3] [4]models.FindAgentByIDfor consistency and reliability in agent lookup. [1] [2]Testing and mocks:
checkPortChangedinmanaged/services/agents/handler_test.go, covering edge cases such as agent not found, old port zero, port equality, port difference, and type conversion.managed/services/inventory/mock_prometheus_service_test.go,managed/services/management/mock_prometheus_service_test.go, andmanaged/services/server/mock_prometheus_service_test.goto support the newForceConfigurationUpdatemethod. [1] [2] [3]Dependency and configuration updates:
github.com/DATA-DOG/go-sqlmockdependency from v1.5.0 to v1.5.2 ingo.modfor improved testing support.issue-845-fixin.mockery.yamlas per the warning message.go-consistenttool for two major reasons:golangci-lint