From d45b3b5f1d83b80b391283a11d9dd016ad4d8d30 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Tue, 28 Jul 2026 01:20:34 +0300 Subject: [PATCH 1/6] PMM-14665 Skip nodes internal to a PMM deployment when adding a service In an HA deployment the PostgreSQL cluster backing PMM's persistence layer runs pmm-client sidecars, so its pods were offered in the Nodes dropdown as valid monitoring delegates. Those Nodes are dedicated and must not take on extra monitoring workloads. PMM Server now accepts PMM_INTERNAL_NODE_NAME_PREFIXES, reports the Nodes it matches as is_pmm_internal_node, and rejects requests which delegate monitoring of a remote address to an Agent running on one of them. Local addresses remain allowed, so those Nodes keep monitoring the services inside their own pod. AddAzureDatabaseRequest gains pmm_agent_id, which the UI has always sent but the server discarded, placing Azure exporters on the PMM Server unconditionally. --- api/descriptor.bin | Bin 819175 -> 819633 bytes api/management/v1/azure.pb.go | 21 +- api/management/v1/azure.pb.validate.go | 2 + api/management/v1/azure.proto | 2 + .../add_azure_database_responses.go | 3 + .../management_service/get_node_responses.go | 4 + .../list_nodes_responses.go | 4 + api/management/v1/json/v1.json | 15 ++ api/management/v1/node.pb.go | 20 +- api/management/v1/node.pb.validate.go | 2 + api/management/v1/node.proto | 3 + api/swagger/swagger-dev.json | 15 ++ api/swagger/swagger.json | 15 ++ managed/cmd/pmm-managed/main.go | 21 ++ managed/cmd/pmm-managed/main_test.go | 14 + .../add_service_exporter_timeout_test.go | 2 +- managed/services/management/agent_test.go | 2 +- .../services/management/annotation_test.go | 2 +- managed/services/management/azure_database.go | 16 +- .../services/management/internal_node_test.go | 250 ++++++++++++++++++ managed/services/management/node.go | 64 ++--- managed/services/management/node_test.go | 6 +- managed/services/management/rds_test.go | 2 +- managed/services/management/service.go | 104 +++++++- managed/services/management/service_test.go | 4 +- managed/utils/envvars/parser.go | 3 + managed/utils/envvars/parser_test.go | 12 + 27 files changed, 545 insertions(+), 63 deletions(-) create mode 100644 managed/services/management/internal_node_test.go diff --git a/api/descriptor.bin b/api/descriptor.bin index e66dfa7c6f4d6140eb73b5a24ad7d69b5eba6284..2eb67c948a0bba19f6e623e2ec7fba024a8dc1da 100644 GIT binary patch delta 1018 zcmX|<-Afcv7{=$FnVp?;*3a={*A?t+8Y-k>(1iwtU{Du=nD^V}9(8DUMrLLe4Ki?2 zk?taf=mRQ*EYq}P#I=jU>!`mX2)c;=fO_Y++r_~A&Uw!Jyzg_)kH3R^D;Mth;LE<^ ztA5Pa{J5`wBoCLAKbL7xw@TCD`z2y3@#mB#NAmd~bBDBq7w?i6Dl-_ZnJX5vxfyPG zSu>w%XV60?D6Wk}O94I^*+ctEmAc?LG9^m@OeXJ!T43$baRwX=Sl_7WuDEvSd3T+F| zJRxcvzJ^^dNt@DV4A5rN%|?Tpmd70{SIAm+o~N2|dVQkNbjORu>y@2wXp=mW$>#9s z_{YKTpNW=e_|~9zz9tdYz9#8qq2uIYN*n&PaGTjWkJ%UO9=EV&+9d2H6kpOVWRRU7A1dYISZ)vWHld8m5?<>*$P|^TiYTbFx~WB&IJL%2Jh~5U z9o1bl*@w(oVWJy@+5t$16&D?)2cWkKsV+-+S7G#8yU_(v(52v6nXqL<6o)_>s$#4g zLQ``U^$1&rX(J`kS$$WplcS~9A>m96?FgjfiZQ`jN1!LgVS;HHY8m8uF&?6z)7g3Nc}VjfpYFOwiC! zs04DWTM9a0MVe?unlIC&QYqg6xymCkOkc!RUwmqbd2+> zJ1{v_XOb#&6^N?3dXkDtO zEGc2?a+DZ5r}seY4=skHdtjO|YNSZ5>VPyf!xly+f3t_zRy%-Wb%yIYaABfGQ(g=lpEaX9oL`vRCEE@OP7F38b%6{A&5gOiy!+l8@8gsEa?arb$@ Nhx}3K_wsK?{{aqcXa4{I diff --git a/api/management/v1/azure.pb.go b/api/management/v1/azure.pb.go index 04a05c47ec1..cd0f5b9bd3a 100644 --- a/api/management/v1/azure.pb.go +++ b/api/management/v1/azure.pb.go @@ -381,8 +381,10 @@ type AddAzureDatabaseRequest struct { Type DiscoverAzureDatabaseType `protobuf:"varint,25,opt,name=type,proto3,enum=management.v1.DiscoverAzureDatabaseType" json:"type,omitempty"` // Connection timeout for exporter (if set). ConnectionTimeout *durationpb.Duration `protobuf:"bytes,26,opt,name=connection_timeout,json=connectionTimeout,proto3" json:"connection_timeout,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // The pmm-agent identifier which should run agents. Defaults to the PMM Server's own pmm-agent. + PmmAgentId string `protobuf:"bytes,27,opt,name=pmm_agent_id,json=pmmAgentId,proto3" json:"pmm_agent_id,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *AddAzureDatabaseRequest) Reset() { @@ -597,6 +599,13 @@ func (x *AddAzureDatabaseRequest) GetConnectionTimeout() *durationpb.Duration { return nil } +func (x *AddAzureDatabaseRequest) GetPmmAgentId() string { + if x != nil { + return x.PmmAgentId + } + return "" +} + type AddAzureDatabaseResponse struct { state protoimpl.MessageState `protogen:"open.v1"` unknownFields protoimpl.UnknownFields @@ -658,7 +667,8 @@ const file_management_v1_azure_proto_rawDesc = "" + "node_model\x18\n" + " \x01(\tR\tnodeModel\"\x85\x01\n" + "\x1dDiscoverAzureDatabaseResponse\x12d\n" + - "\x17azure_database_instance\x18\x01 \x03(\v2,.management.v1.DiscoverAzureDatabaseInstanceR\x15azureDatabaseInstance\"\xfc\t\n" + + "\x17azure_database_instance\x18\x01 \x03(\v2,.management.v1.DiscoverAzureDatabaseInstanceR\x15azureDatabaseInstance\"\x9e\n" + + "\n" + "\x17AddAzureDatabaseRequest\x12\x1f\n" + "\x06region\x18\x01 \x01(\tB\a\xfaB\x04r\x02\x10\x01R\x06region\x12\x0e\n" + "\x02az\x18\x02 \x01(\tR\x02az\x12(\n" + @@ -688,7 +698,9 @@ const file_management_v1_azure_proto_rawDesc = "" + "\x16disable_query_examples\x18\x17 \x01(\bR\x14disableQueryExamples\x12?\n" + "\x1ctablestats_group_table_limit\x18\x18 \x01(\x05R\x19tablestatsGroupTableLimit\x12<\n" + "\x04type\x18\x19 \x01(\x0e2(.management.v1.DiscoverAzureDatabaseTypeR\x04type\x12R\n" + - "\x12connection_timeout\x18\x1a \x01(\v2\x19.google.protobuf.DurationB\b\xfaB\x05\xaa\x01\x022\x00R\x11connectionTimeout\x1a?\n" + + "\x12connection_timeout\x18\x1a \x01(\v2\x19.google.protobuf.DurationB\b\xfaB\x05\xaa\x01\x022\x00R\x11connectionTimeout\x12 \n" + + "\fpmm_agent_id\x18\x1b \x01(\tR\n" + + "pmmAgentId\x1a?\n" + "\x11CustomLabelsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\x1a\n" + @@ -726,7 +738,6 @@ var ( (*durationpb.Duration)(nil), // 7: google.protobuf.Duration } ) - var file_management_v1_azure_proto_depIdxs = []int32{ 0, // 0: management.v1.DiscoverAzureDatabaseInstance.type:type_name -> management.v1.DiscoverAzureDatabaseType 2, // 1: management.v1.DiscoverAzureDatabaseResponse.azure_database_instance:type_name -> management.v1.DiscoverAzureDatabaseInstance diff --git a/api/management/v1/azure.pb.validate.go b/api/management/v1/azure.pb.validate.go index be225936500..c66ee420f6e 100644 --- a/api/management/v1/azure.pb.validate.go +++ b/api/management/v1/azure.pb.validate.go @@ -637,6 +637,8 @@ func (m *AddAzureDatabaseRequest) validate(all bool) error { } } + // no validation rules for PmmAgentId + if len(errors) > 0 { return AddAzureDatabaseRequestMultiError(errors) } diff --git a/api/management/v1/azure.proto b/api/management/v1/azure.proto index 2d6eb7b5622..8f7d7aca3be 100644 --- a/api/management/v1/azure.proto +++ b/api/management/v1/azure.proto @@ -130,6 +130,8 @@ message AddAzureDatabaseRequest { google.protobuf.Duration connection_timeout = 26 [(validate.rules).duration = { gte: {seconds: 0} }]; + // The pmm-agent identifier which should run agents. Defaults to the PMM Server's own pmm-agent. + string pmm_agent_id = 27; } message AddAzureDatabaseResponse {} diff --git a/api/management/v1/json/client/management_service/add_azure_database_responses.go b/api/management/v1/json/client/management_service/add_azure_database_responses.go index 44e631ba826..9363d4f9dfb 100644 --- a/api/management/v1/json/client/management_service/add_azure_database_responses.go +++ b/api/management/v1/json/client/management_service/add_azure_database_responses.go @@ -272,6 +272,9 @@ type AddAzureDatabaseBody struct { // Connection timeout for exporter (if set). ConnectionTimeout string `json:"connection_timeout,omitempty"` + + // The pmm-agent identifier which should run agents. Defaults to the PMM Server's own pmm-agent. + PMMAgentID string `json:"pmm_agent_id,omitempty"` } // Validate validates this add azure database body diff --git a/api/management/v1/json/client/management_service/get_node_responses.go b/api/management/v1/json/client/management_service/get_node_responses.go index 9c9faa6b669..bf4972126b2 100644 --- a/api/management/v1/json/client/management_service/get_node_responses.go +++ b/api/management/v1/json/client/management_service/get_node_responses.go @@ -584,6 +584,10 @@ type GetNodeOKBodyNode struct { // True if this node is a PMM Server node (HA mode). IsPMMServerNode bool `json:"is_pmm_server_node,omitempty"` + + // True if this node belongs to the internal infrastructure of a PMM deployment + // (e.g. the HA persistence layer) and must not host user monitoring workloads. + IsPMMInternalNode bool `json:"is_pmm_internal_node,omitempty"` } // Validate validates this get node OK body node diff --git a/api/management/v1/json/client/management_service/list_nodes_responses.go b/api/management/v1/json/client/management_service/list_nodes_responses.go index 1f3aba69c3a..2bf6834181b 100644 --- a/api/management/v1/json/client/management_service/list_nodes_responses.go +++ b/api/management/v1/json/client/management_service/list_nodes_responses.go @@ -593,6 +593,10 @@ type ListNodesOKBodyNodesItems0 struct { // True if this node is a PMM Server node (HA mode). IsPMMServerNode bool `json:"is_pmm_server_node,omitempty"` + + // True if this node belongs to the internal infrastructure of a PMM deployment + // (e.g. the HA persistence layer) and must not host user monitoring workloads. + IsPMMInternalNode bool `json:"is_pmm_internal_node,omitempty"` } // Validate validates this list nodes OK body nodes items0 diff --git a/api/management/v1/json/v1.json b/api/management/v1/json/v1.json index e59222a34a5..3addb73ba60 100644 --- a/api/management/v1/json/v1.json +++ b/api/management/v1/json/v1.json @@ -798,6 +798,11 @@ "description": "True if this node is a PMM Server node (HA mode).", "type": "boolean", "x-order": 18 + }, + "is_pmm_internal_node": { + "description": "True if this node belongs to the internal infrastructure of a PMM deployment\n(e.g. the HA persistence layer) and must not host user monitoring workloads.", + "type": "boolean", + "x-order": 19 } } }, @@ -1355,6 +1360,11 @@ "description": "True if this node is a PMM Server node (HA mode).", "type": "boolean", "x-order": 18 + }, + "is_pmm_internal_node": { + "description": "True if this node belongs to the internal infrastructure of a PMM deployment\n(e.g. the HA persistence layer) and must not host user monitoring workloads.", + "type": "boolean", + "x-order": 19 } }, "x-order": 0 @@ -7111,6 +7121,11 @@ "description": "Connection timeout for exporter (if set).", "type": "string", "x-order": 25 + }, + "pmm_agent_id": { + "description": "The pmm-agent identifier which should run agents. Defaults to the PMM Server's own pmm-agent.", + "type": "string", + "x-order": 26 } } } diff --git a/api/management/v1/node.pb.go b/api/management/v1/node.pb.go index 149fb30ad94..e620859ed53 100644 --- a/api/management/v1/node.pb.go +++ b/api/management/v1/node.pb.go @@ -619,8 +619,11 @@ type UniversalNode struct { InstanceId string `protobuf:"bytes,18,opt,name=instance_id,json=instanceId,proto3" json:"instance_id,omitempty"` // True if this node is a PMM Server node (HA mode). IsPmmServerNode bool `protobuf:"varint,19,opt,name=is_pmm_server_node,json=isPmmServerNode,proto3" json:"is_pmm_server_node,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // True if this node belongs to the internal infrastructure of a PMM deployment + // (e.g. the HA persistence layer) and must not host user monitoring workloads. + IsPmmInternalNode bool `protobuf:"varint,20,opt,name=is_pmm_internal_node,json=isPmmInternalNode,proto3" json:"is_pmm_internal_node,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *UniversalNode) Reset() { @@ -786,6 +789,13 @@ func (x *UniversalNode) GetIsPmmServerNode() bool { return false } +func (x *UniversalNode) GetIsPmmInternalNode() bool { + if x != nil { + return x.IsPmmInternalNode + } + return false +} + type ListNodesRequest struct { state protoimpl.MessageState `protogen:"open.v1"` // Node type to be filtered out. @@ -1159,7 +1169,7 @@ const file_management_v1_node_proto_rawDesc = "" + "\anode_id\x18\x01 \x01(\tB\a\xfaB\x04r\x02\x10\x01R\x06nodeId\x12\x14\n" + "\x05force\x18\x02 \x01(\bR\x05force\"2\n" + "\x16UnregisterNodeResponse\x12\x18\n" + - "\awarning\x18\x01 \x01(\tR\awarning\"\x9d\t\n" + + "\awarning\x18\x01 \x01(\tR\awarning\"\xce\t\n" + "\rUniversalNode\x12\x17\n" + "\anode_id\x18\x01 \x01(\tR\x06nodeId\x12\x1b\n" + "\tnode_type\x18\x02 \x01(\tR\bnodeType\x12\x1b\n" + @@ -1185,7 +1195,8 @@ const file_management_v1_node_proto_rawDesc = "" + "\bservices\x18\x11 \x03(\v2$.management.v1.UniversalNode.ServiceR\bservices\x12\x1f\n" + "\vinstance_id\x18\x12 \x01(\tR\n" + "instanceId\x12+\n" + - "\x12is_pmm_server_node\x18\x13 \x01(\bR\x0fisPmmServerNode\x1an\n" + + "\x12is_pmm_server_node\x18\x13 \x01(\bR\x0fisPmmServerNode\x12/\n" + + "\x14is_pmm_internal_node\x18\x14 \x01(\bR\x11isPmmInternalNode\x1an\n" + "\aService\x12\x1d\n" + "\n" + "service_id\x18\x01 \x01(\tR\tserviceId\x12!\n" + @@ -1255,7 +1266,6 @@ var ( (*timestamppb.Timestamp)(nil), // 21: google.protobuf.Timestamp } ) - var file_management_v1_node_proto_depIdxs = []int32{ 16, // 0: management.v1.AddNodeParams.node_type:type_name -> inventory.v1.NodeType 11, // 1: management.v1.AddNodeParams.custom_labels:type_name -> management.v1.AddNodeParams.CustomLabelsEntry diff --git a/api/management/v1/node.pb.validate.go b/api/management/v1/node.pb.validate.go index 34411b3f938..0e606deeec4 100644 --- a/api/management/v1/node.pb.validate.go +++ b/api/management/v1/node.pb.validate.go @@ -906,6 +906,8 @@ func (m *UniversalNode) validate(all bool) error { // no validation rules for IsPmmServerNode + // no validation rules for IsPmmInternalNode + if len(errors) > 0 { return UniversalNodeMultiError(errors) } diff --git a/api/management/v1/node.proto b/api/management/v1/node.proto index d5e26d3bc57..e4cdfa5c0b0 100644 --- a/api/management/v1/node.proto +++ b/api/management/v1/node.proto @@ -165,6 +165,9 @@ message UniversalNode { string instance_id = 18; // True if this node is a PMM Server node (HA mode). bool is_pmm_server_node = 19; + // True if this node belongs to the internal infrastructure of a PMM deployment + // (e.g. the HA persistence layer) and must not host user monitoring workloads. + bool is_pmm_internal_node = 20; } message ListNodesRequest { diff --git a/api/swagger/swagger-dev.json b/api/swagger/swagger-dev.json index d5c8fbc6c35..f60f3684439 100644 --- a/api/swagger/swagger-dev.json +++ b/api/swagger/swagger-dev.json @@ -22290,6 +22290,11 @@ "description": "True if this node is a PMM Server node (HA mode).", "type": "boolean", "x-order": 18 + }, + "is_pmm_internal_node": { + "description": "True if this node belongs to the internal infrastructure of a PMM deployment\n(e.g. the HA persistence layer) and must not host user monitoring workloads.", + "type": "boolean", + "x-order": 19 } } }, @@ -22847,6 +22852,11 @@ "description": "True if this node is a PMM Server node (HA mode).", "type": "boolean", "x-order": 18 + }, + "is_pmm_internal_node": { + "description": "True if this node belongs to the internal infrastructure of a PMM deployment\n(e.g. the HA persistence layer) and must not host user monitoring workloads.", + "type": "boolean", + "x-order": 19 } }, "x-order": 0 @@ -28603,6 +28613,11 @@ "description": "Connection timeout for exporter (if set).", "type": "string", "x-order": 25 + }, + "pmm_agent_id": { + "description": "The pmm-agent identifier which should run agents. Defaults to the PMM Server's own pmm-agent.", + "type": "string", + "x-order": 26 } } } diff --git a/api/swagger/swagger.json b/api/swagger/swagger.json index c31e2eb7b2e..45f2c498d8e 100644 --- a/api/swagger/swagger.json +++ b/api/swagger/swagger.json @@ -21317,6 +21317,11 @@ "description": "True if this node is a PMM Server node (HA mode).", "type": "boolean", "x-order": 18 + }, + "is_pmm_internal_node": { + "description": "True if this node belongs to the internal infrastructure of a PMM deployment\n(e.g. the HA persistence layer) and must not host user monitoring workloads.", + "type": "boolean", + "x-order": 19 } } }, @@ -21874,6 +21879,11 @@ "description": "True if this node is a PMM Server node (HA mode).", "type": "boolean", "x-order": 18 + }, + "is_pmm_internal_node": { + "description": "True if this node belongs to the internal infrastructure of a PMM deployment\n(e.g. the HA persistence layer) and must not host user monitoring workloads.", + "type": "boolean", + "x-order": 19 } }, "x-order": 0 @@ -27630,6 +27640,11 @@ "description": "Connection timeout for exporter (if set).", "type": "string", "x-order": 25 + }, + "pmm_agent_id": { + "description": "The pmm-agent identifier which should run agents. Defaults to the PMM Server's own pmm-agent.", + "type": "string", + "x-order": 26 } } } diff --git a/managed/cmd/pmm-managed/main.go b/managed/cmd/pmm-managed/main.go index 2468e9778a0..279e694ae15 100644 --- a/managed/cmd/pmm-managed/main.go +++ b/managed/cmd/pmm-managed/main.go @@ -234,6 +234,20 @@ type gRPCServerDeps struct { versionCache *versioncache.Service vmdb *victoriametrics.Service vmalert *vmalert.Service + internalNodePrefixes []string +} + +// parseNodeNamePrefixes splits a comma-separated list of Node name prefixes. +func parseNodeNamePrefixes(value string) []string { + var prefixes []string + for p := range strings.SplitSeq(value, ",") { + p = strings.TrimSpace(p) + if p != "" { + prefixes = append(prefixes, p) + } + } + + return prefixes } // runGRPCServer runs gRPC server until context is canceled, then gracefully stops it. @@ -304,6 +318,7 @@ func runGRPCServer(ctx context.Context, deps *gRPCServerDeps) { deps.db, deps.agentsRegistry, deps.agentsStateUpdater, deps.connectionCheck, deps.serviceInfoBroker, deps.vmdb, deps.versionCache, deps.grafanaClient, v1.NewAPI(*deps.vmClient), + deps.internalNodePrefixes, ) managementv1.RegisterManagementServiceServer(gRPCServer, managementSvc) @@ -743,6 +758,11 @@ func main() { //nolint:gocognit,maintidx,cyclop Default("9762"). Int() + internalNodePrefixesF := kingpin.Flag("internal-node-name-prefixes", + "Comma-separated list of Node name prefixes reserved for the internal infrastructure of this PMM deployment"). + Envar("PMM_INTERNAL_NODE_NAME_PREFIXES"). + String() + supervisordConfigDirF := kingpin.Flag("supervisord-config-dir", "Supervisord configuration directory").Required().String() logLevelF := kingpin.Flag("log-level", "Set logging level").Envar("PMM_LOG_LEVEL").Default("info").Enum("trace", "debug", "info", "warn", "error", "fatal") @@ -1196,6 +1216,7 @@ func main() { //nolint:gocognit,maintidx,cyclop grafanaClient: grafanaClient, handler: agentsHandler, ha: haService, + internalNodePrefixes: parseNodeNamePrefixes(*internalNodePrefixesF), jobsService: jobsService, minioClient: minioClient, pbmPITRService: pbmPITRService, diff --git a/managed/cmd/pmm-managed/main_test.go b/managed/cmd/pmm-managed/main_test.go index 17e536726f0..6b309ed1337 100644 --- a/managed/cmd/pmm-managed/main_test.go +++ b/managed/cmd/pmm-managed/main_test.go @@ -206,3 +206,17 @@ func formatPkgName(t *testing.T, name string) string { return name } + +func TestParseNodeNamePrefixes(t *testing.T) { + for _, tc := range []struct { + value string + expected []string + }{ + {value: "", expected: nil}, + {value: ",,", expected: nil}, + {value: "pmm-pmm-ha-pg-db-", expected: []string{"pmm-pmm-ha-pg-db-"}}, + {value: " pmm-pmm-ha-pg-db- , pmm-pmm-ha-ch- ", expected: []string{"pmm-pmm-ha-pg-db-", "pmm-pmm-ha-ch-"}}, + } { + assert.Equal(t, tc.expected, parseNodeNamePrefixes(tc.value), tc.value) + } +} diff --git a/managed/services/management/add_service_exporter_timeout_test.go b/managed/services/management/add_service_exporter_timeout_test.go index 73b5ada35b1..2138536f467 100644 --- a/managed/services/management/add_service_exporter_timeout_test.go +++ b/managed/services/management/add_service_exporter_timeout_test.go @@ -75,7 +75,7 @@ func TestAddServiceExporterTimeout(t *testing.T) { vmClient.AssertExpectations(t) }) - s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient) + s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient, nil) want := durationpb.New(17 * time.Second) t.Run("MySQL", func(t *testing.T) { diff --git a/managed/services/management/agent_test.go b/managed/services/management/agent_test.go index 676ccaf1b2f..622184b8211 100644 --- a/managed/services/management/agent_test.go +++ b/managed/services/management/agent_test.go @@ -97,7 +97,7 @@ func setup(t *testing.T) (context.Context, *ManagementService, func(t *testing.T vmClient.AssertExpectations(t) } - s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient) + s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient, nil) return ctx, s, teardown } diff --git a/managed/services/management/annotation_test.go b/managed/services/management/annotation_test.go index 88398ce1dbb..79f7dc227bb 100644 --- a/managed/services/management/annotation_test.go +++ b/managed/services/management/annotation_test.go @@ -66,7 +66,7 @@ func TestAnnotations(t *testing.T) { vmClient := &mockVictoriaMetricsClient{} vmClient.Test(t) - s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient) + s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient, nil) teardown := func(t *testing.T) { t.Helper() diff --git a/managed/services/management/azure_database.go b/managed/services/management/azure_database.go index 71c61928b4c..69909c21e8c 100644 --- a/managed/services/management/azure_database.go +++ b/managed/services/management/azure_database.go @@ -195,6 +195,16 @@ func (s *ManagementService) AddAzureDatabase(ctx context.Context, req *managemen } l := logger.Get(ctx).WithField("component", "discover/azureDatabase") + + pmmAgentID := models.PMMServerAgentID + if req.GetPmmAgentId() != "" { + pmmAgentID = req.GetPmmAgentId() + } + err := s.checkNodeIsEligible(ctx, pmmAgentID, req.Address) + if err != nil { + return nil, err + } + // tweak according to API docs if req.NodeName == "" { req.NodeName = req.InstanceId @@ -260,7 +270,7 @@ func (s *ManagementService) AddAzureDatabase(ctx context.Context, req *managemen if req.AzureDatabaseExporter { azureDatabaseExporter, err := models.CreateAgent(tx.Querier, models.AzureDatabaseExporterType, &models.CreateAgentParams{ - PMMAgentID: models.PMMServerAgentID, + PMMAgentID: pmmAgentID, ServiceID: service.ServiceID, AzureOptions: models.AzureOptionsFromRequest(req), }) @@ -271,7 +281,7 @@ func (s *ManagementService) AddAzureDatabase(ctx context.Context, req *managemen } metricsExporter, err := models.CreateAgent(tx.Querier, exporterType, &models.CreateAgentParams{ - PMMAgentID: models.PMMServerAgentID, + PMMAgentID: pmmAgentID, ServiceID: service.ServiceID, Username: req.Username, Password: req.Password, @@ -302,7 +312,7 @@ func (s *ManagementService) AddAzureDatabase(ctx context.Context, req *managemen if req.Qan { qanAgent, err := models.CreateAgent(tx.Querier, qanAgentType, &models.CreateAgentParams{ - PMMAgentID: models.PMMServerAgentID, + PMMAgentID: pmmAgentID, ServiceID: service.ServiceID, Username: req.Username, Password: req.Password, diff --git a/managed/services/management/internal_node_test.go b/managed/services/management/internal_node_test.go new file mode 100644 index 00000000000..35d5d93057f --- /dev/null +++ b/managed/services/management/internal_node_test.go @@ -0,0 +1,250 @@ +// 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 . + +package management + +import ( + "fmt" + "testing" + + "github.com/prometheus/common/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "gopkg.in/reform.v1" + "gopkg.in/reform.v1/dialects/postgresql" + + managementv1 "github.com/percona/pmm/api/management/v1" + "github.com/percona/pmm/managed/models" + "github.com/percona/pmm/managed/utils/testdb" + "github.com/percona/pmm/managed/utils/tests" + "github.com/percona/pmm/utils/logger" +) + +// internalNodePrefix mimics what the PMM HA Helm chart reports for its PostgreSQL cluster: +// the Nodes are named "-" by the PostgreSQL operator. +const internalNodePrefix = "pmm-pmm-ha-pg-db-" + +func TestIsInternalNode(t *testing.T) { + s := &ManagementService{internalNodePrefixes: []string{internalNodePrefix, "pmm-pmm-ha-ch-"}} + + for nodeName, expected := range map[string]bool{ + "pmm-pmm-ha-pg-db-instance1-qjjl-0": true, + "pmm-pmm-ha-ch-0": true, + "pmm-ha-0": false, + "pmm-server": false, + "": false, + } { + assert.Equal(t, expected, s.isInternalNode(nodeName), nodeName) + } + + t.Run("no prefixes configured", func(t *testing.T) { + s := &ManagementService{} + assert.False(t, s.isInternalNode(internalNodePrefix+"instance1-qjjl-0")) + }) +} + +func TestAddServiceTarget(t *testing.T) { + const ( + agentID = "00000000-0000-4000-8000-000000000005" + address = "mysql.example.com" + ) + + for _, tc := range []struct { + name string + req *managementv1.AddServiceRequest + expectedAgentID string + expectedAddress string + }{ + { + name: "MySQL", + req: &managementv1.AddServiceRequest{Service: &managementv1.AddServiceRequest_Mysql{ + Mysql: &managementv1.AddMySQLServiceParams{PmmAgentId: agentID, Address: address}, + }}, + expectedAgentID: agentID, + expectedAddress: address, + }, + { + name: "MongoDB", + req: &managementv1.AddServiceRequest{Service: &managementv1.AddServiceRequest_Mongodb{ + Mongodb: &managementv1.AddMongoDBServiceParams{PmmAgentId: agentID, Address: address}, + }}, + expectedAgentID: agentID, + expectedAddress: address, + }, + { + name: "PostgreSQL", + req: &managementv1.AddServiceRequest{Service: &managementv1.AddServiceRequest_Postgresql{ + Postgresql: &managementv1.AddPostgreSQLServiceParams{PmmAgentId: agentID, Address: address}, + }}, + expectedAgentID: agentID, + expectedAddress: address, + }, + { + name: "ProxySQL", + req: &managementv1.AddServiceRequest{Service: &managementv1.AddServiceRequest_Proxysql{ + Proxysql: &managementv1.AddProxySQLServiceParams{PmmAgentId: agentID, Address: address}, + }}, + expectedAgentID: agentID, + expectedAddress: address, + }, + { + name: "Valkey", + req: &managementv1.AddServiceRequest{Service: &managementv1.AddServiceRequest_Valkey{ + Valkey: &managementv1.AddValkeyServiceParams{PmmAgentId: agentID, Address: address}, + }}, + expectedAgentID: agentID, + expectedAddress: address, + }, + { + name: "RDS", + req: &managementv1.AddServiceRequest{Service: &managementv1.AddServiceRequest_Rds{ + Rds: &managementv1.AddRDSServiceParams{PmmAgentId: agentID, Address: address}, + }}, + expectedAgentID: agentID, + expectedAddress: address, + }, + { + name: "External Services are scraped on the Node they run on", + req: &managementv1.AddServiceRequest{Service: &managementv1.AddServiceRequest_External{ + External: &managementv1.AddExternalServiceParams{RunsOnNodeId: "00000000-0000-4000-8000-000000000006"}, + }}, + }, + { + name: "HAProxy Services are scraped on the Node they run on", + req: &managementv1.AddServiceRequest{Service: &managementv1.AddServiceRequest_Haproxy{ + Haproxy: &managementv1.AddHAProxyServiceParams{NodeId: "00000000-0000-4000-8000-000000000006"}, + }}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + pmmAgentID, address := addServiceTarget(tc.req) + assert.Equal(t, tc.expectedAgentID, pmmAgentID) + assert.Equal(t, tc.expectedAddress, address) + }) + } +} + +func TestListNodesMarksInternalNodes(t *testing.T) { + ctx := logger.Set(t.Context(), t.Name()) + + sqlDB := testdb.Open(t, models.SetupFixtures, nil) + t.Cleanup(func() { + require.NoError(t, sqlDB.Close()) + }) + db := reform.NewDB(sqlDB, postgresql.Dialect, reform.NewPrintfLogger(t.Logf)) + + node, err := models.CreateNode(db.Querier, models.GenericNodeType, &models.CreateNodeParams{ + NodeName: internalNodePrefix + "instance1-qjjl-0", + Address: "10.1.2.3", + }) + require.NoError(t, err) + + ar := &mockAgentsRegistry{} + ar.Test(t) + ar.On("IsConnected", mock.Anything).Return(false) + + vmdb := &mockPrometheusService{} + vmdb.Test(t) + + vmClient := &mockVictoriaMetricsClient{} + vmClient.Test(t) + vmClient.On("Query", ctx, mock.Anything, mock.Anything).Return(model.Vector{}, nil, nil) + + s := NewManagementService(db, ar, nil, nil, nil, vmdb, nil, nil, vmClient, []string{internalNodePrefix}) + + res, err := s.ListNodes(ctx, &managementv1.ListNodesRequest{}) + require.NoError(t, err) + + isInternal := make(map[string]bool, len(res.Nodes)) + for _, n := range res.Nodes { + isInternal[n.NodeName] = n.IsPmmInternalNode + } + assert.True(t, isInternal[node.NodeName], node.NodeName) + assert.False(t, isInternal["pmm-server"]) +} + +func TestCheckNodeIsEligible(t *testing.T) { + ctx := logger.Set(t.Context(), t.Name()) + + sqlDB := testdb.Open(t, models.SetupFixtures, nil) + t.Cleanup(func() { + require.NoError(t, sqlDB.Close()) + }) + db := reform.NewDB(sqlDB, postgresql.Dialect, reform.NewPrintfLogger(t.Logf)) + + node, err := models.CreateNode(db.Querier, models.GenericNodeType, &models.CreateNodeParams{ + NodeName: internalNodePrefix + "instance1-qjjl-0", + Address: "10.1.2.3", + }) + require.NoError(t, err) + agent, err := models.CreatePMMAgent(db.Querier, node.NodeID, nil) + require.NoError(t, err) + + s := NewManagementService(db, nil, nil, nil, nil, nil, nil, nil, nil, []string{internalNodePrefix}) + expectedErr := status.New(codes.FailedPrecondition, fmt.Sprintf( + "Node '%s' is a part of the internal infrastructure of this PMM deployment and cannot monitor other services.", node.NodeName)) + + t.Run("a remote address on an internal Node is rejected", func(t *testing.T) { + err := s.checkNodeIsEligible(ctx, agent.AgentID, "mysql.example.com") + tests.AssertGRPCError(t, expectedErr, err) + }) + + t.Run("local addresses on an internal Node are allowed", func(t *testing.T) { + for _, address := range []string{"", "localhost", "127.0.0.1", "::1"} { + assert.NoError(t, s.checkNodeIsEligible(ctx, agent.AgentID, address), address) + } + }) + + t.Run("a remote address on a regular Node is allowed", func(t *testing.T) { + assert.NoError(t, s.checkNodeIsEligible(ctx, models.PMMServerAgentID, "mysql.example.com")) + }) + + t.Run("no prefixes configured", func(t *testing.T) { + s := NewManagementService(db, nil, nil, nil, nil, nil, nil, nil, nil, nil) + assert.NoError(t, s.checkNodeIsEligible(ctx, agent.AgentID, "mysql.example.com")) + }) + + t.Run("AddService rejects an internal Node", func(t *testing.T) { + res, err := s.AddService(ctx, &managementv1.AddServiceRequest{Service: &managementv1.AddServiceRequest_Mysql{ + Mysql: &managementv1.AddMySQLServiceParams{ + PmmAgentId: agent.AgentID, + ServiceName: "test-mysql", + Address: "mysql.example.com", + Port: 3306, + }, + }}) + assert.Nil(t, res) + tests.AssertGRPCError(t, expectedErr, err) + }) + + t.Run("AddAzureDatabase rejects an internal Node", func(t *testing.T) { + _, err := models.UpdateSettings(sqlDB, &models.ChangeSettingsParams{ + EnableAzurediscover: new(true), + }) + require.NoError(t, err) + + res, err := s.AddAzureDatabase(ctx, &managementv1.AddAzureDatabaseRequest{ + PmmAgentId: agent.AgentID, + InstanceId: "test-azure", + Address: "test.mysql.database.azure.com", + Port: 3306, + }) + assert.Nil(t, res) + tests.AssertGRPCError(t, expectedErr, err) + }) +} diff --git a/managed/services/management/node.go b/managed/services/management/node.go index 5e8e13bc399..16597810788 100644 --- a/managed/services/management/node.go +++ b/managed/services/management/node.go @@ -316,22 +316,23 @@ func (s *ManagementService) ListNodes(ctx context.Context, req *managementv1.Lis } uNode := &managementv1.UniversalNode{ - Address: node.Address, - CustomLabels: labels, - NodeId: node.NodeID, - NodeName: node.NodeName, - NodeType: string(node.NodeType), - Az: node.AZ, - CreatedAt: timestamppb.New(node.CreatedAt), - ContainerId: pointer.GetString(node.ContainerID), - ContainerName: pointer.GetString(node.ContainerName), - Distro: node.Distro, - MachineId: pointer.GetString(node.MachineID), - NodeModel: node.NodeModel, - Region: pointer.GetString(node.Region), - UpdatedAt: timestamppb.New(node.UpdatedAt), - InstanceId: node.InstanceID, - IsPmmServerNode: node.IsPMMServerNode, + Address: node.Address, + CustomLabels: labels, + NodeId: node.NodeID, + NodeName: node.NodeName, + NodeType: string(node.NodeType), + Az: node.AZ, + CreatedAt: timestamppb.New(node.CreatedAt), + ContainerId: pointer.GetString(node.ContainerID), + ContainerName: pointer.GetString(node.ContainerName), + Distro: node.Distro, + MachineId: pointer.GetString(node.MachineID), + NodeModel: node.NodeModel, + Region: pointer.GetString(node.Region), + UpdatedAt: timestamppb.New(node.UpdatedAt), + InstanceId: node.InstanceID, + IsPmmServerNode: node.IsPMMServerNode, + IsPmmInternalNode: s.isInternalNode(node.NodeName), } freshUp, hasFresh := metrics[node.NodeID] @@ -397,21 +398,22 @@ func (s *ManagementService) GetNode(ctx context.Context, req *managementv1.GetNo } uNode := &managementv1.UniversalNode{ - Address: node.Address, - Az: node.AZ, - CreatedAt: timestamppb.New(node.CreatedAt), - ContainerId: pointer.GetString(node.ContainerID), - ContainerName: pointer.GetString(node.ContainerName), - CustomLabels: labels, - Distro: node.Distro, - MachineId: pointer.GetString(node.MachineID), - NodeId: node.NodeID, - NodeName: node.NodeName, - NodeType: string(node.NodeType), - NodeModel: node.NodeModel, - Region: pointer.GetString(node.Region), - UpdatedAt: timestamppb.New(node.UpdatedAt), - IsPmmServerNode: node.IsPMMServerNode, + Address: node.Address, + Az: node.AZ, + CreatedAt: timestamppb.New(node.CreatedAt), + ContainerId: pointer.GetString(node.ContainerID), + ContainerName: pointer.GetString(node.ContainerName), + CustomLabels: labels, + Distro: node.Distro, + MachineId: pointer.GetString(node.MachineID), + NodeId: node.NodeID, + NodeName: node.NodeName, + NodeType: string(node.NodeType), + NodeModel: node.NodeModel, + Region: pointer.GetString(node.Region), + UpdatedAt: timestamppb.New(node.UpdatedAt), + IsPmmServerNode: node.IsPMMServerNode, + IsPmmInternalNode: s.isInternalNode(node.NodeName), } freshUp, hasFresh := metrics[node.NodeID] diff --git a/managed/services/management/node_test.go b/managed/services/management/node_test.go index 9b83eecdc58..156f38fb944 100644 --- a/managed/services/management/node_test.go +++ b/managed/services/management/node_test.go @@ -88,7 +88,7 @@ func TestNodeService(t *testing.T) { vmClient.AssertExpectations(t) } - s := NewManagementService(db, r, state, nil, nil, vmdb, nil, authProvider, vmClient) + s := NewManagementService(db, r, state, nil, nil, vmdb, nil, authProvider, vmClient, nil) return ctx, s, teardown } @@ -272,7 +272,7 @@ func TestNodeService(t *testing.T) { grafanaClient := &mockGrafanaClient{} grafanaClient.Test(t) - s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient) + s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient, nil) teardown := func(t *testing.T) { t.Helper() @@ -549,7 +549,7 @@ func TestNodeService(t *testing.T) { vmClient := &mockVictoriaMetricsClient{} vmClient.Test(t) - s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient) + s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient, nil) teardown := func(t *testing.T) { t.Helper() diff --git a/managed/services/management/rds_test.go b/managed/services/management/rds_test.go index e92220e97fc..e7a9deca14d 100644 --- a/managed/services/management/rds_test.go +++ b/managed/services/management/rds_test.go @@ -81,7 +81,7 @@ func TestRDSService(t *testing.T) { vmClient.AssertExpectations(t) }() - s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient) + s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient, nil) t.Run("DiscoverRDS", func(t *testing.T) { t.Run("ListRegions", func(t *testing.T) { diff --git a/managed/services/management/service.go b/managed/services/management/service.go index c92ef207839..868aed0499e 100644 --- a/managed/services/management/service.go +++ b/managed/services/management/service.go @@ -49,6 +49,10 @@ type ManagementService struct { //nolint:revive grafanaClient grafanaClient vmClient victoriaMetricsClient l *logrus.Entry + + // internalNodePrefixes holds the Node name prefixes reserved for the internal + // infrastructure of this PMM deployment, e.g. its HA persistence layer. + internalNodePrefixes []string } // upMetricSelectors match the per-service-type "up" metrics that back the service status. @@ -80,21 +84,35 @@ func NewManagementService( vc versionCache, grafanaClient grafanaClient, vmClient victoriaMetricsClient, + internalNodePrefixes []string, ) *ManagementService { return &ManagementService{ - db: db, - r: r, - state: state, - cc: cc, - sib: sib, - vmdb: vmdb, - vc: vc, - grafanaClient: grafanaClient, - vmClient: vmClient, - l: logrus.WithField("service", "management"), + db: db, + r: r, + state: state, + cc: cc, + sib: sib, + vmdb: vmdb, + vc: vc, + grafanaClient: grafanaClient, + vmClient: vmClient, + l: logrus.WithField("service", "management"), + internalNodePrefixes: internalNodePrefixes, } } +// isInternalNode reports whether the Node belongs to the internal infrastructure of this +// PMM deployment and therefore must not host user monitoring workloads. +func (s *ManagementService) isInternalNode(nodeName string) bool { + for _, prefix := range s.internalNodePrefixes { + if strings.HasPrefix(nodeName, prefix) { + return true + } + } + + return false +} + // A map to check if the service is supported. // NOTE: known external services appear to match the vendor names, // (e.g. "mysql", "mongodb", "postgresql", "valkey", "proxysql", "haproxy"), @@ -108,8 +126,74 @@ var supportedServices = map[string]inventoryv1.ServiceType{ string(models.HAProxyServiceType): inventoryv1.ServiceType_SERVICE_TYPE_HAPROXY_SERVICE, } +// localAddresses resolve to the Node an Agent runs on. Monitoring them delegates no work to +// that Node beyond what it already does for itself. +var localAddresses = map[string]struct{}{"": {}, "localhost": {}, "127.0.0.1": {}, "::1": {}} + +// checkNodeIsEligible rejects requests which delegate monitoring of a remote address to an +// Agent running on a Node reserved for the internal infrastructure of this PMM deployment. +// Those Nodes still monitor the services inside their own pod, hence the address check. +func (s *ManagementService) checkNodeIsEligible(ctx context.Context, pmmAgentID, address string) error { + if len(s.internalNodePrefixes) == 0 || pmmAgentID == "" { + return nil + } + _, isLocal := localAddresses[address] + if isLocal { + return nil + } + + agent, err := models.FindAgentByID(s.db.WithContext(ctx), pmmAgentID) + if err != nil { + return err + } + nodeID := pointer.GetString(agent.RunsOnNodeID) + if nodeID == "" { + return nil + } + + node, err := models.FindNodeByID(s.db.WithContext(ctx), nodeID) + if err != nil { + return err + } + if !s.isInternalNode(node.NodeName) { + return nil + } + + return status.Errorf(codes.FailedPrecondition, + "Node '%s' is a part of the internal infrastructure of this PMM deployment and cannot monitor other services.", node.NodeName) +} + +// addServiceTarget returns the pmm-agent which is to run the Service's Agents together with the +// address it is to monitor, for the Service types which delegate monitoring to an existing Node. +func addServiceTarget(req *managementv1.AddServiceRequest) (string, string) { + switch req.Service.(type) { + case *managementv1.AddServiceRequest_Mysql: + return req.GetMysql().GetPmmAgentId(), req.GetMysql().GetAddress() + case *managementv1.AddServiceRequest_Mongodb: + return req.GetMongodb().GetPmmAgentId(), req.GetMongodb().GetAddress() + case *managementv1.AddServiceRequest_Postgresql: + return req.GetPostgresql().GetPmmAgentId(), req.GetPostgresql().GetAddress() + case *managementv1.AddServiceRequest_Proxysql: + return req.GetProxysql().GetPmmAgentId(), req.GetProxysql().GetAddress() + case *managementv1.AddServiceRequest_Valkey: + return req.GetValkey().GetPmmAgentId(), req.GetValkey().GetAddress() + case *managementv1.AddServiceRequest_Rds: + return req.GetRds().GetPmmAgentId(), req.GetRds().GetAddress() + default: + // External and HAProxy Services are scraped on the Node their exporter runs on, + // so they cannot offload work onto it. + return "", "" + } +} + // AddService add a Service and its Agents. func (s *ManagementService) AddService(ctx context.Context, req *managementv1.AddServiceRequest) (*managementv1.AddServiceResponse, error) { + pmmAgentID, address := addServiceTarget(req) + err := s.checkNodeIsEligible(ctx, pmmAgentID, address) + if err != nil { + return nil, err + } + switch req.Service.(type) { case *managementv1.AddServiceRequest_Mysql: return s.addMySQL(ctx, req.GetMysql()) diff --git a/managed/services/management/service_test.go b/managed/services/management/service_test.go index f1e93d63695..ddadd53dd91 100644 --- a/managed/services/management/service_test.go +++ b/managed/services/management/service_test.go @@ -89,7 +89,7 @@ func TestServiceService(t *testing.T) { vmClient.AssertExpectations(t) } - s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient) + s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient, nil) return ctx, s, teardown } @@ -325,7 +325,7 @@ func TestServiceService(t *testing.T) { vmClient.AssertExpectations(t) } - s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient) + s := NewManagementService(db, ar, state, cc, sib, vmdb, vc, grafanaClient, vmClient, nil) return ctx, s, teardown, vmdb } diff --git a/managed/utils/envvars/parser.go b/managed/utils/envvars/parser.go index fb78164b65a..89b75612df8 100644 --- a/managed/utils/envvars/parser.go +++ b/managed/utils/envvars/parser.go @@ -123,6 +123,9 @@ func ParseEnvVars(envs []string) (*models.ChangeSettingsParams, []error, []strin case "PERCONA_TELEMETRY_DISABLE": // skip the Pillars telemetry environment variable continue + case "PMM_INTERNAL_NODE_NAME_PREFIXES": + // skip the env variable that is already handled by kingpin + continue case "PMM_ENABLE_UPDATES": b, err := strconv.ParseBool(v) if err != nil { diff --git a/managed/utils/envvars/parser_test.go b/managed/utils/envvars/parser_test.go index 24211d072c7..09312e3f6a3 100644 --- a/managed/utils/envvars/parser_test.go +++ b/managed/utils/envvars/parser_test.go @@ -136,6 +136,18 @@ func TestEnvVarValidator(t *testing.T) { assert.Nil(t, gotWarns) }) + t.Run("Skipped internal node name prefixes env var", func(t *testing.T) { + t.Parallel() + + envs := []string{"PMM_INTERNAL_NODE_NAME_PREFIXES=pmm-pmm-ha-pg-db-"} + expectedEnvVars := &models.ChangeSettingsParams{} + + gotEnvVars, gotErrs, gotWarns := ParseEnvVars(envs) + assert.Equal(t, expectedEnvVars, gotEnvVars) + assert.Nil(t, gotErrs) + assert.Nil(t, gotWarns) + }) + t.Run("Invalid env variables values", func(t *testing.T) { t.Parallel() From 4dcae6314e48223e6504cdfbbd613efcff9358ba Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Tue, 28 Jul 2026 02:20:09 +0300 Subject: [PATCH 2/6] PMM-14665 Do not re-register an already registered pmm-agent `pmm-agent setup` registered the Node on every run, and a forced registration makes PMM Server drop the Node together with every Service on it. That is harmless for the database operators, whose sidecars re-add their own services on each container start, but it would silently delete user-added services from a long-lived PMM Client pod, which is what the pmm-ha Helm chart is about to pre-provision as monitoring delegates. Registration is now skipped when the Agent already holds an ID and is being set up against the same PMM Server it is registered with. `--force` still registers the Node again, and pointing an Agent at a different PMM Server registers it there as before. --- agent/commands/setup.go | 26 ++++++++- agent/commands/setup_test.go | 93 +++++++++++++++++++++++++++++++++ agent/config/config.go | 10 ++-- agent/config/config_test.go | 8 +-- agent/config/encryption_test.go | 8 +-- 5 files changed, 132 insertions(+), 13 deletions(-) create mode 100644 agent/commands/setup_test.go diff --git a/agent/commands/setup.go b/agent/commands/setup.go index d674398a497..e18fe914259 100644 --- a/agent/commands/setup.go +++ b/agent/commands/setup.go @@ -30,6 +30,27 @@ import ( mservice "github.com/percona/pmm/api/management/v1/json/client/management_service" ) +// skipRegistration reports whether `pmm-agent setup` can leave the Node registration as it is. +// An Agent which already holds an ID is registered with PMM Server, and registering it again makes +// the server drop the Node together with every Service on it. That is only done on demand, or when +// the Agent is being pointed at a different PMM Server, which does not know it yet. +func skipRegistration(cfg *config.Config, configFilepath string, l *logrus.Entry) bool { + if cfg.Setup.SkipRegistration { + return true + } + if cfg.ID == "" || cfg.Setup.Force { + return false + } + + fileCfg, err := config.LoadFromFile(configFilepath, &cfg.Encryption) + if err != nil { + l.Warnf("Failed to read the configuration file %s, registering the Node: %s", configFilepath, err) + return false + } + + return fileCfg.Server.Address == cfg.Server.Address +} + // Setup implements `pmm-agent setup` command. func Setup() { /* @@ -71,7 +92,10 @@ func Setup() { os.Exit(1) } - if !cfg.Setup.SkipRegistration { + if skipRegistration(cfg, configFilepath, l) { + fmt.Printf("Node is already registered with %s, pmm-agent ID is %s. Use --force to register it again.\n", + cfg.Server.Address, cfg.ID) + } else { register(cfg, l) } diff --git a/agent/commands/setup_test.go b/agent/commands/setup_test.go new file mode 100644 index 00000000000..028748ca036 --- /dev/null +++ b/agent/commands/setup_test.go @@ -0,0 +1,93 @@ +// 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 . + +package commands + +import ( + "os" + "path/filepath" + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/percona/pmm/agent/config" +) + +func TestSkipRegistration(t *testing.T) { + t.Parallel() + + const ( + agentID = "5a2b8a4b-2b9d-4a5f-9a11-2b6a3f6f9a11" + serverAddress = "pmm.example.com:443" + ) + + // writeConfigFile stores a configuration file holding the Agent ID and the PMM Server address. + writeConfigFile := func(t *testing.T, address string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), "pmm-agent.yaml") + err := config.SaveToFile(path, &config.Config{ID: agentID, Server: config.Server{Address: address}}, t.Name()) + require.NoError(t, err) + + return path + } + + t.Run("a new Agent registers", func(t *testing.T) { + t.Parallel() + + cfg := &config.Config{Server: config.Server{Address: serverAddress}} + assert.False(t, skipRegistration(cfg, writeConfigFile(t, serverAddress), logrus.WithField("test", t.Name()))) + }) + + t.Run("a registered Agent does not register again", func(t *testing.T) { + t.Parallel() + + cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}} + assert.True(t, skipRegistration(cfg, writeConfigFile(t, serverAddress), logrus.WithField("test", t.Name()))) + }) + + t.Run("a registered Agent registers with a different PMM Server", func(t *testing.T) { + t.Parallel() + + cfg := &config.Config{ID: agentID, Server: config.Server{Address: "new-pmm.example.com:443"}} + assert.False(t, skipRegistration(cfg, writeConfigFile(t, serverAddress), logrus.WithField("test", t.Name()))) + }) + + t.Run("a registered Agent registers again when forced", func(t *testing.T) { + t.Parallel() + + cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}, Setup: config.Setup{Force: true}} + assert.False(t, skipRegistration(cfg, writeConfigFile(t, serverAddress), logrus.WithField("test", t.Name()))) + }) + + t.Run("registration is skipped on demand", func(t *testing.T) { + t.Parallel() + + cfg := &config.Config{ID: agentID, Setup: config.Setup{SkipRegistration: true, Force: true}} + assert.True(t, skipRegistration(cfg, "not-exist.yaml", logrus.WithField("test", t.Name()))) + }) + + t.Run("a registered Agent registers when the configuration file cannot be read", func(t *testing.T) { + t.Parallel() + + path := writeConfigFile(t, serverAddress) + require.NoError(t, os.Remove(path)) + + cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}} + assert.False(t, skipRegistration(cfg, path, logrus.WithField("test", t.Name()))) + }) +} diff --git a/agent/config/config.go b/agent/config/config.go index 06d26e85a44..816e70db84c 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -339,7 +339,7 @@ func get(args []string, cfg *Config, l *logrus.Entry) (string, error) { //nolint return configFileF, err } l.Infof("Loading configuration file %s.", configFileF) - fileCfg, err := loadFromFile(configFileF, &cfg.Encryption) + fileCfg, err := LoadFromFile(configFileF, &cfg.Encryption) if err != nil { return configFileF, err } @@ -512,7 +512,8 @@ func Application(cfg *Config) (*kingpin.Application, *string) { setupCmd.Flag("az", "Node availability zone [PMM_AGENT_SETUP_AZ]"). Envar("PMM_AGENT_SETUP_AZ").StringVar(&cfg.Setup.Az) - setupCmd.Flag("force", "Remove Node with that name with all dependent Services and Agents if one exist [PMM_AGENT_SETUP_FORCE]"). + setupCmd.Flag("force", "Register the Node even if this pmm-agent is registered already, removing the Node with"+ + " that name together with all dependent Services and Agents if one exists [PMM_AGENT_SETUP_FORCE]"). Envar("PMM_AGENT_SETUP_FORCE").BoolVar(&cfg.Setup.Force) setupCmd.Flag("skip-registration", "Skip registration on PMM Server [PMM_AGENT_SETUP_SKIP_REGISTRATION]"). Envar("PMM_AGENT_SETUP_SKIP_REGISTRATION").BoolVar(&cfg.Setup.SkipRegistration) @@ -533,11 +534,12 @@ func Application(cfg *Config) (*kingpin.Application, *string) { return app, configFileF } -// loadFromFile loads configuration from file. +// LoadFromFile loads the configuration stored in the file at the given path, +// ignoring both command-line flags and environment variables. // As a special case, if file does not exist, it returns ConfigFileDoesNotExistError. // Other errors are returned if file exists, but configuration can't be loaded due to permission problems, // YAML parsing problems, etc. -func loadFromFile(path string, enc *Encryption) (*Config, error) { +func LoadFromFile(path string, enc *Encryption) (*Config, error) { _, err := os.Stat(path) if errors.Is(err, fs.ErrNotExist) { return nil, ConfigFileDoesNotExistError(path) diff --git a/agent/config/config_test.go b/agent/config/config_test.go index 48286677f34..3c04998f105 100644 --- a/agent/config/config_test.go +++ b/agent/config/config_test.go @@ -46,13 +46,13 @@ func TestLoadFromFile(t *testing.T) { t.Run("Normal", func(t *testing.T) { name := writeConfig(t, &Config{ID: "agent-id"}) - cfg, err := loadFromFile(name, nil) + cfg, err := LoadFromFile(name, nil) require.NoError(t, err) assert.Equal(t, &Config{ID: "agent-id"}, cfg) }) t.Run("NotExist", func(t *testing.T) { - cfg, err := loadFromFile("not-exist.yaml", nil) + cfg, err := LoadFromFile("not-exist.yaml", nil) assert.Equal(t, ConfigFileDoesNotExistError("not-exist.yaml"), err) assert.Nil(t, cfg) }) @@ -61,7 +61,7 @@ func TestLoadFromFile(t *testing.T) { name := writeConfig(t, &Config{ID: "agent-id"}) require.NoError(t, os.Chmod(name, 0o000)) - cfg, err := loadFromFile(name, nil) + cfg, err := LoadFromFile(name, nil) var targetErr *os.PathError require.ErrorAs(t, err, &targetErr) assert.Equal(t, "open", err.(*os.PathError).Op) //nolint:errorlint @@ -73,7 +73,7 @@ func TestLoadFromFile(t *testing.T) { name := writeConfig(t, nil) require.NoError(t, os.WriteFile(name, []byte(`not YAML`), 0o666)) //nolint:gosec - cfg, err := loadFromFile(name, nil) + cfg, err := LoadFromFile(name, nil) var targetErr *yaml.TypeError require.ErrorAs(t, err, &targetErr) require.EqualError(t, err, "yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `not YAML` into config.Config") diff --git a/agent/config/encryption_test.go b/agent/config/encryption_test.go index d229098a3a8..f84fba40d7e 100644 --- a/agent/config/encryption_test.go +++ b/agent/config/encryption_test.go @@ -77,7 +77,7 @@ func TestEncryption(t *testing.T) { KeyFile: key, } configfilef := writeConfig(t, &Config{ID: "agent-id", Encryption: enc}) - cfg, err := loadFromFile(configfilef, &enc) + cfg, err := LoadFromFile(configfilef, &enc) require.NoError(t, err) assert.Equal(t, &Config{ID: "agent-id"}, cfg) }) @@ -91,7 +91,7 @@ func TestEncryption(t *testing.T) { KeyFilePassword: password, } configfilef := writeConfig(t, &Config{ID: "agent-id", Encryption: enc}) - cfg, err := loadFromFile(configfilef, &enc) + cfg, err := LoadFromFile(configfilef, &enc) require.NoError(t, err) assert.Equal(t, &Config{ID: "agent-id"}, cfg) }) @@ -105,7 +105,7 @@ func TestEncryption(t *testing.T) { KeyFilePassword: password, }}) - cfg, err := loadFromFile(configfilef, &Encryption{ + cfg, err := LoadFromFile(configfilef, &Encryption{ KeyFile: key, KeyFilePassword: "hgfedcba", }) @@ -123,7 +123,7 @@ func TestEncryption(t *testing.T) { configfilef := writeConfig(t, &Config{ID: "agent-id", Encryption: Encryption{ KeyFile: key2, }}) - cfg, err := loadFromFile(configfilef, &Encryption{ + cfg, err := LoadFromFile(configfilef, &Encryption{ KeyFile: key1, KeyFilePassword: password, }) From 6cf3a3455cf53bc0ddc0c031bed55725f611a17a Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Tue, 28 Jul 2026 02:45:22 +0300 Subject: [PATCH 3/6] PMM-14665 Format the code --- api/management/v1/azure.pb.go | 1 + api/management/v1/node.pb.go | 1 + managed/services/management/internal_node_test.go | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/api/management/v1/azure.pb.go b/api/management/v1/azure.pb.go index cd0f5b9bd3a..0266389ebc3 100644 --- a/api/management/v1/azure.pb.go +++ b/api/management/v1/azure.pb.go @@ -738,6 +738,7 @@ var ( (*durationpb.Duration)(nil), // 7: google.protobuf.Duration } ) + var file_management_v1_azure_proto_depIdxs = []int32{ 0, // 0: management.v1.DiscoverAzureDatabaseInstance.type:type_name -> management.v1.DiscoverAzureDatabaseType 2, // 1: management.v1.DiscoverAzureDatabaseResponse.azure_database_instance:type_name -> management.v1.DiscoverAzureDatabaseInstance diff --git a/api/management/v1/node.pb.go b/api/management/v1/node.pb.go index e620859ed53..7402fee8c0b 100644 --- a/api/management/v1/node.pb.go +++ b/api/management/v1/node.pb.go @@ -1266,6 +1266,7 @@ var ( (*timestamppb.Timestamp)(nil), // 21: google.protobuf.Timestamp } ) + var file_management_v1_node_proto_depIdxs = []int32{ 16, // 0: management.v1.AddNodeParams.node_type:type_name -> inventory.v1.NodeType 11, // 1: management.v1.AddNodeParams.custom_labels:type_name -> management.v1.AddNodeParams.CustomLabelsEntry diff --git a/managed/services/management/internal_node_test.go b/managed/services/management/internal_node_test.go index 35d5d93057f..461ebfd6a58 100644 --- a/managed/services/management/internal_node_test.go +++ b/managed/services/management/internal_node_test.go @@ -197,7 +197,8 @@ func TestCheckNodeIsEligible(t *testing.T) { s := NewManagementService(db, nil, nil, nil, nil, nil, nil, nil, nil, []string{internalNodePrefix}) expectedErr := status.New(codes.FailedPrecondition, fmt.Sprintf( - "Node '%s' is a part of the internal infrastructure of this PMM deployment and cannot monitor other services.", node.NodeName)) + "Node '%s' is a part of the internal infrastructure of this PMM deployment and cannot monitor other services.", node.NodeName, + )) t.Run("a remote address on an internal Node is rejected", func(t *testing.T) { err := s.checkNodeIsEligible(ctx, agent.AgentID, "mysql.example.com") From 6c156f6b95740c815edc0d50a901c13739acf53e Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Tue, 28 Jul 2026 02:51:27 +0300 Subject: [PATCH 4/6] PMM-14665 Fix the license header --- agent/commands/setup_test.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/agent/commands/setup_test.go b/agent/commands/setup_test.go index 028748ca036..043f208bb54 100644 --- a/agent/commands/setup_test.go +++ b/agent/commands/setup_test.go @@ -1,17 +1,16 @@ // 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. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at // -// 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. +// http://www.apache.org/licenses/LICENSE-2.0 // -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. package commands From ee03acd287e942b1132505e2a146f6606a5adc70 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Tue, 28 Jul 2026 10:03:17 +0300 Subject: [PATCH 5/6] PMM-14665 Register the Node again when PMM Server does not know the Agent Skipping registration for an Agent which already holds an ID strands it whenever PMM Server no longer knows that ID, e.g. after the server was reinstalled or restored from a backup taken before the Agent was registered. pmm-agent then loops on "No Agent with ID" while the Node monitors nothing. `pmm-agent setup` now asks the server whether it still knows the Agent and registers the Node again when it does not. Credentials the server rejects count as an answer too: registering reports that with an actionable message, whereas skipping it would fail silently later on. An unreachable server is not an answer: a failed check keeps the registration, so an Agent still starts while PMM Server has no leader elected yet. --- agent/commands/clients.go | 32 +++++++++++++ agent/commands/clients_test.go | 82 ++++++++++++++++++++++++++++++++++ agent/commands/setup.go | 41 ++++++++++++++--- agent/commands/setup_test.go | 36 ++++++++++++--- agent/packages.dot | 1 + 5 files changed, 181 insertions(+), 11 deletions(-) create mode 100644 agent/commands/clients_test.go diff --git a/agent/commands/clients.go b/agent/commands/clients.go index bb935640553..d0a78ef214b 100644 --- a/agent/commands/clients.go +++ b/agent/commands/clients.go @@ -34,6 +34,8 @@ import ( "github.com/percona/pmm/agent/config" agentlocalClient "github.com/percona/pmm/api/agentlocal/v1/json/client" + inventoryClient "github.com/percona/pmm/api/inventory/v1/json/client" + aservice "github.com/percona/pmm/api/inventory/v1/json/client/agents_service" managementClient "github.com/percona/pmm/api/management/v1/json/client" mservice "github.com/percona/pmm/api/management/v1/json/client/management_service" "github.com/percona/pmm/utils/tlsconfig" @@ -135,6 +137,36 @@ func setServerTransport(u *url.URL, insecureTLS bool, l *logrus.Entry) { } managementClient.Default.SetTransport(transport) + inventoryClient.Default.SetTransport(transport) +} + +// serverKnowsAgent reports whether PMM Server has an Agent with the given ID. +// An error is returned when PMM Server could not be asked, so that the caller can tell +// "the Agent is gone" apart from "the answer is unknown". +// +// This method is not thread-safe. +func serverKnowsAgent(agentID string) (bool, error) { + _, err := inventoryClient.Default.AgentsService.GetAgent(&aservice.GetAgentParams{ + AgentID: agentID, + Context: context.Background(), + }) + if err == nil { + return true, nil + } + + e, ok := errors.AsType[*aservice.GetAgentDefault](err) + if !ok { + return false, err + } + + switch e.Code() { + // PMM Server either does not know this Agent, or does not accept its credentials. Registering + // again is the only way forward, and it reports a credentials problem with a clear message. + case http.StatusNotFound, http.StatusUnauthorized, http.StatusForbidden: + return false, nil + default: + return false, err + } } // ParseKeyValuePair parses --custom-labels flag value. diff --git a/agent/commands/clients_test.go b/agent/commands/clients_test.go new file mode 100644 index 00000000000..8027ea3b60e --- /dev/null +++ b/agent/commands/clients_test.go @@ -0,0 +1,82 @@ +// Copyright (C) 2023 Percona LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package commands + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The subtests share the package level API clients, so they cannot run in parallel. +func TestServerKnowsAgent(t *testing.T) { + const agentID = "5a2b8a4b-2b9d-4a5f-9a11-2b6a3f6f9a11" + + for _, tc := range []struct { + name string + statusCode int + known bool + unknowable bool + }{ + { + name: "PMM Server knows the Agent", + statusCode: http.StatusOK, + known: true, + }, + { + name: "PMM Server does not know the Agent", + statusCode: http.StatusNotFound, + }, + { + name: "PMM Server does not accept the credentials", + statusCode: http.StatusUnauthorized, + }, + { + name: "PMM Server forbids the request", + statusCode: http.StatusForbidden, + }, + { + name: "PMM Server cannot answer", + statusCode: http.StatusServiceUnavailable, + unknowable: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) { + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(tc.statusCode) + _, _ = rw.Write([]byte(`{"message": "` + tc.name + `"}`)) + })) + t.Cleanup(server.Close) + + u, err := url.Parse(server.URL) + require.NoError(t, err) + setServerTransport(u, true, logrus.WithField("test", t.Name())) + + known, err := serverKnowsAgent(agentID) + if tc.unknowable { + require.Error(t, err) + } else { + require.NoError(t, err) + } + assert.Equal(t, tc.known, known) + }) + } +} diff --git a/agent/commands/setup.go b/agent/commands/setup.go index e18fe914259..16848078d16 100644 --- a/agent/commands/setup.go +++ b/agent/commands/setup.go @@ -30,11 +30,39 @@ import ( mservice "github.com/percona/pmm/api/management/v1/json/client/management_service" ) +// registrationCheck reports whether PMM Server no longer knows the Agent described by cfg, +// so that the Node has to be registered again. +type registrationCheck func(cfg *config.Config, l *logrus.Entry) bool + +// mustRegisterOnServer reports whether PMM Server no longer knows this Agent. The server may have +// been reinstalled, or restored from a backup taken before the Agent was registered, leaving the +// Agent with an ID nothing recognizes. An unreachable server is not an answer: an Agent has to be +// able to start while PMM Server has no leader yet, so its registration is kept in that case. +func mustRegisterOnServer(cfg *config.Config, l *logrus.Entry) bool { + u := cfg.Server.URL() + if u == nil { + // register reports the missing server address with an actionable message + return true + } + setServerTransport(u, cfg.Server.InsecureTLS, l) + + known, err := serverKnowsAgent(cfg.ID) + if err != nil { + l.Warnf("Failed to check the registration of pmm-agent %s with %s, keeping it: %s", cfg.ID, cfg.Server.Address, err) + return false + } + if !known { + fmt.Printf("PMM Server at %s does not know pmm-agent %s, registering the Node again.\n", cfg.Server.Address, cfg.ID) + } + + return !known +} + // skipRegistration reports whether `pmm-agent setup` can leave the Node registration as it is. // An Agent which already holds an ID is registered with PMM Server, and registering it again makes -// the server drop the Node together with every Service on it. That is only done on demand, or when -// the Agent is being pointed at a different PMM Server, which does not know it yet. -func skipRegistration(cfg *config.Config, configFilepath string, l *logrus.Entry) bool { +// the server drop the Node together with every Service on it. That is only done on demand, when the +// Agent is being pointed at a different PMM Server, or when the server no longer knows the Agent. +func skipRegistration(cfg *config.Config, configFilepath string, mustRegister registrationCheck, l *logrus.Entry) bool { if cfg.Setup.SkipRegistration { return true } @@ -47,8 +75,11 @@ func skipRegistration(cfg *config.Config, configFilepath string, l *logrus.Entry l.Warnf("Failed to read the configuration file %s, registering the Node: %s", configFilepath, err) return false } + if fileCfg.Server.Address != cfg.Server.Address { + return false + } - return fileCfg.Server.Address == cfg.Server.Address + return !mustRegister(cfg, l) } // Setup implements `pmm-agent setup` command. @@ -92,7 +123,7 @@ func Setup() { os.Exit(1) } - if skipRegistration(cfg, configFilepath, l) { + if skipRegistration(cfg, configFilepath, mustRegisterOnServer, l) { fmt.Printf("Node is already registered with %s, pmm-agent ID is %s. Use --force to register it again.\n", cfg.Server.Address, cfg.ID) } else { diff --git a/agent/commands/setup_test.go b/agent/commands/setup_test.go index 043f208bb54..f1585311486 100644 --- a/agent/commands/setup_test.go +++ b/agent/commands/setup_test.go @@ -45,39 +45,63 @@ func TestSkipRegistration(t *testing.T) { return path } + // serverKnows answers as PMM Server which still has the Agent registered. + serverKnows := func(*config.Config, *logrus.Entry) bool { return false } + + // serverForgot answers as PMM Server which was reinstalled and knows nothing about the Agent. + serverForgot := func(*config.Config, *logrus.Entry) bool { return true } + + // serverNotAsked fails the test if PMM Server is asked at all. + serverNotAsked := func(*config.Config, *logrus.Entry) bool { + t.Errorf("PMM Server should not be asked about the registration") + return false + } + t.Run("a new Agent registers", func(t *testing.T) { t.Parallel() cfg := &config.Config{Server: config.Server{Address: serverAddress}} - assert.False(t, skipRegistration(cfg, writeConfigFile(t, serverAddress), logrus.WithField("test", t.Name()))) + path := writeConfigFile(t, serverAddress) + assert.False(t, skipRegistration(cfg, path, serverNotAsked, logrus.WithField("test", t.Name()))) }) t.Run("a registered Agent does not register again", func(t *testing.T) { t.Parallel() cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}} - assert.True(t, skipRegistration(cfg, writeConfigFile(t, serverAddress), logrus.WithField("test", t.Name()))) + path := writeConfigFile(t, serverAddress) + assert.True(t, skipRegistration(cfg, path, serverKnows, logrus.WithField("test", t.Name()))) + }) + + t.Run("a registered Agent registers again when PMM Server does not know it", func(t *testing.T) { + t.Parallel() + + cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}} + path := writeConfigFile(t, serverAddress) + assert.False(t, skipRegistration(cfg, path, serverForgot, logrus.WithField("test", t.Name()))) }) t.Run("a registered Agent registers with a different PMM Server", func(t *testing.T) { t.Parallel() cfg := &config.Config{ID: agentID, Server: config.Server{Address: "new-pmm.example.com:443"}} - assert.False(t, skipRegistration(cfg, writeConfigFile(t, serverAddress), logrus.WithField("test", t.Name()))) + path := writeConfigFile(t, serverAddress) + assert.False(t, skipRegistration(cfg, path, serverNotAsked, logrus.WithField("test", t.Name()))) }) t.Run("a registered Agent registers again when forced", func(t *testing.T) { t.Parallel() cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}, Setup: config.Setup{Force: true}} - assert.False(t, skipRegistration(cfg, writeConfigFile(t, serverAddress), logrus.WithField("test", t.Name()))) + path := writeConfigFile(t, serverAddress) + assert.False(t, skipRegistration(cfg, path, serverNotAsked, logrus.WithField("test", t.Name()))) }) t.Run("registration is skipped on demand", func(t *testing.T) { t.Parallel() cfg := &config.Config{ID: agentID, Setup: config.Setup{SkipRegistration: true, Force: true}} - assert.True(t, skipRegistration(cfg, "not-exist.yaml", logrus.WithField("test", t.Name()))) + assert.True(t, skipRegistration(cfg, "not-exist.yaml", serverNotAsked, logrus.WithField("test", t.Name()))) }) t.Run("a registered Agent registers when the configuration file cannot be read", func(t *testing.T) { @@ -87,6 +111,6 @@ func TestSkipRegistration(t *testing.T) { require.NoError(t, os.Remove(path)) cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}} - assert.False(t, skipRegistration(cfg, path, logrus.WithField("test", t.Name()))) + assert.False(t, skipRegistration(cfg, path, serverNotAsked, logrus.WithField("test", t.Name()))) }) } diff --git a/agent/packages.dot b/agent/packages.dot index d24bb7f4b8f..79e551962f9 100644 --- a/agent/packages.dot +++ b/agent/packages.dot @@ -56,6 +56,7 @@ digraph packages { "/commands" -> "/serviceinfobroker"; "/commands" -> "/tailog"; "/commands" -> "/versioner"; + "/commands.test" -> "/commands"; "/connectionchecker" -> "/config"; "/connectionchecker" -> "/tlshelpers"; "/connectionchecker.test" -> "/connectionchecker"; From 9af8db5b557ecc6d9073226ff88e90953d8042f5 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Tue, 28 Jul 2026 11:04:51 +0300 Subject: [PATCH 6/6] PMM-14665 Revert the pmm-agent registration changes Keeping this ticket to what it is about: reporting the Nodes of a PMM deployment's own infrastructure as internal and refusing to delegate monitoring to them. Making `pmm-agent setup` idempotent is what would let a pre-provisioned PMM Client pod register once and keep its identity across restarts, but the chart can do that with a one-time init container instead. The agent side is worth its own effort: it changes pmm-agent CLI behaviour, it needs a release note, and the surrounding questions - how an Agent recovers when PMM Server no longer knows it, and how an orphaned Node is reclaimed - deserve their own design and review. --- agent/commands/clients.go | 32 --------- agent/commands/clients_test.go | 82 ---------------------- agent/commands/setup.go | 57 +--------------- agent/commands/setup_test.go | 116 -------------------------------- agent/config/config.go | 10 ++- agent/config/config_test.go | 8 +-- agent/config/encryption_test.go | 8 +-- agent/packages.dot | 1 - 8 files changed, 13 insertions(+), 301 deletions(-) delete mode 100644 agent/commands/clients_test.go delete mode 100644 agent/commands/setup_test.go diff --git a/agent/commands/clients.go b/agent/commands/clients.go index d0a78ef214b..bb935640553 100644 --- a/agent/commands/clients.go +++ b/agent/commands/clients.go @@ -34,8 +34,6 @@ import ( "github.com/percona/pmm/agent/config" agentlocalClient "github.com/percona/pmm/api/agentlocal/v1/json/client" - inventoryClient "github.com/percona/pmm/api/inventory/v1/json/client" - aservice "github.com/percona/pmm/api/inventory/v1/json/client/agents_service" managementClient "github.com/percona/pmm/api/management/v1/json/client" mservice "github.com/percona/pmm/api/management/v1/json/client/management_service" "github.com/percona/pmm/utils/tlsconfig" @@ -137,36 +135,6 @@ func setServerTransport(u *url.URL, insecureTLS bool, l *logrus.Entry) { } managementClient.Default.SetTransport(transport) - inventoryClient.Default.SetTransport(transport) -} - -// serverKnowsAgent reports whether PMM Server has an Agent with the given ID. -// An error is returned when PMM Server could not be asked, so that the caller can tell -// "the Agent is gone" apart from "the answer is unknown". -// -// This method is not thread-safe. -func serverKnowsAgent(agentID string) (bool, error) { - _, err := inventoryClient.Default.AgentsService.GetAgent(&aservice.GetAgentParams{ - AgentID: agentID, - Context: context.Background(), - }) - if err == nil { - return true, nil - } - - e, ok := errors.AsType[*aservice.GetAgentDefault](err) - if !ok { - return false, err - } - - switch e.Code() { - // PMM Server either does not know this Agent, or does not accept its credentials. Registering - // again is the only way forward, and it reports a credentials problem with a clear message. - case http.StatusNotFound, http.StatusUnauthorized, http.StatusForbidden: - return false, nil - default: - return false, err - } } // ParseKeyValuePair parses --custom-labels flag value. diff --git a/agent/commands/clients_test.go b/agent/commands/clients_test.go deleted file mode 100644 index 8027ea3b60e..00000000000 --- a/agent/commands/clients_test.go +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright (C) 2023 Percona LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package commands - -import ( - "net/http" - "net/http/httptest" - "net/url" - "testing" - - "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// The subtests share the package level API clients, so they cannot run in parallel. -func TestServerKnowsAgent(t *testing.T) { - const agentID = "5a2b8a4b-2b9d-4a5f-9a11-2b6a3f6f9a11" - - for _, tc := range []struct { - name string - statusCode int - known bool - unknowable bool - }{ - { - name: "PMM Server knows the Agent", - statusCode: http.StatusOK, - known: true, - }, - { - name: "PMM Server does not know the Agent", - statusCode: http.StatusNotFound, - }, - { - name: "PMM Server does not accept the credentials", - statusCode: http.StatusUnauthorized, - }, - { - name: "PMM Server forbids the request", - statusCode: http.StatusForbidden, - }, - { - name: "PMM Server cannot answer", - statusCode: http.StatusServiceUnavailable, - unknowable: true, - }, - } { - t.Run(tc.name, func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) { - rw.Header().Set("Content-Type", "application/json") - rw.WriteHeader(tc.statusCode) - _, _ = rw.Write([]byte(`{"message": "` + tc.name + `"}`)) - })) - t.Cleanup(server.Close) - - u, err := url.Parse(server.URL) - require.NoError(t, err) - setServerTransport(u, true, logrus.WithField("test", t.Name())) - - known, err := serverKnowsAgent(agentID) - if tc.unknowable { - require.Error(t, err) - } else { - require.NoError(t, err) - } - assert.Equal(t, tc.known, known) - }) - } -} diff --git a/agent/commands/setup.go b/agent/commands/setup.go index 16848078d16..d674398a497 100644 --- a/agent/commands/setup.go +++ b/agent/commands/setup.go @@ -30,58 +30,6 @@ import ( mservice "github.com/percona/pmm/api/management/v1/json/client/management_service" ) -// registrationCheck reports whether PMM Server no longer knows the Agent described by cfg, -// so that the Node has to be registered again. -type registrationCheck func(cfg *config.Config, l *logrus.Entry) bool - -// mustRegisterOnServer reports whether PMM Server no longer knows this Agent. The server may have -// been reinstalled, or restored from a backup taken before the Agent was registered, leaving the -// Agent with an ID nothing recognizes. An unreachable server is not an answer: an Agent has to be -// able to start while PMM Server has no leader yet, so its registration is kept in that case. -func mustRegisterOnServer(cfg *config.Config, l *logrus.Entry) bool { - u := cfg.Server.URL() - if u == nil { - // register reports the missing server address with an actionable message - return true - } - setServerTransport(u, cfg.Server.InsecureTLS, l) - - known, err := serverKnowsAgent(cfg.ID) - if err != nil { - l.Warnf("Failed to check the registration of pmm-agent %s with %s, keeping it: %s", cfg.ID, cfg.Server.Address, err) - return false - } - if !known { - fmt.Printf("PMM Server at %s does not know pmm-agent %s, registering the Node again.\n", cfg.Server.Address, cfg.ID) - } - - return !known -} - -// skipRegistration reports whether `pmm-agent setup` can leave the Node registration as it is. -// An Agent which already holds an ID is registered with PMM Server, and registering it again makes -// the server drop the Node together with every Service on it. That is only done on demand, when the -// Agent is being pointed at a different PMM Server, or when the server no longer knows the Agent. -func skipRegistration(cfg *config.Config, configFilepath string, mustRegister registrationCheck, l *logrus.Entry) bool { - if cfg.Setup.SkipRegistration { - return true - } - if cfg.ID == "" || cfg.Setup.Force { - return false - } - - fileCfg, err := config.LoadFromFile(configFilepath, &cfg.Encryption) - if err != nil { - l.Warnf("Failed to read the configuration file %s, registering the Node: %s", configFilepath, err) - return false - } - if fileCfg.Server.Address != cfg.Server.Address { - return false - } - - return !mustRegister(cfg, l) -} - // Setup implements `pmm-agent setup` command. func Setup() { /* @@ -123,10 +71,7 @@ func Setup() { os.Exit(1) } - if skipRegistration(cfg, configFilepath, mustRegisterOnServer, l) { - fmt.Printf("Node is already registered with %s, pmm-agent ID is %s. Use --force to register it again.\n", - cfg.Server.Address, cfg.ID) - } else { + if !cfg.Setup.SkipRegistration { register(cfg, l) } diff --git a/agent/commands/setup_test.go b/agent/commands/setup_test.go deleted file mode 100644 index f1585311486..00000000000 --- a/agent/commands/setup_test.go +++ /dev/null @@ -1,116 +0,0 @@ -// Copyright (C) 2023 Percona LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package commands - -import ( - "os" - "path/filepath" - "testing" - - "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/percona/pmm/agent/config" -) - -func TestSkipRegistration(t *testing.T) { - t.Parallel() - - const ( - agentID = "5a2b8a4b-2b9d-4a5f-9a11-2b6a3f6f9a11" - serverAddress = "pmm.example.com:443" - ) - - // writeConfigFile stores a configuration file holding the Agent ID and the PMM Server address. - writeConfigFile := func(t *testing.T, address string) string { - t.Helper() - - path := filepath.Join(t.TempDir(), "pmm-agent.yaml") - err := config.SaveToFile(path, &config.Config{ID: agentID, Server: config.Server{Address: address}}, t.Name()) - require.NoError(t, err) - - return path - } - - // serverKnows answers as PMM Server which still has the Agent registered. - serverKnows := func(*config.Config, *logrus.Entry) bool { return false } - - // serverForgot answers as PMM Server which was reinstalled and knows nothing about the Agent. - serverForgot := func(*config.Config, *logrus.Entry) bool { return true } - - // serverNotAsked fails the test if PMM Server is asked at all. - serverNotAsked := func(*config.Config, *logrus.Entry) bool { - t.Errorf("PMM Server should not be asked about the registration") - return false - } - - t.Run("a new Agent registers", func(t *testing.T) { - t.Parallel() - - cfg := &config.Config{Server: config.Server{Address: serverAddress}} - path := writeConfigFile(t, serverAddress) - assert.False(t, skipRegistration(cfg, path, serverNotAsked, logrus.WithField("test", t.Name()))) - }) - - t.Run("a registered Agent does not register again", func(t *testing.T) { - t.Parallel() - - cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}} - path := writeConfigFile(t, serverAddress) - assert.True(t, skipRegistration(cfg, path, serverKnows, logrus.WithField("test", t.Name()))) - }) - - t.Run("a registered Agent registers again when PMM Server does not know it", func(t *testing.T) { - t.Parallel() - - cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}} - path := writeConfigFile(t, serverAddress) - assert.False(t, skipRegistration(cfg, path, serverForgot, logrus.WithField("test", t.Name()))) - }) - - t.Run("a registered Agent registers with a different PMM Server", func(t *testing.T) { - t.Parallel() - - cfg := &config.Config{ID: agentID, Server: config.Server{Address: "new-pmm.example.com:443"}} - path := writeConfigFile(t, serverAddress) - assert.False(t, skipRegistration(cfg, path, serverNotAsked, logrus.WithField("test", t.Name()))) - }) - - t.Run("a registered Agent registers again when forced", func(t *testing.T) { - t.Parallel() - - cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}, Setup: config.Setup{Force: true}} - path := writeConfigFile(t, serverAddress) - assert.False(t, skipRegistration(cfg, path, serverNotAsked, logrus.WithField("test", t.Name()))) - }) - - t.Run("registration is skipped on demand", func(t *testing.T) { - t.Parallel() - - cfg := &config.Config{ID: agentID, Setup: config.Setup{SkipRegistration: true, Force: true}} - assert.True(t, skipRegistration(cfg, "not-exist.yaml", serverNotAsked, logrus.WithField("test", t.Name()))) - }) - - t.Run("a registered Agent registers when the configuration file cannot be read", func(t *testing.T) { - t.Parallel() - - path := writeConfigFile(t, serverAddress) - require.NoError(t, os.Remove(path)) - - cfg := &config.Config{ID: agentID, Server: config.Server{Address: serverAddress}} - assert.False(t, skipRegistration(cfg, path, serverNotAsked, logrus.WithField("test", t.Name()))) - }) -} diff --git a/agent/config/config.go b/agent/config/config.go index 816e70db84c..06d26e85a44 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -339,7 +339,7 @@ func get(args []string, cfg *Config, l *logrus.Entry) (string, error) { //nolint return configFileF, err } l.Infof("Loading configuration file %s.", configFileF) - fileCfg, err := LoadFromFile(configFileF, &cfg.Encryption) + fileCfg, err := loadFromFile(configFileF, &cfg.Encryption) if err != nil { return configFileF, err } @@ -512,8 +512,7 @@ func Application(cfg *Config) (*kingpin.Application, *string) { setupCmd.Flag("az", "Node availability zone [PMM_AGENT_SETUP_AZ]"). Envar("PMM_AGENT_SETUP_AZ").StringVar(&cfg.Setup.Az) - setupCmd.Flag("force", "Register the Node even if this pmm-agent is registered already, removing the Node with"+ - " that name together with all dependent Services and Agents if one exists [PMM_AGENT_SETUP_FORCE]"). + setupCmd.Flag("force", "Remove Node with that name with all dependent Services and Agents if one exist [PMM_AGENT_SETUP_FORCE]"). Envar("PMM_AGENT_SETUP_FORCE").BoolVar(&cfg.Setup.Force) setupCmd.Flag("skip-registration", "Skip registration on PMM Server [PMM_AGENT_SETUP_SKIP_REGISTRATION]"). Envar("PMM_AGENT_SETUP_SKIP_REGISTRATION").BoolVar(&cfg.Setup.SkipRegistration) @@ -534,12 +533,11 @@ func Application(cfg *Config) (*kingpin.Application, *string) { return app, configFileF } -// LoadFromFile loads the configuration stored in the file at the given path, -// ignoring both command-line flags and environment variables. +// loadFromFile loads configuration from file. // As a special case, if file does not exist, it returns ConfigFileDoesNotExistError. // Other errors are returned if file exists, but configuration can't be loaded due to permission problems, // YAML parsing problems, etc. -func LoadFromFile(path string, enc *Encryption) (*Config, error) { +func loadFromFile(path string, enc *Encryption) (*Config, error) { _, err := os.Stat(path) if errors.Is(err, fs.ErrNotExist) { return nil, ConfigFileDoesNotExistError(path) diff --git a/agent/config/config_test.go b/agent/config/config_test.go index 3c04998f105..48286677f34 100644 --- a/agent/config/config_test.go +++ b/agent/config/config_test.go @@ -46,13 +46,13 @@ func TestLoadFromFile(t *testing.T) { t.Run("Normal", func(t *testing.T) { name := writeConfig(t, &Config{ID: "agent-id"}) - cfg, err := LoadFromFile(name, nil) + cfg, err := loadFromFile(name, nil) require.NoError(t, err) assert.Equal(t, &Config{ID: "agent-id"}, cfg) }) t.Run("NotExist", func(t *testing.T) { - cfg, err := LoadFromFile("not-exist.yaml", nil) + cfg, err := loadFromFile("not-exist.yaml", nil) assert.Equal(t, ConfigFileDoesNotExistError("not-exist.yaml"), err) assert.Nil(t, cfg) }) @@ -61,7 +61,7 @@ func TestLoadFromFile(t *testing.T) { name := writeConfig(t, &Config{ID: "agent-id"}) require.NoError(t, os.Chmod(name, 0o000)) - cfg, err := LoadFromFile(name, nil) + cfg, err := loadFromFile(name, nil) var targetErr *os.PathError require.ErrorAs(t, err, &targetErr) assert.Equal(t, "open", err.(*os.PathError).Op) //nolint:errorlint @@ -73,7 +73,7 @@ func TestLoadFromFile(t *testing.T) { name := writeConfig(t, nil) require.NoError(t, os.WriteFile(name, []byte(`not YAML`), 0o666)) //nolint:gosec - cfg, err := LoadFromFile(name, nil) + cfg, err := loadFromFile(name, nil) var targetErr *yaml.TypeError require.ErrorAs(t, err, &targetErr) require.EqualError(t, err, "yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `not YAML` into config.Config") diff --git a/agent/config/encryption_test.go b/agent/config/encryption_test.go index f84fba40d7e..d229098a3a8 100644 --- a/agent/config/encryption_test.go +++ b/agent/config/encryption_test.go @@ -77,7 +77,7 @@ func TestEncryption(t *testing.T) { KeyFile: key, } configfilef := writeConfig(t, &Config{ID: "agent-id", Encryption: enc}) - cfg, err := LoadFromFile(configfilef, &enc) + cfg, err := loadFromFile(configfilef, &enc) require.NoError(t, err) assert.Equal(t, &Config{ID: "agent-id"}, cfg) }) @@ -91,7 +91,7 @@ func TestEncryption(t *testing.T) { KeyFilePassword: password, } configfilef := writeConfig(t, &Config{ID: "agent-id", Encryption: enc}) - cfg, err := LoadFromFile(configfilef, &enc) + cfg, err := loadFromFile(configfilef, &enc) require.NoError(t, err) assert.Equal(t, &Config{ID: "agent-id"}, cfg) }) @@ -105,7 +105,7 @@ func TestEncryption(t *testing.T) { KeyFilePassword: password, }}) - cfg, err := LoadFromFile(configfilef, &Encryption{ + cfg, err := loadFromFile(configfilef, &Encryption{ KeyFile: key, KeyFilePassword: "hgfedcba", }) @@ -123,7 +123,7 @@ func TestEncryption(t *testing.T) { configfilef := writeConfig(t, &Config{ID: "agent-id", Encryption: Encryption{ KeyFile: key2, }}) - cfg, err := LoadFromFile(configfilef, &Encryption{ + cfg, err := loadFromFile(configfilef, &Encryption{ KeyFile: key1, KeyFilePassword: password, }) diff --git a/agent/packages.dot b/agent/packages.dot index 79e551962f9..d24bb7f4b8f 100644 --- a/agent/packages.dot +++ b/agent/packages.dot @@ -56,7 +56,6 @@ digraph packages { "/commands" -> "/serviceinfobroker"; "/commands" -> "/tailog"; "/commands" -> "/versioner"; - "/commands.test" -> "/commands"; "/connectionchecker" -> "/config"; "/connectionchecker" -> "/tlshelpers"; "/connectionchecker.test" -> "/connectionchecker";