Skip to content

Commit

Permalink
[8.7] [Fleet] Fix agent policy selection and creation when there are …
Browse files Browse the repository at this point in the history
…a large number of agent policies ( > 1k) (#151119) (#151131)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[Fleet] Fix agent policy selection and creation when there are a
large number of agent policies ( > 1k)
(#151119)](#151119)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Mark
Hopkin","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-02-14T13:36:17Z","message":"[Fleet]
Fix agent policy selection and creation when there are a large number of
agent policies ( > 1k) (#151119)\n\n## Summary\r\n\r\nCloses
#150605\r\n\r\nWhen adding an integration, the agent policy selector
(create or select\r\nexisting agent policy) does not work if there are
too many agent\r\npolicies.\r\n\r\nIn an environment with a large number
of agent policies (e.g 1000+) the\r\nfollowing API request times out or
takes 30 - 60 seconds to
respond:\r\n\r\n```\r\nhttp://localhost:5601/mark/api/fleet/agent_policies?page=1&perPage=10000&sortField=name&sortOrder=asc&full=true\r\n```\r\n\r\nThat
is because for each agent policy we get the saved object twice,
get\r\nALL package policies, and count the agents, so for 1000 agents we
fun\r\n4000 queries using `pmap` in some places.\r\n\r\nthis PR changes
it so that we do not get the agent count or `full` agent\r\npolicy for
every agent policy when populating the agent policy select,\r\ninstead
we only get the selected agent policy.\r\n\r\nThis has meant that I have
to separately query package policies in order\r\nto do the limited
packages and APM output checks, but the component now\r\nloads instantly
on an environment with 3k agent policies.\r\n\r\nKey changes:\r\n\r\n-
add `noAgentCount` query param to agent policies API, to not count
the\r\nagents for all agent policies\r\n- populate `agents` in the get
one agent policy API, this was an\r\ninconsistency in our agent policy
API\r\n- when selecting an existing agent policy, only get the full
agent\r\npolicy for the selected agent
policy\r\n\r\n\r\n###Testing\r\n\r\nNo behaviour should have changed for
the agent policy select, some niche\r\nbehaviour of the
selector:\r\n\r\n- if adding the APM integration, any agent policy with
a logstash output\r\nconfigured for integration data should be
disabled\r\n- if adding a limited integration e.g apm or defend, any
agent policy\r\nalready containing that integration should be
disabled\r\n\r\n<img width=\"1052\" alt=\"Screenshot 2023-02-14 at 10 45
03\"\r\nsrc=\"https://user-images.githubusercontent.com/3315046/218745430-4a8f0ded-1e0b-4319-bc2c-cc5253b4cdd2.png\">","sha":"9a52ef4baf07baa8a877252e3a740cfde1624063","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Fleet","v8.7.0","v8.8.0"],"number":151119,"url":"https://github.com/elastic/kibana/pull/151119","mergeCommit":{"message":"[Fleet]
Fix agent policy selection and creation when there are a large number of
agent policies ( > 1k) (#151119)\n\n## Summary\r\n\r\nCloses
#150605\r\n\r\nWhen adding an integration, the agent policy selector
(create or select\r\nexisting agent policy) does not work if there are
too many agent\r\npolicies.\r\n\r\nIn an environment with a large number
of agent policies (e.g 1000+) the\r\nfollowing API request times out or
takes 30 - 60 seconds to
respond:\r\n\r\n```\r\nhttp://localhost:5601/mark/api/fleet/agent_policies?page=1&perPage=10000&sortField=name&sortOrder=asc&full=true\r\n```\r\n\r\nThat
is because for each agent policy we get the saved object twice,
get\r\nALL package policies, and count the agents, so for 1000 agents we
fun\r\n4000 queries using `pmap` in some places.\r\n\r\nthis PR changes
it so that we do not get the agent count or `full` agent\r\npolicy for
every agent policy when populating the agent policy select,\r\ninstead
we only get the selected agent policy.\r\n\r\nThis has meant that I have
to separately query package policies in order\r\nto do the limited
packages and APM output checks, but the component now\r\nloads instantly
on an environment with 3k agent policies.\r\n\r\nKey changes:\r\n\r\n-
add `noAgentCount` query param to agent policies API, to not count
the\r\nagents for all agent policies\r\n- populate `agents` in the get
one agent policy API, this was an\r\ninconsistency in our agent policy
API\r\n- when selecting an existing agent policy, only get the full
agent\r\npolicy for the selected agent
policy\r\n\r\n\r\n###Testing\r\n\r\nNo behaviour should have changed for
the agent policy select, some niche\r\nbehaviour of the
selector:\r\n\r\n- if adding the APM integration, any agent policy with
a logstash output\r\nconfigured for integration data should be
disabled\r\n- if adding a limited integration e.g apm or defend, any
agent policy\r\nalready containing that integration should be
disabled\r\n\r\n<img width=\"1052\" alt=\"Screenshot 2023-02-14 at 10 45
03\"\r\nsrc=\"https://user-images.githubusercontent.com/3315046/218745430-4a8f0ded-1e0b-4319-bc2c-cc5253b4cdd2.png\">","sha":"9a52ef4baf07baa8a877252e3a740cfde1624063"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/151119","number":151119,"mergeCommit":{"message":"[Fleet]
Fix agent policy selection and creation when there are a large number of
agent policies ( > 1k) (#151119)\n\n## Summary\r\n\r\nCloses
#150605\r\n\r\nWhen adding an integration, the agent policy selector
(create or select\r\nexisting agent policy) does not work if there are
too many agent\r\npolicies.\r\n\r\nIn an environment with a large number
of agent policies (e.g 1000+) the\r\nfollowing API request times out or
takes 30 - 60 seconds to
respond:\r\n\r\n```\r\nhttp://localhost:5601/mark/api/fleet/agent_policies?page=1&perPage=10000&sortField=name&sortOrder=asc&full=true\r\n```\r\n\r\nThat
is because for each agent policy we get the saved object twice,
get\r\nALL package policies, and count the agents, so for 1000 agents we
fun\r\n4000 queries using `pmap` in some places.\r\n\r\nthis PR changes
it so that we do not get the agent count or `full` agent\r\npolicy for
every agent policy when populating the agent policy select,\r\ninstead
we only get the selected agent policy.\r\n\r\nThis has meant that I have
to separately query package policies in order\r\nto do the limited
packages and APM output checks, but the component now\r\nloads instantly
on an environment with 3k agent policies.\r\n\r\nKey changes:\r\n\r\n-
add `noAgentCount` query param to agent policies API, to not count
the\r\nagents for all agent policies\r\n- populate `agents` in the get
one agent policy API, this was an\r\ninconsistency in our agent policy
API\r\n- when selecting an existing agent policy, only get the full
agent\r\npolicy for the selected agent
policy\r\n\r\n\r\n###Testing\r\n\r\nNo behaviour should have changed for
the agent policy select, some niche\r\nbehaviour of the
selector:\r\n\r\n- if adding the APM integration, any agent policy with
a logstash output\r\nconfigured for integration data should be
disabled\r\n- if adding a limited integration e.g apm or defend, any
agent policy\r\nalready containing that integration should be
disabled\r\n\r\n<img width=\"1052\" alt=\"Screenshot 2023-02-14 at 10 45
03\"\r\nsrc=\"https://user-images.githubusercontent.com/3315046/218745430-4a8f0ded-1e0b-4319-bc2c-cc5253b4cdd2.png\">","sha":"9a52ef4baf07baa8a877252e3a740cfde1624063"}}]}]
BACKPORT-->

