diff --git a/forge/ee/routes/teamBroker/3rdPartyBroker.js b/forge/ee/routes/teamBroker/3rdPartyBroker.js index 481d9616c6..a1a65f7194 100644 --- a/forge/ee/routes/teamBroker/3rdPartyBroker.js +++ b/forge/ee/routes/teamBroker/3rdPartyBroker.js @@ -445,16 +445,30 @@ module.exports = async function (app) { }, async (request, reply) => { if (request.params.brokerId === 'team-broker') { const agent = await app.db.models.TeamBrokerAgent.byTeam(request.team.hashid) - if (agent && agent.state !== 'running') { + if (agent?.state === 'suspending') { + reply.status(409).send({ code: 'agent_suspending', error: 'The Team Broker agent is still suspending, try again shortly' }) + } else if (agent?.state === 'starting') { + // Another start is already launching the agent: starting is a no-op + reply.status(200).send({}) + } else if (agent && agent.state !== 'running') { await app.containers.sendBrokerAgentCommand(agent, 'start') reply.status(200).send({}) } else if (!agent) { + // Stays 'starting' until the driver has launched the agent, so a suspend + // arriving meanwhile is refused rather than missing a process it can't stop yet const agent = await app.db.models.TeamBrokerAgent.create({ - state: 'running', + state: 'starting', TeamId: request.team.id }) await agent.reload({ include: [{ model: app.db.models.Team }] }) - await app.containers.startBrokerAgent(agent) + try { + await app.containers.startBrokerAgent(agent) + } catch (err) { + await agent.destroy() + throw err + } + agent.state = 'running' + await agent.save() reply.status(200).send({}) } else { // Agent already running: starting is a no-op @@ -509,7 +523,13 @@ module.exports = async function (app) { }, async (request, reply) => { if (request.params.brokerId === 'team-broker') { const agent = await app.db.models.TeamBrokerAgent.byTeam(request.team.id) - if (agent) { + if (agent?.state === 'starting') { + reply.status(409).send({ code: 'agent_starting', error: 'The Team Broker agent is still starting, try again shortly' }) + } else if (agent) { + // Marked before stopping, so a start arriving before the row is removed is refused + // rather than treating the agent as already running + agent.state = 'suspending' + await agent.save() await app.containers.stopBrokerAgent(agent) setTimeout(async () => { await agent.destroy() diff --git a/test/unit/forge/ee/routes/teamBroker/3rdPartyBroker_spec.js b/test/unit/forge/ee/routes/teamBroker/3rdPartyBroker_spec.js index 9b3c3ae421..2cf80a3579 100644 --- a/test/unit/forge/ee/routes/teamBroker/3rdPartyBroker_spec.js +++ b/test/unit/forge/ee/routes/teamBroker/3rdPartyBroker_spec.js @@ -1,5 +1,7 @@ const { Op } = require('sequelize') -const should = require('should') // eslint-disable-line +const should = require('should') +const sinon = require('sinon') + const setup = require('../../setup') const FF_UTIL = require('flowforge-test-utils') @@ -806,6 +808,91 @@ describe('3rd Party Broker API', function () { response.statusCode.should.equal(200) }) + it('start keeps a new team-broker agent starting until the driver has launched it', async function () { + let stateDuringLaunch + const startBrokerAgent = sinon.stub(app.containers, 'startBrokerAgent').callsFake(async () => { + stateDuringLaunch = (await app.db.models.TeamBrokerAgent.byTeam(app.team.id)).state + }) + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/start`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(200) + stateDuringLaunch.should.equal('starting') + const agent = await app.db.models.TeamBrokerAgent.byTeam(app.team.id) + agent.state.should.equal('running') + } finally { + startBrokerAgent.restore() + } + }) + + it('start removes the team-broker agent if the driver fails to launch it', async function () { + const startBrokerAgent = sinon.stub(app.containers, 'startBrokerAgent').rejects(new Error('launch failed')) + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/start`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(500) + const agent = await app.db.models.TeamBrokerAgent.byTeam(app.team.id) + should(agent?.state).be.undefined() + } finally { + startBrokerAgent.restore() + } + }) + + it('start replies 409 while the team-broker agent is suspending', async function () { + await app.db.models.TeamBrokerAgent.create({ state: 'suspending', TeamId: app.team.id }) + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/start`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(409) + response.json().should.have.property('code', 'agent_suspending') + }) + + it('suspend replies 409 while the team-broker agent is starting', async function () { + await app.db.models.TeamBrokerAgent.create({ state: 'starting', TeamId: app.team.id }) + const stopBrokerAgent = sinon.spy(app.containers, 'stopBrokerAgent') + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/suspend`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(409) + response.json().should.have.property('code', 'agent_starting') + stopBrokerAgent.called.should.be.false() + const agent = await app.db.models.TeamBrokerAgent.byTeam(app.team.id) + agent.state.should.equal('starting') + } finally { + stopBrokerAgent.restore() + } + }) + + it('suspend marks the team-broker agent suspending before stopping it', async function () { + await app.db.models.TeamBrokerAgent.create({ state: 'running', TeamId: app.team.id }) + let stateDuringStop + const stopBrokerAgent = sinon.stub(app.containers, 'stopBrokerAgent').callsFake(async () => { + stateDuringStop = (await app.db.models.TeamBrokerAgent.byTeam(app.team.id)).state + }) + try { + const response = await app.inject({ + method: 'POST', + url: `/api/v1/teams/${app.team.hashid}/brokers/team-broker/suspend`, + cookies: { sid: TestObjects.tokens.bob } + }) + response.statusCode.should.equal(200) + stateDuringStop.should.equal('suspending') + } finally { + stopBrokerAgent.restore() + } + }) + it('suspend replies not_found when there is no team-broker agent', async function () { const response = await app.inject({ method: 'POST',