-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add retries and retryTimeout
option
#20
Changes from 3 commits
bc1b070
77bbae4
111195f
a0d88ad
6b377f0
f7e973f
baac84e
31de382
3f7a15b
766ee75
f32f7b7
0bb4d36
cba5c68
6eb6f8b
336ce1d
5e03d74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,82 +1,30 @@ | ||
import {getInput, info, saveState, setFailed} from '@actions/core' | ||
import {exec} from '@actions/exec' | ||
import {join} from 'path' | ||
import {tmpdir} from 'os' | ||
import {promises} from 'fs' | ||
import {wait} from './wait' | ||
import optionMappingJson from './option-mapping.json' | ||
import {getInput, saveState, setFailed, warning} from '@actions/core' | ||
import {start} from './start' | ||
|
||
const LOG_FILE = '/srv/sauce-connect.log' | ||
const PID_FILE = '/srv/sauce-connect.pid' | ||
const READY_FILE = '/opt/sauce-connect-action/sc.ready' | ||
|
||
type OptionMapping = { | ||
actionOption: string | ||
dockerOption: string | ||
required?: boolean | ||
flag?: boolean | ||
} | ||
const optionMappings: OptionMapping[] = optionMappingJson | ||
|
||
function buildOptions(): string[] { | ||
const params = [ | ||
`--logfile=${LOG_FILE}`, | ||
`--pidfile=${PID_FILE}`, | ||
`--readyfile=${READY_FILE}`, | ||
`--verbose` | ||
] | ||
|
||
for (const optionMapping of optionMappings) { | ||
const input = getInput(optionMapping.actionOption, { | ||
required: optionMapping.required | ||
}) | ||
|
||
if (input === '') { | ||
// user input nothing for this option | ||
} else if (optionMapping.flag) { | ||
// for flag options like --doctor option | ||
params.push(`--${optionMapping.dockerOption}`) | ||
} else { | ||
params.push(`--${optionMapping.dockerOption}=${input}`) | ||
} | ||
} | ||
return params | ||
} | ||
const retryDelays = [1, 1, 1, 2, 3, 4, 5, 10, 20, 40, 60].map(a => a * 1000) | ||
|
||
async function run(): Promise<void> { | ||
const DIR_IN_HOST = await promises.mkdtemp( | ||
join(tmpdir(), `sauce-connect-action`) | ||
) | ||
const containerVersion = getInput('scVersion') | ||
const containerName = `saucelabs/sauce-connect:${containerVersion}` | ||
try { | ||
await exec('docker', ['pull', containerName]) | ||
let containerId = '' | ||
await exec( | ||
'docker', | ||
[ | ||
'run', | ||
'--network=host', | ||
'--detach', | ||
'-v', | ||
`${DIR_IN_HOST}:/opt/sauce-connect-action`, | ||
'--rm', | ||
containerName | ||
].concat(buildOptions()), | ||
{ | ||
listeners: { | ||
stdout: (data: Buffer) => { | ||
containerId += data.toString() | ||
} | ||
} | ||
const retryTimeout = parseInt(getInput('retryTimeout') || '0', 10) * 1000 | ||
const startTime = Date.now() | ||
|
||
for (let i = 0; ; i++) { | ||
christian-bromann marked this conversation as resolved.
Show resolved
Hide resolved
|
||
try { | ||
const containerId = await start() | ||
saveState('containerId', containerId) | ||
return | ||
} catch (e) { | ||
if (Date.now() - startTime >= retryTimeout) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means by default there will be no retries, correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep I thought we should keep the existing behaviour the same? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am wondering if we should enable retries by default. We expect the tunnel to start with the first try so we wouldn't modify the existing behavior anyway. And having the action to retry rather than fail seems to me like an desired feature. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok cool. Do you think 10 minutes would be a good default? |
||
break | ||
} | ||
) | ||
saveState('containerId', containerId.trim()) | ||
await wait(DIR_IN_HOST) | ||
info('SC ready') | ||
} catch (error) { | ||
setFailed(error.message) | ||
const delay = retryDelays[Math.min(retryDelays.length - 1, i)] | ||
warning( | ||
`Error occurred on attempt ${i + 1}. Retrying in ${delay} ms...` | ||
) | ||
await new Promise(resolve => setTimeout(() => resolve(), delay)) | ||
} | ||
throw new Error('Timed out') | ||
} | ||
} | ||
|
||
run() | ||
// eslint-disable-next-line github/no-then | ||
run().catch(error => setFailed(error.message)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
import {getInput, info} from '@actions/core' | ||
import {exec} from '@actions/exec' | ||
import {join} from 'path' | ||
import {tmpdir} from 'os' | ||
import {promises} from 'fs' | ||
import {wait} from './wait' | ||
import optionMappingJson from './option-mapping.json' | ||
import {stopContainer} from './stop-container' | ||
|
||
const LOG_FILE = '/srv/sauce-connect.log' | ||
const PID_FILE = '/srv/sauce-connect.pid' | ||
const READY_FILE = '/opt/sauce-connect-action/sc.ready' | ||
|
||
type OptionMapping = { | ||
actionOption: string | ||
dockerOption: string | ||
required?: boolean | ||
flag?: boolean | ||
} | ||
const optionMappings: OptionMapping[] = optionMappingJson | ||
|
||
function buildOptions(): string[] { | ||
const params = [ | ||
`--logfile=${LOG_FILE}`, | ||
`--pidfile=${PID_FILE}`, | ||
`--readyfile=${READY_FILE}`, | ||
`--verbose` | ||
] | ||
|
||
for (const optionMapping of optionMappings) { | ||
const input = getInput(optionMapping.actionOption, { | ||
required: optionMapping.required | ||
}) | ||
|
||
if (input === '') { | ||
// user input nothing for this option | ||
} else if (optionMapping.flag) { | ||
// for flag options like --doctor option | ||
params.push(`--${optionMapping.dockerOption}`) | ||
} else { | ||
params.push(`--${optionMapping.dockerOption}=${input}`) | ||
} | ||
} | ||
return params | ||
} | ||
|
||
export async function start(): Promise<string> { | ||
tjenkinson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const DIR_IN_HOST = await promises.mkdtemp( | ||
join(tmpdir(), `sauce-connect-action`) | ||
) | ||
const containerVersion = getInput('scVersion') | ||
const containerName = `saucelabs/sauce-connect:${containerVersion}` | ||
await exec('docker', ['pull', containerName]) | ||
let containerId = '' | ||
await exec( | ||
'docker', | ||
[ | ||
'run', | ||
'--network=host', | ||
'--detach', | ||
'-v', | ||
`${DIR_IN_HOST}:/opt/sauce-connect-action`, | ||
'--rm', | ||
containerName | ||
].concat(buildOptions()), | ||
{ | ||
listeners: { | ||
stdout: (data: Buffer) => { | ||
containerId += data.toString() | ||
} | ||
} | ||
} | ||
) | ||
containerId = containerId.trim() | ||
try { | ||
await wait(DIR_IN_HOST) | ||
info('SC ready') | ||
} finally { | ||
await stopContainer(containerId) | ||
} | ||
return containerId | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import {exec} from '@actions/exec' | ||
import {info} from 'console' | ||
|
||
export async function stopContainer(containerId: string): Promise<void> { | ||
info(`Trying to stop the docker container with ID ${containerId}...`) | ||
await exec('docker', ['container', 'stop', containerId]) | ||
info('Done.') | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vscode auto reformatted this file. Can revert the style changes if you like