[Fleet] Prevent agent upgrade requests when already upgrading (#170963)

## Summary

Closes https://github.com/elastic/kibana/issues/168171

Prevent upgrading an agent if it is already upgrading.

Agents that report upgrade details are considered as upgrading when the
`upgrade_details` field exists and is not equal to `UPG_FAILED` (cf.
[this
comment](https://github.com/elastic/kibana/issues/168171#issuecomment-1788409967)).

Agents that do _not_ report upgrade details are considered as upgrading
when the `upgrade_started_at` field is set and the `upgraded_at` field
is not. NB: this is existing behaviour, this PR does not change this
logic.

### Checklist

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Jill Guyonnet 2023-11-16 11:12:55 +01:00 committed by GitHub
parent 19e6a4ba06
commit 01ab7ea6c9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 292 additions and 15 deletions

View file

@ -5,23 +5,29 @@
* 2.0.
*/
import type { Agent } from '../types/models/agent';
import type { Agent, AgentUpgradeDetails } from '../types/models/agent';
import { getRecentUpgradeInfoForAgent, isAgentUpgradeable } from './is_agent_upgradeable';
import {
getRecentUpgradeInfoForAgent,
isAgentUpgradeable,
isAgentUpgrading,
} from './is_agent_upgradeable';
const getAgent = ({
version,
upgradeable = false,
unenrolling = false,
unenrolled = false,
updating = false,
upgrading = false,
upgradeDetails,
minutesSinceUpgrade,
}: {
version: string;
upgradeable?: boolean;
unenrolling?: boolean;
unenrolled?: boolean;
updating?: boolean;
upgrading?: boolean;
upgradeDetails?: AgentUpgradeDetails;
minutesSinceUpgrade?: number;
}): Agent => {
const agent: Agent = {
@ -98,9 +104,12 @@ const getAgent = ({
if (unenrolled) {
agent.unenrolled_at = '2020-10-01T14:43:27.255Z';
}
if (updating) {
if (upgrading) {
agent.upgrade_started_at = new Date(Date.now()).toISOString();
}
if (upgradeDetails) {
agent.upgrade_details = upgradeDetails;
}
if (minutesSinceUpgrade) {
agent.upgraded_at = new Date(Date.now() - minutesSinceUpgrade * 6e4).toISOString();
}
@ -171,11 +180,49 @@ describe('Fleet - isAgentUpgradeable', () => {
isAgentUpgradeable(getAgent({ version: '7.9.0', upgradeable: true }), '8.0.0', '7.9.0')
).toBe(false);
});
it('returns false if agent reports upgradeable, but is already updating', () => {
it('returns false if agent with no upgrade details reports upgradeable, but is already upgrading', () => {
expect(
isAgentUpgradeable(getAgent({ version: '7.9.0', upgradeable: true, updating: true }), '8.0.0')
isAgentUpgradeable(
getAgent({ version: '7.9.0', upgradeable: true, upgrading: true }),
'8.0.0'
)
).toBe(false);
});
it('returns false if agent reports upgradeable, but has an upgrade status other than failed', () => {
expect(
isAgentUpgradeable(
getAgent({
version: '7.9.0',
upgradeable: true,
upgradeDetails: {
target_version: '8.0.0',
action_id: 'XXX',
state: 'UPG_REQUESTED',
},
}),
'8.0.0'
)
).toBe(false);
});
it('returns true if agent reports upgradeable and has a failed upgrade status', () => {
expect(
isAgentUpgradeable(
getAgent({
version: '7.9.0',
upgradeable: true,
upgradeDetails: {
target_version: '8.0.0',
action_id: 'XXX',
state: 'UPG_FAILED',
metadata: {
error_msg: 'Upgrade timed out',
},
},
}),
'8.0.0'
)
).toBe(true);
});
it('returns false if the agent reports upgradeable but was upgraded less than 10 minutes ago', () => {
expect(
isAgentUpgradeable(
@ -215,3 +262,49 @@ describe('hasAgentBeenUpgradedRecently', () => {
).toBe(false);
});
});
describe('isAgentUpgrading', () => {
it('returns true if the agent has an upgrade status other than failed', () => {
expect(
isAgentUpgrading(
getAgent({
version: '7.9.0',
upgradeDetails: {
target_version: '8.0.0',
action_id: 'XXX',
state: 'UPG_REQUESTED',
},
})
)
).toBe(true);
});
it('returns false if the agent has a failed upgrade status', () => {
expect(
isAgentUpgrading(
getAgent({
version: '7.9.0',
upgradeDetails: {
target_version: '8.0.0',
action_id: 'XXX',
state: 'UPG_FAILED',
metadata: {
error_msg: 'Upgrade timed out',
},
},
})
)
).toBe(false);
});
it('returns true if the agent is upgrading but has no upgrade details', () => {
expect(
isAgentUpgrading(
getAgent({
version: '7.9.0',
upgrading: true,
})
)
).toBe(true);
});
});

View file

@ -30,11 +30,9 @@ export function isAgentUpgradeable(
if (!agent.local_metadata.elastic.agent.upgradeable) {
return false;
}
// check that the agent is not already in the process of updating
if (agent.upgrade_started_at && !agent.upgraded_at) {
if (isAgentUpgrading(agent)) {
return false;
}
// check that the agent has not been upgraded more recently than the monitoring period
if (getRecentUpgradeInfoForAgent(agent).hasBeenUpgradedRecently) {
return false;
}
@ -80,3 +78,10 @@ export function getRecentUpgradeInfoForAgent(agent: Agent): {
return { hasBeenUpgradedRecently, timeToWaitMs };
}
export function isAgentUpgrading(agent: Agent) {
if (agent.upgrade_details) {
return agent.upgrade_details.state !== 'UPG_FAILED';
}
return agent.upgrade_started_at && !agent.upgraded_at;
}

View file

@ -443,7 +443,7 @@ export interface AgentUpgradeDetails {
target_version: string;
action_id: string;
state: AgentUpgradeStateType;
metadata: {
metadata?: {
scheduled_at?: string;
download_percent?: number;
failed_state?: AgentUpgradeStateType;

View file

@ -77,7 +77,7 @@ function getStatusComponents(agentUpgradeDetails?: AgentUpgradeDetails) {
id="xpack.fleet.agentUpgradeStatusTooltip.upgradeScheduled"
defaultMessage="The agent has been instructed to upgrade.{upgradeStartDelay}"
values={{
upgradeStartDelay: getUpgradeStartDelay(agentUpgradeDetails.metadata.scheduled_at),
upgradeStartDelay: getUpgradeStartDelay(agentUpgradeDetails.metadata?.scheduled_at),
}}
/>
),
@ -97,7 +97,9 @@ function getStatusComponents(agentUpgradeDetails?: AgentUpgradeDetails) {
id="xpack.fleet.agentUpgradeStatusTooltip.upgradeDownloading"
defaultMessage="Downloading the new agent artifact version{downloadEstimate}."
values={{
downloadEstimate: getDownloadEstimate(agentUpgradeDetails?.metadata.download_percent),
downloadEstimate: getDownloadEstimate(
agentUpgradeDetails?.metadata?.download_percent
),
}}
/>
),
@ -202,7 +204,7 @@ function getStatusComponents(agentUpgradeDetails?: AgentUpgradeDetails) {
id="xpack.fleet.agentUpgradeStatusTooltip.upgradeFailed"
defaultMessage="Upgrade failed: {errorMsg}."
values={{
errorMsg: agentUpgradeDetails?.metadata.error_msg,
errorMsg: agentUpgradeDetails?.metadata?.error_msg,
}}
/>
),

View file

@ -24,6 +24,7 @@ import {
getRecentUpgradeInfoForAgent,
isAgentUpgradeable,
AGENT_UPGRADE_COOLDOWN_IN_MIN,
isAgentUpgrading,
} from '../../../common/services';
import { getMaxVersion } from '../../../common/services/get_min_max_version';
import { getAgentById } from '../../services/agents';
@ -99,6 +100,16 @@ export const postAgentUpgradeHandler: RequestHandler<
},
});
}
if (!force && isAgentUpgrading(agent)) {
return response.customError({
statusCode: 400,
body: {
message: `agent ${request.params.agentId} is already upgrading`,
},
});
}
if (!force && !isAgentUpgradeable(agent, latestAgentVersion, version)) {
return response.customError({
statusCode: 400,

View file

@ -76,7 +76,12 @@ export async function upgradeBatch(
const latestAgentVersion = await getLatestAvailableVersion();
const upgradeableResults = await Promise.allSettled(
agentsToCheckUpgradeable.map(async (agent) => {
// Filter out agents currently unenrolling, unenrolled, recently upgraded or not upgradeable b/c of version check
// Filter out agents that are:
// - currently unenrolling
// - unenrolled
// - recently upgraded
// - currently upgrading
// - upgradeable b/c of version check
const isNotAllowed =
getRecentUpgradeInfoForAgent(agent).hasBeenUpgradedRecently ||
(!options.force && !isAgentUpgradeable(agent, latestAgentVersion, options.version));

View file

@ -443,6 +443,167 @@ export default function (providerContext: FtrProviderContext) {
})
.expect(200);
});
it('should respond 400 if trying to upgrade an already upgrading agent with no upgrade details', async () => {
await es.update({
id: 'agent1',
refresh: 'wait_for',
index: AGENTS_INDEX,
body: {
doc: {
upgrade_started_at: new Date(Date.now() - 9 * 6e4).toISOString(),
local_metadata: {
elastic: {
agent: {
upgradeable: true,
version: '0.0.0',
},
},
},
},
},
});
const response = await supertest
.post(`/api/fleet/agents/agent1/upgrade`)
.set('kbn-xsrf', 'xxx')
.send({
version: fleetServerVersion,
})
.expect(400);
expect(response.body.message).to.contain('is already upgrading');
});
it('should respond 200 if trying to upgrade an already upgrading agent with no upgrade details with force flag', async () => {
await es.update({
id: 'agent1',
refresh: 'wait_for',
index: AGENTS_INDEX,
body: {
doc: {
upgrade_started_at: new Date(Date.now() - 9 * 6e4).toISOString(),
local_metadata: {
elastic: {
agent: {
upgradeable: true,
version: '0.0.0',
},
},
},
},
},
});
await supertest
.post(`/api/fleet/agents/agent1/upgrade`)
.set('kbn-xsrf', 'xxx')
.send({
version: fleetServerVersion,
force: true,
})
.expect(200);
});
it('should respond 400 if trying to upgrade an already upgrading agent with upgrade details', async () => {
await es.update({
id: 'agent1',
refresh: 'wait_for',
index: AGENTS_INDEX,
body: {
doc: {
local_metadata: {
elastic: {
agent: {
upgradeable: true,
version: '0.0.0',
},
},
},
upgrade_details: {
target_version: fleetServerVersion,
action_id: 'XXX',
state: 'UPG_REQUESTED',
},
},
},
});
const response = await supertest
.post(`/api/fleet/agents/agent1/upgrade`)
.set('kbn-xsrf', 'xxx')
.send({
version: fleetServerVersion,
})
.expect(400);
expect(response.body.message).to.contain('is already upgrading');
});
it('should respond 200 if trying to upgrade an already upgrading agent with upgrade details with force flag', async () => {
await es.update({
id: 'agent1',
refresh: 'wait_for',
index: AGENTS_INDEX,
body: {
doc: {
local_metadata: {
elastic: {
agent: {
upgradeable: true,
version: '0.0.0',
},
},
},
upgrade_details: {
target_version: fleetServerVersion,
action_id: 'XXX',
state: 'UPG_REQUESTED',
},
},
},
});
await supertest
.post(`/api/fleet/agents/agent1/upgrade`)
.set('kbn-xsrf', 'xxx')
.send({
version: fleetServerVersion,
force: true,
})
.expect(200);
});
it('should respond 200 if trying to upgrade an agent with a failed upgrade status', async () => {
await es.update({
id: 'agent1',
refresh: 'wait_for',
index: AGENTS_INDEX,
body: {
doc: {
local_metadata: {
elastic: {
agent: {
upgradeable: true,
version: '0.0.0',
},
},
},
upgrade_details: {
target_version: fleetServerVersion,
action_id: 'XXX',
state: 'UPG_FAILED',
metadata: {
error_msg: 'Upgrade timed out',
},
},
},
},
});
await supertest
.post(`/api/fleet/agents/agent1/upgrade`)
.set('kbn-xsrf', 'xxx')
.send({
version: fleetServerVersion,
})
.expect(200);
});
});
describe('multiple agents', () => {