Co-authored-by: Mark Hopkin <[email protected]>
  • Loading branch information
kibanamachine and hop-dev authored Feb 14, 2023
1 parent d335ebd commit c449b2a
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 42 deletions.
8 changes: 8 additions & 0 deletions x-pack/plugins/fleet/common/openapi/bundled.json
Original file line number Diff line number Diff line change
Expand Up @@ -2511,6 +2511,14 @@
"in": "query",
"name": "full",
"description": "When set to true, retrieve the related package policies for each agent policy."
},
{
"schema": {
"type": "boolean"
},
"in": "query",
"name": "noAgentCount",
"description": "When set to true, do not count how many agents are in the agent policy, this can improve performance if you are searching over a large number of agent policies. The \"agents\" property will always be 0 if set to true."
}
],
"description": ""
Expand Down
9 changes: 9 additions & 0 deletions x-pack/plugins/fleet/common/openapi/bundled.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1551,6 +1551,15 @@ paths:
description: >-
When set to true, retrieve the related package policies for each
agent policy.
- schema:
type: boolean
in: query
name: noAgentCount
description: >-
When set to true, do not count how many agents are in the agent
policy, this can improve performance if you are searching over a
large number of agent policies. The "agents" property will always be
0 if set to true.
description: ''
post:
summary: Agent policy - Create
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/fleet/common/openapi/paths/agent_policies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ get:
in: query
name: full
description: When set to true, retrieve the related package policies for each agent policy.
- schema:
type: boolean
in: query
name: noAgentCount
description: When set to true, do not count how many agents are in the agent policy, this can improve performance if you are searching over a large number of agent policies. The "agents" property will always be 0 if set to true.

