From 0f92d0734396912aaec4d2867e20d29714240dea Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Thu, 15 Dec 2022 10:13:42 +0100
Subject: [PATCH 01/11] refactored bulk update tags retry

---
 .../components/tags_add_remove.test.tsx       | 22 ++++-
 .../components/tags_add_remove.tsx            | 26 +-----
 .../fleet/server/services/agents/crud.ts      |  5 +-
 .../services/agents/update_agent_tags.test.ts | 92 ++++++++++++++++++-
 .../agents/update_agent_tags_action_runner.ts | 49 ++++++----
 5 files changed, 148 insertions(+), 46 deletions(-)

diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.test.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.test.tsx
index 465db5236338c..4f4e7e24097f4 100644
--- a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.test.tsx
+++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.test.tsx
@@ -292,7 +292,16 @@ describe('TagsAddRemove', () => {
 
     expect(mockBulkUpdateTags).toHaveBeenCalledWith(
       'query',
-      ['newTag2', 'newTag'],
+      ['newTag'],
+      [],
+      expect.anything(),
+      'Tag created',
+      'Tag creation failed'
+    );
+
+    expect(mockBulkUpdateTags).toHaveBeenCalledWith(
+      'query',
+      ['newTag2'],
       [],
       expect.anything(),
       'Tag created',
@@ -316,7 +325,16 @@ describe('TagsAddRemove', () => {
     expect(mockBulkUpdateTags).toHaveBeenCalledWith(
       '',
       [],
-      ['tag2', 'tag1'],
+      ['tag1'],
+      expect.anything(),
+      undefined,
+      undefined
+    );
+
+    expect(mockBulkUpdateTags).toHaveBeenCalledWith(
+      '',
+      [],
+      ['tag2'],
       expect.anything(),
       undefined,
       undefined
diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.tsx
index 8307bc3467cc2..8e539954e5204 100644
--- a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.tsx
+++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/tags_add_remove.tsx
@@ -120,32 +120,10 @@ export const TagsAddRemove: React.FC<Props> = ({
         errorMessage
       );
     } else {
-      // sending updated tags to add/remove, in case multiple actions are done quickly and the first one is not yet propagated
-      const updatedTagsToAdd = tagsToAdd.concat(
-        labels
-          .filter(
-            (tag) =>
-              tag.checked === 'on' &&
-              !selectedTags.includes(tag.label) &&
-              !tagsToRemove.includes(tag.label)
-          )
-          .map((tag) => tag.label)
-      );
-      const updatedTagsToRemove = tagsToRemove.concat(
-        labels
-          .filter(
-            (tag) =>
-              tag.checked !== 'on' &&
-              selectedTags.includes(tag.label) &&
-              !tagsToAdd.includes(tag.label)
-          )
-          .map((tag) => tag.label)
-      );
-
       updateTagsHook.bulkUpdateTags(
         agents!,
-        updatedTagsToAdd,
-        updatedTagsToRemove,
+        tagsToAdd,
+        tagsToRemove,
         (hasCompleted) => handleTagsUpdated(tagsToAdd, tagsToRemove, hasCompleted),
         successMessage,
         errorMessage
diff --git a/x-pack/plugins/fleet/server/services/agents/crud.ts b/x-pack/plugins/fleet/server/services/agents/crud.ts
index 97efef4f226c5..c3ad82518625f 100644
--- a/x-pack/plugins/fleet/server/services/agents/crud.ts
+++ b/x-pack/plugins/fleet/server/services/agents/crud.ts
@@ -155,7 +155,8 @@ export function getElasticsearchQuery(
   kuery: string,
   showInactive = false,
   includeHosted = false,
-  hostedPolicies: string[] = []
+  hostedPolicies: string[] = [],
+  extraFilters: string[] = []
 ): estypes.QueryDslQueryContainer | undefined {
   const filters = [];
 
@@ -171,6 +172,8 @@ export function getElasticsearchQuery(
     filters.push('NOT (policy_id:{policyIds})'.replace('{policyIds}', hostedPolicies.join(',')));
   }
 
+  filters.push(...extraFilters);
+
   const kueryNode = _joinFilters(filters);
   return kueryNode ? toElasticsearchQuery(kueryNode) : undefined;
 }
diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
index a37165483f136..c1170c5055043 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
@@ -10,6 +10,7 @@ import { elasticsearchServiceMock, savedObjectsClientMock } from '@kbn/core/serv
 
 import { createClientMock } from './action.mock';
 import { updateAgentTags } from './update_agent_tags';
+import { updateTagsBatch } from './update_agent_tags_action_runner';
 
 jest.mock('../app_context', () => {
   return {
@@ -28,6 +29,7 @@ jest.mock('../agent_policy', () => {
   return {
     agentPolicyService: {
       getByIDs: jest.fn().mockResolvedValue([{ id: 'hosted-agent-policy', is_managed: true }]),
+      list: jest.fn().mockResolvedValue({ items: [] }),
     },
   };
 });
@@ -73,7 +75,7 @@ describe('update_agent_tags', () => {
 
     expect(esClient.updateByQuery).toHaveBeenCalledWith(
       expect.objectContaining({
-        conflicts: 'abort',
+        conflicts: 'proceed',
         index: '.fleet-agents',
         query: { terms: { _id: ['agent1'] } },
         script: expect.objectContaining({
@@ -152,6 +154,19 @@ describe('update_agent_tags', () => {
     expect(errorResults.body[1].error).toEqual('error reason');
   });
 
+  it('should throw error on version conflicts', async () => {
+    esClient.updateByQuery.mockReset();
+    esClient.updateByQuery.mockResolvedValue({
+      failures: [],
+      updated: 0,
+      version_conflicts: 100,
+    } as any);
+
+    await expect(
+      updateAgentTags(soClient, esClient, { agentIds: ['agent1'] }, ['one'], [])
+    ).rejects.toThrowError('version conflict of 100 agents');
+  });
+
   it('should run add tags async when actioning more agents than batch size', async () => {
     esClient.search.mockResolvedValue({
       hits: {
@@ -180,4 +195,79 @@ describe('update_agent_tags', () => {
 
     expect(mockRunAsync).toHaveBeenCalled();
   });
+
+  it('should add tags filter if only one tag to add', async () => {
+    await updateTagsBatch(
+      soClient,
+      esClient,
+      [],
+      {},
+      {
+        tagsToAdd: ['new'],
+        tagsToRemove: [],
+        kuery: '',
+      }
+    );
+
+    const updateByQuery = esClient.updateByQuery.mock.calls[0][0] as any;
+    expect(updateByQuery.query).toEqual({
+      bool: {
+        filter: [
+          { bool: { minimum_should_match: 1, should: [{ match: { active: true } }] } },
+          {
+            bool: {
+              must_not: { bool: { minimum_should_match: 1, should: [{ match: { tags: 'new' } }] } },
+            },
+          },
+        ],
+      },
+    });
+  });
+
+  it('should add tags filter if only one tag to remove', async () => {
+    await updateTagsBatch(
+      soClient,
+      esClient,
+      [],
+      {},
+      {
+        tagsToAdd: [],
+        tagsToRemove: ['remove'],
+        kuery: '',
+      }
+    );
+
+    const updateByQuery = esClient.updateByQuery.mock.calls[0][0] as any;
+    expect(JSON.stringify(updateByQuery.query)).toContain(
+      '{"bool":{"should":[{"match":{"tags":"remove"}}],"minimum_should_match":1}}'
+    );
+  });
+
+  it('should write total from updateByQuery result if query returns less results', async () => {
+    esClient.updateByQuery.mockReset();
+    esClient.updateByQuery.mockResolvedValue({ failures: [], updated: 0, total: 50 } as any);
+
+    await updateTagsBatch(
+      soClient,
+      esClient,
+      [],
+      {},
+      {
+        tagsToAdd: ['new'],
+        tagsToRemove: [],
+        kuery: '',
+        total: 100,
+      }
+    );
+
+    const agentAction = esClient.create.mock.calls[0][0] as any;
+    expect(agentAction?.body).toEqual(
+      expect.objectContaining({
+        action_id: expect.anything(),
+        agents: [],
+        type: 'UPDATE_TAGS',
+        total: 50,
+      })
+    );
+  });
 });
diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
index 042707b80df24..7687810db6c18 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
@@ -63,6 +63,7 @@ export class UpdateAgentTagsActionRunner extends ActionRunner {
         actionId: this.actionParams.actionId,
         total: this.actionParams.total,
         kuery: this.actionParams.kuery,
+        isRetry: !!this.retryParams.retryCount,
       }
     );
 
@@ -82,6 +83,7 @@ export async function updateTagsBatch(
     actionId?: string;
     total?: number;
     kuery?: string;
+    isRetry?: boolean;
   }
 ): Promise<{ actionId: string; updated?: number; took?: number }> {
   const errors: Record<Agent['id'], Error> = { ...outgoingErrors };
@@ -108,7 +110,13 @@ export async function updateTagsBatch(
     });
     const hostedIds = hostedPolicies.items.map((item) => item.id);
 
-    query = getElasticsearchQuery(options.kuery, false, false, hostedIds);
+    const extraFilters = [];
+    if (options.tagsToAdd.length === 1 && options.tagsToRemove.length === 0) {
+      extraFilters.push(`NOT (tags:${options.tagsToAdd[0]})`);
+    } else if (options.tagsToRemove.length === 1) {
+      extraFilters.push(`tags:${options.tagsToRemove[0]}`);
+    }
+    query = getElasticsearchQuery(options.kuery, false, false, hostedIds, extraFilters);
   } else {
     query = {
       terms: {
@@ -150,7 +158,7 @@ export async function updateTagsBatch(
           updatedAt: new Date().toISOString(),
         },
       },
-      conflicts: 'abort', // relying on the task to retry in case of conflicts
+      conflicts: 'proceed', // relying on the task to retry in case of conflicts - retry only conflicted agents
     });
   } catch (error) {
     throw new Error('Caught error: ' + JSON.stringify(error).slice(0, 1000));
@@ -161,14 +169,17 @@ export async function updateTagsBatch(
   const actionId = options.actionId ?? uuid();
   const total = options.total ?? givenAgents.length;
 
-  // creating an action doc so that update tags  shows up in activity
-  await createAgentAction(esClient, {
-    id: actionId,
-    agents: options.kuery === undefined ? agentIds : [],
-    created_at: new Date().toISOString(),
-    type: 'UPDATE_TAGS',
-    total,
-  });
+  if (!options.isRetry) {
+    // creating an action doc so that update tags  shows up in activity
+    await createAgentAction(esClient, {
+      id: actionId,
+      agents: options.kuery === undefined ? agentIds : [],
+      created_at: new Date().toISOString(),
+      type: 'UPDATE_TAGS',
+      // for kuery cases, the total can be less than the initial selection, as we filter out tags that are already added/removed
+      total: options.kuery === undefined ? total : res.total,
+    });
+  }
 
   // creating unique 0...n ids to use as agentId, as we don't have all agent ids in case of action by kuery
   const getArray = (count: number) => [...Array(count).keys()];
@@ -198,18 +209,20 @@ export async function updateTagsBatch(
   }
 
   // writing hosted agent errors - hosted agents filtered out
-  if ((res.total ?? total) < total) {
+  if (options.kuery === undefined && hostedAgentIds.length > 0) {
     await bulkCreateAgentActionResults(
       esClient,
-      (options.kuery === undefined ? hostedAgentIds : getArray(total - (res.total ?? total))).map(
-        (id) => ({
-          agentId: id + '',
-          actionId,
-          error: hostedAgentError,
-        })
-      )
+      hostedAgentIds.map((id) => ({
+        agentId: id + '',
+        actionId,
+        error: hostedAgentError,
+      }))
     );
   }
 
+  if (res.version_conflicts ?? 0 > 0) {
+    throw new Error(`version conflict of ${res.version_conflicts} agents`);
+  }
+
   return { actionId, updated: res.updated, took: res.took };
 }

From c098438016806b3be712d1ac50ec74eb354d2b10 Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Thu, 15 Dec 2022 14:07:46 +0100
Subject: [PATCH 02/11] fixed test

---
 .../agents/update_agent_tags_action_runner.ts |  2 +-
 .../apis/agents/update_agent_tags.ts          | 78 +++++++++++++------
 2 files changed, 55 insertions(+), 25 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
index 7687810db6c18..69b76603a2f34 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
@@ -113,7 +113,7 @@ export async function updateTagsBatch(
     const extraFilters = [];
     if (options.tagsToAdd.length === 1 && options.tagsToRemove.length === 0) {
       extraFilters.push(`NOT (tags:${options.tagsToAdd[0]})`);
-    } else if (options.tagsToRemove.length === 1) {
+    } else if (options.tagsToRemove.length === 1 && options.tagsToAdd.length === 0) {
       extraFilters.push(`tags:${options.tagsToRemove[0]}`);
     }
     query = getElasticsearchQuery(options.kuery, false, false, hostedIds, extraFilters);
diff --git a/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts b/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts
index ed3a147fe8ec7..e47d322c726f1 100644
--- a/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts
+++ b/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts
@@ -87,29 +87,11 @@ export default function (providerContext: FtrProviderContext) {
         });
       });
 
-      it('should bulk update tags of multiple agents by kuery in batches', async () => {
-        const { body: actionBody } = await supertest
-          .post(`/api/fleet/agents/bulk_update_agent_tags`)
-          .set('kbn-xsrf', 'xxx')
-          .send({
-            agents: 'active: true',
-            tagsToAdd: ['newTag'],
-            tagsToRemove: ['existingTag'],
-            batchSize: 3,
-          })
-          .expect(200);
-
-        const actionId = actionBody.actionId;
-
-        const verifyActionResult = async () => {
-          const { body } = await supertest.get(`/api/fleet/agents`).set('kbn-xsrf', 'xxx');
-          expect(body.total).to.eql(4);
-          body.items.forEach((agent: any) => {
-            expect(agent.tags.includes('newTag')).to.be(true);
-            expect(agent.tags.includes('existingTag')).to.be(false);
-          });
-        };
-
+      async function pollResult(
+        actionId: string,
+        nbAgentsAck: number,
+        verifyActionResult: Function
+      ) {
         await new Promise((resolve, reject) => {
           let attempts = 0;
           const intervalId = setInterval(async () => {
@@ -122,7 +104,7 @@ export default function (providerContext: FtrProviderContext) {
               body: { items: actionStatuses },
             } = await supertest.get(`/api/fleet/agents/action_status`).set('kbn-xsrf', 'xxx');
             const action = actionStatuses.find((a: any) => a.actionId === actionId);
-            if (action && action.nbAgentsAck === 4) {
+            if (action && action.nbAgentsAck === nbAgentsAck) {
               clearInterval(intervalId);
               await verifyActionResult();
               resolve({});
@@ -131,6 +113,54 @@ export default function (providerContext: FtrProviderContext) {
         }).catch((e) => {
           throw e;
         });
+      }
+
+      it('should bulk update tags of multiple agents by kuery in batches - add', async () => {
+        const { body: actionBody } = await supertest
+          .post(`/api/fleet/agents/bulk_update_agent_tags`)
+          .set('kbn-xsrf', 'xxx')
+          .send({
+            agents: 'active: true',
+            tagsToAdd: ['newTag'],
+            tagsToRemove: [],
+            batchSize: 3,
+          })
+          .expect(200);
+
+        const actionId = actionBody.actionId;
+
+        const verifyActionResult = async () => {
+          const { body } = await supertest
+            .get(`/api/fleet/agents?kuery=tags:newTag`)
+            .set('kbn-xsrf', 'xxx');
+          expect(body.total).to.eql(4);
+        };
+
+        await pollResult(actionId, 4, verifyActionResult);
+      });
+
+      it('should bulk update tags of multiple agents by kuery in batches - remove', async () => {
+        const { body: actionBody } = await supertest
+          .post(`/api/fleet/agents/bulk_update_agent_tags`)
+          .set('kbn-xsrf', 'xxx')
+          .send({
+            agents: 'active: true',
+            tagsToAdd: [],
+            tagsToRemove: ['existingTag'],
+            batchSize: 3,
+          })
+          .expect(200);
+
+        const actionId = actionBody.actionId;
+
+        const verifyActionResult = async () => {
+          const { body } = await supertest
+            .get(`/api/fleet/agents?kuery=tags:existingTag`)
+            .set('kbn-xsrf', 'xxx');
+          expect(body.total).to.eql(0);
+        };
+
+        await pollResult(actionId, 2, verifyActionResult);
       });
 
       it('should return a 403 if user lacks fleet all permissions', async () => {

From ed60158f795d202fcef38ce1fab900d474aebffe Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Fri, 16 Dec 2022 11:06:06 +0100
Subject: [PATCH 03/11] fix action results on retry

---
 .../services/agents/update_agent_tags_action_runner.ts      | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
index 69b76603a2f34..1be5ccb17e73e 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
@@ -181,7 +181,7 @@ export async function updateTagsBatch(
     });
   }
 
-  // creating unique 0...n ids to use as agentId, as we don't have all agent ids in case of action by kuery
+  // creating unique ids to use as agentId, as we don't have all agent ids in case of action by kuery
   const getArray = (count: number) => [...Array(count).keys()];
 
   // writing successful action results
@@ -189,8 +189,8 @@ export async function updateTagsBatch(
     await bulkCreateAgentActionResults(
       esClient,
 
-      (options.kuery === undefined ? agentIds : getArray(res.updated!)).map((id) => ({
-        agentId: id + '',
+      (options.kuery === undefined ? agentIds : getArray(res.updated!)).map(() => ({
+        agentId: uuid(),
         actionId,
       }))
     );

From 2c3660df0acab69d7ceb9d9fc43b7a05773975fb Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Fri, 16 Dec 2022 11:26:07 +0100
Subject: [PATCH 04/11] fix and test

---
 .../services/agents/update_agent_tags.test.ts | 21 +++++++++++++++++++
 .../agents/update_agent_tags_action_runner.ts |  7 +++----
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
index c1170c5055043..1998f346dc726 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
@@ -112,6 +112,27 @@ describe('update_agent_tags', () => {
     expect(actionResults.body[1].error).not.toBeDefined();
   });
 
+  it('should update action results on success - kuery', async () => {
+    await updateTagsBatch(
+      soClient,
+      esClient,
+      [],
+      {},
+      {
+        tagsToAdd: ['new'],
+        tagsToRemove: [],
+        kuery: '',
+      }
+    );
+
+    const actionResults = esClient.bulk.mock.calls[0][0] as any;
+    const agentIds = actionResults?.body
+      ?.filter((i: any) => i.agent_id)
+      .map((i: any) => i.agent_id);
+    expect(agentIds[0]).toHaveLength(36); // uuid
+    expect(actionResults.body[1].error).not.toBeDefined();
+  });
+
   it('should write error action results for hosted agent when agentIds are passed', async () => {
     const { esClient: esClientMock, agentInHostedDoc } = createClientMock();
 
diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
index 1be5ccb17e73e..4e757e92624ec 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
@@ -182,15 +182,14 @@ export async function updateTagsBatch(
   }
 
   // creating unique ids to use as agentId, as we don't have all agent ids in case of action by kuery
-  const getArray = (count: number) => [...Array(count).keys()];
+  const getArray = (count: number) => Array.from({ length: count }, () => uuid());
 
   // writing successful action results
   if (res.updated ?? 0 > 0) {
     await bulkCreateAgentActionResults(
       esClient,
-
-      (options.kuery === undefined ? agentIds : getArray(res.updated!)).map(() => ({
-        agentId: uuid(),
+      (options.kuery === undefined ? agentIds : getArray(res.updated!)).map((id) => ({
+        agentId: id,
         actionId,
       }))
     );

From 3db4cb624424bec0550f0630197cd53b711bf36b Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Fri, 16 Dec 2022 14:31:05 +0100
Subject: [PATCH 05/11] use doc count in /action_status for udpate tags

---
 .../fleet/server/services/agents/action_status.ts | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/agents/action_status.ts b/x-pack/plugins/fleet/server/services/agents/action_status.ts
index 5c6753425cbc7..b34d699eb3b33 100644
--- a/x-pack/plugins/fleet/server/services/agents/action_status.ts
+++ b/x-pack/plugins/fleet/server/services/agents/action_status.ts
@@ -69,12 +69,15 @@ export async function getActionStatuses(
     const nbAgentsActioned = action.nbAgentsActioned || action.nbAgentsActionCreated;
     const cardinalityCount = (matchingBucket?.agent_count as any)?.value ?? 0;
     const docCount = matchingBucket?.doc_count ?? 0;
-    const nbAgentsAck = Math.min(
-      docCount,
-      // only using cardinality count when count lower than precision threshold
-      docCount > PRECISION_THRESHOLD ? docCount : cardinalityCount,
-      nbAgentsActioned
-    );
+    const nbAgentsAck =
+      action.type === 'UPDATE_TAGS'
+        ? docCount
+        : Math.min(
+            docCount,
+            // only using cardinality count when count lower than precision threshold
+            docCount > PRECISION_THRESHOLD ? docCount : cardinalityCount,
+            nbAgentsActioned
+          );
     const completionTime = (matchingBucket?.max_timestamp as any)?.value_as_string;
     const complete = nbAgentsAck >= nbAgentsActioned;
     const cancelledAction = cancelledActions.find((a) => a.actionId === action.actionId);

From 0edb4320041ded903810d147def6248020868f1b Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Fri, 16 Dec 2022 14:39:33 +0100
Subject: [PATCH 06/11] write error results on last retry with version
 conflicts

---
 .../services/agents/update_agent_tags.test.ts | 27 +++++++++++++++++++
 .../agents/update_agent_tags_action_runner.ts | 17 +++++++++---
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
index 1998f346dc726..50d462023a8f6 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
@@ -188,6 +188,33 @@ describe('update_agent_tags', () => {
     ).rejects.toThrowError('version conflict of 100 agents');
   });
 
+  it('should write out error results on last retry with version conflicts', async () => {
+    esClient.updateByQuery.mockReset();
+    esClient.updateByQuery.mockResolvedValue({
+      failures: [],
+      updated: 0,
+      version_conflicts: 100,
+    } as any);
+
+    await expect(
+      updateTagsBatch(
+        soClient,
+        esClient,
+        [],
+        {},
+        {
+          tagsToAdd: ['new'],
+          tagsToRemove: [],
+          kuery: '',
+          total: 100,
+          retryCount: 3,
+        }
+      )
+    ).rejects.toThrowError('version conflict of 100 agents');
+    const errorResults = esClient.bulk.mock.calls[0][0] as any;
+    expect(errorResults.body[1].error).toEqual('version conflict on 3rd retry');
+  });
+
   it('should run add tags async when actioning more agents than batch size', async () => {
     esClient.search.mockResolvedValue({
       hits: {
diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
index 4e757e92624ec..452ffc92ad416 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
@@ -63,7 +63,7 @@ export class UpdateAgentTagsActionRunner extends ActionRunner {
         actionId: this.actionParams.actionId,
         total: this.actionParams.total,
         kuery: this.actionParams.kuery,
-        isRetry: !!this.retryParams.retryCount,
+        retryCount: this.retryParams.retryCount,
       }
     );
 
@@ -83,7 +83,7 @@ export async function updateTagsBatch(
     actionId?: string;
     total?: number;
     kuery?: string;
-    isRetry?: boolean;
+    retryCount?: number;
   }
 ): Promise<{ actionId: string; updated?: number; took?: number }> {
   const errors: Record<Agent['id'], Error> = { ...outgoingErrors };
@@ -169,7 +169,7 @@ export async function updateTagsBatch(
   const actionId = options.actionId ?? uuid();
   const total = options.total ?? givenAgents.length;
 
-  if (!options.isRetry) {
+  if (options.retryCount === undefined) {
     // creating an action doc so that update tags  shows up in activity
     await createAgentAction(esClient, {
       id: actionId,
@@ -220,6 +220,17 @@ export async function updateTagsBatch(
   }
 
   if (res.version_conflicts ?? 0 > 0) {
+    // write out error results on last retry, so action is not stuck in progress
+    if (options.retryCount === 3) {
+      await bulkCreateAgentActionResults(
+        esClient,
+        getArray(res.version_conflicts!).map((id) => ({
+          agentId: id,
+          actionId,
+          error: 'version conflict on 3rd retry',
+        }))
+      );
+    }
     throw new Error(`version conflict of ${res.version_conflicts} agents`);
   }
 

From 4c25056d5d54540b43a4ac94346d583eec7cfd10 Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Fri, 16 Dec 2022 15:46:27 +0100
Subject: [PATCH 07/11] removed hosted agents error from udpate tags without
 kuery too

---
 .../server/services/agents/action_status.ts      |  2 +-
 .../services/agents/update_agent_tags.test.ts    | 13 ++++++-------
 .../agents/update_agent_tags_action_runner.ts    | 16 +---------------
 3 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/agents/action_status.ts b/x-pack/plugins/fleet/server/services/agents/action_status.ts
index b34d699eb3b33..c36e13a4441ca 100644
--- a/x-pack/plugins/fleet/server/services/agents/action_status.ts
+++ b/x-pack/plugins/fleet/server/services/agents/action_status.ts
@@ -71,7 +71,7 @@ export async function getActionStatuses(
     const docCount = matchingBucket?.doc_count ?? 0;
     const nbAgentsAck =
       action.type === 'UPDATE_TAGS'
-        ? docCount
+        ? Math.min(docCount, nbAgentsActioned)
         : Math.min(
             docCount,
             // only using cardinality count when count lower than precision threshold
diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
index 50d462023a8f6..c357ed0e11edf 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.test.ts
@@ -92,6 +92,9 @@ describe('update_agent_tags', () => {
   });
 
   it('should update action results on success', async () => {
+    esClient.updateByQuery.mockReset();
+    esClient.updateByQuery.mockResolvedValue({ failures: [], updated: 1, total: 1 } as any);
+
     await updateAgentTags(soClient, esClient, { agentIds: ['agent1'] }, ['one'], []);
 
     const agentAction = esClient.create.mock.calls[0][0] as any;
@@ -133,11 +136,11 @@ describe('update_agent_tags', () => {
     expect(actionResults.body[1].error).not.toBeDefined();
   });
 
-  it('should write error action results for hosted agent when agentIds are passed', async () => {
+  it('should skip hosted agent from total when agentIds are passed', async () => {
     const { esClient: esClientMock, agentInHostedDoc } = createClientMock();
 
     esClientMock.updateByQuery.mockReset();
-    esClientMock.updateByQuery.mockResolvedValue({ failures: [], updated: 0, total: '0' } as any);
+    esClientMock.updateByQuery.mockResolvedValue({ failures: [], updated: 0, total: 0 } as any);
 
     await updateAgentTags(
       soClient,
@@ -153,13 +156,9 @@ describe('update_agent_tags', () => {
         action_id: expect.anything(),
         agents: [],
         type: 'UPDATE_TAGS',
-        total: 1,
+        total: 0,
       })
     );
-
-    const errorResults = esClientMock.bulk.mock.calls[0][0] as any;
-    expect(errorResults.body[1].agent_id).toEqual(agentInHostedDoc._id);
-    expect(errorResults.body[1].error).toEqual('Cannot modify tags on a hosted agent');
   });
 
   it('should write error action results when failures are returned', async () => {
diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
index 452ffc92ad416..f2bc62dd2784d 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
@@ -167,7 +167,6 @@ export async function updateTagsBatch(
   appContextService.getLogger().debug(JSON.stringify(res).slice(0, 1000));
 
   const actionId = options.actionId ?? uuid();
-  const total = options.total ?? givenAgents.length;
 
   if (options.retryCount === undefined) {
     // creating an action doc so that update tags  shows up in activity
@@ -176,8 +175,7 @@ export async function updateTagsBatch(
       agents: options.kuery === undefined ? agentIds : [],
       created_at: new Date().toISOString(),
       type: 'UPDATE_TAGS',
-      // for kuery cases, the total can be less than the initial selection, as we filter out tags that are already added/removed
-      total: options.kuery === undefined ? total : res.total,
+      total: res.total,
     });
   }
 
@@ -207,18 +205,6 @@ export async function updateTagsBatch(
     );
   }
 
-  // writing hosted agent errors - hosted agents filtered out
-  if (options.kuery === undefined && hostedAgentIds.length > 0) {
-    await bulkCreateAgentActionResults(
-      esClient,
-      hostedAgentIds.map((id) => ({
-        agentId: id + '',
-        actionId,
-        error: hostedAgentError,
-      }))
-    );
-  }
-
   if (res.version_conflicts ?? 0 > 0) {
     // write out error results on last retry, so action is not stuck in progress
     if (options.retryCount === 3) {

From 1e12eee2698c58e8bfd1ffa6fa1442f7024d0128 Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Fri, 16 Dec 2022 16:05:58 +0100
Subject: [PATCH 08/11] fixed types

---
 .../services/agents/update_agent_tags_action_runner.ts       | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
index f2bc62dd2784d..4e87b93943565 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
@@ -96,11 +96,6 @@ export async function updateTagsBatch(
     hostedAgentError
   );
   const agentIds = filteredAgents.map((agent) => agent.id);
-  const hostedAgentIds = givenAgents
-    .filter(
-      (agent) => filteredAgents.find((filteredAgent) => filteredAgent.id === agent.id) === undefined
-    )
-    .map((agent) => agent.id);
 
   let query: estypes.QueryDslQueryContainer | undefined;
   if (options.kuery !== undefined) {

From b003d9b648070c316362f2aa8bd3dfe9028c4c4e Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Mon, 19 Dec 2022 10:55:03 +0100
Subject: [PATCH 09/11] fixed integration test

---
 .../fleet_api_integration/apis/agents/update_agent_tags.ts    | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts b/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts
index e47d322c726f1..65bf277d17ae6 100644
--- a/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts
+++ b/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts
@@ -210,8 +210,8 @@ export default function (providerContext: FtrProviderContext) {
           .get(`/api/fleet/agents/action_status`)
           .set('kbn-xsrf', 'xxx');
         const actionStatus = body.items[0];
-        expect(actionStatus.status).to.eql('FAILED');
-        expect(actionStatus.nbAgentsFailed).to.eql(1);
+        expect(actionStatus.status).to.eql('COMPLETE');
+        expect(actionStatus.nbAgentsAck).to.eql(1);
       });
     });
   });

From 53e670b27e1d448dad6507d5aae53587ebd2d5e0 Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Mon, 19 Dec 2022 12:28:35 +0100
Subject: [PATCH 10/11] simplified logic to use retry for all kuery actions

---
 .../services/agents/update_agent_tags.ts      | 41 ++++++-------------
 .../apis/agents/update_agent_tags.ts          | 25 +----------
 2 files changed, 15 insertions(+), 51 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.ts
index 79636ecae1015..dad2053f7ed59 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags.ts
@@ -11,9 +11,7 @@ import type { ElasticsearchClient, SavedObjectsClientContract } from '@kbn/core/
 import type { Agent } from '../../types';
 import { AgentReassignmentError } from '../../errors';
 
-import { SO_SEARCH_LIMIT } from '../../constants';
-
-import { getAgentDocuments, getAgentsByKuery } from './crud';
+import { getAgentDocuments } from './crud';
 import type { GetAgentsOptions } from '.';
 import { searchHitToAgent } from './helpers';
 import { UpdateAgentTagsActionRunner, updateTagsBatch } from './update_agent_tags_action_runner';
@@ -30,7 +28,7 @@ export async function updateAgentTags(
   tagsToRemove: string[]
 ): Promise<{ actionId: string }> {
   const outgoingErrors: Record<Agent['id'], Error> = {};
-  let givenAgents: Agent[] = [];
+  const givenAgents: Agent[] = [];
 
   if ('agentIds' in options) {
     const givenAgentsResults = await getAgentDocuments(esClient, options.agentIds);
@@ -44,30 +42,17 @@ export async function updateAgentTags(
       }
     }
   } else if ('kuery' in options) {
-    const batchSize = options.batchSize ?? SO_SEARCH_LIMIT;
-    const res = await getAgentsByKuery(esClient, {
-      kuery: options.kuery,
-      showInactive: options.showInactive ?? false,
-      page: 1,
-      perPage: batchSize,
-    });
-    if (res.total <= batchSize) {
-      givenAgents = res.agents;
-    } else {
-      return await new UpdateAgentTagsActionRunner(
-        esClient,
-        soClient,
-        {
-          ...options,
-          batchSize,
-          total: res.total,
-          kuery: options.kuery,
-          tagsToAdd,
-          tagsToRemove,
-        },
-        { pitId: '' }
-      ).runActionAsyncWithRetry();
-    }
+    return await new UpdateAgentTagsActionRunner(
+      esClient,
+      soClient,
+      {
+        ...options,
+        kuery: options.kuery,
+        tagsToAdd,
+        tagsToRemove,
+      },
+      { pitId: '' }
+    ).runActionAsyncWithRetry();
   }
 
   return await updateTagsBatch(soClient, esClient, givenAgents, outgoingErrors, {
diff --git a/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts b/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts
index 65bf277d17ae6..05c1a1f8ae6c7 100644
--- a/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts
+++ b/x-pack/test/fleet_api_integration/apis/agents/update_agent_tags.ts
@@ -68,25 +68,6 @@ export default function (providerContext: FtrProviderContext) {
         expect(agent2data.body.item.tags).to.eql(['existingTag']);
       });
 
-      it('should allow to update tags of multiple agents by kuery', async () => {
-        await supertest
-          .post(`/api/fleet/agents/bulk_update_agent_tags`)
-          .set('kbn-xsrf', 'xxx')
-          .send({
-            agents: 'active: true',
-            tagsToAdd: ['newTag'],
-            tagsToRemove: ['existingTag'],
-          })
-          .expect(200);
-
-        const { body } = await supertest.get(`/api/fleet/agents`).set('kbn-xsrf', 'xxx');
-        expect(body.total).to.eql(4);
-        body.items.forEach((agent: any) => {
-          expect(agent.tags.includes('newTag')).to.be(true);
-          expect(agent.tags.includes('existingTag')).to.be(false);
-        });
-      });
-
       async function pollResult(
         actionId: string,
         nbAgentsAck: number,
@@ -115,7 +96,7 @@ export default function (providerContext: FtrProviderContext) {
         });
       }
 
-      it('should bulk update tags of multiple agents by kuery in batches - add', async () => {
+      it('should bulk update tags of multiple agents by kuery - add', async () => {
         const { body: actionBody } = await supertest
           .post(`/api/fleet/agents/bulk_update_agent_tags`)
           .set('kbn-xsrf', 'xxx')
@@ -123,7 +104,6 @@ export default function (providerContext: FtrProviderContext) {
             agents: 'active: true',
             tagsToAdd: ['newTag'],
             tagsToRemove: [],
-            batchSize: 3,
           })
           .expect(200);
 
@@ -139,7 +119,7 @@ export default function (providerContext: FtrProviderContext) {
         await pollResult(actionId, 4, verifyActionResult);
       });
 
-      it('should bulk update tags of multiple agents by kuery in batches - remove', async () => {
+      it('should bulk update tags of multiple agents by kuery - remove', async () => {
         const { body: actionBody } = await supertest
           .post(`/api/fleet/agents/bulk_update_agent_tags`)
           .set('kbn-xsrf', 'xxx')
@@ -147,7 +127,6 @@ export default function (providerContext: FtrProviderContext) {
             agents: 'active: true',
             tagsToAdd: [],
             tagsToRemove: ['existingTag'],
-            batchSize: 3,
           })
           .expect(200);
 

From 7fe37bb3be1cdb2f1666b072c6bf1671a83eb715 Mon Sep 17 00:00:00 2001
From: Julia Bardi <julia.bardi@elastic.co>
Date: Tue, 20 Dec 2022 09:36:31 +0100
Subject: [PATCH 11/11] fix review comments

---
 .../fleet/server/services/agents/action_runner.ts      |  6 ++++--
 .../services/agents/update_agent_tags_action_runner.ts | 10 +++++-----
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/agents/action_runner.ts b/x-pack/plugins/fleet/server/services/agents/action_runner.ts
index 18af331980238..83b61a340bfed 100644
--- a/x-pack/plugins/fleet/server/services/agents/action_runner.ts
+++ b/x-pack/plugins/fleet/server/services/agents/action_runner.ts
@@ -22,6 +22,8 @@ import { getAgentActions } from './actions';
 import { closePointInTime, getAgentsByKuery } from './crud';
 import type { BulkActionsResolver } from './bulk_actions_resolver';
 
+export const MAX_RETRY_COUNT = 3;
+
 export interface ActionParams {
   kuery: string;
   showInactive?: boolean;
@@ -110,8 +112,8 @@ export abstract class ActionRunner {
                 `Retry #${this.retryParams.retryCount} of task ${this.retryParams.taskId} failed: ${error.message}`
               );
 
-            if (this.retryParams.retryCount === 3) {
-              const errorMessage = 'Stopping after 3rd retry. Error: ' + error.message;
+            if (this.retryParams.retryCount === MAX_RETRY_COUNT) {
+              const errorMessage = `Stopping after ${MAX_RETRY_COUNT}rd retry. Error: ${error.message}`;
               appContextService.getLogger().warn(errorMessage);
 
               // clean up tasks after 3rd retry reached
diff --git a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
index 4e87b93943565..af538260bb163 100644
--- a/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
+++ b/x-pack/plugins/fleet/server/services/agents/update_agent_tags_action_runner.ts
@@ -20,7 +20,7 @@ import { agentPolicyService } from '../agent_policy';
 
 import { SO_SEARCH_LIMIT } from '../../../common/constants';
 
-import { ActionRunner } from './action_runner';
+import { ActionRunner, MAX_RETRY_COUNT } from './action_runner';
 
 import { BulkActionTaskType } from './bulk_actions_resolver';
 import { filterHostedPolicies } from './filter_hosted_agents';
@@ -175,13 +175,13 @@ export async function updateTagsBatch(
   }
 
   // creating unique ids to use as agentId, as we don't have all agent ids in case of action by kuery
-  const getArray = (count: number) => Array.from({ length: count }, () => uuid());
+  const getUuidArray = (count: number) => Array.from({ length: count }, () => uuid());
 
   // writing successful action results
   if (res.updated ?? 0 > 0) {
     await bulkCreateAgentActionResults(
       esClient,
-      (options.kuery === undefined ? agentIds : getArray(res.updated!)).map((id) => ({
+      (options.kuery === undefined ? agentIds : getUuidArray(res.updated!)).map((id) => ({
         agentId: id,
         actionId,
       }))
@@ -202,10 +202,10 @@ export async function updateTagsBatch(
 
   if (res.version_conflicts ?? 0 > 0) {
     // write out error results on last retry, so action is not stuck in progress
-    if (options.retryCount === 3) {
+    if (options.retryCount === MAX_RETRY_COUNT) {
       await bulkCreateAgentActionResults(
         esClient,
-        getArray(res.version_conflicts!).map((id) => ({
+        getUuidArray(res.version_conflicts!).map((id) => ({
           agentId: id,
           actionId,
           error: 'version conflict on 3rd retry',