From 9cdd18dfb093edd2bd8cb07f820f42ce7891666e Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Tue, 28 Jul 2026 01:21:11 +0300 Subject: [PATCH 1/3] PMM-14665 Skip nodes internal to a PMM deployment when adding a service Filter the Nodes which PMM Server reports as internal (is_pmm_internal_node) out of the Nodes dropdown: they are dedicated to PMM's own infrastructure, such as the persistence layer of an HA deployment. Preselect a Node other than a PMM Server one in an HA deployment, where the PMM Server pods are meant to stay free of monitoring workloads. A single-node deployment keeps preselecting pmm-server as before. --- .../NodesAgents/NodesAgents.test.tsx | 24 ++++++++++++ .../FormParts/NodesAgents/NodesAgents.tsx | 14 ++++--- .../app/percona/inventory/Inventory.types.ts | 1 + .../inventory/__mocks__/Inventory.service.ts | 39 +++++++++++++++++++ .../shared/core/reducers/nodes/nodes.utils.ts | 3 ++ 5 files changed, 76 insertions(+), 5 deletions(-) diff --git a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx index a7a800fbb4a87..291f89cca915c 100644 --- a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx +++ b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx @@ -7,6 +7,7 @@ import { InventoryService } from 'app/percona/inventory/Inventory.service'; import { nodesMockMultipleAgentsNoPMMServer, nodesMock, + nodesMockHA, nodesMockOneAgentNoPMMServer, } from 'app/percona/inventory/__mocks__/Inventory.service'; import * as NodesReducer from 'app/percona/shared/core/reducers/nodes/nodes'; @@ -55,6 +56,29 @@ describe('Nodes Agents:: ', () => { await waitFor(() => expect(screen.getByTestId('node')).toHaveTextContent(nodesMock[0].node_id)); }); + it('should not offer nodes internal to the PMM deployment', async () => { + jest.spyOn(InventoryService, 'getNodes').mockReturnValue(Promise.resolve({ nodes: nodesMockHA })); + + setup(); + + await waitFor(() => expect(fetchNodesActionActionSpy).toHaveBeenCalled()); + + selectEvent.openMenu(screen.getByLabelText('Nodes')); + + expect(screen.queryByText('pmm-pmm-ha-pg-db-instance1-qjjl-0')).not.toBeInTheDocument(); + expect(screen.getByText('pmm-ha-0')).toBeInTheDocument(); + }); + + it('should prefer a node other than a PMM Server one in an HA deployment', async () => { + jest.spyOn(InventoryService, 'getNodes').mockReturnValue(Promise.resolve({ nodes: nodesMockHA })); + + setup(); + + await waitFor(() => expect(fetchNodesActionActionSpy).toHaveBeenCalled()); + + await waitFor(() => expect(screen.getByTestId('node')).toHaveTextContent('external-client-id')); + }); + it('should not pick any agent when the selected node is not pmm-server', async () => { jest .spyOn(InventoryService, 'getNodes') diff --git a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.tsx b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.tsx index f537ae22dafee..6137a21af1856 100644 --- a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.tsx +++ b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.tsx @@ -76,13 +76,17 @@ export const NodesAgents: FC = ({ form }) => { if (nodesOptions.length === 0) { loadData(); } else if (!selectedNode) { - // preselect pmm-server node - const pmmServerNode = - nodesOptions.find((node) => node.value === PMM_SERVER_NODE_ID) || + // A deployment with more than one PMM Server node runs in HA mode, where the PMM Server + // pods are meant to stay free of monitoring workloads. Prefer any other node there, + // otherwise preselect the pmm-server node as usual. + const isHighlyAvailable = nodesOptions.filter((node) => node.isPMMServerNode).length > 1; + const preselectedNode = + (isHighlyAvailable ? nodesOptions.find((node) => !node.isPMMServerNode) : undefined) ?? + nodesOptions.find((node) => node.value === PMM_SERVER_NODE_ID) ?? nodesOptions.find((node) => node.isPMMServerNode); - if (pmmServerNode) { - setNodeAndAgent(pmmServerNode); + if (preselectedNode) { + setNodeAndAgent(preselectedNode); } } // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/public/app/percona/inventory/Inventory.types.ts b/public/app/percona/inventory/Inventory.types.ts index ed155c4f084e8..8d973cf59358c 100644 --- a/public/app/percona/inventory/Inventory.types.ts +++ b/public/app/percona/inventory/Inventory.types.ts @@ -160,6 +160,7 @@ export interface NodeDB { status: ServiceStatus; services?: ServiceNodeListDB[]; is_pmm_server_node: boolean; + is_pmm_internal_node?: boolean; } export interface NodeListDBPayload { diff --git a/public/app/percona/inventory/__mocks__/Inventory.service.ts b/public/app/percona/inventory/__mocks__/Inventory.service.ts index 51a8f5f1ad4fb..ca50bb045efee 100644 --- a/public/app/percona/inventory/__mocks__/Inventory.service.ts +++ b/public/app/percona/inventory/__mocks__/Inventory.service.ts @@ -72,6 +72,45 @@ export const nodesMock = [ }, ]; +const haNodeMock = (nodeId: string, nodeName: string, isPMMServerNode: boolean, isInternalNode = false) => ({ + node_id: nodeId, + node_type: 'generic', + node_name: nodeName, + is_pmm_server_node: isPMMServerNode, + is_pmm_internal_node: isInternalNode, + machine_id: '', + distro: '', + node_model: '', + container_id: '', + container_name: '', + address: '10.1.2.3', + region: '', + az: '', + custom_labels: {}, + created_at: '2026-07-27T08:05:31.079300Z', + updated_at: '2026-07-27T08:05:31.079300Z', + status: ServiceStatus.UP, + agents: [ + { + agent_id: `${nodeId}-pmm-agent`, + agent_type: AgentType.pmmAgent, + status: ServiceAgentStatus.RUNNING, + is_connected: true, + }, + ], + services: [], +}); + +// Mimics a PMM HA deployment: three PMM Server Nodes, an external PMM Client and the Nodes of +// PMM's own PostgreSQL cluster, which PMM Server reports as internal. +export const nodesMockHA = [ + haNodeMock('pmm-ha-0-id', 'pmm-ha-0', true), + haNodeMock('pmm-ha-1-id', 'pmm-ha-1', true), + haNodeMock('pmm-ha-2-id', 'pmm-ha-2', true), + haNodeMock('external-client-id', 'external-client', false), + haNodeMock('pg-db-instance1-id', 'pmm-pmm-ha-pg-db-instance1-qjjl-0', false, true), +]; + export const nodesMockMultipleAgentsNoPMMServer = [ { node_id: '324234234', diff --git a/public/app/percona/shared/core/reducers/nodes/nodes.utils.ts b/public/app/percona/shared/core/reducers/nodes/nodes.utils.ts index ac3d30f676853..a56f31e97b63b 100644 --- a/public/app/percona/shared/core/reducers/nodes/nodes.utils.ts +++ b/public/app/percona/shared/core/reducers/nodes/nodes.utils.ts @@ -58,6 +58,9 @@ export const nodeFromDbMapper = (nodeFromDb: NodeDB[]): Node[] => { export const nodesOptionsMapper = (nodeFromDb: NodeDB[]): NodesOption[] => nodeFromDb + // Nodes belonging to a PMM deployment's own infrastructure (e.g. the HA persistence layer) + // are dedicated and must not be delegated any monitoring workloads. + .filter((node) => !node.is_pmm_internal_node) .map((node) => { const agents = (node.agents || []) .filter((agent) => agent.agent_type === AgentType.pmmAgent) From a7f0e7a3e51e33f6047917d10699bdca80544b58 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Fri, 28 Aug 2026 02:37:31 +0300 Subject: [PATCH 2/3] PMM-14665 Stop relying on the hard-coded pmm-server identifier The Nodes dropdown compared Node and Agent identifiers against the literal "pmm-server". Those identifiers are moving to generated ones, at which point the comparisons would quietly stop matching: no Node would be preselected and the address would be prefilled with localhost for the PMM Server Node too. Every one of them is replaced by the is_pmm_server_node flag the API already reports, which carries the same meaning without depending on an identifier. Preselection now takes the PMM Server Node where it is still offered, and the first eligible Node otherwise, which is what an HA deployment gets now that PMM Server reports its own Nodes as internal. A Node running several pmm-agents is left for the user to pick from, as before. --- .../AddRemoteInstance.service.tsx | 3 +- .../NodesAgents/NodesAgents.constants.tsx | 2 - .../NodesAgents/NodesAgents.test.tsx | 8 ++-- .../FormParts/NodesAgents/NodesAgents.tsx | 43 +++++++------------ .../inventory/__mocks__/Inventory.service.ts | 12 +++--- 5 files changed, 27 insertions(+), 41 deletions(-) delete mode 100644 public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.constants.tsx diff --git a/public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.service.tsx b/public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.service.tsx index 04b703ecdfc63..6dfcecc5e31d0 100644 --- a/public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.service.tsx +++ b/public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.service.tsx @@ -1,6 +1,5 @@ import { CancelToken } from 'axios'; -import { PMM_SERVER_NODE_AGENT_ID } from 'app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.constants'; import { MetricsMode } from 'app/percona/inventory/Inventory.types'; import { Databases } from 'app/percona/shared/core'; import { apiManagement } from 'app/percona/shared/helpers/api'; @@ -207,7 +206,7 @@ export const toPayload = (values: any, discoverName?: string, type?: InstanceAva data.pmm_agent_id = values.pmm_agent_id.value; - if (data.pmm_agent_id === PMM_SERVER_NODE_AGENT_ID || data.node.isPMMServerNode) { + if (data.node.isPMMServerNode) { data.metrics_mode = MetricsMode.PULL; } else { data.metrics_mode = MetricsMode.PUSH; diff --git a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.constants.tsx b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.constants.tsx deleted file mode 100644 index bc62f0170f771..0000000000000 --- a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.constants.tsx +++ /dev/null @@ -1,2 +0,0 @@ -export const PMM_SERVER_NODE_ID = 'pmm-server'; -export const PMM_SERVER_NODE_AGENT_ID = 'pmm-server'; diff --git a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx index 291f89cca915c..ce815cf6b19e6 100644 --- a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx +++ b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx @@ -65,18 +65,20 @@ describe('Nodes Agents:: ', () => { selectEvent.openMenu(screen.getByLabelText('Nodes')); + // the PostgreSQL cluster backing PMM and the PMM Server nodes themselves expect(screen.queryByText('pmm-pmm-ha-pg-db-instance1-qjjl-0')).not.toBeInTheDocument(); - expect(screen.getByText('pmm-ha-0')).toBeInTheDocument(); + expect(screen.queryByText('pmm-ha-0')).not.toBeInTheDocument(); + expect(screen.getByText('pmm-pmm-ha-client-0')).toBeInTheDocument(); }); - it('should prefer a node other than a PMM Server one in an HA deployment', async () => { + it('should preselect the pre-provisioned client in an HA deployment', async () => { jest.spyOn(InventoryService, 'getNodes').mockReturnValue(Promise.resolve({ nodes: nodesMockHA })); setup(); await waitFor(() => expect(fetchNodesActionActionSpy).toHaveBeenCalled()); - await waitFor(() => expect(screen.getByTestId('node')).toHaveTextContent('external-client-id')); + await waitFor(() => expect(screen.getByTestId('node')).toHaveTextContent('pmm-ha-client-0-id')); }); it('should not pick any agent when the selected node is not pmm-server', async () => { diff --git a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.tsx b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.tsx index 6137a21af1856..26a5065f01069 100644 --- a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.tsx +++ b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.tsx @@ -4,13 +4,9 @@ import { useField } from 'react-final-form'; import { useStyles2 } from '@grafana/ui'; import { Messages } from 'app/percona/add-instance/components/AddRemoteInstance/FormParts/FormParts.messages'; import { getStyles } from 'app/percona/add-instance/components/AddRemoteInstance/FormParts/FormParts.styles'; -import { - PMM_SERVER_NODE_AGENT_ID, - PMM_SERVER_NODE_ID, -} from 'app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.constants'; import { NodesAgentsProps } from 'app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.types'; import { GET_NODES_CANCEL_TOKEN } from 'app/percona/inventory/Inventory.constants'; -import { AgentsOption, NodesOption } from 'app/percona/inventory/Inventory.types'; +import { NodesOption } from 'app/percona/inventory/Inventory.types'; import { SelectField } from 'app/percona/shared/components/Form/SelectFieldCore'; import { useCancelToken } from 'app/percona/shared/components/hooks/cancelToken.hook'; import { nodesOptionsMapper } from 'app/percona/shared/core/reducers/nodes'; @@ -45,28 +41,23 @@ export const NodesAgents: FC = ({ form }) => { // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - const changeAgentValue = (value: AgentsOption) => { - if (!form?.getState().values?.address) { - if (value.label !== PMM_SERVER_NODE_AGENT_ID) { - form?.change('address', 'localhost'); - } else { - form?.change('address', ''); - } + // A service monitored by the PMM Server node itself is a remote one, so its address is left for + // the user to fill in. Any other node runs next to what it monitors, hence the localhost default. + const prefillAddress = (node?: NodesOption) => { + if (node && !form?.getState().values?.address) { + form?.change('address', node.isPMMServerNode ? '' : 'localhost'); } }; const setNodeAndAgent = (value: NodesOption) => { form?.change('node', value); - let selectedAgent: AgentsOption | undefined; - if (value.agents && value.agents?.length > 1) { - selectedAgent = value.agents.find((item) => item.value === PMM_SERVER_NODE_AGENT_ID); - } else if (value.agents && value.agents?.length === 1) { - selectedAgent = value.agents[0]; - } + // A node running several pmm-agents is ambiguous, so the agent is left for the user to pick. + const selectedAgent = value.agents?.length === 1 ? value.agents[0] : undefined; + if (selectedAgent) { form?.change('pmm_agent_id', selectedAgent); - changeAgentValue(selectedAgent); + prefillAddress(value); } else { form?.change('pmm_agent_id', undefined); } @@ -76,14 +67,10 @@ export const NodesAgents: FC = ({ form }) => { if (nodesOptions.length === 0) { loadData(); } else if (!selectedNode) { - // A deployment with more than one PMM Server node runs in HA mode, where the PMM Server - // pods are meant to stay free of monitoring workloads. Prefer any other node there, - // otherwise preselect the pmm-server node as usual. - const isHighlyAvailable = nodesOptions.filter((node) => node.isPMMServerNode).length > 1; - const preselectedNode = - (isHighlyAvailable ? nodesOptions.find((node) => !node.isPMMServerNode) : undefined) ?? - nodesOptions.find((node) => node.value === PMM_SERVER_NODE_ID) ?? - nodesOptions.find((node) => node.isPMMServerNode); + // PMM Server reports the nodes it does not want monitoring delegated to, and they are already + // filtered out. Whatever is left is eligible, the PMM Server node being the natural default + // where it is still offered - a single-node deployment has no other node to pick. + const preselectedNode = nodesOptions.find((node) => node.isPMMServerNode) ?? nodesOptions[0]; if (preselectedNode) { setNodeAndAgent(preselectedNode); @@ -116,7 +103,7 @@ export const NodesAgents: FC = ({ form }) => { options={selectedNode?.agents || []} name="pmm_agent_id" data-testid="agents-selectbox" - onChange={(event) => changeAgentValue(event as AgentsOption)} + onChange={() => prefillAddress(selectedNode)} className={styles.selectField} aria-label={Messages.form.labels.nodesAgents.agents} validators={selectedNode ? [validators.required] : undefined} diff --git a/public/app/percona/inventory/__mocks__/Inventory.service.ts b/public/app/percona/inventory/__mocks__/Inventory.service.ts index ca50bb045efee..9a0c8d0ec5795 100644 --- a/public/app/percona/inventory/__mocks__/Inventory.service.ts +++ b/public/app/percona/inventory/__mocks__/Inventory.service.ts @@ -101,13 +101,13 @@ const haNodeMock = (nodeId: string, nodeName: string, isPMMServerNode: boolean, services: [], }); -// Mimics a PMM HA deployment: three PMM Server Nodes, an external PMM Client and the Nodes of -// PMM's own PostgreSQL cluster, which PMM Server reports as internal. +// Mimics a PMM HA deployment as PMM Server reports it: the PMM Server Nodes and the Nodes of PMM's +// own PostgreSQL cluster are internal, leaving the pre-provisioned PMM Client to be monitored with. export const nodesMockHA = [ - haNodeMock('pmm-ha-0-id', 'pmm-ha-0', true), - haNodeMock('pmm-ha-1-id', 'pmm-ha-1', true), - haNodeMock('pmm-ha-2-id', 'pmm-ha-2', true), - haNodeMock('external-client-id', 'external-client', false), + haNodeMock('pmm-ha-0-id', 'pmm-ha-0', true, true), + haNodeMock('pmm-ha-1-id', 'pmm-ha-1', true, true), + haNodeMock('pmm-ha-2-id', 'pmm-ha-2', true, true), + haNodeMock('pmm-ha-client-0-id', 'pmm-pmm-ha-client-0', false), haNodeMock('pg-db-instance1-id', 'pmm-pmm-ha-pg-db-instance1-qjjl-0', false, true), ]; From e82067ed499a31b6916348fc7d62c475321a6cec Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Fri, 28 Aug 2026 11:19:42 +0300 Subject: [PATCH 3/3] PMM-14665 Fix the tests broken by the new node filtering Two suites depended on behaviour this branch changed. The internal-node test asked for the client node by text while it is now also the preselected value, so it matched both the selected value and the menu option. It now asserts on both matches, which doubles as proof that the menu is open and the absence assertions above it are not passing vacuously. The payload test paired the PMM Server Agent with a Node that was not flagged as the PMM Server one, which only worked while the metrics mode was decided by the Agent identifier. The fixture now says what it means and keeps covering PULL. --- .../AddRemoteInstance/AddRemoteInstance.service.test.tsx | 2 ++ .../FormParts/NodesAgents/NodesAgents.test.tsx | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.service.test.tsx b/public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.service.test.tsx index 8a6a57cef814b..ca22cf6c3a2dd 100644 --- a/public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.service.test.tsx +++ b/public/app/percona/add-instance/components/AddRemoteInstance/AddRemoteInstance.service.test.tsx @@ -70,9 +70,11 @@ describe('AddRemoteInstanceService:: ', () => { pmm_agent_id: { value: 'pmm-server', }, + // the exporter runs on the PMM Server node, which is what makes the metrics mode PULL node: { value: 'node1', label: 'node1', + isPMMServerNode: true, }, }; diff --git a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx index ce815cf6b19e6..86caf2d018e49 100644 --- a/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx +++ b/public/app/percona/add-instance/components/AddRemoteInstance/FormParts/NodesAgents/NodesAgents.test.tsx @@ -68,7 +68,9 @@ describe('Nodes Agents:: ', () => { // the PostgreSQL cluster backing PMM and the PMM Server nodes themselves expect(screen.queryByText('pmm-pmm-ha-pg-db-instance1-qjjl-0')).not.toBeInTheDocument(); expect(screen.queryByText('pmm-ha-0')).not.toBeInTheDocument(); - expect(screen.getByText('pmm-pmm-ha-client-0')).toBeInTheDocument(); + // the client is both the selected value and an option, which also proves the menu is open and + // the assertions above are not passing vacuously + expect(screen.getAllByText('pmm-pmm-ha-client-0').length).toBeGreaterThan(1); }); it('should preselect the pre-provisioned client in an HA deployment', async () => {