description: ''
post:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { ListResult, ListWithKuery, BulkGetResult } from './common';

export interface GetAgentPoliciesRequest {
query: ListWithKuery & {
noAgentCount?: boolean;
full?: boolean;
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,20 @@ import {
} from '@elastic/eui';

import { Error } from '../../../../../components';
import type {
AgentPolicy,
Output,
PackageInfo,
GetAgentPoliciesResponseItem,
} from '../../../../../types';
import type { AgentPolicy, Output, PackageInfo } from '../../../../../types';
import { isPackageLimited, doesAgentPolicyAlreadyIncludePackage } from '../../../../../services';
import {
useGetAgentPolicies,
useGetOutputs,
sendGetOneAgentPolicy,
useFleetStatus,
useGetPackagePolicies,
} from '../../../../../hooks';
import {
FLEET_APM_PACKAGE,
SO_SEARCH_LIMIT,
outputType,
PACKAGE_POLICY_SAVED_OBJECT_TYPE,
} from '../../../../../../../../common/constants';

const AgentPolicyFormRow = styled(EuiFormRow)`
Expand All @@ -58,22 +55,36 @@ function useAgentPoliciesOptions(packageInfo?: PackageInfo) {
perPage: SO_SEARCH_LIMIT,
sortField: 'name',
sortOrder: 'asc',
full: true,
noAgentCount: true, // agentPolicy.agents will always be 0
full: false, // package_policies will always be empty
});
const agentPolicies = useMemo(
() => agentPoliciesData?.items.filter((policy) => !policy.is_managed) || [],
[agentPoliciesData?.items]
);

const agentPoliciesById = useMemo(() => {
return agentPolicies.reduce((acc: { [key: string]: GetAgentPoliciesResponseItem }, policy) => {
acc[policy.id] = policy;
return acc;
}, {});
}, [agentPolicies]);

const { data: outputsData, isLoading: isOutputLoading } = useGetOutputs();

// get all package policies with apm integration or the current integration
const { data: packagePoliciesForThisPackage, isLoading: isLoadingPackagePolicies } =
useGetPackagePolicies({
page: 1,
perPage: SO_SEARCH_LIMIT,
kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.package.name: ${packageInfo?.name}`,
});

const packagePoliciesForThisPackageByAgentPolicyId = useMemo(
() =>
packagePoliciesForThisPackage?.items.reduce(
(acc: { [key: string]: boolean }, packagePolicy) => {
acc[packagePolicy.policy_id] = true;
return acc;
},
{}
),
[packagePoliciesForThisPackage?.items]
);

const { getDataOutputForPolicy } = useMemo(() => {
const defaultOutput = (outputsData?.items ?? []).find((output) => output.is_default);
const outputsById = (outputsData?.items ?? []).reduce(
Expand All @@ -85,7 +96,7 @@ function useAgentPoliciesOptions(packageInfo?: PackageInfo) {
);

return {
getDataOutputForPolicy: (policy: AgentPolicy) => {
getDataOutputForPolicy: (policy: Pick<AgentPolicy, 'data_output_id'>) => {
return policy.data_output_id ? outputsById[policy.data_output_id] : defaultOutput;
},
};
Expand All @@ -94,20 +105,19 @@ function useAgentPoliciesOptions(packageInfo?: PackageInfo) {
const agentPolicyOptions: Array<EuiSuperSelectOption<string>> = useMemo(
() =>
packageInfo
? agentPolicies.map((agentConf) => {
const isLimitedPackageAlreadyInPolicy = doesAgentPolicyHaveLimitedPackage(
agentConf,
packageInfo
);
? agentPolicies.map((policy) => {
const isLimitedPackageAlreadyInPolicy =
isPackageLimited(packageInfo) &&
packagePoliciesForThisPackageByAgentPolicyId?.[policy.id];

const isAPMPackageAndDataOutputIsLogstash =
packageInfo.name === FLEET_APM_PACKAGE &&
getDataOutputForPolicy(agentConf)?.type === outputType.Logstash;
packageInfo?.name === FLEET_APM_PACKAGE &&
getDataOutputForPolicy(policy)?.type === outputType.Logstash;

return {
inputDisplay: (
<>
<EuiText size="s">{agentConf.name}</EuiText>
<EuiText size="s">{policy.name}</EuiText>
{isAPMPackageAndDataOutputIsLogstash && (
<>
<EuiSpacer size="xs" />
Expand All @@ -121,21 +131,25 @@ function useAgentPoliciesOptions(packageInfo?: PackageInfo) {
)}
</>
),
value: agentConf.id,
value: policy.id,
disabled: isLimitedPackageAlreadyInPolicy || isAPMPackageAndDataOutputIsLogstash,
'data-test-subj': 'agentPolicyItem',
};
})
: [],
[agentPolicies, packageInfo, getDataOutputForPolicy]
[
packageInfo,
agentPolicies,
packagePoliciesForThisPackageByAgentPolicyId,
getDataOutputForPolicy,
]
);

return {
agentPoliciesError,
isLoading: isOutputLoading || isAgentPoliciesLoading,
agentPolicies,
agentPoliciesById,
isLoading: isOutputLoading || isAgentPoliciesLoading || isLoadingPackagePolicies,
agentPolicyOptions,
agentPolicies,
};
}

Expand All @@ -154,26 +168,39 @@ export const StepSelectAgentPolicy: React.FunctionComponent<{
}> = ({
packageInfo,
agentPolicy,
updateAgentPolicy,
updateAgentPolicy: updateSelectedAgentPolicy,
setHasAgentPolicyError,
selectedAgentPolicyId,
}) => {
const { isReady: isFleetReady } = useFleetStatus();

const [selectedAgentPolicyError, setSelectedAgentPolicyError] = useState<Error>();

const { agentPolicies, agentPoliciesById, isLoading, agentPoliciesError, agentPolicyOptions } =
const { isLoading, agentPoliciesError, agentPolicyOptions, agentPolicies } =
useAgentPoliciesOptions(packageInfo);
// Selected agent policy state
const [selectedPolicyId, setSelectedPolicyId] = useState<string | undefined>(
agentPolicy?.id ??
(selectedAgentPolicyId || (agentPolicies.length === 1 ? agentPolicies[0].id : undefined))
);

const [isLoadingSelectedAgentPolicy, setIsLoadingSelectedAgentPolicy] = useState<boolean>(false);
const [selectedAgentPolicy, setSelectedAgentPolicy] = useState<AgentPolicy | undefined>(
agentPolicy
);

const updateAgentPolicy = useCallback(
(selectedPolicy: AgentPolicy | undefined) => {
setSelectedAgentPolicy(selectedPolicy);
updateSelectedAgentPolicy(selectedPolicy);
},
[updateSelectedAgentPolicy]
);
// Update parent selected agent policy state
useEffect(() => {
const fetchAgentPolicyInfo = async () => {
if (selectedPolicyId) {
setIsLoadingSelectedAgentPolicy(true);
const { data, error } = await sendGetOneAgentPolicy(selectedPolicyId);
if (error) {
setSelectedAgentPolicyError(error);
Expand All @@ -182,6 +209,7 @@ export const StepSelectAgentPolicy: React.FunctionComponent<{
setSelectedAgentPolicyError(undefined);
updateAgentPolicy(data.item);
}
setIsLoadingSelectedAgentPolicy(false);
} else {
setSelectedAgentPolicyError(undefined);
updateAgentPolicy(undefined);
Expand Down Expand Up @@ -268,23 +296,21 @@ export const StepSelectAgentPolicy: React.FunctionComponent<{
</EuiFlexGroup>
}
helpText={
isFleetReady && selectedPolicyId ? (
isFleetReady && selectedPolicyId && !isLoadingSelectedAgentPolicy ? (
<FormattedMessage
id="xpack.fleet.createPackagePolicy.StepSelectPolicy.agentPolicyAgentsDescriptionText"
defaultMessage="{count, plural, one {# agent is} other {# agents are}} enrolled with the selected agent policy."
values={{
count: agentPoliciesById[selectedPolicyId]?.agents ?? 0,
count: selectedAgentPolicy?.agents ?? 0,
}}
/>
) : null
}
isInvalid={Boolean(
!selectedPolicyId ||
!packageInfo ||
doesAgentPolicyHaveLimitedPackage(
agentPoliciesById[selectedPolicyId],
packageInfo
)
(selectedAgentPolicy &&
doesAgentPolicyHaveLimitedPackage(selectedAgentPolicy, packageInfo))
)}
error={
!selectedPolicyId ? (
Expand All @@ -308,7 +334,7 @@ export const StepSelectAgentPolicy: React.FunctionComponent<{
}
)}
fullWidth
isLoading={isLoading || !packageInfo}
isLoading={isLoading || !packageInfo || isLoadingSelectedAgentPolicy}
options={agentPolicyOptions}
valueOfSelected={selectedPolicyId}
onChange={onChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ export const StepSelectHosts: React.FunctionComponent<Props> = ({
perPage: SO_SEARCH_LIMIT,
sortField: 'name',
sortOrder: 'asc',
full: true,
full: false, // package_policies will always be empty
noAgentCount: true, // agentPolicy.agents will always be 0
});
if (err) {
// eslint-disable-next-line no-console
Expand Down
12 changes: 8 additions & 4 deletions x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const getAgentPoliciesHandler: FleetRequestHandler<
const fleetContext = await context.fleet;
const soClient = fleetContext.internalSoClient;
const esClient = coreContext.elasticsearch.client.asInternalUser;
const { full: withPackagePolicies = false, ...restOfQuery } = request.query;
const { full: withPackagePolicies = false, noAgentCount = false, ...restOfQuery } = request.query;
try {
const { items, total, page, perPage } = await agentPolicyService.list(soClient, {
withPackagePolicies,
Expand All @@ -88,9 +88,11 @@ export const getAgentPoliciesHandler: FleetRequestHandler<
page,
perPage,
};

await populateAssignedAgentsCount(esClient, soClient, items);

if (!noAgentCount) {
await populateAssignedAgentsCount(esClient, soClient, items);
} else {
items.forEach((item) => (item.agents = 0));
}
return response.ok({ body });
} catch (error) {
return defaultFleetErrorHandler({ error, response });
Expand Down Expand Up @@ -136,10 +138,12 @@ export const getOneAgentPolicyHandler: RequestHandler<
TypeOf<typeof GetOneAgentPolicyRequestSchema.params>
> = async (context, request, response) => {
const coreContext = await context.core;
const esClient = coreContext.elasticsearch.client.asInternalUser;
const soClient = coreContext.savedObjects.client;
try {
const agentPolicy = await agentPolicyService.get(soClient, request.params.agentPolicyId);
if (agentPolicy) {
await populateAssignedAgentsCount(esClient, soClient, [agentPolicy]);
const body: GetOneAgentPolicyResponse = {
item: agentPolicy,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { ListWithKuerySchema, BulkRequestBodySchema } from './common';

export const GetAgentPoliciesRequestSchema = {
query: ListWithKuerySchema.extends({
noAgentCount: schema.maybe(schema.boolean()),
full: schema.maybe(schema.boolean()),
}),
};
Expand Down

0 comments on commit c449b2a

Please sign in to comment.